diff --git a/src/features/chat/app-server/transports/session-transports.ts b/src/features/chat/app-server/transports/session-transports.ts index 9168287b..19801f8d 100644 --- a/src/features/chat/app-server/transports/session-transports.ts +++ b/src/features/chat/app-server/transports/session-transports.ts @@ -31,6 +31,7 @@ import type { } from "../../application/threads/thread-loading-transport"; import type { ThreadMutationTransport, ThreadRollbackSnapshot } from "../../application/threads/thread-mutation-transport"; import type { ThreadStartTransport } from "../../application/threads/thread-start-transport"; +import type { ThreadSubscriptionTransport } from "../../application/threads/thread-subscription-transport"; import type { ChatTurnTransport } from "../../application/turns/turn-transport"; import { threadStreamItemsFromTurns } from "../mappers/thread-stream/turn-items"; @@ -65,6 +66,7 @@ export interface ChatConnectedSessionTransports { readonly threadResume: ThreadResumeTransport; readonly threadMutation: ThreadMutationTransport; readonly threadEphemeral: EphemeralThreadTransport; + readonly threadSubscription: ThreadSubscriptionTransport; readonly threadGoal: ThreadGoalTransport; } @@ -83,6 +85,7 @@ export function createChatConnectedSessionTransports(host: ChatAppServerTranspor threadResume: createChatThreadResumeTransport(host), threadMutation: createChatThreadMutationTransport(host), threadEphemeral: createChatEphemeralThreadTransport(host), + threadSubscription: createChatThreadSubscriptionTransport(host), threadGoal: createChatThreadGoalTransport(host), }; } @@ -204,6 +207,18 @@ function createChatEphemeralThreadTransport(host: ChatAppServerTransportHost): E }; } +function createChatThreadSubscriptionTransport(host: ChatAppServerTransportHost): ThreadSubscriptionTransport { + return { + unsubscribeThread: async (threadId) => { + const result = await withCurrentChatAppServerClient(host, async (client) => { + await unsubscribeThread(client, threadId, { timeoutMs: 5_000 }); + return true; + }); + return result ?? false; + }, + }; +} + function createChatThreadGoalReadTransport(host: CurrentChatAppServerClientHost): ThreadGoalReadTransport { return { readThreadGoal: (threadId) => readThreadGoalFromCurrentClient(host, threadId), diff --git a/src/features/chat/application/threads/persistent-navigation-lifecycle.ts b/src/features/chat/application/threads/persistent-navigation-lifecycle.ts new file mode 100644 index 00000000..6103645a --- /dev/null +++ b/src/features/chat/application/threads/persistent-navigation-lifecycle.ts @@ -0,0 +1,59 @@ +import { activeThreadState } from "../state/root-reducer"; +import type { ChatStateStore } from "../state/store"; +import { chatTurnBusy } from "../turns/turn-state"; +import type { EphemeralThreadLifecycle } from "./ephemeral-thread-lifecycle"; +import type { ThreadSubscriptionTransport } from "./thread-subscription-transport"; + +export interface PersistentNavigationLifecycle { + prepareForPersistentNavigation(targetThreadId: string | null): Promise; + completePersistentNavigation(preparation: PersistentNavigationPreparation): Promise; +} + +export type PersistentNavigationPreparation = + | { readonly kind: "ready" } + | { readonly kind: "unsubscribe-after-resume"; readonly threadId: string; readonly targetThreadId: string }; + +interface PersistentNavigationLifecycleHost { + stateStore: ChatStateStore; + ephemeral: EphemeralThreadLifecycle; + subscriptions: ThreadSubscriptionTransport; + addSystemMessage(text: string): void; +} + +export function createPersistentNavigationLifecycle(host: PersistentNavigationLifecycleHost): PersistentNavigationLifecycle { + return { + async prepareForPersistentNavigation(targetThreadId): Promise { + const state = host.stateStore.getState(); + const active = activeThreadState(state); + if (!active || active.id === targetThreadId) return { kind: "ready" }; + + if (active.lifetime?.kind === "ephemeral") { + return (await host.ephemeral.prepareForPersistentNavigation()) ? { kind: "ready" } : null; + } + if (active.provenance?.kind !== "subagent" || !chatTurnBusy(state)) return { kind: "ready" }; + if (targetThreadId !== null) { + return { kind: "unsubscribe-after-resume", threadId: active.id, targetThreadId }; + } + + try { + if (await host.subscriptions.unsubscribeThread(active.id)) return { kind: "ready" }; + host.addSystemMessage("Could not leave the running subagent. Try again before navigating."); + } catch (error) { + host.addSystemMessage(error instanceof Error ? error.message : String(error)); + } + return null; + }, + + async completePersistentNavigation(preparation): Promise { + if (preparation.kind !== "unsubscribe-after-resume") return; + if (activeThreadState(host.stateStore.getState())?.id !== preparation.targetThreadId) return; + try { + if (!(await host.subscriptions.unsubscribeThread(preparation.threadId))) { + host.addSystemMessage("Could not unsubscribe from the previous subagent."); + } + } catch (error) { + host.addSystemMessage(error instanceof Error ? error.message : String(error)); + } + }, + }; +} diff --git a/src/features/chat/application/threads/thread-navigation-actions.ts b/src/features/chat/application/threads/thread-navigation-actions.ts index 173d921c..7fe25261 100644 --- a/src/features/chat/application/threads/thread-navigation-actions.ts +++ b/src/features/chat/application/threads/thread-navigation-actions.ts @@ -2,6 +2,7 @@ import { activeThreadState } from "../state/root-reducer"; import type { ChatStateStore } from "../state/store"; import { chatTurnBusy } from "../turns/turn-state"; import type { ActiveThreadIdentitySync } from "./active-thread-identity-sync"; +import type { PersistentNavigationLifecycle } from "./persistent-navigation-lifecycle"; import { canSwitchToThread } from "./thread-switching"; export interface ThreadNavigationActionsHost { @@ -12,7 +13,7 @@ export interface ThreadNavigationActionsHost { resumeThread: (threadId: string) => Promise; addSystemMessage: (text: string) => void; focusComposer: () => void; - prepareForPersistentNavigation?: () => Promise; + navigation: PersistentNavigationLifecycle; } export interface ThreadNavigationActions { @@ -28,17 +29,19 @@ export function createThreadNavigationActions(host: ThreadNavigationActionsHost) return; } - if (host.prepareForPersistentNavigation && !(await host.prepareForPersistentNavigation())) return; host.closeForThreadSelection(); if (await host.focusThreadInOpenView(threadId)) return; + const preparation = await host.navigation.prepareForPersistentNavigation(threadId); + if (!preparation) return; await host.resumeThread(threadId); + await host.navigation.completePersistentNavigation(preparation); }; return { async startNewThread(): Promise { const state = host.stateStore.getState(); if (chatTurnBusy(state) && activeThreadState(state)?.provenance?.kind !== "subagent") return; - if (host.prepareForPersistentNavigation && !(await host.prepareForPersistentNavigation())) return; + if (!(await host.navigation.prepareForPersistentNavigation(null))) return; host.identity.clearActiveThreadIdentity(); host.stateStore.dispatch({ type: "ui/panel-set", panel: null }); @@ -48,7 +51,6 @@ export function createThreadNavigationActions(host: ThreadNavigationActionsHost) selectThread, async selectThreadFromToolbar(threadId) { if (!canSwitchToThread(host.stateStore.getState(), threadId)) return; - host.stateStore.dispatch({ type: "ui/panel-set", panel: null }); await selectThread(threadId); }, diff --git a/src/features/chat/application/threads/thread-subscription-transport.ts b/src/features/chat/application/threads/thread-subscription-transport.ts new file mode 100644 index 00000000..5ec8f97a --- /dev/null +++ b/src/features/chat/application/threads/thread-subscription-transport.ts @@ -0,0 +1,3 @@ +export interface ThreadSubscriptionTransport { + unsubscribeThread(threadId: string): Promise; +} diff --git a/src/features/chat/application/threads/thread-switching.ts b/src/features/chat/application/threads/thread-switching.ts index cdea08b6..6ff06b43 100644 --- a/src/features/chat/application/threads/thread-switching.ts +++ b/src/features/chat/application/threads/thread-switching.ts @@ -1,6 +1,6 @@ -import { activeThreadId, type ChatState } from "../state/root-reducer"; +import { activeThreadId, activeThreadState, type ChatState } from "../state/root-reducer"; import { chatTurnBusy } from "../turns/turn-state"; export function canSwitchToThread(state: ChatState, threadId: string): boolean { - return !chatTurnBusy(state) || threadId === activeThreadId(state); + return !chatTurnBusy(state) || threadId === activeThreadId(state) || activeThreadState(state)?.provenance?.kind === "subagent"; } diff --git a/src/features/chat/host/bundles/thread-bundle.ts b/src/features/chat/host/bundles/thread-bundle.ts index 5987cba9..3730620e 100644 --- a/src/features/chat/host/bundles/thread-bundle.ts +++ b/src/features/chat/host/bundles/thread-bundle.ts @@ -13,6 +13,7 @@ import { type ActiveThreadIdentitySync, createActiveThreadIdentitySync } from ". import { type AutoTitleCoordinator, createAutoTitleCoordinator } from "../../application/threads/auto-title-coordinator"; import { createGoalActions, createThreadGoalSyncActions } from "../../application/threads/goal-actions"; import { HistoryController } from "../../application/threads/history-controller"; +import type { PersistentNavigationLifecycle } from "../../application/threads/persistent-navigation-lifecycle"; import { activeThreadRenameTitleContext, createThreadRenameEditorActions, @@ -100,7 +101,7 @@ interface ChatPanelThreadActionInput { lifecycle: ChatPanelThreadLifecycleBundle; refreshActiveThreads: () => Promise; notifyActiveThreadIdentityChanged: () => void; - prepareForPersistentNavigation: () => Promise; + navigation: PersistentNavigationLifecycle; } interface ChatPanelThreadActionBundle { @@ -306,7 +307,7 @@ export function createThreadActionBundle(host: ChatPanelThreadHost, input: ChatP focusComposer: () => { composerController.focusComposer(); }, - prepareForPersistentNavigation: input.prepareForPersistentNavigation, + navigation: input.navigation, }); return { actions, toolbarPanelActions, navigation }; } diff --git a/src/features/chat/host/session-runtime.ts b/src/features/chat/host/session-runtime.ts index 69d35469..cbf9d820 100644 --- a/src/features/chat/host/session-runtime.ts +++ b/src/features/chat/host/session-runtime.ts @@ -9,6 +9,10 @@ import type { ChatAction, ChatConnectionPhase } from "../application/state/root- import type { ChatStateStore } from "../application/state/store"; import type { ActiveThreadIdentitySync } from "../application/threads/active-thread-identity-sync"; import { createEphemeralThreadLifecycle, type EphemeralThreadLifecycle } from "../application/threads/ephemeral-thread-lifecycle"; +import { + createPersistentNavigationLifecycle, + type PersistentNavigationLifecycle, +} from "../application/threads/persistent-navigation-lifecycle"; import type { RestorationController } from "../application/threads/restoration-controller"; import type { ResumeActions } from "../application/threads/resume-actions"; import type { ChatResumeWorkTracker } from "../application/threads/resume-work"; @@ -37,6 +41,7 @@ interface ChatPanelSessionRuntimeParts { restoration: RestorationController; identity: ActiveThreadIdentitySync; ephemeral: EphemeralThreadLifecycle; + navigation: PersistentNavigationLifecycle; }; composer: { controller: ChatComposerController; @@ -216,6 +221,12 @@ function composeChatPanelSessionRuntime(host: ChatPanelSessionRuntimeHost): Chat notifyActiveThreadIdentityChanged, interruptTurn: (threadId, turnId) => appServer.turn.interruptTurn(threadId, turnId), }); + const navigation = createPersistentNavigationLifecycle({ + stateStore, + ephemeral, + subscriptions: appServer.threadSubscription, + addSystemMessage: status.addSystemMessage, + }); const threadActions = createThreadActionBundle(host, { appServer, status, @@ -224,7 +235,7 @@ function composeChatPanelSessionRuntime(host: ChatPanelSessionRuntimeHost): Chat lifecycle: threadLifecycle, refreshActiveThreads, notifyActiveThreadIdentityChanged, - prepareForPersistentNavigation: () => ephemeral.prepareForPersistentNavigation(), + navigation, }); const reconnectHost = { stateStore, @@ -326,6 +337,7 @@ function composeChatPanelSessionRuntime(host: ChatPanelSessionRuntimeHost): Chat restoration: threadLifecycle.restoration, identity: threadLifecycle.identity, ephemeral, + navigation, }, composer: { controller: composerController, diff --git a/src/features/chat/host/session.ts b/src/features/chat/host/session.ts index f6d5a950..636188fb 100644 --- a/src/features/chat/host/session.ts +++ b/src/features/chat/host/session.ts @@ -178,8 +178,10 @@ export class ChatPanelSession implements ChatPanelHandle { } async openThread(threadId: string): Promise { - if (!(await this.runtime.thread.ephemeral.prepareForPersistentNavigation())) return; + const preparation = await this.runtime.thread.navigation.prepareForPersistentNavigation(threadId); + if (!preparation) return; await this.runtime.thread.resume.resumeThread(threadId); + await this.runtime.thread.navigation.completePersistentNavigation(preparation); this.focusComposer(); } diff --git a/tests/features/chat/app-server/inbound/handler.test.ts b/tests/features/chat/app-server/inbound/handler.test.ts index 973be60c..09c52620 100644 --- a/tests/features/chat/app-server/inbound/handler.test.ts +++ b/tests/features/chat/app-server/inbound/handler.test.ts @@ -645,6 +645,39 @@ describe("ChatInboundHandler", () => { }); describe("interactive server requests", () => { + it("keeps matching requests actionable while a subagent is active", () => { + const state = chatStateFixture({ + activeThread: { + id: "thread-active", + provenance: { + kind: "subagent", + subagentKind: "thread-spawn", + parentThreadId: "parent", + sessionId: "session", + depth: 1, + agentNickname: "Scout", + agentRole: "explorer", + }, + }, + turn: { lifecycle: { kind: "running", turnId: "turn-active" } }, + }); + const handler = handlerForState(state); + + handler.handleServerRequest({ + id: 41, + method: "item/tool/requestUserInput", + params: { + threadId: "thread-active", + turnId: "turn-active", + itemId: "input-1", + questions: [{ id: "scope", header: "Scope", question: "What should I do?", isOther: false, isSecret: false, options: null }], + autoResolutionMs: null, + }, + }); + + expect(handler.currentState().requests.pendingUserInputs).toHaveLength(1); + }); + it("queues and resolves requestUserInput server requests", () => { let state = chatStateFixture(); state = chatStateWith(state, { diff --git a/tests/features/chat/app-server/transports.test.ts b/tests/features/chat/app-server/transports.test.ts index 46418045..2db4cfbe 100644 --- a/tests/features/chat/app-server/transports.test.ts +++ b/tests/features/chat/app-server/transports.test.ts @@ -325,6 +325,20 @@ describe("chat app-server transports", () => { await expect(transport.resumeThread("thread")).resolves.toBeNull(); }); + it("unsubscribes a panel from a persistent thread without interrupting its turn", async () => { + const request = vi.fn().mockResolvedValue({ status: "unsubscribed" }); + const client = { request } as unknown as AppServerClient; + const transport = createTestGateway({ + currentClient: () => client, + connectedClient: vi.fn().mockResolvedValue(client), + }).threadSubscription; + + await expect(transport.unsubscribeThread("child")).resolves.toBe(true); + + expect(request).toHaveBeenCalledWith("thread/unsubscribe", { threadId: "child" }, { timeoutMs: 5_000 }); + expect(request).not.toHaveBeenCalledWith("turn/interrupt", expect.anything()); + }); + it("distinguishes absent goals from unavailable goal clients", async () => { const client = { request: vi.fn().mockResolvedValue({ goal: null }) } as unknown as AppServerClient; const transport = createTestGateway({ diff --git a/tests/features/chat/application/threads/persistent-navigation-lifecycle.test.ts b/tests/features/chat/application/threads/persistent-navigation-lifecycle.test.ts new file mode 100644 index 00000000..f30bfaa6 --- /dev/null +++ b/tests/features/chat/application/threads/persistent-navigation-lifecycle.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createChatStateStore } from "../../../../../src/features/chat/application/state/store"; +import type { EphemeralThreadLifecycle } from "../../../../../src/features/chat/application/threads/ephemeral-thread-lifecycle"; +import { + createPersistentNavigationLifecycle, + type PersistentNavigationLifecycle, +} from "../../../../../src/features/chat/application/threads/persistent-navigation-lifecycle"; +import type { ThreadSubscriptionTransport } from "../../../../../src/features/chat/application/threads/thread-subscription-transport"; + +describe("persistent navigation lifecycle", () => { + it("unsubscribes a running persistent subagent only after the target resume becomes active", async () => { + const store = runningSubagentStore(); + const subscriptions = subscriptionTransport(); + const ephemeral = ephemeralLifecycle(); + const lifecycle = createLifecycle({ stateStore: store, subscriptions, ephemeral }); + + const preparation = await lifecycle.prepareForPersistentNavigation("other"); + + expect(preparation).toEqual({ kind: "unsubscribe-after-resume", threadId: "child", targetThreadId: "other" }); + expect(subscriptions.unsubscribeThread).not.toHaveBeenCalled(); + expect(ephemeral.prepareForPersistentNavigation).not.toHaveBeenCalled(); + expect(store.getState().panelThread).toMatchObject({ kind: "active", thread: { id: "child" } }); + resumeInteractiveThread(store, "other"); + + if (!preparation) throw new Error("Expected navigation preparation."); + await lifecycle.completePersistentNavigation(preparation); + + expect(subscriptions.unsubscribeThread).toHaveBeenCalledWith("child"); + }); + + it("keeps the subagent subscribed when target resume does not replace it", async () => { + const store = runningSubagentStore(); + const subscriptions = subscriptionTransport(); + const lifecycle = createLifecycle({ stateStore: store, subscriptions }); + + const preparation = await lifecycle.prepareForPersistentNavigation("other"); + if (!preparation) throw new Error("Expected navigation preparation."); + await lifecycle.completePersistentNavigation(preparation); + + expect(subscriptions.unsubscribeThread).not.toHaveBeenCalled(); + expect(store.getState().panelThread).toMatchObject({ kind: "active", thread: { id: "child" } }); + }); + + it("blocks clearing a running subagent when pre-unsubscribe fails", async () => { + const store = runningSubagentStore(); + const subscriptions = subscriptionTransport(); + vi.mocked(subscriptions.unsubscribeThread).mockResolvedValueOnce(false).mockResolvedValueOnce(true); + const addSystemMessage = vi.fn(); + const lifecycle = createLifecycle({ stateStore: store, subscriptions, addSystemMessage }); + + await expect(lifecycle.prepareForPersistentNavigation(null)).resolves.toBeNull(); + + expect(store.getState().panelThread).toMatchObject({ kind: "active", thread: { id: "child" } }); + expect(addSystemMessage).toHaveBeenCalledWith("Could not leave the running subagent. Try again before navigating."); + }); + + it("does not unsubscribe when the target is the already active subagent", async () => { + const subscriptions = subscriptionTransport(); + const lifecycle = createLifecycle({ stateStore: runningSubagentStore(), subscriptions }); + + await expect(lifecycle.prepareForPersistentNavigation("child")).resolves.toEqual({ kind: "ready" }); + + expect(subscriptions.unsubscribeThread).not.toHaveBeenCalled(); + }); +}); + +function runningSubagentStore() { + const store = createChatStateStore(); + store.dispatch({ + type: "active-thread/resumed", + approvalPolicyKnown: true, + sandboxPolicyKnown: true, + permissionProfileKnown: true, + approvalPolicy: null, + sandboxPolicy: null, + activePermissionProfile: null, + thread: { + id: "child", + cliVersion: "test", + provenance: { + kind: "subagent", + subagentKind: "thread-spawn", + parentThreadId: "parent", + sessionId: "session", + depth: 1, + agentNickname: "Scout", + agentRole: "explorer", + }, + } as never, + cwd: "/vault", + model: null, + reasoningEffort: null, + serviceTier: null, + approvalsReviewer: null, + }); + store.dispatch({ type: "turn/started", threadId: "child", turnId: "turn" }); + return store; +} + +function resumeInteractiveThread(store: ReturnType, threadId: string): void { + store.dispatch({ + type: "active-thread/resumed", + approvalPolicyKnown: true, + sandboxPolicyKnown: true, + permissionProfileKnown: true, + approvalPolicy: null, + sandboxPolicy: null, + activePermissionProfile: null, + thread: { id: threadId, cliVersion: "test", provenance: { kind: "interactive" } } as never, + cwd: "/vault", + model: null, + reasoningEffort: null, + serviceTier: null, + approvalsReviewer: null, + }); +} + +function createLifecycle(options: { + stateStore: ReturnType; + subscriptions: ThreadSubscriptionTransport; + ephemeral?: EphemeralThreadLifecycle; + addSystemMessage?: (text: string) => void; +}): PersistentNavigationLifecycle { + return createPersistentNavigationLifecycle({ + stateStore: options.stateStore, + subscriptions: options.subscriptions, + ephemeral: options.ephemeral ?? ephemeralLifecycle(), + addSystemMessage: options.addSystemMessage ?? vi.fn(), + }); +} + +function subscriptionTransport(result = true): ThreadSubscriptionTransport { + return { unsubscribeThread: vi.fn().mockResolvedValue(result) }; +} + +function ephemeralLifecycle(): EphemeralThreadLifecycle { + return { + open: vi.fn(), + prepareForPersistentNavigation: vi.fn().mockResolvedValue(true), + dispose: vi.fn().mockResolvedValue(undefined), + }; +} diff --git a/tests/features/chat/application/threads/thread-navigation-actions.test.ts b/tests/features/chat/application/threads/thread-navigation-actions.test.ts index 31fd1002..ba416e3e 100644 --- a/tests/features/chat/application/threads/thread-navigation-actions.test.ts +++ b/tests/features/chat/application/threads/thread-navigation-actions.test.ts @@ -3,6 +3,10 @@ import { describe, expect, it, vi } from "vitest"; import { createChatState } from "../../../../../src/features/chat/application/state/root-reducer"; import { type ChatStateStore, createChatStateStore } from "../../../../../src/features/chat/application/state/store"; import type { ActiveThreadIdentitySync } from "../../../../../src/features/chat/application/threads/active-thread-identity-sync"; +import type { + PersistentNavigationLifecycle, + PersistentNavigationPreparation, +} from "../../../../../src/features/chat/application/threads/persistent-navigation-lifecycle"; import { createThreadNavigationActions, type ThreadNavigationActionsHost, @@ -52,6 +56,7 @@ function createActionsHarness(overrides: Partial = resumeThread: vi.fn().mockResolvedValue(undefined), addSystemMessage: vi.fn(), focusComposer: vi.fn(), + navigation: navigationMock(), ...overrides, }; return { actions: createThreadNavigationActions(host), host, stateStore }; @@ -84,16 +89,32 @@ describe("ThreadNavigationActions", () => { }); it("starts a blank chat while a subagent turn continues", async () => { - const { actions, host, stateStore } = createActionsHarness(); + const navigation = navigationMock(); + const { actions, host, stateStore } = createActionsHarness({ navigation }); resumeThreadState(stateStore, "child", true); stateStore.dispatch({ type: "turn/started", threadId: "child", turnId: "turn" }); await actions.startNewThread(); + expect(navigation.prepareForPersistentNavigation).toHaveBeenCalledWith(null); expect(host.identity.clearActiveThreadIdentity).toHaveBeenCalledOnce(); + expectCallBefore(navigation.prepareForPersistentNavigation, host.identity.clearActiveThreadIdentity as ReturnType); expect(host.focusComposer).toHaveBeenCalledOnce(); }); + it("keeps a running subagent active when unsubscribe preparation fails", async () => { + const navigation = navigationMock(null); + const { actions, host, stateStore } = createActionsHarness({ navigation }); + resumeThreadState(stateStore, "child", true); + stateStore.dispatch({ type: "turn/started", threadId: "child", turnId: "turn" }); + + await actions.startNewThread(); + + expect(navigation.prepareForPersistentNavigation).toHaveBeenCalledWith(null); + expect(host.identity.clearActiveThreadIdentity).not.toHaveBeenCalled(); + expect(host.focusComposer).not.toHaveBeenCalled(); + }); + it("focuses an already open thread without resuming it", async () => { const { actions, host } = createActionsHarness({ focusThreadInOpenView: vi.fn().mockResolvedValue(true), @@ -106,6 +127,22 @@ describe("ThreadNavigationActions", () => { expect(host.resumeThread).not.toHaveBeenCalled(); }); + it("keeps a running subagent subscribed when the selected thread is already open elsewhere", async () => { + const navigation = navigationMock(); + const { actions, host, stateStore } = createActionsHarness({ + navigation, + focusThreadInOpenView: vi.fn().mockResolvedValue(true), + }); + resumeThreadState(stateStore, "child", true); + stateStore.dispatch({ type: "turn/started", threadId: "child", turnId: "turn" }); + + await actions.selectThread("other"); + + expect(host.focusThreadInOpenView).toHaveBeenCalledWith("other"); + expect(navigation.prepareForPersistentNavigation).not.toHaveBeenCalled(); + expect(host.resumeThread).not.toHaveBeenCalled(); + }); + it("resumes the thread when it is not already open", async () => { const { actions, host } = createActionsHarness(); @@ -115,6 +152,21 @@ describe("ThreadNavigationActions", () => { expect(host.resumeThread).toHaveBeenCalledWith("thread"); }); + it("allows switching away from a running subagent after unsubscribe preparation", async () => { + const preparation = { kind: "unsubscribe-after-resume", threadId: "child", targetThreadId: "other" } as const; + const navigation = navigationMock(preparation); + const { actions, host, stateStore } = createActionsHarness({ navigation }); + resumeThreadState(stateStore, "child", true); + stateStore.dispatch({ type: "turn/started", threadId: "child", turnId: "turn" }); + + await actions.selectThread("other"); + + expect(navigation.prepareForPersistentNavigation).toHaveBeenCalledWith("other"); + expect(host.closeForThreadSelection).toHaveBeenCalledOnce(); + expect(host.resumeThread).toHaveBeenCalledWith("other"); + expectCallBefore(host.resumeThread as ReturnType, navigation.completePersistentNavigation); + }); + it("blocks switching away while a turn is running", async () => { const { actions, host, stateStore } = createActionsHarness(); resumeThreadState(stateStore, "active"); @@ -153,3 +205,20 @@ describe("ThreadNavigationActions", () => { expect(host.resumeThread).not.toHaveBeenCalled(); }); }); + +function expectCallBefore(first: ReturnType, second: ReturnType): void { + const firstCall = first.mock.invocationCallOrder[0]; + const secondCall = second.mock.invocationCallOrder[0]; + if (firstCall === undefined || secondCall === undefined) throw new Error("Expected both mocks to have been called."); + expect(firstCall).toBeLessThan(secondCall); +} + +function navigationMock(preparation: PersistentNavigationPreparation | null = { kind: "ready" }): PersistentNavigationLifecycle & { + prepareForPersistentNavigation: ReturnType; + completePersistentNavigation: ReturnType; +} { + return { + prepareForPersistentNavigation: vi.fn().mockResolvedValue(preparation), + completePersistentNavigation: vi.fn().mockResolvedValue(undefined), + }; +} diff --git a/tests/features/chat/host/view-connection.test.ts b/tests/features/chat/host/view-connection.test.ts index 88a023f7..e61e461c 100644 --- a/tests/features/chat/host/view-connection.test.ts +++ b/tests/features/chat/host/view-connection.test.ts @@ -897,6 +897,88 @@ describe("CodexChatView connection lifecycle", () => { expect(requestSaveLayout).toHaveBeenCalledTimes(1); }); + it("resumes another persistent thread before unsubscribing a running subagent", async () => { + const client = connectedClient({ + "thread/resume": vi.fn((params: unknown) => { + const threadId = (params as { threadId: string }).threadId; + return Promise.resolve( + threadId === "child" + ? resumedThread(threadId, { parentThreadId: "parent", threadSource: "subAgentThreadSpawn" }) + : resumedThread(threadId), + ); + }), + "thread/unsubscribe": vi.fn().mockResolvedValue({ status: "unsubscribed" }), + }); + connectionMock.state.client = client; + const view = await chatView(); + + await view.surface.openThread("child"); + connectionMock.state.onNotification?.({ + method: "turn/started", + params: { + threadId: "child", + turn: { + id: "turn-child", + status: "inProgress", + startedAt: 1, + completedAt: null, + durationMs: null, + error: null, + itemsView: "full", + items: [], + }, + }, + } satisfies Extract); + + await view.surface.openThread("other"); + + const unsubscribeCall = client.request.mock.calls.findIndex(([method]) => method === "thread/unsubscribe"); + const otherResumeCall = client.request.mock.calls.findIndex( + ([method, params]) => method === "thread/resume" && (params as { threadId: string }).threadId === "other", + ); + expect(unsubscribeCall).toBeGreaterThanOrEqual(0); + expect(unsubscribeCall).toBeGreaterThan(otherResumeCall); + expect(client.request).not.toHaveBeenCalledWith("turn/interrupt", expect.anything()); + expect(view.surface.openPanelSnapshot()).toMatchObject({ threadId: "other" }); + }); + + it("keeps a running subagent subscribed when openThread cannot resume the target", async () => { + const client = connectedClient({ + "thread/resume": vi.fn((params: unknown) => { + const threadId = (params as { threadId: string }).threadId; + return Promise.resolve( + threadId === "child" ? resumedThread(threadId, { parentThreadId: "parent", threadSource: "subAgentThreadSpawn" }) : null, + ); + }), + "thread/unsubscribe": vi.fn().mockResolvedValue({ status: "unsubscribed" }), + }); + connectionMock.state.client = client; + const view = await chatView(); + + await view.surface.openThread("child"); + connectionMock.state.onNotification?.({ + method: "turn/started", + params: { + threadId: "child", + turn: { + id: "turn-child", + status: "inProgress", + startedAt: 1, + completedAt: null, + durationMs: null, + error: null, + itemsView: "full", + items: [], + }, + }, + } satisfies Extract); + + await view.surface.openThread("other"); + + expect(requestMethods(client).filter((method) => method === "thread/unsubscribe")).toEqual([]); + expect(view.surface.openPanelSnapshot()).toMatchObject({ threadId: "child", turnLifecycle: { kind: "running", turnId: "turn-child" } }); + }); + it("resets to an unstarted empty chat without starting a thread", async () => { const requestSaveLayout = vi.fn(); const client = connectedClient(); @@ -1209,7 +1291,7 @@ function startedThread(threadId: string) { }; } -function resumedThread(threadId: string) { +function resumedThread(threadId: string, threadOverrides: Record = {}) { return { thread: { id: threadId, @@ -1217,6 +1299,7 @@ function resumedThread(threadId: string) { preview: "Restored thread", cwd: "/vault", cliVersion: "0.0.0", + ...threadOverrides, }, cwd: "/vault", model: null,