Skip to content

Commit d43f5b2

Browse files
committed
fix(tui): replace 500ms sidebar poll with persistent WebSocket (#200)
The TUI sidebar polled the server plugin every 500ms over localhost HTTP. The TUI runner and server runner are separate Bun contexts in the same process, so each poll opened a fresh loopback TCP connection (Bun's fetch isn't pooled to our server) — ~10ms of client work twice a second, idle, forever. That was the entire source of idle TUI CPU: a few percent on fast machines, ~20% on slower ones, additive per sidebar plugin. Replace the poll with a single persistent WebSocket. The RPC server moves from node:http to Bun.serve, keeping all HTTP request/reply routes unchanged and adding a /ws endpoint; the server pushes each queued notification over it the instant it's queued. isTuiConnected is now exact socket liveness instead of a 3s poll-drain window, so server-initiated dialogs route to the TUI reliably. Bearer-token auth in the hello, per-session scoped delivery, backlog replay on (re)connect, auto-reconnect with port rediscovery. The client keeps a property-read-only session watcher (no network at idle) to re-scope the socket on session switch. Deletes the dead poll path (pending-notifications handler, consumeTuiMessages). Pi is unaffected (it does not import the RPC server/client).
1 parent bdab97c commit d43f5b2

10 files changed

Lines changed: 728 additions & 399 deletions

File tree

.alfonso/release-notes/v0.30.2.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# v0.30.2
2+
3+
A patch release fixing high idle CPU from the TUI sidebar.
4+
5+
## Fixes
6+
7+
- **High idle CPU from the TUI sidebar (#200).** The sidebar polled the plugin every 500ms over a localhost connection. Because the TUI and the plugin run in separate runtimes inside the same process, each poll opened a brand-new loopback connection, so an idle session kept burning CPU continuously (a few percent on fast machines, noticeably more on slower ones, and additive when other sidebar plugins were also installed). The sidebar now holds a single persistent WebSocket to the plugin and receives updates the instant they happen, instead of polling. Idle CPU drops back to baseline. Thanks to @null-axiom for the report and the cross-plugin cross-check that pointed at the shared mechanism.
8+
9+
As part of this, the plugin's detection of whether a TUI is connected is now based on the live connection itself rather than a recent-poll timeout, so server-initiated dialogs (`/ctx-status`, `/ctx-recomp`, the upgrade prompt) route to the TUI reliably instead of depending on timing.

packages/plugin/src/plugin/rpc-handlers.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import {
4444
shouldShowAnnouncement,
4545
} from "../shared/announcement";
4646
import { log } from "../shared/logger";
47-
import { drainNotifications } from "../shared/rpc-notifications";
4847
import type { MagicContextRpcServer } from "../shared/rpc-server";
4948
import type { EmbedDetail, SidebarSnapshot, StatusDetail } from "../shared/rpc-types";
5049
import { applyStickySnapshotCache } from "./sidebar-snapshot-cache";
@@ -914,23 +913,10 @@ export function registerRpcHandlers(
914913
return { toastDurationMs: resolved };
915914
});
916915

917-
rpcServer.handle("pending-notifications", async (params) => {
918-
const lastReceivedId = Number(params.lastReceivedId ?? 0);
919-
// Scope drain to the TUI's active session so a notification tagged for a
920-
// different session (e.g. an upgrade dialog triggered by another client
921-
// sharing this process) is never delivered here. sessionId is optional
922-
// for back-compat: an older TUI that omits it falls back to the previous
923-
// unscoped behavior.
924-
const sessionId =
925-
typeof params.sessionId === "string" && params.sessionId.length > 0
926-
? params.sessionId
927-
: undefined;
928-
const notifications = drainNotifications(
929-
Number.isFinite(lastReceivedId) ? lastReceivedId : 0,
930-
sessionId,
931-
);
932-
return { messages: notifications } as unknown as Record<string, unknown>;
933-
});
916+
// Server→TUI notification delivery is no longer an HTTP poll. The TUI holds a
917+
// persistent WebSocket (rpc-server `/ws`); the server pushes each queued
918+
// notification over it and replays the unacked backlog on the hello. See
919+
// rpc-server.ts + rpc-notifications.ts.
934920

935921
// Startup announcement — called by the TUI plugin once per session to decide
936922
// whether to show the "What's new" dialog. We deliberately read state via

packages/plugin/src/shared/announcement.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ import { getMagicContextStorageDir } from "./data-path";
2424
* Bump only when there are user-visible changes worth a startup dialog.
2525
* Does NOT need to match the published package version.
2626
*/
27-
export const ANNOUNCEMENT_VERSION = "0.30.1";
27+
export const ANNOUNCEMENT_VERSION = "0.30.2";
2828

2929
/**
3030
* Short, user-facing bullet strings. Keep each line ~80 chars or shorter so the
3131
* TUI dialog renders cleanly without horizontal scroll on a typical terminal.
3232
*/
3333
export const ANNOUNCEMENT_FEATURES: ReadonlyArray<string> = [
34-
"Local embeddings work on OpenCode Desktop again (#195): /ctx-embed no longer fails with 'Unsupported device: cpu' on the Desktop app.",
34+
"Fixed high idle CPU from the TUI sidebar (#200): it now uses a single persistent connection to the plugin instead of polling, so an idle session no longer burns CPU.",
3535
];
3636

3737
/**

packages/plugin/src/shared/rpc-client.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,20 @@ export class MagicContextRpcClient {
9797
}
9898
}
9999

100+
/** Resolve the live server's port + bearer token (for opening the WS push
101+
* channel). Reuses the same health-checked port-file discovery as `call`,
102+
* so the WS client and the HTTP client always agree on which server instance
103+
* (and token) to use. Returns null when no live server is found. */
104+
async resolveEndpoint(): Promise<{ port: number; token: string | null } | null> {
105+
try {
106+
const port = await this.resolvePort();
107+
if (port === null) return null;
108+
return { port, token: this.token };
109+
} catch {
110+
return null;
111+
}
112+
}
113+
100114
private async resolvePort(): Promise<number | null> {
101115
if (this.port && this.healthChecked) {
102116
return this.port;

packages/plugin/src/shared/rpc-notifications.test.ts

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { describe, expect, test } from "bun:test";
2-
import { drainNotifications, isTuiConnected, pushNotification } from "./rpc-notifications";
2+
import {
3+
drainNotifications,
4+
isTuiConnected,
5+
type NotificationSink,
6+
pushNotification,
7+
registerNotificationSink,
8+
} from "./rpc-notifications";
39

410
describe("rpc notifications", () => {
511
test("keeps messages queued until the client acks their id", () => {
@@ -45,17 +51,68 @@ describe("rpc notifications", () => {
4551
expect(poll.map((m) => m.type).sort()).toEqual(["x", "y"]);
4652
});
4753

48-
test("isTuiConnected is per-session: a TUI on session A does not mark session B connected", () => {
49-
// A TUI draining for tuiA must not make tuiB's producers think a TUI is
50-
// polling for tuiB (which would route tuiB's /ctx-status, upgrade
51-
// reminder, etc. to the dialog path and lose them in the unrelated TUI).
52-
// Use ids no other test drains so the per-session window is unambiguous.
53-
drainNotifications(0, "ses_tuiA_only");
54-
expect(isTuiConnected("ses_tuiA_only")).toBe(true);
55-
expect(isTuiConnected("ses_tuiB_never_drained")).toBe(false);
56-
// The session-less (global) query still reports recent activity for the
57-
// legacy callers that have no session context.
54+
test("isTuiConnected reflects live WS sinks per-session", () => {
55+
// No sinks → nothing connected.
56+
expect(isTuiConnected("ses_anything")).toBe(false);
57+
expect(isTuiConnected()).toBe(false);
58+
59+
// A live sink scoped to session A marks ONLY A connected (so B's producers
60+
// don't misroute B's /ctx-status / upgrade reminder to the dialog path and
61+
// lose it in an unrelated TUI), and the global query is also "connected".
62+
const unregister = registerNotificationSink({ sessionId: "ses_A", send: () => {} });
63+
expect(isTuiConnected("ses_A")).toBe(true);
64+
expect(isTuiConnected("ses_B")).toBe(false);
5865
expect(isTuiConnected()).toBe(true);
66+
67+
// Closing the socket removes the sink → disconnected again.
68+
unregister();
69+
expect(isTuiConnected("ses_A")).toBe(false);
70+
expect(isTuiConnected()).toBe(false);
71+
});
72+
73+
test("a session-less sink counts as connected for any session query", () => {
74+
const unregister = registerNotificationSink({ sessionId: undefined, send: () => {} });
75+
expect(isTuiConnected("ses_whatever")).toBe(true);
76+
expect(isTuiConnected()).toBe(true);
77+
unregister();
78+
});
79+
80+
test("pushNotification fans out live to a matching sink and skips a foreign session", () => {
81+
drainNotifications(Number.MAX_SAFE_INTEGER);
82+
const received: string[] = [];
83+
const sink: NotificationSink = {
84+
sessionId: "ses_live",
85+
send: (n) => received.push(n.type),
86+
};
87+
const unregister = registerNotificationSink(sink);
88+
89+
pushNotification("for-live", { action: "show-status-dialog" }, "ses_live");
90+
pushNotification("for-other", { action: "show-status-dialog" }, "ses_other");
91+
pushNotification("global", { action: "show-status-dialog" });
92+
93+
// The sink sees its own session + global, never the foreign session.
94+
expect(received.sort()).toEqual(["for-live", "global"]);
95+
unregister();
96+
});
97+
98+
test("a dead sink (throwing send) does not block delivery to other sinks", () => {
99+
drainNotifications(Number.MAX_SAFE_INTEGER);
100+
const live: string[] = [];
101+
const unregDead = registerNotificationSink({
102+
sessionId: undefined,
103+
send: () => {
104+
throw new Error("socket dead");
105+
},
106+
});
107+
const unregLive = registerNotificationSink({
108+
sessionId: undefined,
109+
send: (n) => live.push(n.type),
110+
});
111+
// Must not throw, and the live sink still receives it.
112+
expect(() => pushNotification("resilient", { ok: true })).not.toThrow();
113+
expect(live).toEqual(["resilient"]);
114+
unregDead();
115+
unregLive();
59116
});
60117

61118
test("queue-cap eviction is session-fair: a noisy session cannot evict another session's newest unseen item", () => {

packages/plugin/src/shared/rpc-notifications.ts

Lines changed: 75 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,73 @@ export interface RpcNotification {
1616

1717
let queue: RpcNotification[] = [];
1818
let nextNotificationId = 1;
19-
// Timestamp of last drain — used to detect if a TUI is actively polling.
20-
// The TUI polls every 500ms; we consider it connected if it polled within
21-
// the last 3 seconds (6× the poll interval, tolerates transient delays).
22-
//
23-
// PER-SESSION: a single server process can serve MANY sessions (e.g. a TUI on
24-
// session A plus an OpenCode Desktop opened on session B for the same project,
25-
// whose newer RPC server this TUI's port discovery then selects). The TUI
26-
// poller drains with ITS active session id, so a session is "TUI-connected"
27-
// only if a TUI recently drained FOR THAT session. A process-global timestamp
28-
// would make session B's producers (`/ctx-status`, upgrade reminder) take the
29-
// TUI-dialog path because session A's TUI is polling — queuing a B-scoped
30-
// dialog action that A's poller correctly refuses to show, so B's notice is
31-
// lost (it also suppressed B's non-TUI fallback). Tracking drains per session
32-
// routes each producer to the right delivery path.
33-
const lastDrainAtBySession = new Map<string, number>();
34-
let lastDrainAtAny = 0;
35-
const TUI_CONNECTED_WINDOW_MS = 3_000;
3619

37-
/** Push a notification for TUI to pick up via polling. */
20+
/**
21+
* A connected TUI notification sink — one per authenticated WebSocket. The RPC
22+
* server registers a sink when a TUI socket authenticates (hello) and removes
23+
* it on close. `send` is sink-agnostic (the server owns the actual WS socket)
24+
* so this module stays free of Bun/WS types.
25+
*/
26+
export interface NotificationSink {
27+
/** The TUI's active session at connect time (its hello scope). */
28+
sessionId?: string;
29+
/** Deliver one notification over this sink's live socket. */
30+
send: (notification: RpcNotification) => void;
31+
}
32+
33+
// Live sinks replace the old poll-drain-timestamp inference. "TUI connected for
34+
// a session" is now exact socket liveness — accurate and immediate — instead of
35+
// "did a 500ms poll drain within the last 3s". Per-session scoping still matters:
36+
// one process can serve MANY sessions (a TUI on session A plus an OpenCode
37+
// Desktop opened on session B for the same project, whose newer RPC server this
38+
// TUI's port discovery then selects). Each sink carries ITS session, so a
39+
// B-scoped producer (`/ctx-status`, upgrade reminder) only sees B's TUI as
40+
// connected and routes its dialog there, never to A.
41+
const sinks = new Set<NotificationSink>();
42+
43+
/** Register a live TUI sink. Returns an unregister fn (call on socket close). */
44+
export function registerNotificationSink(sink: NotificationSink): () => void {
45+
sinks.add(sink);
46+
return () => {
47+
sinks.delete(sink);
48+
};
49+
}
50+
51+
/** Whether a given notification may be delivered to a given sink. A global
52+
* notification (no sessionId) reaches every sink; a session-scoped one reaches
53+
* only sinks for that session (or session-less sinks). Mirrors the drain filter
54+
* from the sink's perspective. */
55+
function notificationMatchesSink(notification: RpcNotification, sink: NotificationSink): boolean {
56+
return (
57+
notification.sessionId === undefined ||
58+
sink.sessionId === undefined ||
59+
notification.sessionId === sink.sessionId
60+
);
61+
}
62+
63+
/** Push a notification to the TUI. Fans out to any live WS sink immediately and
64+
* also enqueues it so a TUI that is momentarily disconnected (reconnecting, or
65+
* not yet connected) still receives it on its next hello via the backlog drain.
66+
* At-least-once: a live push that the socket drops is re-delivered from the
67+
* queue on reconnect (pruned only when the client acks via `lastReceivedId`). */
3868
export function pushNotification(
3969
type: string,
4070
payload: Record<string, unknown>,
4171
sessionId?: string,
4272
): void {
43-
queue.push({ id: nextNotificationId++, type, payload, sessionId });
73+
const notification: RpcNotification = { id: nextNotificationId++, type, payload, sessionId };
74+
queue.push(notification);
75+
// Fan out to every live sink this notification is scoped to. A delivery throw
76+
// (dead socket mid-send) must not block other sinks or the caller.
77+
for (const sink of sinks) {
78+
if (!notificationMatchesSink(notification, sink)) continue;
79+
try {
80+
sink.send(notification);
81+
} catch {
82+
// Socket died between liveness check and send; the close handler will
83+
// unregister it, and the queue backlog re-delivers on reconnect.
84+
}
85+
}
4486
// Cap queue size to prevent unbounded growth if a TUI is not draining.
4587
// Session-FAIR eviction: a naive `slice(-50)` drops the globally-oldest
4688
// items, so a noisy session could evict ANOTHER session's single unseen
@@ -66,13 +108,12 @@ export function pushNotification(
66108
}
67109

68110
/** Return pending notifications after acking the client's last received id.
69-
* Updates lastDrainAt so isTuiConnected() reflects recent activity.
70111
*
71112
* Session scoping: when `sessionId` is provided, only notifications tagged for
72113
* that session (or session-less/global ones) are returned and pruned — a
73114
* notification tagged for a DIFFERENT session is never handed to this client
74115
* and is never pruned by this client's ack. This matters because the in-memory
75-
* queue is per-process but a TUI can end up draining a process that also serves
116+
* queue is per-process but a TUI can end up bound to a process that also serves
76117
* OTHER sessions: e.g. opening OpenCode Desktop on the same project starts a
77118
* newer RPC server that the TUI's port discovery (newest-pid-wins) then selects,
78119
* so a Desktop-session upgrade-dialog action would otherwise surface in an
@@ -82,11 +123,9 @@ export function pushNotification(
82123
*
83124
* Delivery is at-least-once (non-destructive return + prune-on-ack): a returned
84125
* notification stays queued until a later call acks it via a higher
85-
* `lastReceivedId`, so a lost poll response re-delivers on the next poll. */
126+
* `lastReceivedId`, so a dropped WS socket re-delivers the backlog on reconnect
127+
* (the client sends its `lastReceivedId` in the hello). */
86128
export function drainNotifications(lastReceivedId = 0, sessionId?: string): RpcNotification[] {
87-
const now = Date.now();
88-
lastDrainAtAny = now;
89-
if (sessionId !== undefined) lastDrainAtBySession.set(sessionId, now);
90129
const matchesClient = (notification: RpcNotification): boolean =>
91130
sessionId === undefined ||
92131
notification.sessionId === undefined ||
@@ -103,20 +142,20 @@ export function drainNotifications(lastReceivedId = 0, sessionId?: string): RpcN
103142
);
104143
}
105144

106-
/** Whether a TUI client is actively polling for notifications.
107-
* Returns true only if a TUI has drained within the last 3 seconds.
145+
/** Whether a TUI client is connected via a live notification socket.
146+
* Now exact socket liveness (a registered WS sink), not a poll-drain timestamp.
108147
*
109-
* Pass `sessionId` (preferred) to ask whether a TUI is polling FOR THAT
148+
* Pass `sessionId` (preferred) to ask whether a TUI is connected FOR THAT
110149
* SESSION — this is what producers (`/ctx-status`, `/ctx-recomp`, the upgrade
111150
* reminder) must use to decide dialog-vs-message, so a TUI on a different
112-
* session in the same process does not misroute their delivery. Omit it only
113-
* for legacy/global callers that genuinely have no session context; they fall
114-
* back to "any session recently drained" (the pre-per-session behavior). */
151+
* session in the same process does not misroute their delivery. A session-less
152+
* sink (legacy/global) counts for any session query. Omit `sessionId` only for
153+
* callers with no session context; they get "any sink connected". */
115154
export function isTuiConnected(sessionId?: string): boolean {
116-
const now = Date.now();
117-
if (sessionId !== undefined) {
118-
const at = lastDrainAtBySession.get(sessionId) ?? 0;
119-
return at > 0 && now - at < TUI_CONNECTED_WINDOW_MS;
155+
if (sinks.size === 0) return false;
156+
if (sessionId === undefined) return true;
157+
for (const sink of sinks) {
158+
if (sink.sessionId === undefined || sink.sessionId === sessionId) return true;
120159
}
121-
return lastDrainAtAny > 0 && now - lastDrainAtAny < TUI_CONNECTED_WINDOW_MS;
160+
return false;
122161
}

0 commit comments

Comments
 (0)