mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-22 03:03:56 +00:00
d44786685c
* feat(cli): surface auto-memory dream tasks in Background tasks dialog Adds `dream` as a fourth kind in the Background tasks pill + dialog, alongside agent / shell / monitor. Subscribes to MemoryManager via its existing subscribe() / listTasksByType() API and adapts MemoryTaskRecord into a DreamDialogEntry view-model. Zero changes to the core package. Filters out `pending` (sub-second transition) and `skipped` (every UserQuery that misses the gate creates one) records. Caps retained terminal entries at 3 since `MemoryManager.tasks` has no eviction path; without the cap, completed dreams would accumulate over the project's lifetime (mirrors MonitorRegistry's terminal cap pattern). Extract tasks are intentionally NOT surfaced — they fire on every UserQuery, would flood the pill, and the `memory_saved` toast in useGeminiStream already covers their completion signal. Read-only for now: cancellation requires MemoryManager.cancelTask + task_stop integration which lands in a follow-up PR. The dialog suppresses the "x stop" hint for dream entries until then to avoid silent no-op keystrokes. Refs #3634 * docs(cli): rephrase dream filter comment to focus on extract vs dream The earlier comment compared the design to a non-qwen-code product; restate the rationale in terms of the local extract / dream split (extract fires every UserQuery and surfaces via memory_saved toast, dream fires after gates and warrants pill / dialog visibility). * feat(core,cli): cancel dream consolidation tasks via dialog and task_stop Wires cancellation for the auto-memory dream task kind: - `MemoryManager.cancelTask(taskId)` — aborts the dream's fork-agent via a new per-task AbortController, marks the record `cancelled` before aborting so the runDream catch path can detect user-intent and avoid overwriting with a generic `failed`. The existing finally block releases the consolidation lock as the agent unwinds. - `MemoryManager.getTask(id)` — point lookup helper so cross-cutting consumers like `task_stop` can route by id without a project root. - AbortSignal threaded through `scheduleDream` → `runDream` → `runManagedAutoMemoryDream` → `planManagedAutoMemoryDreamByAgent` → `runForkedAgent.abortSignal` (already supported). - `task_stop` tool gets a 4th dispatch branch: dream task ids look up via MemoryManager and route through `cancelTask`. Extract is intentionally NOT cancellable — it runs synchronously on the request loop, cancelling would interfere with the user's own turn. - `BackgroundTasksDialog` x-stop hint suppression for dream is removed (was a PR-1 placeholder); `cancelSelected` dream branch now calls `memoryManager.cancelTask`. - `MemoryTaskStatus` gains `'cancelled'`. Dream view-model widens status union and filter to surface cancelled entries (terminal cap continues to apply). Refs #3634 * fix(core,cli): address review feedback on dream cancellation surface - DreamDetailBody: comment said cancellation "lands in PR-2" but the same PR wires `cancelSelected` for dream entries. Reword to describe what's actually shipped + flag in-flight progress as the real follow-up. - task_stop: drop the unreachable `!aborted` error branch. The status guard above already confirms `running`, and `cancelTask` is synchronous; in this branch it cannot return false. * fix(core): handle resolved-cancel path from runForkedAgent for dream tasks runForkedAgent maps AgentTerminateMode.CANCELLED to a resolved {status: 'cancelled'} result rather than rejecting. The cancel-via- task_stop path landed in the previous commit assumed the call would throw — when it didn't, the runDream success path overwrote the user-cancelled record with 'completed' AND bumped lastDreamAt metadata, suppressing the next legitimate dream cycle. Two-layer defense: - dreamAgentPlanner now rethrows when the fork agent reports cancelled status (mirrors the existing failed-status throw). This is the source-of-truth fix. - runDream now checks abortSignal.aborted after the await as defense in depth. If anything in the call chain ever forgets to propagate, this guard short-circuits the success path before metadata write. Updates the existing dreamAgentPlanner test that previously pinned the buggy "returns cancelled without throwing" behavior. Adds a manager test that simulates the resolves-on-abort scenario directly to verify the consumer-side guard catches anything the planner might miss. * fix(core,cli): address review feedback on dream UX, perf, and comment clarity Three fixes from the post-cancellation-PR review: - scheduleDream now sets an initial `progressText` ("Scheduled managed auto-memory dream.") on the in-flight record. Without this, the dialog Detail's Progress section stayed empty until completion — the PR description's mid-flight screenshot showed text that production never actually rendered. - useBackgroundTaskView gates the MemoryManager.subscribe listener on a dream-content signature. The manager fires for every task transition (extract included, ~2x per UserQuery), but the dialog has no extract surface; without this dedup each extract notify forced a full 4-source re-merge + a fresh setEntries reference, re-rendering the dialog and pill on entries that hadn't changed. Added a test that pins the reference-stability invariant. - task-stop comment was misleading — said "the status guard above already confirmed running", but for the dream branch the running check happens IN this branch (not earlier). Reworded. * fix(core,cli): tighten cancelTask contract, dedup dream snapshot reads, sync test helper Four fixes from the latest review pass: - cancelTask() now enforces the AbortController invariant. The old `ac?.abort()` returned `true` even when no controller was found, meaning callers could see a successful return while the dream was not actually aborted (and would leak the consolidation lock until the agent finished naturally). The controller is registered synchronously alongside `status='running'`, so a missing controller for a running record is a contract violation — return false without flipping status so the caller knows the abort didn't take. - useBackgroundTaskView's `refresh()` now reuses the dream snapshot the memory listener fetched for its dedup gate. The previous version re-read `listTasksByType('dream')` inside `computeDreamSig()` and again inside `refresh()` — extra work AND a race window where the gate signature could come from a different snapshot than the one used to build dreamEntries. Single read, single source of truth. - The `dream()` test helper widened to include `'cancelled'` so it matches the production `MemoryTaskStatus` union. Added a small test asserting cancelled dreams flow through the kind discriminator (the dialog's terminal-cap window depends on showing the user the outcome of the abort they just triggered). - Dropped the stale "PR-1 read-only / PR-2 cancellation" block comment above the dream-entries describe block — both are in this PR now. * fix(core,cli): address self-review + Copilot feedback on dream surface Combined fixes for the latest review pass — self-review notes (U1, U2, U5, U6) plus Copilot's three new comments (C1, C2, C3): - **Footer dream-indicator dedupe (U1)**: removed `useDreamRunning` + `✦ dreaming` right-column text. The new Background tasks pill already counts dream tasks alongside agent / shell / monitor; showing both produced two simultaneous signals for the same state. - **task_stop dispatch surfaces extract distinctly (U2 + C1)**: a new `TASK_STOP_NOT_CANCELLABLE` error type fires when the task id resolves to a known-but-not-cancellable record (extract). Previously extract ids fell through to `NOT_FOUND`, misleading the model into thinking the id was never valid. Also surfaces the missing-controller case from `cancelTask` as an explicit error rather than reporting phantom success. - **MemoryManager.cancelTask logs missing-controller violation (C2)**: the silent `return false` for the missing-AbortController case now emits a `debugLogger.warn` so the inconsistency is observable in debug bundles. Without the log a runaway dream burning tokens would leave no trail. - **MemoryManager.subscribe taskType filter (C3)**: subscribers can now opt into per-type notify routing via `subscribe(fn, { taskType })`. Internal `notify()` calls pass the changed task's type so filtered consumers wake only on relevant transitions. The bg-tasks UI hook uses this to skip the per-UserQuery extract notify entirely — drops the per-extract O(n) signature work to zero. - **runDream guards against late-cancel overwrite (U5)**: the success path now re-checks `abortSignal.aborted` between metadata read/write and before the final `update({status: 'completed'})`. Closes the ~tens-of-ms race window where `cancelTask` flipping status to `'cancelled'` would silently lose to the success continuation overwriting with `'completed'` + bumping `lastDreamAt`. - **DreamDialogEntry.endTime semantic comment (U6)**: documents that `endTime` for cancelled records is the cancel-call moment (not the fork unwind), so a future maintainer doesn't treat it as a real fork-finish timestamp. Tests: new `subscribe() taskType filter` describe block in manager, new task_stop tests for `NOT_CANCELLABLE` (extract) and missing- AbortController (dream); existing test renamed/widened. * fix(core,cli): tighten error semantics + comments on dream surface Four fixes from the latest review pass: - New `TASK_STOP_INTERNAL_ERROR` error type for the missing- AbortController contract violation. Previously the dispatcher reused `TASK_STOP_NOT_RUNNING`, which is misleading — the task IS running, cancellation just couldn't be delivered. Distinct type signals "this is unexpected, file a bug" vs `NOT_CANCELLABLE` which signals "expected behavior, use a different approach". - Reworded the `useBackgroundTaskView` filter comment. Said "every UserQuery that misses the gate creates one [skipped record]" but `scheduleDream` returns `{status: 'skipped'}` early without creating a record for most gate misses; only the acquireDreamLock/EEXIST race actually stores a `'skipped'` record. - Strengthened the `subscribe() unsubscribe` test. The previous version asserted "not called yet" without firing any notify after unsubscribe, so a regression that left the listener attached would still pass. Now schedules an extract before AND after the unsubscribe, verifying the call count doesn't increment. - Moved `const debugLogger = createDebugLogger(...)` below the full import block in manager.ts. Previous version sat between imports, violating eslint-plugin-import's `import/first` rule (didn't trip lint locally, but worth fixing before it does). * fix(core): skip scheduleDream early when params.config is missing `ScheduleDreamParams.config` is optional in the type so test paths can omit it, but production callers always pass one. Without a config, `runManagedAutoMemoryDream` throws because the fork-agent execution requires it. With dream tasks now visible in the Background tasks dialog, that throw becomes a noisy `failed` entry the user sees but didn't trigger. Convert the omitted-config case to the same `disabled` skip path that an explicitly-disabled config takes, so a no-config call short-circuits before any record is stored. Existing tests that relied on the old "no config = proceed past the disabled gate" behavior now pass an explicit `makeMockConfig()` (matching what they would do in any realistic scenario). New test pins the no-config skip behavior + asserts no record was stored (so a regression that drops the early skip would produce a visible failed entry in the dialog and fail the test). * fix(core): plug subscribe Map leak + dream.ts late-cancel ordering bug Two fixes from the latest review pass: - `MemoryManager.subscribe`'s typed-branch unsubscribe deleted the listener from its per-type Set but left the empty Set sitting in `subscribersByType`. Over a long-running session with repeated React mount/unmount of the bg-tasks view, that accumulates dead Map entries forever. Drop the entry when the bucket goes empty. - `runManagedAutoMemoryDream` writes metadata after the fork agent returns (`bumpMetadata` → `rebuildManagedAutoMemoryIndex` → `updateDreamMetadataResult`). If the user presses 'x' between the fork's success return and these writes, the writes proceed and bump `lastDreamAt` — leaving the visible UI ('Stopped') disagreeing with the scheduler gate (sees a recent successful dream and suppresses the next cycle). manager.ts already short-circuits its own metadata write via the post-await abort check, but it can't block writes that already happened inside dream.ts. Adds the same abort-signal check between each write step here. * fix(core): swallow releaseDreamLock errors so they don't poison outcome If `releaseDreamLock` throws inside the inner finally (e.g. filesystem error on the lock file), the exception propagates to the outer catch and overwrites a successfully-completed dream record with 'failed'. The on-disk metadata is already up-to-date at that point, so the user sees a contradictory state — `lastDreamAt` was bumped but the UI shows a failure. Wrap the release in a try/catch with `debugLogger.warn`. The lock file is still cleaned up on the next session via the existing staleness sweep, so swallowing the release error doesn't risk a permanent stuck lock. * fix(core): thread abortSignal into dream metadata writes The pre-call abort checks in `runManagedAutoMemoryDream` close most of the late-cancel race window, but each metadata helper itself does read → mutate → write across two awaits. If the user cancels between those two awaits the write still happens, persisting `lastDreamAt` for an aborted run and suppressing the next legitimate dream cycle. Thread `abortSignal` into `bumpMetadata` and `updateDreamMetadataResult`; both now re-check between the read and the write, returning early without persisting when the signal has already fired. The pre-call checks remain as the first line of defense; this guards the race that opens after the call enters. * fix(core): close dream cancel race + surface lock-release failures Two related fixes from the latest review pass: - Move scheduler-gating metadata writes out of `runManagedAutoMemoryDream` and into `MemoryManager.runDream`, sequenced AFTER the status='completed' flip. The previous shape left a race window where cancellation arriving during/after `fs.writeFile` could persist `lastDreamAt` while the manager flipped status to 'cancelled' — visible UI ('Stopped') would disagree with the scheduler gate (sees a recent successful dream), suppressing the next legitimate dream cycle. The new order makes the gating metadata write race-free: once status !== 'running', cancelTask refuses, so any cancel arriving during the metadata write is ignored. The remaining "cancel raced the synchronous status update" window is handled by a post-update abort recheck that restores 'cancelled' and skips the metadata write. Drops the now-dead `bumpMetadata` helper from dream.ts; index rebuild stays there since it's informational, not gating. - Surface `releaseDreamLock` failures on the task record's metadata (`lockReleaseError`). The previous fix logged-and-swallowed only, so a Windows EPERM or ENOENT race would silently block subsequent dreams as 'locked' with no UI signal explaining why dreaming had stopped. Logger keeps emitting the warn for debug bundles. * fix(core,cli): address 8 review findings on dream surface Reverts the metadata-write behavior regression, plugs the storeWith reentrancy hole, surfaces lock/metadata warnings in the UI, and a handful of cleanups: - Wrap gating-metadata read+write in try/catch (manager.ts). The PR moved metadata writes from dream.ts (best-effort, swallowed) to manager.ts (unguarded). A throw from readDreamMetadata / writeDreamMetadata now propagates to the outer catch and overwrites a successfully-completed dream with 'failed' — the dream actually did its work and touched files are visible. New catch logs + writes `metadataWriteError` on the record so the UI can explain why the next dream may re-fire sooner than expected. - Register the AbortController BEFORE storeWith in scheduleDream. storeWith fires a notify; a subscriber synchronously calling cancelTask(record.id) would otherwise see status='running' but no controller, hitting the missing-controller defensive warn path and reporting a phantom failure on a brand-new dream. - Surface `lockReleaseError` and `metadataWriteError` in the dream view-model and DreamDetailBody (rendered as warnings, not errors, so the terminal status stays Completed). Previous fix wrote them to record.metadata only — nothing in the cli read or rendered them, so users still had no UI signal. - Preserve `result.touchedTopics` on the unreachable cancel-raced- status-update branch. If a future refactor introduces an await there, the restored cancelled record would otherwise drop the already-produced result; the UI would report a clean cancellation even though memory files were already modified. - Add `'cancelled'` to MemoryDreamEvent status union and emit a cancelled telemetry event from the runDream catch path. Without this a cancelled dream is indistinguishable from one that never scheduled in the first place. - Drop the dead abortSignal param from updateDreamMetadataResult in dream.ts (no caller passes it after the manager.ts move). - Swap declaration order of `computeDreamSig` and `refresh` in useBackgroundTaskView.ts (TDZ-fragile against a future refactor that adds a synchronous refresh call between them). - Update the unreachable cancel-raced-update branch comment to describe what's actually true ("defense-in-depth, unreachable today") instead of the confusing "cancelTask flipped" path. - cancelSelected now checks the cancelTask return value and logs via debugLogger when false. Today this branch is unreachable thanks to the controller-register-before-storeWith fix above, but if a future refactor breaks the invariant the silent ignore would let the user think the cancel took effect. - Mirror the manual /dream metadata path's `recentSessionIdsSinceDream = []` reset in the auto path — field is dead code today but keeping the two write sites in sync avoids surprises. Telemetry metric `recordMemoryDreamMetrics` widened to accept the new 'cancelled' status (downstream consumer in loggers.ts). * fix(memory): same-session recovery when releaseDreamLock throws The prior fix surfaced lockReleaseError on the dialog so the user knows the lock release failed (Windows EPERM, ENOENT race, disk full, etc.) — but until next process start, dreamLockExists() still sees a fresh-mtime lock owned by an alive PID (us!) and silently suppresses every subsequent scheduleDream() call as `{status: 'skipped', skippedReason: 'locked'}`. The user sees the warning AND zero further dream activity, and the staleness sweep that would clean the leaked lock only runs at session start. Adds a `dreamLockReleaseFailed` flag set in the catch. The next scheduleDream() force-cleans the leaked lock file via fs.rm({force: true}) before the existence check, so dream scheduling resumes within the same session. Best-effort: if even the forced rm fails (truly unrecoverable filesystem state), falls through to the existing 'locked' skip path. This is an incremental improvement on top of b00ecdebd's UI-surface fix. The two together give the full story: warning visible → automatic recovery on next attempt. * fix(memory): real cancelled-dream duration + clarify index-rebuild ordering Two follow-up suggestions from review: - **`duration_ms: 0` in cancelled-dream telemetry** (manager.ts): the user-cancel path emitted `MemoryDreamEvent` with `duration_ms: 0`, which would silently skew latency histograms / p95 metrics by treating cancelled dreams as instant. Capture `dreamStartMs = Date.now()` at the top of `runDream` and emit the real elapsed time in the cancel branch. - **Misleading index-rebuild comment** (dream.ts:75–84): the comment claimed the index rebuild "is still done before returning when topics were touched", but the code returns early on `abortSignal?.aborted` BEFORE the rebuild. Rewrote the comment to describe the actual cancel-aware ordering — abort returns partial result without rebuilding (rebuild is expensive; next dream cycle will rebuild against the latest files anyway), live path rebuilds only when topics changed. 22 / 22 manager tests pass; tsc clean. |
||
|---|---|---|
| .. | ||
| scripts | ||
| src | ||
| vendor | ||
| index.ts | ||
| package.json | ||
| test-setup.ts | ||
| tsconfig.json | ||
| vitest.config.ts | ||