mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-10 20:30:13 +00:00
feat(cli): wire Monitor entries into combined Background tasks dialog (#3791)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): wire Monitor entries into combined Background tasks dialog
Phase C mirror follow-up of Issue #3634, structurally a clean repeat of
#3720 but for Monitor (third consumer of the kind framework).
Core (MonitorRegistry):
- Add `setStatusChangeCallback` mirroring `BackgroundShellRegistry` /
`BackgroundTaskRegistry`. Fires synchronously inside `register()` (so
subscribers see lifecycle start) and inside `settle()` (so subscribers
see every running → terminal transition: complete / fail / cancel /
emitEvent's auto-stop at maxEvents).
- Subscriber failures are caught and logged but do not poison the
registry — same defensive contract as the other two registries.
CLI:
- `useBackgroundTaskView` subscribes to all three registries (agent +
shell + monitor) and merges by `startTime`. `DialogEntry` union
extended with `(MonitorEntry & { kind: 'monitor' })`. `entryId`
switches over the three kinds and returns the right id field.
- `BackgroundTasksPill.getPillLabel` adds monitor to its KIND_NAMES
table; grouping order is shell → agent → monitor (monitor last
because it tends to be the longest-lived, least urgent to glance at).
- `BackgroundTasksDialog`:
- `rowLabel` returns `[monitor] <description>` for monitor rows.
- New `MonitorDetailBody` showing command / status / pid / event
count / dropped lines. No Progress block (monitors don't fire
activity callbacks per-event).
- DetailBody dispatcher gains the monitor branch.
- `BackgroundTaskViewContext.cancelSelected` routes monitor cancels via
`monitorRegistry.cancel(monitorId)`. Monitor's cancel is synchronous
(settle + abort happen inside the registry), matching the same path
task_stop already uses.
Tests: 6 new core tests (registry callbacks fire on register / each
terminal transition / not on emitEvent / on auto-stop / clear stops
notifications / subscriber-failure isolation), 4 new pill tests
(singular / plural / 3-kind grouping / running-only filter), dialog
mock extended with `getMonitorRegistry`.
* fix(cli): add exhaustive default arms to DialogEntry switches
ESLint's `default-case` rule requires every switch to have a default
arm even when TypeScript can prove the union is exhaustive. Add
`default: { const _exhaustive: never = entry; throw ... }` to the
five switches added in this PR — same pattern keeps both the runtime
guard and the compile-time exhaustiveness check.
* fix(core): fire statusChange in MonitorRegistry.reset()
The newly-added `setStatusChangeCallback` subscriber misses `reset()`,
so a `/clear` or session reset leaves stale monitor rows visible in the
combined Background tasks dialog until an unrelated register/settle
event happens. Both BackgroundShellRegistry and BackgroundTaskRegistry
already fire statusChange on their reset paths — Monitor was the
outlier.
Fix: fire `statusChange()` (no arg) after `monitors.clear()`, with an
early return when the registry is already empty so we don't notify on
a no-op reset. Two new tests cover both branches.
* fix(cli,core): address PR 3791 review feedback
Four review threads from /review's second pass on top of f26b700:
1. **MonitorDetailBody live counters were stale.** `eventCount` and
`droppedLines` came from the `useBackgroundTaskView` snapshot, which
only refreshes on `statusChange`, and `emitEvent` deliberately
doesn't fire `statusChange` (the per-event churn would burn the pill
/ AppContainer). Fix: extend the existing `selectedEntry` useMemo to
re-resolve monitors from `monitorRegistry.get()` on each
`activityTick`, mirroring the agent path. The pre-existing 1s
wall-clock interval already drives the recompute while the entry is
running, so no new callback wiring is needed.
2. **Settle reasons weren't persisted on `MonitorEntry`.** `fail()`,
`complete(exitCode)`, max-events auto-stop, and idle-timeout all
passed their reason strings only to the chat-history terminal
notification — opening the dialog detail view after the monitor
died showed the bare status with no clue why. Add `exitCode?` and
`error?` fields (mirrors `BackgroundShellEntry`); populate them
before `settle()` in each path; render them in `MonitorDetailBody`
with appropriate colour (red for `failed`, warning for
auto-stopped `completed`).
3. **`monitorCancel` mock had no test asserting it.** Add a dedicated
test that opens detail on a monitor entry, presses `x`, and verifies
`monitorRegistry.cancel(monitorId)` was called and the agent
registry's cancel was NOT called. Pins the kind switch in
`cancelSelected` so a regression flipping the monitor branch to
anything else (e.g. `requestCancel`) would fail loudly.
4. **`MonitorStatusChangeCallback` docstring was wrong.** It claimed
the callback fires on running → terminal transitions, but
`register()` (nothing → running) and `reset()` (mass clear, fired
with no entry) also fire it. Rewrite the docstring to enumerate the
actual fire sites and explicitly note that `emitEvent` is excluded
(per-event churn).
* docs(cli,core): tighten MonitorEntry.error and rowLabel comments
Two doc-precision fixes from copilot's PR 3791 review pass:
- `MonitorEntry.error` enumerated max-events as the only auto-stop
reason that populates the field, but `idle-timeout` also writes it
(was added in the same earlier commit). Rewrote to list both current
reasons and explicitly note: any future auto-stop reason should
populate this field too. Also clarified that `cancelled` and
natural-exit `completed` paths intentionally don't.
- `rowLabel`'s shell-branch comment claimed long commands are
"truncated by the row renderer's MaxSizedBox", but ListBody renders
rows with plain `<Text>` (no MaxSizedBox / truncation helper). Long
text wraps. Rewrote to describe actual behaviour and note the
intentional decision to leave it wrapping (the dialog is what users
open to see context — truncating defeats the purpose).
* test(cli): cover MonitorDetailBody render branches + useBackgroundTaskView
Two coverage gaps from /review's third pass on PR 3791:
- New file `useBackgroundTaskView.test.ts` (6 cases) directly exercises
the merge logic with a stub config: empty when config is null, merges
three registries, sorts by startTime, tags `kind`, subscribes on
mount, refreshes when any registry fires statusChange, clears all
three subscriptions on unmount.
- New `MonitorDetailBody render branches` describe block in
`BackgroundTasksDialog.test.tsx` (8 cases) covers the conditional
pieces my last commit added: title + Command, pid show/hide,
eventCount singular vs plural, droppedLines show/hide, exitCode
display, Error block (failed) vs Stopped because (auto-stop), and
the all-undefined no-block case.
This commit is contained in:
parent
a08d48b75c
commit
cdadbcdb33
9 changed files with 791 additions and 49 deletions
|
|
@ -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<AgentDialogEntry> = {}): DialogEntry {
|
|||
} as DialogEntry;
|
||||
}
|
||||
|
||||
function monitorEntry(overrides: Partial<DialogEntry> = {}): 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<typeof useBackgroundTaskViewActions>;
|
||||
state: ReturnType<typeof useBackgroundTaskViewState>;
|
||||
|
|
@ -61,6 +93,7 @@ interface Harness {
|
|||
cancel: ReturnType<typeof vi.fn>;
|
||||
resume: ReturnType<typeof vi.fn>;
|
||||
abandon: ReturnType<typeof vi.fn>;
|
||||
monitorCancel: ReturnType<typeof vi.fn>;
|
||||
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<DialogEntry> = {}) {
|
||||
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<DialogEntry>),
|
||||
).toContain('pid 4242');
|
||||
expect(openMonitorDetail()).not.toContain('pid ');
|
||||
});
|
||||
|
||||
it('uses singular "1 event" / plural "N events"', () => {
|
||||
const f1 = openMonitorDetail({ eventCount: 1 } as Partial<DialogEntry>);
|
||||
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<DialogEntry>);
|
||||
expect(f5).toContain('5 events');
|
||||
});
|
||||
|
||||
it('renders droppedLines only when > 0', () => {
|
||||
expect(
|
||||
openMonitorDetail({ droppedLines: 0 } as Partial<DialogEntry>),
|
||||
).not.toContain('dropped');
|
||||
expect(
|
||||
openMonitorDetail({ droppedLines: 3 } as Partial<DialogEntry>),
|
||||
).toContain('3 dropped');
|
||||
});
|
||||
|
||||
it('renders exitCode in subtitle when defined', () => {
|
||||
expect(
|
||||
openMonitorDetail({
|
||||
status: 'completed',
|
||||
exitCode: 0,
|
||||
} as Partial<DialogEntry>),
|
||||
).toContain('exit 0');
|
||||
expect(
|
||||
openMonitorDetail({
|
||||
status: 'completed',
|
||||
exitCode: 1,
|
||||
} as Partial<DialogEntry>),
|
||||
).toContain('exit 1');
|
||||
});
|
||||
|
||||
it('renders Error block for failed status', () => {
|
||||
const f = openMonitorDetail({
|
||||
status: 'failed',
|
||||
error: 'spawn ENOENT',
|
||||
} as Partial<DialogEntry>);
|
||||
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<DialogEntry>);
|
||||
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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 `<Text>`, 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] <command>`. 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' ? (
|
||||
<AgentDetailBody entry={entry} maxHeight={maxHeight} maxWidth={maxWidth} />
|
||||
) : (
|
||||
<ShellDetailBody entry={entry} maxHeight={maxHeight} maxWidth={maxWidth} />
|
||||
);
|
||||
}> = ({ entry, maxHeight, maxWidth }) => {
|
||||
switch (entry.kind) {
|
||||
case 'agent':
|
||||
return (
|
||||
<AgentDetailBody
|
||||
entry={entry}
|
||||
maxHeight={maxHeight}
|
||||
maxWidth={maxWidth}
|
||||
/>
|
||||
);
|
||||
case 'shell':
|
||||
return (
|
||||
<ShellDetailBody
|
||||
entry={entry}
|
||||
maxHeight={maxHeight}
|
||||
maxWidth={maxWidth}
|
||||
/>
|
||||
);
|
||||
case 'monitor':
|
||||
return (
|
||||
<MonitorDetailBody
|
||||
entry={entry}
|
||||
maxHeight={maxHeight}
|
||||
maxWidth={maxWidth}
|
||||
/>
|
||||
);
|
||||
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 (
|
||||
<MaxSizedBox
|
||||
maxHeight={maxHeight}
|
||||
maxWidth={maxWidth}
|
||||
overflowDirection="bottom"
|
||||
>
|
||||
<Box>
|
||||
<Text bold color={theme.text.accent}>
|
||||
{title}
|
||||
</Text>
|
||||
</Box>
|
||||
<Box>
|
||||
{terminal && (
|
||||
<Text color={terminal.color}>
|
||||
{`${terminal.icon} ${STATUS_VERBS[entry.status]} · `}
|
||||
</Text>
|
||||
)}
|
||||
<Text color={theme.text.secondary}>{dimSubtitleParts.join(' · ')}</Text>
|
||||
</Box>
|
||||
|
||||
<Box />
|
||||
<Box>
|
||||
<Text bold dimColor>
|
||||
Command
|
||||
</Text>
|
||||
</Box>
|
||||
<Box>
|
||||
<Text wrap="truncate-end">{entry.command}</Text>
|
||||
</Box>
|
||||
|
||||
{hasError && (
|
||||
<Fragment>
|
||||
<Box />
|
||||
<Box>
|
||||
<Text bold color={errorColor}>
|
||||
{errorIsFailure ? 'Error' : 'Stopped because'}
|
||||
</Text>
|
||||
</Box>
|
||||
<Box>
|
||||
<Text color={errorColor} wrap="wrap">
|
||||
{entry.error}
|
||||
</Text>
|
||||
</Box>
|
||||
</Fragment>
|
||||
)}
|
||||
</MaxSizedBox>
|
||||
);
|
||||
};
|
||||
|
||||
// ─── Dialog shell ──────────────────────────────────────────
|
||||
|
||||
interface BackgroundTasksDialogProps {
|
||||
|
|
@ -522,15 +642,30 @@ export const BackgroundTasksDialog: React.FC<BackgroundTasksDialogProps> = ({
|
|||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -34,6 +34,24 @@ function shellEntry(overrides: Partial<DialogEntry> = {}): DialogEntry {
|
|||
} as DialogEntry;
|
||||
}
|
||||
|
||||
function monitorEntry(overrides: Partial<DialogEntry> = {}): 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([
|
||||
|
|
|
|||
|
|
@ -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(', ');
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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]);
|
||||
|
||||
|
|
|
|||
182
packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts
Normal file
182
packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts
Normal file
|
|
@ -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<typeof vi.fn>;
|
||||
/** 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<ReturnType<typeof agent>> = [];
|
||||
const monitors: Array<ReturnType<typeof monitor>> = [];
|
||||
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],
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -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]);
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -76,6 +76,22 @@ export interface MonitorEntry {
|
|||
idleTimeoutMs: number;
|
||||
idleTimer?: ReturnType<typeof setTimeout>;
|
||||
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<string, MonitorEntry>();
|
||||
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');
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue