mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(sidebar): stop reporting an interrupted agent as done (STA-5357)
An interrupted turn rendered on the worktree card and the tab glyph as `done` —
visually identical to a clean completion. A cancelled or dead turn read as
finished work, and with several agents it contributed no signal at all, so a
sibling that merely finished could hide it entirely.
`interrupted` is a flag clamped onto `done` at parse time, never its own state,
so the activity summary never even received it: its Pick was
{state, workingMode}, and the `done` branch swallowed it into hasLiveDone.
Adds the flag end to end — summary, both activity hooks, the resolver, the card
glyph and the tab badge — and slots it below permission and above working, which
is where SUMMARY_STATE_ORDER already ranked it for the text summary. The card and
the expanded agent list now agree.
The tab-bar test that asserted the old behavior is flipped rather than deleted:
it explicitly pinned `done` to mirror the card, and that premise is gone.
This commit is contained in:
@@ -53,6 +53,21 @@ const StatusIndicator = React.memo(function StatusIndicator({
|
||||
)
|
||||
}
|
||||
|
||||
if (status === 'interrupted') {
|
||||
return (
|
||||
<span
|
||||
className={cn('inline-flex h-3 w-3 shrink-0 items-center justify-center', className)}
|
||||
title={resolvedTitle}
|
||||
{...rest}
|
||||
>
|
||||
{/* 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). */}
|
||||
<span className="block size-1.5 rounded-full bg-red-500" />
|
||||
</span>
|
||||
)
|
||||
}
|
||||
|
||||
if (status === 'permission') {
|
||||
return (
|
||||
<span
|
||||
|
||||
@@ -26,6 +26,7 @@ export function useWorktreeActivityStatus(worktreeId: string): WorktreeStatus {
|
||||
hasPermission,
|
||||
hasLiveWorking,
|
||||
hasLiveMonitoring,
|
||||
hasInterrupted,
|
||||
hasLiveDone,
|
||||
hasRetainedDone,
|
||||
agentStatusPaneIdsByTabId
|
||||
@@ -46,6 +47,7 @@ export function useWorktreeActivityStatus(worktreeId: string): WorktreeStatus {
|
||||
hasPermission,
|
||||
hasLiveWorking,
|
||||
hasLiveMonitoring,
|
||||
hasInterrupted,
|
||||
hasLiveDone,
|
||||
hasRetainedDone
|
||||
}),
|
||||
@@ -59,6 +61,7 @@ export function useWorktreeActivityStatus(worktreeId: string): WorktreeStatus {
|
||||
hasPermission,
|
||||
hasLiveWorking,
|
||||
hasLiveMonitoring,
|
||||
hasInterrupted,
|
||||
hasLiveDone,
|
||||
hasRetainedDone
|
||||
]
|
||||
|
||||
@@ -34,6 +34,7 @@ export function selectWorktreeActivityStatuses(
|
||||
hasPermission,
|
||||
hasLiveWorking,
|
||||
hasLiveMonitoring,
|
||||
hasInterrupted,
|
||||
hasLiveDone,
|
||||
hasRetainedDone,
|
||||
agentStatusPaneIdsByTabId
|
||||
@@ -50,6 +51,7 @@ export function selectWorktreeActivityStatuses(
|
||||
hasPermission,
|
||||
hasLiveWorking,
|
||||
hasLiveMonitoring,
|
||||
hasInterrupted,
|
||||
hasLiveDone,
|
||||
hasRetainedDone
|
||||
})
|
||||
|
||||
@@ -16,6 +16,9 @@ export type WorktreeAgentActivitySummary = {
|
||||
hasPermission: boolean
|
||||
hasLiveWorking: boolean
|
||||
hasLiveMonitoring: boolean
|
||||
/** A fresh hook entry carrying `interrupted`. Separate from `hasLiveDone` because the flag only ever
|
||||
* rides on `done` — folding it in reports a cancelled turn as a clean finish (STA-5357). */
|
||||
hasInterrupted: boolean
|
||||
hasLiveDone: boolean
|
||||
hasRetainedDone: boolean
|
||||
agentStatusPaneIdsByTabId: Record<string, ReadonlySet<string>>
|
||||
@@ -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<AgentStatusEntry, 'state' | 'workingMode'>
|
||||
entry: Pick<AgentStatusEntry, 'state' | 'workingMode' | 'interrupted'>
|
||||
): 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
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -20,6 +20,7 @@ type TerminalTabActivityFlags = {
|
||||
hasPermission: boolean
|
||||
hasLiveWorking: boolean
|
||||
hasLiveMonitoring: boolean
|
||||
hasInterrupted: boolean
|
||||
hasLiveDone: boolean
|
||||
paneIds: Set<string>
|
||||
}
|
||||
@@ -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':
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -16,6 +16,7 @@ export type WorktreeStatus =
|
||||
| 'working'
|
||||
| 'monitoring'
|
||||
| 'permission'
|
||||
| 'interrupted'
|
||||
| 'done'
|
||||
| 'inactive'
|
||||
|
||||
@@ -31,6 +32,7 @@ const STATUS_LABELS: Record<WorktreeStatus, string> = {
|
||||
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'
|
||||
|
||||
Reference in New Issue
Block a user