From d3ad77d8856470cb0fc5835b830e747509758564 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 13 Sep 2026 23:41:12 -0700 Subject: [PATCH] fix(native-chat): preserve background task ownership across restarts --- .../claude/claude-background-task-frames.ts | 1 + .../claude-background-task-rows.test.ts | 74 +++++++++++++++++++ .../claude/claude-background-task-rows.ts | 37 ++++++++-- .../claude-subagent-task-frames.test.ts | 14 ++++ 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/main/claude/claude-background-task-frames.ts b/src/main/claude/claude-background-task-frames.ts index 91c5320602e..063efd9808b 100644 --- a/src/main/claude/claude-background-task-frames.ts +++ b/src/main/claude/claude-background-task-frames.ts @@ -80,6 +80,7 @@ export function taskName(frame: Record): string | undefined { export function classifyClaudeBackgroundTaskKind(taskType: unknown): ClaudeBackgroundTaskKind { switch (taskType) { case 'local_agent': + case 'local_subagent': return 'agent' case 'local_workflow': return 'workflow' diff --git a/src/main/claude/claude-background-task-rows.test.ts b/src/main/claude/claude-background-task-rows.test.ts index 2447b9c8df0..aa86d04306b 100644 --- a/src/main/claude/claude-background-task-rows.test.ts +++ b/src/main/claude/claude-background-task-rows.test.ts @@ -183,6 +183,25 @@ describe('claude background task rows', () => { expect(items).toEqual([]) }) + it('leaves legacy local_subagent tasks to the subagent roster', () => { + const { rows, items } = harness() + rows.observe({ + type: 'system', + subtype: 'task_started', + task_id: 'task-legacy-agent', + task_type: 'local_subagent', + subagent_type: 'explorer' + }) + rows.observe({ + type: 'system', + subtype: 'task_notification', + task_id: 'task-legacy-agent', + status: 'failed', + summary: 'the child failed' + }) + expect(items).toEqual([]) + }) + it('writes nothing for ambient housekeeping the user never asked for', () => { const { rows, items } = harness() rows.observe({ ...START_BASH, task_id: 'ambient-1', ambient: true }) @@ -216,6 +235,25 @@ describe('claude background task rows', () => { expect(items).toEqual([]) }) + it('keeps terminal ownership after a tracked task reports foregrounded', () => { + const { rows, latest } = harness() + rows.observe(START_BASH) + rows.observe({ + type: 'system', + subtype: 'task_updated', + task_id: START_BASH.task_id, + patch: { is_backgrounded: false, status: 'running' } + }) + rows.observe({ + type: 'system', + subtype: 'task_notification', + task_id: START_BASH.task_id, + status: 'failed', + summary: 'foreground transition failed' + }) + expect(latest()).toMatchObject({ state: 'blocked', summary: 'foreground transition failed' }) + }) + it('revises in place rather than opening a row from a patch', () => { const { rows, items, latest } = harness() rows.observe({ @@ -454,6 +492,42 @@ describe('claude background task rows', () => { }) }) + it('reopens a settled task after its row was evicted when the parent alias changes', () => { + const { rows, keys, latest } = harness([FORWARDED_TOOL, 'toolu_second']) + rows.observe({ ...START_BASH, task_id: 'evicted-restart' }) + rows.observe({ + type: 'system', + subtype: 'task_notification', + task_id: 'evicted-restart', + tool_use_id: FORWARDED_TOOL, + status: 'completed' + }) + for (let index = 0; index < 63; index += 1) { + rows.observe({ ...START_BASH, task_id: `settled-${index}` }) + rows.observe({ + type: 'system', + subtype: 'task_notification', + task_id: `settled-${index}`, + status: 'completed' + }) + } + // The first settled row is evicted to make room for this one. + rows.observe({ ...START_BASH, task_id: 'evictor' }) + rows.observe({ + ...START_BASH, + task_id: 'evicted-restart', + tool_use_id: 'toolu_second', + description: 'second invocation' + }) + + expect([...new Set(keys())]).toContain('claude-background-task:evicted-restart#2') + expect(latest()).toMatchObject({ + taskId: 'evicted-restart', + state: 'working', + label: 'second invocation' + }) + }) + it('declines coverage so the fallback still reports when every row slot is live', () => { // The row map is bounded. A task that cannot be admitted for lack of a slot // is not silently swallowed: coverage is declined so the generic fallback diff --git a/src/main/claude/claude-background-task-rows.ts b/src/main/claude/claude-background-task-rows.ts index 639096a3e05..43e6ffc4359 100644 --- a/src/main/claude/claude-background-task-rows.ts +++ b/src/main/claude/claude-background-task-rows.ts @@ -55,6 +55,10 @@ export class ClaudeBackgroundTaskRows { private readonly generations = new Map() private readonly foreign = new Map() private readonly terminalTaskIds = new Set() + /** The parent alias for the terminal run, when one was reported. Keeping it + * lets an evicted row distinguish a late duplicate start from a genuine + * restart under a fresh tool invocation. */ + private readonly terminalToolUseIds = new Map() private readonly ids = new ClaudeSubagentIds() private readonly now: () => number @@ -106,6 +110,7 @@ export class ClaudeBackgroundTaskRows { this.generations.clear() this.foreign.clear() this.terminalTaskIds.clear() + this.terminalToolUseIds.clear() this.ids.clear() } @@ -152,8 +157,21 @@ export class ClaudeBackgroundTaskRows { } return true } + let restartedTerminal = false if (this.terminalTaskIds.has(id)) { - return true + const previousToolUseId = this.terminalToolUseIds.get(id) + const currentToolUseId = claudeBackgroundTaskToolUseId(message) + // A terminal edge that had no usable tool id cannot prove a later start + // is a new run, so keep the conservative orphan guard. When both runs + // name their parent, a different alias is the provider's restart signal. + if ( + previousToolUseId === undefined || + currentToolUseId === undefined || + previousToolUseId === currentToolUseId + ) { + return true + } + restartedTerminal = true } if (!this.admitsFirstRun(message)) { return true @@ -161,6 +179,10 @@ export class ClaudeBackgroundTaskRows { if (!this.ensureRowSlot()) { return false } + if (restartedTerminal) { + this.terminalTaskIds.delete(id) + this.terminalToolUseIds.delete(id) + } this.openRow(id, message, (this.generations.get(id) ?? 0) + 1) return true } @@ -188,7 +210,7 @@ export class ClaudeBackgroundTaskRows { // Remembered even for a task never admitted: Orca is deliberately stricter // than the reference here, which keeps no trace of one. It stops a late // announcement from opening a row for work already reported finished. - this.rememberTerminalId(id) + this.rememberTerminalId(id, claudeBackgroundTaskToolUseId(message)) if (!this.rows.has(id)) { // Matched on `task_id` alone. A terminal frame for a task that was never // admitted names nothing this transcript is tracking, so it yields no @@ -202,13 +224,16 @@ export class ClaudeBackgroundTaskRows { private observePatch(id: string, message: Record): boolean { const patch = record(message.patch) - if (patch?.is_backgrounded === false) { + // A tracked row remains this owner's responsibility even if a later patch + // reports foreground execution; its terminal notification still revises + // the durable row. Only an untracked task belongs to the foreground owner. + if (patch?.is_backgrounded === false && !this.rows.has(id)) { this.rememberForeign(id, 'foreground') return true } const change = claudeBackgroundTaskPatchChange(message) if (change.state && isSettledBackgroundTaskState(change.state)) { - this.rememberTerminalId(id) + this.rememberTerminalId(id, claudeBackgroundTaskToolUseId(message)) } // A patch is folded into the row it names and is never a row of its own, so // an untracked task takes no row from it. @@ -286,16 +311,18 @@ export class ClaudeBackgroundTaskRows { } } - private rememberTerminalId(id: string): void { + private rememberTerminalId(id: string, toolUseId?: string): void { if (this.terminalTaskIds.has(id)) { this.terminalTaskIds.delete(id) } this.terminalTaskIds.add(id) + this.terminalToolUseIds.set(id, toolUseId ?? this.rows.get(id)?.toolUseId) while (this.terminalTaskIds.size > MAX_TERMINAL_TASK_IDS) { const oldest = this.terminalTaskIds.values().next() if (oldest.done || oldest.value === id) { break } + this.terminalToolUseIds.delete(oldest.value) this.terminalTaskIds.delete(oldest.value) } } diff --git a/src/main/claude/claude-subagent-task-frames.test.ts b/src/main/claude/claude-subagent-task-frames.test.ts index 230ef45e19c..58950307ba5 100644 --- a/src/main/claude/claude-subagent-task-frames.test.ts +++ b/src/main/claude/claude-subagent-task-frames.test.ts @@ -33,6 +33,20 @@ describe('readClaudeSubagentTaskFrame', () => { }) }) + it('announces the legacy local_subagent task type', () => { + expect( + readClaudeSubagentTaskFrame( + system('task_started', { + task_id: 'task-legacy-subagent', + tool_use_id: 'toolu_legacy', + task_type: 'local_subagent', + subagent_type: 'code-reviewer', + description: 'Review the diff' + }) + ) + ).toMatchObject({ announcesSubagent: true, excluded: false }) + }) + it('excludes a backgrounded shell command even though it carries a tool_use_id', () => { const frame = readClaudeSubagentTaskFrame( system('task_started', {