From b5121e445bbf2ef0ac8d6117ce1dd1f5dea33467 Mon Sep 17 00:00:00 2001 From: RelaxJonh <92573950+RelaxJonh@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:51:02 +0700 Subject: [PATCH 1/3] fix(desktop): normalise timeout/error subagent statuses to terminal (#73728) The backend emits terminal statuses including 'timeout' and 'error' in subagent.complete payloads, but asStatus() only recognised 'completed', 'failed', 'interrupted', and 'queued'. Unrecognised values fell through to 'running', making timed-out subagents immortal in the active status stack. Fix: map timeout/error to 'failed', cancelled/canceled to 'interrupted'. Nonterminal unknown statuses still default to 'running' for forward compatibility. Fixes #73728 --- apps/desktop/src/store/subagents.test.ts | 37 ++++++++++++++++++++++++ apps/desktop/src/store/subagents.ts | 8 +++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/store/subagents.test.ts b/apps/desktop/src/store/subagents.test.ts index 254a7a22f4760..b6fb278bde219 100644 --- a/apps/desktop/src/store/subagents.test.ts +++ b/apps/desktop/src/store/subagents.test.ts @@ -190,4 +190,41 @@ describe('subagent store', () => { .sort() ).toEqual(['c', 'd']) }) + + // Regression test for #73728: backend terminal statuses like `timeout` and + // `error` were normalised to `running`, making timed-out subagents immortal + // in the active status stack. `cancelled`/`canceled` must also map to + // `interrupted`. + it('normalises backend terminal statuses to recognised SubagentStatus values', () => { + upsertSubagent('s1', { goal: 'a', status: 'running', subagent_id: 'a', task_index: 0 }) + upsertSubagent('s1', { goal: 'b', status: 'running', subagent_id: 'b', task_index: 1 }) + upsertSubagent('s1', { goal: 'c', status: 'running', subagent_id: 'c', task_index: 2 }) + upsertSubagent('s1', { goal: 'd', status: 'running', subagent_id: 'd', task_index: 3 }) + + // Emit terminal events with backend-native status strings + upsertSubagent('s1', { status: 'timeout', subagent_id: 'a', task_index: 0, summary: 'timed out' }, false, 'subagent.complete') + upsertSubagent('s1', { status: 'error', subagent_id: 'b', task_index: 1, summary: 'errored' }, false, 'subagent.complete') + upsertSubagent('s1', { status: 'cancelled', subagent_id: 'c', task_index: 2 }, false, 'subagent.complete') + upsertSubagent('s1', { status: 'canceled', subagent_id: 'd', task_index: 3 }, false, 'subagent.complete') + + const items = listFor('s1') + const byId = Object.fromEntries(items.map(i => [i.id, i])) + + // timeout → failed + expect(byId['a']?.status).toBe('failed') + expect(byId['a']?.currentTool).toBeUndefined() + + // error → failed + expect(byId['b']?.status).toBe('failed') + + // cancelled → interrupted + expect(byId['c']?.status).toBe('interrupted') + + // canceled → interrupted + expect(byId['d']?.status).toBe('interrupted') + + // All four are terminal — prune should remove them all + pruneFinishedSessionSubagents('s1') + expect(listFor('s1')).toHaveLength(0) + }) }) diff --git a/apps/desktop/src/store/subagents.ts b/apps/desktop/src/store/subagents.ts index 6127b15183c25..27402fe4493db 100644 --- a/apps/desktop/src/store/subagents.ts +++ b/apps/desktop/src/store/subagents.ts @@ -55,8 +55,12 @@ const str = (v: unknown) => (isStr(v) ? v : '') const num = (v: unknown) => (typeof v === 'number' && Number.isFinite(v) ? v : undefined) const strList = (v: unknown) => (Array.isArray(v) ? v.filter(isStr) : []) -const asStatus = (v: unknown): SubagentStatus => - v === 'completed' || v === 'failed' || v === 'interrupted' || v === 'queued' ? v : 'running' +const asStatus = (v: unknown): SubagentStatus => { + if (v === 'completed' || v === 'failed' || v === 'interrupted' || v === 'queued') return v + if (v === 'timeout' || v === 'error') return 'failed' + if (v === 'cancelled' || v === 'canceled') return 'interrupted' + return 'running' +} const compact = (text: string, max = PREVIEW_MAX) => { const line = text.replace(/\s+/g, ' ').trim() From 577f874ad846aaa519ce0a22d5207e34e7d71817 Mon Sep 17 00:00:00 2001 From: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:17:33 -0700 Subject: [PATCH 2/3] fix(desktop): fail closed on unrecognized subagent.complete statuses; surface timeout reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the #73728 normalization fix (supersedes the event-agnostic fallback the maintainers flagged as incomplete): - subagent.complete is terminal by definition — an unrecognized status on it now renders as 'failed' instead of falling through to 'running', which would recreate the immortal false-active row for any future backend status (the keep_open request on #73859). - Live events keep the lenient 'running' fallback. - Synthesize a 'Timed out after Xs' summary from duration_seconds when the backend completes with status 'timeout' and no summary, so the failed row explains itself. - Tests: timeout reason synthesis + pruning, event-aware fail-closed vs lenient live fallback (13 total). --- apps/desktop/src/store/subagents.test.ts | 48 ++++++++++++++++++++++++ apps/desktop/src/store/subagents.ts | 40 ++++++++++++++++---- 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/store/subagents.test.ts b/apps/desktop/src/store/subagents.test.ts index b6fb278bde219..57045fe5f0473 100644 --- a/apps/desktop/src/store/subagents.test.ts +++ b/apps/desktop/src/store/subagents.test.ts @@ -227,4 +227,52 @@ describe('subagent store', () => { pruneFinishedSessionSubagents('s1') expect(listFor('s1')).toHaveLength(0) }) + + // The backend completes subagents with status "timeout" (hard child timeout, + // delegation.child_timeout_seconds) and no summary — synthesize the reason + // so the failed row explains itself instead of rendering as a bare failure. + it('maps backend timeout status to a terminal failure with a synthesized reason', () => { + upsertSubagent('s1', { goal: 'scan files', status: 'running', subagent_id: 't1', task_index: 0 }) + upsertSubagent( + 's1', + { status: 'timeout', subagent_id: 't1', task_index: 0, duration_seconds: 612.3 }, + false, + 'subagent.complete' + ) + + const item = listFor('s1')[0] + expect(item?.status).toBe('failed') + expect(item?.durationSeconds).toBe(612.3) + expect(item?.summary).toBe('Timed out after 612.3s') + + // A timed-out row must be pruned at the next message.start boundary like + // any other finished row — it must not linger as a live spinner. + pruneFinishedSessionSubagents('s1') + expect(listFor('s1')).toHaveLength(0) + }) + + // Fail-closed guard: subagent.complete is terminal by definition, so an + // unrecognized status on it must not resurrect a row as 'running'. Live + // events keep the lenient fallback (a status we don't know is still active). + it('fails closed on unrecognized completion statuses but stays lenient for live events', () => { + upsertSubagent('s1', { goal: 'scan files', status: 'running', subagent_id: 'u1', task_index: 0 }) + upsertSubagent( + 's1', + { status: 'some_future_terminal_status', subagent_id: 'u1', task_index: 0 }, + false, + 'subagent.complete' + ) + expect(listFor('s1')[0]?.status).toBe('failed') + expect(activeSubagentCount(listFor('s1'))).toBe(0) + + upsertSubagent('s1', { goal: 'scan files', status: 'running', subagent_id: 'u2', task_index: 1 }) + upsertSubagent( + 's1', + { status: 'some_future_live_status', subagent_id: 'u2', task_index: 1, text: 'still working' }, + false, + 'subagent.progress' + ) + expect(listFor('s1')[1]?.status).toBe('running') + expect(activeSubagentCount(listFor('s1'))).toBe(1) + }) }) diff --git a/apps/desktop/src/store/subagents.ts b/apps/desktop/src/store/subagents.ts index 27402fe4493db..b962a64501cca 100644 --- a/apps/desktop/src/store/subagents.ts +++ b/apps/desktop/src/store/subagents.ts @@ -55,10 +55,27 @@ const str = (v: unknown) => (isStr(v) ? v : '') const num = (v: unknown) => (typeof v === 'number' && Number.isFinite(v) ? v : undefined) const strList = (v: unknown) => (Array.isArray(v) ? v.filter(isStr) : []) -const asStatus = (v: unknown): SubagentStatus => { - if (v === 'completed' || v === 'failed' || v === 'interrupted' || v === 'queued') return v - if (v === 'timeout' || v === 'error') return 'failed' - if (v === 'cancelled' || v === 'canceled') return 'interrupted' +const asStatus = (v: unknown, terminalEvent = false): SubagentStatus => { + if (v === 'completed' || v === 'failed' || v === 'interrupted' || v === 'queued') { + return v + } + + if (v === 'timeout' || v === 'error') { + return 'failed' + } + + if (v === 'cancelled' || v === 'canceled') { + return 'interrupted' + } + + // Fail closed on completion: a subagent.complete event is terminal by + // definition, so an unrecognized status must render as a failure rather + // than leave a dead row spinning as 'running' forever. Live events keep + // the lenient 'running' fallback. + if (terminalEvent) { + return 'failed' + } + return 'running' } @@ -110,6 +127,15 @@ const appendStream = (stream: SubagentStreamEntry[], entry: SubagentStreamEntry) return [...stream, entry].slice(-MAX_STREAM) } +// The backend sends no summary on a hard child timeout (only a preview like +// "Timed out after 612.3s" + duration_seconds). Synthesize it so the terminal +// row explains why it failed instead of rendering as a bare failure. +const timeoutSummary = (payload: SubagentPayload): string => { + const seconds = num(payload.duration_seconds) + + return str(payload.status) === 'timeout' ? `Timed out after ${seconds ?? '?'}s` : '' +} + function streamFromPayload( payload: SubagentPayload, status: SubagentStatus, @@ -141,7 +167,7 @@ function streamFromPayload( out.push({ at, kind: 'thinking', text }) } - const summary = compact(str(payload.summary) || str(payload.text)) + const summary = compact(str(payload.summary) || str(payload.text) || timeoutSummary(payload)) if (TERMINAL.has(status) && summary) { out.push({ at, isError: status === 'failed', kind: 'summary', text: summary }) @@ -152,7 +178,7 @@ function streamFromPayload( function toProgress(payload: SubagentPayload, prev: SubagentProgress | undefined, eventType = ''): SubagentProgress { const at = Date.now() - const status = asStatus(payload.status) + const status = asStatus(payload.status, eventType === 'subagent.complete') const tool = str(payload.tool_name) const stream = streamFromPayload(payload, status, eventType, at).reduce(appendStream, prev?.stream ?? []) const filesRead = strList(payload.files_read) @@ -177,7 +203,7 @@ function toProgress(payload: SubagentPayload, prev: SubagentProgress | undefined filesRead: filesRead.length ? filesRead : (prev?.filesRead ?? []), filesWritten: filesWritten.length ? filesWritten : (prev?.filesWritten ?? []), stream, - summary: str(payload.summary) || prev?.summary, + summary: str(payload.summary) || prev?.summary || timeoutSummary(payload) || undefined, currentTool: TERMINAL.has(status) ? undefined : tool || prev?.currentTool } } From 18ff61a46db6b256f41f9708ae1fbb33b35f6c66 Mon Sep 17 00:00:00 2001 From: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:18:56 -0700 Subject: [PATCH 3/3] fix(desktop): prefer synthesized timeout summary over stale progress text Review feedback: prev?.summary could shadow the 'Timed out after Xs' reason when a live event had populated it. timeoutSummary() now wins for raw timeout status; add coverage for the missing-duration placeholder. --- apps/desktop/src/store/subagents.test.ts | 7 +++++++ apps/desktop/src/store/subagents.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/store/subagents.test.ts b/apps/desktop/src/store/subagents.test.ts index 57045fe5f0473..e486cbca0a464 100644 --- a/apps/desktop/src/store/subagents.test.ts +++ b/apps/desktop/src/store/subagents.test.ts @@ -251,6 +251,13 @@ describe('subagent store', () => { expect(listFor('s1')).toHaveLength(0) }) + it('falls back to a placeholder when timeout duration is missing', () => { + upsertSubagent('s1', { goal: 'scan files', status: 'running', subagent_id: 't2', task_index: 0 }) + upsertSubagent('s1', { status: 'timeout', subagent_id: 't2', task_index: 0 }, false, 'subagent.complete') + + expect(listFor('s1')[0]?.summary).toBe('Timed out after ?s') + }) + // Fail-closed guard: subagent.complete is terminal by definition, so an // unrecognized status on it must not resurrect a row as 'running'. Live // events keep the lenient fallback (a status we don't know is still active). diff --git a/apps/desktop/src/store/subagents.ts b/apps/desktop/src/store/subagents.ts index b962a64501cca..3f7b15bbce19f 100644 --- a/apps/desktop/src/store/subagents.ts +++ b/apps/desktop/src/store/subagents.ts @@ -203,7 +203,7 @@ function toProgress(payload: SubagentPayload, prev: SubagentProgress | undefined filesRead: filesRead.length ? filesRead : (prev?.filesRead ?? []), filesWritten: filesWritten.length ? filesWritten : (prev?.filesWritten ?? []), stream, - summary: str(payload.summary) || prev?.summary || timeoutSummary(payload) || undefined, + summary: str(payload.summary) || timeoutSummary(payload) || prev?.summary || undefined, currentTool: TERMINAL.has(status) ? undefined : tool || prev?.currentTool } }