From 55a69eb45075f1d04353edb4dcdc15ca395e5350 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:57:55 -0700 Subject: [PATCH] fix(sidebar): give a subagent row its own activity clock instead of its parent's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A subagent child row borrowed its parent's `updatedAt` as its staleness clock and set no `evidenceObservedAt`, so the sidebar's "No update in N" label reported the PARENT's silence as the child's. With one child that reads correctly by coincidence — the child's activity is what advanced the parent's clock. With siblings it is wrong: a child quiet for ten minutes reads "No update in 0m" because a sibling just pinged. Reuse `evidenceObservedAt`, the field that already carries this meaning on `AgentStatusEntry` and whose reader already prefers it and already falls back to `updatedAt`. Add it as an optional field on the two child-bearing wire types and stamp it at the producer boundaries that already classify an event as the child's: - Claude hook roster: every child-attributed tool event and the lead's background-task fold. - Codex hook roster: every child-attributed lifecycle and tool event. The stamp takes an explicit observation argument, because two callers (the transcript scan and the restore seed) pass a SPAWN stamp as their creation clock and reusing it would backdate the child's recency to its own birth. - Structured background tasks: `task_started`, `task_updated`, the aggregate roster frame, and — the load-bearing one — `task_progress`. That branch previously dropped a frame entirely unless the task was backgrounded AND the frame carried token usage, discarding the only per-child liveness signal a backgrounded child emits. A usage-free progress frame now refreshes the clock, which changes publish FREQUENCY on that shared path. `startedAt` is untouched everywhere. It is the spawn stamp two roster sorts, `compareWorktreeAgentRows`, cap eviction, and the equality/shed digest all depend on, so this adds a second field rather than moving that one. The time column is untouched too: it is documented bimodal (finish-time for a done row, start-time for an active one), so "14m" on a working child is a correct elapsed duration. Absent means the host never reported it, never "never active" — deliberately NOT normalized to 0 the way `startedAt` is, which would render as decades of silence. The reader keeps the parent `updatedAt` borrow as the documented old-host fallback. The shed-roster digest deliberately excludes the new field: it moves on every child tool event, so folding it into an IDENTITY digest would make a shed roster almost never match its cached twin, and an unrestored roster blanks live child rows and unblocks hibernation. Moves the tracked-row merge and the progress-frame fold next to the row type they belong to, which also keeps the tracker inside its line budget. --- .../claude-background-task-tracker.test.ts | 103 +++++++++++++++--- .../claude/claude-background-task-tracker.ts | 36 +++--- .../claude/claude-settled-background-tasks.ts | 50 ++++++++- .../claude-structured-session-close.test.ts | 16 ++- .../StructuredAgentSessionStatusBridge.tsx | 5 + .../sidebar/useWorktreeAgentRows.test.ts | 22 ++++ .../worktree-subagent-child-rows.test.ts | 66 ++++++++++- .../sidebar/worktree-subagent-child-rows.ts | 7 ++ ...ent-hook-listener-claude-subagents.test.ts | 2 + .../providers/claude-roster-state.ts | 5 + .../providers/codex-events.ts | 8 +- src/shared/agent-hook-relay.test.ts | 10 ++ src/shared/agent-hook-relay.ts | 5 + .../agent-session-background-task-wire.ts | 6 + src/shared/agent-status-types.ts | 11 ++ src/shared/claude-subagent-roster.test.ts | 31 ++++++ src/shared/claude-subagent-roster.ts | 13 +++ src/shared/codex-subagent-roster.test.ts | 43 ++++++++ src/shared/codex-subagent-roster.ts | 23 +++- 19 files changed, 418 insertions(+), 44 deletions(-) diff --git a/src/main/claude/claude-background-task-tracker.test.ts b/src/main/claude/claude-background-task-tracker.test.ts index 43912bfa560..efdb3309852 100644 --- a/src/main/claude/claude-background-task-tracker.test.ts +++ b/src/main/claude/claude-background-task-tracker.test.ts @@ -62,7 +62,9 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }] + tasks: [ + { id: 'task-1', kind: 'agent', state: 'working', startedAt: 100, evidenceObservedAt: 100 } + ] }) // The turn settling changes nothing the strip renders. @@ -89,7 +91,9 @@ describe('ClaudeBackgroundTaskTracker', () => { tracker.observe(system('task_updated', { task_id: 'task-1', patch: { is_backgrounded: true } })) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'command', state: 'working', startedAt: 100 }] + tasks: [ + { id: 'task-1', kind: 'command', state: 'working', startedAt: 100, evidenceObservedAt: 100 } + ] }) }) @@ -113,7 +117,8 @@ describe('ClaudeBackgroundTaskTracker', () => { kind: 'command', description: 'run the build', state: 'working', - startedAt: 100 + startedAt: 100, + evidenceObservedAt: 100 } ] }) @@ -177,8 +182,12 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-b', kind: 'agent', state: 'working', startedAt: 200 }], - settledTasks: [{ id: 'task-a', kind: 'agent', state: 'done', startedAt: 100 }] + tasks: [ + { id: 'task-b', kind: 'agent', state: 'working', startedAt: 200, evidenceObservedAt: 200 } + ], + settledTasks: [ + { id: 'task-a', kind: 'agent', state: 'done', startedAt: 100, evidenceObservedAt: 100 } + ] }) expect(tracker.stoppableTaskIds).toEqual(['task-b']) @@ -240,7 +249,8 @@ describe('ClaudeBackgroundTaskTracker', () => { kind: 'command', description: 'Sleep for 25 seconds', state: 'working', - startedAt: 200 + startedAt: 200, + evidenceObservedAt: 200 } ], settledTasks: [ @@ -250,6 +260,7 @@ describe('ClaudeBackgroundTaskTracker', () => { description: 'Sleep for 5 seconds', state: 'done', startedAt: 100, + evidenceObservedAt: 200, totalTokens: 18130 } ] @@ -292,10 +303,39 @@ describe('ClaudeBackgroundTaskTracker', () => { name: 'general-purpose', state: 'working', startedAt: 100, + evidenceObservedAt: 100, totalTokens: 14866 }) }) + it('times a backgrounded child from a usage-free progress frame', () => { + const tracker = trackerAt([100, 4000]) + tracker.observe( + system('task_started', { + task_id: 'agent-1', + task_type: 'local_agent', + subagent_type: 'general-purpose', + description: 'Sleep 6 seconds test', + is_backgrounded: true + }) + ) + + // A progress frame carrying no usage is still proof the child is alive now. + expect( + tracker.observe(system('task_progress', { task_id: 'agent-1', description: 'Running Bash' })) + ).toBe(true) + expect(tracker.state?.tasks?.[0]).toEqual({ + id: 'agent-1', + kind: 'agent', + // Progress descriptions are transient activity, never the task's name. + description: 'Sleep 6 seconds test', + name: 'general-purpose', + state: 'working', + startedAt: 100, + evidenceObservedAt: 4000 + }) + }) + it('maps terminal statuses onto settled states', () => { const tracker = trackerAt([100, 200]) tracker.observe( @@ -306,7 +346,7 @@ describe('ClaudeBackgroundTaskTracker', () => { ) tracker.observe(system('task_notification', { task_id: 'failed', status: 'failed' })) expect(tracker.state?.settledTasks).toEqual([ - { id: 'failed', kind: 'agent', state: 'blocked', startedAt: 200 } + { id: 'failed', kind: 'agent', state: 'blocked', startedAt: 200, evidenceObservedAt: 200 } ]) }) @@ -336,8 +376,22 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.state).toEqual({ state: 'monitoring', tasks: [ - { id: 'task-agent', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 }, - { id: 'task-bash', kind: 'command', description: 'bash', state: 'working', startedAt: 100 } + { + id: 'task-agent', + kind: 'agent', + description: 'agent', + state: 'working', + startedAt: 100, + evidenceObservedAt: 100 + }, + { + id: 'task-bash', + kind: 'command', + description: 'bash', + state: 'working', + startedAt: 100, + evidenceObservedAt: 100 + } ] }) @@ -364,9 +418,11 @@ describe('ClaudeBackgroundTaskTracker', () => { { task_id: 'task-2', task_type: 'local_bash' } ]) ) + // task-1's spawn stamp holds at 100 while the roster frame re-observes it at + // 200: the two clocks are separate, which is the whole point of the second one. expect(tracker.state?.tasks).toEqual([ - { id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }, - { id: 'task-2', kind: 'command', state: 'working', startedAt: 200 } + { id: 'task-1', kind: 'agent', state: 'working', startedAt: 100, evidenceObservedAt: 200 }, + { id: 'task-2', kind: 'command', state: 'working', startedAt: 200, evidenceObservedAt: 200 } ]) }) @@ -416,7 +472,14 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.state).toEqual({ state: 'monitoring', tasks: [ - { id: 'task-live', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 } + { + id: 'task-live', + kind: 'agent', + description: 'agent', + state: 'working', + startedAt: 100, + evidenceObservedAt: 100 + } ] }) }) @@ -487,7 +550,15 @@ describe('ClaudeBackgroundTaskTracker', () => { ) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'monitor', state: 'monitoring', startedAt: 100 }] + tasks: [ + { + id: 'task-live', + kind: 'monitor', + state: 'monitoring', + startedAt: 100, + evidenceObservedAt: 100 + } + ] }) expect( tracker.observe(system('task_updated', { task_id: 'task-live', patch: { status: 'killed' } })) @@ -506,7 +577,8 @@ describe('ClaudeBackgroundTaskTracker', () => { id: taskType, kind: taskType === 'local_workflow' ? 'workflow' : 'monitor', state: taskType === 'local_workflow' ? 'working' : 'monitoring', - startedAt: 100 + startedAt: 100, + evidenceObservedAt: 100 } ] }) @@ -572,7 +644,8 @@ describe('ClaudeBackgroundTaskTracker', () => { kind: 'command', description: 'command', state: 'working', - startedAt: 100 + startedAt: 100, + evidenceObservedAt: 100 } ] }) diff --git a/src/main/claude/claude-background-task-tracker.ts b/src/main/claude/claude-background-task-tracker.ts index 14f4271b7d8..6c432056ff5 100644 --- a/src/main/claude/claude-background-task-tracker.ts +++ b/src/main/claude/claude-background-task-tracker.ts @@ -15,7 +15,9 @@ import { } from './claude-background-task-frames' import { ClaudeSettledBackgroundTasks, + applyClaudeTaskProgressFrame, claudeBackgroundTaskDetail, + mergeTrackedClaudeBackgroundTask, type TrackedClaudeBackgroundTask } from './claude-settled-background-tasks' @@ -132,14 +134,11 @@ export class ClaudeBackgroundTaskTracker { return true } if (message.subtype === 'task_progress') { - // Progress `description` is the current activity ("Running "), not - // the task's name — only usage (and a missing identity) may update. const existing = this.tasks.get(id) - const totalTokens = taskUsageTotalTokens(message) - if (!existing?.backgrounded || totalTokens === undefined) { + if (!existing?.backgrounded) { return false } - this.tasks.set(id, { ...existing, totalTokens, name: existing.name ?? taskName(message) }) + this.tasks.set(id, applyClaudeTaskProgressFrame(existing, message, this.now())) return true } if (message.subtype === 'task_updated') { @@ -161,13 +160,17 @@ export class ClaudeBackgroundTaskTracker { if (this.aggregateRosterObserved && backgrounded && !this.tasks.has(id)) { return false } + // One clock read: the spawn stamp and the first observation are the same + // moment here, and they diverge from the next frame onwards. + const observedAt = this.now() this.upsert(id, { backgrounded, kind, description: taskDescription(message.description), name: taskName(message), state: liveClaudeTaskRunState(message.status) ?? undefined, - startedAt: this.now() + startedAt: observedAt, + evidenceObservedAt: observedAt }) return true } @@ -195,13 +198,15 @@ export class ClaudeBackgroundTaskTracker { liveState !== null || (patchKind !== undefined && patchKind !== 'unknown') if (hasContent && (!this.aggregateRosterObserved || existing)) { + const observedAt = this.now() this.upsert(id, { backgrounded: patch.is_backgrounded === true || existing?.backgrounded === true, kind: patchKind ?? existing?.kind ?? 'unknown', description: taskDescription(patch.description), name: taskName(patch), state: liveState ?? undefined, - startedAt: this.now() + startedAt: observedAt, + evidenceObservedAt: observedAt }) return true } @@ -213,6 +218,8 @@ export class ClaudeBackgroundTaskTracker { return } const prior = new Map(this.tasks) + // The roster frame is one observation of every id it lists, so one clock read. + const observedAt = this.now() this.aggregateRosterObserved = true this.tasks.clear() const roster = new Map() @@ -242,7 +249,8 @@ export class ClaudeBackgroundTaskTracker { description: taskDescription(task.description) ?? existing?.description, name: taskName(task) ?? existing?.name, state: liveClaudeTaskRunState(task.status) ?? existing?.state, - startedAt: existing?.startedAt ?? this.now(), + startedAt: existing?.startedAt ?? observedAt, + evidenceObservedAt: observedAt, totalTokens: existing?.totalTokens }) } @@ -292,17 +300,7 @@ export class ClaudeBackgroundTaskTracker { const existing = this.tasks.get(id) ?? this.retention.resume(id) this.terminalTaskIds.delete(id) if (existing) { - this.tasks.set(id, { - backgrounded: existing.backgrounded || task.backgrounded, - // A settled foreground task is not revived by a late edge frame. - liveInTurn: existing.liveInTurn, - kind: task.kind !== 'unknown' ? task.kind : existing.kind, - description: task.description ?? existing.description, - name: task.name ?? existing.name, - state: task.state ?? existing.state, - startedAt: existing.startedAt, - totalTokens: existing.totalTokens - }) + this.tasks.set(id, mergeTrackedClaudeBackgroundTask(existing, task)) return } this.tasks.set(id, { ...task, liveInTurn: true }) diff --git a/src/main/claude/claude-settled-background-tasks.ts b/src/main/claude/claude-settled-background-tasks.ts index 73064becec4..9314d41a616 100644 --- a/src/main/claude/claude-settled-background-tasks.ts +++ b/src/main/claude/claude-settled-background-tasks.ts @@ -1,4 +1,6 @@ -// Retention state for background tasks that have reached a terminal edge. +// The tracked background-task row: its type, how a later frame folds into one, +// how it projects onto the wire, and the retention state for rows that have +// reached a terminal edge. // // The real producer settles a task in two steps inside one tick: // `background_tasks_changed` arrives FIRST with the task already absent, then @@ -12,6 +14,7 @@ import type { AgentSessionBackgroundTask, AgentSessionBackgroundTaskRunState } from '../../shared/agent-session-wire' +import { taskName, taskUsageTotalTokens } from './claude-background-task-frames' const MAX_RETAINED_TASKS = 256 @@ -28,9 +31,51 @@ export type TrackedClaudeBackgroundTask = { /** First-observed epoch ms; preserved across updates and roster replacement * so clients can render elapsed and keep a stable first-seen sort. */ startedAt: number + /** Last epoch ms this task's OWN activity was observed. Moves on every frame + * that proves it alive, which is exactly what `startedAt` must not do. */ + evidenceObservedAt?: number totalTokens?: number } +/** Fold a later lifecycle frame into a tracked row. The first-observed stamp + * never moves; `evidenceObservedAt` moves whenever the frame carried one. That + * divergence is the whole reason the two clocks are separate fields. */ +export function mergeTrackedClaudeBackgroundTask( + existing: TrackedClaudeBackgroundTask, + next: Omit +): TrackedClaudeBackgroundTask { + return { + backgrounded: existing.backgrounded || next.backgrounded, + // A settled foreground task is not revived by a late edge frame. + liveInTurn: existing.liveInTurn, + kind: next.kind !== 'unknown' ? next.kind : existing.kind, + description: next.description ?? existing.description, + name: next.name ?? existing.name, + state: next.state ?? existing.state, + startedAt: existing.startedAt, + evidenceObservedAt: next.evidenceObservedAt ?? existing.evidenceObservedAt, + totalTokens: existing.totalTokens + } +} + +/** Fold a `task_progress` frame into a live row. The frame's ARRIVAL is the + * evidence — it proves the task is alive even carrying no usage, and it is the + * only per-task liveness signal a backgrounded task emits. Its `description` is + * the current activity ("Running ") and never becomes the task's name. */ +export function applyClaudeTaskProgressFrame( + existing: TrackedClaudeBackgroundTask, + message: Record, + observedAt: number +): TrackedClaudeBackgroundTask { + const totalTokens = taskUsageTotalTokens(message) + return { + ...existing, + ...(totalTokens !== undefined ? { totalTokens } : {}), + name: existing.name ?? taskName(message), + evidenceObservedAt: observedAt + } +} + export function claudeBackgroundTaskDetail( id: string, task: TrackedClaudeBackgroundTask @@ -42,6 +87,9 @@ export function claudeBackgroundTaskDetail( ...(task.name ? { name: task.name } : {}), state: task.state ?? (task.kind === 'monitor' ? 'monitoring' : 'working'), startedAt: task.startedAt, + ...(task.evidenceObservedAt !== undefined + ? { evidenceObservedAt: task.evidenceObservedAt } + : {}), ...(task.totalTokens !== undefined ? { totalTokens: task.totalTokens } : {}), // Only a backgrounded row has a stop the host can target; absent means yes. ...(task.backgrounded ? {} : { stoppable: false }) diff --git a/src/main/claude/claude-structured-session-close.test.ts b/src/main/claude/claude-structured-session-close.test.ts index 46bda78cacd..5da30dcaa8c 100644 --- a/src/main/claude/claude-structured-session-close.test.ts +++ b/src/main/claude/claude-structured-session-close.test.ts @@ -90,7 +90,13 @@ describe('Claude published session close lifecycle', () => { { state: 'monitoring', tasks: [ - { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + { + id: 'background-1', + kind: 'agent', + state: 'working', + startedAt: expect.any(Number), + evidenceObservedAt: expect.any(Number) + } ], supportsTaskStop: true } @@ -111,7 +117,13 @@ describe('Claude published session close lifecycle', () => { { state: 'monitoring', tasks: [ - { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + { + id: 'background-1', + kind: 'agent', + state: 'working', + startedAt: expect.any(Number), + evidenceObservedAt: expect.any(Number) + } ], supportsTaskStop: true }, diff --git a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx index 1017c5e87b6..951670228a8 100644 --- a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx +++ b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx @@ -116,6 +116,11 @@ function subagentSnapshotsFromTasks( id, state: subagentStateFromTask(task), startedAt: task.startedAt ?? 0, + // Absent stays absent: an old host reported no per-task clock, which is not + // the same as a task that has never been active. + ...(task.evidenceObservedAt !== undefined + ? { evidenceObservedAt: task.evidenceObservedAt } + : {}), ...(task.name ? { agentType: task.name } : {}), ...(task.description ? { description: task.description } : {}) }) diff --git a/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts b/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts index b02afa1fbfc..ad71376b31d 100644 --- a/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts +++ b/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts @@ -768,6 +768,28 @@ describe('applyAgentRowLineage', () => { expect(ordered[2].lineage).toMatchObject({ depth: 1, isLastSibling: true }) }) + it('gives sibling child rows their own recency, not one borrowed parent clock', () => { + const entry = makeEntry(PANE_KEY_1, 1000, { + state: 'working', + updatedAt: 900_000, + subagents: [ + { id: 'busy', state: 'working', startedAt: 1500, evidenceObservedAt: 899_000 }, + { id: 'quiet', state: 'working', startedAt: 1600, evidenceObservedAt: 300_000 } + ] + }) + const rows = buildWorktreeAgentRows({ + tabs: [makeTab('tab-1')], + entries: [entry], + retained: [], + now: 900_000 + }) + + const children = rows.filter((row) => row.rowSource === 'subagent') + expect(children.map((row) => row.entry.evidenceObservedAt)).toEqual([899_000, 300_000]) + // Spawn stamps are untouched, so the sibling order the user sees is unchanged. + expect(children.map((row) => row.startedAt)).toEqual([1500, 1600]) + }) + it('marks working subagent child rows unverifiable when the parent status is stale', () => { const entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', diff --git a/src/renderer/src/components/sidebar/worktree-subagent-child-rows.test.ts b/src/renderer/src/components/sidebar/worktree-subagent-child-rows.test.ts index c535e3e80ee..4966b9250bf 100644 --- a/src/renderer/src/components/sidebar/worktree-subagent-child-rows.test.ts +++ b/src/renderer/src/components/sidebar/worktree-subagent-child-rows.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest' -import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { AgentStatusEntry, AgentSubagentSnapshot } from '../../../../shared/agent-status-types' +import { agentStatusEvidenceObservedAt } from '../../../../shared/agent-status-freshness' +import { agentNoUpdateLabel } from '@/lib/agent-row-decay-state' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { buildSubagentChildRows } from './worktree-subagent-child-rows' @@ -53,3 +55,65 @@ describe('shared CLI and structured child freshness', () => { } ) }) + +/** Ten minutes of parent silence, so a borrowed clock reads visibly differently + * from a child's own. */ +const PARENT_UPDATED_AT = 1_000_000 +const SPAWNED_AT = 20 + +function parentWithChildren(subagents: AgentSubagentSnapshot[]): AgentStatusEntry { + return { + paneKey: 'parent-pane', + tabId: tab.id, + worktreeId: tab.worktreeId, + state: 'working', + prompt: 'parent prompt', + updatedAt: PARENT_UPDATED_AT, + stateStartedAt: 10, + stateHistory: [], + subagents + } +} + +describe('child rows carry their own activity evidence', () => { + it('times a child from its own observation, not the parent delivery clock', () => { + const parentEntry = parentWithChildren([ + { id: 'child', state: 'working', startedAt: SPAWNED_AT, evidenceObservedAt: 400_000 } + ]) + + const row = buildSubagentChildRows({ parentEntry, tab, parentIsFresh: true })[0] + + expect(row.entry.evidenceObservedAt).toBe(400_000) + expect(agentStatusEvidenceObservedAt(row.entry)).toBe(400_000) + // The spawn stamp still drives the time column and the sibling sort. + expect(row.startedAt).toBe(SPAWNED_AT) + expect(row.entry.stateStartedAt).toBe(SPAWNED_AT) + }) + + it('falls back to the parent clock when a host never reported the child one', () => { + const parentEntry = parentWithChildren([ + { id: 'child', state: 'working', startedAt: SPAWNED_AT } + ]) + + const row = buildSubagentChildRows({ parentEntry, tab, parentIsFresh: true })[0] + + // Absent must read as "this host never said", never as "never active". + expect(row.entry.evidenceObservedAt).toBeUndefined() + expect(agentStatusEvidenceObservedAt(row.entry)).toBe(PARENT_UPDATED_AT) + }) + + it('gives two siblings of one parent different recency, not one shared clock', () => { + const parentEntry = parentWithChildren([ + { id: 'busy', state: 'working', startedAt: SPAWNED_AT, evidenceObservedAt: 999_000 }, + { id: 'quiet', state: 'working', startedAt: SPAWNED_AT, evidenceObservedAt: 400_000 } + ]) + + const [busy, quiet] = buildSubagentChildRows({ parentEntry, tab, parentIsFresh: true }) + + expect(agentStatusEvidenceObservedAt(busy.entry)).not.toBe( + agentStatusEvidenceObservedAt(quiet.entry) + ) + expect(agentNoUpdateLabel(busy.entry, PARENT_UPDATED_AT)).toBe('No update in 0m') + expect(agentNoUpdateLabel(quiet.entry, PARENT_UPDATED_AT)).toBe('No update in 10m') + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-subagent-child-rows.ts b/src/renderer/src/components/sidebar/worktree-subagent-child-rows.ts index e629a8e352b..233b4f1e506 100644 --- a/src/renderer/src/components/sidebar/worktree-subagent-child-rows.ts +++ b/src/renderer/src/components/sidebar/worktree-subagent-child-rows.ts @@ -43,7 +43,14 @@ export function buildSubagentChildRows(args: { const entry: AgentStatusEntry = { state: activeState ?? 'done', prompt: subagent.description ?? subagent.agentType ?? '', + // The parent's clock is the OLD-HOST FALLBACK, not this child's recency: a + // host that reports the child's own observation wins through + // `evidenceObservedAt` below, and readers already prefer it. Dropping the + // borrow would leave an old host reading "No update in ". updatedAt: args.parentEntry.updatedAt, + ...(subagent.evidenceObservedAt !== undefined + ? { evidenceObservedAt: subagent.evidenceObservedAt } + : {}), stateStartedAt: startedAt, agentType: subagent.agentType, model: subagent.model, diff --git a/src/shared/agent-hook-listener-claude-subagents.test.ts b/src/shared/agent-hook-listener-claude-subagents.test.ts index 00d95471d72..f02e0f1e091 100644 --- a/src/shared/agent-hook-listener-claude-subagents.test.ts +++ b/src/shared/agent-hook-listener-claude-subagents.test.ts @@ -145,6 +145,7 @@ describe('shared agent-hook-listener', () => { id: 'a1', state: 'working', startedAt: expect.any(Number), + evidenceObservedAt: expect.any(Number), agentType: 'general-purpose', description: 'Review loop' } @@ -175,6 +176,7 @@ describe('shared agent-hook-listener', () => { id: 'r1', state: 'working', startedAt: expect.any(Number), + evidenceObservedAt: expect.any(Number), agentType: 'code-reviewer', description: undefined } diff --git a/src/shared/agent-hook-listener/providers/claude-roster-state.ts b/src/shared/agent-hook-listener/providers/claude-roster-state.ts index c21166b5ce9..ca64e7fe51b 100644 --- a/src/shared/agent-hook-listener/providers/claude-roster-state.ts +++ b/src/shared/agent-hook-listener/providers/claude-roster-state.ts @@ -169,6 +169,11 @@ export function seedClaudeSubagentRosterFromSnapshots( roster.set(snapshot.id, { state: 'working', startedAt: snapshot.startedAt, + // Why: a restore observes nothing. Carry the persisted clock through so the + // row reads as last heard from then, not as freshly seen at restore. + ...(snapshot.evidenceObservedAt !== undefined + ? { evidenceObservedAt: snapshot.evidenceObservedAt } + : {}), agentType: snapshot.agentType, description: snapshot.description, // Why: the seed can be a phantom (child finished while Orca was down, SubagentStop lost); let a PRESENT background_tasks list omitting the id remove it, not gate the pane 'working' forever. diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index bcd0474b940..98b4f20c94b 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -83,6 +83,7 @@ export function normalizeCodexSubagentLifecycleEvent( } const roster = getOrCreateCodexSubagentRoster(state, paneKey) if (eventName === 'SubagentStart') { + const observedAt = Date.now() upsertCodexSubagent( roster, agentId, @@ -91,7 +92,8 @@ export function normalizeCodexSubagentLifecycleEvent( model: readString(hookPayload, 'model'), state: 'working' }, - Date.now() + observedAt, + observedAt ) } else { finishCodexSubagent(roster, agentId) @@ -131,6 +133,7 @@ export function normalizeCodexEvent( const agentId = readString(hookPayload, 'agent_id') if (agentId) { + const observedAt = Date.now() upsertCodexSubagent( getOrCreateCodexSubagentRoster(state, paneKey), agentId, @@ -139,7 +142,8 @@ export function normalizeCodexEvent( model: readString(hookPayload, 'model'), state: stateName === 'waiting' ? 'waiting' : 'working' }, - Date.now() + observedAt, + observedAt ) return buildCodexChildDrivenStatusPayload(state, eventName, paneKey, hookPayload) } diff --git a/src/shared/agent-hook-relay.test.ts b/src/shared/agent-hook-relay.test.ts index 81615eaffb7..1d0df53ae19 100644 --- a/src/shared/agent-hook-relay.test.ts +++ b/src/shared/agent-hook-relay.test.ts @@ -120,6 +120,16 @@ describe('restoreShedStatusFields', () => { expect(restored).toBe(shed) }) + it('restores across a child-clock advance: the digest is identity, not recency', () => { + // evidenceObservedAt moves on every child tool event. Folding it into the + // identity digest would make a shed roster essentially never match, and an + // absent roster blanks live child rows and unblocks hibernation. The cached + // clock is an older TRUE observation, so restoring it is the safe degrade. + const pinged = [{ ...roster[0], evidenceObservedAt: 900 }] + const restored = restoreShedStatusFields(shed, [createShedSubagentsField(pinged)], cached) + expect(restored.subagents).toEqual(roster) + }) + it('never restores interactivePrompt — a stale answerable card is worse than none', () => { const restored = restoreShedStatusFields(shed, ['interactivePrompt'], cached) expect(restored.interactivePrompt).toBeUndefined() diff --git a/src/shared/agent-hook-relay.ts b/src/shared/agent-hook-relay.ts index dadfa590a4e..45920a8f475 100644 --- a/src/shared/agent-hook-relay.ts +++ b/src/shared/agent-hook-relay.ts @@ -127,6 +127,11 @@ export const AGENT_HOOK_NOTIFICATION_METHOD = 'agent.hook' as const export const AGENT_HOOK_SHED_FIELDS_KEY = 'shedFields' as const const AGENT_HOOK_SHED_SUBAGENTS_DIGEST_PREFIX = 'subagents:sha256:' +/** Roster IDENTITY only. `evidenceObservedAt` is deliberately excluded: it moves on + * every child tool event, so folding it in would make a shed roster almost never + * match its cached twin, and an unrestored roster blanks live child rows and + * unblocks hibernation. The cached clock is an older TRUE observation, so + * restoring it is conservative — it can never fabricate freshness. */ function subagentRosterDigest(subagents: readonly AgentSubagentSnapshot[]): string { const stableRoster = subagents.map(({ id, state, startedAt, agentType, model, description }) => [ id, diff --git a/src/shared/agent-session-background-task-wire.ts b/src/shared/agent-session-background-task-wire.ts index c5e6893fb0b..11212206dba 100644 --- a/src/shared/agent-session-background-task-wire.ts +++ b/src/shared/agent-session-background-task-wire.ts @@ -25,6 +25,11 @@ export type AgentSessionBackgroundTask = { state?: AgentSessionBackgroundTaskRunState /** Host epoch ms when the task was first observed, so clients render elapsed. */ startedAt?: number + /** Host epoch ms the host last observed THIS task's own activity, so a client + * times its recency from the task rather than from the session that owns it. + * Absent means the host never reported it — never "never active" — and the + * client falls back to the clock it already used. */ + evidenceObservedAt?: number /** Cumulative provider-reported token usage, where the provider supplies it. */ totalTokens?: number /** Whether this row's own stop can act on it. Absent means yes: every host @@ -61,6 +66,7 @@ function backgroundTaskFieldsEqual( left.name === right.name && left.state === right.state && left.startedAt === right.startedAt && + left.evidenceObservedAt === right.evidenceObservedAt && left.totalTokens === right.totalTokens && left.stoppable === right.stoppable ) diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index 3a062f7069d..19d5e57546f 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -79,6 +79,12 @@ export type AgentSubagentSnapshot = { state: AgentSubagentState /** Timestamp (ms) when this subagent was first observed. */ startedAt: number + /** Timestamp (ms) the host last observed THIS child's own activity, so a row's + * recency is its own rather than its parent's. Absent means the host never + * reported it (an old host, or a provider whose children emit nothing) — never + * "this child was never active". Deliberately not coerced to 0 like `startedAt`: + * a 0 would render as decades of silence. Readers fall back to `updatedAt`. */ + evidenceObservedAt?: number } export type AgentStatusEntry = { @@ -294,6 +300,10 @@ function normalizeSubagentSnapshot(value: unknown): AgentSubagentSnapshot | null state: obj.state, startedAt: typeof obj.startedAt === 'number' && Number.isFinite(obj.startedAt) ? obj.startedAt : 0, + // Unlike startedAt, an unreadable value stays ABSENT rather than becoming 0. + ...(typeof obj.evidenceObservedAt === 'number' && Number.isFinite(obj.evidenceObservedAt) + ? { evidenceObservedAt: obj.evidenceObservedAt } + : {}), agentType: normalizeOptionalField(obj.agentType, AGENT_TYPE_MAX_LENGTH), model: normalizeOptionalField(obj.model, AGENT_MODEL_MAX_LENGTH), description: normalizeOptionalField(obj.description, AGENT_STATUS_TOOL_INPUT_MAX_LENGTH) @@ -336,6 +346,7 @@ export function agentSubagentsEqual( x.id !== y.id || x.state !== y.state || x.startedAt !== y.startedAt || + x.evidenceObservedAt !== y.evidenceObservedAt || x.agentType !== y.agentType || x.model !== y.model || x.description !== y.description diff --git a/src/shared/claude-subagent-roster.test.ts b/src/shared/claude-subagent-roster.test.ts index df327cde829..b53a5a83ea0 100644 --- a/src/shared/claude-subagent-roster.test.ts +++ b/src/shared/claude-subagent-roster.test.ts @@ -529,3 +529,34 @@ describe('restored-row liveness reap', () => { expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) }) }) + +describe('claude child activity evidence', () => { + it('advances a pinged child clock while its spawn stamp and the sibling sort hold', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'a2', { agentType: 'reviewer' }, 100) + upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'writer' }, 200) + expect(claudeRosterToSnapshots(roster)?.map((snapshot) => snapshot.evidenceObservedAt)).toEqual( + [100, 200] + ) + + // A tool event from ONE child: the quiet sibling must not inherit its recency. + upsertWorkingClaudeSubagent(roster, 'a2', {}, 5000) + + const after = claudeRosterToSnapshots(roster) + expect(after?.map((snapshot) => snapshot.id)).toEqual(['a2', 'a1']) + expect(after?.map((snapshot) => snapshot.startedAt)).toEqual([100, 200]) + expect(after?.map((snapshot) => snapshot.evidenceObservedAt)).toEqual([5000, 200]) + }) + + it('treats a lead inventory that still lists a lane as an observation of that lane', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'reviewer' }, 100) + + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'a1' })], 7000) + + expect(claudeRosterToSnapshots(roster)?.[0]).toMatchObject({ + startedAt: 100, + evidenceObservedAt: 7000 + }) + }) +}) diff --git a/src/shared/claude-subagent-roster.ts b/src/shared/claude-subagent-roster.ts index 7891d19e767..019bf2eacc3 100644 --- a/src/shared/claude-subagent-roster.ts +++ b/src/shared/claude-subagent-roster.ts @@ -22,6 +22,10 @@ export type TrackedClaudeSubagent = { agentType?: string description?: string startedAt: number + /** Last moment THIS child's own activity was observed. Separate from + * `startedAt`, which is the spawn stamp four consumers sort and evict by and + * which must never move. Absent until something observes the child. */ + evidenceObservedAt?: number /** 'idle' = teammate between mailbox turns: alive/resumable, row stays * visible but must not gate the pane 'working'. */ state: 'working' | 'idle' @@ -74,6 +78,8 @@ export function upsertWorkingClaudeSubagent( existing.state = 'working' existing.agentType = fields.agentType ?? existing.agentType existing.description = fields.description ?? existing.description + // Why: every caller passes a real observation moment; `startedAt` stays put. + existing.evidenceObservedAt = now // Why: live activity proves the lifecycle stream owns this id again; // background_tasks omission must stop reaping it (teammate-shaped ids // never appear there). The fold re-tags its own recreations after this. @@ -91,6 +97,7 @@ export function upsertWorkingClaudeSubagent( roster.set(id, { state: 'working', startedAt: now, + evidenceObservedAt: now, agentType: fields.agentType, description: fields.description }) @@ -183,6 +190,9 @@ export function foldClaudeBackgroundTasksIntoRoster( existing.state = 'working' existing.agentType = task.agentType ?? existing.agentType existing.description = task.description ?? existing.description + // Coarser than a child's own tool event, but still an assertion ABOUT the + // child (the provider lists this lane running now), not the parent's clock. + existing.evidenceObservedAt = now existing.listedAsSubagentTask = true // Why: a live inventory listed the id as running — the restored claim is // now confirmed by the current process, so liveness can't reap it. @@ -344,6 +354,9 @@ export function claudeRosterToSnapshots( id, state: tracked.state, startedAt: tracked.startedAt, + ...(tracked.evidenceObservedAt !== undefined + ? { evidenceObservedAt: tracked.evidenceObservedAt } + : {}), agentType: tracked.agentType, description: tracked.description }) diff --git a/src/shared/codex-subagent-roster.test.ts b/src/shared/codex-subagent-roster.test.ts index 0cbb44830c9..d0c0126477b 100644 --- a/src/shared/codex-subagent-roster.test.ts +++ b/src/shared/codex-subagent-roster.test.ts @@ -7,6 +7,7 @@ import { import { codexRosterToSnapshots, finishCodexSubagent, + seedCodexSubagentRoster, setCodexSubagentModel, upsertCodexSubagent, type CodexSubagentRoster @@ -113,3 +114,45 @@ describe('Codex subagent roster', () => { }) }) }) + +describe('Codex child activity evidence', () => { + it('advances a pinged child clock while its spawn stamp and the sibling sort hold', () => { + const roster: CodexSubagentRoster = new Map() + upsertCodexSubagent(roster, 'c1', { state: 'working' }, 100, 100) + upsertCodexSubagent(roster, 'c2', { state: 'working' }, 200, 200) + + // A tool event from ONE child: the quiet sibling must not inherit its recency. + upsertCodexSubagent(roster, 'c1', { state: 'working' }, 5000, 5000) + + const after = codexRosterToSnapshots(roster) + expect(after?.map((snapshot) => snapshot.id)).toEqual(['c1', 'c2']) + expect(after?.map((snapshot) => snapshot.startedAt)).toEqual([100, 200]) + expect(after?.map((snapshot) => snapshot.evidenceObservedAt)).toEqual([5000, 200]) + }) + + it('never backdates recency to a spawn stamp on a call that only carries one', () => { + const roster: CodexSubagentRoster = new Map() + // The transcript scan passes the child's SPAWN time as its creation clock. + upsertCodexSubagent(roster, 'c1', { state: 'working' }, 100) + expect(codexRosterToSnapshots(roster)?.[0].evidenceObservedAt).toBeUndefined() + + upsertCodexSubagent(roster, 'c1', { state: 'working' }, 100, 5000) + setCodexSubagentModel(roster, 'c1', 'gpt-5.4') + + // Model discovery is not lifecycle evidence, so it must not move the clock. + expect(codexRosterToSnapshots(roster)?.[0].evidenceObservedAt).toBe(5000) + }) + + it('restores a seeded child without inventing freshness it never observed', () => { + const roster: CodexSubagentRoster = new Map() + + seedCodexSubagentRoster(roster, [{ id: 'c1', state: 'working', startedAt: 100 }]) + expect(codexRosterToSnapshots(roster)?.[0].evidenceObservedAt).toBeUndefined() + + const carried: CodexSubagentRoster = new Map() + seedCodexSubagentRoster(carried, [ + { id: 'c2', state: 'working', startedAt: 100, evidenceObservedAt: 900 } + ]) + expect(codexRosterToSnapshots(carried)?.[0].evidenceObservedAt).toBe(900) + }) +}) diff --git a/src/shared/codex-subagent-roster.ts b/src/shared/codex-subagent-roster.ts index 5592ed8711f..b265cb6cc9e 100644 --- a/src/shared/codex-subagent-roster.ts +++ b/src/shared/codex-subagent-roster.ts @@ -17,6 +17,9 @@ type TrackedCodexSubagent = { model?: string state: 'working' | 'waiting' startedAt: number + /** Last moment THIS child's own activity was observed. Separate from + * `startedAt`, the spawn stamp the snapshot sort depends on. */ + evidenceObservedAt?: number } export function upsertCodexSubagent( @@ -28,7 +31,12 @@ export function upsertCodexSubagent( model?: string state: 'working' | 'waiting' }, - now: number + now: number, + /** When this child was observed. Deliberately separate from `now`: two callers + * pass a SPAWN stamp there (the transcript scan and the restore seed), and + * reusing it would backdate the child's recency to its own birth. Omitted = + * nothing was observed, so the clock is left exactly as it was. */ + observedAt?: number ): void { const normalizedId = id.trim() if (normalizedId.length === 0 || normalizedId.length > CODEX_SUBAGENT_ID_MAX_LENGTH) { @@ -43,6 +51,7 @@ export function upsertCodexSubagent( existing.description = description ?? existing.description existing.model = model ?? existing.model existing.state = fields.state + existing.evidenceObservedAt = observedAt ?? existing.evidenceObservedAt return } if (roster.size >= AGENT_STATUS_MAX_SUBAGENTS) { @@ -53,7 +62,8 @@ export function upsertCodexSubagent( description, model, state: fields.state, - startedAt: now + startedAt: now, + ...(observedAt !== undefined ? { evidenceObservedAt: observedAt } : {}) }) } @@ -100,7 +110,9 @@ export function seedCodexSubagentRoster( model: snapshot.model, state: snapshot.state }, - snapshot.startedAt + snapshot.startedAt, + // A restore observes nothing: carry the persisted clock, never invent one. + snapshot.evidenceObservedAt ) } } @@ -117,7 +129,10 @@ export function codexRosterToSnapshots( description: tracked.description, model: tracked.model, state: tracked.state, - startedAt: tracked.startedAt + startedAt: tracked.startedAt, + ...(tracked.evidenceObservedAt !== undefined + ? { evidenceObservedAt: tracked.evidenceObservedAt } + : {}) })) snapshots.sort((a, b) => a.startedAt - b.startedAt || a.id.localeCompare(b.id)) return snapshots