From 644c0e34b1950db92d388a694d0f6c3654e83071 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 08:34:53 +0100 Subject: [PATCH 01/24] test: reproduce cold session load ordering Co-authored-by: bobbit-ai --- ...d-session-workspace-ordering-repro.test.ts | 545 ++++++++++++++++++ tests2/tests-map.json | 9 + 2 files changed, 554 insertions(+) create mode 100644 tests2/dom/cold-session-workspace-ordering-repro.test.ts diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts new file mode 100644 index 000000000..bf81f0a58 --- /dev/null +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -0,0 +1,545 @@ +import { beforeAll as __syncBeforeAll } from "vitest"; +import { syncCustomElements as __syncCE } from "./_setup/custom-elements.js"; +__syncBeforeAll(() => __syncCE()); + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const annotationStoreMocks = vi.hoisted(() => ({ + initAnnotationStore: vi.fn(), +})); + +vi.mock("../../src/ui/components/review/AnnotationStore.js", async (importOriginal) => ({ + ...await importOriginal(), + initAnnotationStore: annotationStoreMocks.initAnnotationStore, +})); + +import { + backToSessions, + connectToSession, + disconnectGateway, + flushAndTeardownDraft, + selectSession, + uncacheSession, +} from "../../src/app/session-manager.js"; +import { RemoteAgent } from "../../src/app/remote-agent.js"; +import { + GW_SESSION_KEY, + GW_TOKEN_KEY, + GW_URL_KEY, + setRenderApp, + state, + type GatewaySession, +} from "../../src/app/state.js"; +import { ChatPanel } from "../../src/ui/ChatPanel.js"; +import { storage } from "../../src/app/storage.js"; +import * as dialogsLazy from "../../src/app/dialogs-lazy.js"; +import * as packEntrypoints from "../../src/app/pack-entrypoints.js"; +import * as packPanels from "../../src/app/pack-panels.js"; +import * as packRenderers from "../../src/app/pack-renderers.js"; +import * as reviewSourcesLazy from "../../src/app/review-sources-lazy.js"; +import { stopPreviewSubscription } from "../../src/app/preview-panel.js"; +import { stopInboxSubscription } from "../../src/app/inbox-panel.js"; + +const SESSION_A = "cold-session-a"; +const SESSION_B = "cold-session-b"; +const TRANSCRIPT_SIZE = 321; +const trackedSessions = [SESSION_A, SESSION_B] as const; + +type TimelineEntry = + | { kind: "ws"; sessionId: string; frame: Record } + | { kind: "rest"; sessionId?: string; path: string; method: string }; + +interface DeferredGate { + promise: Promise; + release: () => void; + settled: boolean; +} + +const timeline: TimelineEntry[] = []; +const workspaceGates = new Map(); +const workspaceFetchCount = new Map(); +const transcripts = new Map(); + +function deferredGate(): DeferredGate { + let releasePromise!: () => void; + const gate: DeferredGate = { + promise: new Promise((resolve) => { releasePromise = resolve; }), + release: () => {}, + settled: false, + }; + gate.release = () => { + if (gate.settled) return; + gate.settled = true; + releasePromise(); + }; + return gate; +} + +function gateFor(sessionId: string): DeferredGate { + let gate = workspaceGates.get(sessionId); + if (!gate) { + gate = deferredGate(); + workspaceGates.set(sessionId, gate); + } + return gate; +} + +function gatewaySession(id: string): GatewaySession { + return { + id, + title: id, + cwd: `/fixture/${id}`, + status: "idle", + createdAt: 1, + lastActivity: 1, + clientCount: 1, + }; +} + +function transcriptFor(sessionId: string): any[] { + return Array.from({ length: TRANSCRIPT_SIZE }, (_, index) => ({ + id: `${sessionId}-message-${index}`, + role: index % 2 === 0 ? "user" : "assistant", + content: `${sessionId} transcript row ${index}`, + timestamp: new Date(1_700_000_000_000 + index).toISOString(), + })); +} + +function workspaceFor(sessionId: string) { + const stamp = sessionId === SESSION_A ? 10 : 20; + return { + version: 1, + sessionId, + revision: stamp, + tabs: [ + { + id: "preview:entry:index.html", + kind: "preview", + title: `${sessionId} preview`, + label: "index.html", + source: { type: "preview", sessionId, entry: "index.html", live: true }, + state: { contentHash: `${sessionId}-preview-hash`, mtime: stamp }, + updatedAt: stamp, + }, + { + id: "proposal:goal", + kind: "proposal", + title: `${sessionId} proposal`, + label: "Goal", + source: { type: "proposal", sessionId, proposalType: "goal" }, + state: { fields: { title: `${sessionId} restored goal`, spec: "restored spec" } }, + updatedAt: stamp, + }, + { + id: `review:${sessionId}-review`, + kind: "review", + title: `${sessionId} review`, + label: "Review", + source: { + type: "review", + sessionId, + documentId: `${sessionId}-review`, + title: `${sessionId} review`, + }, + state: { markdown: `# ${sessionId} review` }, + updatedAt: stamp, + }, + ], + activeTabId: "preview:entry:index.html", + sizeMode: "fullscreen", + metadata: { migratedFromLocalStorageAt: stamp }, + updatedAt: stamp, + }; +} + +function sessionIdFromPath(path: string): string | undefined { + return /^\/api\/sessions\/([^/?]+)/.exec(path)?.[1]; +} + +async function fetchFixture(input: RequestInfo | URL, init?: RequestInit): Promise { + const rawUrl = input instanceof Request ? input.url : String(input); + const url = new URL(rawUrl, "http://localhost"); + const method = (init?.method || (input instanceof Request ? input.method : "GET")).toUpperCase(); + const sessionId = sessionIdFromPath(url.pathname); + timeline.push({ kind: "rest", path: `${url.pathname}${url.search}`, method, ...(sessionId ? { sessionId } : {}) }); + + if (url.pathname.endsWith("/side-panel-workspace") && sessionId) { + workspaceFetchCount.set(sessionId, (workspaceFetchCount.get(sessionId) || 0) + 1); + await gateFor(sessionId).promise; + return Response.json(workspaceFor(sessionId)); + } + if (url.pathname.endsWith("/draft") && method === "GET") return new Response(null, { status: 204 }); + if (url.pathname.endsWith("/git-status")) { + return Response.json({ branch: "master", status: [], clean: true }); + } + if (url.pathname.endsWith("/bg-processes")) return Response.json({ processes: [] }); + if (url.pathname.endsWith("/pr-status")) return new Response(null, { status: 204 }); + if (url.pathname === "/api/preview/mount") return new Response(null, { status: 404 }); + if (url.pathname === "/api/sessions") { + return Response.json({ changed: false, generation: 1, sessions: state.gatewaySessions }); + } + if (url.pathname === "/api/goals") { + return Response.json({ changed: false, generation: 1, goals: [] }); + } + if (url.pathname === "/api/projects") return Response.json({ projects: [] }); + if (method === "DELETE") return new Response(null, { status: 204 }); + return Response.json({ + changed: false, + generation: 1, + sessions: state.gatewaySessions, + goals: [], + projects: [], + entries: [], + processes: [], + proposals: [], + }); +} + +class ControlledWebSocket { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSING = 2; + static readonly CLOSED = 3; + static instances: ControlledWebSocket[] = []; + + readonly sessionId: string; + readyState = ControlledWebSocket.OPEN; + onopen: (() => void) | null = null; + onmessage: ((event: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: (() => void) | null = null; + + constructor(url: string) { + this.sessionId = decodeURIComponent(new URL(url).pathname.split("/").pop() || ""); + ControlledWebSocket.instances.push(this); + queueMicrotask(() => { + if (this.readyState !== ControlledWebSocket.OPEN) return; + this.onopen?.(); + queueMicrotask(() => this.receive({ type: "auth_ok" })); + }); + } + + send(raw: string): void { + const frame = JSON.parse(raw) as Record; + timeline.push({ kind: "ws", sessionId: this.sessionId, frame }); + if (frame.type === "get_messages") { + const data = transcripts.get(this.sessionId) || []; + queueMicrotask(() => this.receive({ type: "messages", data })); + } + } + + receive(frame: Record): void { + if (this.readyState !== ControlledWebSocket.OPEN) return; + this.onmessage?.({ data: JSON.stringify(frame) }); + } + + close(): void { + this.readyState = ControlledWebSocket.CLOSED; + } +} + +function renderTranscript(sessionId: string, agent: any): void { + let host = document.querySelector(`[data-cold-transcript="${sessionId}"]`) as HTMLElement | null; + if (!host) { + document.querySelectorAll("[data-cold-transcript]").forEach((node) => node.remove()); + host = document.createElement("section"); + host.dataset.coldTranscript = sessionId; + document.body.append(host); + } + const fragment = document.createDocumentFragment(); + for (const message of agent.state.messages || []) { + const row = document.createElement("div"); + row.dataset.messageId = message.id; + row.textContent = typeof message.content === "string" ? message.content : JSON.stringify(message.content); + fragment.append(row); + } + host.replaceChildren(fragment); +} + +function installChatPanelHarness(): void { + vi.spyOn(ChatPanel.prototype, "setAgent").mockImplementation(async function (this: ChatPanel, agent: any) { + this.agent = agent; + this.agentInterface = { + session: agent, + projectId: undefined, + cwd: "", + gitRepoKnown: "unknown", + gitStatusLoading: false, + bgProcesses: [], + requestUpdate: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + querySelector: vi.fn(), + } as any; + const sessionId = agent.gatewaySessionId as string; + let scheduled = false; + const draw = () => { + scheduled = false; + renderTranscript(sessionId, agent); + }; + draw(); + agent.subscribe((event: any) => { + if (event?.type !== "message_end" || scheduled) return; + scheduled = true; + queueMicrotask(draw); + }); + }); +} + +async function waitFor(predicate: () => boolean, failure: string): Promise { + for (let attempt = 0; attempt < 250; attempt++) { + if (predicate()) return; + await new Promise((resolve) => setTimeout(resolve, 0)); + } + throw new Error(failure); +} + +function getMessagesIndex(sessionId: string): number { + return timeline.findIndex((entry) => + entry.kind === "ws" && entry.sessionId === sessionId && entry.frame.type === "get_messages"); +} + +function firstSessionRestIndex(sessionId: string): number { + return timeline.findIndex((entry) => entry.kind === "rest" && entry.sessionId === sessionId); +} + +function finalTranscriptRow(sessionId: string): HTMLElement | null { + return document.querySelector( + `[data-cold-transcript="${sessionId}"] [data-message-id="${sessionId}-message-${TRANSCRIPT_SIZE - 1}"]`, + ); +} + +beforeEach(() => { + (window as any).happyDOM?.setURL?.("http://localhost/#/"); + setRenderApp(() => {}); + timeline.length = 0; + workspaceGates.clear(); + workspaceFetchCount.clear(); + ControlledWebSocket.instances.length = 0; + transcripts.clear(); + for (const sessionId of trackedSessions) transcripts.set(sessionId, transcriptFor(sessionId)); + + vi.stubGlobal("WebSocket", ControlledWebSocket); + vi.stubGlobal("fetch", vi.fn(fetchFixture)); + vi.stubGlobal("setInterval", vi.fn(() => 1)); + vi.stubGlobal("clearInterval", vi.fn()); + vi.stubGlobal("requestAnimationFrame", vi.fn((callback: FrameRequestCallback) => { + queueMicrotask(() => callback(performance.now())); + return 1; + })); + vi.stubGlobal("cancelAnimationFrame", vi.fn()); + + installChatPanelHarness(); + vi.spyOn(storage.providerKeys, "set").mockResolvedValue(); + vi.spyOn(dialogsLazy, "showConnectionError").mockImplementation(() => {}); + vi.spyOn(packRenderers, "reconcilePackRenderersForProject").mockResolvedValue(); + vi.spyOn(packPanels, "reconcilePackPanelsForProject").mockResolvedValue(); + vi.spyOn(packEntrypoints, "reconcilePackEntrypointsForProject").mockResolvedValue(); + vi.spyOn(reviewSourcesLazy, "loadReviewSources").mockResolvedValue({ + restorePersistedReviewDocuments: (sessionId: string) => { + if (state.selectedSessionId !== sessionId) return; + const title = `${sessionId} review`; + state.reviewDocuments = new Map([[title, { title, markdown: `# ${title}` } as any]]); + state.reviewActiveTab = title; + state.reviewPanelOpen = true; + }, + } as any); + annotationStoreMocks.initAnnotationStore.mockReset(); + annotationStoreMocks.initAnnotationStore.mockResolvedValue(undefined); + + localStorage.clear(); + sessionStorage.clear(); + localStorage.setItem(GW_URL_KEY, "http://localhost"); + localStorage.setItem(GW_TOKEN_KEY, "fixture-token"); + + state.sessionsGeneration = 1; + state.goalsGeneration = 1; + state.gatewaySessions = trackedSessions.map(gatewaySession); + state.archivedSessions = []; + state.goals = []; + state.projects = []; + state.selectedSessionId = null; + state.connectingSessionId = null; + state.switchGeneration = 0; + state.chatPanel = null; + state.remoteAgent = null; + state.connectionStatus = "disconnected"; + state.appView = "authenticated"; + state.activeProjectId = null; + state.activeProposals = {}; + state.projectProposalAcceptedBySessionId = {}; + state.assistantType = null; + state.assistantTab = "chat"; + state.assistantHasProposal = false; + state.isPreviewSession = false; + state.previewPanelMtime = 0; + state.previewPanelEntry = ""; + state.previewPanelContentHash = ""; + state.previewPanelArtifactId = ""; + state.previewPanelFullscreen = false; + state.previewPanelActiveTab = "preview"; + state.previewPanelTab = "chat"; + state.panelTabsBySession = {}; + state.panelTabs = []; + state.activePanelTabId = "chat"; + state.panelWorkspaceActiveBySession = {}; + state.sidePanelWorkspaceBySession = {}; + state.lastWorkspaceRevisionBySession = {}; + delete (state as any).panelWorkspace; + delete (state as any).__lastSidePanelUserActiveSelection; + state.reviewDocuments = new Map(); + state.reviewActiveTab = ""; + state.reviewPanelOpen = false; + state.inboxEntries = []; + state.inboxPanelOpen = false; + state.inboxAddDialogOpen = false; + state.cwdDropdownOpen = false; + document.body.replaceChildren(); +}); + +afterEach(async () => { + for (const gate of workspaceGates.values()) gate.release(); + await Promise.resolve(); + try { backToSessions(); } catch { /* singleton cleanup */ } + try { flushAndTeardownDraft(); } catch { /* singleton cleanup */ } + try { stopPreviewSubscription(); } catch { /* singleton cleanup */ } + try { stopInboxSubscription(); } catch { /* singleton cleanup */ } + try { disconnectGateway(); } catch { /* singleton cleanup */ } + for (const sessionId of trackedSessions) uncacheSession(sessionId); + state.selectedSessionId = null; + state.connectingSessionId = null; + state.chatPanel = null; + state.remoteAgent = null; + setRenderApp(() => {}); + document.body.replaceChildren(); + localStorage.clear(); + sessionStorage.clear(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe("cold session transcript/workspace ordering", () => { + it("requests and renders 321 transcript rows before the single initial workspace fetch settles", async () => { + const pendingConnect = connectToSession(SESSION_A, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) > 0, + "COLD_SESSION_LOAD_ORDERING_REGRESSION: initial workspace hydration never started", + ); + + try { + const requestIndex = getMessagesIndex(SESSION_A); + expect( + requestIndex, + "COLD_SESSION_LOAD_ORDERING_REGRESSION: get_messages was blocked by pending side-panel workspace hydration", + ).toBeGreaterThanOrEqual(0); + const restIndex = firstSessionRestIndex(SESSION_A); + expect( + requestIndex, + "COLD_SESSION_LOAD_ORDERING_REGRESSION: get_messages must precede every session-scoped REST hydration request", + ).toBeLessThan(restIndex); + expect( + workspaceFetchCount.get(SESSION_A), + "COLD_SESSION_LOAD_ORDERING_REGRESSION: a cold connection must have one initial workspace owner", + ).toBe(1); + + await waitFor( + () => finalTranscriptRow(SESSION_A) !== null, + "COLD_SESSION_LOAD_ORDERING_REGRESSION: the 321st transcript row did not render while workspace hydration was pending", + ); + expect(finalTranscriptRow(SESSION_A)?.textContent).toBe(`${SESSION_A} transcript row ${TRANSCRIPT_SIZE - 1}`); + expect(gateFor(SESSION_A).settled).toBe(false); + } finally { + gateFor(SESSION_A).release(); + await pendingConnect.catch(() => {}); + } + + expect(state.sidePanelWorkspaceBySession[SESSION_A]?.tabs.map((tab) => tab.kind)).toEqual([ + "preview", + "proposal", + "review", + ]); + expect(state.activePanelTabId).toBe("preview:entry:index.html"); + expect(state.previewPanelFullscreen).toBe(true); + expect(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(true); + expect(finalTranscriptRow(SESSION_A)?.textContent).toBe(`${SESSION_A} transcript row ${TRANSCRIPT_SIZE - 1}`); + }); + + it("keeps B foreground mirrors and transcript when A workspace completes after a rapid A to B switch", async () => { + const connectA = connectToSession(SESSION_A, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) > 0, + "STALE_WORKSPACE_FOREGROUND_REGRESSION: A hydration did not start", + ); + + const connectB = connectToSession(SESSION_B, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_B) || 0) > 0, + "STALE_WORKSPACE_FOREGROUND_REGRESSION: B hydration did not start", + ); + gateFor(SESSION_B).release(); + await connectB; + await waitFor( + () => finalTranscriptRow(SESSION_B) !== null, + "STALE_WORKSPACE_FOREGROUND_REGRESSION: B transcript did not render", + ); + + expect((state as any).panelWorkspace?.sessionId).toBe(SESSION_B); + gateFor(SESSION_A).release(); + await connectA; + + expect(state.selectedSessionId).toBe(SESSION_B); + expect(state.remoteAgent?.gatewaySessionId).toBe(SESSION_B); + expect( + (state as any).panelWorkspace?.sessionId, + "STALE_WORKSPACE_FOREGROUND_REGRESSION: abandoned A workspace replaced B's foreground mirror", + ).toBe(SESSION_B); + expect(state.panelTabs.every((tab) => (tab.source as any).sessionId === SESSION_B)).toBe(true); + expect(state.activePanelTabId).toBe("preview:entry:index.html"); + expect(state.previewPanelFullscreen).toBe(true); + expect(state.previewPanelEntry).toBe("index.html"); + expect(state.reviewDocuments.has(`${SESSION_B} review`)).toBe(true); + expect(finalTranscriptRow(SESSION_B)?.textContent).toBe(`${SESSION_B} transcript row ${TRANSCRIPT_SIZE - 1}`); + expect(state.sidePanelWorkspaceBySession[SESSION_A]?.sessionId).toBe(SESSION_A); + }); + + it("retains one reconnect workspace hydration and the zero-seq snapshot fallback", async () => { + state.selectedSessionId = SESSION_A; + const remote = new RemoteAgent() as any; + remote._gatewayUrl = "http://localhost"; + remote._authToken = "fixture-token"; + remote._sessionId = SESSION_A; + + await remote._connectWs(false); + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) === 1, + "RECONNECT_WORKSPACE_REGRESSION: reconnect hydration did not run exactly once", + ); + const frames = timeline + .filter((entry): entry is Extract => entry.kind === "ws" && entry.sessionId === SESSION_A) + .map((entry) => entry.frame.type); + expect(frames).toContain("get_messages"); + expect(frames).toContain("get_state"); + expect(workspaceFetchCount.get(SESSION_A)).toBe(1); + + gateFor(SESSION_A).release(); + remote.disconnect(); + }); + + it("reuses the exact cached panel and agent without another socket or workspace fetch", async () => { + gateFor(SESSION_A).release(); + await connectToSession(SESSION_A, true); + const cachedPanel = state.chatPanel; + const cachedAgent = state.remoteAgent; + const socketCount = ControlledWebSocket.instances.length; + const workspaceCount = workspaceFetchCount.get(SESSION_A); + + selectSession(SESSION_B); + await connectToSession(SESSION_A, true); + + expect(state.chatPanel).toBe(cachedPanel); + expect(state.remoteAgent).toBe(cachedAgent); + expect(ControlledWebSocket.instances).toHaveLength(socketCount); + expect(workspaceFetchCount.get(SESSION_A)).toBe(workspaceCount); + expect(localStorage.getItem(GW_SESSION_KEY)).toBe(SESSION_A); + }); +}); diff --git a/tests2/tests-map.json b/tests2/tests-map.json index a6c8bda7d..1b5d90d35 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -497,6 +497,15 @@ "project": "dom" } }, + { + "path": "tests2/dom/cold-session-workspace-ordering-repro.test.ts", + "reason": "Failing-first deterministic DOM regression around real connectToSession and RemoteAgent handshakes: a controlled WebSocket and delayed side-panel workspace fetch prove get_messages and a 321-row transcript do not wait for REST hydration, initial hydration has one owner, late abandoned-session workspace responses cannot replace foreground mirrors, reconnect hydration remains intact, and cached switch-back creates no new socket or workspace request. Failure token COLD_SESSION_LOAD_ORDERING_REGRESSION.", + "execution": { + "runner": "vitest", + "tier": "unit", + "project": "dom" + } + }, { "path": "tests2/browser/journeys/portrait-session-cache.journey.spec.ts", "reason": "Portrait 375x667 real-harness regression journey for session A→list→B→list→A caching, using a pre-click MutationObserver plus stable pi-chat-panel identity and WebSocket creation counts to prove healthy reuse without a loader; also pins stale cached-socket fresh fallback with transcript hydration, landscape direct-switch parity, reload-only cache loss, and deterministic session cleanup.", From 21ae218830e561d101a4eb615582fdace1c260ee Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 08:41:38 +0100 Subject: [PATCH 02/24] Guard active workspace compatibility mirror Co-authored-by: bobbit-ai --- src/app/side-panel-workspace.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/side-panel-workspace.ts b/src/app/side-panel-workspace.ts index 3e9d6ea50..b8153915a 100644 --- a/src/app/side-panel-workspace.ts +++ b/src/app/side-panel-workspace.ts @@ -354,8 +354,8 @@ function syncCompatibilityMirrors(workspace: SidePanelWorkspace): void { state.previewPanelFullscreen = workspace.sizeMode === "fullscreen"; state.inboxPanelOpen = workspace.tabs.some((tab) => tab.id === INBOX_PANEL_TAB_ID && tab.kind === "inbox"); hydrateActivePreviewMirror(workspace, legacyTabs); + (state as unknown as { panelWorkspace?: SidePanelWorkspace }).panelWorkspace = workspace; } - (state as unknown as { panelWorkspace?: SidePanelWorkspace }).panelWorkspace = workspace; } export function getSidePanelWorkspace(sessionId?: string | null): SidePanelWorkspace { From a842621e58ff102674b2b8fb5d82437f80c0cb36 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 08:42:53 +0100 Subject: [PATCH 03/24] Unblock cold transcript hydration Co-authored-by: bobbit-ai --- src/app/remote-agent.ts | 4 +++- src/app/session-manager.ts | 34 +++++++++++++++------------------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/app/remote-agent.ts b/src/app/remote-agent.ts index 2eef3dced..a040e68f9 100644 --- a/src/app/remote-agent.ts +++ b/src/app/remote-agent.ts @@ -843,7 +843,9 @@ export class RemoteAgent { this._reconnectAttempt = 0; this._setConnectionStatus("connected"); resolve(); - void hydrateSidePanelWorkspace(this._sessionId); + // Initial hydration is owned by connectToSession after ChatPanel + // binding. Reconnects still refresh the server workspace here. + if (!initial) void hydrateSidePanelWorkspace(this._sessionId); // S2: deliver any prompts/steers/retries the user issued while // the socket was reconnecting, before resume/snapshot traffic. this._flushOutbox(); diff --git a/src/app/session-manager.ts b/src/app/session-manager.ts index 228e24694..8e8ad09f1 100644 --- a/src/app/session-manager.ts +++ b/src/app/session-manager.ts @@ -1582,27 +1582,19 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o await remote.connect(url, token, sessionId); if (isStale()) { remote.disconnect(); return; } - installConfirmedSessionModelPersistence(remote, sessionId); - await hydrateSidePanelWorkspace(sessionId); - if (isStale()) { remote.disconnect(); return; } - // ── Fire the transcript snapshot request the INSTANT the WS is - // authenticated — before the refreshSessions()/setAgent() awaits below. - // Profiling (boot-timing) showed those awaits stall the snapshot request - // by ~700ms on a cold reload, even though the server builds the snapshot - // in ~200ms. The reducer applies the snapshot to remote.state - // independently of the ChatPanel binding, so the request can fly in - // parallel with all the connect setup; the panel renders the messages - // when setAgent() binds. Proposal checking is deferred here and released - // by runDeferredProposalCheck() after draft restores complete — so an - // early snapshot can't fill form state before drafts restore. - // (Previously this lived ~250 lines below, AFTER those awaits, which - // defeated the "fire early" intent.) + // ── Fire the transcript snapshot request as the first post-auth action. + // No side-panel, proposal, draft, git, or session-list REST hydration may + // start first. The reducer can apply an early snapshot to remote.state; + // setAgent() below binds and paints it before workspace hydration waits. + // Proposal checking remains deferred until draft restores complete. if (isExisting) { remote.deferProposalCheck(); remote.requestMessages(); } + installConfirmedSessionModelPersistence(remote, sessionId); + // Auto-prompt for new assistant sessions — fire IMMEDIATELY after connect // before any draft-restore awaits that could yield and race const AUTO_PROMPTS: Record = { @@ -2361,6 +2353,11 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o }); if (isStale()) { cleanupRemote(remote); return; } + // Initial workspace hydration has one owner and starts only after the + // transcript-bearing agent is bound, so REST latency cannot delay paint. + await hydrateSidePanelWorkspace(sessionId); + if (isStale()) { cleanupRemote(remote); return; } + // Listen for suggest-goal events from assistant messages state.chatPanel.addEventListener('suggest-goal', () => { if (state.remoteAgent) { @@ -2554,10 +2551,9 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o setHashRoute("session", sessionId, true); } - // (Snapshot request + proposal-check deferral were hoisted to fire - // immediately after connect() resolves — see the block right after - // `await remote.connect(...)` above. Firing here, after the - // refreshSessions()/setAgent() awaits, added ~700ms to every reload.) + // (Snapshot request + proposal-check deferral fire immediately after + // connect() resolves. Initial workspace hydration runs only after setAgent + // binds, so neither the request nor transcript paint waits on REST.) // Initial git status and bg process fetch (fire-and-forget). Seed the // tri-state from the client-side repo-cache so known git-less/empty sessions From eaba2e9aa1311aed7b13fb7ac11e1d94617522e7 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:22:42 +0100 Subject: [PATCH 04/24] test: cover delayed hydration lifecycle races Co-authored-by: bobbit-ai --- ...d-session-workspace-ordering-repro.test.ts | 168 +++++++++++++++++- 1 file changed, 163 insertions(+), 5 deletions(-) diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts index bf81f0a58..be9147733 100644 --- a/tests2/dom/cold-session-workspace-ordering-repro.test.ts +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -39,6 +39,11 @@ import * as packRenderers from "../../src/app/pack-renderers.js"; import * as reviewSourcesLazy from "../../src/app/review-sources-lazy.js"; import { stopPreviewSubscription } from "../../src/app/preview-panel.js"; import { stopInboxSubscription } from "../../src/app/inbox-panel.js"; +import { + clearReviewSubmitted, + isReviewSubmitted, + markReviewSubmitted, +} from "../../src/ui/components/review/AnnotationStore.js"; const SESSION_A = "cold-session-a"; const SESSION_B = "cold-session-b"; @@ -259,7 +264,11 @@ function renderTranscript(sessionId: string, agent: any): void { function installChatPanelHarness(): void { vi.spyOn(ChatPanel.prototype, "setAgent").mockImplementation(async function (this: ChatPanel, agent: any) { this.agent = agent; - this.agentInterface = { + let agentInterface: any; + const composer = { + onSend: async (input: string) => agent.prompt(input), + }; + agentInterface = { session: agent, projectId: undefined, cwd: "", @@ -269,8 +278,14 @@ function installChatPanelHarness(): void { requestUpdate: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), - querySelector: vi.fn(), - } as any; + querySelector: vi.fn((selector: string) => { + if (selector !== "message-editor") return null; + const composerHidden = agentInterface.readOnly + && !(agentInterface.nonInteractive && agent.state.isStreaming); + return composerHidden ? null : composer; + }), + }; + this.agentInterface = agentInterface; const sessionId = agent.gatewaySessionId as string; let scheduled = false; const draw = () => { @@ -337,7 +352,7 @@ beforeEach(() => { vi.spyOn(packEntrypoints, "reconcilePackEntrypointsForProject").mockResolvedValue(); vi.spyOn(reviewSourcesLazy, "loadReviewSources").mockResolvedValue({ restorePersistedReviewDocuments: (sessionId: string) => { - if (state.selectedSessionId !== sessionId) return; + if (state.selectedSessionId !== sessionId || isReviewSubmitted(sessionId)) return; const title = `${sessionId} review`; state.reviewDocuments = new Map([[title, { title, markdown: `# ${title}` } as any]]); state.reviewActiveTab = title; @@ -405,7 +420,10 @@ afterEach(async () => { try { stopPreviewSubscription(); } catch { /* singleton cleanup */ } try { stopInboxSubscription(); } catch { /* singleton cleanup */ } try { disconnectGateway(); } catch { /* singleton cleanup */ } - for (const sessionId of trackedSessions) uncacheSession(sessionId); + for (const sessionId of trackedSessions) { + await clearReviewSubmitted(sessionId); + uncacheSession(sessionId); + } state.selectedSessionId = null; state.connectingSessionId = null; state.chatPanel = null; @@ -502,6 +520,146 @@ describe("cold session transcript/workspace ordering", () => { expect(state.sidePanelWorkspaceBySession[SESSION_A]?.sessionId).toBe(SESSION_A); }); + it("does not reopen a submitted review tab when delayed workspace hydration settles", async () => { + await markReviewSubmitted(SESSION_A); + const pendingConnect = connectToSession(SESSION_A, true); + + try { + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) > 0, + "SUBMITTED_REVIEW_HYDRATION_REGRESSION: delayed workspace hydration did not start", + ); + expect(isReviewSubmitted(SESSION_A)).toBe(true); + expect(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(false); + expect(gateFor(SESSION_A).settled).toBe(false); + + gateFor(SESSION_A).release(); + await pendingConnect; + + expect.soft( + state.sidePanelWorkspaceBySession[SESSION_A]?.tabs.some((tab) => tab.kind === "review"), + "SUBMITTED_REVIEW_HYDRATION_REGRESSION: delayed workspace hydration resurrected a submitted review tab", + ).toBe(false); + expect.soft( + state.panelTabs.some((tab) => tab.kind === "review"), + "SUBMITTED_REVIEW_HYDRATION_REGRESSION: submitted review leaked into foreground panel tabs", + ).toBe(false); + expect.soft(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(false); + expect.soft(state.reviewPanelOpen).toBe(false); + } finally { + gateFor(SESSION_A).release(); + await pendingConnect.catch(() => {}); + await clearReviewSubmitted(SESSION_A); + } + }); + + it("keeps the exact cached A agent and panel connected after A to B to A before A hydration releases", async () => { + const originalConnectA = connectToSession(SESSION_A, true); + let connectB: Promise | undefined; + + try { + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) > 0 && finalTranscriptRow(SESSION_A) !== null, + "CACHE_REACTIVATION_HYDRATION_REGRESSION: original A connection did not bind before hydration", + ); + const cachedPanelA = state.chatPanel; + const cachedAgentA = state.remoteAgent; + expect(cachedPanelA).not.toBeNull(); + expect(cachedAgentA).not.toBeNull(); + expect(gateFor(SESSION_A).settled).toBe(false); + + connectB = connectToSession(SESSION_B, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_B) || 0) > 0, + "CACHE_REACTIVATION_HYDRATION_REGRESSION: B hydration did not start", + ); + gateFor(SESSION_B).release(); + await connectB; + + await connectToSession(SESSION_A, true); + expect(state.chatPanel).toBe(cachedPanelA); + expect(state.remoteAgent).toBe(cachedAgentA); + expect(cachedAgentA?.connected).toBe(true); + + gateFor(SESSION_A).release(); + await originalConnectA; + + expect.soft(state.selectedSessionId).toBe(SESSION_A); + expect.soft( + state.remoteAgent === cachedAgentA, + "CACHE_REACTIVATION_HYDRATION_REGRESSION: stale original A completion detached the reactivated cached agent", + ).toBe(true); + expect.soft( + state.chatPanel === cachedPanelA, + "CACHE_REACTIVATION_HYDRATION_REGRESSION: stale original A completion replaced the reactivated cached panel", + ).toBe(true); + expect.soft(cachedAgentA?.connected).toBe(true); + expect.soft(state.chatPanel?.agent === cachedAgentA).toBe(true); + } finally { + gateFor(SESSION_A).release(); + gateFor(SESSION_B).release(); + await Promise.allSettled([originalConnectA, ...(connectB ? [connectB] : [])]); + } + }); + + it("applies readOnly and nonInteractive restrictions before held hydration and blocks composer submission", async () => { + state.gatewaySessions = state.gatewaySessions.map((session) => { + if (session.id === SESSION_A) return { ...session, readOnly: true }; + if (session.id === SESSION_B) return { ...session, nonInteractive: true }; + return session; + }); + const connectReadOnly = connectToSession(SESSION_A, true, { readOnly: true }); + let connectNonInteractive: Promise | undefined; + + try { + await waitFor( + () => (workspaceFetchCount.get(SESSION_A) || 0) > 0, + "SESSION_RESTRICTION_HYDRATION_REGRESSION: read-only hydration did not start", + ); + const readOnlyInterface = state.chatPanel?.agentInterface as any; + expect(readOnlyInterface).toBeTruthy(); + expect(gateFor(SESSION_A).settled).toBe(false); + + connectNonInteractive = connectToSession(SESSION_B, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_B) || 0) > 0, + "SESSION_RESTRICTION_HYDRATION_REGRESSION: non-interactive hydration did not start", + ); + const nonInteractiveInterface = state.chatPanel?.agentInterface as any; + expect(nonInteractiveInterface).toBeTruthy(); + expect(gateFor(SESSION_B).settled).toBe(false); + + const readOnlyComposer = readOnlyInterface.querySelector("message-editor") as { onSend: (input: string) => Promise } | null; + const nonInteractiveComposer = nonInteractiveInterface.querySelector("message-editor") as { onSend: (input: string) => Promise } | null; + await readOnlyComposer?.onSend("read-only composer must not submit"); + await nonInteractiveComposer?.onSend("non-interactive composer must not submit"); + + expect.soft( + readOnlyInterface.readOnly, + "SESSION_RESTRICTION_HYDRATION_REGRESSION: readOnly was deferred until workspace hydration settled", + ).toBe(true); + expect.soft(readOnlyComposer).toBeNull(); + expect.soft( + nonInteractiveInterface.readOnly, + "SESSION_RESTRICTION_HYDRATION_REGRESSION: nonInteractive readOnly was deferred until workspace hydration settled", + ).toBe(true); + expect.soft(nonInteractiveInterface.nonInteractive).toBe(true); + expect.soft(nonInteractiveComposer).toBeNull(); + for (const sessionId of [SESSION_A, SESSION_B]) { + const promptFrames = timeline.filter((entry) => + entry.kind === "ws" && entry.sessionId === sessionId && entry.frame.type === "prompt"); + expect.soft( + promptFrames, + `SESSION_RESTRICTION_HYDRATION_REGRESSION: ${sessionId} submitted through a restricted composer`, + ).toHaveLength(0); + } + } finally { + gateFor(SESSION_A).release(); + gateFor(SESSION_B).release(); + await Promise.allSettled([connectReadOnly, ...(connectNonInteractive ? [connectNonInteractive] : [])]); + } + }); + it("retains one reconnect workspace hydration and the zero-seq snapshot fallback", async () => { state.selectedSessionId = SESSION_A; const remote = new RemoteAgent() as any; From bb1b5b416da3a1fd1ca025ce35ef0bde46459c64 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:25:49 +0100 Subject: [PATCH 05/24] fix(ws): enforce session write policy Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 76 +++++++++- .../session-ws-write-policy.test.ts | 137 ++++++++++++++++++ tests2/tests-map.json | 9 ++ 3 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 tests2/integration/session-ws-write-policy.test.ts diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index e81e41311..08b1cde89 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -326,6 +326,69 @@ export const MAX_AUTHENTICATED_PROMPT_TEXT_BYTES = 8 * 1024 * 1024; const SESSION_COMMAND_SERIALISER = new SessionCommandSerialiser(); const EXTENSION_CHANNEL_WS_ENVELOPE_TOO_LARGE_MESSAGE = `Extension channel frame exceeds maximum envelope size (${MAX_EXTENSION_CHANNEL_WS_ENVELOPE_BYTES} bytes)`; +type SessionMessageWrite = Extract; + +function isSessionMessageWrite(msg: ClientMessage): msg is SessionMessageWrite { + return msg.type === "prompt" + || msg.type === "steer" + || msg.type === "steer_queued" + || msg.type === "remove_queued" + || msg.type === "reorder_queue"; +} + +/** + * Enforce persisted session interaction policy at the authenticated transport + * boundary. UI flags are presentation only: a crafted frame must not prompt a + * read-only reviewer or start/queue work for an automated reviewer. Persisted + * `true` wins over a stale live field; the live field closes the inverse window + * while a metadata update is being flushed. + */ +function rejectSessionMessageWrite( + ws: WebSocket, + msg: SessionMessageWrite, + session: { status: string; readOnly?: boolean; nonInteractive?: boolean }, + persisted?: { readOnly?: boolean; nonInteractive?: boolean }, +): boolean { + if (session.readOnly === true || persisted?.readOnly === true) { + send(ws, { + type: "error", + message: "This session is read-only and does not accept prompts or queue changes.", + code: "SESSION_READ_ONLY", + }); + return true; + } + + if (session.nonInteractive !== true && persisted?.nonInteractive !== true) return false; + if (msg.type === "prompt") { + send(ws, { + type: "error", + message: "Cannot prompt a non-interactive (automated review) session.", + code: "NON_INTERACTIVE_PROMPT", + }); + return true; + } + if (msg.type === "steer") { + // Automated reviewers intentionally remain redirectable during an active + // stream, but an idle/queued steer would start new reviewer work. + if (session.status === "streaming") return false; + send(ws, { + type: "error", + message: "Cannot enqueue a steered prompt for a non-interactive (automated review) session; steer can only redirect it while streaming.", + code: "NON_INTERACTIVE_STEER", + }); + return true; + } + + send(ws, { + type: "error", + message: "Cannot modify the prompt queue for a non-interactive (automated review) session.", + code: "NON_INTERACTIVE_QUEUE_CONTROL", + }); + return true; +} + type ExtensionChannelClientMessageType = Extract['type']; const EXTENSION_CHANNEL_CLIENT_MESSAGE_TYPES: ReadonlySet = new Set([ @@ -706,7 +769,8 @@ export function handleWebSocketConnection( send(ws, { type: "messages", data: stampSnapshotOrder([]) as unknown[] }); return; case "prompt": - // Allow prompts — they'll be queued by enqueuePrompt since status != idle + // Allow ordinary prompts — they'll be queued by enqueuePrompt since status + // != idle. Session interaction policy is still enforced immediately below. break; case "ext_surface_token": // Pack-bound Host API calls (session-menu launchers, panels) may be clicked @@ -722,6 +786,16 @@ export function handleWebSocketConnection( } } + if ( + isSessionMessageWrite(msg) + && rejectSessionMessageWrite( + ws, + msg, + session, + sessionManager.getPersistedSession(sessionId), + ) + ) return; + try { const mintPackSurfaceToken = (tokenMsg: Extract): { ok: true; token: string } | { ok: false; error: string } => { if (!surfaceTokenAuthorityKey || tokenMsg.surfaceTokenKey !== surfaceTokenAuthorityKey) { diff --git a/tests2/integration/session-ws-write-policy.test.ts b/tests2/integration/session-ws-write-policy.test.ts new file mode 100644 index 000000000..9e455a021 --- /dev/null +++ b/tests2/integration/session-ws-write-policy.test.ts @@ -0,0 +1,137 @@ +import { vi } from "vitest"; +import { expect, test } from "./_e2e/in-process-harness.js"; +import { + connectWs, + createSession, + deleteSession, + type WsConnection, + waitForSessionStatus, +} from "./_e2e/e2e-setup.js"; + +async function expectPolicyError( + conn: WsConnection, + frame: Record, + code: string, +): Promise { + const cursor = conn.messageCount(); + conn.send(frame); + const error = await conn.waitForFrom(cursor, message => message.type === "error", 2_000); + expect(error).toMatchObject({ type: "error", code }); +} + +function persistPolicy( + gateway: any, + sessionId: string, + updates: { readOnly?: boolean; nonInteractive?: boolean }, +): void { + const persisted = gateway.sessionManager.getPersistedSession(sessionId); + expect(persisted, "session has a durable policy row").toBeTruthy(); + expect(persisted.projectId, "session policy row is project-scoped").toEqual(expect.any(String)); + gateway.sessionManager.getSessionStore(persisted.projectId).update(sessionId, updates); +} + +async function expectReadFramesStillWork(conn: WsConnection): Promise { + let cursor = conn.messageCount(); + conn.send({ type: "get_messages" }); + await conn.waitForFrom(cursor, message => message.type === "messages", 2_000); + + cursor = conn.messageCount(); + conn.send({ type: "ping" }); + await conn.waitForFrom(cursor, message => message.type === "pong", 2_000); +} + +const QUEUE_CONTROL_FRAMES: ReadonlyArray> = [ + { type: "steer_queued", messageId: "queued-1" }, + { type: "remove_queued", messageId: "queued-1" }, + { type: "reorder_queue", messageIds: ["queued-1"] }, +]; + +test.describe("authenticated WebSocket session write policy", () => { + test("persisted read-only policy rejects prompts, steers, and every queue control while reads remain available", async ({ gateway }) => { + const sessionId = await createSession(); + await waitForSessionStatus(sessionId, "idle"); + const live = gateway.sessionManager.getSession(sessionId); + expect(live).toBeTruthy(); + expect(live.readOnly).not.toBe(true); + persistPolicy(gateway, sessionId, { readOnly: true }); + + const conn = await connectWs(sessionId); + const enqueuePrompt = vi.spyOn(gateway.sessionManager, "enqueuePrompt"); + const deliverLiveSteer = vi.spyOn(gateway.sessionManager, "deliverLiveSteer"); + const steerQueued = vi.spyOn(gateway.sessionManager, "steerQueued"); + const removeQueued = vi.spyOn(gateway.sessionManager, "removeQueued"); + const reorderQueue = vi.spyOn(gateway.sessionManager, "reorderQueue"); + try { + await expectPolicyError(conn, { type: "prompt", text: "must not run" }, "SESSION_READ_ONLY"); + // A read-only session has no streaming-steer carve-out. + live.status = "streaming"; + await expectPolicyError(conn, { type: "steer", text: "must not redirect" }, "SESSION_READ_ONLY"); + live.status = "idle"; + for (const frame of QUEUE_CONTROL_FRAMES) { + await expectPolicyError(conn, frame, "SESSION_READ_ONLY"); + } + + expect(enqueuePrompt).not.toHaveBeenCalled(); + expect(deliverLiveSteer).not.toHaveBeenCalled(); + expect(steerQueued).not.toHaveBeenCalled(); + expect(removeQueued).not.toHaveBeenCalled(); + expect(reorderQueue).not.toHaveBeenCalled(); + await expectReadFramesStillWork(conn); + } finally { + live.status = "idle"; + enqueuePrompt.mockRestore(); + deliverLiveSteer.mockRestore(); + steerQueued.mockRestore(); + removeQueued.mockRestore(); + reorderQueue.mockRestore(); + conn.close(); + await deleteSession(sessionId); + } + }); + + test("persisted non-interactive policy rejects new work and queue controls but permits a streaming steer", async ({ gateway }) => { + const sessionId = await createSession(); + await waitForSessionStatus(sessionId, "idle"); + const live = gateway.sessionManager.getSession(sessionId); + expect(live).toBeTruthy(); + expect(live.nonInteractive).not.toBe(true); + persistPolicy(gateway, sessionId, { nonInteractive: true }); + + const conn = await connectWs(sessionId); + const enqueuePrompt = vi.spyOn(gateway.sessionManager, "enqueuePrompt"); + const deliverLiveSteer = vi.spyOn(gateway.sessionManager, "deliverLiveSteer").mockResolvedValue(undefined); + const steerQueued = vi.spyOn(gateway.sessionManager, "steerQueued"); + const removeQueued = vi.spyOn(gateway.sessionManager, "removeQueued"); + const reorderQueue = vi.spyOn(gateway.sessionManager, "reorderQueue"); + try { + await expectPolicyError(conn, { type: "prompt", text: "must not start review" }, "NON_INTERACTIVE_PROMPT"); + await expectPolicyError(conn, { type: "steer", text: "must not queue review" }, "NON_INTERACTIVE_STEER"); + for (const frame of QUEUE_CONTROL_FRAMES) { + await expectPolicyError(conn, frame, "NON_INTERACTIVE_QUEUE_CONTROL"); + } + + expect(enqueuePrompt).not.toHaveBeenCalled(); + expect(deliverLiveSteer).not.toHaveBeenCalled(); + expect(steerQueued).not.toHaveBeenCalled(); + expect(removeQueued).not.toHaveBeenCalled(); + expect(reorderQueue).not.toHaveBeenCalled(); + await expectReadFramesStillWork(conn); + + live.status = "streaming"; + conn.send({ type: "steer", text: "redirect active review" }); + await vi.waitFor(() => { + expect(deliverLiveSteer).toHaveBeenCalledWith(sessionId, "redirect active review"); + }, { timeout: 2_000 }); + expect(enqueuePrompt).not.toHaveBeenCalled(); + } finally { + live.status = "idle"; + enqueuePrompt.mockRestore(); + deliverLiveSteer.mockRestore(); + steerQueued.mockRestore(); + removeQueued.mockRestore(); + reorderQueue.mockRestore(); + conn.close(); + await deleteSession(sessionId); + } + }); +}); diff --git a/tests2/tests-map.json b/tests2/tests-map.json index 1b5d90d35..d5e4649b1 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -506,6 +506,15 @@ "project": "dom" } }, + { + "path": "tests2/integration/session-ws-write-policy.test.ts", + "reason": "Authenticated WebSocket integration coverage proving persisted read-only and non-interactive session policy is enforced before prompt or queue mutation, while snapshot/ping reads and intentional streaming-only reviewer steer remain available.", + "execution": { + "runner": "vitest", + "tier": "unit", + "project": "integration" + } + }, { "path": "tests2/browser/journeys/portrait-session-cache.journey.spec.ts", "reason": "Portrait 375x667 real-harness regression journey for session A→list→B→list→A caching, using a pre-click MutationObserver plus stable pi-chat-panel identity and WebSocket creation counts to prove healthy reuse without a loader; also pins stale cached-socket fresh fallback with transcript hydration, landscape direct-switch parity, reload-only cache loss, and deterministic session cleanup.", From 32c75ff2944ce881a6c7b204b127b0f36063f24c Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:26:35 +0100 Subject: [PATCH 06/24] Fix delayed hydration lifecycle races Co-authored-by: bobbit-ai --- src/app/remote-agent.ts | 24 +++++++-- src/app/session-manager.ts | 104 +++++++++++++++++++++++++++---------- 2 files changed, 97 insertions(+), 31 deletions(-) diff --git a/src/app/remote-agent.ts b/src/app/remote-agent.ts index a040e68f9..b8926b288 100644 --- a/src/app/remote-agent.ts +++ b/src/app/remote-agent.ts @@ -713,6 +713,19 @@ export class RemoteAgent { get gatewaySessionId() { return this._sessionId; } + /** + * Remove review tabs restored from the server after this session's review + * was already submitted. Initial/reconnect workspace hydration can finish + * after transcript replay performed the same cleanup against an empty local + * workspace, so callers replay it at the workspace-apply boundary. + * + * This intentionally has no active-session guard: cached/background sessions + * still own their keyed workspace and must not retain a submitted review tab. + */ + reconcileSubmittedReviewWorkspace(): void { + if (!this._sessionId || !isReviewSubmitted(this._sessionId)) return; + closeReviewWorkspaceTabs(undefined, { sessionId: this._sessionId, select: false }); + } private _isActiveSession(): boolean { return this._sessionId !== "" && state.selectedSessionId === this._sessionId; } @@ -844,8 +857,13 @@ export class RemoteAgent { this._setConnectionStatus("connected"); resolve(); // Initial hydration is owned by connectToSession after ChatPanel - // binding. Reconnects still refresh the server workspace here. - if (!initial) void hydrateSidePanelWorkspace(this._sessionId); + // binding. Reconnects still refresh the server workspace here and + // then replay submitted-review cleanup against the hydrated tabs. + if (!initial) { + void hydrateSidePanelWorkspace(this._sessionId).then(() => { + this.reconcileSubmittedReviewWorkspace(); + }); + } // S2: deliver any prompts/steers/retries the user issued while // the socket was reconnecting, before resume/snapshot traffic. this._flushOutbox(); @@ -1780,7 +1798,7 @@ export class RemoteAgent { if (!this._isActiveSession()) break; } } else { - closeReviewWorkspaceTabs(undefined, { sessionId: reviewSessionId, select: false }); + this.reconcileSubmittedReviewWorkspace(); } if (this._isActiveSession()) { const reviewSources = await loadReviewSources(); diff --git a/src/app/session-manager.ts b/src/app/session-manager.ts index 8e8ad09f1..e6a18fc0d 100644 --- a/src/app/session-manager.ts +++ b/src/app/session-manager.ts @@ -168,6 +168,32 @@ interface CachedSession { const sessionCache = new Map(); const SESSION_CACHE_MAX = 10; +type InteractionModeTarget = { + readOnly?: boolean; + nonInteractive?: boolean; +}; + +/** Apply every interaction restriction already known without ever clearing one. */ +function applyKnownSessionInteractionMode( + target: InteractionModeTarget, + records: ReadonlyArray | undefined>, + options?: { explicitReadOnly?: boolean; remoteArchived?: boolean }, +): void { + const nonInteractive = records.some((record) => record?.nonInteractive === true); + if (nonInteractive) target.nonInteractive = true; + if ( + options?.explicitReadOnly + || options?.remoteArchived + || nonInteractive + || records.some((record) => record?.readOnly === true + || record?.archived === true + || record?.status === "archived" + || record?.status === "terminated") + ) { + target.readOnly = true; + } +} + function cacheSession(sessionId: string, chatPanel: ChatPanel, remoteAgent: RemoteAgent): void { sessionCache.set(sessionId, { chatPanel, remoteAgent, cachedAt: Date.now() }); // Evict oldest if over limit @@ -1372,6 +1398,16 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o sessionCache.delete(sessionId); // Take ownership state.chatPanel = cached.chatPanel; state.remoteAgent = cached.remoteAgent; + // The cached panel becomes visible as soon as ownership is restored. Apply + // explicit/list-backed restrictions before any await in this fast path. + const cachedKnownSession = state.gatewaySessions.find((s) => s.id === sessionId) + || state.archivedSessions.find((s) => s.id === sessionId); + if (state.chatPanel.agentInterface) { + applyKnownSessionInteractionMode(state.chatPanel.agentInterface, [cachedKnownSession], { + explicitReadOnly: options?.readOnly === true, + remoteArchived: cached.remoteAgent.state?.isArchived === true, + }); + } state.remoteAgent.registerHostApiTransports(); state.connectionStatus = "connected"; state.connectingSessionId = null; @@ -1474,9 +1510,10 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o ai.delegateOf = s.delegateOf; ai.teamGoalId = s.teamGoalId; ai.assistantType = s.assistantType; - if (s.archived || s.status === "terminated" || s.status === "archived") { - ai.readOnly = true; - } + applyKnownSessionInteractionMode(ai, [s], { + explicitReadOnly: options?.readOnly === true, + remoteArchived: cached.remoteAgent.state?.isArchived === true, + }); }; applyFrom(sessionData); const archivedMatch: any = state.archivedSessions?.find((s: any) => s.id === sessionId); @@ -1560,12 +1597,15 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o // Phase 2: async hydrate const gen = state.switchGeneration; const isStale = () => state.switchGeneration !== gen; - // Only null out state.remoteAgent if it's still OUR remote instance. - // A concurrent connectToSession() for a different session may have already - // replaced it — blindly nulling would wipe the newer session's agent. + // A stale connect invocation may have transferred its now-bound remote into + // the cache, or that exact remote may already be active again after A→B→A. + // In either case ownership outlives this invocation and cleanup must not + // disconnect it. Only truly abandoned instances are disposable here. const cleanupRemote = (remote: RemoteAgent) => { + const retained = state.remoteAgent === remote + || [...sessionCache.values()].some((entry) => entry.remoteAgent === remote); + if (retained) return; remote.disconnect(); - if (state.remoteAgent === remote) state.remoteAgent = null; }; state.connectingSessionId = sessionId; @@ -2341,21 +2381,38 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o if (sessionData?.staffId) startInboxSubscription(sessionId, sessionData.staffId); else stopInboxSubscription(); - // Bind draft autosave before setAgent can render an interactive editor; - // setAgent awaits lazy imports after assigning agentInterface, so Lit can - // paint the textarea before setAgent returns under load. + // Resolve every list-backed restriction already available before setAgent; + // a direct-record fetch remains a best-effort fallback after hydration. + let sessionDataAny: any = sessionData + || state.archivedSessions?.find((s: any) => s.id === sessionId); + + // Bind draft autosave before setAgent can render an interactive editor. _bindPromptDraftSession(sessionId); // ── Bind the agent to the early ChatPanel (created before connect - // to show the "Connecting…" shell instantly). - await state.chatPanel!.setAgent(remote as any, { + // to show the "Connecting…" shell instantly). setAgent assigns its + // AgentInterface synchronously before its first lazy-import await. Apply + // known restrictions in that same task, before Lit can paint a composer. + const bindingPanel = state.chatPanel!; + const setAgentPromise = bindingPanel.setAgent(remote as any, { onApiKeyRequired: async () => true, }); + if (bindingPanel.agentInterface) { + applyKnownSessionInteractionMode(bindingPanel.agentInterface, [sessionDataAny], { + explicitReadOnly: options?.readOnly === true, + remoteArchived: remote.state.isArchived === true, + }); + } + await setAgentPromise; if (isStale()) { cleanupRemote(remote); return; } // Initial workspace hydration has one owner and starts only after the // transcript-bearing agent is bound, so REST latency cannot delay paint. await hydrateSidePanelWorkspace(sessionId); + // Transcript replay may have observed the submitted-review marker before + // any server tabs existed locally. Replay that cleanup after hydration, + // including for a now-cached session, before stale-invocation teardown. + remote.reconcileSubmittedReviewWorkspace(); if (isStale()) { cleanupRemote(remote); return; } // Listen for suggest-goal events from assistant messages @@ -2370,8 +2427,6 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o // gatewaySessions — look there as a fallback so the scope-gate fields // (goalId / delegateOf / teamGoalId / assistantType) that gate the // "Continue in new session" footer are threaded through. - let sessionDataAny: any = sessionData - || state.archivedSessions?.find((s: any) => s.id === sessionId); // Archived sessions re-opened directly (no prior sidebar load) aren't in // either cache — fetch the record so the scope-gate fields threaded to // AgentInterface (goalId / delegateOf / teamGoalId / assistantType) are @@ -2398,20 +2453,13 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o } } - // Disable input for archived or explicitly read-only sessions. - // Also honour the REST session record — when re-opening a terminated - // session directly, remote.state.isArchived hasn't settled yet but the - // PersistedSession's `archived` / `status=="terminated"` flags are - // authoritative. - const recordArchived = !!sessionDataAny && (sessionDataAny.archived || sessionDataAny.status === "terminated"); - if (state.chatPanel.agentInterface && (remote.state.isArchived || recordArchived || options?.readOnly)) { - state.chatPanel.agentInterface.readOnly = true; - } - - // Non-interactive sessions (e.g. verification reviewers) — show editor only while streaming (steer-only) - if (state.chatPanel.agentInterface && sessionData?.nonInteractive) { - state.chatPanel.agentInterface.readOnly = true; - state.chatPanel.agentInterface.nonInteractive = true; + // Reconcile any record discovered by the fallback fetch. The common case + // already applied these flags synchronously during setAgent above. + if (state.chatPanel.agentInterface) { + applyKnownSessionInteractionMode(state.chatPanel.agentInterface, [sessionDataAny], { + explicitReadOnly: options?.readOnly === true, + remoteArchived: remote.state.isArchived === true, + }); } // Set up bg process kill/dismiss handlers From d3c17668255fadcb3eb0a058d33e27a8a345af9e Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:32:13 +0100 Subject: [PATCH 07/24] Reconcile submitted review after hydration Co-authored-by: bobbit-ai --- src/app/remote-agent.ts | 47 +++++++++++++++++++++++++++++++------- src/app/session-manager.ts | 2 +- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/app/remote-agent.ts b/src/app/remote-agent.ts index b8926b288..b62bff07e 100644 --- a/src/app/remote-agent.ts +++ b/src/app/remote-agent.ts @@ -49,7 +49,7 @@ import { showFaviconBadge } from "./favicon-badge.js"; import { isEffectivePlayFinishSoundEnabled, type FinishSoundSource } from "./play-finish-sound.js"; import { needsHumanAttentionOnIdleTransition, needsImmediateHumanAttention } from "./notification-policy.js"; import { scheduleGateStatusRefreshForGoal, refreshSessions, scheduleSessionListRefreshFromPush, scheduleStaffListRefreshFromPush } from "./remote-agent-refresh.js"; -import { applySidePanelWorkspaceFromServer, getSidePanelWorkspace, hydrateSidePanelWorkspace } from "./side-panel-workspace.js"; +import { applySidePanelWorkspaceFromServer, closeSidePanelTab, getSidePanelWorkspace, hydrateSidePanelWorkspace } from "./side-panel-workspace.js"; import { shouldRefreshGateStatusForEvent } from "./gate-status-events.js"; import { publishClientMessage, publishClientStatus } from "./session-event-bus.js"; import { registerSessionPoster, unregisterSessionPoster, type SessionPostRequest } from "./session-write-bridge.js"; @@ -722,9 +722,41 @@ export class RemoteAgent { * This intentionally has no active-session guard: cached/background sessions * still own their keyed workspace and must not retain a submitted review tab. */ - reconcileSubmittedReviewWorkspace(): void { - if (!this._sessionId || !isReviewSubmitted(this._sessionId)) return; - closeReviewWorkspaceTabs(undefined, { sessionId: this._sessionId, select: false }); + async reconcileSubmittedReviewWorkspace(): Promise { + const sessionId = this._sessionId; + if (!sessionId || !isReviewSubmitted(sessionId)) return; + const isReviewTab = (tab: { id?: string; kind?: string }): boolean => + tab.kind === "review" || tab.id?.startsWith("review:") === true; + const reviewTabIds = getSidePanelWorkspace(sessionId).tabs + .filter(isReviewTab) + .map((tab) => tab.id); + if (reviewTabIds.length === 0) return; + + // Use the normal mutation path first so successful deletes retain all + // server revision/conflict semantics. Some compatible endpoints return + // 204 for DELETE; the workspace helper then refetches and may receive a + // stale pre-delete snapshot. Reapply the submitted-review policy to that + // settled snapshot below without inventing a new revision. + for (const tabId of reviewTabIds) { + try { + await closeSidePanelTab(tabId, { sessionId }); + } catch (err) { + console.warn("[RemoteAgent] submitted-review workspace cleanup failed:", err); + } + } + if (!isReviewSubmitted(sessionId)) return; + const settled = getSidePanelWorkspace(sessionId); + const tabs = settled.tabs.filter((tab) => !isReviewTab(tab)); + if (tabs.length === settled.tabs.length) return; + const activeTabId = tabs.some((tab) => tab.id === settled.activeTabId) + ? settled.activeTabId + : tabs[0]?.id || ""; + applySidePanelWorkspaceFromServer({ + ...settled, + tabs, + activeTabId, + updatedAt: Date.now(), + }, { source: "rest", force: true }); } private _isActiveSession(): boolean { return this._sessionId !== "" && state.selectedSessionId === this._sessionId; @@ -860,9 +892,8 @@ export class RemoteAgent { // binding. Reconnects still refresh the server workspace here and // then replay submitted-review cleanup against the hydrated tabs. if (!initial) { - void hydrateSidePanelWorkspace(this._sessionId).then(() => { - this.reconcileSubmittedReviewWorkspace(); - }); + void hydrateSidePanelWorkspace(this._sessionId) + .then(() => this.reconcileSubmittedReviewWorkspace()); } // S2: deliver any prompts/steers/retries the user issued while // the socket was reconnecting, before resume/snapshot traffic. @@ -1798,7 +1829,7 @@ export class RemoteAgent { if (!this._isActiveSession()) break; } } else { - this.reconcileSubmittedReviewWorkspace(); + await this.reconcileSubmittedReviewWorkspace(); } if (this._isActiveSession()) { const reviewSources = await loadReviewSources(); diff --git a/src/app/session-manager.ts b/src/app/session-manager.ts index e6a18fc0d..864c5b809 100644 --- a/src/app/session-manager.ts +++ b/src/app/session-manager.ts @@ -2412,7 +2412,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o // Transcript replay may have observed the submitted-review marker before // any server tabs existed locally. Replay that cleanup after hydration, // including for a now-cached session, before stale-invocation teardown. - remote.reconcileSubmittedReviewWorkspace(); + await remote.reconcileSubmittedReviewWorkspace(); if (isStale()) { cleanupRemote(remote); return; } // Listen for suggest-goal events from assistant messages From 1fa272eb67dc78a360ade693e1aa7e2680198d90 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 09:36:47 +0100 Subject: [PATCH 08/24] test: fix cached agent identity typecheck Co-authored-by: bobbit-ai --- tests2/dom/cold-session-workspace-ordering-repro.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts index be9147733..a2cfd5e23 100644 --- a/tests2/dom/cold-session-workspace-ordering-repro.test.ts +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -594,7 +594,10 @@ describe("cold session transcript/workspace ordering", () => { "CACHE_REACTIVATION_HYDRATION_REGRESSION: stale original A completion replaced the reactivated cached panel", ).toBe(true); expect.soft(cachedAgentA?.connected).toBe(true); - expect.soft(state.chatPanel?.agent === cachedAgentA).toBe(true); + expect.soft( + state.chatPanel?.agent as unknown, + "CACHE_REACTIVATION_HYDRATION_REGRESSION: reactivated panel no longer references the exact cached RemoteAgent", + ).toBe(cachedAgentA); } finally { gateFor(SESSION_A).release(); gateFor(SESSION_B).release(); From 3feba080533ea64806fa1d468ffbd24f5fc4a7df Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:09:35 +0100 Subject: [PATCH 09/24] Guard stale session setup continuations Co-authored-by: bobbit-ai --- src/app/session-manager.ts | 41 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/app/session-manager.ts b/src/app/session-manager.ts index 864c5b809..69fefbcb6 100644 --- a/src/app/session-manager.ts +++ b/src/app/session-manager.ts @@ -1386,6 +1386,19 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o // Phase 1: synchronous select selectSession(sessionId, replaceHistory); + // Every connection-setup continuation must still own this selection before + // mutating foreground state or capturing the active ChatPanel. Exact remotes + // already transferred into the cache (or reactivated after A→B→A) remain + // owned and must not be disconnected by an older invocation winding down. + const gen = state.switchGeneration; + const isStale = () => state.switchGeneration !== gen || state.selectedSessionId !== sessionId; + const cleanupRemote = (remote: RemoteAgent) => { + const retained = state.remoteAgent === remote + || [...sessionCache.values()].some((entry) => entry.remoteAgent === remote); + if (retained) return; + remote.disconnect(); + }; + // Fast path: reuse cached session (instant switch-back). If the cached agent's // WebSocket is no longer open, drop it and fall through to a fresh connect so // pack-bound Host API surfaces can re-register their trusted WS minter. @@ -1440,8 +1453,11 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o let sessionData = state.gatewaySessions.find((s) => s.id === sessionId); if (!sessionData) { await refreshSessions().catch(() => { /* fall back to cached list */ }); + if (isStale()) { cleanupRemote(cached.remoteAgent); return; } sessionData = state.gatewaySessions.find((s) => s.id === sessionId); } + const reviewSources = await loadReviewSources(); + if (isStale()) { cleanupRemote(cached.remoteAgent); return; } state.assistantType = sessionData?.assistantType || (sessionData?.goalAssistant ? "goal" : sessionData?.roleAssistant ? "role" @@ -1485,7 +1501,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o state.reviewDocuments = new Map(); state.reviewActiveTab = ""; state.reviewPanelOpen = false; - (await loadReviewSources()).restorePersistedReviewDocuments(sessionId, { select: true }); + reviewSources.restorePersistedReviewDocuments(sessionId, { select: true }); state.inboxEntries = []; if (state.isPreviewSession) startPreviewPolling(); else stopPreviewPolling(); @@ -1595,19 +1611,6 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o void reconcilePackEntrypointsForProject(sessionForPalette?.projectId).catch(() => {}); // Phase 2: async hydrate - const gen = state.switchGeneration; - const isStale = () => state.switchGeneration !== gen; - // A stale connect invocation may have transferred its now-bound remote into - // the cache, or that exact remote may already be active again after A→B→A. - // In either case ownership outlives this invocation and cleanup must not - // disconnect it. Only truly abandoned instances are disposable here. - const cleanupRemote = (remote: RemoteAgent) => { - const retained = state.remoteAgent === remote - || [...sessionCache.values()].some((entry) => entry.remoteAgent === remote); - if (retained) return; - remote.disconnect(); - }; - state.connectingSessionId = sessionId; // Show the chat UI shell immediately with a "Connecting..." state @@ -1621,7 +1624,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o const remote = new RemoteAgent(); await remote.connect(url, token, sessionId); - if (isStale()) { remote.disconnect(); return; } + if (isStale()) { cleanupRemote(remote); return; } // ── Fire the transcript snapshot request as the first post-auth action. // No side-panel, proposal, draft, git, or session-list REST hydration may @@ -1665,6 +1668,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o } else { autoPrompt = AUTO_PROMPTS[options.assistantType]; } + if (isStale()) { cleanupRemote(remote); return; } // The kickoff is the assistant's FIRST message. Flag it non-title-generating // so auto-naming keys off the first GENUINE user message, not the kickoff. if (autoPrompt) remote.prompt(autoPrompt, undefined, { suppressTitleGen: true }); @@ -2342,7 +2346,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o origDisconnect(); }; - if (isStale()) { remote.disconnect(); return; } + if (isStale()) { cleanupRemote(remote); return; } state.connectionStatus = "connected"; state.remoteAgent = remote; @@ -2356,8 +2360,11 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o let sessionData = state.gatewaySessions.find((s) => s.id === sessionId); if (!sessionData) { await refreshSessions().catch(() => { /* fall back to cached list */ }); + if (isStale()) { cleanupRemote(remote); return; } sessionData = state.gatewaySessions.find((s) => s.id === sessionId); } + const reviewSources = await loadReviewSources(); + if (isStale()) { cleanupRemote(remote); return; } state.assistantType = options?.assistantType || sessionData?.assistantType || (options?.isGoalAssistant || sessionData?.goalAssistant ? "goal" @@ -2374,7 +2381,7 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o state.reviewDocuments = new Map(); state.reviewActiveTab = ""; state.reviewPanelOpen = false; - (await loadReviewSources()).restorePersistedReviewDocuments(sessionId, { select: true }); + reviewSources.restorePersistedReviewDocuments(sessionId, { select: true }); state.inboxEntries = []; if (state.isPreviewSession) startPreviewPolling(); else stopPreviewPolling(); From 9e51a1b8f5f037f28abba4eb2d8c8c47969fd824 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:14:19 +0100 Subject: [PATCH 10/24] Close restricted session WS bypasses Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 154 +++++++++++++----- .../session-ws-write-policy.test.ts | 102 +++++++++++- 2 files changed, 203 insertions(+), 53 deletions(-) diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 08b1cde89..76c2a50fc 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -326,66 +326,127 @@ export const MAX_AUTHENTICATED_PROMPT_TEXT_BYTES = 8 * 1024 * 1024; const SESSION_COMMAND_SERIALISER = new SessionCommandSerialiser(); const EXTENSION_CHANNEL_WS_ENVELOPE_TOO_LARGE_MESSAGE = `Extension channel frame exceeds maximum envelope size (${MAX_EXTENSION_CHANNEL_WS_ENVELOPE_BYTES} bytes)`; -type SessionMessageWrite = Extract; -function isSessionMessageWrite(msg: ClientMessage): msg is SessionMessageWrite { - return msg.type === "prompt" - || msg.type === "steer" - || msg.type === "steer_queued" - || msg.type === "remove_queued" - || msg.type === "reorder_queue"; +function isSessionWorkMessage(msg: ClientMessage): msg is SessionWorkMessage { + switch (msg.type) { + case "prompt": + case "steer": + case "steer_queued": + case "remove_queued": + case "reorder_queue": + case "retry": + case "restart_agent": + case "compact": + case "grant_tool_permission": + case "ext_session_write_permit": + case "ext_session_post": + return true; + default: + return false; + } +} + +function sendSessionWorkPolicyError( + ws: WebSocket, + msg: SessionWorkMessage, + code: string, + message: string, +): void { + // Extension session-write callers wait for their request-correlated result; + // a generic error frame would leave the Host API call hanging until timeout. + if (msg.type === "ext_session_write_permit") { + const requestId = typeof msg.requestId === "string" ? msg.requestId : ""; + send(ws, { type: "ext_session_write_permit_result", requestId, ok: false, error: code }); + return; + } + if (msg.type === "ext_session_post") { + const requestId = typeof msg.requestId === "string" ? msg.requestId : ""; + send(ws, { type: "ext_session_post_result", requestId, ok: false, error: code }); + return; + } + send(ws, { type: "error", message, code }); } /** * Enforce persisted session interaction policy at the authenticated transport - * boundary. UI flags are presentation only: a crafted frame must not prompt a - * read-only reviewer or start/queue work for an automated reviewer. Persisted - * `true` wins over a stale live field; the live field closes the inverse window - * while a metadata update is being flushed. + * boundary. UI flags are presentation only: a crafted frame must not enqueue, + * redirect, retry, restart, compact, grant-and-resume, or extension-post work + * in a restricted session. Stop/deny controls remain available because they + * only halt active work. Persisted `true` wins over a stale live field; the live + * field closes the inverse window while a metadata update is being flushed. */ -function rejectSessionMessageWrite( +function rejectRestrictedSessionWork( ws: WebSocket, - msg: SessionMessageWrite, + msg: SessionWorkMessage, session: { status: string; readOnly?: boolean; nonInteractive?: boolean }, persisted?: { readOnly?: boolean; nonInteractive?: boolean }, ): boolean { if (session.readOnly === true || persisted?.readOnly === true) { - send(ws, { - type: "error", - message: "This session is read-only and does not accept prompts or queue changes.", - code: "SESSION_READ_ONLY", - }); + sendSessionWorkPolicyError( + ws, + msg, + "SESSION_READ_ONLY", + "This session is read-only and does not accept agent work commands.", + ); return true; } if (session.nonInteractive !== true && persisted?.nonInteractive !== true) return false; if (msg.type === "prompt") { - send(ws, { - type: "error", - message: "Cannot prompt a non-interactive (automated review) session.", - code: "NON_INTERACTIVE_PROMPT", - }); + sendSessionWorkPolicyError( + ws, + msg, + "NON_INTERACTIVE_PROMPT", + "Cannot prompt a non-interactive (automated review) session.", + ); return true; } if (msg.type === "steer") { // Automated reviewers intentionally remain redirectable during an active // stream, but an idle/queued steer would start new reviewer work. if (session.status === "streaming") return false; - send(ws, { - type: "error", - message: "Cannot enqueue a steered prompt for a non-interactive (automated review) session; steer can only redirect it while streaming.", - code: "NON_INTERACTIVE_STEER", - }); + sendSessionWorkPolicyError( + ws, + msg, + "NON_INTERACTIVE_STEER", + "Cannot enqueue a steered prompt for a non-interactive (automated review) session; steer can only redirect it while streaming.", + ); + return true; + } + if ( + msg.type === "steer_queued" + || msg.type === "remove_queued" + || msg.type === "reorder_queue" + ) { + sendSessionWorkPolicyError( + ws, + msg, + "NON_INTERACTIVE_QUEUE_CONTROL", + "Cannot modify the prompt queue for a non-interactive (automated review) session.", + ); return true; } - send(ws, { - type: "error", - message: "Cannot modify the prompt queue for a non-interactive (automated review) session.", - code: "NON_INTERACTIVE_QUEUE_CONTROL", - }); + sendSessionWorkPolicyError( + ws, + msg, + "NON_INTERACTIVE_WORK_CONTROL", + "Cannot start or alter agent work for a non-interactive (automated review) session.", + ); return true; } @@ -751,6 +812,19 @@ export function handleWebSocketConnection( return; } + // Enforce durable interaction policy before lifecycle-specific routing so + // restricted work frames always receive the same policy response, including + // while the agent is preparing or starting. + if ( + isSessionWorkMessage(msg) + && rejectRestrictedSessionWork( + ws, + msg, + session, + sessionManager.getPersistedSession(sessionId), + ) + ) return; + // Block commands while session is still preparing (worktree being created) // or starting (agent process launched but setup commands still running). // Return safe responses for read-only commands; let prompts through to @@ -770,7 +844,7 @@ export function handleWebSocketConnection( return; case "prompt": // Allow ordinary prompts — they'll be queued by enqueuePrompt since status - // != idle. Session interaction policy is still enforced immediately below. + // != idle. Restricted-session policy was already enforced above. break; case "ext_surface_token": // Pack-bound Host API calls (session-menu launchers, panels) may be clicked @@ -786,16 +860,6 @@ export function handleWebSocketConnection( } } - if ( - isSessionMessageWrite(msg) - && rejectSessionMessageWrite( - ws, - msg, - session, - sessionManager.getPersistedSession(sessionId), - ) - ) return; - try { const mintPackSurfaceToken = (tokenMsg: Extract): { ok: true; token: string } | { ok: false; error: string } => { if (!surfaceTokenAuthorityKey || tokenMsg.surfaceTokenKey !== surfaceTokenAuthorityKey) { diff --git a/tests2/integration/session-ws-write-policy.test.ts b/tests2/integration/session-ws-write-policy.test.ts index 9e455a021..675dcf8d5 100644 --- a/tests2/integration/session-ws-write-policy.test.ts +++ b/tests2/integration/session-ws-write-policy.test.ts @@ -19,6 +19,49 @@ async function expectPolicyError( expect(error).toMatchObject({ type: "error", code }); } +async function expectExtensionPolicyError( + conn: WsConnection, + frame: Record & { requestId: string }, + resultType: "ext_session_write_permit_result" | "ext_session_post_result", + code: string, +): Promise { + const cursor = conn.messageCount(); + conn.send(frame); + const result = await conn.waitForFrom( + cursor, + message => message.type === resultType && message.requestId === frame.requestId, + 2_000, + ); + expect(result).toMatchObject({ + type: resultType, + requestId: frame.requestId, + ok: false, + error: code, + }); +} + +async function expectExtensionWritesRejected( + conn: WsConnection, + code: string, + requestPrefix: string, +): Promise { + await expectExtensionPolicyError(conn, { + type: "ext_session_write_permit", + requestId: `${requestPrefix}-permit`, + surfaceToken: "crafted-surface-token", + contentHash: "0".repeat(64), + }, "ext_session_write_permit_result", code); + await expectExtensionPolicyError(conn, { + type: "ext_session_post", + requestId: `${requestPrefix}-post`, + surfaceToken: "crafted-surface-token", + role: "user", + text: "must not reach the agent", + resumeTurn: true, + nonce: "crafted-write-permit", + }, "ext_session_post_result", code); +} + function persistPolicy( gateway: any, sessionId: string, @@ -32,6 +75,10 @@ function persistPolicy( async function expectReadFramesStillWork(conn: WsConnection): Promise { let cursor = conn.messageCount(); + conn.send({ type: "get_state" }); + await conn.waitForFrom(cursor, message => message.type === "state", 2_000); + + cursor = conn.messageCount(); conn.send({ type: "get_messages" }); await conn.waitForFrom(cursor, message => message.type === "messages", 2_000); @@ -46,8 +93,15 @@ const QUEUE_CONTROL_FRAMES: ReadonlyArray> = [ { type: "reorder_queue", messageIds: ["queued-1"] }, ]; +const WORK_CONTROL_FRAMES: ReadonlyArray> = [ + { type: "retry" }, + { type: "restart_agent" }, + { type: "compact" }, + { type: "grant_tool_permission", toolName: "bash", scope: "tool", mode: "session-only" }, +]; + test.describe("authenticated WebSocket session write policy", () => { - test("persisted read-only policy rejects prompts, steers, and every queue control while reads remain available", async ({ gateway }) => { + test("persisted read-only policy rejects every agent work frame while reads remain available", async ({ gateway }) => { const sessionId = await createSession(); await waitForSessionStatus(sessionId, "idle"); const live = gateway.sessionManager.getSession(sessionId); @@ -61,21 +115,30 @@ test.describe("authenticated WebSocket session write policy", () => { const steerQueued = vi.spyOn(gateway.sessionManager, "steerQueued"); const removeQueued = vi.spyOn(gateway.sessionManager, "removeQueued"); const reorderQueue = vi.spyOn(gateway.sessionManager, "reorderQueue"); + const retryLastPrompt = vi.spyOn(gateway.sessionManager, "retryLastPrompt").mockRejectedValue(new Error("policy guard missed retry")); + const restartAgent = vi.spyOn(gateway.sessionManager, "restartAgent").mockRejectedValue(new Error("policy guard missed restart")); + const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); + const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); try { await expectPolicyError(conn, { type: "prompt", text: "must not run" }, "SESSION_READ_ONLY"); // A read-only session has no streaming-steer carve-out. live.status = "streaming"; await expectPolicyError(conn, { type: "steer", text: "must not redirect" }, "SESSION_READ_ONLY"); live.status = "idle"; - for (const frame of QUEUE_CONTROL_FRAMES) { + for (const frame of [...QUEUE_CONTROL_FRAMES, ...WORK_CONTROL_FRAMES]) { await expectPolicyError(conn, frame, "SESSION_READ_ONLY"); } + await expectExtensionWritesRejected(conn, "SESSION_READ_ONLY", "read-only"); expect(enqueuePrompt).not.toHaveBeenCalled(); expect(deliverLiveSteer).not.toHaveBeenCalled(); expect(steerQueued).not.toHaveBeenCalled(); expect(removeQueued).not.toHaveBeenCalled(); expect(reorderQueue).not.toHaveBeenCalled(); + expect(retryLastPrompt).not.toHaveBeenCalled(); + expect(restartAgent).not.toHaveBeenCalled(); + expect(grantToolPermission).not.toHaveBeenCalled(); + expect(compact).not.toHaveBeenCalled(); await expectReadFramesStillWork(conn); } finally { live.status = "idle"; @@ -84,12 +147,16 @@ test.describe("authenticated WebSocket session write policy", () => { steerQueued.mockRestore(); removeQueued.mockRestore(); reorderQueue.mockRestore(); + retryLastPrompt.mockRestore(); + restartAgent.mockRestore(); + grantToolPermission.mockRestore(); + compact.mockRestore(); conn.close(); await deleteSession(sessionId); } }); - test("persisted non-interactive policy rejects new work and queue controls but permits a streaming steer", async ({ gateway }) => { + test("persisted non-interactive policy permits only a direct streaming steer among agent work frames", async ({ gateway }) => { const sessionId = await createSession(); await waitForSessionStatus(sessionId, "idle"); const live = gateway.sessionManager.getSession(sessionId); @@ -103,18 +170,20 @@ test.describe("authenticated WebSocket session write policy", () => { const steerQueued = vi.spyOn(gateway.sessionManager, "steerQueued"); const removeQueued = vi.spyOn(gateway.sessionManager, "removeQueued"); const reorderQueue = vi.spyOn(gateway.sessionManager, "reorderQueue"); + const retryLastPrompt = vi.spyOn(gateway.sessionManager, "retryLastPrompt").mockRejectedValue(new Error("policy guard missed retry")); + const restartAgent = vi.spyOn(gateway.sessionManager, "restartAgent").mockRejectedValue(new Error("policy guard missed restart")); + const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); + const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); try { await expectPolicyError(conn, { type: "prompt", text: "must not start review" }, "NON_INTERACTIVE_PROMPT"); await expectPolicyError(conn, { type: "steer", text: "must not queue review" }, "NON_INTERACTIVE_STEER"); for (const frame of QUEUE_CONTROL_FRAMES) { await expectPolicyError(conn, frame, "NON_INTERACTIVE_QUEUE_CONTROL"); } + for (const frame of WORK_CONTROL_FRAMES) { + await expectPolicyError(conn, frame, "NON_INTERACTIVE_WORK_CONTROL"); + } - expect(enqueuePrompt).not.toHaveBeenCalled(); - expect(deliverLiveSteer).not.toHaveBeenCalled(); - expect(steerQueued).not.toHaveBeenCalled(); - expect(removeQueued).not.toHaveBeenCalled(); - expect(reorderQueue).not.toHaveBeenCalled(); await expectReadFramesStillWork(conn); live.status = "streaming"; @@ -122,7 +191,20 @@ test.describe("authenticated WebSocket session write policy", () => { await vi.waitFor(() => { expect(deliverLiveSteer).toHaveBeenCalledWith(sessionId, "redirect active review"); }, { timeout: 2_000 }); + // Streaming only carves out the direct steer frame. Alternate retry and + // extension redirect/enqueue paths stay forbidden. + await expectPolicyError(conn, { type: "retry" }, "NON_INTERACTIVE_WORK_CONTROL"); + await expectExtensionWritesRejected(conn, "NON_INTERACTIVE_WORK_CONTROL", "non-interactive-streaming"); + expect(enqueuePrompt).not.toHaveBeenCalled(); + expect(deliverLiveSteer).toHaveBeenCalledTimes(1); + expect(steerQueued).not.toHaveBeenCalled(); + expect(removeQueued).not.toHaveBeenCalled(); + expect(reorderQueue).not.toHaveBeenCalled(); + expect(retryLastPrompt).not.toHaveBeenCalled(); + expect(restartAgent).not.toHaveBeenCalled(); + expect(grantToolPermission).not.toHaveBeenCalled(); + expect(compact).not.toHaveBeenCalled(); } finally { live.status = "idle"; enqueuePrompt.mockRestore(); @@ -130,6 +212,10 @@ test.describe("authenticated WebSocket session write policy", () => { steerQueued.mockRestore(); removeQueued.mockRestore(); reorderQueue.mockRestore(); + retryLastPrompt.mockRestore(); + restartAgent.mockRestore(); + grantToolPermission.mockRestore(); + compact.mockRestore(); conn.close(); await deleteSession(sessionId); } From f8a64093b15bc114c45a889426669e8869c22b97 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:15:27 +0100 Subject: [PATCH 11/24] test: cover pre-bind stale navigation Co-authored-by: bobbit-ai --- ...d-session-workspace-ordering-repro.test.ts | 129 +++++++++++++++++- 1 file changed, 125 insertions(+), 4 deletions(-) diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts index a2cfd5e23..b31cec4fb 100644 --- a/tests2/dom/cold-session-workspace-ordering-repro.test.ts +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -64,6 +64,9 @@ const timeline: TimelineEntry[] = []; const workspaceGates = new Map(); const workspaceFetchCount = new Map(); const transcripts = new Map(); +let sessionListGate: DeferredGate | null = null; +let sessionListFetchCount = 0; +let sessionListResponseSessions: GatewaySession[] | null = null; function deferredGate(): DeferredGate { let releasePromise!: () => void; @@ -181,6 +184,12 @@ async function fetchFixture(input: RequestInfo | URL, init?: RequestInit): Promi if (url.pathname.endsWith("/pr-status")) return new Response(null, { status: 204 }); if (url.pathname === "/api/preview/mount") return new Response(null, { status: 404 }); if (url.pathname === "/api/sessions") { + sessionListFetchCount++; + const delayedList = sessionListGate; + if (delayedList && sessionListFetchCount === 1) await delayedList.promise; + if (sessionListResponseSessions) { + return Response.json({ changed: true, generation: 2, sessions: sessionListResponseSessions }); + } return Response.json({ changed: false, generation: 1, sessions: state.gatewaySessions }); } if (url.pathname === "/api/goals") { @@ -314,8 +323,19 @@ function getMessagesIndex(sessionId: string): number { entry.kind === "ws" && entry.sessionId === sessionId && entry.frame.type === "get_messages"); } -function firstSessionRestIndex(sessionId: string): number { - return timeline.findIndex((entry) => entry.kind === "rest" && entry.sessionId === sessionId); +const FIXTURE_BOOTSTRAP_REST_PATHS = new Set([ + "/api/preview/mount", +]); + +function isRelevantHydrationRestPath(path: string): boolean { + const pathname = path.split("?", 1)[0]; + if (FIXTURE_BOOTSTRAP_REST_PATHS.has(pathname)) return false; + if (pathname === "/api/sessions" || pathname === "/api/projects" || pathname === "/api/goals") return true; + return /^\/api\/sessions\/[^/]+\/(?:side-panel-workspace|draft|git-status)$/.test(pathname); +} + +function firstRelevantRestIndex(): number { + return timeline.findIndex((entry) => entry.kind === "rest" && isRelevantHydrationRestPath(entry.path)); } function finalTranscriptRow(sessionId: string): HTMLElement | null { @@ -330,6 +350,9 @@ beforeEach(() => { timeline.length = 0; workspaceGates.clear(); workspaceFetchCount.clear(); + sessionListGate = null; + sessionListFetchCount = 0; + sessionListResponseSessions = null; ControlledWebSocket.instances.length = 0; transcripts.clear(); for (const sessionId of trackedSessions) transcripts.set(sessionId, transcriptFor(sessionId)); @@ -414,6 +437,7 @@ beforeEach(() => { afterEach(async () => { for (const gate of workspaceGates.values()) gate.release(); + sessionListGate?.release(); await Promise.resolve(); try { backToSessions(); } catch { /* singleton cleanup */ } try { flushAndTeardownDraft(); } catch { /* singleton cleanup */ } @@ -450,10 +474,14 @@ describe("cold session transcript/workspace ordering", () => { requestIndex, "COLD_SESSION_LOAD_ORDERING_REGRESSION: get_messages was blocked by pending side-panel workspace hydration", ).toBeGreaterThanOrEqual(0); - const restIndex = firstSessionRestIndex(SESSION_A); + const restIndex = firstRelevantRestIndex(); + expect( + restIndex, + "COLD_SESSION_LOAD_ORDERING_REGRESSION: the fixture must observe a relevant REST hydration request", + ).toBeGreaterThanOrEqual(0); expect( requestIndex, - "COLD_SESSION_LOAD_ORDERING_REGRESSION: get_messages must precede every session-scoped REST hydration request", + "COLD_SESSION_LOAD_ORDERING_REGRESSION: get_messages must precede the first relevant REST request globally (session-list, projects, goals, workspace, draft, or git)", ).toBeLessThan(restIndex); expect( workspaceFetchCount.get(SESSION_A), @@ -520,6 +548,99 @@ describe("cold session transcript/workspace ordering", () => { expect(state.sidePanelWorkspaceBySession[SESSION_A]?.sessionId).toBe(SESSION_A); }); + it("does not let A cross a delayed session-list await and bind into B's foreground state", async () => { + const sessionA = { ...gatewaySession(SESSION_A), assistantType: "goal" }; + const sessionB = { ...gatewaySession(SESSION_B), assistantType: "support" }; + state.gatewaySessions = [sessionB]; + sessionListResponseSessions = [sessionA, sessionB]; + const delayedSessionList = deferredGate(); + sessionListGate = delayedSessionList; + + const connectA = connectToSession(SESSION_A, true); + let connectB: Promise | undefined; + + try { + await waitFor( + () => sessionListFetchCount === 1, + "PRE_BIND_STALE_NAVIGATION_REGRESSION: A never reached delayed session-list hydration", + ); + expect(getMessagesIndex(SESSION_A)).toBeGreaterThanOrEqual(0); + expect(delayedSessionList.settled).toBe(false); + expect(gateFor(SESSION_A).settled).toBe(false); + + const abandonedPanelA = state.chatPanel; + expect((abandonedPanelA?.agent as any)?.gatewaySessionId).toBeUndefined(); + connectB = connectToSession(SESSION_B, true); + await waitFor( + () => (workspaceFetchCount.get(SESSION_B) || 0) > 0, + "PRE_BIND_STALE_NAVIGATION_REGRESSION: B workspace hydration did not start", + ); + gateFor(SESSION_B).release(); + await connectB; + await waitFor( + () => finalTranscriptRow(SESSION_B) !== null, + "PRE_BIND_STALE_NAVIGATION_REGRESSION: B transcript did not render", + ); + + const panelB = state.chatPanel; + const remoteB = state.remoteAgent; + expect(panelB).not.toBe(abandonedPanelA); + expect(panelB?.agent as unknown).toBe(remoteB); + expect(state.reviewDocuments.has(`${SESSION_B} review`)).toBe(true); + + const inboxB = [{ id: "b-inbox-entry", state: "pending", title: "B inbox" }] as any[]; + state.inboxEntries = inboxB; + state.inboxPanelOpen = true; + state.assistantType = "support"; + state.assistantTab = "preview"; + state.assistantHasProposal = true; + + delayedSessionList.release(); + await connectA; + + expect.soft(state.remoteAgent, "PRE_BIND_STALE_NAVIGATION_REGRESSION: delayed A replaced B's remote").toBe(remoteB); + expect.soft(state.chatPanel, "PRE_BIND_STALE_NAVIGATION_REGRESSION: delayed A replaced B's ChatPanel").toBe(panelB); + expect.soft( + (panelB?.agent as any)?.gatewaySessionId, + "PRE_BIND_STALE_NAVIGATION_REGRESSION: delayed A bound into B's ChatPanel", + ).toBe(SESSION_B); + expect.soft({ + selectedSessionId: state.selectedSessionId, + visibleBTranscript: finalTranscriptRow(SESSION_B)?.textContent, + visibleATranscript: document.querySelector(`[data-cold-transcript="${SESSION_A}"]`) !== null, + panelWorkspaceSessionId: (state as any).panelWorkspace?.sessionId, + panelTabsOwnedByB: state.panelTabs.every((tab) => (tab.source as any).sessionId === SESSION_B), + reviewDocumentTitles: [...state.reviewDocuments.keys()], + reviewActiveTab: state.reviewActiveTab, + reviewPanelOpen: state.reviewPanelOpen, + inboxEntryIds: state.inboxEntries.map((entry) => entry.id), + inboxPanelOpen: state.inboxPanelOpen, + assistantType: state.assistantType, + assistantTab: state.assistantTab, + assistantHasProposal: state.assistantHasProposal, + }, "PRE_BIND_STALE_NAVIGATION_REGRESSION: delayed A overwrote B's panel, review, inbox, or assistant state").toEqual({ + selectedSessionId: SESSION_B, + visibleBTranscript: `${SESSION_B} transcript row ${TRANSCRIPT_SIZE - 1}`, + visibleATranscript: false, + panelWorkspaceSessionId: SESSION_B, + panelTabsOwnedByB: true, + reviewDocumentTitles: [`${SESSION_B} review`], + reviewActiveTab: `${SESSION_B} review`, + reviewPanelOpen: true, + inboxEntryIds: inboxB.map((entry) => entry.id), + inboxPanelOpen: true, + assistantType: "support", + assistantTab: "preview", + assistantHasProposal: true, + }); + } finally { + delayedSessionList.release(); + gateFor(SESSION_A).release(); + gateFor(SESSION_B).release(); + await Promise.allSettled([connectA, ...(connectB ? [connectB] : [])]); + } + }); + it("does not reopen a submitted review tab when delayed workspace hydration settles", async () => { await markReviewSubmitted(SESSION_A); const pendingConnect = connectToSession(SESSION_A, true); From d5b9369b22ea2e76b964bbf1e68885d268f532c9 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:54:53 +0100 Subject: [PATCH 12/24] Block restricted session model controls Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 6 ++ .../session-ws-write-policy.test.ts | 67 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 76c2a50fc..4b94377bc 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -335,6 +335,9 @@ type SessionWorkMessage = Extract> = [ { type: "grant_tool_permission", toolName: "bash", scope: "tool", mode: "session-only" }, ]; +const MODEL_CONTROL_FRAMES: ReadonlyArray> = [ + { type: "set_model", provider: "anthropic", modelId: "claude-sonnet-4-20250514" }, + { type: "set_thinking_level", level: "high" }, + { type: "set_image_model", provider: "openai", modelId: "gpt-image-2" }, +]; + +function spyOnModelControlMutations(gateway: any, live: any) { + return { + setModel: vi.spyOn(live.rpcClient, "setModel"), + setThinkingLevel: vi.spyOn(live.rpcClient, "setThinkingLevel"), + persistSessionModel: vi.spyOn(gateway.sessionManager, "persistSessionModel"), + updateModelNameFile: vi.spyOn(gateway.sessionManager, "updateModelNameFile"), + persistSessionImageModel: vi.spyOn(gateway.sessionManager, "persistSessionImageModel"), + validateImageModel: vi.spyOn(gateway.sessionManager, "isKnownImageModel"), + }; +} + +function expectNoModelControlMutations(spies: ReturnType): void { + for (const spy of Object.values(spies)) expect(spy).not.toHaveBeenCalled(); +} + +function restoreModelControlSpies(spies: ReturnType): void { + for (const spy of Object.values(spies)) spy.mockRestore(); +} + +async function expectModelControlsRejected(conn: WsConnection, code: string): Promise { + for (const frame of MODEL_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); +} + test.describe("authenticated WebSocket session write policy", () => { test("persisted read-only policy rejects every agent work frame while reads remain available", async ({ gateway }) => { const sessionId = await createSession(); @@ -119,6 +148,7 @@ test.describe("authenticated WebSocket session write policy", () => { const restartAgent = vi.spyOn(gateway.sessionManager, "restartAgent").mockRejectedValue(new Error("policy guard missed restart")); const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); + const modelControlSpies = spyOnModelControlMutations(gateway, live); try { await expectPolicyError(conn, { type: "prompt", text: "must not run" }, "SESSION_READ_ONLY"); // A read-only session has no streaming-steer carve-out. @@ -128,6 +158,7 @@ test.describe("authenticated WebSocket session write policy", () => { for (const frame of [...QUEUE_CONTROL_FRAMES, ...WORK_CONTROL_FRAMES]) { await expectPolicyError(conn, frame, "SESSION_READ_ONLY"); } + await expectModelControlsRejected(conn, "SESSION_READ_ONLY"); await expectExtensionWritesRejected(conn, "SESSION_READ_ONLY", "read-only"); expect(enqueuePrompt).not.toHaveBeenCalled(); @@ -139,6 +170,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(restartAgent).not.toHaveBeenCalled(); expect(grantToolPermission).not.toHaveBeenCalled(); expect(compact).not.toHaveBeenCalled(); + expectNoModelControlMutations(modelControlSpies); await expectReadFramesStillWork(conn); } finally { live.status = "idle"; @@ -151,6 +183,7 @@ test.describe("authenticated WebSocket session write policy", () => { restartAgent.mockRestore(); grantToolPermission.mockRestore(); compact.mockRestore(); + restoreModelControlSpies(modelControlSpies); conn.close(); await deleteSession(sessionId); } @@ -174,6 +207,7 @@ test.describe("authenticated WebSocket session write policy", () => { const restartAgent = vi.spyOn(gateway.sessionManager, "restartAgent").mockRejectedValue(new Error("policy guard missed restart")); const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); + const modelControlSpies = spyOnModelControlMutations(gateway, live); try { await expectPolicyError(conn, { type: "prompt", text: "must not start review" }, "NON_INTERACTIVE_PROMPT"); await expectPolicyError(conn, { type: "steer", text: "must not queue review" }, "NON_INTERACTIVE_STEER"); @@ -183,6 +217,8 @@ test.describe("authenticated WebSocket session write policy", () => { for (const frame of WORK_CONTROL_FRAMES) { await expectPolicyError(conn, frame, "NON_INTERACTIVE_WORK_CONTROL"); } + await expectModelControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + expectNoModelControlMutations(modelControlSpies); await expectReadFramesStillWork(conn); @@ -205,6 +241,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(restartAgent).not.toHaveBeenCalled(); expect(grantToolPermission).not.toHaveBeenCalled(); expect(compact).not.toHaveBeenCalled(); + expectNoModelControlMutations(modelControlSpies); } finally { live.status = "idle"; enqueuePrompt.mockRestore(); @@ -216,6 +253,36 @@ test.describe("authenticated WebSocket session write policy", () => { restartAgent.mockRestore(); grantToolPermission.mockRestore(); compact.mockRestore(); + restoreModelControlSpies(modelControlSpies); + conn.close(); + await deleteSession(sessionId); + } + }); + + test("live restricted metadata blocks model controls before RPC, validation, or persistence", async ({ gateway }) => { + const sessionId = await createSession(); + await waitForSessionStatus(sessionId, "idle"); + const live = gateway.sessionManager.getSession(sessionId); + expect(live).toBeTruthy(); + const persisted = gateway.sessionManager.getPersistedSession(sessionId); + expect(persisted?.readOnly).not.toBe(true); + expect(persisted?.nonInteractive).not.toBe(true); + + const conn = await connectWs(sessionId); + const modelControlSpies = spyOnModelControlMutations(gateway, live); + try { + live.readOnly = true; + await expectModelControlsRejected(conn, "SESSION_READ_ONLY"); + + live.readOnly = false; + live.nonInteractive = true; + await expectModelControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + + expectNoModelControlMutations(modelControlSpies); + } finally { + live.readOnly = false; + live.nonInteractive = false; + restoreModelControlSpies(modelControlSpies); conn.close(); await deleteSession(sessionId); } From 5e7fe5209358a97fb3ee74fcc582654fff249593 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:04:45 +0100 Subject: [PATCH 13/24] refactor ws work message classifier Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 55 +++++++++++++++------------------------- 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 4b94377bc..19f734986 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -326,44 +326,31 @@ export const MAX_AUTHENTICATED_PROMPT_TEXT_BYTES = 8 * 1024 * 1024; const SESSION_COMMAND_SERIALISER = new SessionCommandSerialiser(); const EXTENSION_CHANNEL_WS_ENVELOPE_TOO_LARGE_MESSAGE = `Extension channel frame exceeds maximum envelope size (${MAX_EXTENSION_CHANNEL_WS_ENVELOPE_BYTES} bytes)`; +const SESSION_WORK_MESSAGE_TYPES = [ + "prompt", + "steer", + "steer_queued", + "remove_queued", + "reorder_queue", + "retry", + "restart_agent", + "set_model", + "set_image_model", + "set_thinking_level", + "compact", + "grant_tool_permission", + "ext_session_write_permit", + "ext_session_post", +] as const satisfies readonly ClientMessage["type"][]; + type SessionWorkMessage = Extract; +const SESSION_WORK_MESSAGE_TYPE_SET: ReadonlySet = new Set(SESSION_WORK_MESSAGE_TYPES); + function isSessionWorkMessage(msg: ClientMessage): msg is SessionWorkMessage { - switch (msg.type) { - case "prompt": - case "steer": - case "steer_queued": - case "remove_queued": - case "reorder_queue": - case "retry": - case "restart_agent": - case "set_model": - case "set_image_model": - case "set_thinking_level": - case "compact": - case "grant_tool_permission": - case "ext_session_write_permit": - case "ext_session_post": - return true; - default: - return false; - } + return SESSION_WORK_MESSAGE_TYPE_SET.has(msg.type); } function sendSessionWorkPolicyError( From 7ac0ddf65107d140b1b4d9cbac5415c8eddbf229 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:35:01 +0100 Subject: [PATCH 14/24] Restore cold-loaded review documents Co-authored-by: bobbit-ai --- src/app/session-manager.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/session-manager.ts b/src/app/session-manager.ts index 69fefbcb6..3f5b8cc2c 100644 --- a/src/app/session-manager.ts +++ b/src/app/session-manager.ts @@ -2421,6 +2421,10 @@ export async function connectToSession(sessionId: string, isExisting: boolean, o // including for a now-cached session, before stale-invocation teardown. await remote.reconcileSubmittedReviewWorkspace(); if (isStale()) { cleanupRemote(remote); return; } + // The earlier restore runs before hydration so it cannot see cold-loaded + // review tabs. Restore again only after submitted tabs have been removed; + // unsubmitted persisted documents now have authoritative tabs to bind to. + reviewSources.restorePersistedReviewDocuments(sessionId, { select: true }); // Listen for suggest-goal events from assistant messages state.chatPanel.addEventListener('suggest-goal', () => { From 0f2b7237528153e466ab4cd9e853e5c6a3ad178f Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:38:08 +0100 Subject: [PATCH 15/24] test: faithfully cover review restoration Co-authored-by: bobbit-ai --- ...cold-session-workspace-ordering-repro.test.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts index b31cec4fb..6befa8d8a 100644 --- a/tests2/dom/cold-session-workspace-ordering-repro.test.ts +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -376,6 +376,13 @@ beforeEach(() => { vi.spyOn(reviewSourcesLazy, "loadReviewSources").mockResolvedValue({ restorePersistedReviewDocuments: (sessionId: string) => { if (state.selectedSessionId !== sessionId || isReviewSubmitted(sessionId)) return; + const documentId = `${sessionId}-review`; + const hasMatchingWorkspaceTab = state.sidePanelWorkspaceBySession[sessionId]?.tabs.some((tab) => { + if (tab.kind !== "review") return false; + const source = tab.source as Record | undefined; + return source?.documentId === documentId || tab.id === `review:${documentId}`; + }); + if (!hasMatchingWorkspaceTab) return; const title = `${sessionId} review`; state.reviewDocuments = new Map([[title, { title, markdown: `# ${title}` } as any]]); state.reviewActiveTab = title; @@ -494,6 +501,10 @@ describe("cold session transcript/workspace ordering", () => { ); expect(finalTranscriptRow(SESSION_A)?.textContent).toBe(`${SESSION_A} transcript row ${TRANSCRIPT_SIZE - 1}`); expect(gateFor(SESSION_A).settled).toBe(false); + expect( + state.reviewDocuments.has(`${SESSION_A} review`), + "PERSISTED_REVIEW_HYDRATION_REGRESSION: persisted review restored before its matching workspace tab hydrated", + ).toBe(false); } finally { gateFor(SESSION_A).release(); await pendingConnect.catch(() => {}); @@ -506,7 +517,10 @@ describe("cold session transcript/workspace ordering", () => { ]); expect(state.activePanelTabId).toBe("preview:entry:index.html"); expect(state.previewPanelFullscreen).toBe(true); - expect(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(true); + expect( + state.reviewDocuments.has(`${SESSION_A} review`), + "PERSISTED_REVIEW_HYDRATION_REGRESSION: persisted review was not restored after workspace hydration", + ).toBe(true); expect(finalTranscriptRow(SESSION_A)?.textContent).toBe(`${SESSION_A} transcript row ${TRANSCRIPT_SIZE - 1}`); }); From 6c41dc5de7e5e61fb3c51f0b5b7d208da7705a7a Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:45:34 +0100 Subject: [PATCH 16/24] Preserve workspace conflict authority Co-authored-by: bobbit-ai --- src/app/remote-agent.ts | 24 +++---------- src/app/side-panel-workspace.ts | 64 ++++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 24 deletions(-) diff --git a/src/app/remote-agent.ts b/src/app/remote-agent.ts index b62bff07e..fe845ab1c 100644 --- a/src/app/remote-agent.ts +++ b/src/app/remote-agent.ts @@ -732,31 +732,17 @@ export class RemoteAgent { .map((tab) => tab.id); if (reviewTabIds.length === 0) return; - // Use the normal mutation path first so successful deletes retain all - // server revision/conflict semantics. Some compatible endpoints return - // 204 for DELETE; the workspace helper then refetches and may receive a - // stale pre-delete snapshot. Reapply the submitted-review policy to that - // settled snapshot below without inventing a new revision. + // Keep every deletion on the normal mutation path. A confirmed compatible + // 204 settles the optimistic close, while a 409 workspace remains + // authoritative and may be retried once at its newer revision. Refetches, + // network failures, and rollbacks are never locally filtered afterward. for (const tabId of reviewTabIds) { try { - await closeSidePanelTab(tabId, { sessionId }); + await closeSidePanelTab(tabId, { sessionId, retryConflictOnce: true }); } catch (err) { console.warn("[RemoteAgent] submitted-review workspace cleanup failed:", err); } } - if (!isReviewSubmitted(sessionId)) return; - const settled = getSidePanelWorkspace(sessionId); - const tabs = settled.tabs.filter((tab) => !isReviewTab(tab)); - if (tabs.length === settled.tabs.length) return; - const activeTabId = tabs.some((tab) => tab.id === settled.activeTabId) - ? settled.activeTabId - : tabs[0]?.id || ""; - applySidePanelWorkspaceFromServer({ - ...settled, - tabs, - activeTabId, - updatedAt: Date.now(), - }, { source: "rest", force: true }); } private _isActiveSession(): boolean { return this._sessionId !== "" && state.selectedSessionId === this._sessionId; diff --git a/src/app/side-panel-workspace.ts b/src/app/side-panel-workspace.ts index b8153915a..3e39de542 100644 --- a/src/app/side-panel-workspace.ts +++ b/src/app/side-panel-workspace.ts @@ -71,6 +71,11 @@ export interface SidePanelMutationOptions { skipRender?: boolean; } +export interface CloseSidePanelTabOptions extends SidePanelMutationOptions { + sessionId?: string; + retryConflictOnce?: boolean; +} + type PendingMutation = { id: string; baseRevision: number; @@ -574,11 +579,20 @@ function fixtureInitialWorkspace(sessionId: string): SidePanelWorkspace | undefi }; } -async function workspaceRequest(sessionId: string, path: string, init: RequestInit = {}): Promise { +async function workspaceRequest( + sessionId: string, + path: string, + init: RequestInit = {}, + options: { confirmedNoContentWorkspace?: SidePanelWorkspace } = {}, +): Promise { const res = await gatewayFetch(`/api/sessions/${encodeComponent(sessionId)}/side-panel-workspace${path}`, init); const body = await readJsonSafe(res); const workspace = workspaceFromResponseBody(body, sessionId); if (!res.ok) throw new WorkspaceRequestError(`Side-panel workspace request failed: ${res.status}`, res.status, workspace); + // Older/compatible workspace endpoints confirm DELETE with an empty 204 + // rather than the current JSON workspace response. Only callers that supply + // the exact optimistic result may use that response as a successful commit. + if (res.status === 204 && options.confirmedNoContentWorkspace) return options.confirmedNoContentWorkspace; if (!workspace) throw new WorkspaceRequestError("Invalid side-panel workspace response", res.status); return workspace; } @@ -593,7 +607,12 @@ async function fetchWorkspace(sessionId: string): Promise): Promise { +async function settleMutation( + sessionId: string, + mutationId: string, + request: Promise, + onAuthoritativeConflict?: (workspace: SidePanelWorkspace) => void, +): Promise { const sid = panelWorkspaceSessionKey(sessionId); try { const workspace = await request; @@ -605,7 +624,11 @@ async function settleMutation(sessionId: string, mutationId: string, request: Pr const ownsPending = pending?.id === mutationId; if (ownsPending) mutationState.delete(sid); const latest = err instanceof WorkspaceRequestError ? err.workspace : undefined; - if (latest) return applySidePanelWorkspaceFromServer(latest, { source: "rest", force: ownsPending }); + if (latest) { + const authoritative = applySidePanelWorkspaceFromServer(latest, { source: "rest", force: ownsPending }); + if (err instanceof WorkspaceRequestError && err.status === 409) onAuthoritativeConflict?.(authoritative); + return authoritative; + } try { const refetched = await fetchWorkspace(sessionId); if (refetched) return applySidePanelWorkspaceFromServer(refetched, { source: "hydrate", force: ownsPending }); @@ -697,14 +720,45 @@ export async function updateSidePanelTab(tabId: string, patch: Partial { +export async function closeSidePanelTab(tabId: string, options: CloseSidePanelTabOptions = {}): Promise { const sessionId = options.sessionId || activeSessionId() || state.selectedSessionId || ""; const base = mutationBaseWorkspace(sessionId); const tabs = base.tabs.filter((tab) => tab.id !== tabId); const optimistic = withWorkspaceUpdate(base, { tabs, activeTabId: nextActiveAfterClose(base.tabs, tabId, base.activeTabId) }); if (!useServerWorkspaceApi()) return applyFixtureWorkspace(optimistic, options); const mutationId = applyOptimisticWorkspace(optimistic, base, options); - return settleMutation(base.sessionId, mutationId, workspaceRequest(base.sessionId, `/tabs/${encodeComponent(tabId)}`, { method: "DELETE" })); + const sid = panelWorkspaceSessionKey(base.sessionId); + const confirmedNoContentWorkspace = withWorkspaceUpdate(optimistic, { + revision: Math.max(base.revision, state.lastWorkspaceRevisionBySession[sid] ?? base.revision) + 1, + }); + const conflict = { workspace: undefined as SidePanelWorkspace | undefined }; + const requestHeaders = options.baseRevision == null + ? undefined + : { "If-Match": `"${options.baseRevision}"` }; + const settled = await settleMutation( + base.sessionId, + mutationId, + workspaceRequest( + base.sessionId, + `/tabs/${encodeComponent(tabId)}`, + { method: "DELETE", headers: requestHeaders }, + { confirmedNoContentWorkspace }, + ), + (workspace) => { conflict.workspace = workspace; }, + ); + const authoritative = conflict.workspace; + if (!options.retryConflictOnce + || !authoritative + || authoritative.revision <= base.revision + || !authoritative.tabs.some((tab) => tab.id === tabId)) { + return settled; + } + return closeSidePanelTab(tabId, { + ...options, + baseRevision: authoritative.revision, + strictRevision: true, + retryConflictOnce: false, + }); } export async function setActiveSidePanelTab(tabId: string, options: SidePanelMutationOptions & { sessionId?: string } = {}): Promise { From 31e2265ee93fdd28b795c0ba9bdfdfb0583f93c6 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:50:59 +0100 Subject: [PATCH 17/24] Test submitted review conflict reconciliation Co-authored-by: bobbit-ai --- ...d-session-workspace-ordering-repro.test.ts | 234 +++++++++++++++++- 1 file changed, 231 insertions(+), 3 deletions(-) diff --git a/tests2/dom/cold-session-workspace-ordering-repro.test.ts b/tests2/dom/cold-session-workspace-ordering-repro.test.ts index 6befa8d8a..82faff0f7 100644 --- a/tests2/dom/cold-session-workspace-ordering-repro.test.ts +++ b/tests2/dom/cold-session-workspace-ordering-repro.test.ts @@ -22,6 +22,7 @@ import { uncacheSession, } from "../../src/app/session-manager.js"; import { RemoteAgent } from "../../src/app/remote-agent.js"; +import type { SidePanelWorkspace } from "../../src/app/side-panel-workspace.js"; import { GW_SESSION_KEY, GW_TOKEN_KEY, @@ -60,9 +61,22 @@ interface DeferredGate { settled: boolean; } +interface WorkspaceDeleteAttempt { + tabId: string; + baseRevision?: number; +} + +type WorkspaceDeleteOutcome = + | { kind: "confirmed-204" } + | { kind: "conflict-409"; current: SidePanelWorkspace } + | { kind: "network-error"; message: string }; + const timeline: TimelineEntry[] = []; const workspaceGates = new Map(); const workspaceFetchCount = new Map(); +const authoritativeWorkspaces = new Map(); +const workspaceDeleteOutcomes = new Map(); +const workspaceDeleteAttempts = new Map(); const transcripts = new Map(); let sessionListGate: DeferredGate | null = null; let sessionListFetchCount = 0; @@ -113,7 +127,7 @@ function transcriptFor(sessionId: string): any[] { })); } -function workspaceFor(sessionId: string) { +function workspaceFor(sessionId: string): SidePanelWorkspace { const stamp = sessionId === SESSION_A ? 10 : 20; return { version: 1, @@ -160,6 +174,55 @@ function workspaceFor(sessionId: string) { }; } +function cloneWorkspace(workspace: SidePanelWorkspace): SidePanelWorkspace { + return JSON.parse(JSON.stringify(workspace)) as SidePanelWorkspace; +} + +function authoritativeWorkspace(sessionId: string): SidePanelWorkspace { + let workspace = authoritativeWorkspaces.get(sessionId); + if (!workspace) { + workspace = workspaceFor(sessionId); + authoritativeWorkspaces.set(sessionId, workspace); + } + return workspace; +} + +function conflictWorkspace(sessionId: string, revision: number, marker: string): SidePanelWorkspace { + const workspace = workspaceFor(sessionId); + const reviewId = `review:${sessionId}-review`; + return { + ...workspace, + revision, + tabs: workspace.tabs.map((tab) => tab.kind === "review" + ? { ...tab, title: `${sessionId} review ${marker}`, label: `Review ${marker}`, updatedAt: revision } + : { ...tab, title: `${tab.title} ${marker}`, updatedAt: revision }), + activeTabId: reviewId, + sizeMode: "split", + updatedAt: revision, + }; +} + +function requestBaseRevision(input: RequestInfo | URL, init: RequestInit | undefined, url: URL): number | undefined { + const headers = new Headers(input instanceof Request ? input.headers : undefined); + new Headers(init?.headers).forEach((value, key) => headers.set(key, value)); + const ifMatch = headers.get("if-match")?.trim().replace(/^W\//, "").replace(/^"|"$/g, ""); + if (ifMatch) { + const parsed = Number(ifMatch); + if (Number.isInteger(parsed) && parsed >= 0) return parsed; + } + const queryRevision = Number(url.searchParams.get("baseRevision")); + if (url.searchParams.has("baseRevision") && Number.isInteger(queryRevision) && queryRevision >= 0) return queryRevision; + if (typeof init?.body === "string") { + try { + const parsed = JSON.parse(init.body) as { baseRevision?: unknown }; + if (typeof parsed.baseRevision === "number" && Number.isInteger(parsed.baseRevision) && parsed.baseRevision >= 0) { + return parsed.baseRevision; + } + } catch { /* no JSON revision */ } + } + return undefined; +} + function sessionIdFromPath(path: string): string | undefined { return /^\/api\/sessions\/([^/?]+)/.exec(path)?.[1]; } @@ -171,10 +234,44 @@ async function fetchFixture(input: RequestInfo | URL, init?: RequestInit): Promi const sessionId = sessionIdFromPath(url.pathname); timeline.push({ kind: "rest", path: `${url.pathname}${url.search}`, method, ...(sessionId ? { sessionId } : {}) }); + const deleteTabMatch = url.pathname.match(/\/side-panel-workspace\/tabs\/([^/]+)$/); + if (deleteTabMatch && sessionId && method === "DELETE") { + const attempt: WorkspaceDeleteAttempt = { + tabId: decodeURIComponent(deleteTabMatch[1]), + baseRevision: requestBaseRevision(input, init, url), + }; + const attempts = workspaceDeleteAttempts.get(sessionId) || []; + attempts.push(attempt); + workspaceDeleteAttempts.set(sessionId, attempts); + const outcomes = workspaceDeleteOutcomes.get(sessionId) || []; + const outcome = outcomes.shift() || { kind: "confirmed-204" as const }; + if (outcome.kind === "network-error") throw new TypeError(outcome.message); + if (outcome.kind === "conflict-409") { + authoritativeWorkspaces.set(sessionId, cloneWorkspace(outcome.current)); + return Response.json({ + error: "Stale side-panel workspace revision", + code: "STALE_REVISION", + workspace: cloneWorkspace(outcome.current), + }, { status: 409 }); + } + const current = authoritativeWorkspace(sessionId); + const tabs = current.tabs.filter((tab) => tab.id !== attempt.tabId); + const activeTabId = tabs.some((tab) => tab.id === current.activeTabId) + ? current.activeTabId + : tabs[0]?.id || ""; + authoritativeWorkspaces.set(sessionId, { + ...current, + revision: current.revision + 1, + tabs, + activeTabId, + updatedAt: current.updatedAt + 1, + }); + return new Response(null, { status: 204 }); + } if (url.pathname.endsWith("/side-panel-workspace") && sessionId) { workspaceFetchCount.set(sessionId, (workspaceFetchCount.get(sessionId) || 0) + 1); await gateFor(sessionId).promise; - return Response.json(workspaceFor(sessionId)); + return Response.json(cloneWorkspace(authoritativeWorkspace(sessionId))); } if (url.pathname.endsWith("/draft") && method === "GET") return new Response(null, { status: 204 }); if (url.pathname.endsWith("/git-status")) { @@ -344,18 +441,60 @@ function finalTranscriptRow(sessionId: string): HTMLElement | null { ); } +function workspaceMirrorShape(workspace: SidePanelWorkspace | undefined) { + if (!workspace) return undefined; + return { + sessionId: workspace.sessionId, + revision: workspace.revision, + tabs: workspace.tabs.map((tab) => ({ id: tab.id, kind: tab.kind, title: tab.title, label: tab.label })), + activeTabId: workspace.activeTabId, + sizeMode: workspace.sizeMode, + }; +} + +function expectServerConsistentWorkspaceMirrors(sessionId: string, failureToken: string): void { + const authoritative = authoritativeWorkspace(sessionId); + const expectedShape = workspaceMirrorShape(authoritative); + expect.soft( + workspaceMirrorShape(state.sidePanelWorkspaceBySession[sessionId]), + `${failureToken}: keyed workspace diverged from the authoritative server workspace`, + ).toEqual(expectedShape); + expect.soft( + workspaceMirrorShape((state as any).panelWorkspace as SidePanelWorkspace | undefined), + `${failureToken}: foreground panelWorkspace diverged from the authoritative server workspace`, + ).toEqual(expectedShape); + expect.soft( + state.lastWorkspaceRevisionBySession[sessionId], + `${failureToken}: tracked revision diverged from the authoritative server revision`, + ).toBe(authoritative.revision); + expect.soft( + state.panelTabs.map((tab) => ({ id: tab.id, kind: tab.kind, title: tab.title, label: tab.label })), + `${failureToken}: foreground panel tabs diverged from the authoritative server tabs`, + ).toEqual(authoritative.tabs.map((tab) => ({ id: tab.id, kind: tab.kind, title: tab.title, label: tab.label }))); + expect.soft(state.activePanelTabId, `${failureToken}: foreground active tab diverged from the server`).toBe(authoritative.activeTabId); + expect.soft(state.previewPanelFullscreen, `${failureToken}: foreground size mode diverged from the server`).toBe( + authoritative.sizeMode === "fullscreen", + ); +} + beforeEach(() => { (window as any).happyDOM?.setURL?.("http://localhost/#/"); setRenderApp(() => {}); timeline.length = 0; workspaceGates.clear(); workspaceFetchCount.clear(); + authoritativeWorkspaces.clear(); + workspaceDeleteOutcomes.clear(); + workspaceDeleteAttempts.clear(); sessionListGate = null; sessionListFetchCount = 0; sessionListResponseSessions = null; ControlledWebSocket.instances.length = 0; transcripts.clear(); - for (const sessionId of trackedSessions) transcripts.set(sessionId, transcriptFor(sessionId)); + for (const sessionId of trackedSessions) { + transcripts.set(sessionId, transcriptFor(sessionId)); + authoritativeWorkspaces.set(sessionId, workspaceFor(sessionId)); + } vi.stubGlobal("WebSocket", ControlledWebSocket); vi.stubGlobal("fetch", vi.fn(fetchFixture)); @@ -688,6 +827,95 @@ describe("cold session transcript/workspace ordering", () => { } }); + it("reconciles a submitted review through one confirmed 204 delete with a coherent revision", async () => { + workspaceDeleteOutcomes.set(SESSION_A, [{ kind: "confirmed-204" }]); + await markReviewSubmitted(SESSION_A); + gateFor(SESSION_A).release(); + await connectToSession(SESSION_A, true); + + expect( + workspaceDeleteAttempts.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_204_RECONCILIATION_REGRESSION: cleanup must issue exactly one normal DELETE", + ).toEqual([{ tabId: `review:${SESSION_A}-review`, baseRevision: undefined }]); + expect( + workspaceFetchCount.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_204_RECONCILIATION_REGRESSION: confirmed 204 must not trigger a stale workspace refetch", + ).toBe(1); + expect( + authoritativeWorkspace(SESSION_A).revision, + "SUBMITTED_REVIEW_DELETE_204_RECONCILIATION_REGRESSION: fixture authority did not advance after confirmed delete", + ).toBe(11); + expect( + authoritativeWorkspace(SESSION_A).tabs.some((tab) => tab.kind === "review"), + "SUBMITTED_REVIEW_DELETE_204_RECONCILIATION_REGRESSION: confirmed server delete retained the review", + ).toBe(false); + expectServerConsistentWorkspaceMirrors(SESSION_A, "SUBMITTED_REVIEW_DELETE_204_RECONCILIATION_REGRESSION"); + expect.soft(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(false); + expect.soft(state.reviewPanelOpen).toBe(false); + }); + + it("bounds submitted-review conflict retry and preserves the newest 409 workspace", async () => { + const conflictAt11 = conflictWorkspace(SESSION_A, 11, "server-v11"); + const conflictAt12 = conflictWorkspace(SESSION_A, 12, "server-v12"); + workspaceDeleteOutcomes.set(SESSION_A, [ + { kind: "conflict-409", current: conflictAt11 }, + { kind: "conflict-409", current: conflictAt12 }, + ]); + await markReviewSubmitted(SESSION_A); + gateFor(SESSION_A).release(); + await connectToSession(SESSION_A, true); + + expect( + workspaceDeleteAttempts.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_409_AUTHORITY_REGRESSION: cleanup must retry once against the newer revision and then stop", + ).toEqual([ + { tabId: `review:${SESSION_A}-review`, baseRevision: undefined }, + { tabId: `review:${SESSION_A}-review`, baseRevision: 11 }, + ]); + expect( + workspaceFetchCount.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_409_AUTHORITY_REGRESSION: 409 bodies already carry current workspace and must not refetch", + ).toBe(1); + expect( + authoritativeWorkspace(SESSION_A).revision, + "SUBMITTED_REVIEW_DELETE_409_AUTHORITY_REGRESSION: second conflict did not become authoritative", + ).toBe(12); + expect( + authoritativeWorkspace(SESSION_A).tabs.some((tab) => tab.kind === "review"), + "SUBMITTED_REVIEW_DELETE_409_AUTHORITY_REGRESSION: conflict authority fixture unexpectedly removed the review", + ).toBe(true); + expectServerConsistentWorkspaceMirrors(SESSION_A, "SUBMITTED_REVIEW_DELETE_409_AUTHORITY_REGRESSION"); + expect.soft(state.panelTabs.some((tab) => tab.kind === "review")).toBe(true); + expect.soft(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(false); + expect.soft(state.reviewPanelOpen).toBe(false); + }); + + it("keeps an authoritative review after submitted-review delete network failure and refetch", async () => { + workspaceDeleteOutcomes.set(SESSION_A, [ + { kind: "network-error", message: "fixture submitted-review DELETE network failure" }, + ]); + await markReviewSubmitted(SESSION_A); + gateFor(SESSION_A).release(); + await connectToSession(SESSION_A, true); + + expect( + workspaceDeleteAttempts.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_NETWORK_ROLLBACK_REGRESSION: network failure must not cause blind delete retries", + ).toEqual([{ tabId: `review:${SESSION_A}-review`, baseRevision: undefined }]); + expect( + workspaceFetchCount.get(SESSION_A), + "SUBMITTED_REVIEW_DELETE_NETWORK_ROLLBACK_REGRESSION: network failure must perform one authoritative refetch", + ).toBe(2); + expect( + authoritativeWorkspace(SESSION_A).tabs.some((tab) => tab.kind === "review"), + "SUBMITTED_REVIEW_DELETE_NETWORK_ROLLBACK_REGRESSION: refetch fixture unexpectedly removed the review", + ).toBe(true); + expectServerConsistentWorkspaceMirrors(SESSION_A, "SUBMITTED_REVIEW_DELETE_NETWORK_ROLLBACK_REGRESSION"); + expect.soft(state.panelTabs.some((tab) => tab.kind === "review")).toBe(true); + expect.soft(state.reviewDocuments.has(`${SESSION_A} review`)).toBe(false); + expect.soft(state.reviewPanelOpen).toBe(false); + }); + it("keeps the exact cached A agent and panel connected after A to B to A before A hydration releases", async () => { const originalConnectA = connectToSession(SESSION_A, true); let connectB: Promise | undefined; From 64b0e76a40b2fc1ac98c3e8ebbb345157fe5f8dc Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:20:15 +0100 Subject: [PATCH 18/24] fix: restrict title websocket commands Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 19f734986..4136f6a18 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -334,6 +334,9 @@ const SESSION_WORK_MESSAGE_TYPES = [ "reorder_queue", "retry", "restart_agent", + "set_title", + "generate_title", + "summarize_goal_title", "set_model", "set_image_model", "set_thinking_level", From e721a9bbf41f022ad3c30998bb57a3821590fe1d Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:27:08 +0100 Subject: [PATCH 19/24] test: cover restricted title websocket frames Co-authored-by: bobbit-ai --- .../session-ws-write-policy.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests2/integration/session-ws-write-policy.test.ts b/tests2/integration/session-ws-write-policy.test.ts index 14829568e..0e342c941 100644 --- a/tests2/integration/session-ws-write-policy.test.ts +++ b/tests2/integration/session-ws-write-policy.test.ts @@ -106,6 +106,12 @@ const MODEL_CONTROL_FRAMES: ReadonlyArray> = [ { type: "set_image_model", provider: "openai", modelId: "gpt-image-2" }, ]; +const TITLE_CONTROL_FRAMES: ReadonlyArray> = [ + { type: "generate_title" }, + { type: "summarize_goal_title", goalTitle: "Attacker controlled goal title" }, + { type: "set_title", title: "forbidden title" }, +]; + function spyOnModelControlMutations(gateway: any, live: any) { return { setModel: vi.spyOn(live.rpcClient, "setModel"), @@ -125,10 +131,33 @@ function restoreModelControlSpies(spies: ReturnType undefined), + setTitle: vi.spyOn(gateway.sessionManager, "setTitle").mockReturnValue(false), + }; +} + +async function expectNoTitleControlMutations(spies: ReturnType): Promise { + // Title generation is fire-and-forget. Cross an event-loop turn so a handler + // that incorrectly defers either generation call cannot escape the assertion. + await new Promise(resolve => setImmediate(resolve)); + for (const spy of Object.values(spies)) expect(spy).not.toHaveBeenCalled(); +} + +function restoreTitleControlSpies(spies: ReturnType): void { + for (const spy of Object.values(spies)) spy.mockRestore(); +} + async function expectModelControlsRejected(conn: WsConnection, code: string): Promise { for (const frame of MODEL_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); } +async function expectTitleControlsRejected(conn: WsConnection, code: string): Promise { + for (const frame of TITLE_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); +} + test.describe("authenticated WebSocket session write policy", () => { test("persisted read-only policy rejects every agent work frame while reads remain available", async ({ gateway }) => { const sessionId = await createSession(); @@ -149,6 +178,7 @@ test.describe("authenticated WebSocket session write policy", () => { const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); const modelControlSpies = spyOnModelControlMutations(gateway, live); + const titleControlSpies = spyOnTitleControlMutations(gateway); try { await expectPolicyError(conn, { type: "prompt", text: "must not run" }, "SESSION_READ_ONLY"); // A read-only session has no streaming-steer carve-out. @@ -159,6 +189,7 @@ test.describe("authenticated WebSocket session write policy", () => { await expectPolicyError(conn, frame, "SESSION_READ_ONLY"); } await expectModelControlsRejected(conn, "SESSION_READ_ONLY"); + await expectTitleControlsRejected(conn, "SESSION_READ_ONLY"); await expectExtensionWritesRejected(conn, "SESSION_READ_ONLY", "read-only"); expect(enqueuePrompt).not.toHaveBeenCalled(); @@ -171,6 +202,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(grantToolPermission).not.toHaveBeenCalled(); expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); + await expectNoTitleControlMutations(titleControlSpies); await expectReadFramesStillWork(conn); } finally { live.status = "idle"; @@ -184,6 +216,7 @@ test.describe("authenticated WebSocket session write policy", () => { grantToolPermission.mockRestore(); compact.mockRestore(); restoreModelControlSpies(modelControlSpies); + restoreTitleControlSpies(titleControlSpies); conn.close(); await deleteSession(sessionId); } @@ -208,6 +241,7 @@ test.describe("authenticated WebSocket session write policy", () => { const grantToolPermission = vi.spyOn(gateway.sessionManager, "grantToolPermission").mockRejectedValue(new Error("policy guard missed grant")); const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); const modelControlSpies = spyOnModelControlMutations(gateway, live); + const titleControlSpies = spyOnTitleControlMutations(gateway); try { await expectPolicyError(conn, { type: "prompt", text: "must not start review" }, "NON_INTERACTIVE_PROMPT"); await expectPolicyError(conn, { type: "steer", text: "must not queue review" }, "NON_INTERACTIVE_STEER"); @@ -218,7 +252,9 @@ test.describe("authenticated WebSocket session write policy", () => { await expectPolicyError(conn, frame, "NON_INTERACTIVE_WORK_CONTROL"); } await expectModelControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + await expectTitleControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); expectNoModelControlMutations(modelControlSpies); + await expectNoTitleControlMutations(titleControlSpies); await expectReadFramesStillWork(conn); @@ -242,6 +278,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(grantToolPermission).not.toHaveBeenCalled(); expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); + await expectNoTitleControlMutations(titleControlSpies); } finally { live.status = "idle"; enqueuePrompt.mockRestore(); @@ -254,6 +291,7 @@ test.describe("authenticated WebSocket session write policy", () => { grantToolPermission.mockRestore(); compact.mockRestore(); restoreModelControlSpies(modelControlSpies); + restoreTitleControlSpies(titleControlSpies); conn.close(); await deleteSession(sessionId); } From de3f23d0342d2ef5aa4d279861acbe4415c6ce32 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:57:09 +0100 Subject: [PATCH 20/24] Block restricted task mutations Co-authored-by: bobbit-ai --- src/server/ws/handler.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index 4136f6a18..d12323bad 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -326,6 +326,8 @@ export const MAX_AUTHENTICATED_PROMPT_TEXT_BYTES = 8 * 1024 * 1024; const SESSION_COMMAND_SERIALISER = new SessionCommandSerialiser(); const EXTENSION_CHANNEL_WS_ENVELOPE_TOO_LARGE_MESSAGE = `Extension channel frame exceeds maximum envelope size (${MAX_EXTENSION_CHANNEL_WS_ENVELOPE_BYTES} bytes)`; +// Restricted-session work includes agent work, metadata writes, and durable task +// mutations; safety controls, reads, and transport-authorized operations stay separate. const SESSION_WORK_MESSAGE_TYPES = [ "prompt", "steer", @@ -337,6 +339,9 @@ const SESSION_WORK_MESSAGE_TYPES = [ "set_title", "generate_title", "summarize_goal_title", + "task_create", + "task_update", + "task_delete", "set_model", "set_image_model", "set_thinking_level", From 6e9bc8e4dd470fb3944927322dbb3d64d391dcdb Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:02:04 +0100 Subject: [PATCH 21/24] test: guard restricted websocket task frames Co-authored-by: bobbit-ai --- .../session-ws-write-policy.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests2/integration/session-ws-write-policy.test.ts b/tests2/integration/session-ws-write-policy.test.ts index 0e342c941..d777f4501 100644 --- a/tests2/integration/session-ws-write-policy.test.ts +++ b/tests2/integration/session-ws-write-policy.test.ts @@ -1,4 +1,5 @@ import { vi } from "vitest"; +import { TaskManager } from "../../src/server/agent/task-manager.js"; import { expect, test } from "./_e2e/in-process-harness.js"; import { connectWs, @@ -112,6 +113,26 @@ const TITLE_CONTROL_FRAMES: ReadonlyArray> = [ { type: "set_title", title: "forbidden title" }, ]; +const TASK_CONTROL_FRAMES: ReadonlyArray> = [ + { + type: "task_create", + goalId: "11111111-1111-4111-8111-111111111111", + title: "Cross-goal crafted task", + taskType: "implementation", + parentTaskId: "22222222-2222-4222-8222-222222222222", + dependsOn: ["33333333-3333-4333-8333-333333333333"], + }, + { + type: "task_update", + taskId: "44444444-4444-4444-8444-444444444444", + updates: { title: "Cross-goal crafted update", state: "in-progress" }, + }, + { + type: "task_delete", + taskId: "55555555-5555-4555-8555-555555555555", + }, +]; + function spyOnModelControlMutations(gateway: any, live: any) { return { setModel: vi.spyOn(live.rpcClient, "setModel"), @@ -150,6 +171,27 @@ function restoreTitleControlSpies(spies: ReturnType): void { + for (const spy of Object.values(spies)) expect(spy).not.toHaveBeenCalled(); +} + +function restoreTaskControlSpies(spies: ReturnType): void { + for (const spy of Object.values(spies)) spy.mockRestore(); +} + async function expectModelControlsRejected(conn: WsConnection, code: string): Promise { for (const frame of MODEL_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); } @@ -158,6 +200,10 @@ async function expectTitleControlsRejected(conn: WsConnection, code: string): Pr for (const frame of TITLE_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); } +async function expectTaskControlsRejected(conn: WsConnection, code: string): Promise { + for (const frame of TASK_CONTROL_FRAMES) await expectPolicyError(conn, frame, code); +} + test.describe("authenticated WebSocket session write policy", () => { test("persisted read-only policy rejects every agent work frame while reads remain available", async ({ gateway }) => { const sessionId = await createSession(); @@ -179,6 +225,7 @@ test.describe("authenticated WebSocket session write policy", () => { const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); const modelControlSpies = spyOnModelControlMutations(gateway, live); const titleControlSpies = spyOnTitleControlMutations(gateway); + const taskControlSpies = spyOnTaskControlPaths(gateway); try { await expectPolicyError(conn, { type: "prompt", text: "must not run" }, "SESSION_READ_ONLY"); // A read-only session has no streaming-steer carve-out. @@ -190,6 +237,7 @@ test.describe("authenticated WebSocket session write policy", () => { } await expectModelControlsRejected(conn, "SESSION_READ_ONLY"); await expectTitleControlsRejected(conn, "SESSION_READ_ONLY"); + await expectTaskControlsRejected(conn, "SESSION_READ_ONLY"); await expectExtensionWritesRejected(conn, "SESSION_READ_ONLY", "read-only"); expect(enqueuePrompt).not.toHaveBeenCalled(); @@ -203,6 +251,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); + expectNoTaskControlPaths(taskControlSpies); await expectReadFramesStillWork(conn); } finally { live.status = "idle"; @@ -217,6 +266,7 @@ test.describe("authenticated WebSocket session write policy", () => { compact.mockRestore(); restoreModelControlSpies(modelControlSpies); restoreTitleControlSpies(titleControlSpies); + restoreTaskControlSpies(taskControlSpies); conn.close(); await deleteSession(sessionId); } @@ -242,6 +292,7 @@ test.describe("authenticated WebSocket session write policy", () => { const compact = vi.spyOn(live.rpcClient, "compact").mockRejectedValue(new Error("policy guard missed compact")); const modelControlSpies = spyOnModelControlMutations(gateway, live); const titleControlSpies = spyOnTitleControlMutations(gateway); + const taskControlSpies = spyOnTaskControlPaths(gateway); try { await expectPolicyError(conn, { type: "prompt", text: "must not start review" }, "NON_INTERACTIVE_PROMPT"); await expectPolicyError(conn, { type: "steer", text: "must not queue review" }, "NON_INTERACTIVE_STEER"); @@ -253,8 +304,10 @@ test.describe("authenticated WebSocket session write policy", () => { } await expectModelControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); await expectTitleControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + await expectTaskControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); + expectNoTaskControlPaths(taskControlSpies); await expectReadFramesStillWork(conn); @@ -279,6 +332,7 @@ test.describe("authenticated WebSocket session write policy", () => { expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); + expectNoTaskControlPaths(taskControlSpies); } finally { live.status = "idle"; enqueuePrompt.mockRestore(); @@ -292,6 +346,7 @@ test.describe("authenticated WebSocket session write policy", () => { compact.mockRestore(); restoreModelControlSpies(modelControlSpies); restoreTitleControlSpies(titleControlSpies); + restoreTaskControlSpies(taskControlSpies); conn.close(); await deleteSession(sessionId); } From d9f23ef010af9f9a18d99c4e56853c6a1ca716a1 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:04:50 +0100 Subject: [PATCH 22/24] test: isolate task policy lookup assertions Co-authored-by: bobbit-ai --- tests2/integration/session-ws-write-policy.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests2/integration/session-ws-write-policy.test.ts b/tests2/integration/session-ws-write-policy.test.ts index d777f4501..f48bbec58 100644 --- a/tests2/integration/session-ws-write-policy.test.ts +++ b/tests2/integration/session-ws-write-policy.test.ts @@ -184,6 +184,10 @@ function spyOnTaskControlPaths(gateway: any) { }; } +function clearTaskControlSpies(spies: ReturnType): void { + for (const spy of Object.values(spies)) spy.mockClear(); +} + function expectNoTaskControlPaths(spies: ReturnType): void { for (const spy of Object.values(spies)) expect(spy).not.toHaveBeenCalled(); } @@ -237,7 +241,9 @@ test.describe("authenticated WebSocket session write policy", () => { } await expectModelControlsRejected(conn, "SESSION_READ_ONLY"); await expectTitleControlsRejected(conn, "SESSION_READ_ONLY"); + clearTaskControlSpies(taskControlSpies); await expectTaskControlsRejected(conn, "SESSION_READ_ONLY"); + expectNoTaskControlPaths(taskControlSpies); await expectExtensionWritesRejected(conn, "SESSION_READ_ONLY", "read-only"); expect(enqueuePrompt).not.toHaveBeenCalled(); @@ -251,7 +257,6 @@ test.describe("authenticated WebSocket session write policy", () => { expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); - expectNoTaskControlPaths(taskControlSpies); await expectReadFramesStillWork(conn); } finally { live.status = "idle"; @@ -304,10 +309,11 @@ test.describe("authenticated WebSocket session write policy", () => { } await expectModelControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); await expectTitleControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + clearTaskControlSpies(taskControlSpies); await expectTaskControlsRejected(conn, "NON_INTERACTIVE_WORK_CONTROL"); + expectNoTaskControlPaths(taskControlSpies); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); - expectNoTaskControlPaths(taskControlSpies); await expectReadFramesStillWork(conn); @@ -332,7 +338,6 @@ test.describe("authenticated WebSocket session write policy", () => { expect(compact).not.toHaveBeenCalled(); expectNoModelControlMutations(modelControlSpies); await expectNoTitleControlMutations(titleControlSpies); - expectNoTaskControlPaths(taskControlSpies); } finally { live.status = "idle"; enqueuePrompt.mockRestore(); From 0b8daadb4f689beea4598a00fdc3a3a202330189 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:48:42 +0100 Subject: [PATCH 23/24] Document cold session timing results Co-authored-by: bobbit-ai --- docs/design/boot-timing-instrumentation.md | 149 ++++++++++++++++++--- 1 file changed, 130 insertions(+), 19 deletions(-) diff --git a/docs/design/boot-timing-instrumentation.md b/docs/design/boot-timing-instrumentation.md index 8f5e95bad..563093583 100644 --- a/docs/design/boot-timing-instrumentation.md +++ b/docs/design/boot-timing-instrumentation.md @@ -1,17 +1,18 @@ # Boot/reload performance instrumentation -Opt-in instrumentation that measures the cost of a full page reload with hard -numbers, so we can reason about reload disruption (e.g. while an agent is -editing UI code under Vite) with data instead of estimates. +Opt-in instrumentation that measures full-page reload and session transcript +hydration costs with hard numbers, so reload and cold-navigation changes can be +evaluated with data instead of estimates. ## Why A full reload under Vite dev re-does two expensive things: re-evaluating the unbundled module graph (the dev module waterfall) and rehydrating session state -(WebSocket reconnect + `get_state` snapshot replay + full `MessageList` -re-render). Both resist a small time budget, and the snapshot replay scales with -transcript length. Rather than guess, this feature records named milestones -across a reload and writes them somewhere agents can inspect. +(WebSocket reconnect + `get_messages` transcript snapshot replay + full +`MessageList` re-render). Cold session navigation uses the same transcript path. +Both resist a small time budget, and snapshot rendering scales with transcript +length. Rather than guess, this feature records named milestones and writes them +somewhere agents can inspect. ## Surface @@ -35,10 +36,10 @@ across a reload and writes them somewhere agents can inspect. `initApp-start`, `first-render-call` (main.ts), `first-paint` (`pwa-lifecycle.ts::finalizeBoot`, when `#app` actually paints), `ws-open`, `auth-ok` (WS auth handshake done — splits the ws-open→snapshot window into - handshake vs. server-side snapshot wait), `snapshot-received(N msgs)`, - `snapshot-applied`, `post-snapshot-paint` (`remote-agent.ts`). The raw - snapshot frame size is captured as `snapshotChars` to distinguish payload - transfer cost from server-side assembly. + handshake vs. server-side snapshot wait), `get-messages-sent`, + `snapshot-received(N msgs)`, `snapshot-applied`, `post-snapshot-paint` + (`remote-agent.ts`). The raw snapshot frame size is captured as `snapshotChars` + to distinguish payload transfer cost from server-side assembly. - **Server-side snapshot breakdown** (dev harness only): the `get_messages` handler attaches a `SnapshotServerTiming` (`rpcMs` agent assembly / `pipelineMs` server transform / `stampMs` / `stringifyMs` / `bytes` / @@ -69,13 +70,123 @@ Each JSONL line carries `reason`, `isReload`, `total_ms`, `route`, `sessionId`, `Δ prev (ms)` deltas — the delta column shows where the time actually goes (module waterfall vs. first paint vs. snapshot replay). +## Cold session navigation invariant + +A cold switch to an existing session must start transcript transfer before +unrelated REST work. `connectToSession` in the session manager therefore owns +the initial sequence: + +1. After initial WebSocket authentication resolves, call + `RemoteAgent.requestMessages()` as the first post-auth action. This emits + `get-messages-sent` and sends `get_messages`. +2. Only then may side-panel workspace, proposal, draft, git, project/goal, or + session-list REST hydration start. +3. Bind the transcript-bearing agent to the chat panel, then perform the single + initial side-panel workspace hydration. + +This ordering matters because the transcript is the primary session content; +side-panel latency must not hold it behind an independent REST request. The +initial `RemoteAgent` auth handler does not hydrate the workspace. `RemoteAgent` +retains ownership of workspace hydration after non-initial authentication, so +reconnects still refresh server state without duplicating the normal cold-load +fetch. + +Workspace responses remain session-keyed. They can refresh the abandoned +session's keyed cache, but compatibility/foreground mirrors update only when +that session is still active. The session manager also checks the captured +switch generation and selected session after asynchronous boundaries. Together, +these guards prevent a late A response from overwriting B during an A→B switch, +while preserving the exact cached agent and panel on A→B→A switch-back. The +scheduling change does not remove hydration: side-panel tabs, active tab, size +mode, proposals, and review documents still restore through their existing +guarded paths. Workspace revision and conflict rules are unchanged. + +`tests2/dom/cold-session-workspace-ordering-repro.test.ts` pins this contract. It +holds workspace hydration open while verifying that all 321 transcript rows +render, `get_messages` precedes relevant REST requests, and only one initial +workspace GET occurs. It also covers rapid navigation, pre-bind stale responses, +workspace/review restoration, reconnect hydration, and cached switch-back. + +## Controlled cold-navigation measurement + +This is an auditable historical comparison, not a timing threshold for arbitrary +hosts. The fixed tree was +`a4a811d75d05c76aabdab2108863a42c2b058cb5`; the measured pre-fix tree was +`3ee000bb06b12e1f3bc5b80573cca9a67bafa427`. In that pre-fix tree, +`connectToSession` awaited workspace hydration before requesting messages, while +the initial `RemoteAgent` auth handler independently started a second workspace +hydration. + +### Environment and method + +- Windows 11 `10.0.26200` x64; AMD Ryzen AI 9 HX 370; 24 logical CPUs; 63.1 GiB + RAM. +- Node 24.13.1 / V8 13.6; Vitest 4.1.10 with happy-dom; one worker and + `retry: 0`. +- A 321-record transcript and a real 250 ms `setTimeout` in the workspace + endpoint were used for seven sequential cold samples per variant. Controlled + WebSocket snapshots were delivered in a microtask, intentionally removing + real server and network transfer variance. +- The navigation clock started immediately before `connectToSession`. Production + code emitted `auth-ok`, `get-messages-sent`, `snapshot-received(321 msgs)`, and + `snapshot-applied`. Ready meant that transcript row 321 was present in the DOM + and two `requestAnimationFrame` turns had completed, matching the + post-snapshot-paint boundary. +- Baseline assertions required auth→request to be at least 235 ms and observed + two workspace GETs in every baseline sample. This proved the fixture was + exercising the blocking and duplicate-hydration regression rather than merely + comparing noisy runs. + +### Raw samples + +All timing values are milliseconds; values within each cell are in run order. + +| Metric | Pre-fix | Fixed | +|---|---|---| +| auth→request | `[255.375, 251.248, 253.282, 255.059, 259.189, 254.873, 261.206]` | `[0.471, 0.017, 0.011, 0.015, 0.015, 0.022, 0.014]` | +| auth→snapshot | `[255.935, 251.566, 254.032, 255.401, 259.689, 255.894, 261.578]` | `[1.063, 0.290, 0.256, 0.254, 0.260, 0.543, 0.329]` | +| navigation→ready | `[271.247, 258.330, 260.970, 259.569, 267.590, 269.112, 267.751]` | `[16.519, 5.747, 7.079, 3.856, 6.813, 5.835, 4.090]` | +| Workspace GETs | `2` in every sample | `1` in every sample | + +### Median result + +| Metric | Pre-fix | Fixed | Change | +|---|---:|---:|---:| +| auth→request | 255.059 ms | 0.015 ms | −255.044 ms | +| auth→snapshot | 255.894 ms | 0.290 ms | −255.605 ms (−99.9%) | +| navigation→ready | 267.590 ms | 5.835 ms | −261.756 ms (−97.8%) | +| Workspace GETs | 2 | 1 | one initial owner | + +The fixed transcript became ready about 244 ms before the delayed workspace +response. For the fixed 321-record case, the median request→receipt time was +0.273 ms, snapshot receipt→apply was 0.252 ms, and receipt→ready was 4.862 ms. +The result isolates the intended change: transcript startup no longer inherits +workspace latency, and duplicate initial hydration is gone. + +### Transcript-size scaling + +Fixed-tree medians from the same controlled setup show the remaining local cost: + +| Records | request→receipt | receipt→apply | receipt→ready | navigation→ready | +|---:|---:|---:|---:|---:| +| 1 | 0.084 ms | 0.025 ms | 0.565 ms | 1.057 ms | +| 101 | 0.174 ms | 0.104 ms | 1.939 ms | 2.558 ms | +| 321 | 0.273 ms | 0.252 ms | 4.862 ms | 5.835 ms | +| 641 | 0.430 ms | 0.440 ms | 12.840 ms | 13.681 ms | + +This controlled DOM benchmark excludes real server assembly and network transfer +latency, both of which remain after the request is sent immediately. Within the +controlled path, rendering is now the dominant transcript-size-dependent local +cost. This fix intentionally makes no snapshot protocol change. If real large +histories show material latency, evaluate pagination or streaming for transfer +and virtualized rendering for local paint as follow-up work. + ## Tests -- `tests/dev-boot-timing.test.ts` — sink: append/parse, dir creation, limit, - rejection of non-object/oversized samples, byte-cap trimming, malformed-line - skipping. -- `tests/e2e/dev-boot-timing-api.spec.ts` — endpoint gating (403 off-harness), - write+read-back under the harness, 422 on a non-object body. -- `tests/e2e/ui/perf-instrumentation-toggle.spec.ts` — toggle hidden without the - harness; visible/toggles/persists-across-reload and arms reload - instrumentation under the harness. +- `tests2/dom/cold-session-workspace-ordering-repro.test.ts` — cold transcript + ordering, one initial workspace owner, stale navigation, reconnect, and cache + preservation. +- `tests2/core/dev-boot-timing.test.ts` — sink append/parse, directory creation, + limits, malformed entries, and trimming. +- `tests2/integration/dev-boot-timing-api.test.ts` — endpoint gating, write/read + under the harness, and invalid-body rejection. From abf9d3cb313c89f8609d3246e1b5d62b41cbce7b Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:54:31 +0100 Subject: [PATCH 24/24] Clarify session policy and timing evidence Co-authored-by: bobbit-ai --- docs/design/boot-timing-instrumentation.md | 82 ++++++++++++++++------ docs/websocket-protocol.md | 74 +++++++++++++++++++ 2 files changed, 133 insertions(+), 23 deletions(-) diff --git a/docs/design/boot-timing-instrumentation.md b/docs/design/boot-timing-instrumentation.md index 563093583..373a3889b 100644 --- a/docs/design/boot-timing-instrumentation.md +++ b/docs/design/boot-timing-instrumentation.md @@ -105,17 +105,28 @@ guarded paths. Workspace revision and conflict rules are unchanged. holds workspace hydration open while verifying that all 321 transcript rows render, `get_messages` precedes relevant REST requests, and only one initial workspace GET occurs. It also covers rapid navigation, pre-bind stale responses, -workspace/review restoration, reconnect hydration, and cached switch-back. - -## Controlled cold-navigation measurement - -This is an auditable historical comparison, not a timing threshold for arbitrary -hosts. The fixed tree was -`a4a811d75d05c76aabdab2108863a42c2b058cb5`; the measured pre-fix tree was -`3ee000bb06b12e1f3bc5b80573cca9a67bafa427`. In that pre-fix tree, -`connectToSession` awaited workspace hydration before requesting messages, while -the initial `RemoteAgent` auth handler independently started a second workspace -hydration. +workspace/review restoration, reconnect hydration, and cached switch-back. The +committed test pins the 321-row render-before-workspace and single-fetch behavior; +it does not run the seven timing samples, impose the benchmark's 250 ms delay, or +assert the measurements below. + +## Controlled one-off cold-navigation evidence + +The following numbers are controlled one-off benchmark evidence, not a +repository-reproducible benchmark or a timing threshold for arbitrary hosts. The +fixed tree was `a4a811d75d05c76aabdab2108863a42c2b058cb5`; the measured +pre-fix tree was `3ee000bb06b12e1f3bc5b80573cca9a67bafa427`. In the pre-fix +tree, `connectToSession` awaited workspace hydration before requesting messages, +while the initial `RemoteAgent` auth handler independently started a second +workspace hydration. + +The exact source SHAs, environment, raw values, and method keep the result +interpretable. However, the measurement harness and Vitest config lived under +ignored `.bobbit/tmp` paths and were not retained in the repository. Re-running +this comparison requires recreating that temporary fixture from the committed +DOM regression test and running it against the actual fixed and pre-fix source +trees; the commands below are therefore historical invocation records, not +turnkey repeat commands. ### Environment and method @@ -123,19 +134,44 @@ hydration. RAM. - Node 24.13.1 / V8 13.6; Vitest 4.1.10 with happy-dom; one worker and `retry: 0`. -- A 321-record transcript and a real 250 ms `setTimeout` in the workspace - endpoint were used for seven sequential cold samples per variant. Controlled - WebSocket snapshots were delivered in a microtask, intentionally removing - real server and network transfer variance. -- The navigation clock started immediately before `connectToSession`. Production - code emitted `auth-ok`, `get-messages-sent`, `snapshot-received(321 msgs)`, and - `snapshot-applied`. Ready meant that transcript row 321 was present in the DOM - and two `requestAnimationFrame` turns had completed, matching the - post-snapshot-paint boundary. +- The ignored fixture was derived from + `tests2/dom/cold-session-workspace-ordering-repro.test.ts`. It used a + 321-record transcript and a real 250 ms `setTimeout` in the workspace endpoint + for seven sequential cold samples per variant. +- Controlled WebSocket snapshots were delivered in a microtask. This excludes + real network latency, server assembly, and transport time so the comparison + isolates client request ordering and local rendering. +- The clock started immediately before `connectToSession`, so this was a DOM cold + session switch—not a full app reload or deep-link. In contrast, the normal + full reload/deep-link instrumentation described above begins at browser + navigation, includes module and app boot, and writes its completed samples to + the JSONL sink. +- Production code emitted `auth-ok`, `get-messages-sent`, + `snapshot-received(321 msgs)`, and `snapshot-applied`. Ready meant transcript + row 321 was present in the DOM and two `requestAnimationFrame` turns had + completed, matching the post-snapshot-paint boundary. - Baseline assertions required auth→request to be at least 235 ms and observed - two workspace GETs in every baseline sample. This proved the fixture was - exercising the blocking and duplicate-hydration regression rather than merely - comparing noisy runs. + two workspace GETs in every baseline sample. This proved the fixture exercised + the blocking and duplicate-hydration regression rather than merely comparing + noisy runs. + +### Recorded commands + +Fixed variant: + +```bash +MEASURE_VARIANT=fixed MEASURE_SOURCE_SHA=a4a811d75d05c76aabdab2108863a42c2b058cb5 MEASURE_TRANSCRIPT_SIZE=321 MEASURE_SAMPLE_COUNT=7 MEASURE_WORKSPACE_DELAY_MS=250 node node_modules/vitest/vitest.mjs run --config .bobbit/tmp/cold-load-measure/vitest.measure.config.ts +``` + +Baseline variant, run from a temporary detached worktree at the pre-fix SHA: + +```bash +MEASURE_ROOT=.bobbit/tmp/cold-baseline MEASURE_VARIANT=baseline MEASURE_SOURCE_SHA=3ee000bb06b12e1f3bc5b80573cca9a67bafa427 MEASURE_TRANSCRIPT_SIZE=321 MEASURE_SAMPLE_COUNT=7 MEASURE_WORKSPACE_DELAY_MS=250 node node_modules/vitest/vitest.mjs run --config .bobbit/tmp/cold-baseline/.bobbit/tmp/cold-load-measure/vitest.measure.config.ts +``` + +The referenced configs, generated measurement test, and temporary baseline +worktree are absent from the repository. Recreate them before using these +invocations. ### Raw samples diff --git a/docs/websocket-protocol.md b/docs/websocket-protocol.md index 872104474..929dd36d1 100644 --- a/docs/websocket-protocol.md +++ b/docs/websocket-protocol.md @@ -47,6 +47,80 @@ Bobbit keeps normal streaming responsive by bounding payloads before they enter Overflow diagnostics include `outerType`, `innerType` for `{ type: "event" }` frames, serialized `bytes`, recipient kind, and context such as `goalId`. These fields are the first place to look when a reconnect storm follows verification or reviewer activity. +## Authenticated session work policy + +Live session sockets enforce `readOnly` and `nonInteractive` at the authenticated +transport boundary, not only in the browser UI. The server checks both the live +session and its persisted row; `true` in either source activates the restriction. +This closes the window where one representation has updated before the other. +If both restrictions apply, `readOnly` takes precedence. + +The guarded work classifier contains these frames: + +- Agent and queue work: `prompt`, `steer`, `steer_queued`, `remove_queued`, + `reorder_queue`, `retry`, `restart_agent`, `compact`. +- Metadata and model writes: `set_title`, `generate_title`, + `summarize_goal_title`, `set_model`, `set_image_model`, + `set_thinking_level`. +- Durable task and permission writes: `task_create`, `task_update`, + `task_delete`, `grant_tool_permission`. +- Extension session writes: `ext_session_write_permit`, `ext_session_post`. + +### Read-only sessions + +Every guarded frame is rejected with `SESSION_READ_ONLY`. Ordinary guarded +frames receive the generic error envelope: + +```json +{ "type": "error", "code": "SESSION_READ_ONLY", "message": "..." } +``` + +Extension session-write callers require a correlated response so their Host API +call does not wait for a generic error until timeout. They receive: + +```json +{ "type": "ext_session_write_permit_result", "requestId": "...", "ok": false, "error": "SESSION_READ_ONLY" } +``` + +or: + +```json +{ "type": "ext_session_post_result", "requestId": "...", "ok": false, "error": "SESSION_READ_ONLY" } +``` + +A read-only session has no streaming-steer exception. + +### Non-interactive sessions + +The response code identifies the rejected class: + +| Frame | Policy code | +|---|---| +| `prompt` | `NON_INTERACTIVE_PROMPT` | +| `steer` while the current status is not `streaming` | `NON_INTERACTIVE_STEER` | +| `steer_queued`, `remove_queued`, `reorder_queue` | `NON_INTERACTIVE_QUEUE_CONTROL` | +| Every other guarded frame | `NON_INTERACTIVE_WORK_CONTROL` | + +A direct `steer` is permitted only while the session's current status is +`streaming`; this allows an active automated review to be redirected without +starting or queueing new reviewer work. The exception does not extend to +`retry`, queue controls, extension posts, or any other guarded frame. + +Ordinary frames receive `{ "type": "error", "code": "", +"message": "..." }`. As with read-only policy, `ext_session_write_permit` and +`ext_session_post` instead return their respective request-correlated result +envelope with the policy code in `error`. + +### Classifier scope + +`get_state`, `get_messages`, and `ping` remain available under these policies. +`abort` and `deny_tool_permission` also remain outside the guarded work +classifier because they decrease or stop active work. `resume` and the +separately authorized extension channel and surface operations are likewise not +classified here. This section documents only the session-work policy; those +frames can still be rejected by their own authentication, authorization, +lifecycle, validation, size, or replay rules. + ## Client → Server | Type | Fields | Description |