diff --git a/src/client/src/controllers/sessionController.startupDialogs.test.ts b/src/client/src/controllers/sessionController.startupDialogs.test.ts new file mode 100644 index 0000000..e533596 --- /dev/null +++ b/src/client/src/controllers/sessionController.startupDialogs.test.ts @@ -0,0 +1,301 @@ +import { describe, expect, it, vi } from "vitest"; +import { initialAppState } from "../appState"; +import type { ExtensionDialogCloseResponse, ExtensionDialogKind, PendingExtensionDialog } from "../api"; +import { SessionController } from "./sessionController"; +import { defaultApi, deferred, EmitSocket, emptyPage, oldSession, runPendingAnimationFrames, sessionLookupId, status, workspace, type AppState, type SessionActivity, type SessionInfo, type SessionStatus } from "./sessionController.testSupport"; + +const BACKEND_SESSION_ID = "backend-session"; + +function startupActivity(patch: Partial = {}): SessionActivity { + return { + sessionId: BACKEND_SESSION_ID, + phase: "active", + label: "Creating session", + detail: "Loading session extensions", + at: "2026-07-20T00:00:01.000Z", + startup: true, + ...patch, + }; +} + +function dialog(dialogId: string, kind: ExtensionDialogKind = "confirm"): PendingExtensionDialog { + return { + dialogId, + kind, + title: `Dialog ${dialogId}`, + ...(kind === "confirm" ? { message: "Are you sure?" } : {}), + askedAt: "2026-07-20T00:00:00.000Z", + runScoped: false, + }; +} + +function statusWithDialogs(sessionId: string, pendingDialogs: PendingExtensionDialog[]): SessionStatus { + return { ...status(sessionId), pendingDialogs }; +} + +function closeResponse(sessionStatus: SessionStatus, dialogId = "dialog-1"): ExtensionDialogCloseResponse { + return { + result: "closed", + outcome: { + dialogId, + reason: "answered", + answer: true, + askedAt: "2026-07-20T00:00:00.000Z", + closedAt: "2026-07-20T00:01:00.000Z", + }, + sessionStatus, + }; +} + +interface PendingStartHarness { + controller: SessionController; + socket: EmitSocket; + startRequest: ReturnType>; + state: { current: AppState }; +} + +/** + * A controller with one in-flight create whose start request stays open until + * the test resolves it — the browser side of a `session_start` dialog parking + * session readiness. + */ +function pendingStartController(state: { current: AppState }, api: Partial = {}): PendingStartHarness { + const startRequest = deferred(); + const socket = new EmitSocket(); + const controller = new SessionController( + () => state.current, + (patch) => { state.current = { ...state.current, ...patch }; }, + () => undefined, + undefined, + { + api: { + ...defaultApi, + startSession: () => startRequest.promise, + messages: () => Promise.resolve(emptyPage), + status: (session) => Promise.resolve(status(sessionLookupId(session))), + streamSnapshot: () => Promise.resolve({ seq: 0, partial: null }), + thinkingLevels: () => Promise.resolve({ levels: [] }), + ...api, + }, + socket, + }, + ); + return { controller, socket, startRequest, state }; +} + +function beginPendingStart(harness: PendingStartHarness): { start: Promise; tempId: string } { + const start = harness.controller.startSession(); + const tempId = harness.state.current.selectedSession?.id; + if (tempId === undefined) throw new Error("Expected a pending-start row to be selected"); + if (!tempId.startsWith("pending-session-")) throw new Error("Expected a pending-start row to be selected"); + return { start, tempId }; +} + +function reportBackendSessionId(harness: PendingStartHarness, tempId: string): void { + harness.controller.applyGlobalEvent({ type: "session.startup", startupToken: tempId, activity: startupActivity() }); + runPendingAnimationFrames(); +} + +function resolveBackendSession(harness: PendingStartHarness): void { + harness.startRequest.resolve({ ...oldSession, id: BACKEND_SESSION_ID, path: "/tmp/backend-session.jsonl" }); +} + +describe("SessionController session_start dialog startup reachability", () => { + it("subscribes to the backend session as soon as startup progress names it", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const statusCalls: string[] = []; + const harness = pendingStartController(state, { + status: (session) => { + statusCalls.push(sessionLookupId(session)); + return Promise.resolve(status(sessionLookupId(session))); + }, + }); + const { start, tempId } = beginPendingStart(harness); + expect(harness.socket.connectedSessionIds).toEqual([]); + + reportBackendSessionId(harness, tempId); + + expect(harness.socket.connectedSessionIds).toEqual([BACKEND_SESSION_ID]); + await vi.waitFor(() => { expect(statusCalls).toEqual([BACKEND_SESSION_ID]); }); + resolveBackendSession(harness); + await start; + }); + + it("shows a dialog that opens mid-startup on the pending row, answerable before readiness", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const harness = pendingStartController(state); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + + expect(harness.state.current.selectedSession?.id).toBe(tempId); + expect(harness.state.current.pendingDialogs).toEqual([dialog("dialog-1")]); + resolveBackendSession(harness); + await start; + }); + + it("recovers a dialog that opened before the subscription from the mid-startup status", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const harness = pendingStartController(state, { + status: (session) => Promise.resolve(sessionLookupId(session) === BACKEND_SESSION_ID ? statusWithDialogs(BACKEND_SESSION_ID, [dialog("dialog-1")]) : status(sessionLookupId(session))), + }); + const { start, tempId } = beginPendingStart(harness); + + reportBackendSessionId(harness, tempId); + + await vi.waitFor(() => { expect(harness.state.current.pendingDialogs).toEqual([dialog("dialog-1")]); }); + // The per-session map holds the backend session's status for the readiness + // swap, while the row keeps its own temporary identity. + expect(harness.state.current.sessionStatuses[BACKEND_SESSION_ID]?.pendingDialogs).toEqual([dialog("dialog-1")]); + expect(harness.state.current.selectedSession?.id).toBe(tempId); + resolveBackendSession(harness); + await start; + }); + + it("tolerates a daemon that cannot serve status mid-startup", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + // Older daemons 404 the status route until the session is ready; the + // rejection must not disturb the event-driven dialog flow. + let createResolved = false; + const harness = pendingStartController(state, { + status: (session) => sessionLookupId(session) === BACKEND_SESSION_ID && !createResolved + ? Promise.reject(new Error("Session not found")) + : Promise.resolve(status(sessionLookupId(session))), + }); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + await vi.waitFor(() => { expect(harness.state.current.pendingDialogs).toEqual([dialog("dialog-1")]); }); + + expect(harness.state.current.error).toBe(""); + createResolved = true; + resolveBackendSession(harness); + await start; + }); + + it("answers a startup dialog through the real session id and proceeds to the chat view at readiness", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const answerCalls: { sessionId: string; dialogId: string; value: unknown; machineId: string }[] = []; + const harness = pendingStartController(state, { + answerDialog: (session, dialogId, value, machineId) => { + answerCalls.push({ sessionId: sessionLookupId(session), dialogId, value, machineId: machineId ?? "local" }); + return Promise.resolve(closeResponse(status(BACKEND_SESSION_ID))); + }, + }); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + + await harness.controller.answerDialog("dialog-1", true); + + expect(answerCalls).toEqual([{ sessionId: BACKEND_SESSION_ID, dialogId: "dialog-1", value: true, machineId: "local" }]); + expect(harness.state.current.pendingDialogs).toEqual([]); + expect(harness.state.current.closedDialogs).toEqual([{ dialog: dialog("dialog-1"), reason: "answered", answer: true }]); + expect(harness.state.current.error).toBe(""); + + // The answer settled the hook daemon-side, so the create resolves and the + // normal selection flow takes over the now-real session. + resolveBackendSession(harness); + await start; + await vi.waitFor(() => { expect(harness.state.current.selectedSession?.id).toBe(BACKEND_SESSION_ID); }); + expect(harness.state.current.sessions.some((session) => session.id === tempId)).toBe(false); + expect(harness.state.current.sessions.some((session) => session.id === BACKEND_SESSION_ID)).toBe(true); + }); + + it("cancels a startup dialog through the cancel route under the real session id", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const cancelCalls: { sessionId: string; dialogId: string }[] = []; + const harness = pendingStartController(state, { + cancelDialog: (session, dialogId) => { + cancelCalls.push({ sessionId: sessionLookupId(session), dialogId }); + return Promise.resolve({ + result: "closed" as const, + outcome: { dialogId, reason: "cancelled" as const, askedAt: "2026-07-20T00:00:00.000Z", closedAt: "2026-07-20T00:01:00.000Z" }, + sessionStatus: status(BACKEND_SESSION_ID), + }); + }, + }); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + + await harness.controller.cancelDialog("dialog-1"); + + expect(cancelCalls).toEqual([{ sessionId: BACKEND_SESSION_ID, dialogId: "dialog-1" }]); + expect(harness.state.current.pendingDialogs).toEqual([]); + expect(harness.state.current.closedDialogs).toEqual([{ dialog: dialog("dialog-1"), reason: "cancelled" }]); + resolveBackendSession(harness); + await start; + }); + + it("trusts the returned status when the answer loses the race", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const harness = pendingStartController(state, { + answerDialog: () => Promise.resolve({ result: "stale", sessionStatus: statusWithDialogs(BACKEND_SESSION_ID, [dialog("dialog-2")]) }), + }); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + + await harness.controller.answerDialog("dialog-1", true); + + expect(harness.state.current.error).toBe(""); + expect(harness.state.current.closedDialogs).toEqual([]); + expect(harness.state.current.pendingDialogs.map((pending) => pending.dialogId)).toEqual(["dialog-2"]); + resolveBackendSession(harness); + await start; + }); + + it("cannot answer before startup progress names the backend session", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + let answered = false; + const harness = pendingStartController(state, { + answerDialog: () => { + answered = true; + return Promise.resolve(closeResponse(status(BACKEND_SESSION_ID))); + }, + }); + const { start } = beginPendingStart(harness); + + await harness.controller.answerDialog("dialog-1", true); + + expect(answered).toBe(false); + resolveBackendSession(harness); + await start; + }); + + it("re-subscribes when the pending row is re-selected mid-startup", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [oldSession] } }; + const harness = pendingStartController(state); + const { start, tempId } = beginPendingStart(harness); + reportBackendSessionId(harness, tempId); + expect(harness.socket.connectedSessionIds).toEqual([BACKEND_SESSION_ID]); + + await harness.controller.selectSession(oldSession, { updateUrl: false }); + const pendingRow = harness.state.current.sessions.find((session) => session.id === tempId); + if (pendingRow === undefined) throw new Error("Expected the pending row to stay in the session list"); + await harness.controller.selectSession(pendingRow, { updateUrl: false }); + + expect(harness.socket.connectedSessionIds).toEqual([BACKEND_SESSION_ID, oldSession.id, BACKEND_SESSION_ID]); + // Dialog state keeps flowing after the detour. + harness.socket.emit({ type: "dialog.opened", dialog: dialog("dialog-1") }); + expect(harness.state.current.pendingDialogs).toEqual([dialog("dialog-1")]); + resolveBackendSession(harness); + await start; + }); + + it("does not subscribe for another browser's create", async () => { + const state = { current: { ...initialAppState(), selectedWorkspace: workspace, sessions: [] } }; + const harness = pendingStartController(state); + const { start } = beginPendingStart(harness); + + harness.controller.applyGlobalEvent({ type: "session.startup", startupToken: "pending-session-9-other-tab", activity: startupActivity() }); + runPendingAnimationFrames(); + + expect(harness.socket.connectedSessionIds).toEqual([]); + resolveBackendSession(harness); + await start; + }); +}); diff --git a/src/client/src/controllers/sessionController.ts b/src/client/src/controllers/sessionController.ts index ee2e250..127c9c8 100644 --- a/src/client/src/controllers/sessionController.ts +++ b/src/client/src/controllers/sessionController.ts @@ -84,6 +84,13 @@ interface PendingSessionStart { session: ClientPendingStartSessionInfo; queuedSends: QueuedPendingSessionSend[]; discarded: boolean; + /** + * The real session id, learned from the daemon's `session.startup` events + * long before the create request resolves. It is what lets the startup view + * subscribe to the constructing session and answer its `session_start` + * dialogs — the dialogs that gate the readiness the create request waits on. + */ + backendSessionId?: string; } interface SuppressedCreatedSession { @@ -912,7 +919,11 @@ export class SessionController { private async closeOpenDialog(dialogId: string, close: (session: SessionInfo, machineId: string) => Promise): Promise { const state = this.getState(); const session = state.selectedSession; - if (session === undefined || session.archived === true || isClientPendingStartSessionInfo(session)) return; + if (session === undefined || session.archived === true) return; + if (isClientPendingStartSessionInfo(session)) { + await this.closePendingStartDialog(session, dialogId, close); + return; + } const machineId = selectedMachineId(state); const selectionSeq = this.selectionSeq; try { @@ -934,6 +945,36 @@ export class SessionController { } } + /** + * Answer or cancel a `session_start` dialog from the startup view. The row + * is still the pending start, but the dialog belongs to the constructing + * backend session the startup events named, so the close goes out under the + * real id — the only route the daemon can serve before readiness. + */ + private async closePendingStartDialog(session: ClientPendingStartSessionInfo, dialogId: string, close: (session: SessionInfo, machineId: string) => Promise): Promise { + const pending = this.pendingSessionStarts.get(session.id); + const backendSessionId = pending?.backendSessionId; + // Without the real id there is no route to answer through — and no way a + // dialog card could be on screen yet either. + if (pending === undefined || backendSessionId === undefined) return; + const selectionSeq = this.selectionSeq; + try { + const response = await close({ ...session, id: backendSessionId }, pending.machineId); + if (selectionSeq !== this.selectionSeq || this.getState().selectedSession?.id !== session.id) return; + // Same outcome-first ordering as the ready-session path: the card shows + // what the user gave, and the daemon's dialog.closed frame then finds + // the dialog already closed here and stays a no-op. + const outcome: ExtensionDialogOutcome | undefined = response.outcome; + if (outcome !== undefined) { + const dialog = this.getState().pendingDialogs.find((candidate) => candidate.dialogId === outcome.dialogId); + if (dialog !== undefined) this.recordClosedDialog({ dialog, reason: outcome.reason, ...(outcome.answer === undefined ? {} : { answer: outcome.answer }) }); + } + this.applyPendingStartStatus(pending, response.sessionStatus); + } catch (error) { + if (selectionSeq === this.selectionSeq && this.getState().selectedSession?.id === session.id) this.setState({ error: String(error) }); + } + } + private async closeOpenAsk(askId: string, close: (session: SessionInfo, machineId: string) => Promise): Promise { const state = this.getState(); const session = state.selectedSession; @@ -1120,6 +1161,9 @@ export class SessionController { ...(activity === undefined ? {} : { sessionActivities: { ...state.sessionActivities, [session.id]: activity } }), error: "", }); + // Re-selecting the row mid-startup re-establishes the constructing + // session's subscription; the close above dropped it with the old selection. + if (pendingStart?.backendSessionId !== undefined) this.connectPendingStartSocket(pendingStart); if (options?.updateUrl !== false) this.updateUrl(); } @@ -1492,6 +1536,7 @@ export class SessionController { } const pending = event.startupToken === undefined ? undefined : this.pendingSessionStarts.get(event.startupToken); if (pending === undefined || pending.discarded) return; + this.learnPendingStartBackendSession(pending, event.activity.sessionId); // An idle startup phase means the daemon has nothing left to attribute, so // restore this row's own generic wording rather than clearing the text of a // creation request that has not returned yet. @@ -1500,6 +1545,95 @@ export class SessionController { : pendingStartActivity(event.activity, pending.tempId)); } + private learnPendingStartBackendSession(pending: PendingSessionStart, sessionId: string): void { + if (sessionId === "" || pending.backendSessionId !== undefined) return; + pending.backendSessionId = sessionId; + if (this.getState().selectedSession?.id === pending.tempId) this.connectPendingStartSocket(pending); + } + + /** + * Subscribe the selected pending-start row to its constructing session. + * `session_start` dialogs park the create request until answered, so waiting + * for readiness to subscribe would make them unanswerable; the per-session + * event channel and (on this daemon version) the status route both serve a + * session whose startup is still waiting on the user. + */ + private connectPendingStartSocket(pending: PendingSessionStart): void { + const backendSessionId = pending.backendSessionId; + if (backendSessionId === undefined || pending.discarded) return; + const ref: SessionRef = { id: backendSessionId, cwd: pending.cwd }; + this.socket.connect( + ref, + (event) => { this.applyPendingStartEvent(pending, event); }, + () => { this.resyncPendingStartDialogs(pending); }, + pending.machineId, + ); + this.resyncPendingStartDialogs(pending); + } + + /** + * Recover dialogs that opened before this subscription connected (or during + * a reconnect gap) from the daemon's status projection. The HTTP snapshot is + * unordered against socket frames — dialogs the socket already opened or + * closed are newer than anything it can say about them — so only genuinely + * unknown opens are adopted, never a wholesale replace. A daemon that + * predates mid-startup status answers 404 until readiness: tolerated, since + * everything that opens from here still arrives as an event. + */ + private resyncPendingStartDialogs(pending: PendingSessionStart): void { + const backendSessionId = pending.backendSessionId; + if (backendSessionId === undefined) return; + void this.api.status({ id: backendSessionId, cwd: pending.cwd }, pending.machineId).then( + (status) => { + this.applyStatus(status); + if (pending.discarded || this.getState().selectedSession?.id !== pending.tempId) return; + const state = this.getState(); + const knownIds = new Set([ + ...state.pendingDialogs.map((pendingDialog) => pendingDialog.dialogId), + ...state.closedDialogs.map((closed) => closed.dialog.dialogId), + ]); + const recovered = (status.pendingDialogs ?? []).filter((recoveredDialog) => !knownIds.has(recoveredDialog.dialogId)); + if (recovered.length > 0) this.setState({ pendingDialogs: [...state.pendingDialogs, ...recovered] }); + }, + () => undefined, + ); + } + + /** + * Route a constructing session's events onto its pending-start row. Only + * dialog frames and their status reconciliation apply here: everything else + * (transcript, activity, naming) is re-fetched authoritatively by the + * readiness join, and routing it onto a temporary row would pollute state + * keyed for a session that does not exist yet. Frames that arrive after the + * row stopped being the selected pending start belong to the selection flow + * that took over. + */ + private applyPendingStartEvent(pending: PendingSessionStart, event: SessionUiEvent): void { + if (pending.discarded || this.getState().selectedSession?.id !== pending.tempId) return; + if (event.type === "dialog.opened") { + this.applyOpenedDialog(event.dialog); + return; + } + if (event.type === "dialog.closed") { + this.applyClosedDialog(event.dialogId, event.reason, event.answer); + return; + } + if (event.type === "status.update") this.applyPendingStartStatus(pending, event.status); + } + + /** + * Apply a constructing session's status from an ordered channel (the + * socket's own status frame, or a dialog close response): the daemon's + * projection is authoritative there, so the open list is replaced wholesale, + * exactly as applyStatus does for a ready session. The per-session map stays + * truthful too — the readiness swap seeds the selected status from it. + */ + private applyPendingStartStatus(pending: PendingSessionStart, status: SessionStatus): void { + this.applyStatus(status); + if (pending.discarded || this.getState().selectedSession?.id !== pending.tempId) return; + this.setState({ pendingDialogs: status.pendingDialogs ?? [] }); + } + private schedulePendingFlush(): void { if (this.pendingFrame !== undefined) return; this.pendingFrame = requestAnimationFrame(() => { diff --git a/src/server/sessions/piSessionService.extensionDialogs.test.ts b/src/server/sessions/piSessionService.extensionDialogs.test.ts index 88a137f..47e805d 100644 --- a/src/server/sessions/piSessionService.extensionDialogs.test.ts +++ b/src/server/sessions/piSessionService.extensionDialogs.test.ts @@ -520,3 +520,81 @@ describe("PiSessionService extension dialog status projection", () => { await service.dispose(); }); }); + +describe("PiSessionService session_start dialog startup reachability", () => { + /** + * A `session_start` dialog parks session construction before the session + * ever becomes active: the bind below models the issue's probe by awaiting + * a confirm inside extension binding. The dialog must stay reachable — + * statusable and answerable — in that window, or startup could never be + * unblocked from the browser. + */ + function startupDialogService() { + const harness = dialogService(); + const confirmAnswers: (boolean | string | undefined)[] = []; + harness.fake.session.bindExtensions = (bindings) => { + harness.fake.calls.bindExtensions.push(bindings); + if (bindings.uiContext === undefined) return Promise.resolve(); + return bindings.uiContext.confirm("Proceed at startup?", "Really?").then((answer) => { + confirmAnswers.push(answer); + }); + }; + return { ...harness, confirmAnswers }; + } + + async function parkOnStartupDialog(store: PendingExtensionDialogStore): Promise { + await vi.waitFor(() => { + expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toHaveLength(1); + }); + } + + it("serves status for a session still parked on a session_start dialog", async () => { + const { service, store } = startupDialogService(); + const started = service.start("/workspace"); + await parkOnStartupDialog(store); + + const status = await service.status(sessionRef(ACTIVE_SESSION_ID)); + + expect(status.pendingDialogs).toEqual([ + expect.objectContaining({ dialogId: "dialog-1", kind: "confirm", title: "Proceed at startup?", runScoped: false }), + ]); + await service.answerDialog(sessionRef(ACTIVE_SESSION_ID), "dialog-1", true); + await started; + await service.dispose(); + }); + + it("answers a session_start dialog mid-startup so creation can finish", async () => { + const { service, store, confirmAnswers } = startupDialogService(); + const started = service.start("/workspace"); + await parkOnStartupDialog(store); + + const response = await service.answerDialog(sessionRef(ACTIVE_SESSION_ID), "dialog-1", true); + + expect(response.result).toBe("closed"); + expect(response.outcome).toMatchObject({ dialogId: "dialog-1", reason: "answered", answer: true }); + expect(response.sessionStatus.pendingDialogs ?? []).toEqual([]); + const created = await started; + expect(created.id).toBe(ACTIVE_SESSION_ID); + expect(confirmAnswers).toEqual([true]); + expect(store.pendingDialogs(ACTIVE_SESSION_ID)).toEqual([]); + // Readiness handed the session to the active path: a repeat answer races + // lost against the already-closed dialog instead of erroring. + const repeat = await service.answerDialog(sessionRef(ACTIVE_SESSION_ID), "dialog-1", true); + expect(repeat.result).toBe("stale"); + await service.dispose(); + }); + + it("cancels a session_start dialog mid-startup with the kind's cancel value", async () => { + const { service, store, confirmAnswers } = startupDialogService(); + const started = service.start("/workspace"); + await parkOnStartupDialog(store); + + const response = await service.cancelDialog(sessionRef(ACTIVE_SESSION_ID), "dialog-1"); + + expect(response.result).toBe("closed"); + expect(response.outcome).toMatchObject({ dialogId: "dialog-1", reason: "cancelled" }); + await started; + expect(confirmAnswers).toEqual([false]); + await service.dispose(); + }); +}); diff --git a/src/server/sessions/piSessionService.lifecycle.test.ts b/src/server/sessions/piSessionService.lifecycle.test.ts index beeb862..50a7af7 100644 --- a/src/server/sessions/piSessionService.lifecycle.test.ts +++ b/src/server/sessions/piSessionService.lifecycle.test.ts @@ -246,10 +246,15 @@ describe("PiSessionService lifecycle, listing, and reload", () => { const outcomes = await failedLookups; expect(callsWhileOpening).toBe(1); expect(outcomes).toHaveLength(2); - for (const outcome of outcomes) { - expect(outcome.status).toBe("rejected"); - if (outcome.status === "rejected") expect(outcome.reason).toBe(openingError); - } + const [messagesOutcome, statusOutcome] = outcomes; + expect(messagesOutcome.status).toBe("rejected"); + if (messagesOutcome.status === "rejected") expect(messagesOutcome.reason).toBe(openingError); + // Status no longer parks behind the in-flight open: a session still + // binding its extensions is statusable (its session_start dialogs must + // stay answerable for startup to be unblockable at all), so the lookup + // resolves from the startup window rather than sharing the open's fate. + expect(statusOutcome.status).toBe("fulfilled"); + if (statusOutcome.status === "fulfilled") expect(statusOutcome.value).toMatchObject({ sessionId }); expect(service.activeCount()).toBe(0); expect(failed.calls.abort).toBe(1); expect(failed.calls.dispose).toBe(1); diff --git a/src/server/sessions/piSessionService.ts b/src/server/sessions/piSessionService.ts index 478dfb6..01a3ef3 100644 --- a/src/server/sessions/piSessionService.ts +++ b/src/server/sessions/piSessionService.ts @@ -136,6 +136,10 @@ function lookupMatchesActiveSession(ref: PiSessionLookup, active: ActiveSession< return !isPiSessionRef(ref) || cwdPathsEqual(active.runtime.cwd, ref.cwd); } +function lookupMatchesStartupSession(ref: PiSessionLookup, session: PiAgentSession): boolean { + return !isPiSessionRef(ref) || cwdPathsEqual(session.sessionManager.getCwd(), ref.cwd); +} + type QueuedPromptKind = "steer" | "followUp"; interface QueuedPrompt { @@ -755,6 +759,14 @@ export interface PiSessionServiceDependencies { export class PiSessionService implements SessionRouteService { private readonly active = new Map>(); private readonly pendingSessionOpens = new Map(); + /** + * Sessions whose extension binding is still in flight. A `session_start` + * dialog parks that window before the session ever becomes active, so this + * is the only way the dialog answer/cancel and status paths can reach it; + * {@link getOrOpen} never consults it, keeping every other operation gated + * on full readiness. + */ + private readonly startupSessions = new Map(); private readonly activities = new Map(); private readonly heartbeat: NodeJS.Timeout; private readonly commandService: SessionCommandService; @@ -1003,6 +1015,7 @@ export class PiSessionService implements SessionRouteService { } this.active.clear(); this.pendingSessionOpens.clear(); + this.startupSessions.clear(); this.activities.clear(); this.compactionPromptQueues.clear(); this.authLossWarnings.clear(); @@ -1300,7 +1313,7 @@ export class PiSessionService implements SessionRouteService { */ async answerDialog(ref: PiSessionLookup, dialogId: string, value: ExtensionDialogAnswer): Promise { await this.assertWritable(ref); - const session = await this.getOrOpen(ref); + const session = await this.sessionForStatusOrDialogClose(ref); const result = this.pendingExtensionDialogStore.answer(session.sessionId, dialogId, value); if (result.status === "stale") return { result: "stale", sessionStatus: this.statusFromSession(session) }; const { outcome } = result; @@ -1314,7 +1327,7 @@ export class PiSessionService implements SessionRouteService { /** Close an open extension dialog without an answer; the extension's wait settles with its kind's cancel value. */ async cancelDialog(ref: PiSessionLookup, dialogId: string): Promise { await this.assertWritable(ref); - const session = await this.getOrOpen(ref); + const session = await this.sessionForStatusOrDialogClose(ref); const result = this.pendingExtensionDialogStore.cancel(session.sessionId, dialogId, "cancelled"); if (result.status === "stale") return { result: "stale", sessionStatus: this.statusFromSession(session) }; const { outcome } = result; @@ -1781,7 +1794,7 @@ export class PiSessionService implements SessionRouteService { } async status(ref: PiSessionLookup): Promise { - return this.statusFromSession(await this.getOrOpen(ref)); + return this.statusFromSession(await this.sessionForStatusOrDialogClose(ref)); } /** @@ -2718,6 +2731,28 @@ export class PiSessionService implements SessionRouteService { return undefined; } + private startupSessionForLookup(ref: PiSessionLookup): PiAgentSession | undefined { + const sessionId = sessionIdFromLookup(ref); + const exact = this.startupSessions.get(sessionId); + if (exact !== undefined && lookupMatchesStartupSession(ref, exact)) return exact; + for (const [candidateId, session] of this.startupSessions.entries()) { + if (candidateId.startsWith(sessionId) && lookupMatchesStartupSession(ref, session)) return session; + } + return undefined; + } + + /** + * The session to serve a read-only status or a dialog close for, while it + * can still be found: active first, then still starting up, and only then + * the on-demand open path (which a stale close on an idle session needs for + * its status projection). + */ + private async sessionForStatusOrDialogClose(ref: PiSessionLookup): Promise { + const reachable = this.activeForLookup(ref)?.runtime.session ?? this.startupSessionForLookup(ref); + if (reachable !== undefined) return reachable; + return this.getOrOpen(ref); + } + /** * Construct a session while telling waiting browsers which phase of startup * they are waiting on. The reporting wraps the *whole* construction rather @@ -2871,15 +2906,24 @@ export class PiSessionService implements SessionRouteService { generation: SessionNotificationGeneration | undefined, ): Promise { const uiContext = this.sessionUiContext(session, generation); - await session.bindExtensions({ - uiContext, - mode: "rpc", - onError: (error) => { - const message = `${error.extensionPath}: ${error.error}`; - this.publishActivity(session, "extension error", "error", message); - this.events.publish(session.sessionId, { type: "session.error", message }); - }, - }); + // A `session_start` hook can park this bind on a dialog the browser has + // not answered yet. On the initial create/open path the session becomes + // active only after this returns, so register it for the duration: the + // answer that unblocks startup has to be reachable while it waits. + this.startupSessions.set(session.sessionId, session); + try { + await session.bindExtensions({ + uiContext, + mode: "rpc", + onError: (error) => { + const message = `${error.extensionPath}: ${error.error}`; + this.publishActivity(session, "extension error", "error", message); + this.events.publish(session.sessionId, { type: "session.error", message }); + }, + }); + } finally { + this.startupSessions.delete(session.sessionId); + } } private replaceSessionNotificationContext(session: PiAgentSession, generation: SessionNotificationGeneration): void {