Skip to content

Commit 8da04e9

Browse files
mason: fix TUI RPC lifecycle issues
Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 4f34d14 commit 8da04e9

15 files changed

Lines changed: 3774 additions & 67 deletions

packages/plugin/.gitignore

Lines changed: 0 additions & 1 deletion
This file was deleted.

packages/plugin/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
"README.md"
3131
],
3232
"scripts": {
33-
"build": "bun build src/index.ts --outdir dist --target node --format esm --external @opencode-ai/plugin --external @huggingface/transformers --external onnxruntime-web --external bun:sqlite --external node:sqlite && tsc --emitDeclarationOnly",
33+
"build": "bun run build:tui && bun build src/index.ts --outdir dist --target node --format esm --external @opencode-ai/plugin --external @huggingface/transformers --external onnxruntime-web --external bun:sqlite --external node:sqlite && tsc --emitDeclarationOnly",
3434
"build:tui": "bun scripts/build-tui.ts",
35+
"check:tui-compiled": "bun run build:tui && git diff --exit-code -- src/tui-compiled && test -z \"$(git status --porcelain -- src/tui-compiled)\"",
3536
"typecheck": "tsc --noEmit && tsc -p tsconfig.scripts.json",
3637
"test": "bun test",
3738
"lint": "biome check .",

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

Lines changed: 164 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { afterEach, describe, expect, test } from "bun:test";
2-
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
2+
import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
33
import { createServer } from "node:http";
44
import { tmpdir } from "node:os";
55
import { dirname, join } from "node:path";
66
import { MagicContextRpcClient } from "./rpc-client";
7+
import { drainNotifications, isTuiConnected, pushNotification } from "./rpc-notifications";
78
import { MagicContextRpcServer } from "./rpc-server";
8-
import { parseRpcPortFile, rpcPortFilePath } from "./rpc-utils";
9+
import { parseRpcPortFile, type RpcPortFileRecord, rpcPortDir, rpcPortFilePath } from "./rpc-utils";
910

1011
interface TestServer {
1112
port: number;
@@ -56,6 +57,78 @@ function writePortFileForPid(
5657
writeFileSync(portFile, JSON.stringify({ port, pid, started_at: startedAt }), "utf-8");
5758
}
5859

60+
function readNewestPortRecord(storageDir: string, directory: string): RpcPortFileRecord | null {
61+
const records: RpcPortFileRecord[] = [];
62+
for (const entry of readdirSync(rpcPortDir(storageDir, directory))) {
63+
if (!entry.startsWith("port-") || !entry.endsWith(".json")) continue;
64+
const record = parseRpcPortFile(
65+
readFileSync(join(rpcPortDir(storageDir, directory), entry), "utf-8"),
66+
);
67+
if (record) records.push(record);
68+
}
69+
records.sort((a, b) => b.started_at - a.started_at);
70+
return records[0] ?? null;
71+
}
72+
73+
async function waitFor(condition: () => boolean, label: string, timeoutMs = 2_000): Promise<void> {
74+
const start = Date.now();
75+
while (Date.now() - start < timeoutMs) {
76+
if (condition()) return;
77+
await new Promise((resolve) => setTimeout(resolve, 25));
78+
}
79+
throw new Error(`Timed out waiting for ${label}`);
80+
}
81+
82+
async function openSocket(port: number, token: string): Promise<WebSocket> {
83+
const ws = new WebSocket(`ws://127.0.0.1:${port}/ws?token=${encodeURIComponent(token)}`);
84+
await new Promise<void>((resolve, reject) => {
85+
const timeout = setTimeout(() => reject(new Error("socket open timed out")), 2_000);
86+
ws.addEventListener(
87+
"open",
88+
() => {
89+
clearTimeout(timeout);
90+
resolve();
91+
},
92+
{ once: true },
93+
);
94+
ws.addEventListener(
95+
"error",
96+
() => {
97+
clearTimeout(timeout);
98+
reject(new Error("socket open failed"));
99+
},
100+
{ once: true },
101+
);
102+
});
103+
return ws;
104+
}
105+
106+
function waitForJsonMessage<T extends { type?: string }>(
107+
ws: WebSocket,
108+
predicate: (message: T) => boolean,
109+
timeoutMs = 2_000,
110+
): Promise<T> {
111+
return new Promise((resolve, reject) => {
112+
const timeout = setTimeout(() => {
113+
ws.removeEventListener("message", onMessage);
114+
reject(new Error("socket message timed out"));
115+
}, timeoutMs);
116+
const onMessage = (event: MessageEvent) => {
117+
let message: T;
118+
try {
119+
message = JSON.parse(String(event.data)) as T;
120+
} catch {
121+
return;
122+
}
123+
if (!predicate(message)) return;
124+
clearTimeout(timeout);
125+
ws.removeEventListener("message", onMessage);
126+
resolve(message);
127+
};
128+
ws.addEventListener("message", onMessage);
129+
});
130+
}
131+
59132
async function startRpcServer(handler: (method: string) => Response | object): Promise<TestServer> {
60133
const server = createServer(async (req, res) => {
61134
if (req.method === "GET" && req.url === "/health") {
@@ -145,9 +218,7 @@ describe("MagicContextRpcClient", () => {
145218
const port = await server.start();
146219
try {
147220
// Sanity: the port file carries a non-empty token.
148-
const record = parseRpcPortFile(
149-
readFileSync(rpcPortFilePath(storageDir, directory), "utf-8"),
150-
);
221+
const record = readNewestPortRecord(storageDir, directory);
151222
expect(typeof record?.token).toBe("string");
152223
expect((record?.token ?? "").length).toBeGreaterThan(0);
153224

@@ -167,6 +238,94 @@ describe("MagicContextRpcClient", () => {
167238
}
168239
});
169240

241+
test("websocket upgrade rejects missing bearer token before a socket is created", async () => {
242+
const storageDir = makeTempDir();
243+
const directory = "/repo-ws-auth";
244+
const server = new MagicContextRpcServer(storageDir, directory);
245+
const port = await server.start();
246+
try {
247+
const res = await fetch(`http://127.0.0.1:${port}/ws`);
248+
expect(res.status).toBe(401);
249+
expect(isTuiConnected()).toBe(false);
250+
} finally {
251+
server.stop();
252+
}
253+
});
254+
255+
test("re-hello replaces the previous websocket notification sink", async () => {
256+
drainNotifications(Number.MAX_SAFE_INTEGER);
257+
const storageDir = makeTempDir();
258+
const directory = "/repo-ws-rehello";
259+
const server = new MagicContextRpcServer(storageDir, directory);
260+
const port = await server.start();
261+
const record = readNewestPortRecord(storageDir, directory);
262+
expect(typeof record?.token).toBe("string");
263+
264+
const ws = await openSocket(port, record?.token ?? "");
265+
const notifications: unknown[] = [];
266+
ws.addEventListener("message", (event) => {
267+
const message = JSON.parse(String(event.data)) as {
268+
type?: string;
269+
notification?: unknown;
270+
};
271+
if (message.type === "notification") notifications.push(message.notification);
272+
});
273+
274+
try {
275+
ws.send(JSON.stringify({ type: "hello", token: record?.token, sessionId: "ses_A" }));
276+
await waitForJsonMessage(ws, (message) => message.type === "hello-ack");
277+
expect(isTuiConnected("ses_A")).toBe(true);
278+
279+
ws.send(JSON.stringify({ type: "hello", token: record?.token, sessionId: "ses_B" }));
280+
await waitForJsonMessage(ws, (message) => message.type === "hello-ack");
281+
expect(isTuiConnected("ses_A")).toBe(false);
282+
expect(isTuiConnected("ses_B")).toBe(true);
283+
284+
ws.send(JSON.stringify({ type: "hello", token: record?.token, sessionId: "ses_B" }));
285+
await waitForJsonMessage(ws, (message) => message.type === "hello-ack");
286+
pushNotification("live", { ok: true }, "ses_B");
287+
await waitFor(() => notifications.length >= 1, "one live notification");
288+
await new Promise((resolve) => setTimeout(resolve, 50));
289+
expect(notifications).toHaveLength(1);
290+
291+
ws.close();
292+
await waitFor(() => !isTuiConnected(), "socket sink cleanup");
293+
} finally {
294+
try {
295+
ws.close();
296+
} catch {
297+
// best-effort
298+
}
299+
server.stop();
300+
}
301+
});
302+
303+
test("same-process servers keep distinct port files during overlap", async () => {
304+
const storageDir = makeTempDir();
305+
const directory = "/repo-port-collision";
306+
const first = new MagicContextRpcServer(storageDir, directory);
307+
const second = new MagicContextRpcServer(storageDir, directory);
308+
await first.start();
309+
const secondPort = await second.start();
310+
311+
try {
312+
const files = readdirSync(rpcPortDir(storageDir, directory)).filter(
313+
(entry) => entry.startsWith("port-") && entry.endsWith(".json"),
314+
);
315+
expect(files.length).toBeGreaterThanOrEqual(2);
316+
317+
first.stop();
318+
const remaining = readNewestPortRecord(storageDir, directory);
319+
expect(remaining?.port).toBe(secondPort);
320+
321+
const client = new MagicContextRpcClient(storageDir, directory);
322+
expect((await client.resolveEndpoint())?.port).toBe(secondPort);
323+
} finally {
324+
first.stop();
325+
second.stop();
326+
}
327+
});
328+
170329
test("gives up when the port file points at a dead server", async () => {
171330
const storageDir = makeTempDir();
172331
const directory = "/repo";

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

Lines changed: 73 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -107,38 +107,90 @@ export function pushNotification(
107107
}
108108
}
109109

110-
/** Return pending notifications after acking the client's last received id.
110+
export interface DrainNotificationsOptions {
111+
/**
112+
* Cursor for global notifications when a session-scoped client sends separate
113+
* session and global watermarks.
114+
*/
115+
globalLastReceivedId?: number;
116+
/** Ack/drain only the named session, not global notifications. */
117+
sessionOnly?: boolean;
118+
/** Ack/drain only session-less global notifications. */
119+
globalOnly?: boolean;
120+
}
121+
122+
function cursor(value: number | undefined): number {
123+
return typeof value === "number" && Number.isFinite(value) && value > 0 ? value : 0;
124+
}
125+
126+
/** Return pending notifications after pruning only the scopes the client acked.
111127
*
112-
* Session scoping: when `sessionId` is provided, only notifications tagged for
113-
* that session (or session-less/global ones) are returned and pruned — a
114-
* notification tagged for a DIFFERENT session is never handed to this client
115-
* and is never pruned by this client's ack. This matters because the in-memory
116-
* queue is per-process but a TUI can end up bound to a process that also serves
117-
* OTHER sessions: e.g. opening OpenCode Desktop on the same project starts a
118-
* newer RPC server that the TUI's port discovery (newest-pid-wins) then selects,
119-
* so a Desktop-session upgrade-dialog action would otherwise surface in an
120-
* unrelated TUI session. Each client also tracks its own `lastReceivedId`, so a
121-
* global watermark prune would let session A's ack drop session B's still-unseen
122-
* notification — scoping the prune to the acking session prevents that too.
128+
* Session-scoped and global notifications have independent cursors. A TUI can
129+
* switch from session A to session B after handling a high id in A; that high
130+
* watermark must never prune B's lower, still-unseen ids. Global notifications
131+
* are also tracked separately so a global dialog does not become a session
132+
* watermark. Legacy callers that omit options keep the original single-cursor
133+
* behavior.
123134
*
124135
* Delivery is at-least-once (non-destructive return + prune-on-ack): a returned
125-
* notification stays queued until a later call acks it via a higher
126-
* `lastReceivedId`, so a dropped WS socket re-delivers the backlog on reconnect
127-
* (the client sends its `lastReceivedId` in the hello). */
128-
export function drainNotifications(lastReceivedId = 0, sessionId?: string): RpcNotification[] {
136+
* notification stays queued until a later call acks it via the matching scope's
137+
* cursor, so a dropped WS socket re-delivers unhandled backlog on reconnect. */
138+
export function drainNotifications(
139+
lastReceivedId = 0,
140+
sessionId?: string,
141+
options: DrainNotificationsOptions = {},
142+
): RpcNotification[] {
143+
const sessionCursor = cursor(lastReceivedId);
144+
145+
if (options.globalOnly) {
146+
queue = queue.filter(
147+
(notification) =>
148+
notification.sessionId !== undefined || notification.id > sessionCursor,
149+
);
150+
return queue.filter(
151+
(notification) =>
152+
notification.sessionId === undefined && notification.id > sessionCursor,
153+
);
154+
}
155+
156+
if (options.sessionOnly) {
157+
if (sessionId === undefined) return [];
158+
queue = queue.filter(
159+
(notification) =>
160+
notification.sessionId !== sessionId || notification.id > sessionCursor,
161+
);
162+
return queue.filter(
163+
(notification) =>
164+
notification.sessionId === sessionId && notification.id > sessionCursor,
165+
);
166+
}
167+
168+
if (sessionId !== undefined && options.globalLastReceivedId !== undefined) {
169+
const globalCursor = cursor(options.globalLastReceivedId);
170+
queue = queue.filter((notification) => {
171+
if (notification.sessionId === undefined) return notification.id > globalCursor;
172+
if (notification.sessionId === sessionId) return notification.id > sessionCursor;
173+
return true;
174+
});
175+
return queue.filter((notification) => {
176+
if (notification.sessionId === undefined) return notification.id > globalCursor;
177+
return notification.sessionId === sessionId && notification.id > sessionCursor;
178+
});
179+
}
180+
129181
const matchesClient = (notification: RpcNotification): boolean =>
130182
sessionId === undefined ||
131183
notification.sessionId === undefined ||
132184
notification.sessionId === sessionId;
133-
if (lastReceivedId > 0) {
134-
// Prune only notifications THIS client both owns (session-matched) and has
135-
// acked (id <= lastReceivedId). Other sessions' notifications survive.
185+
if (sessionCursor > 0) {
186+
// Legacy single-cursor mode prunes the scopes this client can see. New WS
187+
// clients pass dual cursors above so cross-session watermarks stay isolated.
136188
queue = queue.filter(
137-
(notification) => !(notification.id <= lastReceivedId && matchesClient(notification)),
189+
(notification) => !(notification.id <= sessionCursor && matchesClient(notification)),
138190
);
139191
}
140192
return queue.filter(
141-
(notification) => notification.id > lastReceivedId && matchesClient(notification),
193+
(notification) => notification.id > sessionCursor && matchesClient(notification),
142194
);
143195
}
144196

0 commit comments

Comments
 (0)