diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx index 8d2ce1c2f..2641b440a 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -16,11 +16,19 @@ import { useBackgroundTaskViewState, } from '../../contexts/BackgroundTaskViewContext.js'; import { ConfigContext } from '../../contexts/ConfigContext.js'; -import { useBackgroundTaskView } from '../../hooks/useBackgroundTaskView.js'; +import { + useBackgroundTaskView, + type DialogEntry, +} from '../../hooks/useBackgroundTaskView.js'; import { useKeypress } from '../../hooks/useKeypress.js'; vi.mock('../../hooks/useBackgroundTaskView.js', () => ({ useBackgroundTaskView: vi.fn(), + // Re-export the helper so Dialog renderers can still resolve it under the + // mocked module. Inline impl keeps the test independent of the hook + // module while preserving the discriminator-based id contract. + entryId: (entry: DialogEntry): string => + entry.kind === 'agent' ? entry.agentId : entry.shellId, })); vi.mock('../../hooks/useKeypress.js', () => ({ @@ -30,33 +38,34 @@ vi.mock('../../hooks/useKeypress.js', () => ({ const mockedUseBackgroundTaskView = vi.mocked(useBackgroundTaskView); const mockedUseKeypress = vi.mocked(useKeypress); -function entry(overrides: Partial): BackgroundTaskEntry { +function entry(overrides: Partial = {}): DialogEntry { return { + kind: 'agent', agentId: 'a', description: 'desc', status: 'running', startTime: 0, abortController: new AbortController(), ...overrides, - }; + } as DialogEntry; } interface ProbeHandle { actions: ReturnType; state: ReturnType; - setEntries: (next: readonly BackgroundTaskEntry[]) => void; + setEntries: (next: readonly DialogEntry[]) => void; } interface Harness { cancel: ReturnType; - setEntries: (next: readonly BackgroundTaskEntry[]) => void; + setEntries: (next: readonly DialogEntry[]) => void; pressKey: (key: { name?: string; sequence?: string }) => void; call: (fn: () => void) => void; lastFrame: () => string | undefined; probe: { current: ProbeHandle | null }; } -function setup(initial: readonly BackgroundTaskEntry[]): Harness { +function setup(initial: readonly DialogEntry[]): Harness { const handlers: Array<(key: { name?: string; sequence?: string }) => void> = []; mockedUseKeypress.mockImplementation((cb, opts) => { @@ -64,10 +73,20 @@ function setup(initial: readonly BackgroundTaskEntry[]): Harness { }); const cancel = vi.fn(); + // Stub registry that resolves `.get(agentId)` against the current entries + // snapshot — the dialog now re-reads agent entries via `.get()` to pick up + // live activity/stats mutations the snapshot misses. + let currentEntries: readonly DialogEntry[] = initial; const config = { getBackgroundTaskRegistry: () => ({ cancel, setActivityChangeCallback: vi.fn(), + get: (id: string) => { + const match = currentEntries.find( + (e) => e.kind === 'agent' && e.agentId === id, + ); + return match; + }, }), } as unknown as Config; @@ -94,7 +113,7 @@ function setup(initial: readonly BackgroundTaskEntry[]): Harness { function Probe({ entriesSetter, }: { - entriesSetter: (e: readonly BackgroundTaskEntry[]) => void; + entriesSetter: (e: readonly DialogEntry[]) => void; }) { handle.current = { actions: useBackgroundTaskViewActions(), @@ -110,6 +129,7 @@ function setup(initial: readonly BackgroundTaskEntry[]): Harness { cancel, setEntries(next) { handlers.length = 0; + currentEntries = next; act(() => handle.current!.setEntries(next)); }, pressKey(key) { diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 1f5f20067..5f6828269 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -28,6 +28,16 @@ import { ToolNames, type BackgroundTaskEntry, } from '@qwen-code/qwen-code-core'; +import { + type DialogEntry, + entryId, +} from '../../hooks/useBackgroundTaskView.js'; + +// `DialogEntry['status']` widens BackgroundTaskEntry['status'] with the +// shell status union, but they share the same four values +// (running / completed / failed / cancelled), so handlers keyed on the +// agent enum still cover every shell case. +type EntryStatus = DialogEntry['status']; // Tool-name → display-name lookup (`run_shell_command` → `Shell`). const TOOL_DISPLAY_BY_NAME: Record = Object.fromEntries( @@ -46,7 +56,7 @@ function formatActivityLabel(name: string, description: string | undefined) { } import { formatDuration, formatTokenCount } from '../../utils/formatters.js'; -const STATUS_VERBS: Record = { +const STATUS_VERBS: Record = { running: 'Running', completed: 'Completed', failed: 'Failed', @@ -60,7 +70,7 @@ interface StatusPresentation { } function terminalStatusPresentation( - status: BackgroundTaskEntry['status'], + status: EntryStatus, ): StatusPresentation | null { switch (status) { case 'completed': @@ -86,11 +96,18 @@ function terminalStatusPresentation( } } -function rowLabel(entry: BackgroundTaskEntry): string { - return buildBackgroundEntryLabel(entry, { includePrefix: false }); +function rowLabel(entry: DialogEntry): string { + if (entry.kind === 'agent') { + return buildBackgroundEntryLabel(entry, { includePrefix: false }); + } + // Shell row: `[shell] `. Prefix mirrors the dialog's "section" + // visual hint without needing per-kind section headers (which would + // complicate the windowing math). The command itself is plain text and + // already truncated by the row renderer's MaxSizedBox. + return `[shell] ${entry.command}`; } -function elapsedFor(entry: BackgroundTaskEntry): string { +function elapsedFor(entry: { startTime: number; endTime?: number }): string { const elapsedMs = Math.max( 0, (entry.endTime ?? Date.now()) - entry.startTime, @@ -126,18 +143,18 @@ function truncateToWidth(text: string, maxWidth: number): string { // ─── List mode ───────────────────────────────────────────── const ListBody: React.FC<{ - entries: readonly BackgroundTaskEntry[]; + entries: readonly DialogEntry[]; selectedIndex: number; maxRows: number; }> = ({ entries, selectedIndex, maxRows }) => { - // Keep the "Local agents (N)" section header rendered even when the list - // is empty, so the overlay doesn't collapse into a single line of - // empty-state text when the last agent finishes while the dialog is open. + // Keep the "Background tasks (N)" section header rendered even when the + // list is empty, so the overlay doesn't collapse into a single line of + // empty-state text when the last task finishes while the dialog is open. if (entries.length === 0) { return ( - Local agents + Background tasks (0) @@ -172,7 +189,7 @@ const ListBody: React.FC<{ return ( - Local agents + Background tasks ({entries.length}) @@ -193,7 +210,7 @@ const ListBody: React.FC<{ ? terminal.labelColor : theme.text.primary; return ( - + {isSelected ? '> ' : ' '} @@ -216,11 +233,22 @@ const ListBody: React.FC<{ // ─── Detail mode ─────────────────────────────────────────── const DetailBody: React.FC<{ + entry: DialogEntry; + maxHeight: number; + maxWidth: number; +}> = ({ entry, maxHeight, maxWidth }) => + entry.kind === 'agent' ? ( + + ) : ( + + ); + +const AgentDetailBody: React.FC<{ entry: BackgroundTaskEntry; maxHeight: number; maxWidth: number; }> = ({ entry, maxHeight, maxWidth }) => { - const title = `${entry.subagentType ?? 'Agent'} \u203A ${rowLabel(entry)}`; + const title = `${entry.subagentType ?? 'Agent'} \u203A ${buildBackgroundEntryLabel(entry, { includePrefix: false })}`; const terminal = terminalStatusPresentation(entry.status); const dimSubtitleParts: string[] = [elapsedFor(entry)]; @@ -342,6 +370,85 @@ const DetailBody: React.FC<{ ); }; +const ShellDetailBody: React.FC<{ + entry: import('@qwen-code/qwen-code-core').BackgroundShellEntry; + maxHeight: number; + maxWidth: number; +}> = ({ entry, maxHeight, maxWidth }) => { + const title = `Shell \u203A ${entry.command}`; + + const terminal = terminalStatusPresentation(entry.status); + const dimSubtitleParts: string[] = [elapsedFor(entry)]; + if (entry.pid !== undefined) { + dimSubtitleParts.push(`pid ${entry.pid}`); + } + if (entry.status === 'completed' && entry.exitCode !== undefined) { + dimSubtitleParts.push(`exit ${entry.exitCode}`); + } + + const hasError = entry.status === 'failed' && Boolean(entry.error); + + return ( + + + + {title} + + + + {terminal && ( + + {`${terminal.icon} ${STATUS_VERBS[entry.status]} \u00B7 `} + + )} + + {dimSubtitleParts.join(' \u00B7 ')} + + + + + + + Working dir + + + + {entry.cwd} + + + + + + Output file + + + + {entry.outputPath} + + + {hasError && ( + + + + + Error + + + + + {entry.error} + + + + )} + + ); +}; + // ─── Dialog shell ────────────────────────────────────────── interface BackgroundTasksDialogProps { @@ -376,31 +483,49 @@ export const BackgroundTasksDialog: React.FC = ({ // List mode row budget: terminal height minus chrome (border 2 + title 1 // + two marginTops 2 + hint 1) and list header ("N active agents" 1 + - // marginTop 1 + "Local agents (N)" 1) = 10. + // marginTop 1 + "Background tasks (N)" 1) = 10. const listMaxRows = Math.max(3, availableTerminalHeight - 10); - const selectedEntry = useMemo( - () => entries[selectedIndex] ?? null, - [entries, selectedIndex], - ); + // Activity tick — bumped whenever the watched agent emits an activity + // update, *and* used as a useMemo dep below to refresh the live agent + // entry from the registry. The snapshot in useBackgroundTaskView + // intentionally only refreshes on `statusChange` (so the footer pill + // and AppContainer stay quiet during heavy tool traffic), but the + // detail body must see fresh `recentActivities` / `stats` between + // those transitions — so we re-read from the registry here. + const [activityTick, setActivityTick] = useState(0); - // Tick up a local counter on each activity callback to force the - // detail body to re-render while it's open. The main status - // subscription in useBackgroundTaskView intentionally ignores - // activity updates so the Footer pill and AppContainer don't re-run - // on every tool call a background agent makes. - const [, bumpActivity] = useState(0); - const selectedAgentId = selectedEntry?.agentId; + const selectedEntry = useMemo(() => { + const fromSnapshot = entries[selectedIndex] ?? null; + if (!fromSnapshot || fromSnapshot.kind !== 'agent') return fromSnapshot; + // Re-read the agent from the registry so detail-body fields the + // registry mutates between status transitions (recentActivities, + // stats) are fresh. The shallow spread inside useBackgroundTaskView + // captures `recentActivities` at refresh time, and `appendActivity` + // reassigns `entry.recentActivities = next` on the registry object — + // so the snapshot's reference is detached after the first activity. + const live = config.getBackgroundTaskRegistry().get(fromSnapshot.agentId); + return live ? { ...live, kind: 'agent' as const } : fromSnapshot; + // activityTick is a dep on purpose: the registry mutation is invisible + // to useMemo otherwise and we need to recompute on each activity. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [entries, selectedIndex, config, activityTick]); + + const selectedEntryId = selectedEntry ? entryId(selectedEntry) : undefined; + // Activity callback is agent-only — shells don't emit per-tool events. + const selectedAgentIdForActivity = + selectedEntry?.kind === 'agent' ? selectedEntry.agentId : undefined; useEffect(() => { - if (!dialogOpen || dialogMode !== 'detail' || !selectedAgentId) return; + if (!dialogOpen || dialogMode !== 'detail' || !selectedAgentIdForActivity) + return; const registry = config.getBackgroundTaskRegistry(); const onActivity = (entry: BackgroundTaskEntry) => { - if (entry.agentId !== selectedAgentId) return; - bumpActivity((n) => n + 1); + if (entry.agentId !== selectedAgentIdForActivity) return; + setActivityTick((n) => n + 1); }; registry.setActivityChangeCallback(onActivity); return () => registry.setActivityChangeCallback(undefined); - }, [dialogOpen, dialogMode, config, selectedAgentId]); + }, [dialogOpen, dialogMode, config, selectedAgentIdForActivity]); // Wall-clock tick for the running agent's duration. Activity callbacks // fire when tools run, but duration needs to advance even when the agent @@ -410,13 +535,13 @@ export const BackgroundTasksDialog: React.FC = ({ if ( !dialogOpen || dialogMode !== 'detail' || - !selectedAgentId || + !selectedEntryId || selectedStatus !== 'running' ) return; - const id = setInterval(() => bumpActivity((n) => n + 1), 1000); + const id = setInterval(() => setActivityTick((n) => n + 1), 1000); return () => clearInterval(id); - }, [dialogOpen, dialogMode, selectedAgentId, selectedStatus]); + }, [dialogOpen, dialogMode, selectedEntryId, selectedStatus]); // Auto-fallback to the list view when the selected agent reaches a // terminal state while the user is watching it live. We only exit on @@ -425,8 +550,8 @@ export const BackgroundTasksDialog: React.FC = ({ // view itself renders terminal state fine, so this is a UX choice // (return focus to the running roster) rather than a correctness fix. const initialDetailStatusRef = useRef<{ - agentId: string; - status: BackgroundTaskEntry['status']; + entryId: string; + status: EntryStatus; } | null>(null); useEffect(() => { if (!dialogOpen || dialogMode !== 'detail') { @@ -437,18 +562,18 @@ export const BackgroundTasksDialog: React.FC = ({ // drop back to the list so we don't sit on a "No entry to show" screen. // Hitting this path now is unlikely — terminal entries stay in the // registry — but the entry could disappear if the registry is reset. - if (!selectedAgentId) { + if (!selectedEntryId) { initialDetailStatusRef.current = null; exitDetail(); return; } const seen = initialDetailStatusRef.current; - if (!seen || seen.agentId !== selectedAgentId) { + if (!seen || seen.entryId !== selectedEntryId) { // First render in detail mode for this entry — remember the status we // opened with so we can detect a transition away from 'running' later. if (selectedStatus) { initialDetailStatusRef.current = { - agentId: selectedAgentId, + entryId: selectedEntryId, status: selectedStatus, }; } @@ -461,7 +586,7 @@ export const BackgroundTasksDialog: React.FC = ({ ) { exitDetail(); } - }, [dialogOpen, dialogMode, selectedAgentId, selectedStatus, exitDetail]); + }, [dialogOpen, dialogMode, selectedEntryId, selectedStatus, exitDetail]); useKeypress( (key) => { diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx index ffdd53e8b..e702cf50d 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx @@ -5,57 +5,92 @@ */ import { describe, it, expect } from 'vitest'; -import type { BackgroundTaskEntry } from '@qwen-code/qwen-code-core'; +import type { DialogEntry } from '../../hooks/useBackgroundTaskView.js'; import { getPillLabel } from './BackgroundTasksPill.js'; -function entry(overrides: Partial): BackgroundTaskEntry { +function agentEntry(overrides: Partial = {}): DialogEntry { return { + kind: 'agent', agentId: 'a', description: 'desc', status: 'running', startTime: 0, abortController: new AbortController(), ...overrides, - }; + } as DialogEntry; +} + +function shellEntry(overrides: Partial = {}): DialogEntry { + return { + kind: 'shell', + shellId: 'bg_x', + command: 'sleep 60', + cwd: '/tmp', + status: 'running', + startTime: 0, + outputPath: '/tmp/x.out', + abortController: new AbortController(), + ...overrides, + } as DialogEntry; } describe('getPillLabel', () => { it('uses singular form for one running agent', () => { - expect(getPillLabel([entry({ agentId: 'a' })])).toBe('1 local agent'); + expect(getPillLabel([agentEntry({ agentId: 'a' })])).toBe('1 local agent'); }); it('uses plural form for multiple running agents', () => { expect( getPillLabel([ - entry({ agentId: 'a' }), - entry({ agentId: 'b' }), - entry({ agentId: 'c' }), + agentEntry({ agentId: 'a' }), + agentEntry({ agentId: 'b' }), + agentEntry({ agentId: 'c' }), ]), ).toBe('3 local agents'); }); + it('uses singular form for one running shell', () => { + expect(getPillLabel([shellEntry({ shellId: 'bg_a' })])).toBe('1 shell'); + }); + + it('uses plural form for multiple running shells', () => { + expect( + getPillLabel([ + shellEntry({ shellId: 'bg_a' }), + shellEntry({ shellId: 'bg_b' }), + ]), + ).toBe('2 shells'); + }); + + it('groups by kind when both kinds are running, shells first', () => { + expect( + getPillLabel([ + agentEntry({ agentId: 'a' }), + shellEntry({ shellId: 'bg_a' }), + shellEntry({ shellId: 'bg_b' }), + ]), + ).toBe('2 shells, 1 local agent'); + }); + it('counts only running entries when running and terminal mix', () => { expect( getPillLabel([ - entry({ agentId: 'a', status: 'running' }), - entry({ agentId: 'b', status: 'completed' }), - entry({ agentId: 'c', status: 'cancelled' }), + agentEntry({ agentId: 'a', status: 'running' }), + agentEntry({ agentId: 'b', status: 'completed' }), + shellEntry({ shellId: 'bg_a', status: 'cancelled' }), ]), ).toBe('1 local agent'); }); - it('uses singular done form for one terminal-only entry', () => { - expect(getPillLabel([entry({ agentId: 'a', status: 'completed' })])).toBe( - '1 local agent done', - ); - }); - - it('uses plural done form when all entries are terminal', () => { + it('uses generic done form when all entries are terminal', () => { + expect( + getPillLabel([agentEntry({ agentId: 'a', status: 'completed' })]), + ).toBe('1 task done'); expect( getPillLabel([ - entry({ agentId: 'a', status: 'completed' }), - entry({ agentId: 'b', status: 'failed' }), + agentEntry({ agentId: 'a', status: 'completed' }), + shellEntry({ shellId: 'bg_a', status: 'failed' }), ]), - ).toBe('2 local agents done'); + ).toBe('2 tasks done'); }); }); diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx index fc8edfb4b..6cf88de6c 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx @@ -13,21 +13,44 @@ import { } from '../../contexts/BackgroundTaskViewContext.js'; import { useKeypress, type Key } from '../../hooks/useKeypress.js'; import { theme } from '../../semantic-colors.js'; -import type { BackgroundTaskEntry } from '@qwen-code/qwen-code-core'; +import type { DialogEntry } from '../../hooks/useBackgroundTaskView.js'; + +const KIND_NAMES = { + agent: { singular: 'local agent', plural: 'local agents' }, + shell: { singular: 'shell', plural: 'shells' }, +} as const; /** - * Pill label: counts running entries while any are running; once everything - * has terminated, switches to a "done" form so the pill still invites - * reopening the dialog to inspect final state. + * Pill label: counts running entries grouped by kind while any are + * running ("1 shell, 2 local agents"), and once everything has terminated + * switches to a "done" form so the pill still invites reopening the + * dialog to inspect final state ("3 done"). */ -export function getPillLabel(entries: readonly BackgroundTaskEntry[]): string { - const running = entries.filter((e) => e.status === 'running').length; - if (running > 0) { - return running === 1 ? '1 local agent' : `${running} local agents`; +export function getPillLabel(entries: readonly DialogEntry[]): string { + if (entries.length === 0) return ''; + + const running = entries.filter((e) => e.status === 'running'); + if (running.length > 0) { + return groupAndFormat(running); } - return entries.length === 1 - ? '1 local agent done' - : `${entries.length} local agents done`; + // All terminal — collapse into a single tally; per-kind detail isn't + // useful at this point and would clutter the footer. + return entries.length === 1 ? '1 task done' : `${entries.length} tasks done`; +} + +function groupAndFormat(entries: readonly DialogEntry[]): string { + const counts = { agent: 0, shell: 0 }; + for (const e of entries) counts[e.kind]++; + const parts: string[] = []; + // Order: shell first (matches Claude Code's pill convention), agent second. + if (counts.shell > 0) parts.push(formatCount('shell', counts.shell)); + if (counts.agent > 0) parts.push(formatCount('agent', counts.agent)); + return parts.join(', '); +} + +function formatCount(kind: keyof typeof KIND_NAMES, n: number): string { + const names = KIND_NAMES[kind]; + return `${n} ${n === 1 ? names.singular : names.plural}`; } export const BackgroundTasksPill: React.FC = () => { diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx index cdb6a3605..f508543e2 100644 --- a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx @@ -19,19 +19,23 @@ import { useMemo, useState, } from 'react'; +import { type Config } from '@qwen-code/qwen-code-core'; import { - type BackgroundTaskEntry, - type Config, -} from '@qwen-code/qwen-code-core'; -import { useBackgroundTaskView } from '../hooks/useBackgroundTaskView.js'; + type DialogEntry, + useBackgroundTaskView, +} from '../hooks/useBackgroundTaskView.js'; // ─── Types ────────────────────────────────────────────────── export type BackgroundDialogMode = 'closed' | 'list' | 'detail'; export interface BackgroundTaskViewState { - /** Live snapshot of every background agent entry, ordered by startTime. */ - entries: readonly BackgroundTaskEntry[]; + /** + * Live snapshot of every background entry across both registries + * (subagents + managed shells), ordered by `startTime`. Each entry carries + * a `kind` discriminator so renderers can dispatch on agent vs shell. + */ + entries: readonly DialogEntry[]; /** Index into `entries` for the currently focused row (0-based). */ selectedIndex: number; /** `'closed'` when the overlay isn't mounted; otherwise the active mode. */ @@ -166,8 +170,16 @@ export function BackgroundTaskViewProvider({ if (!config) return; const target = entries[selectedIndex]; if (!target) return; - // cancel() is a no-op for non-running entries, so no pre-check here. - config.getBackgroundTaskRegistry().cancel(target.agentId); + // Both registries' cancel paths are no-ops on non-running entries, so + // no pre-check here. Shell cancel goes through requestCancel — it + // triggers the AbortController only and lets the spawn's settle path + // record the real terminal moment + outcome (mirrors the task_stop + // tool path in #3687). + if (target.kind === 'agent') { + config.getBackgroundTaskRegistry().cancel(target.agentId); + } else { + config.getBackgroundShellRegistry().requestCancel(target.shellId); + } }, [config, entries, selectedIndex]); const state: BackgroundTaskViewState = useMemo( diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts index c55040b6e..7e4fe89fd 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts @@ -5,11 +5,14 @@ */ /** - * useBackgroundTaskView — subscribes to the background task registry's - * status-change callback and maintains a reactive snapshot of every - * `BackgroundTaskEntry`, including terminal ones. Surfaces that only - * care about live work (the footer pill, the composer's Down-arrow - * route) filter for `running` themselves. + * useBackgroundTaskView — subscribes to both registries (background + * subagents and background shells) and merges them into a single ordered + * snapshot of `DialogEntry`s. Both registries fire `statusChange` on + * register too, so a single subscription per registry is enough to keep + * the snapshot fresh for new + transitioning entries. + * + * Surfaces that only care about live work (the footer pill, the + * composer's Down-arrow route) filter for `running` themselves. * * Intentionally ignores activity updates (appendActivity). Tool-call * traffic from a running background agent would otherwise churn the @@ -21,33 +24,63 @@ import { useState, useEffect } from 'react'; import { type BackgroundTaskEntry, + type BackgroundShellEntry, type Config, } from '@qwen-code/qwen-code-core'; +/** + * A unified view-model entry the dialog/pill/context render against. + * Discriminated by `kind`; agent-shaped fields and shell-shaped fields + * are inlined verbatim to keep the renderer code unchanged on the agent + * branch (just guarded by `kind === 'agent'`). + */ +export type DialogEntry = + | (BackgroundTaskEntry & { kind: 'agent' }) + | (BackgroundShellEntry & { kind: 'shell' }); + export interface UseBackgroundTaskViewResult { - entries: readonly BackgroundTaskEntry[]; + entries: readonly DialogEntry[]; +} + +/** Stable id of an entry regardless of kind — used as React key + lookup. */ +export function entryId(entry: DialogEntry): string { + return entry.kind === 'agent' ? entry.agentId : entry.shellId; } export function useBackgroundTaskView( config: Config | null, ): UseBackgroundTaskViewResult { - const [entries, setEntries] = useState([]); + const [entries, setEntries] = useState([]); useEffect(() => { if (!config) return; - const registry = config.getBackgroundTaskRegistry(); + const agentRegistry = config.getBackgroundTaskRegistry(); + const shellRegistry = config.getBackgroundShellRegistry(); - // getAll() returns a fresh array in registration (= startTime) order. - setEntries(registry.getAll()); - - const onStatusChange = () => { - setEntries(registry.getAll()); + const refresh = () => { + const agentEntries: DialogEntry[] = agentRegistry + .getAll() + .map((e) => ({ ...e, kind: 'agent' as const })); + const shellEntries: DialogEntry[] = shellRegistry + .getAll() + .map((e) => ({ ...e, kind: 'shell' as const })); + // Merge by startTime so the order matches launch order across both + // registries (matters when an agent and a shell are launched + // alternately). + const merged = [...agentEntries, ...shellEntries].sort( + (a, b) => a.startTime - b.startTime, + ); + setEntries(merged); }; - registry.setStatusChangeCallback(onStatusChange); + refresh(); + + agentRegistry.setStatusChangeCallback(refresh); + shellRegistry.setStatusChangeCallback(refresh); return () => { - registry.setStatusChangeCallback(undefined); + agentRegistry.setStatusChangeCallback(undefined); + shellRegistry.setStatusChangeCallback(undefined); }; }, [config]); diff --git a/packages/core/src/services/backgroundShellRegistry.test.ts b/packages/core/src/services/backgroundShellRegistry.test.ts index e6c7b85c2..018ceaa9d 100644 --- a/packages/core/src/services/backgroundShellRegistry.test.ts +++ b/packages/core/src/services/backgroundShellRegistry.test.ts @@ -101,6 +101,85 @@ describe('BackgroundShellRegistry', () => { }); }); + describe('callbacks', () => { + it('fires register callback synchronously when an entry is added', () => { + const reg = new BackgroundShellRegistry(); + const seen: string[] = []; + reg.setRegisterCallback((entry) => seen.push(entry.shellId)); + + reg.register(makeEntry({ shellId: 'a' })); + reg.register(makeEntry({ shellId: 'b' })); + + expect(seen).toEqual(['a', 'b']); + }); + + it('fires statusChange callback on register too (mirrors BackgroundTaskRegistry)', () => { + const reg = new BackgroundShellRegistry(); + const seen: string[] = []; + reg.setStatusChangeCallback((e) => seen.push(e.shellId)); + reg.register(makeEntry({ shellId: 'a' })); + reg.register(makeEntry({ shellId: 'b' })); + expect(seen).toEqual(['a', 'b']); + }); + + it('fires statusChange callback on complete / fail / cancel', () => { + const reg = new BackgroundShellRegistry(); + reg.register(makeEntry({ shellId: 'a' })); + reg.register(makeEntry({ shellId: 'b' })); + reg.register(makeEntry({ shellId: 'c' })); + const transitions: Array<{ id: string; status: string }> = []; + reg.setStatusChangeCallback((entry) => + transitions.push({ id: entry.shellId, status: entry.status }), + ); + + reg.complete('a', 0, 1000); + reg.fail('b', 'boom', 1100); + reg.cancel('c', 1200); + + expect(transitions).toEqual([ + { id: 'a', status: 'completed' }, + { id: 'b', status: 'failed' }, + { id: 'c', status: 'cancelled' }, + ]); + }); + + it('does not fire statusChange when a transition is a no-op', () => { + const reg = new BackgroundShellRegistry(); + const transitions: string[] = []; + reg.setStatusChangeCallback((e) => transitions.push(e.shellId)); + reg.register(makeEntry({ shellId: 'a' })); + reg.complete('a', 0, 1000); + transitions.length = 0; + + reg.complete('a', 0, 2000); // already terminal + reg.fail('a', 'late', 2000); // already terminal + reg.cancel('a', 2000); // already terminal + reg.requestCancel('a'); // already terminal — also no fire + + expect(transitions).toEqual([]); + }); + + it('keeps the registry usable when a callback throws', () => { + const reg = new BackgroundShellRegistry(); + reg.setRegisterCallback(() => { + throw new Error('subscriber blew up'); + }); + + expect(() => reg.register(makeEntry({ shellId: 'a' }))).not.toThrow(); + expect(reg.get('a')!.status).toBe('running'); + }); + + it('clears subscriber when set to undefined', () => { + const reg = new BackgroundShellRegistry(); + const seen: string[] = []; + reg.setRegisterCallback((e) => seen.push(e.shellId)); + reg.register(makeEntry({ shellId: 'a' })); + reg.setRegisterCallback(undefined); + reg.register(makeEntry({ shellId: 'b' })); + expect(seen).toEqual(['a']); + }); + }); + describe('requestCancel', () => { it('aborts the signal but leaves status running and endTime undefined', () => { const reg = new BackgroundShellRegistry(); diff --git a/packages/core/src/services/backgroundShellRegistry.ts b/packages/core/src/services/backgroundShellRegistry.ts index 1b74c7be1..dab71f17f 100644 --- a/packages/core/src/services/backgroundShellRegistry.ts +++ b/packages/core/src/services/backgroundShellRegistry.ts @@ -17,6 +17,10 @@ * status. */ +import { createDebugLogger } from '../utils/debugLogger.js'; + +const debugLogger = createDebugLogger('BACKGROUND_SHELLS'); + export type BackgroundShellStatus = | 'running' | 'completed' @@ -47,6 +51,20 @@ export interface BackgroundShellEntry { abortController: AbortController; } +/** Fires when a new entry is registered. */ +export type BackgroundShellRegisterCallback = ( + entry: BackgroundShellEntry, +) => void; + +/** + * Fires on every status transition (running → terminal). Symmetric with + * `BackgroundTaskRegistry.setStatusChangeCallback` so the same UI hook can + * subscribe to both registries. + */ +export type BackgroundShellStatusChangeCallback = ( + entry: BackgroundShellEntry, +) => void; + export class BackgroundShellRegistry { // Entries persist for the session lifetime — no automatic eviction of // terminal entries. For typical interactive sessions (tens of background @@ -56,8 +74,39 @@ export class BackgroundShellRegistry { // is left as a follow-up alongside output-file rotation. private readonly entries = new Map(); + private registerCallback: BackgroundShellRegisterCallback | undefined; + private statusChangeCallback: BackgroundShellStatusChangeCallback | undefined; + + /** + * Subscribe to new-entry events. Called synchronously inside `register()`. + * Setting `undefined` clears the existing subscriber. Single-subscriber on + * purpose — the UI hook is the only consumer in the codebase, and a list + * would invite drift in error-handling. + */ + setRegisterCallback(cb: BackgroundShellRegisterCallback | undefined): void { + this.registerCallback = cb; + } + + /** + * Subscribe to status transitions (running → terminal). Called + * synchronously inside `complete()` / `fail()` / `cancel()` after the + * entry has been mutated. Same single-subscriber rationale as + * `setRegisterCallback`. + */ + setStatusChangeCallback( + cb: BackgroundShellStatusChangeCallback | undefined, + ): void { + this.statusChangeCallback = cb; + } + register(entry: BackgroundShellEntry): void { this.entries.set(entry.shellId, entry); + this.fireRegister(entry); + // Mirror BackgroundTaskRegistry: registration is a status transition + // (nothing → running) so subscribers that only care about + // "what's in the registry now" can subscribe to a single callback + // and see new entries the same way they see status changes. + this.fireStatusChange(entry); } get(shellId: string): BackgroundShellEntry | undefined { @@ -74,6 +123,7 @@ export class BackgroundShellRegistry { entry.status = 'completed'; entry.exitCode = exitCode; entry.endTime = endTime; + this.fireStatusChange(entry); } fail(shellId: string, error: string, endTime: number): void { @@ -82,6 +132,7 @@ export class BackgroundShellRegistry { entry.status = 'failed'; entry.error = error; entry.endTime = endTime; + this.fireStatusChange(entry); } cancel(shellId: string, endTime: number): void { @@ -90,6 +141,28 @@ export class BackgroundShellRegistry { entry.status = 'cancelled'; entry.endTime = endTime; entry.abortController.abort(); + this.fireStatusChange(entry); + } + + private fireRegister(entry: BackgroundShellEntry): void { + if (!this.registerCallback) return; + try { + this.registerCallback(entry); + } catch (error) { + // Subscriber failure must not poison the registry — the spawn path + // has already happened. Swallow + continue so the entry remains + // observable via `getAll()` / `get()`. + debugLogger.error('register callback failed:', error); + } + } + + private fireStatusChange(entry: BackgroundShellEntry): void { + if (!this.statusChangeCallback) return; + try { + this.statusChangeCallback(entry); + } catch (error) { + debugLogger.error('statusChange callback failed:', error); + } } /**