qwen-code/packages/cli
Shaojin Wen 07fdfadc33
feat(cli): include monitors in /tasks + add interactive-mode hint (#3801)
* 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.
2026-05-03 18:45:51 +08:00
..
src feat(cli): include monitors in /tasks + add interactive-mode hint (#3801) 2026-05-03 18:45:51 +08:00
index.ts fix(cli): stop double-wrapping and double-printing API errors in non-interactive mode (#3749) 2026-05-03 08:39:31 +08:00
package.json chore(release): v0.15.6 (#3766) 2026-04-30 15:59:35 +08:00
test-setup.ts fix: prevent bogus shell permission rules in tests 2026-03-20 17:55:33 +08:00
tsconfig.json Add background agent resume and continuation (#3739) 2026-05-01 12:14:33 +08:00
vitest.config.ts refactor(core): Unify package exports and improve dev experience 2026-02-01 11:59:05 +08:00