feat(cli): add configurable plansDirectory for Plan Mode

[shenyankm]

Summary

• What changed: Added a plansDirectory setting for Plan Mode so approved plan files can be stored in a project-specific directory instead of always using the global default.
• Why it changed: This resolves Add configurable plansDirectory setting for Plan Mode (like Gemini CLI / Claude Code) #3548 by allowing users to keep plans inside the project and grant narrower write access for Plan Mode workflows.
• Reviewer focus: Please review path resolution, rejection of directories outside the project root, and the default fallback behavior when plansDirectory is unset.

Validation

• Commands run:

  cd packages/core && npx vitest run src/config/storage.test.ts src/config/config.test.ts
  cd packages/cli && npx vitest run src/config/config.test.ts src/config/settingsSchema.test.ts
  npm run build
  npm run typecheck

• Prompts / inputs used: N/A. This change was validated through focused configuration and storage tests rather than manual TUI interaction.

• Expected result:

  • Relative plansDirectory values resolve from the project root.
  • Paths outside the project root are rejected.
  • The default plan directory remains ~/.qwen/plans when unset.
  • CLI config, generated schema, docs, and plan persistence remain valid.

• Observed result:

  • Core config/storage tests passed: 197 tests.
  • CLI config/schema tests passed: 220 tests, 2 skipped.
  • npm run build completed successfully.
  • npm run typecheck completed successfully.

• Quickest reviewer verification path:

  1. Add "plansDirectory": "./.qwen/plans" to project .qwen/settings.json.
  2. Start Qwen Code from that project.
  3. Enter Plan Mode and approve a plan.
  4. Confirm the plan file is written under .qwen/plans.
  5. Change the setting to "../plans" and confirm startup fails with a configuration error.

• Evidence:

  • Automated tests cover default behavior, relative and absolute custom directories, path traversal rejection, symlink escape prevention, Windows cross-drive handling, mixed separators, CLI config wiring, schema exposure, and atomic plan writes.

Scope / Risk

• Main risk or tradeoff: Path validation is intentionally strict; configured plan directories must resolve inside the project root.
• Not covered / not validated: Manual TUI verification was not run.
• Breaking changes / migration notes: No breaking change. Existing behavior is preserved when plansDirectory is unset.

Testing Matrix

	🍏	🪟	🐧
npm run	⚠️	✅	⚠️
npx	⚠️	✅	⚠️
Docker	⚠️	⚠️	⚠️
Podman	⚠️	N/A	⚠️
Seatbelt	⚠️	N/A	N/A

Testing matrix notes:

• Validated on Windows only.
• Docker, Podman, and Seatbelt were not exercised because this change is limited to configuration resolution and plan file path handling, not sandbox behavior.

Linked Issues / Bugs

Closes #3548

[wenshao]

Critical: When users change plansDirectory from the default (~/.qwen/plans) to a project-local path, all existing plan files become invisible — loadPlan() silently returns undefined with no migration warning. Consider checking the old default location and emitting a console warning if plans exist there but not in the new location.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[shenyankm]

@wenshao 已按您的 review 全部对应处理，并确认最新 CI 通过：

• 对默认 ~/.qwen/plans 迁移风险增加了 legacy plan 检查；配置 plansDirectory 后，如果旧默认目录里还有未迁移的 .md plan，会通过 warning 提醒用户。
• 路径包含性校验改为沿父级向上查找已存在 ancestor 后做 realpath，覆盖 project/data 为 symlink 且 ./data/subdir/plans 中间目录不存在的绕过场景。
• symlink 相关测试改为 mock realpathSync，不再因为系统 EPERM/权限限制而跳过核心断言。
• savePlan / loadPlan 在执行 I/O 前会重新校验配置的 plansDirectory，降低构造后路径变化带来的 TOCTOU 风险。
• 补充了 projectRoot 缺失时 Storage.getPlansDir(undefined/null, './plans') 的错误路径测试。

最新 checks 已全部通过：Linux / Windows / macOS tests、Lint、CodeQL 都是 green。

[wenshao]

Additional Critical finding — test gap:

packages/core/src/config/config.test.ts: No happy-path test for savePlan() / loadPlan() when plansDirectory is configured. All existing I/O tests use the default directory. The only configured-directory I/O test (TOCTOU revalidation) mocks realpathSync to always throw — meaning the actual write/read paths for a custom directory are never verified to succeed. A regression in path.join(this.plansDir, ...) or the atomic-write sequence would go unnoticed.

---

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[shenyankm]

Reply to review #4291847967

Thank you for the thorough review. All findings have been addressed in commit 80add66aa. Below is a point-by-point response:

---

1. [Critical] renameSync fails with EXDEV on cross-device mounts ✅ Fixed

