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.
This commit is contained in:
Shaojin Wen 2026-05-03 12:21:36 +08:00
parent 965e02cb82
commit 040dab2ada
2 changed files with 83 additions and 57 deletions

View file

@ -232,7 +232,13 @@ describe('tasksCommand', () => {
expect(result.content).toContain(
'[mon_auto] completed (Max events reached, 1000 events)',
);
expect(result.content).toContain('[mon_fail] failed: spawn ENOENT');
// 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.

View file

@ -26,56 +26,62 @@ type MonitorTaskEntry = MonitorEntry & { kind: 'monitor' };
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':
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';
}
case 'shell':
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';
}
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'}`;
switch (entry.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:
return `running (${events})`;
}
}
}
if (entry.kind === 'shell') {
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';
default: {
const _exhaustive: never = entry;
throw new Error(
`statusLabel: unknown TaskEntry kind: ${JSON.stringify(_exhaustive)}`,
);
}
}
// 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'}`;
switch (entry.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:
return `running (${events})`;
}
}
function taskLabel(entry: TaskEntry): string {
@ -113,11 +119,22 @@ function taskId(entry: TaskEntry): string {
}
function taskOutputPath(entry: TaskEntry): string | undefined {
if (entry.kind === 'agent') return entry.outputFile;
if (entry.kind === 'shell') return entry.outputPath;
// Monitors stream to the agent via task_notification rather than a
// file on disk — no output path to surface here.
return undefined;
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 = {
@ -174,13 +191,16 @@ export const tasksCommand: SlashCommand = {
// 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. Note: the dialog
// opens via Down arrow + Enter on the footer pill (NOT Ctrl+T —
// that's bound to the MCP tool descriptions toggle elsewhere).
// 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: press ↓ from an empty composer then Enter to open the interactive Background tasks dialog with detail view + live updates.',
'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.',
),
'',
);