diff --git a/src/renderer/src/components/sidebar/StatusIndicator.tsx b/src/renderer/src/components/sidebar/StatusIndicator.tsx index 97726487c19..967c9956996 100644 --- a/src/renderer/src/components/sidebar/StatusIndicator.tsx +++ b/src/renderer/src/components/sidebar/StatusIndicator.tsx @@ -53,6 +53,21 @@ const StatusIndicator = React.memo(function StatusIndicator({ ) } + if (status === 'interrupted') { + return ( + + {/* Why red, and why not the emerald done dot: the turn was cancelled or died, so it needs a + human to restart or look — presenting it as a completion is the bug (STA-5357). Matches the + agent row's own interrupted treatment (AgentStateDot uses bg-red-500). */} + + + ) + } + if (status === 'permission') { return ( > @@ -27,6 +30,7 @@ const EMPTY_SUMMARY: WorktreeAgentActivitySummary = { hasPermission: false, hasLiveWorking: false, hasLiveMonitoring: false, + hasInterrupted: false, hasLiveDone: false, hasRetainedDone: false, agentStatusPaneIdsByTabId: EMPTY_AGENT_STATUS_PANE_IDS_BY_TAB_ID @@ -180,6 +184,7 @@ function summariesEqual( previous.hasPermission === next.hasPermission && previous.hasLiveWorking === next.hasLiveWorking && previous.hasLiveMonitoring === next.hasLiveMonitoring && + previous.hasInterrupted === next.hasInterrupted && previous.hasLiveDone === next.hasLiveDone && previous.hasRetainedDone === next.hasRetainedDone && agentStatusPaneIdsByTabIdEqual( @@ -217,10 +222,14 @@ function agentStatusPaneIdsByTabIdEqual( function applyLiveAgentState( summary: WorktreeAgentActivitySummary, - entry: Pick + entry: Pick ): void { if (entry.state === 'blocked' || entry.state === 'waiting') { summary.hasPermission = true + } else if (entry.interrupted === true) { + // Why before the `done` branch: `interrupted` is clamped to `done` at parse time, so a cancelled + // turn would otherwise set `hasLiveDone` and render as a completed one. + summary.hasInterrupted = true } else if (entry.state === 'working') { if (entry.workingMode === 'monitoring') { summary.hasLiveMonitoring = true diff --git a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts index f173b4261f3..43662bff074 100644 --- a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts +++ b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts @@ -110,7 +110,11 @@ describe('resolveTerminalTabActivityStatus', () => { ).toBe('done') }) - it('treats an interrupted done as done, matching the worktree card', () => { + // Why this flipped (STA-5357): it used to assert `done`, deliberately mirroring the worktree card, + // which had no interrupted state. That made a cancelled turn indistinguishable from a clean finish + // on every glanceable surface. The card now distinguishes it, so the tab follows rather than + // preserving a consistency that was consistently wrong. + it('reports an interrupted done as interrupted, matching the worktree card', () => { const interrupted = entry(FIRST_LEAF_ID, 'done', { interrupted: true }) expect( resolveTerminalTabActivityStatus({ @@ -118,7 +122,22 @@ describe('resolveTerminalTabActivityStatus', () => { agentStatusByPaneKey: { [interrupted.paneKey]: interrupted }, ptyIdsByTabId: LIVE_PTY }) - ).toBe('done') + ).toBe('interrupted') + }) + + it('does not let a finished sibling mask an interrupted pane', () => { + const interrupted = entry(FIRST_LEAF_ID, 'done', { interrupted: true }) + const finished = entry(SECOND_LEAF_ID, 'done') + expect( + resolveTerminalTabActivityStatus({ + tab: TAB, + agentStatusByPaneKey: { + [interrupted.paneKey]: interrupted, + [finished.paneKey]: finished + }, + ptyIdsByTabId: LIVE_PTY + }) + ).toBe('interrupted') }) it('falls back to a live working title when hook status is stale', () => { diff --git a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts index c130a602e1a..923a26c006d 100644 --- a/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts +++ b/src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts @@ -20,6 +20,7 @@ type TerminalTabActivityFlags = { hasPermission: boolean hasLiveWorking: boolean hasLiveMonitoring: boolean + hasInterrupted: boolean hasLiveDone: boolean paneIds: Set } @@ -81,10 +82,12 @@ function getTerminalTabActivityFlags( } else { flags.hasLiveWorking = true } + } else if (entry.interrupted === true) { + // Why before `done` (STA-5357): the flag is clamped to `done`, so folding it in reported a + // cancelled turn as a completed one. This used to mirror the WorktreeCard dot deliberately — + // that dot now distinguishes interrupted, so the tab follows rather than diverging. + flags.hasInterrupted = true } else if (entry.state === 'done') { - // Why: an interrupted `done` still reads as completed here, matching the - // WorktreeCard dot (resolveWorktreeStatus has no interrupted state); only - // the smart-sort ordering treats interrupts as idle. flags.hasLiveDone = true } } @@ -103,6 +106,7 @@ function getOrCreateTerminalTabActivityFlags( hasPermission: false, hasLiveWorking: false, hasLiveMonitoring: false, + hasInterrupted: false, hasLiveDone: false, paneIds: new Set() } @@ -164,6 +168,7 @@ export function resolveTerminalTabActivityStatus({ hasPermission: flags?.hasPermission ?? false, hasLiveWorking: flags?.hasLiveWorking ?? false, hasLiveMonitoring: flags?.hasLiveMonitoring ?? false, + hasInterrupted: flags?.hasInterrupted ?? false, hasLiveDone: flags?.hasLiveDone ?? false, // Why: retained/orchestration promotions are worktree-aggregate concerns; // a tab reflects its own live panes and title only. @@ -180,7 +185,13 @@ export function isTerminalTabActivityLive(status: TerminalTabActivityStatus): bo * Glyph-bearing attention states for a terminal tab (tab bar + Cmd+J recent chats). * Quiet active/inactive map to null so identity icons stay clean. */ -export type TerminalTabAttentionBadge = 'working' | 'monitoring' | 'permission' | 'unread' | 'done' +export type TerminalTabAttentionBadge = + | 'working' + | 'monitoring' + | 'permission' + | 'interrupted' + | 'unread' + | 'done' /** * Single priority ladder shared by the tab strip and Cmd+J recent rows: @@ -202,6 +213,10 @@ export function resolveTerminalTabAttentionBadge({ if (status === 'monitoring') { return 'monitoring' } + // Why above unread and done: a cancelled turn needs a human, and an unread bell would bury it. + if (status === 'interrupted') { + return 'interrupted' + } if (hasUnread) { return 'unread' } @@ -214,11 +229,12 @@ export function resolveTerminalTabAttentionBadge({ /** Map a container activity status onto AgentStateDot's vocabulary (no unread — that's a bell). */ export function terminalTabActivityToAgentDotState( status: TerminalTabActivityStatus -): 'working' | 'monitoring' | 'permission' | 'done' | null { +): 'working' | 'monitoring' | 'permission' | 'interrupted' | 'done' | null { switch (status) { case 'working': case 'monitoring': case 'permission': + case 'interrupted': case 'done': return status case 'active': diff --git a/src/renderer/src/lib/worktree-status.interrupted.test.ts b/src/renderer/src/lib/worktree-status.interrupted.test.ts new file mode 100644 index 00000000000..e51d68d8741 --- /dev/null +++ b/src/renderer/src/lib/worktree-status.interrupted.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { resolveWorktreeStatus } from './worktree-status' + +// Why these live apart from the main suite: they pin the ONE rule STA-5357 adds — +// an interrupted agent is not a completed one, and it must not be masked by a +// sibling that merely finished. +const base = { + tabs: [] as never[], + browserTabs: [] as never[], + ptyIdsByTabId: {}, + hasPermission: false, + hasLiveWorking: false, + hasLiveDone: false, + hasRetainedDone: false +} + +describe('resolveWorktreeStatus — interrupted (STA-5357)', () => { + it('reports interrupted rather than done for an interrupted agent', () => { + // Why `hasLiveDone` is also set: `interrupted` only ever coexists with `done` + // (agent-status-types.ts clamps it), so the done flag is always present too. + // Before the fix that made the card indistinguishable from a clean finish. + expect(resolveWorktreeStatus({ ...base, hasInterrupted: true, hasLiveDone: true })).toBe( + 'interrupted' + ) + }) + + it('keeps interrupted visible when a sibling agent merely finished', () => { + expect(resolveWorktreeStatus({ ...base, hasInterrupted: true, hasLiveDone: true })).not.toBe( + 'done' + ) + }) + + it('outranks working, so a cancelled turn is not hidden by a busy sibling', () => { + expect(resolveWorktreeStatus({ ...base, hasInterrupted: true, hasLiveWorking: true })).toBe( + 'interrupted' + ) + }) + + it('yields to permission — a prompt waiting on the user is more urgent', () => { + expect(resolveWorktreeStatus({ ...base, hasInterrupted: true, hasPermission: true })).toBe( + 'permission' + ) + }) + + it('outranks monitoring', () => { + expect(resolveWorktreeStatus({ ...base, hasInterrupted: true, hasLiveMonitoring: true })).toBe( + 'interrupted' + ) + }) + + it('leaves every other combination alone', () => { + expect(resolveWorktreeStatus({ ...base, hasLiveDone: true })).toBe('done') + expect(resolveWorktreeStatus({ ...base, hasLiveWorking: true })).toBe('working') + expect(resolveWorktreeStatus({ ...base, hasLiveMonitoring: true })).toBe('monitoring') + expect(resolveWorktreeStatus({ ...base, hasPermission: true })).toBe('permission') + }) +}) diff --git a/src/renderer/src/lib/worktree-status.ts b/src/renderer/src/lib/worktree-status.ts index 038e9f8ef11..d9092e78950 100644 --- a/src/renderer/src/lib/worktree-status.ts +++ b/src/renderer/src/lib/worktree-status.ts @@ -16,6 +16,7 @@ export type WorktreeStatus = | 'working' | 'monitoring' | 'permission' + | 'interrupted' | 'done' | 'inactive' @@ -31,6 +32,7 @@ const STATUS_LABELS: Record = { working: 'Working', monitoring: 'Monitoring background tasks', permission: 'Needs permission', + interrupted: 'Interrupted', done: 'Done', inactive: 'Inactive' } @@ -143,6 +145,7 @@ export function resolveWorktreeStatus(args: { hasPermission: boolean hasLiveWorking: boolean hasLiveMonitoring?: boolean + hasInterrupted?: boolean hasLiveDone: boolean hasRetainedDone: boolean }): WorktreeStatus { @@ -164,6 +167,13 @@ export function resolveWorktreeStatus(args: { if (heuristic === 'permission') { return 'permission' } + // Why above working (STA-5357): `interrupted` only ever rides on `done`, so before this it set + // `hasLiveDone` and the card rendered a cancelled turn as a clean finish — and a busy sibling could + // hide it entirely. It sits below permission because a prompt waiting on the user is the more urgent + // ask, and matches SUMMARY_STATE_ORDER so the card and the expanded agent list agree. + if (args.hasInterrupted) { + return 'interrupted' + } // Why: restored cards get the hook snapshot before panes mount; trust the explicit working row so they stay yellow on restart. if (args.hasLiveWorking || heuristic === 'working') { return 'working'