diff --git a/packages/cli/src/ui/commands/tasksCommand.test.ts b/packages/cli/src/ui/commands/tasksCommand.test.ts index 420fe98db..c03b5d176 100644 --- a/packages/cli/src/ui/commands/tasksCommand.test.ts +++ b/packages/cli/src/ui/commands/tasksCommand.test.ts @@ -11,6 +11,7 @@ import { createMockCommandContext } from '../../test-utils/mockCommandContext.js import type { BackgroundShellEntry, BackgroundTaskEntry, + MonitorEntry, } from '@qwen-code/qwen-code-core'; type AgentTaskTestEntry = BackgroundTaskEntry & { @@ -46,19 +47,46 @@ function agentEntry( }; } +function monitorEntry(overrides: Partial = {}): MonitorEntry { + return { + monitorId: 'mon-aaaaaaaa', + command: 'tail -f app.log', + description: 'watch app logs', + status: 'running', + startTime: Date.now() - 4_000, + abortController: new AbortController(), + eventCount: 0, + lastEventTime: 0, + maxEvents: 1000, + idleTimeoutMs: 300_000, + droppedLines: 0, + ...overrides, + }; +} + describe('tasksCommand', () => { let context: CommandContext; let getShells: ReturnType; let getAgents: ReturnType; + let getMonitors: ReturnType; beforeEach(() => { getShells = vi.fn().mockReturnValue([]); getAgents = vi.fn().mockReturnValue([]); + getMonitors = vi.fn().mockReturnValue([]); + // Override createMockCommandContext's `'interactive'` default so the + // hint-suppression tests below see the expected default-no-hint + // behaviour. The `shows the dialog hint only in interactive mode` + // and `acp mode` tests rebind their own context with the mode they + // care about; the rest don't assert on the hint so this default + // doesn't affect their expectations. context = createMockCommandContext({ + executionMode: 'non_interactive', services: { config: { getBackgroundShellRegistry: () => ({ getAll: getShells }), getBackgroundTaskRegistry: () => ({ getAll: getAgents }), + getMonitorRegistry: () => ({ getAll: getMonitors }), }, }, } as unknown as Parameters[0]); @@ -157,4 +185,204 @@ describe('tasksCommand', () => { ); expect(result.content).toContain('[bg_shell] running'); }); + + it('lists monitor entries with eventCount and exit / error suffixes', async () => { + getMonitors.mockReturnValue([ + monitorEntry({ + monitorId: 'mon_run', + description: 'tail dev server log', + status: 'running', + eventCount: 12, + pid: 9999, + }), + monitorEntry({ + monitorId: 'mon_done', + description: 'tail short script', + status: 'completed', + exitCode: 0, + eventCount: 7, + endTime: Date.now() - 1_000, + }), + monitorEntry({ + monitorId: 'mon_auto', + description: 'tail noisy log', + status: 'completed', + error: 'Max events reached', + eventCount: 1000, + endTime: Date.now() - 500, + }), + monitorEntry({ + monitorId: 'mon_fail', + description: 'tail bad path', + status: 'failed', + error: 'spawn ENOENT', + eventCount: 0, + endTime: Date.now() - 100, + }), + ]); + + const result = await tasksCommand.action!(context, ''); + if (!result || result.type !== 'message') { + throw new Error('expected message result'); + } + + expect(result.content).toContain('Background tasks (4 total)'); + // Pluralised eventCount + pid for the running monitor. + expect(result.content).toContain('[mon_run] running (12 events)'); + expect(result.content).toContain('pid=9999'); + expect(result.content).toContain('tail dev server log'); + // Natural completion path uses exitCode + eventCount. + expect(result.content).toContain('[mon_done] completed (exit 0, 7 events)'); + // Auto-stop path uses the error string instead of exit code (so users + // see WHY the monitor stopped, not just that it did). + expect(result.content).toContain( + '[mon_auto] completed (Max events reached, 1000 events)', + ); + // Pin the full string (not just the prefix) so a regression that + // drops the `(N events)` suffix from monitor's failed branch fails + // loudly. The other status branches above already have full-string + // assertions; this one was incidentally shorter. + expect(result.content).toContain( + '[mon_fail] failed: spawn ENOENT (0 events)', + ); + // No on-disk output file for monitors — events stream via + // task_notification, so the "output:" line should not appear for + // any monitor entry. + expect(result.content).not.toContain('output: '); + }); + + it('renders cancelled monitor with eventCount suffix', async () => { + getMonitors.mockReturnValue([ + monitorEntry({ + monitorId: 'mon_cancel', + description: 'tail noisy log', + status: 'cancelled', + eventCount: 3, + endTime: Date.now() - 200, + }), + ]); + const result = await tasksCommand.action!(context, ''); + if (!result || result.type !== 'message') throw new Error('no result'); + expect(result.content).toContain('[mon_cancel] cancelled (3 events)'); + }); + + it('singular form: "1 event" not "1 events"', async () => { + getMonitors.mockReturnValue([ + monitorEntry({ monitorId: 'mon_one', eventCount: 1 }), + ]); + const result = await tasksCommand.action!(context, ''); + if (!result || result.type !== 'message') throw new Error('no result'); + expect(result.content).toContain('running (1 event)'); + // Guard against "1 event" matching the prefix of "1 events" by accident. + expect(result.content).not.toContain('1 events'); + }); + + it('shows the dialog hint only in interactive mode', async () => { + getShells.mockReturnValue([entry({ shellId: 'bg_x' })]); + + // non_interactive (default in beforeEach) — no hint. + const noHint = await tasksCommand.action!(context, ''); + if (!noHint || noHint.type !== 'message') throw new Error('no result'); + expect(noHint.content).not.toContain('Tip:'); + + // Re-bind the same config under an interactive context. + const interactiveCtx = createMockCommandContext({ + executionMode: 'interactive', + services: { + config: { + getBackgroundShellRegistry: () => ({ getAll: getShells }), + getBackgroundTaskRegistry: () => ({ getAll: getAgents }), + getMonitorRegistry: () => ({ getAll: getMonitors }), + }, + }, + } as unknown as Parameters[0]); + const withHint = await tasksCommand.action!(interactiveCtx, ''); + if (!withHint || withHint.type !== 'message') throw new Error('no result'); + expect(withHint.content).toContain('Tip:'); + // Pin the actual key path so a regression that goes back to the + // wrong `Ctrl+T` text (which is bound to the MCP descriptions + // toggle, not the Background tasks dialog) fails loudly. + expect(withHint.content).toContain('↓'); + expect(withHint.content).toContain('Enter'); + expect(withHint.content).not.toContain('Ctrl+T'); + expect(withHint.content).toContain('Background tasks (1 total)'); + }); + + it('suppresses the dialog hint in acp mode (no dialog to point at)', async () => { + // ACP / IDE-bridge consumers (Zed, etc.) have no TTY for the + // dialog — same suppression rationale as non_interactive. Pinning + // this so a future regression that switches to `!== 'non_interactive'` + // (which would wrongly show the hint to ACP) fails loudly. + getShells.mockReturnValue([entry({ shellId: 'bg_x' })]); + const acpCtx = createMockCommandContext({ + executionMode: 'acp', + services: { + config: { + getBackgroundShellRegistry: () => ({ getAll: getShells }), + getBackgroundTaskRegistry: () => ({ getAll: getAgents }), + getMonitorRegistry: () => ({ getAll: getMonitors }), + }, + }, + } as unknown as Parameters[0]); + const result = await tasksCommand.action!(acpCtx, ''); + if (!result || result.type !== 'message') throw new Error('no result'); + expect(result.content).not.toContain('Tip:'); + expect(result.content).toContain('Background tasks (1 total)'); + }); + + it('strips ANSI escape sequences and bare C0 control bytes from entry fields', async () => { + // A monitor whose description / command / error contain raw ANSI + // escape codes or bare C0 control bytes (BEL 0x07 audible bell, + // BS 0x08 cursor backspace, FF 0x0C form-feed, ...) must not reach + // the terminal verbatim - a malicious tool description or spawn + // error could otherwise corrupt display, move the cursor, or beep. + // `stripUnsafeCharacters` removes both classes (ANSI sequences via + // strip-ansi, isolated control bytes via the C0/C1 filter) while + // preserving TAB / CR / LF needed for line breaks. + getMonitors.mockReturnValue([ + monitorEntry({ + monitorId: 'mon_evil', + description: 'bad \x1b[2Jthing\x07', // ANSI clear-screen + BEL + status: 'failed', + error: 'spawn \x1b[31mENOENT\x1b[0m\x08\x08', // ANSI colour + 2 BS + eventCount: 0, + }), + ]); + const result = await tasksCommand.action!(context, ''); + if (!result || result.type !== 'message') throw new Error('no result'); + // No raw control bytes survive. + expect(result.content).not.toContain('\x1b'); // ESC + expect(result.content).not.toContain('\x07'); // BEL + expect(result.content).not.toContain('\x08'); // BS + // Surrounding printable text is preserved so the user still gets a + // useful (just safe) message. + expect(result.content).toContain('bad thing'); + expect(result.content).toContain('spawn ENOENT'); + // TAB / CR / LF stay through - the sanitizer must not eat the line + // breaks that separate task entries. + expect(result.content).toContain('\n'); + }); + + it('merges all three kinds and orders them by startTime', async () => { + const now = Date.now(); + getAgents.mockReturnValue([ + agentEntry({ agentId: 'a_late', startTime: now - 1_000 }), + ]); + getShells.mockReturnValue([ + entry({ shellId: 'bg_early', startTime: now - 10_000 }), + ]); + getMonitors.mockReturnValue([ + monitorEntry({ monitorId: 'mon_mid', startTime: now - 5_000 }), + ]); + + const result = await tasksCommand.action!(context, ''); + if (!result || result.type !== 'message') throw new Error('no result'); + const order = ['bg_early', 'mon_mid', 'a_late'].map((id) => + result.content.indexOf(`[${id}]`), + ); + // All present and strictly increasing — proves startTime sort. + expect(order.every((i) => i >= 0)).toBe(true); + expect(order[0]).toBeLessThan(order[1]); + expect(order[1]).toBeLessThan(order[2]); + }); }); diff --git a/packages/cli/src/ui/commands/tasksCommand.ts b/packages/cli/src/ui/commands/tasksCommand.ts index a8c5c99b4..7af02772f 100644 --- a/packages/cli/src/ui/commands/tasksCommand.ts +++ b/packages/cli/src/ui/commands/tasksCommand.ts @@ -8,11 +8,13 @@ import { buildBackgroundEntryLabel, type BackgroundShellEntry, type BackgroundTaskEntry, + type MonitorEntry, } from '@qwen-code/qwen-code-core'; import type { SlashCommand } from './types.js'; import { CommandKind } from './types.js'; import { t } from '../../i18n/index.js'; import { formatDuration } from '../utils/formatters.js'; +import { stripUnsafeCharacters } from '../utils/textUtils.js'; type AgentTaskEntry = BackgroundTaskEntry & { kind: 'agent'; @@ -20,63 +22,158 @@ type AgentTaskEntry = BackgroundTaskEntry & { }; type ShellTaskEntry = BackgroundShellEntry & { kind: 'shell' }; +type MonitorTaskEntry = MonitorEntry & { kind: 'monitor' }; -type TaskEntry = AgentTaskEntry | ShellTaskEntry; +type TaskEntry = AgentTaskEntry | ShellTaskEntry | MonitorTaskEntry; function statusLabel(entry: TaskEntry): string { - if (entry.kind === 'agent') { - switch (entry.status) { - case 'completed': - return 'completed'; - case 'failed': - return `failed: ${entry.error ?? 'unknown error'}`; - case 'cancelled': - return 'cancelled'; - case 'paused': - return entry.resumeBlockedReason - ? `paused (resume blocked): ${entry.resumeBlockedReason}` - : 'paused'; - case 'running': - default: - return 'running'; + switch (entry.kind) { + case 'agent': { + // Bind to a local so the `never`-typed default below operates on a + // narrow-able value. Using `entry.status` directly inside the default + // hits TS narrowing the whole `entry` to `never` after the case arms + // exhaust the discriminated union, breaking the `.status` access. + const status = entry.status; + switch (status) { + case 'completed': + return 'completed'; + case 'failed': + return `failed: ${entry.error ?? 'unknown error'}`; + case 'cancelled': + return 'cancelled'; + case 'paused': + return entry.resumeBlockedReason + ? `paused (resume blocked): ${entry.resumeBlockedReason}` + : 'paused'; + case 'running': + return 'running'; + default: { + const _exhaustive: never = status; + throw new Error( + `statusLabel(agent): unknown status: ${JSON.stringify(_exhaustive)}`, + ); + } + } + } + case 'shell': { + const status = entry.status; + switch (status) { + case 'completed': + return `completed (exit ${entry.exitCode ?? '?'})`; + case 'failed': + return `failed: ${entry.error ?? 'unknown error'}`; + case 'cancelled': + return 'cancelled'; + case 'running': + return 'running'; + default: { + const _exhaustive: never = status; + throw new Error( + `statusLabel(shell): unknown status: ${JSON.stringify(_exhaustive)}`, + ); + } + } + } + case 'monitor': { + // Append eventCount as a glanceable signal for activity. error (set + // on `failed` and on auto-stopped `completed`) is included verbatim. + const events = `${entry.eventCount} event${entry.eventCount === 1 ? '' : 's'}`; + const status = entry.status; + switch (status) { + case 'completed': + return entry.error + ? `completed (${entry.error}, ${events})` + : `completed (exit ${entry.exitCode ?? '?'}, ${events})`; + case 'failed': + return `failed: ${entry.error ?? 'unknown error'} (${events})`; + case 'cancelled': + return `cancelled (${events})`; + case 'running': + return `running (${events})`; + default: { + const _exhaustive: never = status; + throw new Error( + `statusLabel(monitor): unknown status: ${JSON.stringify(_exhaustive)}`, + ); + } + } + } + default: { + const _exhaustive: never = entry; + throw new Error( + `statusLabel: unknown TaskEntry kind: ${JSON.stringify(_exhaustive)}`, + ); } - } - - switch (entry.status) { - case 'completed': - return `completed (exit ${entry.exitCode ?? '?'})`; - case 'failed': - return `failed: ${entry.error ?? 'unknown error'}`; - case 'cancelled': - return 'cancelled'; - case 'running': - return 'running'; - default: - return 'running'; } } function taskLabel(entry: TaskEntry): string { - if (entry.kind === 'agent') { - return buildBackgroundEntryLabel(entry); + switch (entry.kind) { + case 'agent': + return buildBackgroundEntryLabel(entry); + case 'shell': + return entry.command; + case 'monitor': + return entry.description; + default: { + const _exhaustive: never = entry; + throw new Error( + `taskLabel: unknown TaskEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } } - return entry.command; } function taskId(entry: TaskEntry): 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( + `taskId: unknown TaskEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } + } } function taskOutputPath(entry: TaskEntry): string | undefined { - return entry.kind === 'agent' ? entry.outputFile : entry.outputPath; + switch (entry.kind) { + case 'agent': + return entry.outputFile; + case 'shell': + return entry.outputPath; + case 'monitor': + // Monitors stream to the agent via task_notification rather than a + // file on disk — no output path to surface here. + return undefined; + default: { + const _exhaustive: never = entry; + throw new Error( + `taskOutputPath: unknown TaskEntry kind: ${JSON.stringify(_exhaustive)}`, + ); + } + } } export const tasksCommand: SlashCommand = { name: 'tasks', get description() { - return t('List background tasks'); + return t( + 'List background tasks (text dump — interactive dialog opens via the footer pill)', + ); }, kind: CommandKind.BUILT_IN, + // Kept on all three modes: the interactive dialog (reachable via ↓ + + // Enter on the footer Background tasks pill) is the richer surface + // when a TTY is available, but `non_interactive` and `acp` consumers + // (headless `-p`, IDE bridges, SDK) have no dialog and rely on this + // text dump as the only way to inspect background task state. See the + // interactive-mode hint at the top of the output for the soft redirect. supportedModes: ['interactive', 'non_interactive', 'acp'] as const, action: async (context) => { const { config } = context.services; @@ -96,7 +193,11 @@ export const tasksCommand: SlashCommand = { .getBackgroundShellRegistry() .getAll() .map((entry) => ({ ...entry, kind: 'shell' as const })); - const entries = [...agentEntries, ...shellEntries].sort( + const monitorEntries: MonitorTaskEntry[] = config + .getMonitorRegistry() + .getAll() + .map((entry) => ({ ...entry, kind: 'monitor' as const })); + const entries = [...agentEntries, ...shellEntries, ...monitorEntries].sort( (a, b) => a.startTime - b.startTime, ); @@ -109,14 +210,33 @@ export const tasksCommand: SlashCommand = { } const now = Date.now(); - const lines: string[] = [`Background tasks (${entries.length} total)`, '']; + const lines: string[] = []; + // Soft redirect: in interactive mode the dialog is richer (per-entry + // detail view, live updates, cancel keybinding). Don't show the hint + // in non_interactive / acp — those consumers have no dialog to point + // at and the noise just clutters their output. The wording avoids + // pinning a single-key path because Down may pass through the Arena + // agent tab bar first when subagents are present (`InputPrompt` + // focus chain: agent tab bar → bg pill); calling it "the footer + // Background tasks pill" lets the user reach it however the focus + // chain routes them today. + if (context.executionMode === 'interactive') { + lines.push( + t( + 'Tip: focus the Background tasks pill in the footer (use ↓ from an empty composer) and press Enter for the interactive dialog with detail view + live updates.', + ), + '', + ); + } + lines.push(`Background tasks (${entries.length} total)`, ''); for (const entry of entries) { const endTime = entry.endTime ?? now; const runtime = formatDuration(endTime - entry.startTime, { hideTrailingZeros: true, }); const pidPart = - entry.kind === 'shell' && entry.pid !== undefined + (entry.kind === 'shell' || entry.kind === 'monitor') && + entry.pid !== undefined ? ` pid=${entry.pid}` : ''; lines.push( @@ -128,10 +248,24 @@ export const tasksCommand: SlashCommand = { } } + // Defense in depth: registry entries carry user-supplied / process- + // supplied strings (description, command, error from spawn / settle). + // A maliciously-crafted value could otherwise reach the terminal + // verbatim and corrupt display via: + // - ANSI escape sequences (CSI / OSC / SGR — start with ESC) + // - bare C0 control bytes (BEL 0x07 audible bell, BS 0x08 cursor + // back, VT 0x0B, FF 0x0C, …) + // - C1 control bytes (0x80–0x9F) + // - VT control sequences + // `stripUnsafeCharacters` (textUtils.ts) handles all four classes in + // one pass while preserving TAB / CR / LF that we genuinely need + // for line breaks and tabular formatting. Wrapping the joined + // output once covers every field — including any future kind's + // fields — without per-site sanitization sprawl. return { type: 'message' as const, messageType: 'info' as const, - content: lines.join('\n'), + content: stripUnsafeCharacters(lines.join('\n')), }; }, };