diff --git a/packages/cli/src/ui/components/Footer.tsx b/packages/cli/src/ui/components/Footer.tsx index dde9c28c5b7..b0539f36095 100644 --- a/packages/cli/src/ui/components/Footer.tsx +++ b/packages/cli/src/ui/components/Footer.tsx @@ -5,7 +5,6 @@ */ import type React from 'react'; -import { useCallback, useSyncExternalStore } from 'react'; import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; import { ContextUsageDisplay } from './ContextUsageDisplay.js'; @@ -25,39 +24,12 @@ import { ApprovalMode } from '@qwen-code/qwen-code-core'; import { GeminiSpinner } from './GeminiRespondingSpinner.js'; import { t } from '../../i18n/index.js'; -/** - * Returns true while any dream task for the current project is in - * 'pending' or 'running' state. Uses MemoryManager's subscribe/notify - * mechanism so there is zero polling overhead. - */ -function useDreamRunning(projectRoot: string): boolean { - const config = useConfig(); - - const subscribe = useCallback( - (onStoreChange: () => void) => - config.getMemoryManager().subscribe(onStoreChange), - [config], - ); - - const getSnapshot = useCallback( - () => - config - .getMemoryManager() - .listTasksByType('dream', projectRoot) - .some((task) => task.status === 'pending' || task.status === 'running'), - [config, projectRoot], - ); - - return useSyncExternalStore(subscribe, getSnapshot); -} - export const Footer: React.FC = () => { const uiState = useUIState(); const config = useConfig(); const { vimEnabled, vimMode } = useVimMode(); const { lines: statusLineLines } = useStatusLine(); const configInitMessage = useConfigInitMessage(uiState.isConfigInitialized); - const dreamRunning = useDreamRunning(config.getProjectRoot()); const { promptTokenCount, showAutoAcceptIndicator } = { promptTokenCount: uiState.sessionStats.lastPromptTokenCount, @@ -134,12 +106,10 @@ export const Footer: React.FC = () => { node: Debug Mode, }); } - if (dreamRunning) { - rightItems.push({ - key: 'dream', - node: {t('✦ dreaming')}, - }); - } + // Dream tasks now surface via the BackgroundTasksPill (e.g. "1 dream") + // alongside the other background-task kinds. The previous `✦ dreaming` + // right-column indicator was removed to avoid two simultaneous signals + // for the same underlying state. if (promptTokenCount > 0 && contextWindowSize) { rightItems.push({ key: 'context', diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx index dc6372526f1..5be5e82460e 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -18,6 +18,7 @@ import { import { ConfigContext } from '../../contexts/ConfigContext.js'; import { type AgentDialogEntry, + type DreamDialogEntry, useBackgroundTaskView, type DialogEntry, } from '../../hooks/useBackgroundTaskView.js'; @@ -36,6 +37,8 @@ vi.mock('../../hooks/useBackgroundTaskView.js', () => ({ return entry.shellId; case 'monitor': return entry.monitorId; + case 'dream': + return entry.dreamId; default: { const _exhaustive: never = entry; throw new Error( @@ -53,7 +56,7 @@ vi.mock('../../hooks/useKeypress.js', () => ({ const mockedUseBackgroundTaskView = vi.mocked(useBackgroundTaskView); const mockedUseKeypress = vi.mocked(useKeypress); -function entry(overrides: Partial = {}): DialogEntry { +function entry(overrides: Partial = {}): AgentDialogEntry { return { kind: 'agent', agentId: 'a', @@ -62,7 +65,21 @@ function entry(overrides: Partial = {}): DialogEntry { startTime: 0, abortController: new AbortController(), ...overrides, - } as DialogEntry; + } as AgentDialogEntry; +} + +function dreamEntry( + overrides: Partial = {}, +): DreamDialogEntry { + return { + kind: 'dream', + dreamId: 'd-1', + status: 'running', + startTime: 0, + sessionCount: 7, + progressText: 'Scheduled managed auto-memory dream.', + ...overrides, + }; } function monitorEntry(overrides: Partial = {}): DialogEntry { @@ -94,6 +111,7 @@ interface Harness { resume: ReturnType; abandon: ReturnType; monitorCancel: ReturnType; + dreamCancelTask: ReturnType; setEntries: (next: readonly DialogEntry[]) => void; pressKey: (key: { name?: string; sequence?: string }) => void; call: (fn: () => void) => void; @@ -112,6 +130,7 @@ function setup(initial: readonly DialogEntry[]): Harness { const resume = vi.fn(); const abandon = vi.fn(); const monitorCancel = vi.fn(); + const dreamCancelTask = 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. @@ -138,6 +157,9 @@ function setup(initial: readonly DialogEntry[]): Harness { return match; }, }), + getMemoryManager: () => ({ + cancelTask: dreamCancelTask, + }), resumeBackgroundAgent: resume, abandonBackgroundAgent: abandon, } as unknown as Config; @@ -182,6 +204,7 @@ function setup(initial: readonly DialogEntry[]): Harness { resume, abandon, monitorCancel, + dreamCancelTask, setEntries(next) { handlers.length = 0; currentEntries = next; @@ -447,4 +470,119 @@ describe('BackgroundTasksDialog', () => { expect(f).not.toContain('Stopped because'); }); }); + + describe('dream entries', () => { + // Coverage for the dream task kind in the unified pill / dialog + // plumbing — list rendering, detail body, hint visibility, and + // cancellation routing. Mirrors the agent / shell / monitor + // coverage profile so each kind has parity in this test file. + it('renders the [dream] row with session count in list mode', () => { + const h = setup([dreamEntry({ sessionCount: 7 })]); + h.call(() => h.probe.current!.actions.openDialog()); + + const f = h.lastFrame() ?? ''; + expect(f).toContain('[dream]'); + expect(f).toContain('memory consolidation'); + expect(f).toContain('reviewing 7 sessions'); + }); + + it('renders DreamDetailBody with sessions / progress / topics on detail view', () => { + const h = setup([ + dreamEntry({ + status: 'completed', + sessionCount: 5, + progressText: 'Managed auto-memory dream completed.', + touchedTopics: ['user', 'project', 'feedback'], + }), + ]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + + const f = h.lastFrame() ?? ''; + expect(f).toContain('Dream'); + expect(f).toContain('Sessions reviewing'); + expect(f).toContain('5'); + expect(f).toContain('Progress'); + expect(f).toContain('Managed auto-memory dream completed.'); + expect(f).toContain('Topics touched (3)'); + expect(f).toContain('user'); + expect(f).toContain('project'); + expect(f).toContain('feedback'); + }); + + it('shows the "x stop" hint for a running dream entry', () => { + const h = setup([dreamEntry({ status: 'running' })]); + h.call(() => h.probe.current!.actions.openDialog()); + const f = h.lastFrame() ?? ''; + expect(f).toContain('x stop'); + }); + + it("routes 'x' on a running dream to MemoryManager.cancelTask(dreamId)", () => { + // Pin the dream-cancel branch in `cancelSelected` — flipping it + // to anything else (e.g. shell's `requestCancel`) would silently + // break the only path the user has to stop a runaway dream + // consolidation, since the hint already advertises the action. + const h = setup([dreamEntry({ dreamId: 'd-zzz', status: 'running' })]); + h.call(() => h.probe.current!.actions.openDialog()); + h.pressKey({ sequence: 'x' }); + expect(h.dreamCancelTask).toHaveBeenCalledWith('d-zzz'); + // Belt-and-braces — the registry-side cancel paths must not fire + // for a dream entry, otherwise the wrong AbortController gets + // signalled. + expect(h.cancel).not.toHaveBeenCalled(); + expect(h.monitorCancel).not.toHaveBeenCalled(); + }); + + it('omits the topics block entirely while the dream is still running', () => { + // Topics only get populated via metadata.touchedTopics on + // completion; mid-run the body should hide the section instead of + // rendering an empty header. + const h = setup([dreamEntry({ status: 'running', touchedTopics: [] })]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const f = h.lastFrame() ?? ''; + expect(f).not.toContain('Topics touched'); + }); + + it('renders the Error block on failed status with a "+ Stopped because" verb', () => { + // Dream failures need to surface — they are the user's only signal + // that consolidation didn't happen as expected (success path + // already produces a memory_saved toast in useGeminiStream). + const h = setup([ + dreamEntry({ + status: 'failed', + error: 'Dream agent failed: model timeout', + }), + ]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const f = h.lastFrame() ?? ''; + expect(f).toContain('Failed'); + expect(f).toContain('Error'); + expect(f).toContain('Dream agent failed: model timeout'); + }); + + it('caps visible topics at 8 and renders a "+N more" tail for overflow', () => { + // Real consolidations can touch many memory files; the body must + // not push the hint footer off-screen. Cap mirrors MAX_TOPICS in + // DreamDetailBody. + const manyTopics = Array.from({ length: 12 }, (_, i) => `topic-${i + 1}`); + const h = setup([ + dreamEntry({ status: 'completed', touchedTopics: manyTopics }), + ]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const f = h.lastFrame() ?? ''; + // First 8 visible. + expect(f).toContain('topic-1'); + expect(f).toContain('topic-8'); + // Past the cap — must NOT be inlined. + expect(f).not.toContain('topic-9'); + expect(f).not.toContain('topic-12'); + // Tail summary. + expect(f).toContain('+4 more'); + // Header still reflects the full count, not the capped slice. + expect(f).toContain('Topics touched (12)'); + }); + }); }); diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 3bf40798f15..f3701fb0f5e 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -33,6 +33,7 @@ import { formatDuration, formatTokenCount } from '../../utils/formatters.js'; import { type AgentDialogEntry, type DialogEntry, + type DreamDialogEntry, entryId, } from '../../hooks/useBackgroundTaskView.js'; @@ -118,6 +119,13 @@ function rowLabel(entry: DialogEntry): string { return `[shell] ${entry.command}`; case 'monitor': return `[monitor] ${entry.description}`; + case 'dream': { + const sessionsHint = + entry.sessionCount !== undefined + ? ` reviewing ${entry.sessionCount} session${entry.sessionCount === 1 ? '' : 's'}` + : ''; + return `[dream] memory consolidation${sessionsHint}`; + } default: { const _exhaustive: never = entry; throw new Error( @@ -282,6 +290,14 @@ const DetailBody: React.FC<{ maxWidth={maxWidth} /> ); + case 'dream': + return ( + + ); default: { const _exhaustive: never = entry; throw new Error( @@ -291,6 +307,189 @@ const DetailBody: React.FC<{ } }; +// ─── Dream detail body ───────────────────────────────────── +// +// Shows what the agent is reviewing (session count), what it has +// touched (topic files, only populated on completion), and the latest +// progress text from MemoryManager. Cancellation is wired through the +// shared `x stop` keystroke (handled by `cancelSelected` in the +// context, which routes dream entries to `MemoryManager.cancelTask`). +// In-flight progress is still static — the dream's fork agent reports +// only at schedule + completion via MemoryManager.update; live +// per-turn phase reporting requires extending runForkedAgent's +// AgentPathParams with an onAssistantMessage callback (separate PR). +// +// Layout follows the Shell/Monitor convention — flat children of +// MaxSizedBox separated by empty `` spacers (nesting a +// `flexDirection="column"` container inside MaxSizedBox eats the +// children silently). +const DreamDetailBody: React.FC<{ + entry: DreamDialogEntry; + maxHeight: number; + maxWidth: number; +}> = ({ entry, maxHeight, maxWidth }) => { + const title = 'Dream'; + const terminal = terminalStatusPresentation(entry.status); + const dimSubtitleParts: string[] = [elapsedFor(entry)]; + if (entry.sessionCount !== undefined) { + dimSubtitleParts.push( + `${entry.sessionCount} session${entry.sessionCount === 1 ? '' : 's'}`, + ); + } + if (entry.touchedTopics && entry.touchedTopics.length > 0) { + dimSubtitleParts.push( + `${entry.touchedTopics.length} topic${entry.touchedTopics.length === 1 ? '' : 's'}`, + ); + } + + // Topic file lists can grow for an active session sweep; cap the + // displayed slice and add a "+N more" tail rather than letting the + // dialog body push the hint footer off-screen. + const MAX_TOPICS = 8; + const topics = entry.touchedTopics ?? []; + const visibleTopics = topics.slice(0, MAX_TOPICS); + const hiddenTopicCount = Math.max(0, topics.length - visibleTopics.length); + const hasError = Boolean(entry.error); + + return ( + + + + {title} + + + + {terminal && ( + + {`${terminal.icon} ${STATUS_VERBS[entry.status]} · `} + + )} + {dimSubtitleParts.join(' · ')} + + + {entry.sessionCount !== undefined && ( + + + + + Sessions reviewing + + + + {String(entry.sessionCount)} + + + )} + + {entry.progressText && ( + + + + + Progress + + + + {entry.progressText} + + + )} + + {topics.length > 0 && ( + + + + + {`Topics touched (${topics.length})`} + + + {visibleTopics.map((topic) => ( + + {` · ${topic}`} + + ))} + {hiddenTopicCount > 0 && ( + + {` · +${hiddenTopicCount} more`} + + )} + + )} + + {hasError && ( + + + + + Error + + + + + {entry.error} + + + + )} + + {/* + Lock-release / metadata-write warnings on a successfully- + completed dream. Rendered as warnings (not errors) so the + terminal status stays Completed; explains why subsequent + dreams may be silently skipped as 'locked' (lock release + failure) or why the scheduler gate isn't picking up the + latest run (metadata write failure). + */} + {entry.lockReleaseError && ( + + + + + Lock release warning + + + + + {entry.lockReleaseError} + + + + + {`Subsequent dreams may be skipped as locked until the next session's staleness sweep cleans the file.`} + + + + )} + {entry.metadataWriteError && ( + + + + + Metadata write warning + + + + + {entry.metadataWriteError} + + + + + {`The scheduler gate did not see this dream's timestamp; the next dream cycle may re-fire sooner than usual.`} + + + + )} + + ); +}; + const AgentDetailBody: React.FC<{ entry: AgentDialogEntry; maxHeight: number; diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx index 5721d890e0f..2647056e787 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx @@ -34,6 +34,17 @@ function shellEntry(overrides: Partial = {}): DialogEntry { } as DialogEntry; } +function dreamEntry(overrides: Partial = {}): DialogEntry { + return { + kind: 'dream', + dreamId: 'd-1', + status: 'running', + startTime: 0, + sessionCount: 5, + ...overrides, + } as DialogEntry; +} + function monitorEntry(overrides: Partial = {}): DialogEntry { return { kind: 'monitor', @@ -153,4 +164,44 @@ describe('getPillLabel', () => { ]), ).toBe('2 tasks done'); }); + + it('uses singular form for one running dream', () => { + expect(getPillLabel([dreamEntry({ dreamId: 'd-1' })])).toBe('1 dream'); + }); + + it('uses plural form for multiple running dreams', () => { + expect( + getPillLabel([ + dreamEntry({ dreamId: 'd-1' }), + dreamEntry({ dreamId: 'd-2' }), + ]), + ).toBe('2 dreams'); + }); + + it('places dream last in the kind ordering (shell, agent, monitor, dream)', () => { + // Ordering is asserted explicitly because it's a UX choice — dream + // is system-initiated (not user-triggered) and the user is least + // likely to need it at a glance, so it sits to the right of the + // user-launched kinds. + expect( + getPillLabel([ + dreamEntry({ dreamId: 'd-1' }), + agentEntry({ agentId: 'a' }), + shellEntry({ shellId: 'bg_a' }), + monitorEntry({ monitorId: 'mon-a' }), + ]), + ).toBe('1 shell, 1 local agent, 1 monitor, 1 dream'); + }); + + it('counts only running dreams when terminal dreams mix in', () => { + // Mirrors the existing monitor + agent terminal-mix tests so dream + // gets the same coverage profile. + expect( + getPillLabel([ + dreamEntry({ dreamId: 'd-a', status: 'running' }), + dreamEntry({ dreamId: 'd-b', status: 'completed' }), + dreamEntry({ dreamId: 'd-c', status: 'failed' }), + ]), + ).toBe('1 dream'); + }); }); diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx index 99b08887c25..347b80fa9b3 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx @@ -19,6 +19,7 @@ const KIND_NAMES = { agent: { singular: 'local agent', plural: 'local agents' }, shell: { singular: 'shell', plural: 'shells' }, monitor: { singular: 'monitor', plural: 'monitors' }, + dream: { singular: 'dream', plural: 'dreams' }, } as const; /** @@ -48,15 +49,17 @@ export function getPillLabel(entries: readonly DialogEntry[]): string { } function groupAndFormat(entries: readonly DialogEntry[]): string { - const counts = { agent: 0, shell: 0, monitor: 0 }; + const counts = { agent: 0, shell: 0, monitor: 0, dream: 0 }; for (const e of entries) counts[e.kind]++; const parts: string[] = []; // 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. + // agent, then monitor, then dream. Dream sits last because it is + // system-initiated (not user-triggered) and the user is least likely + // to need it at a glance. 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)); + if (counts.dream > 0) parts.push(formatCount('dream', counts.dream)); return parts.join(', '); } diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx index cc95d4f7b19..4fb02f5f9bd 100644 --- a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx @@ -19,12 +19,14 @@ import { useMemo, useState, } from 'react'; -import { type Config } from '@qwen-code/qwen-code-core'; +import { type Config, createDebugLogger } from '@qwen-code/qwen-code-core'; import { type DialogEntry, useBackgroundTaskView, } from '../hooks/useBackgroundTaskView.js'; +const debugLogger = createDebugLogger('BG_TASK_VIEW'); + // ─── Types ────────────────────────────────────────────────── export type BackgroundDialogMode = 'closed' | 'list' | 'detail'; @@ -194,6 +196,28 @@ export function BackgroundTaskViewProvider({ case 'monitor': config.getMonitorRegistry().cancel(target.monitorId); break; + case 'dream': { + // Aborts the dream fork-agent via MemoryManager.cancelTask; + // the manager flips status to 'cancelled' before aborting, and + // the runDream finally block releases the consolidation lock as + // the agent unwinds. Same one-shot fire-and-forget shape as + // shell.requestCancel above. + // + // cancelTask returns false in the contract-violation path + // (running record without an AbortController). Today this is + // unreachable because the controller is registered before + // storeWith fires the notify, but if a future refactor + // breaks the invariant a silent ignore here would let the + // user think the cancel took. Log + leave the dialog open. + const ok = config.getMemoryManager().cancelTask(target.dreamId); + if (!ok) { + debugLogger.warn( + `cancelSelected: dream task ${target.dreamId} could not be cancelled ` + + `(internal state inconsistency — see MemoryManager.cancelTask warn).`, + ); + } + break; + } default: { const _exhaustive: never = target; throw new Error( diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts index 7c71f22d8c2..eb4d8aabee9 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts @@ -25,14 +25,52 @@ function makeFakeRegistry(): FakeRegistry { }; } +interface FakeMemoryManager { + subscribe: ReturnType; + unsubscribe: ReturnType; + /** Captured opts from the most recent subscribe() call (the hook + * passes `{ taskType: 'dream' }` to skip per-extract notifies). */ + lastSubscribeOpts: { taskType?: 'extract' | 'dream' } | undefined; + /** Test helper — invokes the currently-subscribed listener. */ + fire: () => void; +} + +function makeFakeMemoryManager(): FakeMemoryManager { + let listener: (() => void) | undefined; + const ref: { lastSubscribeOpts: FakeMemoryManager['lastSubscribeOpts'] } = { + lastSubscribeOpts: undefined, + }; + const unsubscribe = vi.fn(() => { + listener = undefined; + }); + const subscribe = vi.fn( + (next: () => void, opts?: { taskType?: 'extract' | 'dream' }) => { + listener = next; + ref.lastSubscribeOpts = opts; + return unsubscribe; + }, + ); + return { + subscribe, + unsubscribe, + get lastSubscribeOpts() { + return ref.lastSubscribeOpts; + }, + fire: () => listener?.(), + }; +} + function makeConfig(opts: { agents: () => unknown[]; shells: () => unknown[]; monitors: () => unknown[]; + dreams?: () => unknown[]; }) { const agentReg = makeFakeRegistry(); const shellReg = makeFakeRegistry(); const monitorReg = makeFakeRegistry(); + const memoryMgr = makeFakeMemoryManager(); + const dreams = opts.dreams ?? (() => []); const config = { getBackgroundTaskRegistry: () => ({ @@ -47,9 +85,16 @@ function makeConfig(opts: { ...monitorReg, getAll: opts.monitors, }), + getMemoryManager: () => ({ + subscribe: memoryMgr.subscribe, + // Hook only ever requests dream-typed records; ignore the type arg + // and return whatever the test provided. + listTasksByType: (_type: string, _projectRoot?: string) => dreams(), + }), + getProjectRoot: () => '/test/project', } as unknown as Config; - return { config, agentReg, shellReg, monitorReg }; + return { config, agentReg, shellReg, monitorReg, memoryMgr }; } const agent = (id: string, startTime: number) => ({ @@ -84,6 +129,38 @@ const monitor = (id: string, startTime: number) => ({ droppedLines: 0, }); +// Mirror the MemoryTaskRecord shape that MemoryManager.listTasksByType +// returns. Status defaults to 'running'; tests override to exercise the +// filter (`pending` / `skipped` records must be excluded; `cancelled` +// flows through the same terminal-cap path as `completed` / `failed` +// once the task_stop / dialog cancel keystroke lands one). +const dream = ( + id: string, + startTimeMs: number, + overrides: Partial<{ + status: + | 'pending' + | 'running' + | 'completed' + | 'failed' + | 'cancelled' + | 'skipped'; + progressText: string; + error: string; + metadata: Record; + }> = {}, +) => ({ + id, + taskType: 'dream' as const, + projectRoot: '/test/project', + status: overrides.status ?? ('running' as const), + createdAt: new Date(startTimeMs).toISOString(), + updatedAt: new Date(startTimeMs).toISOString(), + progressText: overrides.progressText, + error: overrides.error, + metadata: overrides.metadata, +}); + describe('useBackgroundTaskView', () => { it('returns empty entries when config is null', () => { const { result } = renderHook(() => useBackgroundTaskView(null)); @@ -154,7 +231,7 @@ describe('useBackgroundTaskView', () => { }); it('clears all three subscriptions on unmount', () => { - const { config, agentReg, shellReg, monitorReg } = makeConfig({ + const { config, agentReg, shellReg, monitorReg, memoryMgr } = makeConfig({ agents: () => [], shells: () => [], monitors: () => [], @@ -178,5 +255,169 @@ describe('useBackgroundTaskView', () => { [expect.any(Function)], [undefined], ]); + // MemoryManager uses subscribe()/unsubscribe rather than the + // setCallback pattern; the unsubscribe returned from subscribe must + // run on cleanup or stale dream listeners leak across remounts. + expect(memoryMgr.subscribe).toHaveBeenCalledTimes(1); + expect(memoryMgr.unsubscribe).toHaveBeenCalledTimes(1); + }); + + it('surfaces dream tasks with kind=dream and skips pending/skipped records', () => { + const { config } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + // Three dream records covering: a pre-fire pending record (must + // not surface — would flood the dialog with one row per + // UserQuery), a running fire (must surface), and a skipped + // gate-miss (must not surface — same flood concern). + dreams: () => [ + dream('d-pending', 100, { status: 'pending' }), + dream('d-running', 200), + dream('d-skipped', 300, { status: 'skipped' }), + ], + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + expect(result.current.entries).toHaveLength(1); + const [only] = result.current.entries; + expect(only.kind).toBe('dream'); + expect(only.status).toBe('running'); + expect(entryId(only)).toBe('d-running'); + }); + + it('caps retained terminal dream entries at 3 most-recent (by updatedAt) plus all running', () => { + // MemoryManager has no eviction; without the cap, accumulating + // completed dreams across a long session would blow up the dialog. + // The cap keeps the dialog glanceable while still surfacing the + // most recent outcomes (mirrors MonitorRegistry's terminal cap). + const baseMs = Date.parse('2026-05-04T12:00:00.000Z'); + const completed = (id: string, mtime: number) => ({ + id, + taskType: 'dream' as const, + projectRoot: '/test/project', + status: 'completed' as const, + createdAt: new Date(baseMs + mtime - 1000).toISOString(), + updatedAt: new Date(baseMs + mtime).toISOString(), + }); + const { config } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + dreams: () => [ + completed('d-old-1', 1_000), + completed('d-old-2', 2_000), + completed('d-mid', 3_000), + completed('d-recent', 4_000), + completed('d-newest', 5_000), + // Plus a running entry that must always survive the cap (caps + // only trim terminals; running dreams are uncapped). + dream('d-running-now', baseMs + 6_000, { status: 'running' }), + ], + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + const ids = result.current.entries.map(entryId).sort(); + // Surviving terminal entries: d-newest, d-recent, d-mid (top 3 by + // updatedAt desc). The two oldest (d-old-1, d-old-2) get dropped. + // The running dream survives unconditionally. + expect(ids).toEqual( + ['d-mid', 'd-newest', 'd-recent', 'd-running-now'].sort(), + ); + }); + + it('surfaces a cancelled dream with kind=dream so the dialog can render the terminal status', () => { + // `'cancelled'` arrives via the dialog `x stop` / `task_stop` path + // which routes through `MemoryManager.cancelTask`. The view-model + // must accept it the same way it accepts `'completed'` / `'failed'`, + // because the dialog's terminal-cap window depends on showing the + // user the outcome of the abort they just triggered. + const { config } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + dreams: () => [dream('d-stopped', 100, { status: 'cancelled' })], + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + expect(result.current.entries).toHaveLength(1); + const [only] = result.current.entries; + expect(only.kind).toBe('dream'); + expect(only.status).toBe('cancelled'); + }); + + it('subscribes to MemoryManager with a dream taskType filter so extract notifies are skipped at the source', () => { + // The taskType filter on MemoryManager.subscribe() is the + // primary perf guard — it prevents the per-UserQuery extract + // notify from waking the bg-tasks UI listener at all (avoids the + // O(n) dream-snapshot fetch + signature compare that would + // otherwise run on every extract transition). Pin the filter so + // a future refactor that drops the opts arg fails the test + // rather than silently re-introducing the wakeups. + const { config, memoryMgr } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + }); + renderHook(() => useBackgroundTaskView(config)); + expect(memoryMgr.subscribe).toHaveBeenCalledTimes(1); + expect(memoryMgr.lastSubscribeOpts).toEqual({ taskType: 'dream' }); + }); + + it('skips setEntries when the memory listener fires with unchanged dream content', () => { + // MemoryManager.subscribe() fires for ALL task transitions, including + // extract task records that have no dialog surface. Without the + // dream-signature dedup, every extract notify would trigger a full + // re-merge + a fresh array reference into setEntries — re-rendering + // the dialog and pill on entries that are byte-identical to the + // previous snapshot. This test pins the dedup by firing the memory + // listener while the dream snapshot stays unchanged and asserting + // that the entries reference is preserved. + const dreams: Array> = [dream('d-only', 100)]; + const { config, memoryMgr } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + dreams: () => dreams, + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + const before = result.current.entries; + expect(before.map(entryId)).toEqual(['d-only']); + + // Fire the memory listener without mutating `dreams`. With the + // signature-dedup in place, this must NOT call setEntries; React + // will then preserve the existing array reference. + act(() => memoryMgr.fire()); + expect(result.current.entries).toBe(before); + + // Sanity check the inverse path: when dreams DO change, the + // listener must propagate. A flipped status should change the + // signature and force a fresh setEntries. + dreams.splice(0, 1, dream('d-only', 100, { status: 'completed' })); + act(() => memoryMgr.fire()); + expect(result.current.entries).not.toBe(before); + expect(result.current.entries[0]?.status).toBe('completed'); + }); + + it('refreshes entries when the memory manager fires its subscribe listener', () => { + const dreams: Array> = []; + const { config, memoryMgr } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + dreams: () => dreams, + }); + const { result } = renderHook(() => useBackgroundTaskView(config)); + expect(result.current.entries).toEqual([]); + + dreams.push(dream('d-1', 100)); + act(() => memoryMgr.fire()); + expect(result.current.entries.map(entryId)).toEqual(['d-1']); + + // A subsequent terminal state update must propagate the new status + // (running → completed) and survive the filter (only pending / + // skipped get dropped). + dreams.splice(0, dreams.length, dream('d-1', 100, { status: 'completed' })); + act(() => memoryMgr.fire()); + const [only] = result.current.entries; + expect(only.kind).toBe('dream'); + expect(only.status).toBe('completed'); }); }); diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts index 7edd3c48a96..dd6c3c496a4 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts @@ -5,11 +5,15 @@ */ /** - * 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 + * useBackgroundTaskView — subscribes to the three background-task + * registries (background subagents, managed shells, and event monitors) + * AND to `MemoryManager` for dream consolidation tasks, merging 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. + * The `MemoryManager.subscribe({ taskType: 'dream' })` filter routes + * dream-task transitions to the same refresh path while skipping the + * per-UserQuery extract notifies that have no dialog surface. * * Surfaces that only care about live work (the footer pill, the * composer's Down-arrow route) filter for `running` themselves. @@ -26,24 +30,75 @@ import { type BackgroundTaskEntry, type BackgroundShellEntry, type Config, + type MemoryTaskRecord, type MonitorEntry, } from '@qwen-code/qwen-code-core'; +// Cap on retained terminal dream entries surfaced via the dialog. +// `MemoryManager.tasks` has no eviction; without this cap the list +// grows unboundedly with completed dreams over the project's lifetime. +// 3 is small enough to stay glanceable yet keeps the most recent +// outcomes visible across rapid succession (e.g. the user opening the +// dialog right after two dreams completed). +const MAX_RETAINED_TERMINAL_DREAMS = 3; + export type AgentDialogEntry = BackgroundTaskEntry & { kind: 'agent'; resumeBlockedReason?: string; }; +/** + * Dream-task adapter. MemoryManager owns its own task records + * (MemoryTaskRecord) and intentionally lives outside the registry trio; + * this view-model wraps the subset of fields the dialog needs and + * narrows status to the four values that ever appear in the dialog + * (skipped/pending records are filtered out at the source). + */ +export type DreamDialogEntry = { + kind: 'dream'; + /** MemoryTaskRecord.id — used as React key + lookup. */ + dreamId: string; + status: 'running' | 'completed' | 'failed' | 'cancelled'; + startTime: number; + /** + * Wall-clock instant the record's `status` last changed. For + * `completed` / `failed` this is when the dream actually finished; + * for `cancelled` this is the moment `cancelTask` ran (NOT when + * the fork agent finishes unwinding — that can lag by seconds for + * agents mid-tool-call). The dialog renders elapsed from this + * value, so a freshly-cancelled record snaps to "Stopped · Ns" + * even while the underlying fork is still releasing the lock. + */ + endTime?: number; + progressText?: string; + error?: string; + /** Number of sessions the dream is reviewing — populated on schedule. */ + sessionCount?: number; + /** Memory topic files written — populated on completion. */ + touchedTopics?: readonly string[]; + /** + * Best-effort warnings populated by `runDream` when post-fork + * housekeeping fails (gating-metadata write or consolidation-lock + * release). The dream itself completed successfully — these are + * informational so the user can explain why subsequent dreams may + * be silently skipped as `'locked'` or why the scheduler gate + * isn't seeing the most recent dream's timestamp. + */ + lockReleaseError?: string; + metadataWriteError?: string; +}; + /** * A unified view-model entry the dialog/pill/context render against. * 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). + * `'shell'` / `'monitor'` / `'dream'` guard, then access fields directly). */ export type DialogEntry = | AgentDialogEntry | (BackgroundShellEntry & { kind: 'shell' }) - | (MonitorEntry & { kind: 'monitor' }); + | (MonitorEntry & { kind: 'monitor' }) + | DreamDialogEntry; export interface UseBackgroundTaskViewResult { entries: readonly DialogEntry[]; @@ -58,6 +113,8 @@ export function entryId(entry: DialogEntry): string { return entry.shellId; case 'monitor': return entry.monitorId; + case 'dream': + return entry.dreamId; default: { const _exhaustive: never = entry; throw new Error( @@ -77,8 +134,27 @@ export function useBackgroundTaskView( const agentRegistry = config.getBackgroundTaskRegistry(); const shellRegistry = config.getBackgroundShellRegistry(); const monitorRegistry = config.getMonitorRegistry(); + const memoryManager = config.getMemoryManager(); + const projectRoot = config.getProjectRoot(); + // Dream snapshot signature, kept as a defense-in-depth dedup for + // the dream-filtered memory listener below. The taskType filter + // already skips the listener entirely on extract notifies; this + // signature additionally absorbs the rare case where dream + // metadata is updated without an observable dialog change. + let lastDreamSig = ''; - const refresh = () => { + // Declared before `refresh` so the function ordering can't trip + // the temporal-dead-zone if a future refactor adds a synchronous + // call to refresh between the two `const` bindings. + const computeDreamSig = (dreams: readonly MemoryTaskRecord[]): string => + dreams.map((t) => `${t.id}:${t.status}:${t.updatedAt}`).join('|'); + + // refresh accepts a pre-fetched dream snapshot so the memory + // listener can reuse the same array it computed for its dedup + // check — avoids a second listTasksByType call AND eliminates the + // race window where the listener's gate sig and the entries it + // builds would otherwise come from two separate snapshots. + const refresh = (dreamSnapshot?: readonly MemoryTaskRecord[]) => { const agentEntries: DialogEntry[] = agentRegistry .getAll() .map((e) => ({ ...e, kind: 'agent' as const })); @@ -88,25 +164,119 @@ export function useBackgroundTaskView( const monitorEntries: DialogEntry[] = monitorRegistry .getAll() .map((e) => ({ ...e, kind: 'monitor' as const })); + // Dream entries: only surface tasks that actually fired. + // `pending` is a sub-second transition state and `skipped` + // records arise from the rare race where the schedule-time + // lock check passed but `acquireDreamLock` then hit EEXIST in + // runDream — these never reflect user-visible work, so filter + // them out. (Most gate misses don't create a record at all; + // scheduleDream returns `{status: 'skipped'}` early without + // touching the task map.) Extract tasks also intentionally + // stay out of this view — they fire on every UserQuery and + // their completion is already covered by the `memory_saved` + // toast in useGeminiStream. + // + // Cap retained terminal entries — MemoryManager.tasks Map has no + // eviction path, so completed/failed dreams accumulate forever + // (every fired dream over the project's lifetime). Without this + // cap the dialog would grow unbounded; with it the user sees all + // running dreams plus the most recent few terminal results + // (mirrors MonitorRegistry.MAX_RETAINED_TERMINAL_MONITORS). + const allDreams = + dreamSnapshot ?? memoryManager.listTasksByType('dream', projectRoot); + const runningDreams = allDreams.filter((t) => t.status === 'running'); + const terminalDreams = allDreams + .filter( + (t) => + t.status === 'completed' || + t.status === 'failed' || + t.status === 'cancelled', + ) + .sort((a, b) => b.updatedAt.localeCompare(a.updatedAt)) + .slice(0, MAX_RETAINED_TERMINAL_DREAMS); + const dreamEntries: DialogEntry[] = [ + ...runningDreams, + ...terminalDreams, + ].map((t) => { + const sessionCount = t.metadata?.['sessionCount']; + const touchedTopics = t.metadata?.['touchedTopics']; + const lockReleaseError = t.metadata?.['lockReleaseError']; + const metadataWriteError = t.metadata?.['metadataWriteError']; + return { + kind: 'dream' as const, + dreamId: t.id, + status: t.status as 'running' | 'completed' | 'failed' | 'cancelled', + startTime: Date.parse(t.createdAt), + endTime: t.status === 'running' ? undefined : Date.parse(t.updatedAt), + progressText: t.progressText, + error: t.error, + sessionCount: + typeof sessionCount === 'number' ? sessionCount : undefined, + touchedTopics: Array.isArray(touchedTopics) + ? (touchedTopics.filter((s) => typeof s === 'string') as string[]) + : undefined, + lockReleaseError: + typeof lockReleaseError === 'string' ? lockReleaseError : undefined, + metadataWriteError: + typeof metadataWriteError === 'string' + ? metadataWriteError + : undefined, + }; + }); // Merge by startTime so the order matches launch order across all - // registries (matters when an agent, shell, and monitor are + // sources (matters when an agent, shell, monitor, and dream are // launched alternately). - const merged = [...agentEntries, ...shellEntries, ...monitorEntries].sort( - (a, b) => a.startTime - b.startTime, - ); + const merged = [ + ...agentEntries, + ...shellEntries, + ...monitorEntries, + ...dreamEntries, + ].sort((a, b) => a.startTime - b.startTime); + // Cache the dream signature derived from the freshly-built + // entries — the memory listener uses this to skip redundant + // setEntries calls when an extract notify fires (extract has no + // dialog surface, so the merged result is identical). Computed + // from the same `allDreams` snapshot used to build dreamEntries + // so the gate value can never desync from what's on screen. + lastDreamSig = computeDreamSig(allDreams); setEntries(merged); }; + // Wrap registry callbacks in a thunk so React's setStatusChange + // signature (no-arg) doesn't accidentally pass an entry into + // refresh's `dreamSnapshot` parameter. + const refreshFromRegistry = () => refresh(); + refresh(); - agentRegistry.setStatusChangeCallback(refresh); - shellRegistry.setStatusChangeCallback(refresh); - monitorRegistry.setStatusChangeCallback(refresh); + agentRegistry.setStatusChangeCallback(refreshFromRegistry); + shellRegistry.setStatusChangeCallback(refreshFromRegistry); + monitorRegistry.setStatusChangeCallback(refreshFromRegistry); + + // Memory listener fires only on dream-task transitions — + // `subscribe({ taskType: 'dream' })` skips the per-extract notify + // entirely so we don't pay the per-UserQuery O(n) signature cost + // for transitions we have no surface for. The dream-content + // signature dedup remains as a second-line guard against the rare + // case where dream metadata is updated without observable changes + // to the dialog (e.g. a future progressText-only patch on the + // same status). The fetched snapshot is forwarded to refresh so + // both the gate and the rendered dreamEntries come from one read. + const memoryListener = () => { + const dreams = memoryManager.listTasksByType('dream', projectRoot); + const sig = computeDreamSig(dreams); + if (sig === lastDreamSig) return; + refresh(dreams); + }; + const unsubscribeMemory = memoryManager.subscribe(memoryListener, { + taskType: 'dream', + }); return () => { agentRegistry.setStatusChangeCallback(undefined); shellRegistry.setStatusChangeCallback(undefined); monitorRegistry.setStatusChangeCallback(undefined); + unsubscribeMemory(); }; }, [config]); diff --git a/packages/core/src/memory/dream.ts b/packages/core/src/memory/dream.ts index 67a4a6af6cb..a76e944a2c8 100644 --- a/packages/core/src/memory/dream.ts +++ b/packages/core/src/memory/dream.ts @@ -23,28 +23,16 @@ export interface AutoMemoryDreamResult { systemMessage?: string; } -async function bumpMetadata(projectRoot: string, now: Date): Promise { - const metadataPath = getAutoMemoryMetadataPath(projectRoot); - try { - const content = await fs.readFile(metadataPath, 'utf-8'); - const metadata = JSON.parse(content) as AutoMemoryMetadata; - metadata.updatedAt = now.toISOString(); - metadata.lastDreamAt = now.toISOString(); - await fs.writeFile( - metadataPath, - `${JSON.stringify(metadata, null, 2)}\n`, - 'utf-8', - ); - } catch { - // Best-effort metadata bump. - } -} - async function runDreamByAgent( projectRoot: string, config: Config, + abortSignal?: AbortSignal, ): Promise { - const result = await planManagedAutoMemoryDreamByAgent(config, projectRoot); + const result = await planManagedAutoMemoryDreamByAgent( + config, + projectRoot, + abortSignal, + ); // Infer which topics were touched from the file paths const touchedTopics = new Set(); @@ -72,6 +60,7 @@ export async function runManagedAutoMemoryDream( projectRoot: string, now = new Date(), config?: Config, + abortSignal?: AbortSignal, ): Promise { await ensureAutoMemoryScaffold(projectRoot, now); const t0 = Date.now(); @@ -82,14 +71,26 @@ export async function runManagedAutoMemoryDream( ); } - const agentResult = await runDreamByAgent(projectRoot, config); + const agentResult = await runDreamByAgent(projectRoot, config, abortSignal); + // Cancel-aware ordering: + // 1. If aborted before this point, return the agent's partial result + // WITHOUT rebuilding the index — index rebuild can be expensive + // and re-running a cancelled dream cycle next time will rebuild + // against the latest topic files anyway. + // 2. If still alive, rebuild the index (informational, powers + // recall) — but only when topics actually changed. + // Scheduler-gating metadata (`lastDreamAt`, `lastDreamSessionId`, + // `lastDreamTouchedTopics`, `lastDreamStatus`) is intentionally NOT + // written here — `MemoryManager.runDream` owns the atomic + // status-flip + metadata-write sequence to close the cancel race + // window where a writeFile finishing concurrently with a cancel + // could persist gating metadata for a record the manager is about + // to mark `'cancelled'`. + if (abortSignal?.aborted) return agentResult; if (agentResult.touchedTopics.length > 0) { - await bumpMetadata(projectRoot, now); await rebuildManagedAutoMemoryIndex(projectRoot); } - await updateDreamMetadataResult(projectRoot, now, agentResult.touchedTopics); - logMemoryDream( config, new MemoryDreamEvent({ diff --git a/packages/core/src/memory/dreamAgentPlanner.test.ts b/packages/core/src/memory/dreamAgentPlanner.test.ts index a84aac18000..4a0f48583d6 100644 --- a/packages/core/src/memory/dreamAgentPlanner.test.ts +++ b/packages/core/src/memory/dreamAgentPlanner.test.ts @@ -136,16 +136,25 @@ describe('dreamAgentPlanner', () => { ).rejects.toThrow('Model timed out'); }); - it('returns cancelled result without throwing', async () => { + it('throws when the agent terminates as cancelled', async () => { + // runForkedAgent maps AgentTerminateMode.CANCELLED to a resolved + // `{status: 'cancelled'}` rather than a rejection. Without + // re-throwing here, `runDreamByAgent` and downstream callers would + // treat an aborted run as a normal completion — bumping + // `lastDreamAt` metadata and overwriting a user-cancelled task + // record with `'completed'`. The throw lets the manager's existing + // catch path (which checks `signal.aborted && status === 'cancelled'`) + // do the right thing. const mockResult: ForkedAgentResult = { status: 'cancelled', + terminateReason: 'CANCELLED', filesTouched: [], }; vi.mocked(runForkedAgent).mockResolvedValue(mockResult); - const result = await planManagedAutoMemoryDreamByAgent(config, projectRoot); - expect(result.status).toBe('cancelled'); - expect(result.filesTouched).toHaveLength(0); + await expect( + planManagedAutoMemoryDreamByAgent(config, projectRoot), + ).rejects.toThrow(/cancelled/i); }); }); diff --git a/packages/core/src/memory/dreamAgentPlanner.ts b/packages/core/src/memory/dreamAgentPlanner.ts index 790d91cc43c..5496eb1e608 100644 --- a/packages/core/src/memory/dreamAgentPlanner.ts +++ b/packages/core/src/memory/dreamAgentPlanner.ts @@ -227,6 +227,7 @@ export function buildConsolidationTaskPrompt( export async function planManagedAutoMemoryDreamByAgent( config: Config, projectRoot: string, + abortSignal?: AbortSignal, ): Promise { const memoryRoot = getAutoMemoryRoot(projectRoot); const transcriptDir = getTranscriptDir(projectRoot); @@ -247,11 +248,23 @@ export async function planManagedAutoMemoryDreamByAgent( ToolNames.WRITE_FILE, ToolNames.EDIT, ], + abortSignal, }); if (result.status === 'failed') { throw new Error(result.terminateReason || 'Dream agent failed'); } + if (result.status === 'cancelled') { + // runForkedAgent maps AgentTerminateMode.CANCELLED → status 'cancelled' + // (resolves rather than rejects). Throw here so callers up the stack + // unwind via their catch paths instead of silently treating an + // aborted dream as a normal completion (which would overwrite the + // user-cancelled record with 'completed' + bump dream metadata). + throw new Error( + result.terminateReason || 'Dream agent cancelled before completion', + ); + } + return result; } diff --git a/packages/core/src/memory/manager.test.ts b/packages/core/src/memory/manager.test.ts index 4860bdaae49..74a20a361e5 100644 --- a/packages/core/src/memory/manager.test.ts +++ b/packages/core/src/memory/manager.test.ts @@ -260,6 +260,77 @@ describe('MemoryManager', () => { }); }); + // ─── subscribe() filter ────────────────────────────────────────────────── + + describe('subscribe() taskType filter', () => { + // The filter exists so high-frequency consumers (the bg-tasks UI + // hook, only rendering dream entries) can skip the per-extract + // notify entirely. Pin the routing both ways: filtered subscribers + // must NOT fire on unrelated transitions, and unfiltered + // subscribers must continue to fire on everything. + it('routes notifies to type-filtered subscribers only when taskType matches', async () => { + vi.mocked(runAutoMemoryExtract).mockResolvedValue({ + touchedTopics: [], + cursor: { sessionId: 'sess', updatedAt: new Date().toISOString() }, + }); + const mgr = new MemoryManager(); + const dreamFilteredFires = vi.fn(); + const extractFilteredFires = vi.fn(); + const unfilteredFires = vi.fn(); + mgr.subscribe(dreamFilteredFires, { taskType: 'dream' }); + mgr.subscribe(extractFilteredFires, { taskType: 'extract' }); + mgr.subscribe(unfilteredFires); + + await mgr.scheduleExtract({ + projectRoot: '/p', + sessionId: 'sess', + history: [{ role: 'user', parts: [{ text: 'hi' }] }], + }); + await mgr.drain(); + + // Extract scheduling fires storeWith (1) + completion update (1) = 2 notifies. + // Dream-filtered subscriber must NOT see them. + expect(dreamFilteredFires).not.toHaveBeenCalled(); + // Both extract-filtered and unfiltered subscribers must see them. + expect(extractFilteredFires.mock.calls.length).toBeGreaterThanOrEqual(1); + expect(unfilteredFires.mock.calls.length).toBeGreaterThanOrEqual(1); + }); + + it('returns an unsubscribe function that drops the filtered listener even when later notifies fire', async () => { + // Verify the unsubscribe actually severs the listener — the + // earlier version of this test only asserted "not called yet" + // without ever firing a notify, so the listener could have + // remained attached and the test would still pass. + vi.mocked(runAutoMemoryExtract).mockResolvedValue({ + touchedTopics: [], + cursor: { sessionId: 'sess', updatedAt: new Date().toISOString() }, + }); + const mgr = new MemoryManager(); + const fires = vi.fn(); + const unsubscribe = mgr.subscribe(fires, { taskType: 'extract' }); + + // First extract should fire the listener (storeWith + completion update). + await mgr.scheduleExtract({ + projectRoot: '/p', + sessionId: 'sess', + history: [{ role: 'user', parts: [{ text: 'hi' }] }], + }); + await mgr.drain(); + const firesBeforeUnsubscribe = fires.mock.calls.length; + expect(firesBeforeUnsubscribe).toBeGreaterThanOrEqual(1); + + // After unsubscribe, a second extract must not increment the count. + unsubscribe(); + await mgr.scheduleExtract({ + projectRoot: '/p', + sessionId: 'sess-2', + history: [{ role: 'user', parts: [{ text: 'hi again' }] }], + }); + await mgr.drain(); + expect(fires.mock.calls.length).toBe(firesBeforeUnsubscribe); + }); + }); + // ─── scheduleDream() ───────────────────────────────────────────────────── describe('scheduleDream()', () => { @@ -314,15 +385,35 @@ describe('MemoryManager', () => { expect(result).toEqual({ status: 'skipped', skippedReason: 'disabled' }); }); + it('skips when params.config is omitted entirely', async () => { + // Without config, runManagedAutoMemoryDream throws — surfacing + // a noisy failed entry in the bg-tasks dialog. The early skip + // converts the omitted-config case to the same disabled-skip + // path so callers can't accidentally produce visible failures + // by leaving config out (the type allows it for test ergonomics). + const mgr = new MemoryManager(); + const result = await mgr.scheduleDream({ + projectRoot, + sessionId: 'sess-no-config', + // config intentionally omitted + now: new Date('2026-04-02T10:00:00.000Z'), + }); + expect(result).toEqual({ status: 'skipped', skippedReason: 'disabled' }); + // Crucially — no record was stored for this skip. + expect(mgr.listTasksByType('dream', projectRoot)).toEqual([]); + }); + it('skips when called again in the same session', async () => { const scanner = vi .fn() .mockResolvedValue(['sess-0', 'sess-1', 'sess-2', 'sess-3', 'sess-4']); const mgr = new MemoryManager(scanner); + const config = makeMockConfig(); const first = await mgr.scheduleDream({ projectRoot, sessionId: 'sess-x', + config, now: new Date('2026-04-01T10:00:00.000Z'), minHoursBetweenDreams: 0, minSessionsBetweenDreams: 1, @@ -333,6 +424,7 @@ describe('MemoryManager', () => { const second = await mgr.scheduleDream({ projectRoot, sessionId: 'sess-x', + config, now: new Date('2026-04-01T11:00:00.000Z'), minHoursBetweenDreams: 0, minSessionsBetweenDreams: 1, @@ -365,6 +457,7 @@ describe('MemoryManager', () => { const result = await mgr.scheduleDream({ projectRoot, sessionId: 'sess-new', + config: makeMockConfig(), now: new Date('2026-04-01T10:00:00.000Z'), minHoursBetweenDreams: 24, minSessionsBetweenDreams: 1, @@ -380,6 +473,7 @@ describe('MemoryManager', () => { const result = await mgr.scheduleDream({ projectRoot, sessionId: 'sess-new', + config: makeMockConfig(), now: new Date('2026-04-01T10:00:00.000Z'), minHoursBetweenDreams: 0, minSessionsBetweenDreams: 5, @@ -401,6 +495,7 @@ describe('MemoryManager', () => { const result = await mgr.scheduleDream({ projectRoot, sessionId: 'sess-x', + config: makeMockConfig(), now: new Date('2026-04-01T10:00:00.000Z'), minHoursBetweenDreams: 0, minSessionsBetweenDreams: 3, @@ -425,6 +520,198 @@ describe('MemoryManager', () => { }); }); + // ─── cancelTask() ──────────────────────────────────────────────────────── + + describe('cancelTask()', () => { + let tempDir: string; + let projectRoot: string; + + beforeEach(async () => { + vi.resetAllMocks(); + process.env['QWEN_CODE_MEMORY_LOCAL'] = '1'; + clearAutoMemoryRootCache(); + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mgr-cancel-')); + projectRoot = path.join(tempDir, 'project'); + await fs.mkdir(projectRoot, { recursive: true }); + await ensureAutoMemoryScaffold( + projectRoot, + new Date('2026-04-01T00:00:00.000Z'), + ); + }); + + afterEach(async () => { + delete process.env['QWEN_CODE_MEMORY_LOCAL']; + clearAutoMemoryRootCache(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('aborts the dream fork agent and marks the record cancelled', async () => { + // The fork's abort signal is captured here so the test can assert + // both the status flip AND the actual signal propagation — only + // the latter guarantees runForkedAgent will unwind. + let capturedSignal: AbortSignal | undefined; + let resolveDreamStarted!: () => void; + const dreamStarted = new Promise((r) => { + resolveDreamStarted = r; + }); + vi.mocked(runManagedAutoMemoryDream).mockImplementation( + async (_root, _now, _config, signal) => { + capturedSignal = signal; + resolveDreamStarted(); + // Simulate a long-running dream that respects the signal. + await new Promise((_, reject) => { + signal?.addEventListener('abort', () => + reject(new Error('aborted')), + ); + }); + return { + touchedTopics: [], + dedupedEntries: 0, + systemMessage: undefined, + }; + }, + ); + + const mgr = new MemoryManager(async () => [ + 'sess-0', + 'sess-1', + 'sess-2', + 'sess-3', + 'sess-4', + ]); + const config = makeMockConfig(); + const result = await mgr.scheduleDream({ + projectRoot, + sessionId: 'sess-x', + config, + now: new Date('2026-04-02T10:00:00.000Z'), + }); + expect(result.status).toBe('scheduled'); + const taskId = result.taskId!; + + // Wait for the fork to actually enter — scheduleDream returns + // before lock acquisition + the fork-agent invocation actually + // run. Cancelling before the fork enters would race the abort + // signal capture and produce a flaky undefined. + await dreamStarted; + + // Cancel must succeed and synchronously flip status; the fork's + // unwind happens later via the abort signal. + const cancelled = mgr.cancelTask(taskId); + expect(cancelled).toBe(true); + expect(mgr.getTask(taskId)?.status).toBe('cancelled'); + expect(capturedSignal?.aborted).toBe(true); + + // Drain so the fork-agent rejection lands and runDream's catch + // path runs — the user-cancel guard must NOT overwrite to + // 'failed'. (Without the guard, the rejected promise sets the + // record to failed with error="aborted".) + await mgr.drain({ timeoutMs: 1000 }); + expect(mgr.getTask(taskId)?.status).toBe('cancelled'); + }); + + it('keeps the record cancelled even when runManagedAutoMemoryDream resolves successfully after abort', async () => { + // The realistic abort path: runForkedAgent maps + // AgentTerminateMode.CANCELLED to a resolved `{status: 'cancelled'}` + // rather than a rejection. dreamAgentPlanner is supposed to + // rethrow that case, but the manager carries an additional + // signal.aborted check after the await as defense in depth. + // This test simulates the "resolved despite cancel" scenario by + // having the mock RESOLVE on abort instead of rejecting — without + // the guard, runDream's success path would overwrite the + // user-cancelled record to 'completed' and bump dream metadata + // for an aborted run. + let resolveStarted!: () => void; + const started = new Promise((r) => { + resolveStarted = r; + }); + vi.mocked(runManagedAutoMemoryDream).mockImplementation( + async (_root, _now, _config, signal) => { + resolveStarted(); + await new Promise((resolve) => { + signal?.addEventListener('abort', () => resolve()); + }); + return { + touchedTopics: ['user', 'project'], + dedupedEntries: 0, + systemMessage: 'Managed auto-memory dream completed.', + }; + }, + ); + + const mgr = new MemoryManager(async () => [ + 'sess-0', + 'sess-1', + 'sess-2', + 'sess-3', + 'sess-4', + ]); + const config = makeMockConfig(); + const result = await mgr.scheduleDream({ + projectRoot, + sessionId: 'sess-x', + config, + now: new Date('2026-04-02T10:00:00.000Z'), + }); + const taskId = result.taskId!; + await started; + mgr.cancelTask(taskId); + await mgr.drain({ timeoutMs: 1000 }); + + expect(mgr.getTask(taskId)?.status).toBe('cancelled'); + // Metadata write must NOT have happened — lastDreamAt should + // still be the scaffold's initial value, not the cancelled-run's + // `now`. (Bumping it would suppress the next legitimate dream.) + const metaRaw = await fs.readFile( + getAutoMemoryMetadataPath(projectRoot), + 'utf-8', + ); + const meta = JSON.parse(metaRaw) as { + lastDreamAt?: string; + lastDreamSessionId?: string; + }; + expect(meta.lastDreamAt).not.toBe('2026-04-02T10:00:00.000Z'); + expect(meta.lastDreamSessionId).not.toBe('sess-x'); + }); + + it('returns false for unknown task ids', async () => { + const mgr = new MemoryManager(); + expect(mgr.cancelTask('does-not-exist')).toBe(false); + }); + + it('returns false for an already-completed dream', async () => { + // The dream's natural completion path runs first, marks the + // record terminal; a subsequent cancel attempt must no-op rather + // than overwrite the recorded outcome (would erase touchedTopics + // metadata the user just saw via memory_saved toast). + vi.mocked(runManagedAutoMemoryDream).mockResolvedValue({ + touchedTopics: [], + dedupedEntries: 0, + systemMessage: undefined, + }); + const mgr = new MemoryManager(async () => [ + 'sess-0', + 'sess-1', + 'sess-2', + 'sess-3', + 'sess-4', + ]); + const config = makeMockConfig(); + const result = await mgr.scheduleDream({ + projectRoot, + sessionId: 'sess-x', + config, + now: new Date('2026-04-02T10:00:00.000Z'), + }); + const taskId = result.taskId!; + // Drain so the dream completes naturally. + await mgr.drain({ timeoutMs: 1000 }); + expect(mgr.getTask(taskId)?.status).toBe('completed'); + expect(mgr.cancelTask(taskId)).toBe(false); + expect(mgr.getTask(taskId)?.status).toBe('completed'); + }); + }); + // ─── resetExtractStateForTests() ───────────────────────────────────────── describe('resetExtractStateForTests()', () => { diff --git a/packages/core/src/memory/manager.ts b/packages/core/src/memory/manager.ts index 86924893cc8..091edf77ee6 100644 --- a/packages/core/src/memory/manager.ts +++ b/packages/core/src/memory/manager.ts @@ -39,7 +39,13 @@ import { randomUUID } from 'node:crypto'; import type { Content, Part } from '@google/genai'; import type { Config } from '../config/config.js'; import { Storage } from '../config/storage.js'; -import { logMemoryExtract, MemoryExtractEvent } from '../telemetry/index.js'; +import { createDebugLogger } from '../utils/debugLogger.js'; +import { + logMemoryDream, + logMemoryExtract, + MemoryDreamEvent, + MemoryExtractEvent, +} from '../telemetry/index.js'; import { isAutoMemPath } from './paths.js'; import { getAutoMemoryConsolidationLockPath, @@ -67,6 +73,8 @@ import { writeDreamManualRunToMetadata } from './dream.js'; import { buildConsolidationTaskPrompt } from './dreamAgentPlanner.js'; import type { AutoMemoryMetadata } from './types.js'; +const debugLogger = createDebugLogger('AUTO_MEMORY_MANAGER'); + // ─── Re-export public types consumed by callers ─────────────────────────────── export type { @@ -87,6 +95,7 @@ export type MemoryTaskStatus = | 'running' | 'completed' | 'failed' + | 'cancelled' | 'skipped'; export interface MemoryTaskRecord { @@ -346,7 +355,17 @@ export class MemoryManager { // ── Task records ──────────────────────────────────────────────────────────── private readonly tasks = new Map(); // ── Subscribers (useSyncExternalStore / custom listeners) ──────────────── + // Subscribers without a taskType filter receive every notify; those + // with a filter receive only notifies whose changed record matches + // (extract OR dream). Filtered subscribers exist so high-frequency + // consumers (e.g. the bg-tasks UI hook, which only cares about + // dream) can skip the per-extract O(n) work that would otherwise + // run on every UserQuery. private readonly subscribers = new Set<() => void>(); + private readonly subscribersByType = new Map< + 'extract' | 'dream', + Set<() => void> + >(); // ── In-flight promises (for drain) ────────────────────────────────────────── private readonly inFlight = new Map>(); @@ -361,6 +380,22 @@ export class MemoryManager { // ── Dream scheduling state ─────────────────────────────────────────────────── private readonly dreamInFlightByKey = new Map(); private readonly dreamLastSessionScanAt = new Map(); + // AbortControllers for in-flight dream tasks, keyed by record id. + // cancelTask() looks up the controller, aborts it (the abort signal + // propagates into runForkedAgent), and marks the record cancelled. + // The runDream finally block clears the entry on settle. + private readonly dreamAbortControllers = new Map(); + // Set to true when releaseDreamLock() throws (e.g., Windows EPERM, + // ENOENT race, disk full). The lock file is then left on disk and + // dreamLockExists() sees a fresh-mtime lock owned by a still-alive + // PID (us!), suppressing every subsequent scheduleDream() call as + // `{status: 'skipped', skippedReason: 'locked'}` — invisible to the + // user once the surfacing UI just shows "Lock release failed" without + // re-firing. Setting this flag tells the next scheduleDream() to + // force-clean the leaked lock file before the existence check, so + // scheduling resumes within the same session instead of waiting for + // next session start's staleness sweep. + private dreamLockReleaseFailed = false; private readonly sessionScanner: SessionScannerFn; constructor(sessionScanner: SessionScannerFn = defaultSessionScanner) { @@ -372,14 +407,49 @@ export class MemoryManager { * Register a listener that is called whenever any task record changes. * Compatible with React’s `useSyncExternalStore`. * Returns an unsubscribe function. + * + * Pass `{ taskType: 'dream' }` (or `'extract'`) to receive only + * notifies whose changed record matches that type. Filtered + * subscribers skip the wakeup entirely for unrelated transitions — + * the dream-only UI hook uses this to avoid doing O(n) signature + * work on every per-UserQuery extract notify. */ - subscribe(listener: () => void): () => void { + subscribe( + listener: () => void, + opts?: { taskType?: 'extract' | 'dream' }, + ): () => void { + if (opts?.taskType) { + const type = opts.taskType; + let set = this.subscribersByType.get(type); + if (!set) { + set = new Set(); + this.subscribersByType.set(type, set); + } + set.add(listener); + return () => { + set!.delete(listener); + // Drop the Map entry when the per-type bucket is empty so the + // long-lived MemoryManager doesn't accumulate empty Sets across + // repeated subscribe/unsubscribe cycles (e.g. React mount / + // unmount in the bg-tasks UI hook). + if (set!.size === 0) this.subscribersByType.delete(type); + }; + } this.subscribers.add(listener); return () => this.subscribers.delete(listener); } - private notify(): void { + /** + * Notify subscribers. Pass the changed task's type so type-filtered + * subscribers can be reached too; the unfiltered subscriber set + * always receives the wakeup either way. + */ + private notify(taskType?: 'extract' | 'dream'): void { for (const fn of this.subscribers) fn(); + if (taskType) { + const typed = this.subscribersByType.get(taskType); + if (typed) for (const fn of typed) fn(); + } } /** Update a record and notify subscribers. */ @@ -390,7 +460,7 @@ export class MemoryManager { >, ): void { updateRecord(record, patch); - this.notify(); + this.notify(record.taskType); } /** @@ -399,7 +469,7 @@ export class MemoryManager { */ private store(record: MemoryTaskRecord): void { this.tasks.set(record.id, record); - this.notify(); + this.notify(record.taskType); } /** @@ -414,7 +484,7 @@ export class MemoryManager { ): void { updateRecord(record, patch); this.tasks.set(record.id, record); - this.notify(); + this.notify(record.taskType); } // ─── Task record query ──────────────────────────────────────────────────────── @@ -656,7 +726,12 @@ export class MemoryManager { async scheduleDream( params: ScheduleDreamParams, ): Promise { - if (params.config && !params.config.getManagedAutoDreamEnabled()) { + // `params.config` is optional only because some test paths omit it; + // production callers always pass it. Without a config the + // fork-agent execution can't start (`runManagedAutoMemoryDream` + // throws). Skip early so a missing-config call doesn't surface a + // failed dream entry in the bg-tasks dialog. + if (!params.config || !params.config.getManagedAutoDreamEnabled()) { return { status: 'skipped', skippedReason: 'disabled' }; } @@ -700,6 +775,22 @@ export class MemoryManager { return { status: 'skipped', skippedReason: 'min_sessions' }; } + // If the previous dream's release failed (lockReleaseError surfaced + // on the dialog), the lock file is still on disk and dreamLockExists() + // would silently suppress every subsequent dream until next process + // start. Force-clean it here so the same session recovers. + if (this.dreamLockReleaseFailed) { + await fs + .rm(getAutoMemoryConsolidationLockPath(params.projectRoot), { + force: true, + }) + .catch(() => { + // Best-effort recovery — if even the forced rm fails (truly + // unrecoverable filesystem state), fall through and let the + // existence check below report 'locked' as before. + }); + this.dreamLockReleaseFailed = false; + } if (await dreamLockExists(params.projectRoot)) { return { status: 'skipped', skippedReason: 'locked' }; } @@ -720,26 +811,103 @@ export class MemoryManager { params.projectRoot, params.sessionId, ); + // Register the AbortController BEFORE storeWith. storeWith fires + // a notify which can synchronously call cancelTask via subscribers + // (e.g. a UI listener). If the controller isn't in + // `dreamAbortControllers` by then, cancelTask falls into the + // missing-controller defensive warn-and-return-false path and the + // model gets a phantom failure on a brand-new dream. Registering + // first means any reentrant cancel sees a complete state. + const abortController = new AbortController(); + this.dreamAbortControllers.set(record.id, abortController); + this.dreamInFlightByKey.set(dedupeKey, record.id); this.storeWith(record, { status: 'running', + // Set the initial progressText so the dialog's Progress section + // has something to show during the in-flight window — fork-agent + // execution exposes no per-turn callback today, so without this + // the section stays empty until completion. + progressText: 'Scheduled managed auto-memory dream.', metadata: { sessionCount: sessionIds.length }, }); - this.dreamInFlightByKey.set(dedupeKey, record.id); const promise = this.track( record.id, - this.runDream(record, dedupeKey, params, now), + this.runDream(record, dedupeKey, params, now, abortController.signal), ); return { status: 'scheduled', taskId: record.id, promise }; } + /** + * Look up a single task record by id. Used by `task_stop` and other + * cross-cutting consumers that have a task id but no project root. + */ + getTask(taskId: string): MemoryTaskRecord | undefined { + return this.tasks.get(taskId); + } + + /** + * Cancel a running dream task. Aborts the dream's fork agent (the + * abort signal threads through `runForkedAgent`), marks the record + * cancelled immediately so the UI reflects user intent, and lets the + * existing `runDream` finally block release the consolidation lock + * via the natural error propagation path. + * + * Returns true if a running task was aborted, false if the task is + * unknown / already terminal / not a dream. Currently only dream + * tasks support cancellation — extract is short-lived and runs + * synchronously through the request loop; cancelling it would + * interfere with the user's own turn. + */ + cancelTask(taskId: string): boolean { + const record = this.tasks.get(taskId); + if (!record) return false; + if (record.taskType !== 'dream') return false; + if (record.status !== 'running') return false; + + // The AbortController is registered synchronously alongside the + // status='running' transition in scheduleDream and only cleared in + // runDream's finally block (which only runs after a terminal + // status transition has already happened). So under normal flow + // an entry that is `running` MUST have a controller. Treat the + // missing-controller case as a contract violation: don't flip + // status (a cancelled record without an aborted fork would leak + // the consolidation lock until the agent finishes naturally) and + // return false so the caller knows the abort didn't take. Log at + // warn level so the inconsistency is observable in debug bundles + // — silent failure here would leave a runaway dream burning tokens + // with no signal to the user or to telemetry. + const ac = this.dreamAbortControllers.get(taskId); + if (!ac) { + debugLogger.warn( + `cancelTask: AbortController missing for running dream task ${taskId}; ` + + `not flipping status. This indicates a logic bug — the controller ` + + `should have been registered in scheduleDream and only cleared ` + + `after a terminal status transition.`, + ); + return false; + } + + // Mark cancelled BEFORE aborting so the runDream catch path can + // detect the user-cancel intent (signal.aborted + status already + // 'cancelled') and avoid overwriting with a generic 'failed'. + this.update(record, { + status: 'cancelled', + progressText: 'Cancelled by user.', + }); + ac.abort(); + return true; + } + private async runDream( record: MemoryTaskRecord, dedupeKey: string, params: ScheduleDreamParams, now: Date, + abortSignal: AbortSignal, ): Promise { + const dreamStartMs = Date.now(); try { try { await acquireDreamLock(params.projectRoot); @@ -761,13 +929,26 @@ export class MemoryManager { params.projectRoot, now, params.config, + abortSignal, ); - const nextMetadata = await readDreamMetadata(params.projectRoot); - nextMetadata.lastDreamAt = now.toISOString(); - nextMetadata.lastDreamSessionId = params.sessionId; - nextMetadata.updatedAt = now.toISOString(); - await writeDreamMetadata(params.projectRoot, nextMetadata); + // Defense-in-depth: runForkedAgent maps cancelled fork-agents + // to a resolved `{status: 'cancelled'}` rather than a rejection. + // dreamAgentPlanner now rethrows that case so the catch path + // below handles it, but if anything in the call chain ever + // forgets to propagate, this guard prevents the success path + // from clobbering the user-cancelled record with 'completed' + // and bumping dream metadata for an aborted run. + if (abortSignal.aborted) { + return record; + } + // Atomic-from-cancel sequence: flip status='completed' BEFORE + // any scheduler-gating metadata write. Once status is no + // longer 'running', cancelTask refuses, so the writeFile that + // follows can't race a flip-to-cancelled. The cancel-raced- + // status-update branch below covers the remaining window + // (cancel landed between the pre-update check and the + // synchronous update). this.update(record, { status: 'completed', progressText: @@ -778,16 +959,120 @@ export class MemoryManager { lastDreamAt: now.toISOString(), }, }); + if (abortSignal.aborted) { + // Defense-in-depth: unreachable today (no `await` between + // the pre-update check and the synchronous update above, + // so JS's single-threaded execution prevents + // `signal.aborted` from transitioning between them — a + // cancelTask landing inside the storeWith notify would + // already have flipped status, and our update would have + // raced ahead of it to 'completed'). Kept against a future + // refactor that introduces an `await` between the two + // checks. Preserves the touched-topic metadata on the + // restored cancelled record so the user can still tell + // memory files were modified before the abort took. + this.update(record, { + status: 'cancelled', + progressText: 'Cancelled after memory changes.', + metadata: { + touchedTopics: result.touchedTopics, + dedupedEntries: result.dedupedEntries, + }, + }); + return record; + } + // Status is now 'completed'; cancelTask will refuse from + // here on out. Safe to write scheduler-gating metadata + // without a race window. + // + // Wrap the read/write in a try/catch — pre-PR `bumpMetadata` + // in dream.ts swallowed errors as best-effort; without this + // wrap a transient ENOENT / EPERM on the metadata file would + // propagate to the outer catch and overwrite a + // legitimately-completed dream with `'failed'`. The dream + // already did its work (touched files are on disk and + // visible). Trade-off: the next dream cycle won't see a + // bumped lastDreamAt and may re-fire — same trade as the + // original best-effort behavior. + try { + const nextMetadata = await readDreamMetadata(params.projectRoot); + nextMetadata.lastDreamAt = now.toISOString(); + nextMetadata.lastDreamSessionId = params.sessionId; + nextMetadata.updatedAt = now.toISOString(); + nextMetadata.lastDreamTouchedTopics = result.touchedTopics; + nextMetadata.lastDreamStatus = + result.touchedTopics.length > 0 ? 'updated' : 'noop'; + // Mirror the manual /dream path's reset so the two write + // sites don't drift. The field is currently dead code on + // main (only ever written, never read) but keeping the two + // paths in sync avoids surprises if a future change starts + // reading it. + nextMetadata.recentSessionIdsSinceDream = []; + await writeDreamMetadata(params.projectRoot, nextMetadata); + } catch (metaError) { + const message = + metaError instanceof Error ? metaError.message : String(metaError); + debugLogger.warn( + `Failed to persist dream gating metadata for ${record.id}: ${message}`, + ); + this.update(record, { + metadata: { metadataWriteError: message }, + }); + } } finally { - await releaseDreamLock(params.projectRoot); + // Lock release errors are logged AND surfaced on the record's + // metadata so the user can see why subsequent dreams may be + // skipped as 'locked'. If releasing throws (e.g., EPERM on + // Windows, ENOENT race), letting it propagate to the outer + // catch would overwrite a successfully-completed dream with + // 'failed'. The on-disk lock will be cleaned up on the next + // session start via the staleness sweep, so swallowing the + // error here doesn't risk a permanently-stuck lock. + try { + await releaseDreamLock(params.projectRoot); + } catch (lockError) { + const message = + lockError instanceof Error ? lockError.message : String(lockError); + debugLogger.warn( + `Failed to release dream lock for task ${record.id}: ${message}. ` + + `Next scheduleDream() will force-clean the leaked lock.`, + ); + this.dreamLockReleaseFailed = true; + this.update(record, { + metadata: { lockReleaseError: message }, + }); + } } } catch (error) { + // User-cancel path: cancelTask already aborted the signal AND + // marked the record cancelled. The fork agent throws an abort + // error which lands here; don't overwrite with 'failed'. + if (abortSignal.aborted && record.status === 'cancelled') { + if (params.config) { + logMemoryDream( + params.config, + new MemoryDreamEvent({ + trigger: 'auto', + status: 'cancelled', + deduped_entries: 0, + touched_topics: [], + // Real elapsed time the cancelled dream consumed before + // the user stopped it — without this, latency histograms + // / p95 metrics would silently treat cancelled dreams as + // 0ms and skew toward the success path. + duration_ms: Date.now() - dreamStartMs, + }), + ); + } + return record; + } this.update(record, { status: 'failed', error: error instanceof Error ? error.message : String(error), }); } finally { this.dreamInFlightByKey.delete(dedupeKey); + this.dreamAbortControllers.delete(record.id); } return record; } diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index bcd577a2845..72717804973 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -959,7 +959,7 @@ export function recordMemoryDreamMetrics( durationMs: number, attrs: { trigger: 'auto' | 'manual'; - status: 'updated' | 'noop' | 'failed'; + status: 'updated' | 'noop' | 'failed' | 'cancelled'; deduped_entries: number; }, ): void { diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 764147297d3..fb604d49a0b 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -1204,7 +1204,7 @@ export class MemoryDreamEvent implements BaseTelemetryEvent { 'event.timestamp': string; /** 'auto' = scheduler-triggered; 'manual' = user ran /dream */ trigger: 'auto' | 'manual'; - status: 'updated' | 'noop' | 'failed'; + status: 'updated' | 'noop' | 'failed' | 'cancelled'; deduped_entries: number; touched_topics_count: number; touched_topics: string; @@ -1212,7 +1212,7 @@ export class MemoryDreamEvent implements BaseTelemetryEvent { constructor(params: { trigger: 'auto' | 'manual'; - status: 'updated' | 'noop' | 'failed'; + status: 'updated' | 'noop' | 'failed' | 'cancelled'; deduped_entries: number; touched_topics: string[]; duration_ms: number; diff --git a/packages/core/src/tools/task-stop.test.ts b/packages/core/src/tools/task-stop.test.ts index d6d8fc4d839..ed2c500d526 100644 --- a/packages/core/src/tools/task-stop.test.ts +++ b/packages/core/src/tools/task-stop.test.ts @@ -25,11 +25,19 @@ describe('TaskStopTool', () => { abandonBackgroundAgent = vi.fn(); shellRegistry = new BackgroundShellRegistry(); monitorRegistry = new MonitorRegistry(); + // Default fake MemoryManager — every test that doesn't care about + // dream gets an empty stub so the 4th-route lookup falls through to + // the not-found branch instead of crashing on undefined. + const memoryManager = { + getTask: vi.fn(() => undefined), + cancelTask: vi.fn(() => false), + }; config = { getBackgroundTaskRegistry: () => registry, abandonBackgroundAgent, getBackgroundShellRegistry: () => shellRegistry, getMonitorRegistry: () => monitorRegistry, + getMemoryManager: () => memoryManager, } as unknown as Config; tool = new TaskStopTool(config); }); @@ -268,4 +276,156 @@ describe('TaskStopTool', () => { expect(result.llmContent).toContain('completed'); }); }); + + describe('dream task support', () => { + it('cancels a running dream by routing through MemoryManager.cancelTask', async () => { + const cancelTask = vi.fn(() => true); + const dreamRecord = { + id: 'dream-running-1', + taskType: 'dream' as const, + projectRoot: '/p', + status: 'running' as const, + createdAt: '2026-05-04T12:00:00.000Z', + updatedAt: '2026-05-04T12:00:00.000Z', + }; + const memoryManager = { + getTask: vi.fn((id: string) => + id === 'dream-running-1' ? dreamRecord : undefined, + ), + cancelTask, + }; + const localConfig = { + getBackgroundTaskRegistry: () => registry, + abandonBackgroundAgent, + getBackgroundShellRegistry: () => shellRegistry, + getMonitorRegistry: () => monitorRegistry, + getMemoryManager: () => memoryManager, + } as unknown as Config; + const localTool = new TaskStopTool(localConfig); + + const result = await localTool.validateBuildAndExecute( + { task_id: 'dream-running-1' }, + new AbortController().signal, + ); + + expect(cancelTask).toHaveBeenCalledWith('dream-running-1'); + expect(result.error).toBeUndefined(); + expect(result.llmContent).toContain('Cancellation requested'); + expect(result.llmContent).toContain('dream task "dream-running-1"'); + }); + + it('returns NOT_RUNNING when the dream is already terminal', async () => { + // Mirrors the agent / shell / monitor not-running guards so a + // model retry against an already-finished dream surfaces the + // distinct error type instead of "not found". + const dreamRecord = { + id: 'dream-done-1', + taskType: 'dream' as const, + projectRoot: '/p', + status: 'completed' as const, + createdAt: '2026-05-04T12:00:00.000Z', + updatedAt: '2026-05-04T12:01:00.000Z', + }; + const cancelTask = vi.fn(() => false); + const memoryManager = { + getTask: vi.fn(() => dreamRecord), + cancelTask, + }; + const localConfig = { + getBackgroundTaskRegistry: () => registry, + abandonBackgroundAgent, + getBackgroundShellRegistry: () => shellRegistry, + getMonitorRegistry: () => monitorRegistry, + getMemoryManager: () => memoryManager, + } as unknown as Config; + const localTool = new TaskStopTool(localConfig); + + const result = await localTool.validateBuildAndExecute( + { task_id: 'dream-done-1' }, + new AbortController().signal, + ); + + expect(cancelTask).not.toHaveBeenCalled(); + expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_NOT_RUNNING); + expect(result.llmContent).toContain('Background dream "dream-done-1"'); + expect(result.llmContent).toContain('completed'); + }); + + it('returns NOT_CANCELLABLE when the task id resolves to an extract record', async () => { + // Extract is short-lived and runs on the request path; cancelling + // it would interfere with the user's own turn. The dispatch must + // distinguish "task exists but isn't cancellable" from "task + // doesn't exist" — without the distinct error type, a model + // retrying against an extract id would incorrectly conclude the + // id was never valid. + const extractRecord = { + id: 'extract-running-1', + taskType: 'extract' as const, + projectRoot: '/p', + status: 'running' as const, + createdAt: '2026-05-04T12:00:00.000Z', + updatedAt: '2026-05-04T12:00:00.000Z', + }; + const cancelTask = vi.fn(); + const memoryManager = { + getTask: vi.fn(() => extractRecord), + cancelTask, + }; + const localConfig = { + getBackgroundTaskRegistry: () => registry, + abandonBackgroundAgent, + getBackgroundShellRegistry: () => shellRegistry, + getMonitorRegistry: () => monitorRegistry, + getMemoryManager: () => memoryManager, + } as unknown as Config; + const localTool = new TaskStopTool(localConfig); + + const result = await localTool.validateBuildAndExecute( + { task_id: 'extract-running-1' }, + new AbortController().signal, + ); + + expect(cancelTask).not.toHaveBeenCalled(); + expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_NOT_CANCELLABLE); + expect(result.llmContent).toContain('extract'); + expect(result.llmContent).toContain('not cancellable'); + }); + + it('returns an error when cancelTask returns false (missing AbortController)', async () => { + // The MemoryManager.cancelTask contract returns false when the + // AbortController is missing for a running record — a logic- + // level invariant violation. task_stop must surface the failure + // rather than report a phantom success, otherwise the model + // believes the dream is being aborted while it actually keeps + // burning tokens. + const dreamRecord = { + id: 'dream-broken-1', + taskType: 'dream' as const, + projectRoot: '/p', + status: 'running' as const, + createdAt: '2026-05-04T12:00:00.000Z', + updatedAt: '2026-05-04T12:00:00.000Z', + }; + const memoryManager = { + getTask: vi.fn(() => dreamRecord), + cancelTask: vi.fn(() => false), + }; + const localConfig = { + getBackgroundTaskRegistry: () => registry, + abandonBackgroundAgent, + getBackgroundShellRegistry: () => shellRegistry, + getMonitorRegistry: () => monitorRegistry, + getMemoryManager: () => memoryManager, + } as unknown as Config; + const localTool = new TaskStopTool(localConfig); + + const result = await localTool.validateBuildAndExecute( + { task_id: 'dream-broken-1' }, + new AbortController().signal, + ); + + expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_INTERNAL_ERROR); + expect(result.llmContent).toContain('could not be cancelled'); + }); + }); }); diff --git a/packages/core/src/tools/task-stop.ts b/packages/core/src/tools/task-stop.ts index da9df034b19..bbe75806258 100644 --- a/packages/core/src/tools/task-stop.ts +++ b/packages/core/src/tools/task-stop.ts @@ -136,6 +136,67 @@ class TaskStopInvocation extends BaseToolInvocation< }; } + // MemoryManager memory tasks (dream + extract). Memory tasks live + // outside the registry trio (MemoryManager owns its own task map). + // Only `dream` is cancellable — extract is short-lived and runs on + // the request loop, so cancelling it would interfere with the + // user's own turn. Surface a distinct error for known-but-not- + // cancellable records so the model doesn't conclude the id was + // never valid (which would happen if we fell through to NOT_FOUND). + const memoryManager = this.config.getMemoryManager(); + const memoryRecord = memoryManager.getTask(taskId); + if (memoryRecord) { + if (memoryRecord.taskType !== 'dream') { + return { + llmContent: + `Error: Memory task "${taskId}" (${memoryRecord.taskType}) is ` + + `not cancellable. Only dream consolidation tasks support ` + + `cancellation; extract tasks run on the request loop and ` + + `complete in milliseconds.`, + returnDisplay: `Task not cancellable (${memoryRecord.taskType}).`, + error: { + message: `task is not cancellable: ${taskId} (${memoryRecord.taskType})`, + type: ToolErrorType.TASK_STOP_NOT_CANCELLABLE, + }, + }; + } + if (memoryRecord.status !== 'running') { + return notRunningError('dream', taskId, memoryRecord.status); + } + // cancelTask returns false if the AbortController is missing for + // a running record (logic-level invariant violation; see + // MemoryManager.cancelTask). Surface that explicitly so the model + // sees the cancel didn't take and doesn't claim success. + const cancelled = memoryManager.cancelTask(taskId); + if (!cancelled) { + // Distinct from TASK_STOP_NOT_RUNNING (the task IS running) + // and TASK_STOP_NOT_CANCELLABLE (the kind supports cancel, + // we just couldn't deliver it). INTERNAL_ERROR signals that + // this is unexpected and worth filing — the abort controller + // should have been registered alongside status='running' in + // scheduleDream. + return { + llmContent: + `Error: Dream task "${taskId}" could not be cancelled ` + + `(internal state inconsistency — abort controller missing).`, + returnDisplay: 'Dream cancellation failed (internal state).', + error: { + message: `dream cancel failed: ${taskId}`, + type: ToolErrorType.TASK_STOP_INTERNAL_ERROR, + }, + }; + } + return { + llmContent: + `Cancellation requested for dream task "${taskId}". ` + + `The fork agent is being aborted; the consolidation lock will ` + + `be released as the agent unwinds. Status is visible via the ` + + `interactive Background tasks dialog (focus the footer Background ` + + `tasks pill, then Enter).`, + returnDisplay: `Cancelled dream: ${taskId}`, + }; + } + return { llmContent: `Error: No background task found with ID "${taskId}".`, returnDisplay: 'Task not found.', @@ -148,7 +209,7 @@ class TaskStopInvocation extends BaseToolInvocation< } function notRunningError( - kind: 'agent' | 'shell' | 'monitor', + kind: 'agent' | 'shell' | 'monitor' | 'dream', taskId: string, status: string, ): ToolResult { diff --git a/packages/core/src/tools/tool-error.ts b/packages/core/src/tools/tool-error.ts index 6a29472c669..10023d50217 100644 --- a/packages/core/src/tools/tool-error.ts +++ b/packages/core/src/tools/tool-error.ts @@ -70,6 +70,8 @@ export enum ToolErrorType { // TaskStop-specific Errors TASK_STOP_NOT_FOUND = 'task_stop_not_found', TASK_STOP_NOT_RUNNING = 'task_stop_not_running', + TASK_STOP_NOT_CANCELLABLE = 'task_stop_not_cancellable', + TASK_STOP_INTERNAL_ERROR = 'task_stop_internal_error', // SendMessage-specific Errors SEND_MESSAGE_NOT_FOUND = 'send_message_not_found',