From 0d5195977caf1c50c7f016e4883789c3d8c06e3f Mon Sep 17 00:00:00 2001 From: liuxuezhuo Date: Thu, 12 Nov 2026 14:27:16 +0800 Subject: [PATCH] fix(chat): end background-subagent turns and correlate their completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stop hook blocked every turn end while a background subagent was alive and told the model to wait on TaskOutput, a tool qodercli never exposed. Turns that launched background work looped on the same explanation until interrupted, and interrupts left the state behind, so the session stayed unusable. The CLI already feeds completion notifications back into the model's context on its own; drop the hook and let turns end normally. The notification's task_id comes from the CLI's task registry and shares no id space with the agentId parsed out of the Agent tool result, so the block was never released and stayed on Running for the rest of the conversation. Correlate by the notification's tool_use_id — the launching Agent call, which is already our key — keeping the agentId lookup as a fallback and letting a notification claim a block whose agentId was never parsed. Co-authored-by: QoderAI (Qwen 3.8 Max) --- CHANGELOG.md | 10 + src/core/runtime/chat-runtime.ts | 2 - src/core/runtime/types.ts | 4 - src/core/types/chat.ts | 10 +- .../subagent-stream-coordinator.ts | 3 +- .../chat/services/subagent-manager.ts | 29 +- .../chat/tabs/tab-runtime-callbacks.ts | 3 - src/qoder/runtime/qoder-chat-runtime.ts | 28 -- .../runtime/qoder-query-options-builder.ts | 4 - src/qoder/runtime/qoder-response-router.ts | 7 + src/qoder/runtime/subagent-hooks.ts | 31 --- src/qoder/stream/transform-qoder-message.ts | 4 +- .../chat/services/subagent-manager.test.ts | 249 ++++++------------ .../qoder/runtime/qoder-chat-runtime.test.ts | 10 +- .../qoder-query-options-builder.test.ts | 1 - .../runtime/qoder-response-router.test.ts | 42 +++ .../unit/qoder/runtime/subagent-hooks.test.ts | 83 ------ .../stream/transform-qoder-message.test.ts | 35 ++- 18 files changed, 211 insertions(+), 344 deletions(-) delete mode 100644 src/qoder/runtime/subagent-hooks.ts delete mode 100644 tests/unit/qoder/runtime/subagent-hooks.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ef75033..4b6a956 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,16 @@ version with its date and start a fresh empty `[Unreleased]` above it. - The current-note chip above the composer is gone. Use the "Excluded tags" setting to keep a note out of context. +### Fixed + +- A turn that launched background subagents could refuse to finish: it was + repeatedly told to wait on a tool it had never been given, so the reply + looped until interrupted. Background results now reach the model on their + own and the turn ends normally. + +- A finished background subagent no longer stays stuck on "Running", and is + no longer reported as abandoned when switching conversations. + ## [1.0.16] - 2026-09-29 ### Changed diff --git a/src/core/runtime/chat-runtime.ts b/src/core/runtime/chat-runtime.ts index 5f03914..afad868 100644 --- a/src/core/runtime/chat-runtime.ts +++ b/src/core/runtime/chat-runtime.ts @@ -13,7 +13,6 @@ import type { ExitPlanModeCallback, PreparedChatTurn, SessionUpdateResult, - SubagentRuntimeState, } from './types'; export interface ChatRuntime { @@ -50,7 +49,6 @@ export interface ChatRuntime { setAskUserQuestionCallback(callback: AskUserQuestionCallback | null): void; setExitPlanModeCallback(callback: ExitPlanModeCallback | null): void; setPermissionModeSyncCallback(callback: ((sdkMode: string) => void) | null): void; - setSubagentStateSource(getState: () => SubagentRuntimeState): void; setAutoTurnCallback(callback: AutoTurnCallback | null): void; consumeTurnMetadata(): ChatTurnMetadata; diff --git a/src/core/runtime/types.ts b/src/core/runtime/types.ts index 8a5c061..691377f 100644 --- a/src/core/runtime/types.ts +++ b/src/core/runtime/types.ts @@ -95,10 +95,6 @@ export interface ChatRewindResult { export type ChatRewindMode = 'conversation' | 'code-and-conversation'; -export interface SubagentRuntimeState { - hasRunning: boolean; -} - export interface ChatTurnMetadata { userMessageId?: string; assistantMessageId?: string; diff --git a/src/core/types/chat.ts b/src/core/types/chat.ts index 7d89b55..479551e 100644 --- a/src/core/types/chat.ts +++ b/src/core/types/chat.ts @@ -158,7 +158,15 @@ export type StreamChunk = | { type: 'done' } | { type: 'usage'; usage: UsageInfo; sessionId?: string | null } | { type: 'context_compacted' } - | { type: 'async_subagent_result'; agentId: string; status: 'completed' | 'error'; result?: string } + | { + type: 'async_subagent_result'; + /** CLI task registry id — a different id space from the agentId in the Agent tool result. */ + taskId: string; + /** The launching Agent tool_use id; the only reliable correlation key, but optional in the protocol. */ + toolUseId?: string; + status: 'completed' | 'error'; + result?: string; + } | { type: 'subagent_tool_use'; subagentId: string; id: string; name: string; input: Record } | { type: 'subagent_tool_result'; subagentId: string; id: string; content: string; isError?: boolean; toolUseResult?: ToolUseResult }; diff --git a/src/features/chat/controllers/subagent-stream-coordinator.ts b/src/features/chat/controllers/subagent-stream-coordinator.ts index 0e23f16..397a547 100644 --- a/src/features/chat/controllers/subagent-stream-coordinator.ts +++ b/src/features/chat/controllers/subagent-stream-coordinator.ts @@ -171,9 +171,10 @@ export class SubagentStreamCoordinator { chunk: Extract, ): Promise { const handled = this.deps.subagentManager.handleAsyncSubagentResult( - chunk.agentId, + chunk.taskId, chunk.status, chunk.result, + chunk.toolUseId, ); await this.hydrateAsyncSubagentToolCalls(handled); if (handled) this.deps.showThinkingIndicator(); diff --git a/src/features/chat/services/subagent-manager.ts b/src/features/chat/services/subagent-manager.ts index 1cf65d2..9746e5a 100644 --- a/src/features/chat/services/subagent-manager.ts +++ b/src/features/chat/services/subagent-manager.ts @@ -437,22 +437,30 @@ export class SubagentManager { } public handleAsyncSubagentResult( - agentId: string, + taskId: string, status: 'completed' | 'error', - result?: string + result?: string, + toolUseId?: string ): SubagentInfo | undefined { - const subagent = this.activeAsyncSubagents.get(agentId); - if (!subagent || subagent.asyncStatus !== 'running') { + // The CLI's task_id comes from its own task registry and shares no id space + // with the agentId parsed out of the Agent tool result. The notification's + // tool_use_id is the launching Agent tool_use id, which is ours. + const subagent = + (toolUseId ? this.getByTaskId(toolUseId) : undefined) ?? + this.activeAsyncSubagents.get(taskId); + if (!subagent || subagent.asyncStatus === 'completed' || subagent.asyncStatus === 'error') { return undefined; } - subagent.agentId = subagent.agentId || agentId; + const agentId = subagent.agentId; subagent.asyncStatus = status; subagent.status = status; subagent.result = result?.trim() || (status === 'error' ? 'Background task failed.' : 'Background task completed.'); subagent.completedAt = Date.now(); - this.activeAsyncSubagents.delete(agentId); + if (agentId) this.activeAsyncSubagents.delete(agentId); + this.pendingAsyncSubagents.delete(subagent.id); + this.taskIdToAgentId.delete(subagent.id); for (const [toolId, mappedAgentId] of this.outputToolIdToAgentId.entries()) { if (mappedAgentId === agentId) { this.outputToolIdToAgentId.delete(toolId); @@ -493,15 +501,6 @@ export class SubagentManager { this.onStateChange(subagent); } - // ============================================ - // Hook State - // ============================================ - - public hasRunningSubagents(): boolean { - // pendingAsyncSubagents: awaiting agent_id; activeAsyncSubagents: only holds running entries - return this.pendingAsyncSubagents.size > 0 || this.activeAsyncSubagents.size > 0; - } - // ============================================ // Lifecycle // ============================================ diff --git a/src/features/chat/tabs/tab-runtime-callbacks.ts b/src/features/chat/tabs/tab-runtime-callbacks.ts index 1b6ed2d..5dc360f 100644 --- a/src/features/chat/tabs/tab-runtime-callbacks.ts +++ b/src/features/chat/tabs/tab-runtime-callbacks.ts @@ -39,9 +39,6 @@ export function setupServiceCallbacks(tab: TabData, plugin: QoderianPlugin): voi } return decision; }); - tab.service.setSubagentStateSource(() => ({ - hasRunning: tab.services.subagentManager.hasRunningSubagents(), - })); tab.service.setAutoTurnCallback((result: AutoTurnResult) => renderAutoTriggeredTurn(tab, result)); tab.service.setPermissionModeSyncCallback((sdkMode) => { const mode = sdkMode === 'bypassPermissions' || sdkMode === 'yolo' diff --git a/src/qoder/runtime/qoder-chat-runtime.ts b/src/qoder/runtime/qoder-chat-runtime.ts index 74864d3..3729aca 100644 --- a/src/qoder/runtime/qoder-chat-runtime.ts +++ b/src/qoder/runtime/qoder-chat-runtime.ts @@ -99,7 +99,6 @@ import { buildPromptWithHistoryContext, isSessionExpiredError, } from './session-context'; -import { createStopSubagentHook, type SubagentHookState } from './subagent-hooks'; import { type ClosePersistentQueryOptions, type PersistentQueryConfig, @@ -171,9 +170,6 @@ export class QoderChatRuntime implements ChatRuntime { // SDK command cache — populated on system/init, cleared on persistent query close private cachedSdkCommands: SlashCommand[] = []; - // Subagent hook state source (set from the feature layer). - private subagentStateSource: (() => SubagentHookState) | null = null; - private readonly turnTracker = new QoderTurnTracker(); constructor(plugin: QoderHostContext, services: QoderRuntimeServices) { @@ -613,7 +609,6 @@ export class QoderChatRuntime implements ChatRuntime { externalContextPaths?: string[] ): Options { const baseContext = this.buildQueryOptionsContext(vaultPath, cliPath); - const hooks = this.buildHooks(); const ctx: PersistentQueryContext = { ...baseContext, @@ -622,7 +617,6 @@ export class QoderChatRuntime implements ChatRuntime { ? { sessionId: resumeSessionId, sessionAt: resumeAtMessageId, fork: this.pendingForkSession || undefined } : undefined, canUseTool: this.createApprovalCallback(), - hooks, externalContextPaths, }; @@ -633,22 +627,6 @@ export class QoderChatRuntime implements ChatRuntime { return options; } - /** - * Builds the hooks for SDK options. - * Hooks need access to `this` for dynamic settings, so they're built here. - */ - private buildHooks() { - const hooks: Options['hooks'] = {}; - - // Resolve feature state at hook execution time so a long-lived Query sees - // the latest subagent status. - hooks.Stop = [createStopSubagentHook( - () => this.subagentStateSource?.() ?? { hasRunning: false } - )]; - - return hooks; - } - /** * Starts the background consumer loop that routes chunks to handlers. */ @@ -1100,7 +1078,6 @@ export class QoderChatRuntime implements ChatRuntime { const queryPrompt = buildQoderPromptWithImages(prompt, images); const baseContext = this.buildQueryOptionsContext(cwd, cliPath); const externalContextPaths = queryOptions?.externalContextPaths || []; - const hooks = this.buildHooks(); const hasEditorContext = prompt.includes(' SubagentHookState): void { - this.subagentStateSource = getState; - } - setAutoTurnCallback(callback: AutoTurnCallback | null): void { this.responseRouter.setAutoTurnCallback(callback); } diff --git a/src/qoder/runtime/qoder-query-options-builder.ts b/src/qoder/runtime/qoder-query-options-builder.ts index c32819e..4481da1 100644 --- a/src/qoder/runtime/qoder-query-options-builder.ts +++ b/src/qoder/runtime/qoder-query-options-builder.ts @@ -46,7 +46,6 @@ export interface PersistentQueryContext extends QueryOptionsContext { fork?: boolean; }; canUseTool?: CanUseTool; - hooks: Options['hooks']; externalContextPaths?: string[]; } @@ -55,7 +54,6 @@ export interface ColdStartQueryContext extends QueryOptionsContext { sessionId?: string; modelOverride?: string; canUseTool?: CanUseTool; - hooks: Options['hooks']; mcpMentions?: Set; enabledMcpServers?: Set; allowedTools?: string[]; @@ -151,7 +149,6 @@ export class QueryOptionsBuilder { ctx.canUseTool, ); QueryOptionsBuilder.applyThinking(options, ctx.settings, runtimeModel); - options.hooks = ctx.hooks; // Pull mode: the provider re-reads live settings so model/override // changes apply on the next LLM call without restarting the query. @@ -205,7 +202,6 @@ export class QueryOptionsBuilder { ctx.settings.permissionMode, ctx.canUseTool, ); - options.hooks = ctx.hooks; QueryOptionsBuilder.applyThinking(options, ctx.settings, selectedModel); options.resolveModel = createQoderModelPolicyProvider( () => selectedModel, diff --git a/src/qoder/runtime/qoder-response-router.ts b/src/qoder/runtime/qoder-response-router.ts index 93878d6..c7be445 100644 --- a/src/qoder/runtime/qoder-response-router.ts +++ b/src/qoder/runtime/qoder-response-router.ts @@ -46,6 +46,13 @@ export class QoderResponseRouter { if (index >= 0) this.handlers.splice(index, 1); } + /** + * Known limitation: buffered auto-turn chunks are dropped here. A background + * completion that arrives between turns is now the main thing landing in that + * buffer, so a persistent-query restart racing it loses the result. Keeping + * the buffer across a restart needs replay idempotency first — crash recovery + * re-enqueues the interrupted message, which would render twice. + */ reset(preserveHandlers = false): void { this.autoTurnBuffer = []; this.autoTurnSawStreamText = false; diff --git a/src/qoder/runtime/subagent-hooks.ts b/src/qoder/runtime/subagent-hooks.ts deleted file mode 100644 index e5f71d2..0000000 --- a/src/qoder/runtime/subagent-hooks.ts +++ /dev/null @@ -1,31 +0,0 @@ -import type { HookCallbackMatcher } from '@qoder-ai/qoder-agent-sdk'; - -import type { SubagentRuntimeState } from '../../core/runtime/types'; - -export type SubagentHookState = SubagentRuntimeState; - -const STOP_BLOCK_REASON = 'Background subagents are still running. Use `TaskOutput task_id="..." block=true` to wait for their results before ending your turn.'; - -export function createStopSubagentHook( - getState: () => SubagentHookState -): HookCallbackMatcher { - return { - hooks: [ - async () => { - let hasRunning: boolean; - try { - hasRunning = getState().hasRunning; - } catch { - // State lookup failed — assume subagents are running to be safe. - hasRunning = true; - } - - if (hasRunning) { - return { decision: 'block' as const, reason: STOP_BLOCK_REASON }; - } - - return {}; - }, - ], - }; -} diff --git a/src/qoder/stream/transform-qoder-message.ts b/src/qoder/stream/transform-qoder-message.ts index 78c1fd5..e649b38 100644 --- a/src/qoder/stream/transform-qoder-message.ts +++ b/src/qoder/stream/transform-qoder-message.ts @@ -69,9 +69,11 @@ function transformTaskNotification(message: SDKMessage): StreamChunk | null { } const status = normalizeTaskNotificationStatus(record.status); + const toolUseId = record.tool_use_id; return { type: 'async_subagent_result', - agentId: taskId, + taskId, + ...(typeof toolUseId === 'string' && toolUseId.length > 0 ? { toolUseId } : {}), status, result: normalizeTaskNotificationResult(status, record.summary), }; diff --git a/tests/unit/features/chat/services/subagent-manager.test.ts b/tests/unit/features/chat/services/subagent-manager.test.ts index 8944947..6f469e9 100644 --- a/tests/unit/features/chat/services/subagent-manager.test.ts +++ b/tests/unit/features/chat/services/subagent-manager.test.ts @@ -6,7 +6,6 @@ import { join } from 'path'; import type { SubagentInfo, ToolCallInfo } from '@/core/types'; import { SubagentManager } from '@/features/chat/services/subagent-manager'; import { QoderTaskResultInterpreter } from '@/qoder/runtime/qoder-task-result-interpreter'; -import { createStopSubagentHook } from '@/qoder/runtime/subagent-hooks'; jest.mock('@/features/chat/rendering/subagent-renderer', () => ({ createSubagentBlock: jest.fn().mockImplementation((_parentEl: any, toolId: string, input: any) => ({ @@ -457,6 +456,90 @@ describe('SubagentManager', () => { }); }); + // ============================================ + // CLI Task Notification Correlation + // ============================================ + + describe('task notification correlation', () => { + // The CLI reports completion with a task_id from its own registry, while the + // block is keyed by the agentId parsed out of the Agent tool result. Only + // tool_use_id — the launching Agent call — is shared between the two. + const CLI_TASK_ID = 'a2f86bbc1bc40d8c5'; + const AGENT_ID = 'aExplore-376901111bd15884'; + + function startBackgroundTask(taskToolId: string) { + const context = createManager(); + const parentEl = createMockEl(); + context.manager.handleTaskToolUse( + taskToolId, + { description: 'Background', run_in_background: true }, + parentEl, + ); + context.manager.handleTaskToolResult(taskToolId, JSON.stringify({ agent_id: AGENT_ID })); + return context; + } + + it('completes the block by tool_use_id when task_id is a different id space', () => { + const { manager, updates } = startBackgroundTask('task-1'); + + // Regression guard: the CLI task_id alone never matched, so the block used + // to stay on running for the rest of the conversation. + expect(manager.handleAsyncSubagentResult(CLI_TASK_ID, 'completed', 'Report ready.')).toBeUndefined(); + expect(manager.getByTaskId('task-1')?.asyncStatus).toBe('running'); + + const completed = manager.handleAsyncSubagentResult(CLI_TASK_ID, 'completed', 'Report ready.', 'task-1'); + + expect(completed?.asyncStatus).toBe('completed'); + expect(completed?.status).toBe('completed'); + expect(completed?.agentId).toBe(AGENT_ID); + expect(completed?.result).toBe('Report ready.'); + expect(updates[updates.length - 1]).toEqual( + expect.objectContaining({ + agentId: AGENT_ID, + asyncStatus: 'completed', + result: 'Report ready.', + }), + ); + }); + + it('still matches on task_id alone for results that report the agentId', () => { + const { manager } = createManager(); + const parentEl = createMockEl(); + manager.handleTaskToolUse('task-1', { description: 'Background', run_in_background: true }, parentEl); + manager.handleTaskToolResult('task-1', JSON.stringify({ agent_id: 'agent-123' })); + + const completed = manager.handleAsyncSubagentResult('agent-123', 'completed', 'Background agent finished.'); + + expect(completed?.asyncStatus).toBe('completed'); + expect(completed?.result).toBe('Background agent finished.'); + }); + + it('releases every lookup path so a duplicate notification is a no-op', () => { + const { manager, updates } = startBackgroundTask('task-1'); + + expect(manager.handleAsyncSubagentResult(CLI_TASK_ID, 'completed', 'Report ready.', 'task-1')).toBeDefined(); + const stateChangeCount = updates.length; + + expect(manager.handleAsyncSubagentResult(CLI_TASK_ID, 'completed', 'Report ready.', 'task-1')).toBeUndefined(); + expect(manager.getByTaskId('task-1')).toBeUndefined(); + expect(manager.isPendingAsyncTask('task-1')).toBe(false); + expect(updates.length).toBe(stateChangeCount); + }); + + it('claims a pending block whose agentId was never parsed', () => { + const { manager } = createManager(); + const parentEl = createMockEl(); + manager.handleTaskToolUse('task-1', { description: 'Background', run_in_background: true }, parentEl); + expect(manager.getByTaskId('task-1')?.asyncStatus).toBe('pending'); + + const completed = manager.handleAsyncSubagentResult(CLI_TASK_ID, 'completed', 'Report ready.', 'task-1'); + + expect(completed?.asyncStatus).toBe('completed'); + expect(completed?.result).toBe('Report ready.'); + expect(manager.isPendingAsyncTask('task-1')).toBe(false); + }); + }); + // ============================================ // Async Parsing Edge Cases (via public API) // ============================================ @@ -1277,7 +1360,7 @@ Only this is the final result. expect(result?.mode).toBe('sync'); expect(manager.getSyncSubagent('task-1')).toBeDefined(); expect(manager.isPendingAsyncTask('task-1')).toBe(false); - expect(manager.hasRunningSubagents()).toBe(false); + expect(manager.getByTaskId('task-1')).toBeUndefined(); }); it('treats stringified completed task metadata with agentId as sync when mode is unknown', () => { @@ -1301,7 +1384,7 @@ Only this is the final result. expect(result?.mode).toBe('sync'); expect(manager.getSyncSubagent('task-1')).toBeDefined(); expect(manager.isPendingAsyncTask('task-1')).toBe(false); - expect(manager.hasRunningSubagents()).toBe(false); + expect(manager.getByTaskId('task-1')).toBeUndefined(); }); it('treats sync result text with agentId metadata as sync when mode is unknown', () => { @@ -1329,7 +1412,7 @@ Only this is the final result. expect(result?.mode).toBe('sync'); expect(manager.getSyncSubagent('task-1')).toBeDefined(); expect(manager.isPendingAsyncTask('task-1')).toBe(false); - expect(manager.hasRunningSubagents()).toBe(false); + expect(manager.getByTaskId('task-1')).toBeUndefined(); }); it('resolves to sync on errored task result when mode is unknown', () => { @@ -1544,164 +1627,6 @@ Only this is the final result. }); }); - // ============================================ - // Hook Delivery Methods - // ============================================ - - describe('hook delivery', () => { - describe('hasRunningSubagents', () => { - it('returns false when no subagents exist', () => { - const { manager } = createManager(); - expect(manager.hasRunningSubagents()).toBe(false); - }); - - it('returns true when pending async subagents exist', () => { - const { manager } = createManager(); - const parentEl = createMockEl(); - manager.handleTaskToolUse('task-1', { description: 'Background', run_in_background: true }, parentEl); - - expect(manager.hasRunningSubagents()).toBe(true); - }); - - it('returns true when active running subagents exist', () => { - const { manager } = createManager(); - const parentEl = createMockEl(); - manager.handleTaskToolUse('task-1', { description: 'Background', run_in_background: true }, parentEl); - manager.handleTaskToolResult('task-1', JSON.stringify({ agent_id: 'agent-123' })); - - expect(manager.hasRunningSubagents()).toBe(true); - }); - - it('returns false when all subagents have completed', () => { - const { manager } = createManager(); - const parentEl = createMockEl(); - manager.handleTaskToolUse('task-1', { description: 'Task agent-123', run_in_background: true }, parentEl); - manager.handleTaskToolResult('task-1', JSON.stringify({ agent_id: 'agent-123' })); - manager.handleAgentOutputToolUse({ - id: 'output-agent-123', - name: 'AgentOutput', - input: { agent_id: 'agent-123' }, - status: 'running', - isExpanded: false, - }); - manager.handleAgentOutputToolResult( - 'output-agent-123', - JSON.stringify({ result: 'Result from agent-123' }), - false - ); - - expect(manager.hasRunningSubagents()).toBe(false); - }); - - it('returns false after a live task notification completes an active async subagent', () => { - const { manager, updates } = createManager(); - const parentEl = createMockEl(); - manager.handleTaskToolUse('task-1', { description: 'Background', run_in_background: true }, parentEl); - manager.handleTaskToolResult('task-1', JSON.stringify({ agent_id: 'agent-123' })); - - expect(manager.hasRunningSubagents()).toBe(true); - - const completed = manager.handleAsyncSubagentResult( - 'agent-123', - 'completed', - 'Background agent finished.' - ); - - expect(completed?.asyncStatus).toBe('completed'); - expect(completed?.status).toBe('completed'); - expect(completed?.result).toBe('Background agent finished.'); - expect(manager.hasRunningSubagents()).toBe(false); - expect(updates[updates.length - 1]).toEqual( - expect.objectContaining({ - agentId: 'agent-123', - asyncStatus: 'completed', - result: 'Background agent finished.', - }) - ); - }); - - it('returns false after parallel foreground tasks resolve from completed task results', () => { - const { manager } = createManager(); - const parentEl = createMockEl(); - const syncToolUseResult1 = { - status: 'completed', - agentId: 'agent-sync-1', - content: [{ type: 'text', text: 'Foreground result 1.' }], - }; - const syncToolUseResult2 = { - status: 'completed', - agentId: 'agent-sync-2', - content: [{ type: 'text', text: 'Foreground result 2.' }], - }; - - manager.handleTaskToolUse('task-1', { prompt: 'Foreground 1' }, parentEl); - manager.handleTaskToolUse('task-2', { prompt: 'Foreground 2' }, parentEl); - - const first = manager.renderPendingTaskFromTaskResult( - 'task-1', - '{}', - false, - parentEl, - syncToolUseResult1 - ); - const second = manager.renderPendingTaskFromTaskResult( - 'task-2', - '{}', - false, - parentEl, - syncToolUseResult2 - ); - - expect(first?.mode).toBe('sync'); - expect(second?.mode).toBe('sync'); - - manager.finalizeSyncSubagent('task-1', '{}', false, syncToolUseResult1); - manager.finalizeSyncSubagent('task-2', '{}', false, syncToolUseResult2); - - expect(manager.hasRunningSubagents()).toBe(false); - }); - - it('allows the Stop hook after a foreground task resolves from completed task metadata', async () => { - const { manager } = createManager(); - const parentEl = createMockEl(); - const completedToolUseResult = { - status: 'completed', - agentId: 'agent-sync', - }; - const hook = createStopSubagentHook(() => ({ - hasRunning: manager.hasRunningSubagents(), - })); - - manager.handleTaskToolUse('task-1', { prompt: 'Foreground task' }, parentEl); - const rendered = manager.renderPendingTaskFromTaskResult( - 'task-1', - JSON.stringify(completedToolUseResult, null, 2), - false, - parentEl, - completedToolUseResult, - ); - - expect(rendered?.mode).toBe('sync'); - - manager.finalizeSyncSubagent('task-1', '{}', false, completedToolUseResult); - - const stopResult = await hook.hooks[0]( - { - hook_event_name: 'Stop', - session_id: 'test-session', - transcript_path: '/tmp/transcript', - cwd: '/vault', - stop_hook_active: true, - }, - undefined, - { signal: new AbortController().signal } - ); - - expect(stopResult).toEqual({}); - }); - }); - }); - // ============================================ // Lifecycle // ============================================ diff --git a/tests/unit/qoder/runtime/qoder-chat-runtime.test.ts b/tests/unit/qoder/runtime/qoder-chat-runtime.test.ts index 4315ba7..28a42c7 100644 --- a/tests/unit/qoder/runtime/qoder-chat-runtime.test.ts +++ b/tests/unit/qoder/runtime/qoder-chat-runtime.test.ts @@ -1281,9 +1281,10 @@ describe('QoderChatRuntime', () => { await (service as any).responseRouter.route({ type: 'system', subtype: 'task_notification', - task_id: 'agent-123', + task_id: 'a2f86bbc1bc40d8c5', + tool_use_id: 'toolu_agent_1', status: 'completed', - output_file: '/tmp/agent-123.output', + output_file: '/tmp/a2f86bbc1bc40d8c5.output', summary: 'Agent completed successfully.', uuid: 'notification-1', session_id: 'session-1', @@ -1291,7 +1292,8 @@ describe('QoderChatRuntime', () => { expect(onChunk).toHaveBeenCalledWith({ type: 'async_subagent_result', - agentId: 'agent-123', + taskId: 'a2f86bbc1bc40d8c5', + toolUseId: 'toolu_agent_1', status: 'completed', result: 'Agent completed successfully.', }); @@ -1317,7 +1319,7 @@ describe('QoderChatRuntime', () => { chunks: [ { type: 'async_subagent_result', - agentId: 'agent-456', + taskId: 'agent-456', status: 'completed', result: 'Background agent finished.', }, diff --git a/tests/unit/qoder/runtime/qoder-query-options-builder.test.ts b/tests/unit/qoder/runtime/qoder-query-options-builder.test.ts index 0d546ab..ba294d4 100644 --- a/tests/unit/qoder/runtime/qoder-query-options-builder.test.ts +++ b/tests/unit/qoder/runtime/qoder-query-options-builder.test.ts @@ -37,7 +37,6 @@ function createContext( pluginManager: { getPluginsKey: jest.fn().mockReturnValue(''), } as unknown as AppPluginManager, - hooks: {}, }; } diff --git a/tests/unit/qoder/runtime/qoder-response-router.test.ts b/tests/unit/qoder/runtime/qoder-response-router.test.ts index 788d457..9ae1541 100644 --- a/tests/unit/qoder/runtime/qoder-response-router.test.ts +++ b/tests/unit/qoder/runtime/qoder-response-router.test.ts @@ -14,6 +14,7 @@ function createRouterHarness(options: { pendingSteerAbort?: boolean } = {}) { getTransformOptions: () => ({}), fetchContextUsage, bufferUsage: (chunk: StreamChunk) => chunk, + consumeMetadata: jest.fn().mockReturnValue({}), updateContextWindow: () => null, record: jest.fn(), }, @@ -111,4 +112,45 @@ describe('QoderResponseRouter', () => { expect(events.onDone).toHaveBeenCalledTimes(1); }); }); + + describe('auto-turn buffer', () => { + function createIdleRouterHarness() { + const harness = createRouterHarness(); + harness.router.unregister(harness.handler.id); + const autoTurnCallback = jest.fn(); + harness.router.setAutoTurnCallback(autoTurnCallback); + return { router: harness.router, autoTurnCallback }; + } + + const buildBetweenTurnAssistantMessage = () => buildSDKMessage({ + type: 'assistant', + message: { content: [{ type: 'text', text: 'Before restart.' }] }, + }); + + it('flushes chunks buffered between turns on the next turn-complete', async () => { + const { router, autoTurnCallback } = createIdleRouterHarness(); + + await router.route(buildBetweenTurnAssistantMessage()); + + expect(autoTurnCallback).not.toHaveBeenCalled(); + + await router.route(buildSDKMessage({ type: 'result' })); + + expect(autoTurnCallback).toHaveBeenCalledWith(expect.objectContaining({ + chunks: [expect.objectContaining({ type: 'text', content: 'Before restart.' })], + })); + }); + + // Known limitation, documented on QoderResponseRouter.reset: a + // persistent-query restart racing a background completion loses it. + it('drops buffered chunks across a reset', async () => { + const { router, autoTurnCallback } = createIdleRouterHarness(); + + await router.route(buildBetweenTurnAssistantMessage()); + router.reset(); + await router.route(buildSDKMessage({ type: 'result' })); + + expect(autoTurnCallback).not.toHaveBeenCalled(); + }); + }); }); diff --git a/tests/unit/qoder/runtime/subagent-hooks.test.ts b/tests/unit/qoder/runtime/subagent-hooks.test.ts deleted file mode 100644 index b0e1791..0000000 --- a/tests/unit/qoder/runtime/subagent-hooks.test.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { - createStopSubagentHook, - type SubagentHookState, -} from '@/qoder/runtime/subagent-hooks'; - -describe('SubagentHooks', () => { - describe('createStopSubagentHook', () => { - const createHookInput = () => ({ - hook_event_name: 'Stop' as const, - session_id: 'test-session', - transcript_path: '/tmp/transcript', - cwd: '/vault', - stop_hook_active: true, - }); - - it('allows stop when no running subagents', async () => { - const state: SubagentHookState = { - hasRunning: false, - }; - - const hook = createStopSubagentHook(() => state); - const result = await hook.hooks[0](createHookInput(), undefined, { signal: new AbortController().signal }); - - expect(result).toEqual({}); - }); - - it('blocks stop when subagents are still running', async () => { - const state: SubagentHookState = { - hasRunning: true, - }; - - const hook = createStopSubagentHook(() => state); - const result = await hook.hooks[0](createHookInput(), undefined, { signal: new AbortController().signal }); - - expect(result).toEqual({ - decision: 'block', - reason: expect.stringContaining('still running'), - }); - expect((result as any).reason).toContain('TaskOutput'); - }); - - it('resolves state dynamically at execution time', async () => { - let running = true; - const getState = (): SubagentHookState => ({ - hasRunning: running, - }); - - const hook = createStopSubagentHook(getState); - const opts = { signal: new AbortController().signal }; - - const result1 = await hook.hooks[0](createHookInput(), undefined, opts); - expect((result1 as any).decision).toBe('block'); - - running = false; - const result2 = await hook.hooks[0](createHookInput(), undefined, opts); - expect(result2).toEqual({}); - }); - - it('fails closed when reading subagent state throws', async () => { - const hook = createStopSubagentHook(() => { - throw new Error('tab already torn down'); - }); - - const result = await hook.hooks[0]( - createHookInput(), - undefined, - { signal: new AbortController().signal } - ); - - expect(result).toEqual({ - decision: 'block', - reason: expect.stringContaining('still running'), - }); - }); - - it('has no matcher (applies to all stop events)', () => { - const hook = createStopSubagentHook( - () => ({ hasRunning: false }) - ); - expect(hook.matcher).toBeUndefined(); - }); - }); -}); diff --git a/tests/unit/qoder/stream/transform-qoder-message.test.ts b/tests/unit/qoder/stream/transform-qoder-message.test.ts index ee7da9d..c157ad4 100644 --- a/tests/unit/qoder/stream/transform-qoder-message.test.ts +++ b/tests/unit/qoder/stream/transform-qoder-message.test.ts @@ -105,10 +105,37 @@ describe('transformSDKMessage', () => { const results = [...transformSDKMessage(message)]; - expect(results).toEqual([ + // toStrictEqual: toolUseId is conditionally spread, so an explicit + // `toolUseId: undefined` key must not appear when the CLI omits it. + expect(results).toStrictEqual([ { type: 'async_subagent_result', - agentId: 'agent-123', + taskId: 'agent-123', + status: 'completed', + result: 'Agent completed successfully.', + }, + ]); + expect('toolUseId' in results[0]).toBe(false); + }); + + it('carries tool_use_id so the notification can be matched to its Agent call', () => { + const message = msg({ + type: 'system', + subtype: 'task_notification', + task_id: 'a2f86bbc1bc40d8c5', + tool_use_id: 'toolu_agent_1', + status: 'completed', + output_file: '/tmp/a2f86bbc1bc40d8c5.output', + summary: 'Agent completed successfully.', + } as any); + + const results = [...transformSDKMessage(message)]; + + expect(results).toStrictEqual([ + { + type: 'async_subagent_result', + taskId: 'a2f86bbc1bc40d8c5', + toolUseId: 'toolu_agent_1', status: 'completed', result: 'Agent completed successfully.', }, @@ -127,10 +154,10 @@ describe('transformSDKMessage', () => { const results = [...transformSDKMessage(message)]; - expect(results).toEqual([ + expect(results).toStrictEqual([ { type: 'async_subagent_result', - agentId: 'agent-failed', + taskId: 'agent-failed', status: 'error', result: 'Agent failed.', },