Add /cd command - #4890
Conversation
…d-command # Conflicts: # packages/cli/src/ui/hooks/slashCommandProcessor.ts
|
Qwen Code review did not complete successfully: Qwen review timed out after 55 minutes. See workflow logs. |
| this.chatRecordingService?.resetStoragePaths(); | ||
| await this.refreshCurrentRuntimeStatus(expected); | ||
| this.workspaceContext.applyRootDirectories(workspaceDirectories); | ||
| this.fileDiscoveryService = null; |
There was a problem hiding this comment.
[Suggestion] After /cd, several project-scoped services are not invalidated: extensionManager (project-level extensions from the old directory remain loaded), hookSystem (hooks initialized with old config paths), toolRegistry (MCP server configs from the old project), and mcpServers. Only fileDiscoveryService, sessionService, fileHistoryService, and the file read cache are cleared.
After changing to a different project directory, project-level extensions, hooks, and MCP servers from the old project continue to operate. Users would expect the new directory's project configuration to take effect.
Consider invalidating or refreshing these services here, or document the limitation if a full refresh is too costly for a synchronous /cd operation.
— qwen3.7-max via Qwen Code /review
| } | ||
| const workspaceDirectories = WorkspaceContext.resolveRootDirectories( | ||
| expected, | ||
| this.explicitIncludeDirectories, |
There was a problem hiding this comment.
[Suggestion] this.explicitIncludeDirectories (set at startup from --include-dir CLI flag) is passed verbatim to resolveRootDirectories. After /cd to a completely different directory tree, old include directories remain in the workspace scope — even if they're outside the new project.
For example: qwen --include-dir /secret/project-b started in /home/user/project-a, then /cd /tmp — the workspace still includes /secret/project-b.
Consider clearing explicitIncludeDirectories on /cd, or at minimum warn the user that the original include dirs are still in scope.
— qwen3.7-max via Qwen Code /review
| ) => Promise<void>; | ||
| private initialized: boolean = false; | ||
| readonly storage: Storage; | ||
| storage: Storage; |
There was a problem hiding this comment.
[Suggestion] storage was readonly storage: Storage and is now storage: Storage (public mutable). relocateWorkingDirectory is the only code that reassigns this field, so external mutation is not intended. Keep it private to preserve encapsulation — other code that reads config.storage should go through a getter.
| storage: Storage; | |
| private storage: Storage; |
— qwen3.7-max via Qwen Code /review
| } | ||
| } | ||
|
|
||
| setRootDirectories( |
There was a problem hiding this comment.
[Suggestion] setRootDirectories has zero callers anywhere in the codebase. relocateWorkingDirectory uses resolveRootDirectories (static) + applyRootDirectories directly, bypassing this wrapper. Per the project's convention of not adding speculative abstractions, consider removing this until it's actually needed.
— qwen3.7-max via Qwen Code /review
| } from '../../config/trustedFolders.js'; | ||
| import { t } from '../../i18n/index.js'; | ||
|
|
||
| const pendingTrustedPathConfirmations = new Map<string, string>(); |
There was a problem hiding this comment.
[Suggestion] pendingTrustedPathConfirmations is a module-level Map that only shrinks when the user confirms (delete on line 156). If the user cancels or dismisses the trust prompt, the entry persists for the process lifetime. In long-running sessions or daemon mode, repeated /cd attempts to different untrusted directories cause unbounded growth.
Consider clearing stale entries — either at the start of each /cd invocation, on any non-confirming command, or with a simple size cap:
if (pendingTrustedPathConfirmations.size > 50) {
pendingTrustedPathConfirmations.clear();
}— qwen3.7-max via Qwen Code /review
Local runtime verification report (Linux)Built this branch ( What works well end-to-end
Also checked: reading a file from the old workspace after 🔴 Blocking: a migrated session can no longer be resumed — from anywhereRepro (real TUI):
Root cause: Net effect: The new unit tests verify the file moves but never attempt a resume after migration, which is how this slipped through — worth adding as a regression test alongside the fix. 🔴 CI is red, and both failures are PR-caused (not the known main flakes)
Minor notes (non-blocking)
ConclusionThe interactive |
|
Qwen Code review did not complete successfully: Qwen review timed out after 55 minutes. See workflow logs. |
| } | ||
| } | ||
|
|
||
| applyRootDirectories(resolved: ResolvedWorkspaceDirectories): void { |
There was a problem hiding this comment.
[Critical] applyRootDirectories replaces this.directories and this.initialDirectories with new sets built solely from the primary directory + explicitIncludeDirectories. Any directories previously added at runtime via /directory add (which mutates this.directories via addDirectory) are silently dropped after /cd. The user's workspace configuration is lost with no warning.
| applyRootDirectories(resolved: ResolvedWorkspaceDirectories): void { | |
| applyRootDirectories(resolved: ResolvedWorkspaceDirectories): void { | |
| const newDirectories = resolved.directories; | |
| const newInitialDirectories = resolved.initialDirectories; | |
| // Preserve runtime-added directories that aren't in the new initial set | |
| for (const existing of this.directories) { | |
| if (!this.initialDirectories.has(existing)) { | |
| newDirectories.add(existing); | |
| } | |
| } | |
| const directoriesChanged = | |
| newDirectories.size !== this.directories.size || | |
| ![...newDirectories].every((d) => this.directories.has(d)); | |
| const initialDirectoriesChanged = | |
| newInitialDirectories.size !== this.initialDirectories.size || | |
| ![...newInitialDirectories].every((d) => this.initialDirectories.has(d)); | |
| this.directories = newDirectories; | |
| this.initialDirectories = newInitialDirectories; | |
| this.resolvedPathCache.clear(); | |
| if (directoriesChanged || initialDirectoriesChanged) { | |
| this.notifyDirectoriesChanged(); | |
| } | |
| } |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Local runtime verification report #2 (Linux) — head
|
| Step | Result |
|---|---|
projA → message → /cd projB → quit → qwen --resume <id> from projB |
✅ resumes; history intact — model answers the earlier codeword without tools |
Second hop from the resumed session: /cd ../projC (relative path) |
✅ artifacts move again; runtime.json.work_dir → projC |
--resume picker from projC |
✅ session listed and resumes with both hops of history; relative read_file resolves in projC |
| Re-resume after picker-resume (runtime.json rewritten by new pid) | ✅ stays consistent (session_id unchanged, work_dir=projC) |
| Negative: picker from original projA | ✅ "No sessions found" (no duplicate/stale visibility) |
Negative: --resume <id> from projA |
✅ correctly rejected |
Sanitized-path collision (x y vs x-y sharing one chats dir) |
✅ disambiguation intact; each project sees only its own session — the new runtime-status fallback does not false-accept sessions lacking a runtime.json (headless -p sessions) |
The fix (sessionBelongsToCurrentProject: first-record cwd hash, else runtime.json.work_dir hash) matches what I suggested, and the new unit tests cover list + load of a migrated session.
✅ Other items from last report, re-verified on this head
- i18n:
en.jskey added;npm run check-i18npasses locally; CI Lint is now green. - Trust-confirmation cap:
MAX_PENDING_TRUST_CONFIRMATIONS = 50with unit test; normal trust flow re-verified live (dialog with read/edit/execute disclosure → Yes: move happens, then folder persisted totrustedFolders.json; No: clean abort — no move, no trust entry, no chats dir created). - Regression sweep: bare
/cd→ usage; nonexistent → "Couldn't find a directory"; file target → "is not a directory"; same dir → "Already in"; footer/cwd update; completion popup — all behave as before. - Unit tests: author-listed suites all pass locally — 114 cli + 458 core (incl. the new sessionService migration tests). CI: Ubuntu ✅ macOS ✅ CodeQL ✅.
🔴 Still open: Windows CI red on this PR's own tests (unchanged)
Test (windows-latest) fails on the same two PR-introduced tests as last time — config.test.ts > relocateWorkingDirectory should move current session artifacts… and …should refresh runtime status…, both AssertionError: expected "spy" to be called with arguments: [ …(2) ] (the fs.renameSync(old, new) / runtime-status path assertions). 2 failed / 10350 passed — these are the run's only failures, so it's not the known main-branch dev.test.js flake. Worth determining on a Windows machine whether it's only a test-mock/path-separator artifact or a real canonicalization issue in the relocation path (e.g. drive-letter casing feeding getProjectHash) — the latter would put migrated artifacts in a wrong project dir on real Windows.
🟡 New finding: the ownership fix only covers listSessions/loadSession — 7 other consumers still reject migrated sessions
sessionService.ts still has raw getProjectHash(records[0].cwd) !== this.projectHash checks in removeSession (L771), renameSession (L856), forkSession (L931), countSessionMessages (L423), findSessionsByTitle (L1060), the title-dedup scan (L1133), and sessionExists (L1176). Since the first record's cwd is forever the original directory, every one of these denies a migrated session in its new home. Live-verified consequences (real TUI):
/delete: the migrated session is listed (via the fixedlistSessions) but deleting it fails — "✕ Failed to delete session. Session not found." — file stays on disk. Since it's also gone from the old project, a migrated session becomes undeletable from anywhere via the UI.- In-TUI
/resume <id>: fails with "No session found with ID …" for a migrated session, while CLI--resume <same id>from the same directory succeeds —resumeCommandpre-validates with the unfixedsessionExists. /branchimmediately after/cd: "✕ No conversation to branch." despite a real conversation (control test without/cd: branching works) —branchCommandgates onsessionExists(currentId). So the live, just-moved session loses/branchuntil something rewrites its first record.- Same mechanism, not individually tested: resume-by-title (
gemini.tsx→findSessionsByTitle), ACP/ZedsessionExists/renameSessionby id. (/renameof the current session works — it goes through the recording service, notrenameSession.)
Suggested fix: route all of these through the same sessionBelongsToCurrentProject helper (one-line change per site), plus a regression test for "delete/branch/resume-in-TUI after /cd" to lock it in.
Conclusion
The previously blocking resume regression is genuinely fixed — the whole migrate → resume → re-migrate → re-resume chain now works, with sensible negative behavior and no collision regressions. Lint is green. Remaining before merge, in my view: (a) the Windows job is red on this PR's own two relocation tests and needs a fix or a Windows-side investigation; (b) the ownership-check fallback should be extended to the remaining seven call sites — /delete, in-TUI /resume, and /branch-after-/cd are user-visible breakages of the same root cause the last round fixed for resume, and /branch breaks in the live session right after using the new command. With (a) green and (b) routed through the shared helper, this looks merge-ready to me.
中文版
本地运行时验证报告 #2(Linux)— head cb6f9afba
在 Linux 6.12 / Node v22.22.2 上对更新后的分支做了第二轮验证:重新构建本 head,在 tmux 中用真实 TUI + 真实模型(glm-4.7)驱动,重点覆盖两个新 commit(c474ca806 trust 确认上限、cb6f9afba 迁移会话 resume 修复),并对上一份报告(基于 571f55045)中的场景做了回归扫查。
✅ 上轮阻塞缺陷已修复:迁移后的会话可以正常 resume
端到端重跑完整生命周期(真实 TUI、真实模型,每一跳都检查磁盘文件):
| 步骤 | 结果 |
|---|---|
projA → 发消息 → /cd projB → 退出 → 在 projB 执行 qwen --resume <id> |
✅ 成功恢复;历史完整——模型不调用任何工具直接答出此前的标记词 |
在 resume 出的会话里二跳:/cd ../projC(相对路径) |
✅ 工件再次迁移;runtime.json.work_dir → projC |
在 projC 打开 --resume picker |
✅ 会话被列出且可恢复,两跳历史完整;相对路径 read_file 在 projC 中解析 |
| picker 恢复后再次退出重连(runtime.json 已被新 pid 重写) | ✅ 保持一致(session_id 不变,work_dir=projC) |
| 反向验证:在原目录 projA 打开 picker | ✅ "No sessions found"(无重复/过期可见性) |
反向验证:在 projA 执行 --resume <id> |
✅ 正确拒绝 |
净化路径碰撞(x y 与 x-y 共享同一 chats 目录) |
✅ 消歧逻辑完好;各项目只看到自己的会话——新的 runtime-status fallback 不会误收没有 runtime.json 的会话(headless -p 会话) |
修复方案(sessionBelongsToCurrentProject:先比对首记录 cwd hash,否则回退到 runtime.json.work_dir hash)与我上轮的建议一致,且新增单测覆盖了迁移会话的 list + load。
✅ 上轮报告的其他事项,已在本 head 复验
- i18n:
en.jskey 已补;本地npm run check-i18n通过;CI Lint 已转绿。 - Trust 确认上限:
MAX_PENDING_TRUST_CONFIRMATIONS = 50并附单测;trust 流程实测正常(对话框含读/改/执行披露 → Yes:先迁移、后将目录持久化到trustedFolders.json;No:干净中止——不迁移、不写 trust 条目、不创建 chats 目录)。 - 回归扫查:裸
/cd→ usage;不存在的路径 → "Couldn't find a directory";文件目标 → "is not a directory";同目录 → "Already in";footer/cwd 更新、补全弹窗——行为与上轮一致。 - 单元测试:作者列出的套件本地全部通过——114 cli + 458 core(含新增 sessionService 迁移测试)。CI:Ubuntu ✅ macOS ✅ CodeQL ✅。
🔴 仍未解决:Windows CI 在本 PR 自带测试上仍红(与上轮相同)
Test (windows-latest) 失败的仍是同样两个本 PR 引入的测试——config.test.ts > relocateWorkingDirectory should move current session artifacts… 和 …should refresh runtime status…,均为 AssertionError: expected "spy" to be called with arguments: [ …(2) ](fs.renameSync(old, new) / runtime-status 路径断言)。2 failed / 10350 passed——是该 run 仅有的失败,不是 main 分支已知的 dev.test.js flake。建议在 Windows 机器上确认:只是测试 mock/路径分隔符问题,还是 relocation 路径上真实的规范化问题(例如盘符大小写进入 getProjectHash)——若是后者,真实 Windows 上迁移工件会落到错误的 project 目录。
🟡 新发现:ownership 修复只覆盖了 listSessions/loadSession——还有 7 处消费方仍拒绝迁移会话
sessionService.ts 中仍有裸的 getProjectHash(records[0].cwd) !== this.projectHash 校验:removeSession(L771)、renameSession(L856)、forkSession(L931)、countSessionMessages(L423)、findSessionsByTitle(L1060)、title 去重扫描(L1133)、sessionExists(L1176)。由于首记录的 cwd 永远是原目录,这些校验都会在新家拒绝迁移会话。实测后果(真实 TUI):
/delete:迁移会话会被列出(经由已修复的listSessions),但删除失败——"✕ Failed to delete session. Session not found."——文件留在磁盘。又因为旧项目里也没有它,迁移会话在 UI 层面从任何地方都删不掉。- TUI 内
/resume <id>:对迁移会话报 "No session found with ID …",而同目录下 CLI--resume <同一 id>却成功——resumeCommand用未修复的sessionExists做前置校验。 /cd后立即/branch:"✕ No conversation to branch.",尽管对话真实存在(对照组:不/cd时分支正常)——branchCommand依赖sessionExists(currentId)。也就是说刚迁移完的活动会话立即失去/branch能力。- 同一机制、未逐一实测:按标题 resume(
gemini.tsx→findSessionsByTitle)、ACP/Zed 的按 idsessionExists/renameSession。(对当前会话的/rename正常——它走 recording service,不经过renameSession。)
建议修复:把上述全部调用点统一路由到 sessionBelongsToCurrentProject helper(每处一行改动),并补一条"/cd 之后 delete/branch/TUI 内 resume"的回归测试。
结论
上轮的阻塞性 resume 缺陷已确实修复——迁移 → resume → 再迁移 → 再 resume 的完整链路现在可用,反向行为合理,碰撞消歧无回归,Lint 已绿。我认为合并前还需:(a) Windows job 在本 PR 自带的两个 relocation 测试上仍红,需要修复或在 Windows 侧排查;(b) ownership 校验的 fallback 应扩展到其余七处调用点——/delete、TUI 内 /resume、/cd 后 /branch 是与上轮 resume 同根因的用户可见破坏,且 /branch 在刚用完新命令的活动会话里立即失效。(a) 转绿、(b) 统一走共享 helper 后,我认为即可合并。
|
Qwen Code review did not complete successfully: Qwen review timed out after 55 minutes. See workflow logs. |
Resolve dev.test.js by taking main's version: the branch carried its own copy of the Windows path-normalization fix, and the auto-merge with main's variant left a duplicate, unused module-level normalizePath that failed lint (no-unused-vars). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No high-confidence critical or suggestion issues found. The /cd command implementation is well-structured with proper rollback logic, symlink TOCTOU guards, trust confirmation, and comprehensive test coverage. Lint, typecheck, and all 523 tests pass. Remaining concerns from prior review rounds (session ownership at 7 call sites, Windows CI) are already tracked in existing comments. ✅ — qwen3.7-max via Qwen Code /review
|
Qwen Code review did not complete successfully: Qwen review timed out after 55 minutes. See workflow logs. |
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
No new review findings on this pass. The blocking issue from the earlier review (migrated sessions not resumable) has been resolved in commits cb6f9af and 0a18c8d. The sessionBelongsToCurrentProject fallback via runtime status is sound, and applyRootDirectories now correctly preserves runtime-added directories. All CI code-quality checks pass (Lint, Test on Linux/macOS/Windows, CodeQL). The only failing check is the review-pr workflow itself, which is CI infrastructure, not a code defect. Downgraded from Approve to Comment: CI reporting review-pr as failing. -- claude-sonnet-4 via Qwen Code /review
Local runtime verification report #3 (Linux) — head
|
Scenario (after projA → msg → /cd projB → /cd ../projC relative hop) |
#2 | Now |
|---|---|---|
/delete the migrated session from projC |
listed but "Failed to delete" — undeletable anywhere | ✅ deletes; .jsonl verified gone from disk |
in-TUI /resume <id> of the migrated session |
"No session found with ID …" | ✅ switches with full history (model recalls codeword without tools) |
/rename + quit + --resume <title> from projC (findSessionsByTitle) |
rejected (same root cause) | ✅ resumes by title, both hops of history intact |
Negative checks still correct on this head: --resume <id> and the picker from projA/projB (old homes) are rejected / show "No sessions found" — the fallback doesn't leak sessions across projects. renameSession/countSessionMessages (ACP-facing) are covered by the new unit tests (cli 114 + core 481 all pass locally).
✅ New fix f92d746a0 verified: runtime-added workspace dirs survive /cd
/dir add /tmp/cd3/projX → /cd /tmp/cd3/projB → /dir show now lists projB + projX (previously the runtime-added dir was wiped by the root swap); a live read_file of projX/x.txt returns its contents, so the preserved dir is functionally part of the workspace, not just display. The old root projA is correctly dropped from /dir show. (Note: read_file of a path outside the workspace also succeeds in default approval mode — verified with a no-/cd control session, so that's baseline tool behavior, not a /cd hole.)
✅ Regression sweep on this head
Absolute + relative /cd hops with artifact migration and runtime.json.work_dir updates; trust flow with folderTrust enabled (dialog with read/edit/execute disclosure → No: "Operation cancelled.", no move, no trust entry, no chats dir created; Yes: move + trustedFolders.json entry persisted); bare /cd → usage; nonexistent → "Couldn't find a directory"; file target → "is not a directory"; same dir → "Already in"; footer cwd updates; directory completion popup — all as before.
🔴 Still broken, new failure mode: /branch after /cd — fork load fails and leaks an orphaned session file
Report #2's /branch breakage has moved one layer deeper rather than gone away. Live sequence projA → msg → /cd projB → /branch:
- Where is the config saved? #2 (old):
✕ No conversation to branch.(sessionExistsrejected the migrated parent — now fixed) - This head:
✕ Failed to branch conversation: Failed to load newly forked session
Root cause (code + disk evidence): forkSession copies the records rewriting sessionId to the new UUID but keeps each record's cwd (forever the pre-/cd projA). useBranchCommand.ts then calls loadSession(newSessionId) (step 4) before config.startNewSession (step 5) — but for the fork, sessionBelongsToCurrentProject(newId, cwd=projA) fails both ways: the first-record hash mismatches, and the runtime-status fallback reads chats/<newId>.runtime.json, which is only written by startNewSession. Ordering deadlock — the load that gates the swap can never pass for a fork of a migrated session. (Control: /branch without a prior /cd works fine on this head; in that flow the first-record hash matches so the sidecar isn't needed at load time.)
Worse, the failure leaks state: step 3 already created the fork .jsonl on disk, and the catch block rolls back core state but never unlinks it. The orphan (first-record cwd=projA, no runtime.json) is then invisible to listSessions from both projA (file isn't in its chats dir) and projB (ownership check fails) — so it can't be deleted via the UI from anywhere, and it doesn't migrate on subsequent /cd hops (verified: it stayed in projB's chats dir after the parent moved on to projC). Every /branch attempt after a /cd leaks one such file.
Suggested fix:
- Make the fork belong to where it's created: have
forkSessionstamp the copied records'cwd(at minimum the first record) with the current target dir — or giveloadSessiona skip-ownership option for the just-created fork (the caller knows it made it). Stamping also fixes the picker showing the stale projA path for the fork. - In the
useBranchCommandcatch path, best-effort delete the fork file created in step 3 so a failed branch doesn't strand artifacts. - One trap for the fix: on a successful branch the parent's
runtime.jsonis superseded by the fork's (observed in the control run — after/branch, only the fork had a sidecar). For a migrated parent that sidecar is exactly what keeps it reachable, so a fix that only makes the fork loadable would leave the parent invisible afterwards — and the post-branch hint "To resume the original:/resume <parentId>" would fail. Re-stamping the fork records' cwd (option 1) avoids this only for the fork; the parent likely needs its sidecar preserved or its first record re-stamped during relocation. - Regression test: "fork/branch a session that was relocated via
/cd, then load + list both fork and parent."
🟡 Footnote (pre-existing, not this PR)
/delete removes the session .jsonl but leaves its .runtime.json sidecar behind — reproduced with a never-migrated control session too, and the sidecar infra predates this PR, so not a regression here. Worth an eventual cleanup since stale sidecars now participate in ownership decisions.
Conclusion
Both report-#2 merge blockers are genuinely resolved and live-verified: Windows CI is green via a legitimate test-only fix, and the ownership unification makes /delete, in-TUI /resume, and title-based resume work on migrated sessions, with cross-project rejection intact. The workspace-dirs preservation fix also checks out. The one remaining user-visible defect is /branch after /cd: still broken on this head (new failure mode further down the same root-cause chain) and now leaking undeletable orphaned session files per attempt. Since /branch is the documented escape hatch right after /cd (the success path even advertises /resume of the original), I'd fix this before merge — items 1–2 above are small and the rest of the command is in good shape.
中文版
本地运行时验证报告 #3(Linux)— head 7addd9756
第三轮验证,Linux 6.12 / Node v22.22.2:worktree 同步到本 head,真实 npm ci + 完整 bundle 重建(bundle 内嵌 GIT_COMMIT_INFO: 7addd9756),tmux 中以真实 TUI + 真实模型(glm-4.7)驱动,重点覆盖报告 #2 之后的四个 commit(f86547e4d、0a18c8d3a、f92d746a0、merge 7addd9756)。每一跳都检查磁盘状态(~/.qwen/projects/<dir>/chats/*.jsonl + *.runtime.json)。
✅ 报告 #2 阻塞项 (a) 已解决:Windows CI 转绿,且修复仅涉及测试
f86547e4d 把两个失败的 relocation 测试从 new Storage(baseParams.targetDir)(原始路径)改为 new Storage(config.getTargetDir())(生产代码实际使用的规范化路径)——证实失败是测试夹具的路径规范化问题,不是真实的 relocation 缺陷。Test (windows-latest) 现已通过(20m59s),ubuntu/macos/Lint/CodeQL 同绿。唯一的红色检查是 Qwen review bot 超时,与本变更无关。
✅ 报告 #2 阻塞项 (b) 已解决:所有 ownership 校验接受迁移会话——已实测
0a18c8d3a 把其余 7 处裸 getProjectHash(records[0].cwd) 比较(removeSession、renameSession、forkSession、countSessionMessages、findSessionsByTitle、title 去重、sessionExists)统一路由到共享的 sessionBelongsToCurrentProject helper——至此全部 9 处 ownership 校验都走该 helper——并新增单测。报告 #2 的三个用户可见破坏,真实 TUI 端到端重测:
场景(projA → 发消息 → /cd projB → 相对路径 /cd ../projC 二跳后) |
#2 | 现在 |
|---|---|---|
在 projC /delete 迁移会话 |
列出但 "Failed to delete"——任何地方都删不掉 | ✅ 删除成功;磁盘确认 .jsonl 已移除 |
TUI 内 /resume <id> 迁移会话 |
"No session found with ID …" | ✅ 切换成功且历史完整(模型不用工具直接答出标记词) |
/rename + 退出 + 在 projC --resume <title>(findSessionsByTitle) |
同根因被拒 | ✅ 按标题恢复,两跳历史完整 |
反向校验在本 head 仍正确:从 projA/projB(旧家)--resume <id> 被拒、picker 显示 "No sessions found"——fallback 不会让会话跨项目泄露。renameSession/countSessionMessages(ACP 侧)由新单测覆盖(本地 cli 114 + core 481 全部通过)。
✅ 新修复 f92d746a0 验证通过:运行时添加的 workspace 目录在 /cd 后保留
/dir add /tmp/cd3/projX → /cd /tmp/cd3/projB → /dir show 现在列出 projB + projX(此前根替换会清掉运行时添加的目录);实测 read_file 读取 projX/x.txt 返回内容,说明保留的目录是功能性的 workspace 成员而非仅显示。旧根 projA 正确地从 /dir show 移除。(注:默认审批模式下 read_file 读 workspace 外路径也会成功——用无 /cd 的对照会话验证过,这是工具基线行为,不是 /cd 留下的口子。)
✅ 本 head 回归扫查
绝对 + 相对路径 /cd 多跳、工件迁移、runtime.json.work_dir 更新;trust 流程(启用 folderTrust:对话框含读/改/执行披露 → No:"Operation cancelled."、不迁移、不写 trust、不建 chats 目录;Yes:迁移 + trustedFolders.json 持久化);裸 /cd → usage;不存在路径 → "Couldn't find a directory";文件目标 → "is not a directory";同目录 → "Already in";footer cwd 更新;目录补全弹窗——全部与此前一致。
🔴 仍然损坏、新失败形态:/cd 后 /branch——fork 加载失败并泄漏孤儿会话文件
报告 #2 的 /branch 破坏没有消失,而是下沉了一层。实测序列 projA → 发消息 → /cd projB → /branch:
- Where is the config saved? #2(旧):
✕ No conversation to branch.(sessionExists拒绝迁移的父会话——现已修复) - 本 head:
✕ Failed to branch conversation: Failed to load newly forked session
根因(代码 + 磁盘证据):forkSession 复制记录时把 sessionId 改写为新 UUID,但保留每条记录的 cwd(永远是 /cd 前的 projA)。useBranchCommand.ts 随后在 config.startNewSession(第 5 步)之前调用 loadSession(newSessionId)(第 4 步)——而对 fork 来说,sessionBelongsToCurrentProject(newId, cwd=projA) 两条路都不通:首记录 hash 不匹配,runtime-status fallback 要读 chats/<newId>.runtime.json,而该文件恰恰要等 startNewSession 才写入。时序死锁——门控切换的加载对"迁移会话的 fork"永远不可能通过。(对照组:本 head 上不经 /cd 的 /branch 正常;该流程首记录 hash 直接匹配,加载时不需要边车文件。)
更糟的是失败会泄漏状态:第 3 步已经在磁盘创建了 fork .jsonl,catch 块只回滚内存状态、从不删除它。这个孤儿(首记录 cwd=projA、无 runtime.json)对 projA(文件不在它的 chats 目录)和 projB(ownership 校验失败)的 listSessions 都不可见——UI 层面从任何地方都删不掉,后续 /cd 也不会带走它(实测:父会话迁到 projC 后它仍留在 projB 的 chats 目录)。/cd 之后每尝试一次 /branch 就泄漏一个文件。
修复建议:
- 让 fork 归属于它的创建地:
forkSession把复制记录的cwd(至少首记录)改写为当前 target dir——或给loadSession加一个跳过 ownership 的选项供刚创建 fork 的调用方使用(调用方明确知道是自己刚建的)。改写 cwd 同时能修正 picker 中 fork 显示过期 projA 路径的问题。 useBranchCommand的 catch 路径 best-effort 删除第 3 步创建的 fork 文件,失败的 branch 不应留下工件。- 修复时的一个坑:branch 成功路径上父会话的
runtime.json会被 fork 的取代(对照组观察到——/branch后只有 fork 有边车文件)。对迁移过的父会话,这个边车恰是它保持可达的依据,只让 fork 可加载的修复会让父会话事后不可见——branch 成功后提示的 "To resume the original:/resume <parentId>" 会失效。方案 1 的 cwd 改写只救 fork;父会话大概率需要在 relocation 时保留边车或改写首记录。 - 回归测试:"fork/branch 一个经
/cd迁移过的会话,然后 load + list fork 和父会话"。
🟡 脚注(既有行为,非本 PR)
/delete 删除会话 .jsonl 但留下 .runtime.json 边车——用从未迁移的对照会话同样复现,且边车基建先于本 PR 存在,不算本 PR 回归。但既然过期边车现在参与 ownership 判定,值得后续清理。
结论
报告 #2 的两个合并阻塞项都已确实解决并经实测:Windows CI 通过正当的测试侧修复转绿;ownership 统一让 /delete、TUI 内 /resume、按标题恢复在迁移会话上全部可用,且跨项目拒绝完好。workspace 目录保留修复同样验证通过。剩余的唯一用户可见缺陷是 /cd 后 /branch:本 head 仍坏(同根因链上更深一层的新失败形态),且每次尝试都泄漏无法删除的孤儿会话文件。鉴于 /branch 是 /cd 之后的标准逃生通道(成功路径甚至会提示 /resume 原会话),建议合并前修复——上面第 1–2 项改动很小,命令的其余部分状态良好。
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
LGTM. Well-structured implementation with thorough test coverage. Key areas reviewed:
- Trust confirmation flow: Correctly resolves paths before prompting and re-validates after confirmation (TOCTOU protection via realpath comparison).
- Session artifact migration: Proper rollback on failure, including cross-device (EXDEV) fallback with copy+unlink and cleanup of partial copies.
relocateWorkingDirectory: Sound ordering — finalize/flush recording service, move artifacts, then update config state. Rollback restores cwd, storage, and transcript path.sessionBelongsToCurrentProject: Correct use of runtime status sidecar to identify migrated sessions whose first-record cwd no longer matches the project hash.parseSlashCommandwhitespace fix: ThecommandText.slice(argsStart).trim()approach correctly preserves internal whitespace in command arguments, withindexOf(part, argsStart)tracking consumed positions to avoid false matches when argument text repeats a command/subcommand name.applyRootDirectories: Correctly preserves runtime-added (non-initial) directories when swapping roots.- Branch cleanup: Good addition of
removeSessionfor failed branch sessions.
No blocking issues found.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No high-confidence findings. Downgraded from Approve to Comment: CI failing (review-pr). Implementation is well-structured with proper rollback logic, symlink TOCTOU guards, trust confirmation flow, and comprehensive test coverage (595 tests pass, tsc/eslint clean). 8 low-confidence suggestions identified for human review (stale SessionStart context, Config state consistency around async gaps, parseSlashCommand global whitespace change, EXDEV fallback hardening, test coverage gaps, observability logging). 5 prior-round inline comments at current commit not re-reported. — qwen3.7-max via Qwen Code /review
Local runtime verification (maintainer)Built this PR locally ( Environment: macOS (Darwin 25.5.0), Node v22.22.2, tmux 3.6a. Focused unit tests (from the PR test plan)
Interactive verification matrix (real TUI, real model)
Observations (non-blocking)
Verdict: runtime behavior matches the PR description on macOS — error paths, trust flow, session migration, context refresh, completion, and resume-after-relocation all work as advertised in real usage. |
* feat(cli): add /cd command * fix(cli): stabilize cd command checks * fix(cli): cap pending cd trust confirmations * fix(core): allow resuming migrated cd sessions * test(core): fix cd relocation path expectations * fix(core): accept migrated sessions in all project checks * fix(core): preserve runtime workspace dirs on cd * fix(cli): keep branched relocated sessions loadable --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
5 participants
What this PR does
Adds a
/cd <path>slash command that changes the active session working directory without restarting the CLI. The command validates the target directory, prompts before trusting a new workspace path, updates workspace roots and cwd-backed services, migrates the active session transcript/runtime/worktree files to the new workspace session directory, refreshes system and directory context, and keeps path completion usable for directory arguments.Why it's needed
Users currently need to restart the CLI when they want the session to operate from a different project directory. This makes multi-repo or parent/child workspace navigation awkward, leaves no first-class way to move the active tool context during a session, and would otherwise leave the active conversation artifacts attached to the previous workspace after changing directories.
Reviewer Test Plan
How to verify
Run the focused CLI tests for the new slash command, completion/parser behavior, command registration, and slash command processor integration. Run the focused core tests for config relocation, client context refresh, workspace context root replacement, and session artifact migration. Build, lint, and typecheck the repo to confirm the change compiles across packages.
Evidence (Before & After)
Before:
/cdwas not registered as a builtin command, so users could not change the session working directory from inside the CLI. After:/cd <path>validates and applies a new working directory, handles trust confirmation for untrusted paths, moves active session artifacts into the destination workspace session directory, refreshes directory/system context, and reports non-fatal memory refresh failures as warnings.Tested on
Environment (optional)
Local macOS worktree with Node.js project dependencies already installed. Verified with
npx vitest run src/ui/commands/cdCommand.test.ts src/ui/commands/directoryCommand.test.tsx src/utils/commands.test.ts src/services/BuiltinCommandLoader.test.ts src/ui/hooks/slashCommandProcessor.test.tsfrompackages/cli,npx vitest run src/config/config.test.ts src/core/client.test.ts src/utils/workspaceContext.test.tsfrompackages/core, focused reruns ofnpx vitest run src/ui/commands/cdCommand.test.tsandnpx vitest run src/config/config.test.ts, plusnpm run build,npm run typecheck,npm run lint -- --quiet, and Prettier check for the touched files.Risk & Scope
npm run preflight, Windows manual verification, and Linux manual verification were not run locally.Linked Issues
Closes #4879
中文说明
What this PR does
新增
/cd <path>slash command，用于在不重启 CLI 的情况下切换当前会话的工作目录。该命令会校验目标目录，在新 workspace 路径未被信任时请求确认，更新 workspace roots 和依赖 cwd 的服务，把当前会话的 transcript/runtime/worktree 文件迁移到新 workspace 的 session 目录，刷新 system/directory context，并为目录参数保留路径补全能力。Why it's needed
用户现在如果想让会话从另一个项目目录运行，通常需要重启 CLI。多仓库或父子 workspace 之间切换时这很不方便，也缺少一个在会话中移动 active tool context 的一等入口；如果只切换 cwd 而不迁移会话文件，当前 conversation artifact 仍会留在旧 workspace 下。
Reviewer Test Plan
How to verify
运行聚焦的 CLI 测试，覆盖新 slash command、补全/解析行为、命令注册和 slash command processor 集成。运行聚焦的 core 测试，覆盖 config relocation、client context refresh、workspace context root replacement 和 session artifact migration。再运行 build、lint 与 typecheck，确认跨 package 编译通过。
Evidence (Before & After)
Before:
/cd不是 builtin command，用户无法在 CLI 内切换会话工作目录。After:/cd <path>会校验并应用新的工作目录，为未信任路径处理确认流程，把当前 session artifacts 移到目标 workspace 的 session 目录，刷新 directory/system context，并把非致命的 memory refresh 失败作为 warning 返回。Tested on
Environment (optional)
本地 macOS worktree，项目依赖已安装。验证命令包括在
packages/cli下运行npx vitest run src/ui/commands/cdCommand.test.ts src/ui/commands/directoryCommand.test.tsx src/utils/commands.test.ts src/services/BuiltinCommandLoader.test.ts src/ui/hooks/slashCommandProcessor.test.ts，在packages/core下运行npx vitest run src/config/config.test.ts src/core/client.test.ts src/utils/workspaceContext.test.ts，本轮聚焦重跑npx vitest run src/ui/commands/cdCommand.test.ts和npx vitest run src/config/config.test.ts，以及从仓库根目录运行npm run build、npm run typecheck、npm run lint -- --quiet和 touched files 的 Prettier check。Risk & Scope
npm run preflight，也没有进行 Windows 和 Linux 手工验证。Linked Issues
Closes #4879