Added EXDEV fallback in packages/core/src/config/config.ts: when renameSync throws EXDEV, we fall back to copyFileSync(tmpPath, filePath) followed by unlinkSync(tmpPath). This preserves atomic-write intent while supporting cross-device scenarios.

---

2. [Critical] plansDirectory schema category doesn't match its actual JSON path ✅ Fixed

Updated docs/users/configuration/settings.md to restore the top-level keys carve-out: "Most settings should be placed within their corresponding top-level category object. A few top-level settings like proxy and plansDirectory remain direct root keys for compatibility." This aligns the documentation with the actual runtime behavior where settings.plansDirectory is read as a root key.

---

3. [Critical] Test gap — no happy-path I/O test for configured plansDirectory ✅ Fixed

Added should save and load plan from configured plansDirectory test in packages/core/src/config/config.test.ts. It mocks the full fs round-trip (save → rename → load) with an in-memory Map to verify the atomic-write sequence succeeds when plansDirectory is set to a custom relative path.

---

4. [Suggestion] getPlanFileNames only handles ENOENT ✅ Fixed

Extended error handling in packages/core/src/config/config.ts to cover EACCES and EPERM. When these occur, we push a user-visible warning to this.warnings and return [] instead of re-throwing. This prevents the legacy migration check from being silently swallowed.

---

5. [Suggestion] getPlanFilePath trusts sessionId without path-traversal validation ✅ Fixed

Introduced Storage.sanitizePlanSessionId(sessionId) in packages/core/src/config/storage.ts which normalizes backslashes, applies path.basename(), strips leading dots and Windows-invalid filename characters, and falls back to _ if empty. Config.getPlanFilePath() now uses this sanitizer, closing the SDK-level traversal vector.

---

6. [Suggestion] ~ (tilde) in configured plansDirectory is not expanded ✅ Fixed

Replaced manual path.resolve logic with Storage.resolvePath() in packages/core/src/config/storage.ts, which already handles tilde expansion (~ → os.homedir()). Added test coverage for this case.

---

7. [Suggestion] bareMode silently discards plansDirectory ✅ Fixed

Removed the bareMode ? undefined : ternary in packages/cli/src/config/config.ts. plansDirectory is now preserved in bare mode since plan file storage is not security-sensitive. Added test should preserve plansDirectory in bare mode.

---

8. [Suggestion] Dead code: Storage.getPlanFilePath() has no production callers ✅ Fixed

Added an inline comment in packages/core/src/config/storage.ts: "Kept for tests and SDK callers that still use Storage helpers directly." The method is retained for backward compatibility with external SDK consumers and test convenience.

---

9. [Suggestion] Narrow TOCTOU window between validation and I/O ✅ Fixed

After renameSync (or copyFileSync fallback) completes, savePlan() in packages/core/src/config/config.ts now calls assertPlanFilePathWithinTargetDir(filePath) to re-validate the final resolved path. If containment fails, we unlinkSync(filePath) to roll back the write and re-throw the error.

---

10. [Suggestion] addLegacyPlanLocationWarning errors only go to debugLogger ✅ Fixed

Errors in addLegacyPlanLocationWarning now push to both this.warnings (user-visible) and this.debugLogger.warn (diagnostic) in packages/core/src/config/config.ts, matching the pattern used elsewhere in the class.

---

All changes have been validated with:

• npm run build ✅
• npm run typecheck ✅
• npm run lint ✅
• Core config tests: 155 passed ✅
• Core storage tests: 63 passed ✅
• CLI config tests: all passed ✅

[wenshao]

No new issues found. All 13 findings from the previous review round have been addressed. LGTM! ✅

4 pre-existing tsc errors remain in packages/cli/src/config/config.test.ts (lines 1867, 1872, 1885, 1899) — getMcpServers() undefined handling in MCP config parsing tests. These predate this PR and are not caused by these changes.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

No new issues found in the incremental changes. All 11 previously flagged concerns are addressed: sessionId sanitization, symlink-aware containment checks, atomic writes with EXDEV fallback, tilde expansion, legacy migration warnings, EACCES/EPERM handling, and bareMode fix. Build and all 444 tests pass.

LGTM! ✅ — DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

Local tmux verification transcript, for the record.

1. Anti-corruption sanity (post-3850 lesson)

