feat(cli): default auto-dream/auto-skill to on and add /memory toggle#4547
Merged
Conversation
Contributor
📋 Review SummaryThis PR makes two related changes to the managed-memory experience: (1) defaults 🔍 General Feedback
🎯 Specific Feedback🟡 HighFile: Currently the implementation has:
Fix: In the 🟢 MediumFile: File: 🔵 LowFile:
These appear correct but would benefit from native speaker confirmation if available. File: ✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
|
Walking through the review points: Auto-dream → list navigation (🟡 High) — false positive. The current code at MemoryDialog.tsx:312-314 already routes
i18n native-speaker confirmation (🔵 Low) — |
wenshao
requested changes
May 26, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR aligns managed-memory defaults by enabling Auto-dream and Auto-skill by default, and exposes an Auto-skill toggle in the /memory dialog with workspace persistence.
Changes:
- Default
memory.enableManagedAutoDreamandmemory.enableAutoSkilltotrueacross config/schema sources. - Add an
Auto-skilltoggle row to the CLI/memoryTUI (including keyboard navigation and persistence). - Update i18n strings and regenerate the VS Code companion settings schema.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vscode-ide-companion/schemas/settings.schema.json | Regenerates schema to reflect new default values for managed-memory settings. |
| packages/core/src/config/config.ts | Changes core Config defaults for Auto-dream and Auto-skill to enabled. |
| packages/cli/src/config/settingsSchema.ts | Updates CLI settings schema defaults for Auto-dream and Auto-skill. |
| packages/cli/src/config/config.ts | Updates CLI config loader defaults for Auto-dream and Auto-skill. |
| packages/cli/src/ui/components/MemoryDialog.tsx | Adds Auto-skill toggle row and updates focus/keypress handling for it. |
| packages/cli/src/ui/components/MemoryDialog.test.tsx | Extends test mock config to include getAutoSkillEnabled(). |
| packages/cli/src/i18n/locales/* | Adds localized string for Auto-skill: {{status}}. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenshao
reviewed
May 26, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Review (re-run, no new code changes since prior review)
Deterministic analysis: tsc clean, eslint clean (0 findings)
Build: pass | Tests: 181 pass (2 MemoryDialog + 179 config)
Suggestions
[Suggestion] Docs: settings reference shows stale defaults
docs/users/configuration/settings.md (line ~277) still shows memory.enableManagedAutoDream defaulting to false, but this PR changes the default to true in all code layers. Additionally, memory.enableAutoSkill (now also defaulting to true) is not documented in this table at all. Users consulting the docs reference will see the wrong default value. Consider updating the Default column to true and adding an enableAutoSkill row.
[Suggestion] Tests: no config default assertions for new true defaults
packages/cli/src/config/config.test.ts has no test asserting the new true defaults for getManagedAutoDreamEnabled() or getAutoSkillEnabled(). The bare-mode test (line ~2395) only asserts getManagedAutoMemoryEnabled() returns false. A future refactor that accidentally reverts either default would go undetected. Consider adding default-mode assertions for all three memory toggle getters.
Prior review Criticals (already discussed, not re-reported)
The 2 Criticals from the prior review round (bareMode guard on enableManagedAutoDream, behavioral tests for autoSkill toggle) remain open and were addressed by the author in the discussion thread.
— qwen3.7-max via Qwen Code /review
|
This PR shares modified code with PR #4533 (feat(skills): /skills picker dialog). Shared functions/files:
Recommendation: These PRs both modify CLI configuration and skill/auto feature behavior. Consider reviewing and merging them together to avoid merge conflicts. This conflict was detected automatically by CodeScope skills — structural analysis of call graphs and file modifications. |
|
This PR shares modified code with PR #4533 (feat(skills): /skills picker dialog). Shared functions/files:
Recommendation: These PRs both modify CLI configuration and skill/auto feature behavior. Consider reviewing and merging them together to avoid merge conflicts. This conflict was detected automatically by CodeScope skills — structural analysis of call graphs and file modifications. |
wenshao
reviewed
May 26, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No new review findings on re-run (code unchanged since prior review at 0c195c8). Downgraded from Approve to Comment: CI failing (copilot-pull-request-reviewer). Prior inline Critical comments (bareMode guard, behavioral tests) remain open. — qwen3.7-max via Qwen Code /review
Bring the managed memory pipeline closer to its intended out-of-the-box experience: auto-dream and auto-skill now default to enabled (matching the existing auto-memory default), so users get summarized memories and reusable project skills without having to opt in. The /memory dialog previously only exposed Auto-memory and Auto-dream toggles. With auto-skill now on by default, users need an equally discoverable way to opt out, so this adds an Auto-skill row alongside the existing two with the same focus/Enter toggle semantics and workspace-scoped persistence (memory.enableAutoSkill). Default-value updates are kept consistent across all three sources of truth (settings schema, CLI loader, core Config), and the generated vscode settings.schema.json is regenerated to match.
The new Auto-skill toggle row reads config.getAutoSkillEnabled() at render time; without it on the mocked config the component throws and the existing list-navigation tests assert against an empty frame.
- enableManagedAutoDream in loadCliConfig was missing the bareMode guard that its two siblings already had; once the default flipped to true, this caused a raw-field inconsistency in bare-mode sessions (the getter still returned false via its own !getBareMode() guard, but the Config.enableManagedAutoDream field itself was now true). - docs/users/configuration/settings.md still listed enableManagedAutoDream's default as false, and was missing the new enableAutoSkill row entirely. Both fixed. - MemoryDialog.test.tsx now covers the autoSkill row render, the new focus chain (list ↑ autoSkill ↑ autoDream and back down), and the Enter-toggle path that writes memory.enableAutoSkill to workspace settings. - config.test.ts gains a non-bare default test asserting all three getManaged*Enabled() / getAutoSkillEnabled() return true, and the bare-mode test now asserts auto-dream/auto-skill also resolve to false in bare mode.
0c195c8 to
e3de1b5
Compare
Collaborator
Author
|
Round 4 wrap-up — commit
All inline threads resolved. |
wenshao
reviewed
May 27, 2026
Collaborator
✅ Local Verification Report — PR #4547Maintainer local verification on macOS in tmux. All four steps of the Reviewer Test Plan reproduce green — including the actual TUI exercise (open Environment
Targeted unit tests
Reviewer Test Plan — TUI verification (the PR's main UX claim)Built ✅ Step 1: All three rows present and read ✅ Step 2 — focus chain (each capture below is the visible
Round-trip is exact and matches the PR description. ✅ Step 3 — Enter toggles + persists: with {
"memory": {
"enableAutoSkill": false
}
}✅ Step 4 — explicit user setting overrides default: relaunched the CLI against the workspace that now has
Coverage of the bare-mode fix (commit
|
Collaborator
Author
|
Acknowledged — no further changes from my side. PR is ready when you are. |
wenshao
approved these changes
May 27, 2026
wenshao
reviewed
May 27, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] docs/users/features/memory.md — stale /memory dialog description after adding third toggle. Line 106 says "You can turn off just the automatic saving, just the periodic cleanup, or both" (implies exactly two toggles; now three). Lines 127-128 bullet list omits auto-skill. JSON example at line ~113 shows only enableManagedAutoMemory + enableManagedAutoDream, missing enableAutoSkill.
— qwen3.7-max via Qwen Code /review
| expect(lastFrame()).toContain('Auto-skill: off'); | ||
| }); | ||
|
|
||
| it('chains focus list ↑ autoSkill ↑ autoDream ↑ autoMemory and back down', () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] Test name says "chains focus list ↑ autoSkill ↑ autoDream ↑ autoMemory and back down" but the test body never navigates to autoMemory — it stops at autoDream and reverses. The autoDream → autoMemory UP transition is not covered.
Either extend the body:
// autoDream ↑ → autoMemory
pressKey({ name: 'up' });
expect(lastFrame()).toContain('› Auto-memory:');
// autoMemory ↓ → autoDream
pressKey({ name: 'down' });
expect(lastFrame()).toContain('› Auto-dream:');Or rename to match actual coverage: 'chains focus list ↑ autoSkill ↑ autoDream and back down'.
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
May 27, 2026
| it('renders the Auto-skill row with the status from config', () => { | ||
| const { lastFrame } = render(<MemoryDialog onClose={vi.fn()} />); | ||
|
|
||
| // beforeEach mocks getAutoSkillEnabled => false |
Collaborator
There was a problem hiding this comment.
[Suggestion] Test name says "chains focus list ↑ autoSkill ↑ autoDream ↑ autoMemory and back down" but the test body never navigates to autoMemory — it stops at autoDream and reverses. The autoDream → autoMemory UP transition is not covered.
Either extend the body:
// autoDream ↑ → autoMemory
pressKey({ name: 'up' });
expect(lastFrame()).toContain('› Auto-memory:');
// autoMemory ↓ → autoDream
pressKey({ name: 'down' });
expect(lastFrame()).toContain('› Auto-dream:');Or rename to match actual coverage: 'chains focus list ↑ autoSkill ↑ autoDream and back down'.
— qwen3.7-max via Qwen Code /review
pomelo-nwu
approved these changes
May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Two related changes to the managed-memory experience around the
/memorydialog. First, two settings that previously defaulted to off —memory.enableManagedAutoDreamandmemory.enableAutoSkill— now default to on, matching the existingmemory.enableManagedAutoMemorydefault and aligning the three managed-memory subsystems behind a single "enabled by default" policy. Second, the/memorydialog now exposes anAuto-skilltoggle row next to the existingAuto-memoryandAuto-dreamrows, with the same focus/Enter semantics and workspace-scoped persistence (writesmemory.enableAutoSkill). The default-value change is mirrored across all three sources of truth (settings schema, CLI loader, coreConfig), and the generatedvscode-ide-companion/schemas/settings.schema.jsonis regenerated to match.Why it's needed
The managed-memory pipeline gives users summarised memories (
auto-memory), periodic consolidation (auto-dream), and reusable project skills (auto-skill) — but only the first was on by default. The other two required knowing the settings path to opt in, so most users never saw them. Defaulting all three on lets new and existing users get the full pipeline without configuration. The newAuto-skillrow in/memorykeeps the opt-out discoverable now that the feature is enabled by default — without it,Auto-skillwould be the only managed-memory subsystem with no visible kill switch in the dialog.Reviewer Test Plan
How to verify
memory.*settings, runqwenand open/memory. All three rows (Auto-memory,Auto-dream,Auto-skill) should readon.↑from the file list moves throughAuto-skill→Auto-dream→Auto-memory;↓reverses the path back to the list. Each toggle row shows›and a success-coloured prefix when focused.Auto-skillfocused, pressEnter— the row flipson↔off, and the workspace.qwen/settings.jsongains amemory.enableAutoSkill: <bool>entry. Re-opening the dialog reflects the stored value.memory.enableAutoSkill: falseexplicitly set in settings, the dialog still loads withAuto-skill: off— the new default never overrides an explicit user value.Evidence (Before & After)
Before — only two toggle rows; a fresh workspace shows
Auto-dream: off:```
│ Memory │
│ │
│ Auto-memory: on │
│ Auto-dream: off · never · /dream to run │
│ │
│ › 1. User memory Saved in ~/.qwen/QWEN.md │
│ 2. Project memory Saved in QWEN.md │
│ 3. Open auto-memory folder │
```
After — three toggle rows; a fresh workspace shows all three
on:```
│ Memory │
│ │
│ Auto-memory: on │
│ Auto-dream: on · never · /dream to run │
│ Auto-skill: on │
│ │
│ › 1. User memory Saved in ~/.qwen/QWEN.md │
│ 2. Project memory Saved in QWEN.md │
│ 3. Open auto-memory folder │
```
Focus moves to the new row after pressing
↑from the file list:```
│ Auto-memory: on │
│ Auto-dream: on · never · /dream to run │
│ › Auto-skill: on │
```
Pressing
Entertoggles the row and persists to workspace settings (.qwen/settings.jsonafter a single toggle):```json
{ "memory": { "enableAutoSkill": false } }
```
Tested on
Environment (optional)
Local
dist/cli.jsvianpm run build && npm run bundle. Manual TUI verification under tmux (200×50). Workspace.qwen/settings.jsonconfirmed to receivememory.enableAutoSkillafter toggling. Typecheck + lint pass. The e2e suite was started but interrupted at ~18 minutes (8 files passing, remainder hit unrelated API connection errors).Risk & Scope
auto-skillon by default exposes more users to its background extraction work — a behaviour change for existing users who never opted in. Mitigated by the new in-dialog toggle and by the fact that explicit user settings continue to override the default.memory.enableManagedAutoDreamormemory.enableAutoSkillset in their settings keep their value; only users relying on the implicit default see new behaviour.Linked Issues
N/A
中文说明
本 PR 做了什么
围绕 `/memory` dialog 的两项相关改动。其一：原本默认关闭的 `memory.enableManagedAutoDream` 与 `memory.enableAutoSkill` 现在默认开启，与已经默认开启的 `memory.enableManagedAutoMemory` 对齐，把 managed-memory 的三个子系统统一为"默认启用"。其二：`/memory` dialog 在 `Auto-memory` 与 `Auto-dream` 两行旁新增 `Auto-skill` toggle 行，焦点 / Enter 切换语义、workspace 级持久化（写入 `memory.enableAutoSkill`）均与现有两项保持一致。默认值改动在三处 source of truth（settings schema、CLI loader、core `Config`）同步更新，并重新生成 `vscode-ide-companion/schemas/settings.schema.json`。
为什么需要
managed-memory 管道由三块组成：摘要记忆（`auto-memory`）、阶段性整理（`auto-dream`）、可复用项目 skill（`auto-skill`）—— 但只有第一项默认开启。后两项需要用户知道 settings 字段才能启用，绝大多数人压根没见过。把三者默认全部开启，可以让新老用户直接获得完整 pipeline，无需手动配置。`Auto-skill` 默认开启后也需要一个直观的关闭入口，新增 toggle 行就是给它一个对等的 kill switch——否则 `Auto-skill` 会成为唯一在 dialog 里没有可见关闭开关的 managed-memory 子系统。
Reviewer 验证步骤
如何验证
Before / After 证据
参见上方英文段的 tmux capture 截图。
测试环境
运行环境（可选）
通过 `npm run build && npm run bundle` 产出 `dist/cli.js`，tmux（200×50）下做了手动 TUI 验证。toggle 后 workspace 的 `.qwen/settings.json` 确认写入 `memory.enableAutoSkill`。typecheck + lint 通过。e2e suite 启动后约 18 分钟被中止（8 个文件全过，剩余受到与本 PR 无关的 API 连接错误影响）。
风险与范围
关联 Issue
N/A