From 09dc1f01393c1541481d3b5a61d9586dcfb88a26 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 1 Jul 2026 21:01:23 +0200 Subject: [PATCH] fix(collab): handle pending guest ui requests --- packages/coding-agent/src/collab/host.ts | 9 +++++-- .../test/collab/read-only.test.ts | 22 +++++++++++++++++ packages/collab-web/src/lib/client.ts | 24 +++++++++++++++---- packages/collab-web/test/client.test.ts | 15 ++++++++++++ 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/src/collab/host.ts b/packages/coding-agent/src/collab/host.ts index 4da5b77bf..5fc22f3c1 100644 --- a/packages/coding-agent/src/collab/host.ts +++ b/packages/coding-agent/src/collab/host.ts @@ -123,7 +123,7 @@ export class CollabHost { #unsubscribe?: () => void; #peers = new Map(); #uiReqSeq = 0; - #pendingUi = new Map(); + #pendingUi = new Map(); #lastStateJson = ""; #stateDebounce: Timer | null = null; #streamingInterval: Timer | null = null; @@ -180,7 +180,7 @@ export class CollabHost { const onAbort = (): void => settle(undefined); if (signal?.aborted) return Promise.resolve(undefined); signal?.addEventListener("abort", onAbort, { once: true }); - this.#pendingUi.set(reqId, { resolve: settle }); + this.#pendingUi.set(reqId, { request: fullRequest, resolve: settle }); this.#sendWritablePeers({ t: "ui-request", request: fullRequest }); return promise; } @@ -400,6 +400,11 @@ export class CollabHost { fromPeer, ); this.#sendSnapshotChunks(entries, fromPeer); + if (canWrite) { + for (const pending of this.#pendingUi.values()) { + socket.send({ t: "ui-request", request: pending.request }, fromPeer); + } + } this.#ctx.session.emitNotice( "info", `${cleanName} joined the collab session${canWrite ? "" : " (read-only)"}`, diff --git a/packages/coding-agent/test/collab/read-only.test.ts b/packages/coding-agent/test/collab/read-only.test.ts index 60e54d9f0..26eb1a452 100644 --- a/packages/coding-agent/test/collab/read-only.test.ts +++ b/packages/coding-agent/test/collab/read-only.test.ts @@ -341,6 +341,28 @@ describe("collab read-only links", () => { expect(end).toEqual({ t: "ui-request-end", reqId: request.request.reqId }); }); + it("replays pending host UI requests to writable guests that join later", async () => { + const firstGuest = await joinAsGuest(host.link, "writer-ui-first"); + guestCleanups.push(() => firstGuest.socket.close()); + const firstWelcome = await firstGuest.nextFrame(); + if (firstWelcome.t !== "welcome") throw new Error(`expected welcome, got ${firstWelcome.t}`); + + const pending = host.requestGuestUi({ kind: "editor", title: "Pending?", prefill: "draft" }); + if (!pending) throw new Error("expected writable guest UI request"); + const firstRequest = await firstGuest.nextFrame(); + if (firstRequest.t !== "ui-request") throw new Error(`expected ui-request, got ${firstRequest.t}`); + + const secondGuest = await joinAsGuest(host.link, "writer-ui-second"); + guestCleanups.push(() => secondGuest.socket.close()); + const secondWelcome = await secondGuest.nextFrame(); + if (secondWelcome.t !== "welcome") throw new Error(`expected welcome, got ${secondWelcome.t}`); + const replayed = await secondGuest.nextFrame(); + expect(replayed).toEqual(firstRequest); + + secondGuest.socket.send({ t: "ui-response", reqId: firstRequest.request.reqId, value: "late" }); + expect(await pending).toBe("late"); + }); + it("treats a forged write token as read-only", async () => { const { prompts } = harness; diff --git a/packages/collab-web/src/lib/client.ts b/packages/collab-web/src/lib/client.ts index 4b1b0b22d..4d692427a 100644 --- a/packages/collab-web/src/lib/client.ts +++ b/packages/collab-web/src/lib/client.ts @@ -107,6 +107,7 @@ export class GuestClient { #working = false; #readOnly = false; #uiRequest: CollabUiRequest | null = null; + #uiRequestQueue: CollabUiRequest[] = []; #notices: readonly Notice[] = []; #snapshot: GuestSnapshot; @@ -166,7 +167,7 @@ export class GuestClient { sendUiResponse(reqId: number, value?: CollabUiResponseValue): void { this.#socket.send({ t: "ui-response", reqId, value }); if (this.#uiRequest?.reqId === reqId) { - this.#uiRequest = null; + this.#showNextUiRequest(); this.#commit(); } } @@ -226,7 +227,7 @@ export class GuestClient { pending.resolve(null); } this.#pendingTranscripts.clear(); - this.#uiRequest = null; + this.#clearUiRequests(); this.#commit(); this.#socket.close(); } @@ -284,7 +285,7 @@ export class GuestClient { this.#lifecycle = new Map(); this.#working = frame.state.isStreaming; this.#readOnly = frame.readOnly === true; - this.#uiRequest = null; + this.#clearUiRequests(); this.#welcomed = true; this.#clearWelcomeTimer(); if (frame.entryCount === 0) { @@ -341,10 +342,12 @@ export class GuestClient { } break; case "ui-request": - this.#uiRequest = frame.request; + if (this.#uiRequest) this.#uiRequestQueue = [...this.#uiRequestQueue, frame.request]; + else this.#uiRequest = frame.request; break; case "ui-request-end": - if (this.#uiRequest?.reqId === frame.reqId) this.#uiRequest = null; + if (this.#uiRequest?.reqId === frame.reqId) this.#showNextUiRequest(); + else this.#uiRequestQueue = this.#uiRequestQueue.filter(request => request.reqId !== frame.reqId); break; case "transcript": { const pending = this.#pendingTranscripts.get(frame.reqId); @@ -454,6 +457,17 @@ export class GuestClient { this.#notices = next; } + #clearUiRequests(): void { + this.#uiRequest = null; + this.#uiRequestQueue = []; + } + + #showNextUiRequest(): void { + const [next, ...rest] = this.#uiRequestQueue; + this.#uiRequest = next ?? null; + this.#uiRequestQueue = rest; + } + #buildSnapshot(): GuestSnapshot { return { phase: this.#phase, diff --git a/packages/collab-web/test/client.test.ts b/packages/collab-web/test/client.test.ts index 773f413df..2f92df1ab 100644 --- a/packages/collab-web/test/client.test.ts +++ b/packages/collab-web/test/client.test.ts @@ -270,6 +270,21 @@ describe("GuestClient frame apply", () => { expect(client.getSnapshot().uiRequest).toBeNull(); }); + it("queues overlapping host UI requests until the active one resolves", () => { + const client = liveClient(); + const first = { reqId: 9, kind: "select" as const, title: "First?", options: ["A"] }; + const second = { reqId: 10, kind: "editor" as const, title: "Second?", prefill: "draft" }; + client.applyFrameForTest({ t: "ui-request", request: first }); + client.applyFrameForTest({ t: "ui-request", request: second }); + expect(client.getSnapshot().uiRequest).toEqual(first); + + client.applyFrameForTest({ t: "ui-request-end", reqId: 9 }); + expect(client.getSnapshot().uiRequest).toEqual(second); + + client.applyFrameForTest({ t: "ui-request-end", reqId: 10 }); + expect(client.getSnapshot().uiRequest).toBeNull(); + }); + it("snapshot reference is stable between frames and replaced per frame", () => { const client = liveClient(); const before = client.getSnapshot();