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 b264d8512..dc6372526 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -28,8 +28,22 @@ vi.mock('../../hooks/useBackgroundTaskView.js', () => ({ // 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, + entryId: (entry: DialogEntry): string => { + switch (entry.kind) { + case 'agent': + return entry.agentId; + case 'shell': + return entry.shellId; + case 'monitor': + return entry.monitorId; + default: { + const _exhaustive: never = entry; + throw new Error( + `entryId: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } + } + }, })); vi.mock('../../hooks/useKeypress.js', () => ({ @@ -51,6 +65,24 @@ function entry(overrides: Partial = {}): DialogEntry { } as DialogEntry; } +function monitorEntry(overrides: Partial = {}): DialogEntry { + return { + kind: 'monitor', + monitorId: 'mon-1', + command: 'tail -f app.log', + description: 'watch app logs', + status: 'running', + startTime: 0, + abortController: new AbortController(), + eventCount: 0, + lastEventTime: 0, + maxEvents: 1000, + idleTimeoutMs: 300_000, + droppedLines: 0, + ...overrides, + } as DialogEntry; +} + interface ProbeHandle { actions: ReturnType; state: ReturnType; @@ -61,6 +93,7 @@ interface Harness { cancel: ReturnType; resume: ReturnType; abandon: ReturnType; + monitorCancel: ReturnType; setEntries: (next: readonly DialogEntry[]) => void; pressKey: (key: { name?: string; sequence?: string }) => void; call: (fn: () => void) => void; @@ -78,6 +111,7 @@ function setup(initial: readonly DialogEntry[]): Harness { const cancel = vi.fn(); const resume = vi.fn(); const abandon = vi.fn(); + const monitorCancel = 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. @@ -93,6 +127,17 @@ function setup(initial: readonly DialogEntry[]): Harness { return match; }, }), + getMonitorRegistry: () => ({ + cancel: monitorCancel, + // Resolve `.get(monitorId)` against the snapshot so the dialog's + // `selectedEntry` re-resolution path works for monitor kind too. + get: (id: string) => { + const match = currentEntries.find( + (e) => e.kind === 'monitor' && e.monitorId === id, + ); + return match; + }, + }), resumeBackgroundAgent: resume, abandonBackgroundAgent: abandon, } as unknown as Config; @@ -136,6 +181,7 @@ function setup(initial: readonly DialogEntry[]): Harness { cancel, resume, abandon, + monitorCancel, setEntries(next) { handlers.length = 0; currentEntries = next; @@ -193,6 +239,25 @@ describe('BackgroundTasksDialog', () => { expect(h.probe.current!.state.dialogMode).toBe('list'); }); + it('routes monitor cancel via monitorRegistry.cancel(monitorId)', () => { + // Pin the monitor-cancel branch in `cancelSelected` — flipping it to + // anything else (e.g. shell's `requestCancel`) would silently break, + // since neither task_stop nor the dialog-test mocks fail loudly on + // the wrong method name. + const mon = monitorEntry({ monitorId: 'mon-zzz', status: 'running' }); + const h = setup([mon]); + + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + expect(h.probe.current!.state.dialogMode).toBe('detail'); + + h.pressKey({ sequence: 'x' }); + expect(h.monitorCancel).toHaveBeenCalledWith('mon-zzz'); + // Agent registry's cancel must NOT be called for a monitor entry — + // belt-and-braces guard against the kind switch falling through. + expect(h.cancel).not.toHaveBeenCalled(); + }); + it('keeps detail mode when an already-terminal entry is opened (no spurious fallback)', () => { const done = entry({ agentId: 'a', status: 'completed' }); const h = setup([done]); @@ -290,4 +355,96 @@ describe('BackgroundTasksDialog', () => { expect(detailFrame).toContain('Temporary resume setup failed.'); expect(detailFrame).toContain('r resume'); }); + + describe('MonitorDetailBody render branches', () => { + function openMonitorDetail(monitorOverrides: Partial = {}) { + const mon = monitorEntry({ + monitorId: 'mon-z', + description: 'watch app logs', + command: 'tail -f app.log', + ...monitorOverrides, + }); + const h = setup([mon]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + return h.lastFrame() ?? ''; + } + + it('renders title from description and shows Command block', () => { + const f = openMonitorDetail(); + expect(f).toContain('Monitor'); + expect(f).toContain('watch app logs'); + expect(f).toContain('Command'); + expect(f).toContain('tail -f app.log'); + }); + + it('renders pid when defined, omits when undefined', () => { + expect( + openMonitorDetail({ pid: 4242 } as Partial), + ).toContain('pid 4242'); + expect(openMonitorDetail()).not.toContain('pid '); + }); + + it('uses singular "1 event" / plural "N events"', () => { + const f1 = openMonitorDetail({ eventCount: 1 } as Partial); + expect(f1).toContain('1 event'); + // Guard against false positive — substring "1 event" also matches "1 events". + expect(f1).not.toContain('1 events'); + + const f5 = openMonitorDetail({ eventCount: 5 } as Partial); + expect(f5).toContain('5 events'); + }); + + it('renders droppedLines only when > 0', () => { + expect( + openMonitorDetail({ droppedLines: 0 } as Partial), + ).not.toContain('dropped'); + expect( + openMonitorDetail({ droppedLines: 3 } as Partial), + ).toContain('3 dropped'); + }); + + it('renders exitCode in subtitle when defined', () => { + expect( + openMonitorDetail({ + status: 'completed', + exitCode: 0, + } as Partial), + ).toContain('exit 0'); + expect( + openMonitorDetail({ + status: 'completed', + exitCode: 1, + } as Partial), + ).toContain('exit 1'); + }); + + it('renders Error block for failed status', () => { + const f = openMonitorDetail({ + status: 'failed', + error: 'spawn ENOENT', + } as Partial); + expect(f).toContain('Error'); + expect(f).toContain('spawn ENOENT'); + // The auto-stop label must not appear on a `failed` entry — the + // two error-block branches share a render slot, so a regression + // collapsing them would silently swap the user-facing wording. + expect(f).not.toContain('Stopped because'); + }); + + it('renders "Stopped because" block for completed with auto-stop reason', () => { + const f = openMonitorDetail({ + status: 'completed', + error: 'Max events reached', + } as Partial); + expect(f).toContain('Stopped because'); + expect(f).toContain('Max events reached'); + }); + + it('omits the error block entirely when error is undefined', () => { + const f = openMonitorDetail({ status: 'completed' }); + expect(f).not.toContain('Error'); + expect(f).not.toContain('Stopped because'); + }); + }); }); diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 1d1e4d310..3bf40798f 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -27,6 +27,7 @@ import { ToolDisplayNames, ToolNames, type BackgroundTaskEntry, + type MonitorEntry, } from '@qwen-code/qwen-code-core'; import { formatDuration, formatTokenCount } from '../../utils/formatters.js'; import { @@ -103,14 +104,27 @@ function terminalStatusPresentation( } function rowLabel(entry: DialogEntry): string { - if (entry.kind === 'agent') { - return buildBackgroundEntryLabel(entry, { includePrefix: false }); + switch (entry.kind) { + case 'agent': + return buildBackgroundEntryLabel(entry, { includePrefix: false }); + case 'shell': + // Shell / monitor prefixes mirror the dialog's "section" visual hint + // without needing per-kind section headers (which would complicate + // the windowing math). Long commands / descriptions wrap (ListBody + // renders rows with plain ``, no truncation helper), which + // is acceptable for the dialog's information-density profile — + // adding `wrap="truncate-end"` here would hide context the user + // explicitly opened the dialog to see. + return `[shell] ${entry.command}`; + case 'monitor': + return `[monitor] ${entry.description}`; + default: { + const _exhaustive: never = entry; + throw new Error( + `rowLabel: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } } - // 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: { startTime: number; endTime?: number }): string { @@ -242,12 +256,40 @@ const DetailBody: React.FC<{ entry: DialogEntry; maxHeight: number; maxWidth: number; -}> = ({ entry, maxHeight, maxWidth }) => - entry.kind === 'agent' ? ( - - ) : ( - - ); +}> = ({ entry, maxHeight, maxWidth }) => { + switch (entry.kind) { + case 'agent': + return ( + + ); + case 'shell': + return ( + + ); + case 'monitor': + return ( + + ); + default: { + const _exhaustive: never = entry; + throw new Error( + `DetailBody: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } + } +}; const AgentDetailBody: React.FC<{ entry: AgentDialogEntry; @@ -473,6 +515,84 @@ const ShellDetailBody: React.FC<{ ); }; +const MonitorDetailBody: React.FC<{ + entry: MonitorEntry; + maxHeight: number; + maxWidth: number; +}> = ({ entry, maxHeight, maxWidth }) => { + const title = `Monitor › ${entry.description}`; + + const terminal = terminalStatusPresentation(entry.status); + const dimSubtitleParts: string[] = [elapsedFor(entry)]; + if (entry.pid !== undefined) { + dimSubtitleParts.push(`pid ${entry.pid}`); + } + dimSubtitleParts.push( + `${entry.eventCount} event${entry.eventCount === 1 ? '' : 's'}`, + ); + if (entry.droppedLines > 0) { + dimSubtitleParts.push(`${entry.droppedLines} dropped`); + } + if (entry.exitCode !== undefined) { + dimSubtitleParts.push(`exit ${entry.exitCode}`); + } + + // `entry.error` is set on `failed` (spawn error) and on `completed` + // when the monitor was auto-stopped (max events / idle timeout). Worth + // surfacing whenever it exists, regardless of terminal status. + const hasError = Boolean(entry.error); + const errorIsFailure = entry.status === 'failed'; + const errorColor = errorIsFailure ? theme.status.error : theme.status.warning; + + return ( + + + + {title} + + + + {terminal && ( + + {`${terminal.icon} ${STATUS_VERBS[entry.status]} · `} + + )} + {dimSubtitleParts.join(' · ')} + + + + + + Command + + + + {entry.command} + + + {hasError && ( + + + + + {errorIsFailure ? 'Error' : 'Stopped because'} + + + + + {entry.error} + + + + )} + + ); +}; + // ─── Dialog shell ────────────────────────────────────────── interface BackgroundTasksDialogProps { @@ -522,15 +642,30 @@ export const BackgroundTasksDialog: React.FC = ({ 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; + if (!fromSnapshot) return fromSnapshot; + // Re-read the entry from the registry on each activityTick so + // detail-body fields the registry mutates between status transitions + // are fresh. The snapshot in useBackgroundTaskView only refreshes on + // statusChange (so the pill / AppContainer don't churn under heavy + // tool / event traffic), so for the detail view we have to re-resolve + // explicitly: + // - agent: `recentActivities` is reassigned by `appendActivity`, + // which fires `activityChange` (subscribed below). + // - monitor: `eventCount` / `droppedLines` are mutated by + // `emitEvent`, which intentionally does NOT fire `statusChange` + // to avoid per-event refresh churn. The 1s wall-clock tick below + // drives the recompute instead. + // Shells don't mutate detail-visible fields between statusChange + // events, so the snapshot stays correct for them. + if (fromSnapshot.kind === 'agent') { + const live = config.getBackgroundTaskRegistry().get(fromSnapshot.agentId); + return live ? { ...live, kind: 'agent' as const } : fromSnapshot; + } + if (fromSnapshot.kind === 'monitor') { + const live = config.getMonitorRegistry().get(fromSnapshot.monitorId); + return live ? { ...live, kind: 'monitor' as const } : fromSnapshot; + } + return 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 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 49d2b7889..5721d890e 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx @@ -34,6 +34,24 @@ function shellEntry(overrides: Partial = {}): DialogEntry { } as DialogEntry; } +function monitorEntry(overrides: Partial = {}): DialogEntry { + return { + kind: 'monitor', + monitorId: 'mon-1', + command: 'tail -f app.log', + description: 'watch app logs', + status: 'running', + startTime: 0, + abortController: new AbortController(), + eventCount: 0, + lastEventTime: 0, + maxEvents: 1000, + idleTimeoutMs: 300_000, + droppedLines: 0, + ...overrides, + } as DialogEntry; +} + describe('getPillLabel', () => { it('uses singular form for one running agent', () => { expect(getPillLabel([agentEntry({ agentId: 'a' })])).toBe('1 local agent'); @@ -72,6 +90,42 @@ describe('getPillLabel', () => { ).toBe('2 shells, 1 local agent'); }); + it('uses singular form for one running monitor', () => { + expect(getPillLabel([monitorEntry({ monitorId: 'mon-a' })])).toBe( + '1 monitor', + ); + }); + + it('uses plural form for multiple running monitors', () => { + expect( + getPillLabel([ + monitorEntry({ monitorId: 'mon-a' }), + monitorEntry({ monitorId: 'mon-b' }), + ]), + ).toBe('2 monitors'); + }); + + it('groups all three kinds with shells → agents → monitors order', () => { + expect( + getPillLabel([ + agentEntry({ agentId: 'a' }), + shellEntry({ shellId: 'bg_a' }), + monitorEntry({ monitorId: 'mon-a' }), + monitorEntry({ monitorId: 'mon-b' }), + ]), + ).toBe('1 shell, 1 local agent, 2 monitors'); + }); + + it('counts only running entries when monitors mix with terminal entries', () => { + expect( + getPillLabel([ + monitorEntry({ monitorId: 'mon-a', status: 'running' }), + monitorEntry({ monitorId: 'mon-b', status: 'completed' }), + monitorEntry({ monitorId: 'mon-c', status: 'cancelled' }), + ]), + ).toBe('1 monitor'); + }); + it('counts only running entries when running and terminal mix', () => { expect( getPillLabel([ diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx index 540b06620..99b08887c 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx @@ -18,6 +18,7 @@ import type { DialogEntry } from '../../hooks/useBackgroundTaskView.js'; const KIND_NAMES = { agent: { singular: 'local agent', plural: 'local agents' }, shell: { singular: 'shell', plural: 'shells' }, + monitor: { singular: 'monitor', plural: 'monitors' }, } as const; /** @@ -47,12 +48,15 @@ export function getPillLabel(entries: readonly DialogEntry[]): string { } function groupAndFormat(entries: readonly DialogEntry[]): string { - const counts = { agent: 0, shell: 0 }; + const counts = { agent: 0, shell: 0, monitor: 0 }; for (const e of entries) counts[e.kind]++; const parts: string[] = []; - // Order: shell first (matches Claude Code's pill convention), agent second. + // Order: shell first (matches Claude Code's pill convention), then + // agent, then monitor. Monitor sits last because it tends to be the + // longest-lived entry and least urgent to glance at. if (counts.shell > 0) parts.push(formatCount('shell', counts.shell)); if (counts.agent > 0) parts.push(formatCount('agent', counts.agent)); + if (counts.monitor > 0) parts.push(formatCount('monitor', counts.monitor)); return parts.join(', '); } diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx index be3456f26..cc95d4f7b 100644 --- a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx @@ -177,15 +177,29 @@ export function BackgroundTaskViewProvider({ config.abandonBackgroundAgent(target.agentId); return; } - // 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); + // All three 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). Monitor cancel is + // synchronous: settle + abort happen inside the registry's cancel(), + // matching its own task_stop path. + switch (target.kind) { + case 'agent': + config.getBackgroundTaskRegistry().cancel(target.agentId); + break; + case 'shell': + config.getBackgroundShellRegistry().requestCancel(target.shellId); + break; + case 'monitor': + config.getMonitorRegistry().cancel(target.monitorId); + break; + default: { + const _exhaustive: never = target; + throw new Error( + `cancelSelected: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } } }, [config, entries, selectedIndex]); diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts new file mode 100644 index 000000000..7c71f22d8 --- /dev/null +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts @@ -0,0 +1,182 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import type { Config } from '@qwen-code/qwen-code-core'; +import { useBackgroundTaskView, entryId } from './useBackgroundTaskView.js'; + +interface FakeRegistry { + setStatusChangeCallback: ReturnType; + /** Test helper — invokes the currently-set callback. */ + fire: () => void; +} + +function makeFakeRegistry(): FakeRegistry { + let cb: (() => void) | undefined; + return { + setStatusChangeCallback: vi.fn((next: (() => void) | undefined) => { + cb = next; + }), + fire: () => cb?.(), + }; +} + +function makeConfig(opts: { + agents: () => unknown[]; + shells: () => unknown[]; + monitors: () => unknown[]; +}) { + const agentReg = makeFakeRegistry(); + const shellReg = makeFakeRegistry(); + const monitorReg = makeFakeRegistry(); + + const config = { + getBackgroundTaskRegistry: () => ({ + ...agentReg, + getAll: opts.agents, + }), + getBackgroundShellRegistry: () => ({ + ...shellReg, + getAll: opts.shells, + }), + getMonitorRegistry: () => ({ + ...monitorReg, + getAll: opts.monitors, + }), + } as unknown as Config; + + return { config, agentReg, shellReg, monitorReg }; +} + +const agent = (id: string, startTime: number) => ({ + agentId: id, + description: 'desc', + status: 'running' as const, + startTime, + abortController: new AbortController(), +}); + +const shell = (id: string, startTime: number) => ({ + shellId: id, + command: 'sleep 60', + cwd: '/tmp', + status: 'running' as const, + startTime, + outputPath: '/tmp/x.out', + abortController: new AbortController(), +}); + +const monitor = (id: string, startTime: number) => ({ + monitorId: id, + command: 'tail -f log', + description: 'watch logs', + status: 'running' as const, + startTime, + abortController: new AbortController(), + eventCount: 0, + lastEventTime: 0, + maxEvents: 1000, + idleTimeoutMs: 300_000, + droppedLines: 0, +}); + +describe('useBackgroundTaskView', () => { + it('returns empty entries when config is null', () => { + const { result } = renderHook(() => useBackgroundTaskView(null)); + expect(result.current.entries).toEqual([]); + }); + + it('merges entries from all three registries on mount', () => { + const { config } = makeConfig({ + agents: () => [agent('a1', 100)], + shells: () => [shell('s1', 50)], + monitors: () => [monitor('m1', 200)], + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + expect(result.current.entries).toHaveLength(3); + // Sort order is by startTime ascending — shell (50) → agent (100) → monitor (200). + expect(result.current.entries.map(entryId)).toEqual(['s1', 'a1', 'm1']); + }); + + it('tags each merged entry with the right `kind` discriminator', () => { + const { config } = makeConfig({ + agents: () => [agent('a1', 0)], + shells: () => [shell('s1', 0)], + monitors: () => [monitor('m1', 0)], + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + const kinds = result.current.entries.map((e) => e.kind).sort(); + expect(kinds).toEqual(['agent', 'monitor', 'shell']); + }); + + it('subscribes to all three registries on mount', () => { + const { config, agentReg, shellReg, monitorReg } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + }); + renderHook(() => useBackgroundTaskView(config)); + expect(agentReg.setStatusChangeCallback).toHaveBeenCalledWith( + expect.any(Function), + ); + expect(shellReg.setStatusChangeCallback).toHaveBeenCalledWith( + expect.any(Function), + ); + expect(monitorReg.setStatusChangeCallback).toHaveBeenCalledWith( + expect.any(Function), + ); + }); + + it('refreshes entries when any registry fires statusChange', () => { + const agents: Array> = []; + const monitors: Array> = []; + const { config, agentReg, monitorReg } = makeConfig({ + agents: () => agents, + shells: () => [], + monitors: () => monitors, + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + expect(result.current.entries).toEqual([]); + + // Simulate registry mutation + statusChange fire from each registry. + agents.push(agent('a1', 100)); + act(() => agentReg.fire()); + expect(result.current.entries.map(entryId)).toEqual(['a1']); + + monitors.push(monitor('m1', 50)); + act(() => monitorReg.fire()); + // monitor's startTime (50) sorts before agent's (100). + expect(result.current.entries.map(entryId)).toEqual(['m1', 'a1']); + }); + + it('clears all three subscriptions on unmount', () => { + const { config, agentReg, shellReg, monitorReg } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + }); + const { unmount } = renderHook(() => useBackgroundTaskView(config)); + unmount(); + // Each setStatusChangeCallback should have been called twice — once + // with the refresh function on mount, once with `undefined` on + // cleanup. Failing this check would mean stale subscribers can fire + // into an unmounted component (warning + state-update on unmounted + // tree, sometimes crashes the next render). + expect(agentReg.setStatusChangeCallback.mock.calls).toEqual([ + [expect.any(Function)], + [undefined], + ]); + expect(shellReg.setStatusChangeCallback.mock.calls).toEqual([ + [expect.any(Function)], + [undefined], + ]); + expect(monitorReg.setStatusChangeCallback.mock.calls).toEqual([ + [expect.any(Function)], + [undefined], + ]); + }); +}); diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts index 5359a7339..7edd3c48a 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts @@ -5,11 +5,11 @@ */ /** - * 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. + * useBackgroundTaskView — subscribes to all three registries (background + * subagents, managed shells, and event monitors) and merges them into a + * single ordered snapshot of `DialogEntry`s. Each registry fires + * `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. @@ -26,6 +26,7 @@ import { type BackgroundTaskEntry, type BackgroundShellEntry, type Config, + type MonitorEntry, } from '@qwen-code/qwen-code-core'; export type AgentDialogEntry = BackgroundTaskEntry & { @@ -35,13 +36,14 @@ export type AgentDialogEntry = BackgroundTaskEntry & { /** * 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'`). + * Discriminated by `kind`; per-kind fields are inlined verbatim so + * renderer code can stay mechanical (`entry.kind === 'agent'` / + * `'shell'` / `'monitor'` guard, then access fields directly). */ export type DialogEntry = | AgentDialogEntry - | (BackgroundShellEntry & { kind: 'shell' }); + | (BackgroundShellEntry & { kind: 'shell' }) + | (MonitorEntry & { kind: 'monitor' }); export interface UseBackgroundTaskViewResult { entries: readonly DialogEntry[]; @@ -49,7 +51,20 @@ export interface UseBackgroundTaskViewResult { /** 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; + switch (entry.kind) { + case 'agent': + return entry.agentId; + case 'shell': + return entry.shellId; + case 'monitor': + return entry.monitorId; + default: { + const _exhaustive: never = entry; + throw new Error( + `entryId: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } + } } export function useBackgroundTaskView( @@ -61,6 +76,7 @@ export function useBackgroundTaskView( if (!config) return; const agentRegistry = config.getBackgroundTaskRegistry(); const shellRegistry = config.getBackgroundShellRegistry(); + const monitorRegistry = config.getMonitorRegistry(); const refresh = () => { const agentEntries: DialogEntry[] = agentRegistry @@ -69,10 +85,13 @@ export function useBackgroundTaskView( 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( + const monitorEntries: DialogEntry[] = monitorRegistry + .getAll() + .map((e) => ({ ...e, kind: 'monitor' as const })); + // Merge by startTime so the order matches launch order across all + // registries (matters when an agent, shell, and monitor are + // launched alternately). + const merged = [...agentEntries, ...shellEntries, ...monitorEntries].sort( (a, b) => a.startTime - b.startTime, ); setEntries(merged); @@ -82,10 +101,12 @@ export function useBackgroundTaskView( agentRegistry.setStatusChangeCallback(refresh); shellRegistry.setStatusChangeCallback(refresh); + monitorRegistry.setStatusChangeCallback(refresh); return () => { agentRegistry.setStatusChangeCallback(undefined); shellRegistry.setStatusChangeCallback(undefined); + monitorRegistry.setStatusChangeCallback(undefined); }; }, [config]); diff --git a/packages/core/src/services/monitorRegistry.test.ts b/packages/core/src/services/monitorRegistry.test.ts index 81bf8b7d9..dc09ec70e 100644 --- a/packages/core/src/services/monitorRegistry.test.ts +++ b/packages/core/src/services/monitorRegistry.test.ts @@ -584,4 +584,102 @@ describe('MonitorRegistry', () => { registry.register(createEntry({ monitorId: 'mon-new' })), ).not.toThrow(); }); + + describe('setStatusChangeCallback', () => { + it('fires once on register (nothing → running)', () => { + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.register(createEntry({ monitorId: 'a' })); + expect(cb).toHaveBeenCalledTimes(1); + expect(cb.mock.calls[0]?.[0]).toMatchObject({ + monitorId: 'a', + status: 'running', + }); + }); + + it('fires on every running → terminal transition (complete / fail / cancel)', () => { + const cb = vi.fn(); + registry.register(createEntry({ monitorId: 'a' })); + registry.register(createEntry({ monitorId: 'b' })); + registry.register(createEntry({ monitorId: 'c' })); + registry.setStatusChangeCallback(cb); + + registry.complete('a', 0); + registry.fail('b', 'oops'); + registry.cancel('c'); + + expect(cb).toHaveBeenCalledTimes(3); + expect(cb.mock.calls[0]?.[0]).toMatchObject({ + monitorId: 'a', + status: 'completed', + }); + expect(cb.mock.calls[1]?.[0]).toMatchObject({ + monitorId: 'b', + status: 'failed', + }); + expect(cb.mock.calls[2]?.[0]).toMatchObject({ + monitorId: 'c', + status: 'cancelled', + }); + }); + + it('does not fire on non-status events (emitEvent without auto-stop)', () => { + registry.register(createEntry({ monitorId: 'a', maxEvents: 10 })); + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.emitEvent('a', 'log line 1'); + registry.emitEvent('a', 'log line 2'); + expect(cb).not.toHaveBeenCalled(); + }); + + it('fires when emitEvent auto-stops at maxEvents (settle path)', () => { + registry.register(createEntry({ monitorId: 'a', maxEvents: 2 })); + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.emitEvent('a', 'line 1'); + registry.emitEvent('a', 'line 2'); // this hits maxEvents, settle('completed') + expect(cb).toHaveBeenCalledTimes(1); + expect(cb.mock.calls[0]?.[0]).toMatchObject({ + monitorId: 'a', + status: 'completed', + }); + }); + + it('clearing the callback stops further notifications', () => { + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.register(createEntry({ monitorId: 'a' })); + registry.setStatusChangeCallback(undefined); + registry.complete('a', 0); + expect(cb).toHaveBeenCalledTimes(1); // register only + }); + + it('callback failure does not poison the registry', () => { + const cb = vi.fn(() => { + throw new Error('subscriber blew up'); + }); + registry.setStatusChangeCallback(cb); + expect(() => + registry.register(createEntry({ monitorId: 'a' })), + ).not.toThrow(); + expect(registry.get('a')).toBeDefined(); + }); + + it('fires once on reset() so dialog snapshots clear stale rows', () => { + registry.register(createEntry({ monitorId: 'a' })); + registry.register(createEntry({ monitorId: 'b' })); + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.reset(); + expect(cb).toHaveBeenCalledTimes(1); + expect(registry.getAll()).toEqual([]); + }); + + it('reset() on an empty registry does not fire statusChange', () => { + const cb = vi.fn(); + registry.setStatusChangeCallback(cb); + registry.reset(); + expect(cb).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/core/src/services/monitorRegistry.ts b/packages/core/src/services/monitorRegistry.ts index a22d80031..81c9396dc 100644 --- a/packages/core/src/services/monitorRegistry.ts +++ b/packages/core/src/services/monitorRegistry.ts @@ -76,6 +76,22 @@ export interface MonitorEntry { idleTimeoutMs: number; idleTimer?: ReturnType; droppedLines: number; + /** Exit code from the underlying process, when known. */ + exitCode?: number; + /** + * Reason for terminal status, when one exists. Mirrors + * `BackgroundShellEntry.error`. Populated for: + * - `failed` — spawn error (passed to `fail(monitorId, error)`). + * - `completed` via auto-stop — currently `'Max events reached'` + * from `emitEvent` and `'Idle timeout'` from the idle timer; any + * future auto-stop reason should populate this field too so the + * detail view stays a complete record of why the monitor stopped. + * Not populated for `cancelled` (no semantic reason — the user / agent + * just asked to stop) or for `completed` via natural process exit + * (the `exitCode` field carries that signal instead). + * Surfaced in the dialog's `MonitorDetailBody`. + */ + error?: string; } export interface MonitorNotificationMeta { @@ -93,6 +109,25 @@ export type MonitorNotificationCallback = ( export type MonitorRegisterCallback = (entry: MonitorEntry) => void; +/** + * Fires on any change to the registry's contents that a snapshot + * subscriber needs to observe — concretely: `register()` (nothing → + * running), `settle()` (running → terminal: complete / fail / cancel / + * emitEvent's auto-stop at maxEvents / idle timeout), and `reset()` + * (mass clear, fired with no entry). + * + * Does NOT fire on `emitEvent` per se — per-event registry mutations + * (eventCount / droppedLines) are deliberately excluded so the footer + * pill and AppContainer don't churn under heavy event traffic. The + * dialog's detail view re-resolves selected monitor entries from the + * registry directly when it needs live counters. + * + * Symmetric with `BackgroundTaskRegistry.setStatusChangeCallback` and + * `BackgroundShellRegistry.setStatusChangeCallback` so the same UI hook + * can subscribe to all three registries. + */ +export type MonitorStatusChangeCallback = (entry?: MonitorEntry) => void; + interface MonitorCancelOptions { notify?: boolean; } @@ -101,6 +136,7 @@ export class MonitorRegistry { private readonly monitors = new Map(); private notificationCallback?: MonitorNotificationCallback; private registerCallback?: MonitorRegisterCallback; + private statusChangeCallback?: MonitorStatusChangeCallback; register(entry: MonitorEntry): void { if (this.getRunning().length >= MAX_CONCURRENT_MONITORS) { @@ -119,6 +155,11 @@ export class MonitorRegistry { debugLogger.error('Failed to emit register callback:', error); } } + // Mirror BackgroundTaskRegistry / BackgroundShellRegistry: 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); } /** @@ -151,6 +192,10 @@ export class MonitorRegistry { debugLogger.info( `Monitor ${monitorId} reached max events (${entry.maxEvents}), stopping`, ); + // Persist the reason so the dialog's detail view can surface it + // after the monitor terminates (the chat-history notification is + // separate and not visible from /tasks dialog reopens). + entry.error = 'Max events reached'; this.settle(entry, 'completed'); entry.abortController.abort(); this.emitTerminalNotification(entry, 'Max events reached'); @@ -162,6 +207,7 @@ export class MonitorRegistry { const entry = this.monitors.get(monitorId); if (!entry || entry.status !== 'running') return; + if (exitCode !== null) entry.exitCode = exitCode; this.settle(entry, 'completed'); debugLogger.info( `Monitor completed: ${monitorId} (exit ${exitCode}, ${entry.eventCount} events)`, @@ -177,6 +223,7 @@ export class MonitorRegistry { const entry = this.monitors.get(monitorId); if (!entry || entry.status !== 'running') return; + entry.error = error; this.settle(entry, 'failed'); debugLogger.info(`Monitor failed: ${monitorId}: ${error}`); this.emitTerminalNotification(entry, error); @@ -218,6 +265,16 @@ export class MonitorRegistry { this.registerCallback = cb; } + /** + * Subscribe to status transitions (register + every running → terminal + * settle). Single-subscriber on purpose — the dialog hook is the only + * consumer in the codebase, and a list would invite drift in + * error-handling. + */ + setStatusChangeCallback(cb: MonitorStatusChangeCallback | undefined): void { + this.statusChangeCallback = cb; + } + abortAll(options: MonitorCancelOptions = {}): void { for (const entry of Array.from(this.monitors.values())) { this.cancel(entry.monitorId, options); @@ -226,6 +283,7 @@ export class MonitorRegistry { } reset(): void { + if (this.monitors.size === 0) return; for (const entry of this.monitors.values()) { this.clearIdleTimer(entry); if (entry.status === 'running') { @@ -233,6 +291,12 @@ export class MonitorRegistry { } } this.monitors.clear(); + // Notify subscribers that the registry's contents changed wholesale + // — without this, the dialog snapshot in `useBackgroundTaskView` + // would keep rendering the now-cleared rows until an unrelated + // register/settle event happens. Mirrors BackgroundShellRegistry / + // BackgroundTaskRegistry's reset paths. + this.fireStatusChange(); } // --- Internal helpers --- @@ -245,6 +309,16 @@ export class MonitorRegistry { entry.endTime = Date.now(); this.clearIdleTimer(entry); this.pruneTerminalEntries(); + this.fireStatusChange(entry); + } + + private fireStatusChange(entry?: MonitorEntry): void { + if (!this.statusChangeCallback) return; + try { + this.statusChangeCallback(entry); + } catch (error) { + debugLogger.error('statusChange callback failed:', error); + } } private pruneTerminalEntries(): void { @@ -273,6 +347,9 @@ export class MonitorRegistry { ); entry.abortController.abort(); if (entry.status !== 'running') return; + // Same rationale as the max-events branch in `emitEvent`: persist + // the reason so the dialog detail view can show it after settle. + entry.error = 'Idle timeout'; this.settle(entry, 'completed'); this.emitTerminalNotification(entry, 'Idle timeout'); }