From 5337ccdfac36003f7df042e9da9f89ae449f6e99 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 14:20:47 +0800 Subject: [PATCH 1/5] fix(tui): show real terminal status for background agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Agent tool's run_in_background=true call returns a non-error ToolResult whose body just says "status: running". The transcript card derived its done/failed badge from that result, so every terminated background agent — including ones reconcile reclassifies as lost on resume — kept the green "✓ Completed" label even when the actual task failed, was killed, or never came back. Push the real BackgroundTaskInfo.status into the matching Agent card so the badge reflects what happened. The card's resolver prefers subagent agentId (live) and falls back to the description on resume; on resume the apply step also runs after replay finishes so the agent group can reach the borrowed components. Also adds an agent-core regression test that pins live, busy, group, race, and resume scenarios for the bg notification chain. --- .changeset/bg-agent-terminal-status.md | 5 + .../tui/components/messages/agent-group.ts | 11 + .../src/tui/components/messages/tool-call.ts | 89 ++++- .../tui/controllers/session-event-handler.ts | 11 + .../src/tui/controllers/session-replay.ts | 31 ++ .../src/tui/controllers/streaming-ui.ts | 68 ++++ .../tui/components/messages/tool-call.test.ts | 104 ++++++ .../agent/bg-idle-notification-repro.test.ts | 308 ++++++++++++++++++ 8 files changed, 624 insertions(+), 3 deletions(-) create mode 100644 .changeset/bg-agent-terminal-status.md create mode 100644 packages/agent-core/test/agent/bg-idle-notification-repro.test.ts diff --git a/.changeset/bg-agent-terminal-status.md b/.changeset/bg-agent-terminal-status.md new file mode 100644 index 0000000000..8c3d10130e --- /dev/null +++ b/.changeset/bg-agent-terminal-status.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Show the real terminal status of background agents in the transcript so lost, failed, and killed ones no longer appear as completed. diff --git a/apps/kimi-code/src/tui/components/messages/agent-group.ts b/apps/kimi-code/src/tui/components/messages/agent-group.ts index 593ae97466..d936d01b81 100644 --- a/apps/kimi-code/src/tui/components/messages/agent-group.ts +++ b/apps/kimi-code/src/tui/components/messages/agent-group.ts @@ -54,6 +54,17 @@ export class AgentGroupComponent extends Container { return this.entries.length; } + /** + * Exposes the borrowed tool call components so external code (e.g. + * routing background task terminal events back to the corresponding + * Agent card) can reach them — the group renders the tcs' snapshots + * but never mounts the tcs as Container children, so a plain tree + * walk of `transcriptContainer` cannot discover them. + */ + getToolComponents(): readonly ToolCallComponent[] { + return this.entries.map((entry) => entry.tc); + } + /** * Borrows a standalone `ToolCallComponent` into the group as a hidden state * container. Snapshot changes trigger throttled refreshes. Re-attaching the diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index 6887070b38..f35edf9651 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -96,6 +96,22 @@ export interface ToolCallReadSnapshot { readonly lines: number; } +function backgroundFailureMessage( + status: 'completed' | 'failed' | 'killed' | 'lost' | undefined, +): string | undefined { + switch (status) { + case 'lost': + return 'Background agent lost (session restarted before completion)'; + case 'killed': + return 'Background agent killed'; + case 'failed': + return 'Background agent failed'; + case 'completed': + case undefined: + return undefined; + } +} + function str(v: unknown): string { return typeof v === 'string' ? v : ''; } @@ -474,6 +490,25 @@ export class ToolCallComponent extends Container { private subagentThinkingText = ''; // ── Subagent lifecycle state from subagent.spawned/completed/failed ── private subagentPhase: 'spawning' | 'running' | 'done' | 'failed' | 'backgrounded' | undefined; + /** + * Authoritative terminal phase for a backgrounded subagent. Set from + * `BackgroundTaskInfo.status` via `setBackgroundTaskTerminalStatus` once + * the backing task reaches a terminal state — either live (a bg agent + * fails / is killed) or on resume (reconcile reclassifies a still-running + * task as `lost`). Beats the spawn-success ToolResult in + * `getSubagentSnapshot`, which would otherwise mislabel every terminated + * background agent — including lost ones — as `✓ Completed`. + */ + private backgroundTaskTerminalPhase: 'done' | 'failed' | undefined; + /** Raw `BackgroundTaskInfo.status` paired with `backgroundTaskTerminalPhase`. + * Drives the snapshot's `errorText` so failed/lost/killed bg agents get a + * meaningful error line instead of leaking the spawn-success ToolResult body. */ + private backgroundTaskTerminalStatus: + | 'completed' + | 'failed' + | 'killed' + | 'lost' + | undefined; private subagentContextTokens: number | undefined; private subagentUsage: TokenUsage | undefined; private subagentResultSummary: string | undefined; @@ -707,7 +742,19 @@ export class ToolCallComponent extends Container { // `backgrounded` has no result because background agents do not enter the // transcript. const derivedPhase: ToolCallSubagentSnapshot['phase'] = - this.result !== undefined ? (this.result.is_error ? 'failed' : 'done') : this.subagentPhase; + this.backgroundTaskTerminalPhase ?? + (this.result !== undefined + ? this.result.is_error + ? 'failed' + : 'done' + : this.subagentPhase); + const errorText = + this.subagentError ?? + (this.backgroundTaskTerminalPhase === 'failed' + ? backgroundFailureMessage(this.backgroundTaskTerminalStatus) + : derivedPhase === 'failed' + ? this.result?.output + : undefined); return { toolCallId: this.toolCall.id, toolName: this.toolCall.name, @@ -717,8 +764,7 @@ export class ToolCallComponent extends Container { toolCount: finished, tokens, isError: derivedPhase === 'failed', - errorText: - this.subagentError ?? (derivedPhase === 'failed' ? this.result?.output : undefined), + errorText, latestActivity, }; } @@ -934,6 +980,43 @@ export class ToolCallComponent extends Container { this.ui?.requestRender(); } + /** + * Records the actual terminal status of the backing background task so + * the snapshot phase no longer relies on the spawn-success ToolResult. + * Called for `agent-*` background tasks both live (when the bg agent + * terminates non-successfully) and on resume (when reconcile + * reclassifies a previously-running task as `lost`). + */ + setBackgroundTaskTerminalStatus(status: 'completed' | 'failed' | 'killed' | 'lost'): void { + const phase: 'done' | 'failed' = status === 'completed' ? 'done' : 'failed'; + if ( + this.backgroundTaskTerminalPhase === phase && + this.backgroundTaskTerminalStatus === status + ) { + return; + } + this.backgroundTaskTerminalPhase = phase; + this.backgroundTaskTerminalStatus = status; + this.subagentEndedAtMs ??= Date.now(); + this.syncSubagentElapsedTimer(); + this.notifySnapshotChange(); + } + + /** Spawned subagent id, if any. Used by routing to find a tool call's + * backing subagent when reconciling background task lifecycle events. */ + getSubagentAgentId(): string | undefined { + return this.subagentAgentId; + } + + /** `args.description` for `Agent` tool calls, used as a resume-path + * fallback when the wire format pre-dates persisted subagent ids and + * the only stable cross-restart identifier is the description string. */ + getAgentToolDescription(): string | undefined { + if (this.toolCall.name !== 'Agent') return undefined; + const desc = this.toolCall.args['description']; + return typeof desc === 'string' ? desc : undefined; + } + appendSubagentText(text: string, kind: SubagentTextKind = 'text'): void { if (kind === 'thinking') { this.subagentThinkingText += text; diff --git a/apps/kimi-code/src/tui/controllers/session-event-handler.ts b/apps/kimi-code/src/tui/controllers/session-event-handler.ts index 0e310ba912..bd3685387c 100644 --- a/apps/kimi-code/src/tui/controllers/session-event-handler.ts +++ b/apps/kimi-code/src/tui/controllers/session-event-handler.ts @@ -872,6 +872,17 @@ export class SessionEventHandler { } if (event.type === 'background.task.terminated' && isTerminal) { + if (info.taskId.startsWith('agent-')) { + // The Agent tool's spawn-success ToolResult is not an error, so the + // parent toolCall card would otherwise render `✓ Completed` for any + // terminated bg agent — including `lost` / `failed` / `killed`. + // Push the actual terminal status so the card matches reality. + this.host.streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: info.agentId, + description: info.description, + status: info.status, + }); + } if (!this.backgroundTaskTranscriptedTerminal.has(info.taskId)) { if (info.taskId.startsWith('bash-')) { this.appendBackgroundTaskEntry(info); diff --git a/apps/kimi-code/src/tui/controllers/session-replay.ts b/apps/kimi-code/src/tui/controllers/session-replay.ts index f806cd8145..50d9f91ba0 100644 --- a/apps/kimi-code/src/tui/controllers/session-replay.ts +++ b/apps/kimi-code/src/tui/controllers/session-replay.ts @@ -66,6 +66,7 @@ export class SessionReplayRenderer { this.hydrateSnapshot(main); this.renderRecords(main); + this.applyTerminalBackgroundAgentStatuses(main); return true; } catch (error) { const message = formatErrorMessage(error); @@ -104,6 +105,36 @@ export class SessionReplayRenderer { this.host.streamingUI.setTodoList(todos); } + /** + * Push real terminal status into each replayed `Agent` card whose + * backing background task is already in a terminal state. Runs AFTER + * `renderRecords` because the tool call components only exist once the + * replay has mounted them — `hydrateBackgroundState` runs too early to + * reach them. Without this, terminated bg agents (including ones that + * reconcile reclassified as `lost`) keep the spawn-success ToolResult's + * default of `✓ Completed`. + */ + private applyTerminalBackgroundAgentStatuses(agent: ResumedAgentState): void { + for (const info of agent.background) { + if (!info.taskId.startsWith('agent-')) continue; + if (!isTerminalBackgroundTask(info)) continue; + const status = info.status; + if ( + status !== 'completed' && + status !== 'failed' && + status !== 'killed' && + status !== 'lost' + ) { + continue; + } + this.host.streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: info.agentId, + description: info.description, + status, + }); + } + } + private hydrateBackgroundState(agent: ResumedAgentState): void { const { state, sessionEventHandler } = this.host; const projection = replayBackgroundProjection(agent.background); diff --git a/apps/kimi-code/src/tui/controllers/streaming-ui.ts b/apps/kimi-code/src/tui/controllers/streaming-ui.ts index 769df37670..1de71c0f04 100644 --- a/apps/kimi-code/src/tui/controllers/streaming-ui.ts +++ b/apps/kimi-code/src/tui/controllers/streaming-ui.ts @@ -174,6 +174,74 @@ export class StreamingUIController { } } + /** + * Push the actual terminal status of a background agent task into the + * matching `Agent` tool call component so its snapshot phase no longer + * trusts the spawn-success ToolResult (which would otherwise label every + * terminated bg agent — including `lost` ones — as `✓ Completed`). + * + * Resolution order, picked to handle both live and resume: + * 1. `agentId` → matches the subagent id recorded on `spawned` + * (live path; replayed tool calls currently do not carry a + * subagent id back, so this branch only catches live mismatches). + * 2. Description fallback — required on resume because reconcile's + * `BackgroundTaskInfo` is the only handle the TUI has to a + * replayed Agent card. Only used when exactly one Agent tool call + * shares the description; ambiguous matches are skipped to avoid + * corrupting an unrelated card. + * + * Search scope includes both in-flight components and already-mounted + * cards (some live in `transcriptContainer` standalone, others are + * borrowed by an `AgentGroupComponent` and reachable only via + * `getToolComponents()`). + * + * Returns true iff a component was found and updated. + */ + applyBackgroundTaskTerminalStatus(args: { + agentId?: string | undefined; + description: string; + status: 'completed' | 'failed' | 'killed' | 'lost'; + }): boolean { + let agentIdMatch: ToolCallComponent | undefined; + let descMatch: ToolCallComponent | undefined; + let descAmbiguous = false; + const visit = (tc: ToolCallComponent): void => { + if (agentIdMatch !== undefined) return; + if (args.agentId !== undefined && tc.getSubagentAgentId() === args.agentId) { + agentIdMatch = tc; + return; + } + if (tc.getAgentToolDescription() !== args.description) return; + if (descMatch !== undefined) { + descAmbiguous = true; + return; + } + descMatch = tc; + }; + + for (const tc of this._pendingToolComponents.values()) { + visit(tc); + if (agentIdMatch !== undefined) break; + } + if (agentIdMatch === undefined) { + for (const child of this.host.state.transcriptContainer.children) { + if (child instanceof ToolCallComponent) { + visit(child); + } else if (child instanceof AgentGroupComponent) { + for (const tc of child.getToolComponents()) { + visit(tc); + if (agentIdMatch !== undefined) break; + } + } + if (agentIdMatch !== undefined) break; + } + } + const target = agentIdMatch ?? (descAmbiguous ? undefined : descMatch); + if (target === undefined) return false; + target.setBackgroundTaskTerminalStatus(args.status); + return true; + } + /** Registers a tool call that arrived via tool.call.started. * Clears any pending streaming state for this id, updates or creates the * component, and returns whether the call was new (no previous entry). */ diff --git a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts index 7e803a1826..13f56c6b90 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts @@ -739,6 +739,110 @@ describe('ToolCallComponent', () => { expect(out).not.toContain('Used Agent'); }); + describe('background agent terminal state vs spawn-success ToolResult', () => { + // The Agent tool returns a "task spawned" result the moment a + // run_in_background=true call lands. That result is not an error and its + // body says `status: running`, so for backgrounded agents `this.result` + // alone cannot distinguish a successful completion from a failure / lost + // task. The fix is `setBackgroundTaskTerminalStatus`, which overrides the + // result-based derivation with the actual BackgroundTaskInfo status. + const spawnSuccessResult = { + tool_call_id: 'call_bg_agent', + output: [ + 'task_id: agent-deadbeef', + 'status: running', + 'agent_id: agent-0', + 'actual_subagent_type: coder', + 'automatic_notification: true', + ].join('\n'), + is_error: false, + }; + + function makeBackgroundAgentComponent(): ToolCallComponent { + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { + description: 'background agent 1', + run_in_background: true, + }, + }, + spawnSuccessResult, + darkColors, + ); + component.onSubagentSpawned({ + agentId: 'agent-0', + agentName: 'coder', + runInBackground: true, + }); + return component; + } + + it('reads as "done" by default after spawn — the existing behavior the fix replaces', () => { + // This pins the legacy behavior. Without overrides the snapshot + // trusts the spawn-success result and reports phase='done'. The + // 'lost' / 'killed' / 'failed' overrides below must beat this. + const component = makeBackgroundAgentComponent(); + expect(component.getSubagentSnapshot().phase).toBe('done'); + }); + + it('setBackgroundTaskTerminalStatus("lost") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('lost'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + // The agent-group renderer uses snap.errorText for the "Error:" line. + // The spawn-success ToolResult must NOT leak as the failure message. + expect(snap.errorText).toContain('lost'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("killed") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('killed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + expect(snap.errorText).toContain('killed'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("failed") flips the snapshot phase to "failed"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('failed'); + expect(snap.errorText).toContain('failed'); + expect(snap.errorText).not.toContain('task_id:'); + }); + + it('setBackgroundTaskTerminalStatus("completed") keeps the snapshot phase at "done"', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('completed'); + const snap = component.getSubagentSnapshot(); + expect(snap.phase).toBe('done'); + expect(snap.errorText).toBeUndefined(); + }); + + it('overrides win even when set before the spawn-success result is recorded', () => { + // Order-independence guard: reconcile may run before tool result + // has been replayed back into the component on some boot paths. + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'background agent A', run_in_background: true }, + }, + undefined, + darkColors, + ); + component.setBackgroundTaskTerminalStatus('lost'); + // Now the spawn-success result lands. + component.setResult({ ...spawnSuccessResult, tool_call_id: 'call_bg_agent' }); + expect(component.getSubagentSnapshot().phase).toBe('failed'); + }); + }); + it('scrolls the Write streaming preview to the last COMMAND_PREVIEW_LINES', () => { const lines: string[] = []; for (let i = 1; i <= 30; i++) lines.push(`line${String(i)}`); diff --git a/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts b/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts new file mode 100644 index 0000000000..c9ba13d0be --- /dev/null +++ b/packages/agent-core/test/agent/bg-idle-notification-repro.test.ts @@ -0,0 +1,308 @@ +/** + * Repro for bug: "after a group of background agents complete, the + * main agent doesn't receive notifications". + * + * Unlike `background-manager.test.ts` (which mocks `agent.turn.steer`), + * this file drives a real `Agent` instance so we can verify the + * full chain: + * + * onLiveTaskTerminal → notifyBackgroundTask → turn.steer() + * → (idle) launch() → turnWorker() → LLM generate called with + * the notification XML in history + * → (busy) buffered into steerBuffer → flushed on next loop step + * + * If either scenario fails to inject the notification into the next + * LLM call, the scripted LLM will throw "Unexpected generate call", + * making the failure mode explicit. + */ + +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'pathe'; + +import { describe, expect, it, vi } from 'vitest'; + +import { appendTaskOutput, writeTask } from '../../src/tools/background/persist'; +import { testAgent } from './harness/agent'; + +describe('background notification → main agent (real Agent instance)', () => { + it('IDLE: completed bg agent auto-starts a new turn with XML', async () => { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + expect(ctx.agent.turn.hasActiveTurn).toBe(false); + expect(ctx.llmCalls.length).toBe(0); + + // The expected auto-launched turn will call generate once, then end. + ctx.mockNextResponse({ type: 'text', text: 'ack from main agent' }); + + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'background agent finished its job' }), + 'idle-state repro', + ); + + await ctx.agent.background.waitForTerminal(taskId); + + // Give the steer→launch→turnWorker→generate chain time to run. + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(1); + }, + { timeout: 2000 }, + ); + + // The latest LLM call must include the notification XML the + // BackgroundManager injected via `turn.steer`. + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + expect(flatHistoryText).toContain(' { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // Step 1 of the user-prompted turn: produce no tool call, end turn. + // But to give the steerBuffer a chance to be flushed we want a + // multi-step turn. So instead: queue a text response for step 1 + // that DOESN'T end the turn yet (set finishReason to tool_calls + // is wrong because we have no tool call). Easiest is to chain two + // responses: first one is text-only (so step ends), the steer + // notification arrives during that step, then a second LLM call + // happens that should contain the notification. + // + // Actually with the scripted-generate harness, a text-only + // response yields finishReason='completed' and the turn ends. + // To force a 2-step turn we need the first step to emit a tool + // call. Since we configured no tools, we can't. So this BUSY + // case is hard to model without LLM-side multi-step. Instead we + // test the buffer mechanism directly: + + const steerSpy = vi.spyOn(ctx.agent.turn, 'steer'); + + // Pretend a turn is active by calling prompt and not awaiting end. + // Queue a response that will be consumed. + ctx.mockNextResponse({ type: 'text', text: 'first turn ack' }); + const promptPromise = ctx.rpc.prompt({ + input: [{ type: 'text', text: 'kick off a turn' }], + }); + + // Right after kicking off, register a background task that + // completes immediately. The notification should be steer()d + // while activeTurn is still set, landing in the steerBuffer. + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'busy-state bg result' }), + 'busy-state repro', + ); + + // Wait for the first turn to end. + await promptPromise; + await ctx.untilTurnEnd(); + + // steer() must have been called at least once for our task. + await vi.waitFor(() => { + expect(steerSpy).toHaveBeenCalled(); + }); + const matchingCall = steerSpy.mock.calls.find((c) => { + const origin = c[1] as { kind?: string; taskId?: string } | undefined; + return origin?.kind === 'background_task' && origin.taskId === taskId; + }); + expect(matchingCall).toBeDefined(); + + // After the turn ends, the steerBuffer should be flushed — + // i.e. the notification text appears as a user message in + // the agent's context history. + const data = ctx.agent.context.data(); + const flatContext = JSON.stringify(data); + expect(flatContext).toContain(' { + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // Only one auto-launched turn is expected; its beforeStep should + // drain ALL buffered notifications. So one queued response is enough. + ctx.mockNextResponse({ type: 'text', text: 'ack group' }); + + const taskIds = [ + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #1 result' }), + 'group-1', + ), + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #2 result' }), + 'group-2', + ), + ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'bg #3 result' }), + 'group-3', + ), + ]; + + for (const id of taskIds) { + await ctx.agent.background.waitForTerminal(id); + } + + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(1); + }, + { timeout: 2000 }, + ); + + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + + // ⚠️ Each of the 3 tasks' notifications must show up in the LLM + // history of the (single) auto-launched turn. + for (const id of taskIds) { + expect(flatHistoryText).toContain(id); + } + expect(flatHistoryText).toContain('bg #1 result'); + expect(flatHistoryText).toContain('bg #2 result'); + expect(flatHistoryText).toContain('bg #3 result'); + }); + + it('RACE: bg completion fires AFTER LLM returns but BEFORE activeTurn is cleared', async () => { + // We're hunting a window: shouldContinueAfterStop reads an empty + // steerBuffer → returns { continue: false } → runTurn unwinds → + // finally block hasn't yet set activeTurn = null. If a steer() + // lands in this window, it gets buffered, then activeTurn=null + // and the buffer is never flushed until the next user prompt. + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // 1st turn: prompted by user — produces text and ends. + ctx.mockNextResponse({ type: 'text', text: 'first user-prompted ack' }); + + // Schedule the bg completion to fire when the first turn ends. + // The cleanest trigger: hook into the `turn.ended` event. + let onTurnEnded: () => void = () => {}; + const turnEndedPromise = new Promise((resolve) => { + onTurnEnded = resolve; + }); + ctx.emitter.on('turn.ended', () => { + onTurnEnded(); + }); + + // Kick off the user-prompted turn — don't await yet. + await ctx.rpc.prompt({ + input: [{ type: 'text', text: 'hello main agent' }], + }); + + // Wait until turn.ended fires. + await ctx.untilTurnEnd(); + await turnEndedPromise; + + // At this point activeTurn should be null. Now fire the bg + // completion — this is the IDLE path, NOT the racy one. We + // queue an LLM response so the auto-launched turn can run. + ctx.mockNextResponse({ type: 'text', text: 'auto ack from bg notification' }); + const taskId = ctx.agent.background.registerAgentTask( + Promise.resolve({ result: 'post-turn bg result' }), + 'race-after-turn', + ); + + await ctx.agent.background.waitForTerminal(taskId); + + // The notification arriving while idle should auto-launch a turn. + await vi.waitFor( + () => { + expect(ctx.llmCalls.length).toBeGreaterThanOrEqual(2); + }, + { timeout: 2000 }, + ); + + const lastCall = ctx.llmCalls.at(-1)!; + const flatHistoryText = JSON.stringify(lastCall.history); + expect(flatHistoryText).toContain(' { + // Scenario the user described: kimi exits while bg tasks are + // running; on next start, resume() loads them from disk and + // reconcile() classifies them as terminal (lost for in-process + // agent tasks; possibly completed for bash tasks if the process + // wrote a terminal state). The restore path uses + // `appendUserMessage`, NOT `steer`, so: + // - Notification XML lands in context history ✓ + // - No new turn is launched ✗ + // - User sees nothing happen until they type + // + // This test pins that current behavior so any change shows up. + + const sessionDir = await mkdtemp(join(tmpdir(), 'kimi-bg-resume-repro-')); + try { + // Simulate a previous session's bash bg task that completed + // before exit and an agent bg task that didn't (will be lost). + await writeTask(sessionDir, { + task_id: 'bash-prev0000', + command: 'echo previous', + description: 'previous bash task', + pid: 12345, + started_at: 1_700_000_000, + ended_at: 1_700_000_005, + exit_code: 0, + status: 'completed', + }); + await appendTaskOutput(sessionDir, 'bash-prev0000', 'previous bash output'); + + await writeTask(sessionDir, { + task_id: 'agent-prev0000', + command: '[agent] previous agent task', + description: 'previous agent task', + pid: 0, + started_at: 1_700_000_000, + ended_at: null, + exit_code: null, + status: 'running', + }); + + const ctx = testAgent(); + ctx.configure({ tools: [] }); + + // We do NOT mock any LLM response. If the resume path + // mistakenly launches a turn, scripted-generate throws + // "Unexpected generate call" and the test fails loudly. + ctx.agent.background.attachSessionDir(sessionDir); + const steerSpy = vi.spyOn(ctx.agent.turn, 'steer'); + + // Reproduce Agent.resume()'s post-replay sequence. + await ctx.agent.background.loadFromDisk(); + const reconcileResult = await ctx.agent.background.reconcile(); + + // The agent-* running task should now be lost. + expect(reconcileResult.lost).toContain('agent-prev0000'); + + // Give the silent append a beat. + await vi.waitFor(() => { + const flatContext = JSON.stringify(ctx.agent.context.data()); + expect(flatContext).toContain('bash-prev0000'); + expect(flatContext).toContain('agent-prev0000'); + }); + + // Hard assertion: steer was NOT called for either restored task. + // The notifications were silently appended, so no new turn ran. + expect(steerSpy).not.toHaveBeenCalled(); + expect(ctx.llmCalls.length).toBe(0); + expect(ctx.agent.turn.hasActiveTurn).toBe(false); + + // Both notifications are in context, waiting for the user. + const flatContext = JSON.stringify(ctx.agent.context.data()); + expect(flatContext).toContain('previous bash output'); + expect(flatContext).toMatch(/task\.completed/); + expect(flatContext).toMatch(/task\.lost/); + } finally { + await rm(sessionDir, { recursive: true, force: true }); + } + }); +}); From d9849cc93f9e3d51d36d6dbda5e38618556ac5d6 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 14:39:19 +0800 Subject: [PATCH 2/5] fix(tui): also propagate bg agent terminal status to standalone cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standalone Agent cards (only one Agent tool call in a step, never upgraded into an AgentGroupComponent) bypassed the previous `setBackgroundTaskTerminalStatus` path: the standalone header reads `getDerivedSubagentPhase`, which still derived `done` from the non-error spawn-success ToolResult, and the method did not request a header/content rebuild. Lost/failed/killed bg agents in this shape still rendered as `✓ Completed`. Thread the override through `getDerivedSubagentPhase`, populate `subagentError` with the friendly failure message so both render paths share one source of truth, and trigger the same header + content rebuild that `onSubagentFailed` does. Also include the override in `hasSubagentState` / the subagent-block early-return so a replayed solo bg agent (no replayed subagent block, no sub-tool activity) switches to the subagent-aware layout instead of the generic `Used Agent` rendering. Adds two standalone-render regression tests so the path no longer relies on the grouped snapshot to stay correct. --- .../src/tui/components/messages/tool-call.ts | 48 +++++++++---------- .../tui/components/messages/tool-call.test.ts | 26 ++++++++++ 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index f35edf9651..4111d564f5 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -495,20 +495,12 @@ export class ToolCallComponent extends Container { * `BackgroundTaskInfo.status` via `setBackgroundTaskTerminalStatus` once * the backing task reaches a terminal state — either live (a bg agent * fails / is killed) or on resume (reconcile reclassifies a still-running - * task as `lost`). Beats the spawn-success ToolResult in - * `getSubagentSnapshot`, which would otherwise mislabel every terminated + * task as `lost`). Beats the spawn-success ToolResult in both render + * paths (`getDerivedSubagentPhase` for standalone, `getSubagentSnapshot` + * for grouped), which would otherwise mislabel every terminated * background agent — including lost ones — as `✓ Completed`. */ private backgroundTaskTerminalPhase: 'done' | 'failed' | undefined; - /** Raw `BackgroundTaskInfo.status` paired with `backgroundTaskTerminalPhase`. - * Drives the snapshot's `errorText` so failed/lost/killed bg agents get a - * meaningful error line instead of leaking the spawn-success ToolResult body. */ - private backgroundTaskTerminalStatus: - | 'completed' - | 'failed' - | 'killed' - | 'lost' - | undefined; private subagentContextTokens: number | undefined; private subagentUsage: TokenUsage | undefined; private subagentResultSummary: string | undefined; @@ -749,12 +741,7 @@ export class ToolCallComponent extends Container { : 'done' : this.subagentPhase); const errorText = - this.subagentError ?? - (this.backgroundTaskTerminalPhase === 'failed' - ? backgroundFailureMessage(this.backgroundTaskTerminalStatus) - : derivedPhase === 'failed' - ? this.result?.output - : undefined); + this.subagentError ?? (derivedPhase === 'failed' ? this.result?.output : undefined); return { toolCallId: this.toolCall.id, toolName: this.toolCall.name, @@ -989,16 +976,20 @@ export class ToolCallComponent extends Container { */ setBackgroundTaskTerminalStatus(status: 'completed' | 'failed' | 'killed' | 'lost'): void { const phase: 'done' | 'failed' = status === 'completed' ? 'done' : 'failed'; - if ( - this.backgroundTaskTerminalPhase === phase && - this.backgroundTaskTerminalStatus === status - ) { - return; - } + if (this.backgroundTaskTerminalPhase === phase) return; this.backgroundTaskTerminalPhase = phase; - this.backgroundTaskTerminalStatus = status; + // Surface a friendly failure line through the same `subagentError` slot + // that `onSubagentFailed` writes. The standalone card reads this in + // `buildSingleSubagentBlock`; the group card reads it via `errorText` in + // `getSubagentSnapshot`. A pre-existing real error from a prior + // `onSubagentFailed` event is more informative — keep it. + if (phase === 'failed' && this.subagentError === undefined) { + this.subagentError = backgroundFailureMessage(status); + } this.subagentEndedAtMs ??= Date.now(); this.syncSubagentElapsedTimer(); + this.headerText.setText(this.buildHeader()); + this.rebuildContent(); this.notifySnapshotChange(); } @@ -1233,7 +1224,8 @@ export class ToolCallComponent extends Container { this.ongoingSubCalls.size === 0 && this.finishedSubCalls.length === 0 && this.subagentText.length === 0 && - this.subagentPhase === undefined + this.subagentPhase === undefined && + this.backgroundTaskTerminalPhase === undefined ) { return; } @@ -1356,7 +1348,8 @@ export class ToolCallComponent extends Container { this.subToolActivities.size > 0 || this.subagentText.length > 0 || this.subagentThinkingText.length > 0 || - this.subagentPhase !== undefined + this.subagentPhase !== undefined || + this.backgroundTaskTerminalPhase !== undefined ); } @@ -1371,6 +1364,9 @@ export class ToolCallComponent extends Container { | 'failed' | 'backgrounded' | undefined { + if (this.backgroundTaskTerminalPhase !== undefined) { + return this.backgroundTaskTerminalPhase; + } if (this.result !== undefined) return this.result.is_error ? 'failed' : 'done'; return this.subagentPhase; } diff --git a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts index 13f56c6b90..fb9199935b 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts @@ -841,6 +841,32 @@ describe('ToolCallComponent', () => { component.setResult({ ...spawnSuccessResult, tool_call_id: 'call_bg_agent' }); expect(component.getSubagentSnapshot().phase).toBe('failed'); }); + + // Standalone render path — when only ONE Agent tool call lands in a + // step, the card is never upgraded into an `AgentGroupComponent` and is + // mounted on its own. The standalone header derives its label from + // `getDerivedSubagentPhase()` (separate from `getSubagentSnapshot`). + // Without the override threading into that path AND a header rebuild, + // a lost bg agent keeps the green "✓ Completed" label. + it('standalone render: lost bg agent must show Failed/Lost, not Completed', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('lost'); + const out = strip(component.render(120).join('\n')); + expect(out).not.toContain('Completed'); + expect(out).toMatch(/Failed|Lost/); + // Friendly failure message must reach the rendered card. + expect(out).toContain('lost'); + expect(out).not.toContain('task_id:'); + }); + + it('standalone render: completed bg agent still shows Completed', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('completed'); + const out = strip(component.render(120).join('\n')); + expect(out).toContain('Completed'); + expect(out).not.toMatch(/Failed/); + expect(out).not.toContain('task_id:'); + }); }); it('scrolls the Write streaming preview to the last COMMAND_PREVIEW_LINES', () => { From a80e8f76e3fa0ebb23f5fc6d3f15352e3c80885d Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 15:05:56 +0800 Subject: [PATCH 3/5] feat(agent-core): make resume actionable from the lost-task notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A backgrounded subagent that ends as `lost`/`failed`/`killed` is already a soft-recoverable thing — `subagentHost.resume` will reanimate the persisted Agent instance — but the LLM had to dig through the original spawn-success ToolResult to find the right id and figure out the recovery shape on its own. The two look-alike identifiers (the BackgroundManager `task_id` aka `source_id`, and the `subagentHost` `agent_id`) regularly got confused in practice. Surface what the model needs at the moment of decision: - Add `agent_id` as a top-level `` attribute for agent-* tasks, so the right id is structural, not buried in prose. Render path keeps backward-compat by omitting the attribute when no agent_id is known (bash tasks, old sessions). - On non-success agent terminal states, append a recovery paragraph to the body: the precise `Agent(resume=...)` call, the disambiguation between `agent_id` and `source_id`, the `run_in_background` option, and what state survives the restart vs. what may need to be redone. - Tighten the spawn-time `resume_hint` with the same disambiguation and an explicit pointer at the `task.lost`/`task.failed`/`task.killed` recovery trigger. - Persist `agent_id` and `subagent_type` in PersistedTask so the recovery body still works after a session restart, where in-memory `BackgroundTaskInfo.agentId` would otherwise be undefined. Optional fields keep the disk schema forward/backward compatible — pre-PR records load without them and silently fall back to the original short body. --- .changeset/bg-agent-terminal-status.md | 3 +- .../agent-core/src/agent/background/index.ts | 53 +++++++++++++-- .../src/agent/context/notification-xml.ts | 20 +++++- .../src/tools/background/manager.ts | 11 ++++ .../src/tools/background/persist.ts | 12 ++++ .../src/tools/builtin/collaboration/agent.ts | 2 +- .../test/agent/background-manager.test.ts | 64 +++++++++++++++++++ .../agent-core/test/agent/context.test.ts | 36 +++++++++++ packages/agent-core/test/tools/agent.test.ts | 8 +++ 9 files changed, 200 insertions(+), 9 deletions(-) diff --git a/.changeset/bg-agent-terminal-status.md b/.changeset/bg-agent-terminal-status.md index 8c3d10130e..e895d826a4 100644 --- a/.changeset/bg-agent-terminal-status.md +++ b/.changeset/bg-agent-terminal-status.md @@ -1,5 +1,6 @@ --- +"@moonshot-ai/agent-core": patch "@moonshot-ai/kimi-code": patch --- -Show the real terminal status of background agents in the transcript so lost, failed, and killed ones no longer appear as completed. +Show the real terminal status of background agents in the transcript so lost, failed, and killed ones no longer appear as completed, and include the resume agent id and recovery instructions in the failure notification so the model can resume reliably. diff --git a/packages/agent-core/src/agent/background/index.ts b/packages/agent-core/src/agent/background/index.ts index f8753df1bb..190e3b7202 100644 --- a/packages/agent-core/src/agent/background/index.ts +++ b/packages/agent-core/src/agent/background/index.ts @@ -17,6 +17,12 @@ type BackgroundTaskNotification = Record & { readonly type: string; readonly source_kind: 'background_task'; readonly source_id: string; + /** Subagent id for agent-* tasks. Surfaced as a structured attribute so + * the LLM can pass it verbatim to `Agent(resume=...)` without confusing + * it with `source_id` (the BackgroundManager ledger id). Omitted for + * bash background tasks and for restored tasks whose previous session + * pre-dates agent_id persistence. */ + readonly agent_id?: string | undefined; readonly title: string; readonly severity: 'info' | 'warning'; readonly body: string; @@ -126,19 +132,18 @@ export class BackgroundManager extends BackgroundProcessManager { const tailOutput = (await this.getOutputSnapshot(info.taskId, NOTIFICATION_TAIL_BYTES)) .preview; if (this.hasDeliveredNotification(origin)) return; - const label = info.taskId.startsWith('agent-') ? 'agent' : 'task'; + const isAgentTask = info.taskId.startsWith('agent-'); + const label = isAgentTask ? 'agent' : 'task'; const notification: BackgroundTaskNotification = { id: notificationId, category: 'task', type: `task.${info.status}`, source_kind: 'background_task', source_id: info.taskId, + agent_id: isAgentTask ? info.agentId : undefined, title: `Background ${label} ${info.status}`, severity: info.status === 'completed' ? 'info' : 'warning', - body: - info.status === 'killed' && info.stopReason - ? `${info.description} was killed: ${info.stopReason}.` - : `${info.description} ${info.status}.`, + body: buildBackgroundTaskNotificationBody(info, isAgentTask), tail_output: tailOutput, }; const content = [ @@ -191,3 +196,41 @@ export class BackgroundManager extends BackgroundProcessManager { function notificationKey(origin: BackgroundTaskOrigin): string { return `${origin.taskId}\0${origin.status}\0${origin.notificationId}`; } + +/** + * Build the human/LLM-readable body that lands in the `` + * XML. For agent-* tasks that ended non-successfully and whose subagent id + * we still know, append a paragraph telling the LLM exactly how to resume + * — which id to pass, how to distinguish it from the look-alike `source_id`, + * and what state the resumed subagent will and will not have. The intent is + * to make recovery a one-shot decision instead of a memory lookup against + * the original spawn-success ToolResult. + * + * Bash tasks, successful agent tasks, and restored agent tasks from + * sessions that pre-date `agent_id` persistence keep the original + * single-sentence body. + */ +function buildBackgroundTaskNotificationBody( + info: BackgroundTaskInfo, + isAgentTask: boolean, +): string { + const baseLine = + info.status === 'killed' && info.stopReason + ? `${info.description} was killed: ${info.stopReason}.` + : `${info.description} ${info.status}.`; + + if (!isAgentTask) return baseLine; + if (info.status === 'completed') return baseLine; + const agentId = info.agentId; + if (agentId === undefined || agentId === info.taskId) return baseLine; + + const recovery = [ + '', + `To recover or continue this subagent, call Agent(resume="${agentId}", prompt="Pick up where you left off; redo the last tool call if its result was never observed.").`, + `Use agent_id ("${agentId}"), NOT source_id / task_id ("${info.taskId}") — the two look alike but only agent_id is accepted by the resume parameter.`, + 'Add run_in_background=true to keep it backgrounded, or omit it to take the result inline in the current turn.', + 'The subagent retains its full prior context across the restart, but any in-flight tool call lost its result and may need to be redone.', + ].join('\n'); + + return `${baseLine}${recovery}`; +} diff --git a/packages/agent-core/src/agent/context/notification-xml.ts b/packages/agent-core/src/agent/context/notification-xml.ts index 72e584b48b..45eda702c2 100644 --- a/packages/agent-core/src/agent/context/notification-xml.ts +++ b/packages/agent-core/src/agent/context/notification-xml.ts @@ -3,7 +3,7 @@ * shared between the live ContextMemory and the projector. * * Output shape: - * + * * Title: ... * Severity: ... * @@ -15,6 +15,13 @@ * The opening-tag names (``) are * load-bearing for the projector's `mergeAdjacentUserMessages` detector * — rename requires updating the detector too. + * + * `agent_id` is emitted only for background_task notifications whose + * source task is an agent subagent — surfacing it structurally lets the + * LLM identify the correct id to pass to `Agent(resume=...)` without + * having to grep the body or the original spawn-success ToolResult. + * It is intentionally a separate attribute from `source_id`: the two + * look alike (`agent-...`) but live in different namespaces. */ export function renderNotificationXml(data: Record): string { @@ -23,12 +30,14 @@ export function renderNotificationXml(data: Record): string { const type = stringAttr(data['type'], 'unknown'); const sourceKind = stringAttr(data['source_kind'], 'unknown'); const sourceId = stringAttr(data['source_id'], 'unknown'); + const agentId = optionalStringAttr(data['agent_id']); const title = typeof data['title'] === 'string' ? data['title'] : ''; const severity = typeof data['severity'] === 'string' ? data['severity'] : ''; const body = typeof data['body'] === 'string' ? data['body'] : ''; + const agentIdAttr = agentId === undefined ? '' : ` agent_id="${agentId}"`; const lines: string[] = [ - ``, + ``, ]; if (title.length > 0) lines.push(`Title: ${title}`); if (severity.length > 0) lines.push(`Severity: ${severity}`); @@ -70,3 +79,10 @@ function stringAttr(value: unknown, fallback: string): string { // where double-escaping would be noisier than literal punctuation. return value.replaceAll('&', '&').replaceAll('"', '"'); } + +/** Like `stringAttr` but returns `undefined` instead of a fallback so the + * caller can omit the attribute entirely when the source value is absent. */ +function optionalStringAttr(value: unknown): string | undefined { + if (typeof value !== 'string' || value.length === 0) return undefined; + return value.replaceAll('&', '&').replaceAll('"', '"'); +} diff --git a/packages/agent-core/src/tools/background/manager.ts b/packages/agent-core/src/tools/background/manager.ts index 97f77fc9e6..c87cbe6679 100644 --- a/packages/agent-core/src/tools/background/manager.ts +++ b/packages/agent-core/src/tools/background/manager.ts @@ -1055,6 +1055,7 @@ export class BackgroundProcessManager { private persistLive(entry: ManagedProcess): Promise { if (this.sessionDir === undefined) return Promise.resolve(); const sessionDir = this.sessionDir; + const isAgentTask = entry.taskId.startsWith('agent-'); const task: PersistedTask = { task_id: entry.taskId, command: entry.command, @@ -1067,6 +1068,12 @@ export class BackgroundProcessManager { approval_reason: entry.approvalReason, timed_out: entry.timedOut, stop_reason: entry.stopReason, + // Only persist subagent identifiers for agent tasks. The base-class + // fallback `agentId ?? taskId` (registerAgentTask) makes them equal + // for tasks registered without an explicit id — skip those too so the + // disk record stays honest about whether we know a real agent_id. + agent_id: isAgentTask && entry.agentId !== entry.taskId ? entry.agentId : undefined, + subagent_type: isAgentTask ? entry.subagentType : undefined, }; entry.persistWriteQueue = entry.persistWriteQueue .then(() => writeTask(sessionDir, task)) @@ -1185,6 +1192,8 @@ function persistedToInfo(t: PersistedTask): BackgroundTaskInfo { approvalReason: t.approval_reason, timedOut: t.timed_out, stopReason: t.stop_reason, + agentId: t.agent_id, + subagentType: t.subagent_type, }; } @@ -1201,5 +1210,7 @@ function infoToPersisted(info: BackgroundTaskInfo): PersistedTask { approval_reason: info.approvalReason, timed_out: info.timedOut, stop_reason: info.stopReason, + agent_id: info.agentId === info.taskId ? undefined : info.agentId, + subagent_type: info.subagentType, }; } diff --git a/packages/agent-core/src/tools/background/persist.ts b/packages/agent-core/src/tools/background/persist.ts index 91725868af..aaa73b6f56 100644 --- a/packages/agent-core/src/tools/background/persist.ts +++ b/packages/agent-core/src/tools/background/persist.ts @@ -65,6 +65,18 @@ export interface PersistedTask { readonly cwd?: string | undefined; } | undefined; + /** + * Subagent identifier for agent-* tasks (the id `subagentHost.resume` + * accepts). Persisted so a session restart can re-emit recovery + * instructions in the next `` without forcing the LLM to + * cross-reference the original spawn-success ToolResult. Omitted for + * bash tasks. Optional in the schema for forward/backward compatibility: + * pre-PR sessions reload without it and simply skip the recovery hint. + */ + readonly agent_id?: string | undefined; + /** Subagent profile name (agent-* tasks only). Persisted for symmetry + * with `agent_id` so resume surfaces match between disk and memory. */ + readonly subagent_type?: string | undefined; } function tasksDirOf(sessionDir: string): string { diff --git a/packages/agent-core/src/tools/builtin/collaboration/agent.ts b/packages/agent-core/src/tools/builtin/collaboration/agent.ts index 9838f86f9a..943d801fbf 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/agent.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/agent.ts @@ -283,7 +283,7 @@ export class AgentTool implements BuiltinTool { `description: ${args.description}`, '', `next_step: The completion arrives automatically in a later turn — no polling needed. To peek at progress without blocking, call TaskOutput(task_id="${taskId}", block=false).`, - `resume_hint: To continue this same subagent instance later, call Agent(resume="${handle.agentId}", prompt="...").`, + `resume_hint: To continue or recover this same subagent later, call Agent(resume="${handle.agentId}", prompt="..."). The parameter is agent_id ("${handle.agentId}"), NOT task_id ("${taskId}") or source_id from a later . Recovery cases: a later for this subagent — its conversation history is preserved across session restarts and resume will pick it up.`, ]; return { output: lines.join('\n') }; } diff --git a/packages/agent-core/test/agent/background-manager.test.ts b/packages/agent-core/test/agent/background-manager.test.ts index 680c26a1bb..e255d0c4de 100644 --- a/packages/agent-core/test/agent/background-manager.test.ts +++ b/packages/agent-core/test/agent/background-manager.test.ts @@ -622,6 +622,70 @@ describe('BackgroundManager — RPC event emission', () => { ); }); + describe('agent task failure body — actionable recovery instructions', () => { + // For agent-* tasks that end non-successfully (lost / failed / killed), + // the notification body must carry enough information for the LLM to + // recover via `Agent(resume=...)` without digging through old context. + // Three things must land in the body: + // 1. The agent_id (NOT the task_id / source_id) — that is what + // `subagentHost.resume` actually takes. + // 2. An explicit disambiguation between agent_id and source_id — + // they look alike and the LLM regularly confuses them. + // 3. The notification must also surface agent_id as a structural + // XML attribute, not just buried in prose. + it('failed agent task body includes resume instructions with the correct agent_id', async () => { + // Promise.reject (non-AbortError) routes through the registerAgentTask + // `.catch` branch and lands at status `failed`, which is the same + // agent-* failure branch reconcile uses for `lost` tasks. + const taskId = agent.background.registerAgentTask( + Promise.reject(new Error('subagent crashed')), + 'inspect repository', + { agentId: 'agent-7' }, + ); + await agent.background.waitForTerminal(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).toContain('agent_id="agent-7"'); + expect(text).toMatch(/Agent\(resume="agent-7"/); + expect(text).toMatch(/agent_id.*not.*source_id|source_id.*not.*agent_id/i); + }); + + it('completed agent task body does NOT add resume instructions', async () => { + const taskId = agent.background.registerAgentTask( + Promise.resolve({ result: 'all good' }), + 'inspect repository', + { agentId: 'agent-8' }, + ); + await agent.background.wait(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).toContain('agent_id="agent-8"'); + // Recovery prose belongs to failure bodies only. + expect(text).not.toMatch(/Agent\(resume="agent-8"/); + }); + + it('bash task body never mentions resume — bash background tasks are not resumable', async () => { + const taskId = agent.background.register(immediateProcess(1), 'false', 'shell'); + await agent.background.waitForTerminal(taskId); + + await vi.waitFor(() => { + expect(agent.turn.steer).toHaveBeenCalled(); + }); + const [content] = vi.mocked(agent.turn.steer).mock.calls[0]!; + const text = (content as Array<{ text: string }>)[0]!.text; + expect(text).not.toContain('agent_id='); + expect(text).not.toMatch(/Agent\(resume=/); + }); + }); + // Note: the `records.restoring` guard is enforced inside `Agent.emitEvent` // (see agent/index.ts). BackgroundManager unconditionally forwards // lifecycle events to the agent; suppression is the agent's job. diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 9a36adfbff..e784085a9d 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -496,6 +496,42 @@ describe('Agent context notification projection', () => { expect(text.trimEnd()).toMatch(/<\/notification>$/); }); + it('renders an agent_id attribute when the notification carries one', () => { + // Background agent tasks (taskId starts with `agent-`) own a separate + // `agent_id` for the spawned subagent. Surfacing it as a top-level XML + // attribute lets the LLM resume the right thing without having to dig + // it out of the body or cross-reference the spawn-success ToolResult. + const text = renderNotificationXml({ + id: 'n_lost1', + category: 'task', + type: 'task.lost', + source_kind: 'background_task', + source_id: 'agent-w7gq3wwj', + agent_id: 'agent-0', + title: 'Background agent lost', + severity: 'warning', + body: 'Background agent 1 lost.', + }); + + expect(text).toContain('source_id="agent-w7gq3wwj"'); + expect(text).toContain('agent_id="agent-0"'); + }); + + it('omits the agent_id attribute when the notification does not carry one', () => { + const text = renderNotificationXml({ + id: 'n_bash', + category: 'task', + type: 'task.completed', + source_kind: 'background_task', + source_id: 'bash-abcdef00', + title: 'Background task completed', + severity: 'info', + body: 'echo done completed.', + }); + + expect(text).not.toContain('agent_id='); + }); + it('does not render task output blocks for non-task notifications', () => { const text = renderNotificationXml({ id: '', diff --git a/packages/agent-core/test/tools/agent.test.ts b/packages/agent-core/test/tools/agent.test.ts index 9a4e4837cf..85fed2a104 100644 --- a/packages/agent-core/test/tools/agent.test.ts +++ b/packages/agent-core/test/tools/agent.test.ts @@ -475,6 +475,14 @@ describe('AgentTool', () => { // M9: resume_hint — continue the same subagent instance expect(result.output).toContain('resume_hint:'); expect(result.output).toContain('Agent(resume="agent-child"'); + // The hint disambiguates the two look-alike identifiers in this output: + // `agent_id` (what `subagentHost.resume` accepts) and `task_id` (the + // BackgroundManager ledger id, which also shows up as `source_id` in + // later entries). LLMs regularly copy the wrong one. + expect(result.output).toMatch(/agent_id.*not.*task_id|task_id.*not.*agent_id/i); + // Recovery scenario — `task.lost` etc. — must be called out so the + // model knows the hint is not only for happy-path follow-up work. + expect(result.output).toMatch(/task\.lost|task\.failed|task\.killed/); }); it('rejects background subagents when background management is unavailable', async () => { From 22289b402230c93a2bd70ca1b36c57485bc7ba54 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 15:32:09 +0800 Subject: [PATCH 4/5] fix(tui): route bg-agent terminal events by stable agent_id, not description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tc.subagentAgentId` is left undefined for every backgrounded agent. `handleSubagentSpawned` early-returns for `runInBackground` before calling `tc.onSubagentSpawned`, and the wire replay path drops the `subagent` block entirely (`toolCallFromReplayMessage` returns only id/name/args). So the `agentId` branch in `applyBackgroundTaskTerminalStatus` never matched in practice, every call fell through to the description-based fallback, and the persisted `agent_id` we added in the previous commit was effectively dead. That fallback also has a real failure mode: if a foreground Agent and a backgrounded Agent share the same `args.description`, the only candidate found is the live (unrelated) card, which gets incorrectly relabeled as the lost task's terminal state. Parse `agent_id: agent-N` out of the AgentTool spawn-success ToolResult body inside `getSubagentAgentId` so the id is always recoverable, regardless of whether the in-memory subagent metadata was ever populated. Foreground and backgrounded Agent cards now carry distinct ids and route correctly. Also pipe the real `subagent.failed` error through to the parent card. The background branch of `handleSubagentFailed` previously only appended the dedicated transcript entry; the parent Agent card was left with the generic "Background agent failed" written by the later `background.task.terminated` event. Add an optional `errorText` to `setBackgroundTaskTerminalStatus` / `applyBackgroundTaskTerminalStatus` and pass `event.error` through on the failed branch — the real reason now reaches both the card and the entry. --- .../src/tui/components/messages/tool-call.ts | 69 +++++++++++--- .../tui/controllers/session-event-handler.ts | 11 +++ .../src/tui/controllers/streaming-ui.ts | 9 +- .../tui/components/messages/tool-call.test.ts | 91 +++++++++++++++++++ 4 files changed, 166 insertions(+), 14 deletions(-) diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index 4111d564f5..d0fcc1f6c7 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -974,18 +974,39 @@ export class ToolCallComponent extends Container { * terminates non-successfully) and on resume (when reconcile * reclassifies a previously-running task as `lost`). */ - setBackgroundTaskTerminalStatus(status: 'completed' | 'failed' | 'killed' | 'lost'): void { + setBackgroundTaskTerminalStatus( + status: 'completed' | 'failed' | 'killed' | 'lost', + options: { errorText?: string | undefined } = {}, + ): void { const phase: 'done' | 'failed' = status === 'completed' ? 'done' : 'failed'; - if (this.backgroundTaskTerminalPhase === phase) return; - this.backgroundTaskTerminalPhase = phase; - // Surface a friendly failure line through the same `subagentError` slot - // that `onSubagentFailed` writes. The standalone card reads this in - // `buildSingleSubagentBlock`; the group card reads it via `errorText` in - // `getSubagentSnapshot`. A pre-existing real error from a prior - // `onSubagentFailed` event is more informative — keep it. - if (phase === 'failed' && this.subagentError === undefined) { - this.subagentError = backgroundFailureMessage(status); + const { errorText } = options; + const phaseUnchanged = this.backgroundTaskTerminalPhase === phase; + let errorChanged = false; + if (phase === 'failed') { + // Surface the failure line through the same `subagentError` slot that + // `onSubagentFailed` writes. The standalone card reads this in + // `buildSingleSubagentBlock`; the group card reads it via `errorText` + // in `getSubagentSnapshot`. Priority: + // 1. Explicit `errorText` from the caller (the real message from a + // live `subagent.failed` event) always wins — it is the most + // informative. + // 2. Existing `subagentError` (could be from a prior + // `onSubagentFailed` or an earlier explicit override) is kept. + // 3. Fall back to a friendly generic so the failure has SOME + // visible explanation when no source has supplied one. + if (errorText !== undefined && this.subagentError !== errorText) { + this.subagentError = errorText; + errorChanged = true; + } else if (this.subagentError === undefined) { + const generic = backgroundFailureMessage(status); + if (generic !== undefined) { + this.subagentError = generic; + errorChanged = true; + } + } } + if (phaseUnchanged && !errorChanged) return; + this.backgroundTaskTerminalPhase = phase; this.subagentEndedAtMs ??= Date.now(); this.syncSubagentElapsedTimer(); this.headerText.setText(this.buildHeader()); @@ -993,10 +1014,32 @@ export class ToolCallComponent extends Container { this.notifySnapshotChange(); } - /** Spawned subagent id, if any. Used by routing to find a tool call's - * backing subagent when reconciling background task lifecycle events. */ + /** + * Subagent id for the backing AgentTool call, used by routing to find a + * tool call's backing subagent when reconciling background task lifecycle + * events. + * + * Two writers, in priority order: + * 1. In-memory `subagentAgentId` — wired by `setSubagentMeta` / + * `onSubagentSpawned` for foreground agents. For backgrounded agents + * this stays undefined: `handleSubagentSpawned` early-returns before + * calling `tc.onSubagentSpawned`, and `applySubagentReplay` early- + * returns when the wire payload omits the `subagent` block — which + * it does for every replayed Agent call. + * 2. The spawn-success ToolResult body — AgentTool unconditionally + * emits `agent_id: agent-N` for every Agent call (foreground and + * background). Parsing it gives the stable identifier even when the + * in-memory field is empty, which is the only way the resume path + * can reliably route a `background.task.terminated` to the right + * card and the only way the live path avoids matching by description + * and accidentally updating an unrelated Agent card that happens to + * share the same `args.description`. + */ getSubagentAgentId(): string | undefined { - return this.subagentAgentId; + if (this.subagentAgentId !== undefined) return this.subagentAgentId; + if (this.toolCall.name !== 'Agent' || this.result === undefined) return undefined; + const match = this.result.output.match(/^agent_id:\s*(agent-[A-Za-z0-9_-]+)/m); + return match?.[1]; } /** `args.description` for `Agent` tool calls, used as a resume-path diff --git a/apps/kimi-code/src/tui/controllers/session-event-handler.ts b/apps/kimi-code/src/tui/controllers/session-event-handler.ts index bd3685387c..3e2636662d 100644 --- a/apps/kimi-code/src/tui/controllers/session-event-handler.ts +++ b/apps/kimi-code/src/tui/controllers/session-event-handler.ts @@ -754,6 +754,17 @@ export class SessionEventHandler { if (backgroundMeta !== undefined) { this.backgroundAgentMetadata.delete(event.subagentId); this.syncBackgroundAgentBadge(); + // Push the real subagent error onto the parent Agent card too — + // `background.task.terminated` arrives separately (possibly later) + // with no error string and would only stamp the generic + // `Background agent failed`. The card and the separate transcript + // entry now share the same actual reason. + streamingUI.applyBackgroundTaskTerminalStatus({ + agentId: event.subagentId, + description: backgroundMeta.description ?? '', + status: 'failed', + errorText: event.error, + }); const taskId = this.findAgentTaskId(event.subagentId); if (taskId !== undefined && this.backgroundTaskTranscriptedTerminal.has(taskId)) { return; diff --git a/apps/kimi-code/src/tui/controllers/streaming-ui.ts b/apps/kimi-code/src/tui/controllers/streaming-ui.ts index 1de71c0f04..ebed769faf 100644 --- a/apps/kimi-code/src/tui/controllers/streaming-ui.ts +++ b/apps/kimi-code/src/tui/controllers/streaming-ui.ts @@ -201,6 +201,13 @@ export class StreamingUIController { agentId?: string | undefined; description: string; status: 'completed' | 'failed' | 'killed' | 'lost'; + /** + * Real failure message to surface on the card. Pass the `subagent.failed` + * event's `error` for live crashes — it is far more useful than the + * friendly generic the card falls back to. Omit on the resume / terminate + * path where no real error is available. + */ + errorText?: string | undefined; }): boolean { let agentIdMatch: ToolCallComponent | undefined; let descMatch: ToolCallComponent | undefined; @@ -238,7 +245,7 @@ export class StreamingUIController { } const target = agentIdMatch ?? (descAmbiguous ? undefined : descMatch); if (target === undefined) return false; - target.setBackgroundTaskTerminalStatus(args.status); + target.setBackgroundTaskTerminalStatus(args.status, { errorText: args.errorText }); return true; } diff --git a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts index fb9199935b..d57e9dfe5f 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts @@ -867,6 +867,97 @@ describe('ToolCallComponent', () => { expect(out).not.toMatch(/Failed/); expect(out).not.toContain('task_id:'); }); + + // Stable id routing — `tc.subagentAgentId` is left undefined for + // backgrounded agents both live (`handleSubagentSpawned` early-returns + // for `runInBackground`, never calling tc.onSubagentSpawned) and on + // resume (the wire format does not carry a `subagent` block back into + // `applySubagentReplay`). The AgentTool's spawn-success ToolResult, + // however, always carries `agent_id: agent-N` — fall back to parsing + // that so callers asking `getSubagentAgentId` always get the right id, + // and `applyBackgroundTaskTerminalStatus` can route by id instead of + // by description (which collides between unrelated cards). + it('getSubagentAgentId parses agent_id from the spawn-success ToolResult', () => { + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'background agent 1', run_in_background: true }, + }, + spawnSuccessResult, + darkColors, + ); + // No spawn metadata was wired in — exactly the resume / backgrounded + // case we are guarding against. + expect(component.getSubagentAgentId()).toBe('agent-0'); + }); + + it('getSubagentAgentId still prefers in-memory subagent metadata when set', () => { + // If `setSubagentMeta` / `onSubagentSpawned` did wire an id, that one + // is authoritative — it survived the in-flight phase before any + // ToolResult landed and can disambiguate concurrent calls. + const component = new ToolCallComponent( + { + id: 'call_bg_agent', + name: 'Agent', + args: { description: 'X', run_in_background: true }, + }, + spawnSuccessResult, + darkColors, + ); + component.setSubagentMeta('agent-explicit', 'coder'); + expect(component.getSubagentAgentId()).toBe('agent-explicit'); + }); + + it('getSubagentAgentId returns undefined for non-Agent tool calls even when output looks similar', () => { + const component = new ToolCallComponent( + { + id: 'call_bash', + name: 'Bash', + args: { command: 'echo agent_id: agent-fake' }, + }, + { + tool_call_id: 'call_bash', + output: 'agent_id: agent-fake\nstatus: running', + is_error: false, + }, + darkColors, + ); + expect(component.getSubagentAgentId()).toBeUndefined(); + }); + + it('setBackgroundTaskTerminalStatus errorText overwrites the friendly generic', () => { + // Live failures arrive via `subagent.failed` with the real error from + // the subagent loop. That string is far more informative than the + // generic "Background agent failed" fallback the friendly path emits. + // When the caller supplies errorText it must win, regardless of + // whether the friendly message was written first. + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed'); + expect(component.getSubagentSnapshot().errorText).toBe('Background agent failed'); + + component.setBackgroundTaskTerminalStatus('failed', { + errorText: 'subagent exceeded max_steps', + }); + expect(component.getSubagentSnapshot().errorText).toBe('subagent exceeded max_steps'); + }); + + it('setBackgroundTaskTerminalStatus errorText is written even on first call', () => { + const component = makeBackgroundAgentComponent(); + component.setBackgroundTaskTerminalStatus('failed', { + errorText: 'OAuth refresh failed', + }); + expect(component.getSubagentSnapshot().errorText).toBe('OAuth refresh failed'); + }); + + it('setBackgroundTaskTerminalStatus does not overwrite a real onSubagentFailed error with the generic', () => { + const component = makeBackgroundAgentComponent(); + component.onSubagentFailed({ error: 'real crash from subagent' }); + // background.task.terminated event arrives later without an errorText + // override; the friendly generic must NOT clobber the real message. + component.setBackgroundTaskTerminalStatus('failed'); + expect(component.getSubagentSnapshot().errorText).toBe('real crash from subagent'); + }); }); it('scrolls the Write streaming preview to the last COMMAND_PREVIEW_LINES', () => { From e0e190b553ed113a75a8b9e420c3eedacf0695e4 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 17:14:45 +0800 Subject: [PATCH 5/5] fix(tui): treat agent_id as authoritative when matching bg terminal events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously `applyBackgroundTaskTerminalStatus` always tried agent_id first and then fell back to description match on miss. That fallback caused two cross-card bugs: 1. On resume, `applyTerminalBackgroundAgentStatuses` iterates every persisted terminal task, including ones whose tool calls fell outside the `REPLAY_TURN_LIMIT` window and were never mounted. Description fallback could route an old `lost` status onto an unrelated recent Agent card sharing the same `args.description`. 2. During the live spawn → terminate window, the same card briefly lives in both `_pendingToolComponents` and `transcriptContainer`. A description-only walk visits the same component twice and flags itself ambiguous, dropping the otherwise unambiguous update. When `args.agentId` is provided we now match only by id and skip on miss. With `getSubagentAgentId` already parsing `agent_id: agent-N` out of the spawn-success ToolResult, the id path is reliable for both live and resume even though `tc.subagentAgentId` is never populated for backgrounded agents. Description fallback is preserved solely for old pre-PR sessions whose persisted records lack `agent_id` — same best-effort behavior as before. --- .../src/tui/controllers/streaming-ui.ts | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/apps/kimi-code/src/tui/controllers/streaming-ui.ts b/apps/kimi-code/src/tui/controllers/streaming-ui.ts index ebed769faf..cb0a332176 100644 --- a/apps/kimi-code/src/tui/controllers/streaming-ui.ts +++ b/apps/kimi-code/src/tui/controllers/streaming-ui.ts @@ -180,15 +180,26 @@ export class StreamingUIController { * trusts the spawn-success ToolResult (which would otherwise label every * terminated bg agent — including `lost` ones — as `✓ Completed`). * - * Resolution order, picked to handle both live and resume: - * 1. `agentId` → matches the subagent id recorded on `spawned` - * (live path; replayed tool calls currently do not carry a - * subagent id back, so this branch only catches live mismatches). - * 2. Description fallback — required on resume because reconcile's - * `BackgroundTaskInfo` is the only handle the TUI has to a - * replayed Agent card. Only used when exactly one Agent tool call - * shares the description; ambiguous matches are skipped to avoid - * corrupting an unrelated card. + * Resolution policy: an `args.agentId` is treated as authoritative — we + * either find a card whose `getSubagentAgentId()` returns the same id + * (in-memory metadata for live foreground, parsed from the spawn-success + * `agent_id: ...` line for live backgrounded and replayed cards) or we + * skip. We deliberately do NOT fall back to description match when + * `agentId` is provided, because: + * - On resume, `applyTerminalBackgroundAgentStatuses` iterates every + * persisted terminal task, including ones whose tool calls fell + * outside the `REPLAY_TURN_LIMIT` window. A description fallback + * would let an old `lost` task stamp its status onto an unrelated + * recent Agent card that happens to share `args.description`. + * - During a live spawn / terminate race, the same card can briefly + * appear in both `_pendingToolComponents` and `transcriptContainer`, + * so a description match could double-visit the same component and + * mark itself ambiguous. agentId match short-circuits on the first + * hit and is immune. + * + * Description fallback is kept as a best-effort path only when + * `agentId` is unknown — that is, on resume of pre-PR sessions whose + * disk records pre-date `agent_id` persistence. * * Search scope includes both in-flight components and already-mounted * cards (some live in `transcriptContainer` standalone, others are @@ -209,13 +220,14 @@ export class StreamingUIController { */ errorText?: string | undefined; }): boolean { + const useAgentIdOnly = args.agentId !== undefined; let agentIdMatch: ToolCallComponent | undefined; let descMatch: ToolCallComponent | undefined; let descAmbiguous = false; const visit = (tc: ToolCallComponent): void => { if (agentIdMatch !== undefined) return; - if (args.agentId !== undefined && tc.getSubagentAgentId() === args.agentId) { - agentIdMatch = tc; + if (useAgentIdOnly) { + if (tc.getSubagentAgentId() === args.agentId) agentIdMatch = tc; return; } if (tc.getAgentToolDescription() !== args.description) return; @@ -243,7 +255,11 @@ export class StreamingUIController { if (agentIdMatch !== undefined) break; } } - const target = agentIdMatch ?? (descAmbiguous ? undefined : descMatch); + const target = useAgentIdOnly + ? agentIdMatch + : descAmbiguous + ? undefined + : descMatch; if (target === undefined) return false; target.setBackgroundTaskTerminalStatus(args.status, { errorText: args.errorText }); return true;