From 07fdfadc33f1497803be3378a30088c243acea3f Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 3 May 2026 18:45:51 +0800 Subject: [PATCH] feat(cli): include monitors in /tasks + add interactive-mode hint (#3801) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): include monitors in /tasks + add interactive-mode hint Phase B closure for Issue #3634. Two coupled changes to /tasks: 1. **Bug fix — include monitors.** The command was last touched before #3684 / #3791 landed, so it merged only agent + shell entries while monitors silently disappeared from the headless / non-interactive / ACP listing path. Add a third registry pull from `getMonitorRegistry()` and wire monitor through statusLabel / taskLabel / taskId / taskOutputPath. Status line includes eventCount (`running (N events)`, `completed (exit 0, N events)`, `completed (Max events reached, N events)` for auto-stop) and pid where defined. 2. **Soft deprecation hint, scoped to interactive mode only.** Once the richer Ctrl+T dialog (#3488 + #3720 + #3791) is available, the text dump is the long-form fallback rather than the primary surface. Show `Tip: Ctrl+T opens the interactive Background tasks dialog with detail view + live updates.` at the top of the output when `executionMode === 'interactive'`. Headless / ACP get the bare list — they have no dialog to point at and the hint would just clutter. Description string also clarified to call out the modal split. Kept on all three executionModes (no deletion) — `/tasks` is the only way headless / ACP / SDK consumers can inspect background-task state. Tests: 4 new cases in tasksCommand.test.ts cover monitor entry formatting (running with pid, natural completion with exitCode, auto-stop with error string, failed), the singular `1 event` form, the interactive-mode hint gating, and the cross-kind merge order. * fix(cli): address PR 3801 review — exhaustive switch + i18n + extra tests Three actionable Suggestions from /review's pass: - `taskLabel` rewritten as a `switch` with a `never`-typed `default` arm, matching the structural-safety pattern already used by `taskId`. Adding a 4th DialogEntry kind in the future will now flip both helpers to compile errors instead of letting `taskLabel` silently fall through to `entry.description` (which the new kind may not have). - Hint string wrapped in `t()` for i18n consistency with the rest of the file. The literal stays as the i18n key default, so today's output is unchanged. - Tests: cover `cancelled` monitor status (was the only one without an inline assertion) and explicit `acp` execution mode hint suppression (pins the suppression rationale so a future regression flipping the check to `!== 'non_interactive'` would fail loudly). * fix(cli): correct /tasks dialog-open hint — Ctrl+T was wrong Tmux verification on PR #3801 caught that the hint string says "Ctrl+T opens the interactive Background tasks dialog" but Ctrl+T is actually bound to the MCP tool descriptions toggle (ContextSummaryDisplay.tsx lines 110-115). The dialog opens via Down arrow on an empty composer (focuses the footer pill) followed by Enter (InputPrompt.tsx 947-968). Same misattribution slipped into PR #3791's first description and was caught + fixed there before merge — this PR carried the wrong wording forward in code. Updates four sites: - The hint string itself: "Tip: press ↓ from an empty composer then Enter to open the interactive Background tasks dialog with detail view + live updates." - The slash-command description: "interactive UI is Ctrl+T" → "interactive dialog opens via the footer pill" - Two inline comments referencing Ctrl+T as the dialog opener - The interactive-mode hint test now pins on `↓` + `Enter` and asserts `not.toContain('Ctrl+T')` so a regression to the wrong wording fails loudly. * fix(cli): address PR 3801 review — exhaustive switch consistency + path-agnostic hint Four Suggestions from the latest /review pass: - `statusLabel` rewritten as a single top-level switch with a `never`-typed default, matching `taskLabel` / `taskId` / `taskOutputPath`. The previous `if`/`if`/fallthrough form would silently apply monitor formatting to a future 4th kind. - `taskOutputPath` gained the same exhaustive default — was the only per-kind helper still relying on implicit fallthrough; would silently omit a 4th-kind output path while the adjacent helpers flip to compile errors. - Hint wording de-specifies the exact keystroke count: `'Tip: focus the Background tasks pill in the footer (use ↓ from an empty composer) and press Enter ...'`. Previous "press ↓ then Enter" phrasing was wrong when the Arena agent tab bar is present — `InputPrompt`'s focus chain routes Down through the tab bar first, so a single Down lands there, not on the bg pill. - Test pin tightened: `[mon_fail] failed: spawn ENOENT (0 events)` is now a full-string assertion instead of a prefix match, so a regression that drops the `(N events)` suffix from monitor's failed branch fails loudly. * fix(cli): sanitize ANSI escape sequences in /tasks output deepseek's review pass flagged that monitor description / error fields are user / process-supplied strings rendered directly to the terminal. A maliciously-crafted tool description or spawn error containing raw ANSI control sequences (clear-screen, cursor-move, colour) would otherwise reach stdout verbatim and corrupt display. Same risk applies to agent error / description and shell error / command — all already-existing renderers with the same exposure that this PR didn't introduce but inherits. So instead of per-field sprinkling, wrap the joined output once with `escapeAnsiCtrlCodes` (no-op when no control chars present, so cost is zero in the common case). One line change in the renderer covers every kind including any future one. Test pins the behaviour: a monitor entry with `\x1b[2J` / `\x1b[31m...` content produces output with no raw ESC bytes and visible escaped `[...]` sequences. * docs(cli): tighten escapeAnsiCtrlCodes comments to match actual scope Two doc-precision Suggestions from copilot's pass on 0840e32f6: - Source comment claimed `escapeAnsiCtrlCodes` is "a no-op when no control chars" but it's narrower than that — it only handles sequences matched by `ansi-regex` (CSI / OSC / SGR — anything starting with ESC). Isolated C0/C1 control bytes like BEL, BS, VT pass through untouched. Updated the comment to enumerate the actual scope and call out that `node:util`'s `stripVTControlCharacters` would be needed if those become a concern. - Test comment had a literal raw ESC byte (octal 033) embedded in the source — visually showed `^[[...]` in editors that render ESC, but was a real ESC byte in the file rather than the escaped `` form the sanitizer produces. Rewrote with a literal `` text description so what the comment shows matches what the assertions check for. * fix(cli): broaden /tasks sanitization + tighten inner switch exhaustiveness Addresses 3 of 5 items from doudouOUC's PR 3801 review: - **Issue 1 (Low) — C0/C1 control byte gap**: switched from `escapeAnsiCtrlCodes` (only handles ESC-initiated ANSI sequences) to `stripUnsafeCharacters` (one-pass strip of ANSI + VT + C0/C1, with TAB/CR/LF preserved). The pre-existing exposure to bare BEL / BS / FF / VT bytes via shell entry strings is now closed for all three kinds. Test rewritten to cover both ANSI sequences AND bare control bytes (BEL, BS), and pins that surrounding printable text and line breaks survive. - **Issue 2 (Low) — inner status switches inconsistent**: the three inner `switch (entry.status)` blocks (agent / shell / monitor) used `case 'running': default: return 'running'` (or duplicated bodies). All three now have explicit `running` cases plus a `never`-typed default that throws — matches the outer `switch (entry.kind)` pattern and means a future status added to any of `BackgroundTaskEntry` / `BackgroundShellEntry` / `MonitorStatus` flips to a compile error here instead of silently returning `'running'`. - **Issue 5 (Nit) — beforeEach default change**: added an inline comment explaining why the test default overrides `createMockCommandContext`'s `'interactive'` default (`'non_interactive'` lets the hint-suppression assertions work without each test rebinding context). Issues 3 and 4 from the review are nits with no action needed (3 is already documented as intentional; 4 is a UX call about hint length that's better handled by user feedback than guess-tweaking). * fix(cli): bind status to local before exhaustive switch — fixes tsc build CI's `tsc --build` (full mode, vs `--noEmit` locally) caught that `switch (entry.status)` followed by a `never`-typed default reading `entry.status` doesn't compile. After the case arms exhaust the discriminated union, TS narrows `entry` itself to `never`, so the `.status` access in the default arm becomes "Property 'status' does not exist on type 'never'" + the resulting `any` value can't be assigned to `never`. Fix: bind `entry.status` to a local `status` const before the inner switch. The local stays typed as the per-kind status union and narrows correctly to `never` at the default arm — `const _exhaustive: never = status` is then `never = never`, valid. Standard exhaustive-switch-on-discriminator pattern; doesn't change runtime behavior or test surface, just gets past TS narrowing on the nested case. --- .../cli/src/ui/commands/tasksCommand.test.ts | 228 ++++++++++++++++++ packages/cli/src/ui/commands/tasksCommand.ts | 212 +++++++++++++--- 2 files changed, 401 insertions(+), 39 deletions(-) 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')), }; }, };