diff --git a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx index d9d5c0a18..bac20923f 100644 --- a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx +++ b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx @@ -156,6 +156,73 @@ describe('', () => { expect(frame).toContain('5s'); }); + it('renders elapsed + token count for completed agents with stats', () => { + // Locks in the cost-visibility win the panel is partly motivated + // by — completed entries should surface `▶ Ns · Nk tokens`. Using + // a completed entry (rather than running) so the assertion is + // stable against the running-tally heuristic. + const { lastFrame } = renderPanel({ + entries: [ + agentEntry({ + agentId: 'done-1', + subagentType: 'researcher', + description: 'researcher: investigate', + status: 'completed', + startTime: -12_000, + endTime: 0, + stats: { totalTokens: 2400, toolUses: 5, durationMs: 12_000 }, + }), + ], + }); + const frame = lastFrame() ?? ''; + expect(frame).toContain('12s'); + expect(frame).toContain('2.4k tokens'); + }); + + it.each([ + ['paused', '⏸'], + ['failed', '✖'], + ['cancelled', '✖'], + ] as const)('renders the %s status with the %s glyph', (status, glyph) => { + // Status routing is otherwise uncovered for paused / failed / + // cancelled — a future regression that flattened the switch + // would slip past the existing running / completed cases. + const { lastFrame } = renderPanel({ + entries: [ + agentEntry({ + agentId: `${status}-1`, + subagentType: 'researcher', + description: 'researcher: status routing fixture', + status, + // paused entries don't carry an endTime; failed / cancelled do. + endTime: status === 'paused' ? undefined : 0, + }), + ], + }); + expect(lastFrame() ?? '').toContain(glyph); + }); + + it('strips the subagentType: prefix from the description case-insensitively', () => { + // `descriptionWithoutPrefix` lowercases both sides — the existing + // tests only feed lowercase prefixes, so a future revert to + // strict `startsWith` would silently re-introduce + // `Researcher: Researcher: …` double-prefix on capitalised inputs. + const { lastFrame } = renderPanel({ + entries: [ + agentEntry({ + agentId: 'cap-1', + subagentType: 'researcher', + description: 'Researcher: cap-mismatch description', + }), + ], + }); + const frame = lastFrame() ?? ''; + // The prefix MUST have been stripped — the descriptive tail + // should appear exactly once, with no leading "Researcher: ". + expect(frame).toContain('cap-mismatch description'); + expect(frame).not.toContain('Researcher: cap-mismatch'); + }); + it('does NOT surface a flavor marker on foreground agents', () => { // Foreground vs background distinction stays with BackgroundTasksDialog // (where cancel semantics differ); the panel reads as a glance roster @@ -275,23 +342,53 @@ describe('', () => { expect(lastFrame() ?? '').toBe(''); }); - it('drops snapshot rows the live registry no longer knows about', () => { - // Foreground subagents unregister silently after the status-change - // callback fires (`unregisterForeground` deletes from the registry - // without emitting another transition). The snapshot still lists the - // entry as `running`, so a naive `live ?? snap` fallback would leave - // a ghost row that never clears. The panel must trust the registry - // and drop the row when `registry.get()` returns undefined. + it('reconciles snapshots the live registry no longer knows about as just-finished', () => { + // `unregisterForeground` calls `emitStatusChange(entry)` BEFORE + // it deletes the entry, so a snapshot taken on that callback + // captures the agent as "still running" while the very next + // render's `registry.get()` returns undefined. Naively falling + // back to the snap leaves a ghost-running row that never clears; + // dropping the row outright makes the agent disappear instantly + // and the user loses the "what just finished?" beat. Synthesize + // a terminal version so the 8s visibility window gives feedback + // and then evicts the row cleanly. const ghost = agentEntry({ agentId: 'ghost-1', subagentType: 'editor', description: 'editor: long-gone foreground task', status: 'running', }); - // Stub registry knows nothing about ghost-1 (simulates the - // post-unregister state). const { config } = makeRegistryConfig([]); const { lastFrame } = renderPanel({ entries: [ghost], config }); + // First frame: synthesized completion, row still on screen. + let frame = lastFrame() ?? ''; + expect(frame).toContain('editor'); + expect(frame).toContain('long-gone foreground task'); + // The synthesis sets status='completed', so the running tally + // should be 0/1 and the bullet should switch to the success ✔. + expect(frame).toContain('(0/1)'); + expect(frame).toContain('✔'); + // After the visibility window the row evicts and the panel hides. + act(() => { + vi.advanceTimersByTime(9000); + }); + frame = lastFrame() ?? ''; + expect(frame).toBe(''); + }); + + it('still drops rows where snapshot is already terminal AND registry is empty', () => { + // No useful state to keep showing — the snap is already terminal + // (so the user already saw the result) and the registry forgot + // about it (so we can't update activity / stats). Drop. + const stale = agentEntry({ + agentId: 'stale-1', + subagentType: 'researcher', + description: 'researcher: stale terminal entry', + status: 'completed', + endTime: 0, + }); + const { config } = makeRegistryConfig([]); + const { lastFrame } = renderPanel({ entries: [stale], config }); expect(lastFrame() ?? '').toBe(''); }); diff --git a/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx b/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx index 31ed77b56..611cb6408 100644 --- a/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx +++ b/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx @@ -27,8 +27,9 @@ */ import type React from 'react'; -import { useContext, useEffect, useMemo, useState } from 'react'; +import { useContext, useEffect, useMemo, useRef, useState } from 'react'; import { Box, Text } from 'ink'; +import { DEFAULT_BUILTIN_SUBAGENT_TYPE as CORE_DEFAULT_SUBAGENT_TYPE } from '@qwen-code/qwen-code-core'; import { useBackgroundTaskViewState } from '../../contexts/BackgroundTaskViewContext.js'; import { ConfigContext } from '../../contexts/ConfigContext.js'; import { theme } from '../../semantic-colors.js'; @@ -61,12 +62,13 @@ const DEFAULT_MAX_ROWS = 5; // because the panel is denser and we have the dialog as the long-term // review surface. const TERMINAL_VISIBLE_MS = 8000; -// `general-purpose` is the default builtin subagent; printing the type -// every row when it's the default just clutters the line — the -// description carries all the meaningful identity. Specialized -// subagents (named in `subagents/builtin-agents.ts` or user-authored) -// still get their type rendered as a bold anchor. -const DEFAULT_SUBAGENT_TYPE = 'general-purpose'; +// Re-export under a panel-local alias so the source of truth stays +// in `subagents/builtin-agents.ts` (a backend rename of the default +// type would otherwise silently re-introduce the redundant +// `general-purpose:` prefix on every row). Specialized subagents +// (other builtins or user-authored types) still get their type +// rendered as a bold anchor. +const DEFAULT_SUBAGENT_TYPE = CORE_DEFAULT_SUBAGENT_TYPE; type LivePanelEntry = AgentDialogEntry & { /** True when the row is past its terminal-visibility window. */ @@ -191,37 +193,84 @@ export const LiveAgentPanel: React.FC = ({ // BackgroundTasksDialog's detail body, which re-reads the registry // on its own activity tick. // - // When `registry.get()` returns undefined the entry is gone (the - // canonical case is a foreground subagent that unregistered after - // its statusChange fired but before the next snapshot refresh — - // `unregisterForeground` deletes silently). Drop the row instead - // of falling back to `snap`: the snapshot still says `running`, - // and that ghost would never clear because the registry has no - // record left to flip it to terminal. + // Three reconciliation paths between the snapshot and the registry: + // 1. Both agree → use live (newest `recentActivities`). + // 2. Snap says still-live (running / paused) but registry forgot + // → most commonly a foreground subagent that finished: + // `unregisterForeground` fires `emitStatusChange(entry)` BEFORE + // it deletes the entry, so the snapshot captures the old + // "still running" state and the next render's `registry.get` + // returns undefined. Synthesize a terminal version with + // `endTime = now` so the 8s visibility window gives the user + // a "the agent finished" beat instead of either a ghost-running + // row that never clears OR an instant disappearance the moment + // the tool returns. + // 3. Snap is already terminal but registry forgot → nothing useful + // to keep showing; drop. // // When `config` itself is undefined (test fixtures that render // without ConfigContext) the panel degrades to snapshot-only — - // there's no live source of truth to drop against, so the - // snapshot is the best we have. + // there's no live source of truth to reconcile against. // // NOTE: this useMemo MUST come before the `if (dialogOpen) return null` // early-return below — React's rules of hooks require hook calls in // identical order each render, so a conditional early-return that // skips a subsequent hook is a violation. + // First-seen-missing timestamps for synthesized terminal entries. + // We need this to survive across useMemo recomputes — without it, + // each tick would re-synthesize the entry with a fresh `now` as + // `endTime`, the visibility-window check (`now - endTime > 8000`) + // would always evaluate to 0, and the row would never expire. The + // ref outlives both the snapshot and the tick state. + const missingSinceRef = useRef>(new Map()); + const liveAgentSnapshots: AgentDialogEntry[] = useMemo(() => { const snapshots = entries.filter(isAgentEntry); if (!config) return snapshots; const registry = config.getBackgroundTaskRegistry(); - return snapshots + // `now` participates in the dependency array so the memo recomputes + // each tick and picks up `recentActivities` the registry mutated in + // place via appendActivity. Reading it here makes the dependency + // semantically honest — without this read a future "remove dead + // dep" cleanup would silently freeze the panel on the first + // tool-call after a snapshot refresh. + const reconcileAt = now; + const seenIds = new Set(); + const next = snapshots .map((snap) => { + seenIds.add(snap.agentId); const live = registry.get(snap.agentId); - return live ? ({ ...live, kind: 'agent' as const } as const) : null; + if (live) { + // Recovered (or never went missing) — drop any stale + // missing-since record so a future re-disappearance + // gets a fresh timestamp. + missingSinceRef.current.delete(snap.agentId); + return { ...live, kind: 'agent' as const }; + } + if (snap.status === 'running' || snap.status === 'paused') { + // Pin the disappearance time on first observation so + // subsequent ticks don't keep resetting endTime to `now`. + let missingSince = missingSinceRef.current.get(snap.agentId); + if (missingSince === undefined) { + missingSince = reconcileAt; + missingSinceRef.current.set(snap.agentId, missingSince); + } + return { + ...snap, + status: 'completed' as const, + endTime: snap.endTime ?? missingSince, + }; + } + return null; }) .filter((e): e is AgentDialogEntry => e !== null); - // `now` is a deliberate dep so the memo recomputes each tick and - // captures the latest `recentActivities` mutated in place by the - // registry's appendActivity path. - // eslint-disable-next-line react-hooks/exhaustive-deps + // GC: drop missing-since records for agents that are no longer + // even in the snapshot (e.g. statusChange refreshed and the + // entry left useBackgroundTaskView's view entirely). + for (const id of missingSinceRef.current.keys()) { + if (!seenIds.has(id)) missingSinceRef.current.delete(id); + } + return next; }, [entries, config, now]); // Defense in depth: don't compete with the dialog. Under diff --git a/packages/core/src/subagents/builtin-agents.ts b/packages/core/src/subagents/builtin-agents.ts index 8ddd8f2c0..74102c80a 100644 --- a/packages/core/src/subagents/builtin-agents.ts +++ b/packages/core/src/subagents/builtin-agents.ts @@ -7,6 +7,15 @@ import { ToolDisplayNames, ToolNames } from '../tools/tool-names.js'; import type { SubagentConfig } from './types.js'; +/** + * Canonical name of the default builtin subagent. Exported so UI + * surfaces (e.g. `LiveAgentPanel`'s default-type elision) can compare + * against the same source of truth instead of redeclaring the literal + * — a rename here would otherwise silently break "skip the type + * prefix when it's the default" logic. + */ +export const DEFAULT_BUILTIN_SUBAGENT_TYPE = 'general-purpose'; + /** * Registry of built-in subagents that are always available to all users. * These agents are embedded in the codebase and cannot be modified or deleted. @@ -16,7 +25,7 @@ export class BuiltinAgentRegistry { Omit > = [ { - name: 'general-purpose', + name: DEFAULT_BUILTIN_SUBAGENT_TYPE, description: 'General-purpose agent for researching complex questions, searching for code, and executing multi-step tasks. When you are searching for a keyword or file and are not confident that you will find the right match in the first few tries use this agent to perform the search for you.', systemPrompt: `You are a general-purpose agent. Given the user's message, you should use the tools available to complete the task. Do what has been asked; nothing more, nothing less. When you complete the task, respond with a concise report covering what was done and any key findings — the caller will relay this to the user, so it only needs the essentials. diff --git a/packages/core/src/subagents/index.ts b/packages/core/src/subagents/index.ts index c05c38697..8194798db 100644 --- a/packages/core/src/subagents/index.ts +++ b/packages/core/src/subagents/index.ts @@ -26,7 +26,10 @@ export type { export { SubagentError } from './types.js'; // Built-in agents registry -export { BuiltinAgentRegistry } from './builtin-agents.js'; +export { + BuiltinAgentRegistry, + DEFAULT_BUILTIN_SUBAGENT_TYPE, +} from './builtin-agents.js'; // Validation system export { SubagentValidator } from './validation.js';