fix(sessions): make session_start extension dialogs answerable mid-startup

A dialog opened from a session_start hook parks session construction
before the session ever becomes active, but every servable path gated on
readiness: the answer/cancel routes and status 404'd (or parked behind
the in-flight open), and the client only subscribed once its create
request resolved — so the dialog that gated readiness could never be
answered and always rode to the daemon timeout.

Daemon: hold a startupSessions registry for the duration of extension
binding and let status, answerDialog, and cancelDialog resolve active →
startup → getOrOpen fallback. getOrOpen itself is untouched, so prompts
and other mutations still cannot reach a half-constructed session.
status() no longer parks behind an in-flight open; it resolves from the
startup window (intended semantics change, lifecycle test updated).

Client: the pending-start row learns the real session id from the first
token-matched session.startup event, connects its otherwise idle session
socket to the constructing session, and recovers pre-subscription opens
with a merge-based status resync (the unordered HTTP snapshot only adopts
dialog ids the ordered socket channel never reported). The leg-4 dialog
card renders in the startup view with no component changes, and answers
go out under the real id. Readiness proceeds as before once the hook
settles.
This commit is contained in:
Federico Jaramillo Martinez
2026-07-29 07:52:21 +02:00
parent 8a429e45fa
commit 346607e8bc
5 changed files with 579 additions and 17 deletions
@@ -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<void> {
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();
});
});
@@ -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);
+56 -12
View File
@@ -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<string, ActiveSession<PiSessionRuntime>>();
private readonly pendingSessionOpens = new Map<string, PendingSessionOpen>();
/**
* 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<string, PiAgentSession>();
private readonly activities = new Map<string, { phase: "active" | "idle" | "error"; label: string; detail?: string; at: string }>();
private readonly heartbeat: NodeJS.Timeout;
private readonly commandService: SessionCommandService<PiAgentSession>;
@@ -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<ExtensionDialogCloseResponse> {
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<ExtensionDialogCloseResponse> {
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<ClientSessionStatus> {
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<PiAgentSession> {
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<void> {
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 {