From 35730949378e8c949356a99b487f8c3b599aca38 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 22 Jul 2026 00:00:29 -0700 Subject: [PATCH 1/5] Rollback partial agent startup failures - Stop and close partial sessions before releasing registered ports - Preserve startup errors and existing pre-session failure caching - Cover cleanup ordering, retry, and concurrent session ownership --- .../dispatcher/src/context/appAgentManager.ts | 67 +++++-- .../dispatcher/test/appAgentLifecycle.spec.ts | 189 +++++++++++++++++- 2 files changed, 239 insertions(+), 17 deletions(-) diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts index 894b6be6e..8f380428c 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts @@ -1645,10 +1645,13 @@ export class AppAgentManager implements ActionConfigProvider { // registered so we don't leak. const sessionContextId = randomUUID(); record.sessionContextId = sessionContextId; + let appAgent: AppAgent | undefined; + let sessionContext: SessionContext | undefined; try { - const appAgent = await this.ensureAppAgent(record); + const loadedAppAgent = await this.ensureAppAgent(record); + appAgent = loadedAppAgent; let agentContext: unknown | undefined; - if (appAgent.initializeAgentContext !== undefined) { + if (loadedAppAgent.initializeAgentContext !== undefined) { const options = this.agentInitOptions?.[record.name]; let settings: AppAgentInitSettings | undefined = record.manifest .localView @@ -1668,22 +1671,23 @@ export class AppAgentManager implements ActionConfigProvider { settings.options = options; } agentContext = await callEnsureError(() => - appAgent.initializeAgentContext!(settings), + loadedAppAgent.initializeAgentContext!(settings), ); } - record.sessionContext = createSessionContext( + sessionContext = createSessionContext( record.name, agentContext, context, record.manifest.allowDynamicAgents === true, sessionContextId, ); + record.sessionContext = sessionContext; debug(`Session context created for ${record.name}`); - if (appAgent.startBackgroundTasks !== undefined) { + if (loadedAppAgent.startBackgroundTasks !== undefined) { await callEnsureError(() => - appAgent.startBackgroundTasks!(record.sessionContext!), + loadedAppAgent.startBackgroundTasks!(sessionContext!), ); debug(`Background tasks started for ${record.name}`); } @@ -1692,12 +1696,11 @@ export class AppAgentManager implements ActionConfigProvider { // Errors become a "setup-required" entry rather than crashing // the agent — a misbehaving checkReadiness shouldn't deny the // user the agent. - if (appAgent.checkReadiness !== undefined) { + if (loadedAppAgent.checkReadiness !== undefined) { this.readinessImplementers.add(record.name); try { - const report = await appAgent.checkReadiness( - record.sessionContext!, - ); + const report = + await loadedAppAgent.checkReadiness(sessionContext); this.readiness.set(record.name, report); debug( `Readiness for ${record.name}: ${report.state}` + @@ -1710,13 +1713,45 @@ export class AppAgentManager implements ActionConfigProvider { }); } } - return record.sessionContext; + return sessionContext; } catch (e) { - // Init failed. Release any ports the partially-initialized - // agent managed to register before the failure, then clear - // the id so the next ensureSessionContext gets a fresh one. - this.portRegistrar.releaseAllForSession(sessionContextId); - record.sessionContextId = undefined; + // A newer initialization may already have replaced this attempt. + // Clear shared state only while this lifetime still owns it. + if (record.sessionContextId === sessionContextId) { + record.sessionContext = undefined; + if (sessionContext !== undefined) { + record.sessionContextP = undefined; + } + record.sessionContextId = undefined; + } + if (sessionContext !== undefined && appAgent !== undefined) { + if (appAgent.stopBackgroundTasks !== undefined) { + try { + await appAgent.stopBackgroundTasks(sessionContext); + } catch (rollbackError) { + debugError( + `stopBackgroundTasks failed while rolling back ${record.name}. Error ignored`, + rollbackError, + ); + } + } + try { + await appAgent.closeAgentContext?.(sessionContext); + } catch (rollbackError) { + debugError( + `closeAgentContext failed while rolling back ${record.name}. Error ignored`, + rollbackError, + ); + } + } + try { + this.portRegistrar.releaseAllForSession(sessionContextId); + } catch (rollbackError) { + debugError( + `Port cleanup failed while rolling back ${record.name}. Error ignored`, + rollbackError, + ); + } throw e; } } diff --git a/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts index 49726327e..311452d6a 100644 --- a/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts @@ -2,10 +2,11 @@ // Licensed under the MIT License. import { describe, expect, it } from "@jest/globals"; -import { AppAgentEvent } from "@typeagent/agent-sdk"; +import { AppAgent, AppAgentEvent, SessionContext } from "@typeagent/agent-sdk"; import { AppAgentProvider } from "../src/agentProvider/agentProvider.js"; import { AppAgentManager } from "../src/context/appAgentManager.js"; import { + CommandHandlerContext, emitAgentChangeNotification, reconcileKnownAgents, } from "../src/context/commandHandlerContext.js"; @@ -20,6 +21,32 @@ function fakeProvider(name: string): AppAgentProvider { }; } +function createLifecycleHarness(appAgent: AppAgent) { + const portRegistrar = new PortRegistrar(); + const manager = new AppAgentManager(undefined, portRegistrar); + const record = { + name: "lifecycle", + schemas: new Set(), + actions: new Set(), + commands: false, + manifest: { commandDefaultEnabled: true }, + appAgent, + sessionContext: undefined as SessionContext | undefined, + sessionContextP: undefined as Promise | undefined, + sessionContextId: undefined as string | undefined, + schemaErrors: new Map(), + }; + (manager as any).agents.set(record.name, record); + const context = { + agents: manager, + portRegistrar, + session: { + getSessionDirPath: () => undefined, + }, + } as unknown as CommandHandlerContext; + return { context, manager, portRegistrar, record }; +} + describe("AppAgentManager.removeProvider", () => { it("is a no-op for a provider whose agent was never registered", async () => { const manager = new AppAgentManager(undefined, new PortRegistrar()); @@ -65,6 +92,166 @@ describe("AppAgentManager.removeProvider", () => { }); }); +describe("AppAgentManager startup rollback", () => { + it("stops and closes a partially started agent before releasing its ports", async () => { + const startupError = new Error("startup failed"); + const calls: string[] = []; + let portRegistrar: PortRegistrar; + const harness = createLifecycleHarness({ + startBackgroundTasks: async (sessionContext) => { + calls.push("start"); + sessionContext.registerPort("background", 43123); + throw startupError; + }, + stopBackgroundTasks: async () => { + calls.push("stop"); + expect(portRegistrar.lookup("lifecycle", "background")).toBe( + 43123, + ); + }, + closeAgentContext: async () => { + calls.push("close"); + expect(portRegistrar.lookup("lifecycle", "background")).toBe( + 43123, + ); + }, + }); + portRegistrar = harness.portRegistrar; + const { context, manager } = harness; + + const result = await manager.setState(context); + + expect(result.failed.commands).toEqual([ + ["lifecycle", true, startupError], + ]); + expect(calls).toEqual(["start", "stop", "close"]); + expect(portRegistrar.lookup("lifecycle", "background")).toBeUndefined(); + }); + + it("does not replace the startup error when rollback cleanup fails", async () => { + const startupError = new Error("startup failed"); + const calls: string[] = []; + const { context, manager, portRegistrar } = createLifecycleHarness({ + startBackgroundTasks: async () => { + throw startupError; + }, + stopBackgroundTasks: async () => { + calls.push("stop"); + throw new Error("stop failed"); + }, + closeAgentContext: async () => { + calls.push("close"); + throw new Error("close failed"); + }, + }); + portRegistrar.releaseAllForSession = () => { + calls.push("release"); + throw new Error("release failed"); + }; + + const result = await manager.setState(context); + + expect(result.failed.commands[0]?.[2]).toBe(startupError); + expect(calls).toEqual(["stop", "close", "release"]); + }); + + it("retries startup with a fresh context after rollback", async () => { + const startupError = new Error("startup failed"); + const startedContexts: SessionContext[] = []; + let initializeCount = 0; + const { context, manager } = createLifecycleHarness({ + initializeAgentContext: async () => ({ + attempt: ++initializeCount, + }), + startBackgroundTasks: async (sessionContext) => { + startedContexts.push(sessionContext); + if (startedContexts.length === 1) { + throw startupError; + } + }, + }); + + const first = await manager.setState(context); + const second = await manager.setState(context); + + expect(first.failed.commands[0]?.[2]).toBe(startupError); + expect(second.failed.commands).toEqual([]); + expect(second.changed.commands).toEqual([["lifecycle", true]]); + expect(startedContexts).toHaveLength(2); + expect(startedContexts[1]).not.toBe(startedContexts[0]); + expect(startedContexts.map((item) => item.agentContext)).toEqual([ + { attempt: 1 }, + { attempt: 2 }, + ]); + expect(manager.getSessionContext("lifecycle")).toBe(startedContexts[1]); + }); + + it("preserves cached failures before a session context is created", async () => { + const initializationError = new Error("initialization failed"); + let initializeCount = 0; + const { context, manager } = createLifecycleHarness({ + initializeAgentContext: async () => { + initializeCount++; + throw initializationError; + }, + }); + + const first = await manager.setState(context); + const second = await manager.setState(context); + + expect(first.failed.commands[0]?.[2]).toBe(initializationError); + expect(second.failed.commands[0]?.[2]).toBe(initializationError); + expect(initializeCount).toBe(1); + }); + + it("does not clear a newer session when an older startup fails", async () => { + const startupError = new Error("older startup failed"); + let rejectOlderStartup: (() => void) | undefined; + let startCount = 0; + let olderSessionContextId: string | undefined; + const rolledBackIds: string[] = []; + const { context, manager, portRegistrar, record } = + createLifecycleHarness({ + startBackgroundTasks: async (sessionContext) => { + startCount++; + sessionContext.registerPort( + "background", + 43123 + startCount, + ); + if (startCount === 1) { + olderSessionContextId = sessionContext.sessionContextId; + await new Promise((resolve) => { + rejectOlderStartup = resolve; + }); + throw startupError; + } + }, + stopBackgroundTasks: async (sessionContext) => { + rolledBackIds.push(sessionContext.sessionContextId); + }, + }); + + const older = (manager as any).initializeSessionContext( + record, + context, + ) as Promise; + while (rejectOlderStartup === undefined) { + await Promise.resolve(); + } + const newer = await (manager as any).initializeSessionContext( + record, + context, + ); + rejectOlderStartup(); + + await expect(older).rejects.toBe(startupError); + expect(record.sessionContext).toBe(newer); + expect(record.sessionContextId).toBe(newer.sessionContextId); + expect(portRegistrar.lookup("lifecycle", "background")).toBe(43125); + expect(rolledBackIds).toEqual([olderSessionContextId]); + }); +}); + describe("emitAgentChangeNotification", () => { function captureClientIO() { const messages: string[] = []; From 4436c3ca78c1ad734a1fa52f6ff4fcb2cd0aef0a Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 22 Jul 2026 00:14:45 -0700 Subject: [PATCH 2/5] Keep startup rollback within complexity limits - Extract failure cleanup from initializeSessionContext - Preserve the tested stop, close, port release, and retry behavior - Satisfy the repository cognitive-complexity ratchet --- .../dispatcher/src/context/appAgentManager.ts | 62 ++++++++++++------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts index 8f380428c..faefbd41d 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts @@ -1715,44 +1715,58 @@ export class AppAgentManager implements ActionConfigProvider { } return sessionContext; } catch (e) { - // A newer initialization may already have replaced this attempt. - // Clear shared state only while this lifetime still owns it. - if (record.sessionContextId === sessionContextId) { - record.sessionContext = undefined; - if (sessionContext !== undefined) { - record.sessionContextP = undefined; - } - record.sessionContextId = undefined; + await this.rollbackSessionContextInitialization( + record, + appAgent, + sessionContext, + sessionContextId, + ); + throw e; + } + } + + private async rollbackSessionContextInitialization( + record: AppAgentRecord, + appAgent: AppAgent | undefined, + sessionContext: SessionContext | undefined, + sessionContextId: string, + ): Promise { + // A newer initialization may already have replaced this attempt. + // Clear shared state only while this lifetime still owns it. + if (record.sessionContextId === sessionContextId) { + record.sessionContext = undefined; + if (sessionContext !== undefined) { + record.sessionContextP = undefined; } - if (sessionContext !== undefined && appAgent !== undefined) { - if (appAgent.stopBackgroundTasks !== undefined) { - try { - await appAgent.stopBackgroundTasks(sessionContext); - } catch (rollbackError) { - debugError( - `stopBackgroundTasks failed while rolling back ${record.name}. Error ignored`, - rollbackError, - ); - } - } + record.sessionContextId = undefined; + } + if (sessionContext !== undefined && appAgent !== undefined) { + if (appAgent.stopBackgroundTasks !== undefined) { try { - await appAgent.closeAgentContext?.(sessionContext); + await appAgent.stopBackgroundTasks(sessionContext); } catch (rollbackError) { debugError( - `closeAgentContext failed while rolling back ${record.name}. Error ignored`, + `stopBackgroundTasks failed while rolling back ${record.name}. Error ignored`, rollbackError, ); } } try { - this.portRegistrar.releaseAllForSession(sessionContextId); + await appAgent.closeAgentContext?.(sessionContext); } catch (rollbackError) { debugError( - `Port cleanup failed while rolling back ${record.name}. Error ignored`, + `closeAgentContext failed while rolling back ${record.name}. Error ignored`, rollbackError, ); } - throw e; + } + try { + this.portRegistrar.releaseAllForSession(sessionContextId); + } catch (rollbackError) { + debugError( + `Port cleanup failed while rolling back ${record.name}. Error ignored`, + rollbackError, + ); } } From 4204e82dbb4d4eab00969c5501ae6818899354c9 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 22 Jul 2026 00:28:09 -0700 Subject: [PATCH 3/5] Replace startup rollback change - Remove the superseded lifecycle rollback implementation - Remove its specialized failure-injection coverage - Restore the branch to the frozen baseline before the replacement fix --- .../dispatcher/src/context/appAgentManager.ts | 81 ++------ .../dispatcher/test/appAgentLifecycle.spec.ts | 189 +----------------- 2 files changed, 17 insertions(+), 253 deletions(-) diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts index faefbd41d..894b6be6e 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts @@ -1645,13 +1645,10 @@ export class AppAgentManager implements ActionConfigProvider { // registered so we don't leak. const sessionContextId = randomUUID(); record.sessionContextId = sessionContextId; - let appAgent: AppAgent | undefined; - let sessionContext: SessionContext | undefined; try { - const loadedAppAgent = await this.ensureAppAgent(record); - appAgent = loadedAppAgent; + const appAgent = await this.ensureAppAgent(record); let agentContext: unknown | undefined; - if (loadedAppAgent.initializeAgentContext !== undefined) { + if (appAgent.initializeAgentContext !== undefined) { const options = this.agentInitOptions?.[record.name]; let settings: AppAgentInitSettings | undefined = record.manifest .localView @@ -1671,23 +1668,22 @@ export class AppAgentManager implements ActionConfigProvider { settings.options = options; } agentContext = await callEnsureError(() => - loadedAppAgent.initializeAgentContext!(settings), + appAgent.initializeAgentContext!(settings), ); } - sessionContext = createSessionContext( + record.sessionContext = createSessionContext( record.name, agentContext, context, record.manifest.allowDynamicAgents === true, sessionContextId, ); - record.sessionContext = sessionContext; debug(`Session context created for ${record.name}`); - if (loadedAppAgent.startBackgroundTasks !== undefined) { + if (appAgent.startBackgroundTasks !== undefined) { await callEnsureError(() => - loadedAppAgent.startBackgroundTasks!(sessionContext!), + appAgent.startBackgroundTasks!(record.sessionContext!), ); debug(`Background tasks started for ${record.name}`); } @@ -1696,11 +1692,12 @@ export class AppAgentManager implements ActionConfigProvider { // Errors become a "setup-required" entry rather than crashing // the agent — a misbehaving checkReadiness shouldn't deny the // user the agent. - if (loadedAppAgent.checkReadiness !== undefined) { + if (appAgent.checkReadiness !== undefined) { this.readinessImplementers.add(record.name); try { - const report = - await loadedAppAgent.checkReadiness(sessionContext); + const report = await appAgent.checkReadiness( + record.sessionContext!, + ); this.readiness.set(record.name, report); debug( `Readiness for ${record.name}: ${report.state}` + @@ -1713,60 +1710,14 @@ export class AppAgentManager implements ActionConfigProvider { }); } } - return sessionContext; + return record.sessionContext; } catch (e) { - await this.rollbackSessionContextInitialization( - record, - appAgent, - sessionContext, - sessionContextId, - ); - throw e; - } - } - - private async rollbackSessionContextInitialization( - record: AppAgentRecord, - appAgent: AppAgent | undefined, - sessionContext: SessionContext | undefined, - sessionContextId: string, - ): Promise { - // A newer initialization may already have replaced this attempt. - // Clear shared state only while this lifetime still owns it. - if (record.sessionContextId === sessionContextId) { - record.sessionContext = undefined; - if (sessionContext !== undefined) { - record.sessionContextP = undefined; - } - record.sessionContextId = undefined; - } - if (sessionContext !== undefined && appAgent !== undefined) { - if (appAgent.stopBackgroundTasks !== undefined) { - try { - await appAgent.stopBackgroundTasks(sessionContext); - } catch (rollbackError) { - debugError( - `stopBackgroundTasks failed while rolling back ${record.name}. Error ignored`, - rollbackError, - ); - } - } - try { - await appAgent.closeAgentContext?.(sessionContext); - } catch (rollbackError) { - debugError( - `closeAgentContext failed while rolling back ${record.name}. Error ignored`, - rollbackError, - ); - } - } - try { + // Init failed. Release any ports the partially-initialized + // agent managed to register before the failure, then clear + // the id so the next ensureSessionContext gets a fresh one. this.portRegistrar.releaseAllForSession(sessionContextId); - } catch (rollbackError) { - debugError( - `Port cleanup failed while rolling back ${record.name}. Error ignored`, - rollbackError, - ); + record.sessionContextId = undefined; + throw e; } } diff --git a/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts index 311452d6a..49726327e 100644 --- a/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts @@ -2,11 +2,10 @@ // Licensed under the MIT License. import { describe, expect, it } from "@jest/globals"; -import { AppAgent, AppAgentEvent, SessionContext } from "@typeagent/agent-sdk"; +import { AppAgentEvent } from "@typeagent/agent-sdk"; import { AppAgentProvider } from "../src/agentProvider/agentProvider.js"; import { AppAgentManager } from "../src/context/appAgentManager.js"; import { - CommandHandlerContext, emitAgentChangeNotification, reconcileKnownAgents, } from "../src/context/commandHandlerContext.js"; @@ -21,32 +20,6 @@ function fakeProvider(name: string): AppAgentProvider { }; } -function createLifecycleHarness(appAgent: AppAgent) { - const portRegistrar = new PortRegistrar(); - const manager = new AppAgentManager(undefined, portRegistrar); - const record = { - name: "lifecycle", - schemas: new Set(), - actions: new Set(), - commands: false, - manifest: { commandDefaultEnabled: true }, - appAgent, - sessionContext: undefined as SessionContext | undefined, - sessionContextP: undefined as Promise | undefined, - sessionContextId: undefined as string | undefined, - schemaErrors: new Map(), - }; - (manager as any).agents.set(record.name, record); - const context = { - agents: manager, - portRegistrar, - session: { - getSessionDirPath: () => undefined, - }, - } as unknown as CommandHandlerContext; - return { context, manager, portRegistrar, record }; -} - describe("AppAgentManager.removeProvider", () => { it("is a no-op for a provider whose agent was never registered", async () => { const manager = new AppAgentManager(undefined, new PortRegistrar()); @@ -92,166 +65,6 @@ describe("AppAgentManager.removeProvider", () => { }); }); -describe("AppAgentManager startup rollback", () => { - it("stops and closes a partially started agent before releasing its ports", async () => { - const startupError = new Error("startup failed"); - const calls: string[] = []; - let portRegistrar: PortRegistrar; - const harness = createLifecycleHarness({ - startBackgroundTasks: async (sessionContext) => { - calls.push("start"); - sessionContext.registerPort("background", 43123); - throw startupError; - }, - stopBackgroundTasks: async () => { - calls.push("stop"); - expect(portRegistrar.lookup("lifecycle", "background")).toBe( - 43123, - ); - }, - closeAgentContext: async () => { - calls.push("close"); - expect(portRegistrar.lookup("lifecycle", "background")).toBe( - 43123, - ); - }, - }); - portRegistrar = harness.portRegistrar; - const { context, manager } = harness; - - const result = await manager.setState(context); - - expect(result.failed.commands).toEqual([ - ["lifecycle", true, startupError], - ]); - expect(calls).toEqual(["start", "stop", "close"]); - expect(portRegistrar.lookup("lifecycle", "background")).toBeUndefined(); - }); - - it("does not replace the startup error when rollback cleanup fails", async () => { - const startupError = new Error("startup failed"); - const calls: string[] = []; - const { context, manager, portRegistrar } = createLifecycleHarness({ - startBackgroundTasks: async () => { - throw startupError; - }, - stopBackgroundTasks: async () => { - calls.push("stop"); - throw new Error("stop failed"); - }, - closeAgentContext: async () => { - calls.push("close"); - throw new Error("close failed"); - }, - }); - portRegistrar.releaseAllForSession = () => { - calls.push("release"); - throw new Error("release failed"); - }; - - const result = await manager.setState(context); - - expect(result.failed.commands[0]?.[2]).toBe(startupError); - expect(calls).toEqual(["stop", "close", "release"]); - }); - - it("retries startup with a fresh context after rollback", async () => { - const startupError = new Error("startup failed"); - const startedContexts: SessionContext[] = []; - let initializeCount = 0; - const { context, manager } = createLifecycleHarness({ - initializeAgentContext: async () => ({ - attempt: ++initializeCount, - }), - startBackgroundTasks: async (sessionContext) => { - startedContexts.push(sessionContext); - if (startedContexts.length === 1) { - throw startupError; - } - }, - }); - - const first = await manager.setState(context); - const second = await manager.setState(context); - - expect(first.failed.commands[0]?.[2]).toBe(startupError); - expect(second.failed.commands).toEqual([]); - expect(second.changed.commands).toEqual([["lifecycle", true]]); - expect(startedContexts).toHaveLength(2); - expect(startedContexts[1]).not.toBe(startedContexts[0]); - expect(startedContexts.map((item) => item.agentContext)).toEqual([ - { attempt: 1 }, - { attempt: 2 }, - ]); - expect(manager.getSessionContext("lifecycle")).toBe(startedContexts[1]); - }); - - it("preserves cached failures before a session context is created", async () => { - const initializationError = new Error("initialization failed"); - let initializeCount = 0; - const { context, manager } = createLifecycleHarness({ - initializeAgentContext: async () => { - initializeCount++; - throw initializationError; - }, - }); - - const first = await manager.setState(context); - const second = await manager.setState(context); - - expect(first.failed.commands[0]?.[2]).toBe(initializationError); - expect(second.failed.commands[0]?.[2]).toBe(initializationError); - expect(initializeCount).toBe(1); - }); - - it("does not clear a newer session when an older startup fails", async () => { - const startupError = new Error("older startup failed"); - let rejectOlderStartup: (() => void) | undefined; - let startCount = 0; - let olderSessionContextId: string | undefined; - const rolledBackIds: string[] = []; - const { context, manager, portRegistrar, record } = - createLifecycleHarness({ - startBackgroundTasks: async (sessionContext) => { - startCount++; - sessionContext.registerPort( - "background", - 43123 + startCount, - ); - if (startCount === 1) { - olderSessionContextId = sessionContext.sessionContextId; - await new Promise((resolve) => { - rejectOlderStartup = resolve; - }); - throw startupError; - } - }, - stopBackgroundTasks: async (sessionContext) => { - rolledBackIds.push(sessionContext.sessionContextId); - }, - }); - - const older = (manager as any).initializeSessionContext( - record, - context, - ) as Promise; - while (rejectOlderStartup === undefined) { - await Promise.resolve(); - } - const newer = await (manager as any).initializeSessionContext( - record, - context, - ); - rejectOlderStartup(); - - await expect(older).rejects.toBe(startupError); - expect(record.sessionContext).toBe(newer); - expect(record.sessionContextId).toBe(newer.sessionContextId); - expect(portRegistrar.lookup("lifecycle", "background")).toBe(43125); - expect(rolledBackIds).toEqual([olderSessionContextId]); - }); -}); - describe("emitAgentChangeNotification", () => { function captureClientIO() { const messages: string[] = []; From 118462f4b25dd4180a3c139c5c18a7a92de203ff Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 22 Jul 2026 00:41:46 -0700 Subject: [PATCH 4/5] Propagate choice callback failures - Let deferred callback errors reject respondToChoice - Cover failure propagation at the public dispatcher boundary --- .../dispatcher/dispatcher/src/dispatcher.ts | 5 +++-- .../dispatcher/test/chainedChoice.spec.ts | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/ts/packages/dispatcher/dispatcher/src/dispatcher.ts b/ts/packages/dispatcher/dispatcher/src/dispatcher.ts index 5684b2052..aa07097e1 100644 --- a/ts/packages/dispatcher/dispatcher/src/dispatcher.ts +++ b/ts/packages/dispatcher/dispatcher/src/dispatcher.ts @@ -550,6 +550,7 @@ export function createDispatcherFromContext( // Use original requestId so display appends to same message group context.currentRequestId = pending.requestId; context.commandResult = undefined; + let commandResult: CommandResult | undefined; try { const appAgent = context.agents.getAppAgent( pending.agentName, @@ -611,11 +612,11 @@ export function createDispatcherFromContext( } } } finally { - const result = context.commandResult; + commandResult = context.commandResult; context.commandResult = undefined; context.currentRequestId = undefined; - return result; } + return commandResult; }); }, async respondToInteraction( diff --git a/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts b/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts index fb61d02ec..91f559d2d 100644 --- a/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts @@ -51,6 +51,13 @@ const handlers = { ), ), }, + fail: { + description: "Returns a choice whose callback fails", + run: async () => + createYesNoChoiceResult(choiceManager, "fail?", async () => { + throw new Error("choice callback failed"); + }), + }, }, } as const; @@ -140,4 +147,13 @@ describe("Chained choice cards", () => { expect(captured[1].message).toBe("second?"); expect(captured[1].choiceId).not.toBe(captured[0].choiceId); }, 10_000); + + it("propagates choice callback failures", async () => { + await awaitCommand(dispatcher, "@choicechain fail"); + const choice = captured.at(-1)!; + + await expect( + dispatcher.respondToChoice(choice.choiceId, true), + ).rejects.toThrow("choice callback failed"); + }); }); From ff461c5241b910dfbd6c336f806f422e6545a298 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 22 Jul 2026 01:40:32 -0700 Subject: [PATCH 5/5] Verify choice recovery after callback failures - Keep the original callback error assertion - Exercise a later chained choice after the rejection - Prove dispatcher cleanup leaves choice handling usable --- .../dispatcher/dispatcher/test/chainedChoice.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts b/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts index 91f559d2d..908bc402e 100644 --- a/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/chainedChoice.spec.ts @@ -148,12 +148,18 @@ describe("Chained choice cards", () => { expect(captured[1].choiceId).not.toBe(captured[0].choiceId); }, 10_000); - it("propagates choice callback failures", async () => { + it("propagates choice callback failures without blocking later choices", async () => { await awaitCommand(dispatcher, "@choicechain fail"); const choice = captured.at(-1)!; await expect( dispatcher.respondToChoice(choice.choiceId, true), ).rejects.toThrow("choice callback failed"); + + await awaitCommand(dispatcher, "@choicechain chain"); + const nextChoice = captured.at(-1)!; + await dispatcher.respondToChoice(nextChoice.choiceId, true); + + expect(captured.at(-1)!.message).toBe("second?"); }); });