$ wc -l + head -1 on PR-touched source files
  3544  packages/core/src/config/config.ts        /**
   399  packages/core/src/config/storage.ts       /**
  1749  packages/cli/src/config/config.ts         /**
  2175  packages/cli/src/config/settingsSchema.ts /**

All four files start with the license /** header and have real content (no @path placeholder corruption). Latest commit 80add66aae is dated 2026-05-15 10:58Z, before my two approving reviews at 11:53Z and 23:51Z — no post-approval force-push.

2. PR-listed core tests

$ cd packages/core && npx vitest run --no-coverage src/config/storage.test.ts src/config/config.test.ts

 RUN  v3.2.4 /tmp/pr4062/packages/core

 ✓ src/config/storage.test.ts (63 tests) 25ms
 ✓ src/config/config.test.ts (155 tests) 113ms

 Test Files  2 passed (2)
      Tests  218 passed (218)
   Duration  2.27s

(PR body lists 197; the actual head has 218 — extra coverage added through the review iteration.)

3. PR-listed cli tests

$ cd packages/cli && npx vitest run --no-coverage src/config/config.test.ts src/config/settingsSchema.test.ts

 ✓ src/config/settingsSchema.test.ts (18 tests) 10ms
 ✓ src/config/config.test.ts (210 tests | 2 skipped) 569ms

 Test Files  2 passed (2)
      Tests  226 passed | 2 skipped (228)
   Duration  3.69s

(PR body lists 220 + 2 skipped; head has 226 + 2 skipped.)

Summary

Check	Result
Anti-corruption file sanity	clean (3544/399/1749/2175 lines, license headers intact)
Post-approval force-push check	none (latest commit pre-dates both approvals)
core storage + config vitest	218 passed
cli config + settingsSchema vitest	226 passed + 2 skipped
Total local tests	444 passed + 2 skipped
Remote CI	Lint / CodeQL / Test mac · ubuntu · windows all SUCCESS
Reviews	2× APPROVED (wenshao, 5/15 11:53Z + 23:51Z), 13 threads all resolved, 0 Critical

Environment

• Linux x86_64, Node v20.19.2 (project engines require ≥22; only the project's own doctorChecks Node-version assertion reflects this in CI's matrix)
• PR head 80add66aae against main merge-base from PR's own merge log
• Worktree at /tmp/pr4062 with node_modules and core/dist symlinked from a parent install

[wenshao]

@shenyankm sorry for the late ping — wanted to merge this just now and hit:

$ gh pr merge 4062 --squash --delete-branch
X Pull request QwenLM/qwen-code#4062 is not mergeable: the merge commit cannot be cleanly created.

mergeStateStatus: DIRTY, mergeable: CONFLICTING. The PR has been sitting since 5/15 and main has moved a fair bit since, particularly across packages/cli/src/config/settingsSchema.ts and the core config.ts — recently merged PRs #4208 (stop hook blocking cap) and #4231 / #4232 (daemon client identity / scoped permission route) all added settings entries in the same areas you touched, so the conflicts are likely there.

Could you:

git fetch origin main
git merge origin/main          # or: git rebase origin/main
# resolve conflicts (mostly settingsSchema.ts additions ordering)
git push                       # or: git push --force-with-lease if you rebased

I've enabled auto-merge — once the conflicts are resolved and CI re-runs green, it'll merge automatically without needing me back in the loop. No new review needed; the approvals carry through unless something substantive shifts during the rebase.

The verification I posted earlier still stands for 80add66aae; if the rebase only touches conflict-resolution lines I won't re-run anything.

[wenshao]

Re-approving after the upstream merge fe3b4e4cbd "Merge upstream/main into sheny/plan-mode-plans-directory". The rebase resolved the conflicts cleanly; PR-only intent is unchanged.

Sanity check (post-3850 reflex)

All four PR-touched files have real content after the merge, no @path placeholder corruption:

3644 lines  packages/core/src/config/config.ts        (was 3544 — picked up main's additions)
 399 lines  packages/core/src/config/storage.ts
1840 lines  packages/cli/src/config/config.ts         (was 1749 — picked up main's additions)
2196 lines  packages/cli/src/config/settingsSchema.ts (was 2175 — picked up main's additions)

All start with /** license header.

Local re-verification on fe3b4e4cbd

$ cd packages/core && npx vitest run src/config/storage.test.ts src/config/config.test.ts
 ✓ src/config/storage.test.ts (63 tests) 24ms
 ✓ src/config/config.test.ts (159 tests) 118ms
 Tests  222 passed (222)

$ cd packages/cli && npx vitest run src/config/config.test.ts src/config/settingsSchema.test.ts
 ✓ src/config/settingsSchema.test.ts (18 tests) 11ms
 ✓ src/config/config.test.ts (220 tests | 2 skipped) 593ms
 Tests  236 passed | 2 skipped (238)

Test counts ticked up vs my pre-rebase run (218 → 222 in core, 226 → 236 in cli) — the extras came from main during the merge, not new code in this PR. No regressions.

Remote CI: Lint / CodeQL / Classify PR already SUCCESS on fe3b4e4cbd; Test mac · ubuntu · windows still IN_PROGRESS.

Re-enabling auto-merge.

LGTM.