feat(cli): reload project runtime after /cd - #10263
Conversation
|
github-actions
Bot
added
the
review/self-reported
|
|
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first (before reading the diff): extend No critical blockers found. Transaction boundaries are right: Two follow-ups, both non-blocking:
Relocation flowsequenceDiagram
participant P1 as cd or ACP caller
participant P2 as Config relocateWorkingDirectory
participant P3 as ProjectRuntimeReloader
participant P4 as LoadedSettings
participant P5 as tools hooks permissions skills MCP
P1->>P2: relocate to target dir
P2->>P3: prepare target settings read-only
P3-->>P2: prepared config with commit and rollback
Note over P2,P3: prepare failure rejects before any state moves
P2->>P4: commit settings swap, pause workspace watcher
P2->>P2: chdir and verify realpath, roll back on mismatch
P2->>P2: apply project runtime config
P2->>P5: reload permissions, tools, hooks fail-closed, skills, subagents
P2->>P5: reset and refresh memory, reinitialize MCP servers
P2->>P5: refresh system instruction and tool declarations
P2->>P5: fire CwdChanged hook with old and new cwd
P2->>P3: complete, resume watcher on the new project
P2-->>P1: success, non-blocking failures reported as warnings
Files changed (30 of 52 shown)
Test evidenceUnattended CI run — no PR code is built or executed in triage; the evidence below is the PR's own CI fetched via the API at the reviewed commit. At review time the two material checks are still running: the main Node suite ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: Real-scenario testing: not driven in this run (unattended CI path — live TUI testing is reserved for the isolated 中文说明代码审查：先独立给出方案再对照 diff——本 PR 的做法与独立方案高度一致：以 prepare/commit/rollback 事务扩展 测试证据：本次为无人值守 CI 运行，不在分诊中构建或执行 PR 代码；证据来自 API 拉取的 PR 自身 CI。评审时两个关键检查仍在运行（ubuntu Node 22 主套件、SDK Java daemon E2E）；已完成的全部通过；macOS/Windows Node 测试按 ci.yml 设计仅在 merge queue/定时/手动触发时运行，PR 上为 skipped，属预期。CI 落定后表格会原地更新。沙盒验证建议： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean review across every stage; only non-blocking follow-ups, and the approval waits on CI, not on doubt. Stepping back: my independent proposal for this problem was, almost step for step, what this PR implements — a prepare/commit/rollback relocation transaction owned by core, driven by the CLI's settings loader, refreshing permissions → tools → hooks → skills/subagents → memory → MCP, with session-owned tools surviving the swap and both TUI and ACP on the same path. Where it goes beyond that baseline it does so in the right places: a fail-closed hook reload (a broken reload drops configured hooks rather than silently keep executing the previous project's), the settings watcher paused across the swap so the old directory is never watched mid-transition, and managed include directories replaced while runtime-added ones survive. The test suite (~840 lines) pins exactly where this kind of change leaks: apply/reject/rollback paths, session-tool preservation, permission-rule replacement with session rules intact, the CwdChanged payload, cron restart on relocation. The problem itself was source-verified during issue triage, and the direction has a direct external reference (Claude Code 2.1.246 shipped the same What keeps this at 4 rather than 5: a handful of read sites still consult the process-global context filename instead of the per-config one (serve memory route, ACP CI status: the main Node suite and the daemon E2E were still running at review time, so no approval is posted in this run. Approval is deferred until CI lands green on the reviewed commit; the marker below carries it. 中文说明置信度：4/5 —— 各阶段审查均干净；只有非阻断的跟进项，等待的是 CI 而不是因为存疑。 退一步看：我对这个问题的独立方案与本 PR 几乎逐步一致——由 core 拥有、CLI settings loader 驱动的 prepare/commit/rollback 迁移事务，依次刷新权限 → 工具 → hooks → skills/子智能体 → memory → MCP，会话级工具在切换中保留，TUI 与 ACP 走同一路径。超出基线的部分也都用在正确的地方：hooks 以 fail-closed 方式重载（重载失败时丢弃已配置 hooks，而不是悄悄继续执行上一个项目的）、切换期间暂停设置监听以避免过渡期仍监听旧目录、受管 include 目录被替换而运行时新增目录得以保留。测试套件（约 840 行）恰好钉住了这类改动最容易泄漏的位置：应用/拒绝/回滚路径、会话工具保留、权限规则替换且会话规则不受影响、CwdChanged 载荷、迁移后 cron 重启。问题本身已在 issue 分诊时于源码中核实，方向也有直接外部参照（Claude Code 2.1.246 已上线相同的 之所以是 4 而不是 5：少数读点仍读取进程级全局上下文文件名而非按配置的（serve memory 路由、ACP CI 状态：评审时主 Node 套件与 daemon E2E 仍在运行，因此本次不发布 approve。批准延迟到 CI 在评审的 commit 上全绿；由下方标记承接。 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| async refreshForProjectChange(): Promise<void> { | ||
| try { | ||
| await this.refreshCache(); |
There was a problem hiding this comment.
[Suggestion] SubagentManager.refreshForProjectChange and SkillManager.refreshForProjectChange (skill-manager.ts:516) have no direct tests — they appear in test code only as bare vi.fn().mockResolvedValue(undefined) mocks in config.test.ts, whose new relocation test does not even assert their call count.
The subagent variant has a specific error path: on refreshCache() failure it deletes the 'project' cache entry, notifies listeners, and rethrows. Mutant: remove the catch block — the rethrow still propagates into projectRuntimeRefreshErrors via config.ts:5826-5829, so error reporting is unchanged, but subagentsCache keeps the old project's subagents enumerated in the new project; no test turns red. The skill variant's conditional updateWatchersFromCache() (only when watchStarted) is equally unwitnessed.
Add a subagent-manager unit test where refreshCache rejects, asserting the project cache entry is removed, change listeners fire, and the error rethrows; add a skill-manager case asserting updateWatchersFromCache runs only when watching has started. Removing the catch cleanup (or the watcher condition) must turn the corresponding test red.
中文说明
SubagentManager.refreshForProjectChange 与 SkillManager.refreshForProjectChange（skill-manager.ts:516）没有直接测试——它们在测试代码中只是 config.test.ts 里的裸 vi.fn().mockResolvedValue(undefined) mock，新的迁移测试甚至没有断言其调用次数。
subagent 变体有特定错误路径：refreshCache() 失败时删除 'project' 缓存条目、通知监听者并重新抛出。突变：移除 catch 块——重抛仍会经 config.ts:5826-5829 传进 projectRuntimeRefreshErrors，错误报告不变，但 subagentsCache 会在新项目里继续枚举旧项目的子智能体；没有测试变红。skill 变体的条件式 updateWatchersFromCache()（仅当 watchStarted）同样没有见证。
建议新增 subagent-manager 单元测试（refreshCache 拒绝时断言项目缓存条目被移除、监听者被通知、错误重抛）和 skill-manager 用例（断言仅在监听启动时运行 updateWatchersFromCache）；删除对应清理后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Not in this commit — these two helpers' own unit tests are coverage work on code this PR did not change in this round, and I kept the commit to the findings with a runtime effect. Leaving the thread open.
中文说明
本次提交未处理——这两个 helper 各自的单元测试属于对本轮未改动代码的覆盖工作，本次提交只包含有运行时影响的发现。该讨论保持打开。
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
Resolves the context-filename conflicts by keeping this branch's session-scoped `config.getContextFileNames()` design and adopting main's renamed globals (`getAllMemoryFilenames` / `setMemoryFilename` / `memoryFileCount`) as the fallbacks. Test mocks of memory-constants export both spellings.
…e reload
The eleven Critical findings, plus the Suggestions that describe runtime
behaviour rather than coverage alone.
Critical:
- R1-1 `commit()` now applies the target's `.env`/`settings.env` to
`process.env` (`reloadEnvironment`, as serve's workspaceReload does)
and `rollback()` restores the previous directory's. `prepare()`
takes the directory being left for that; the core interface gained
the parameter.
- R1-2 A bare session's `/cd` projects from `createMinimalSettings()`
instead of loading the real user-scope files.
- R1-3 `resolveProjectSkillPath` expands home spellings first, so
`%userprofile%` skill directories survive (they were nailed under
the project).
- R1-4 Permission-rule persistence never routes through a minimal
LoadedSettings (`resolvePersistenceSettings`).
- R1-5 `reloadScopeFromDisk` re-runs the migration for a scope that was
migrated in memory, so the first hot-reload after the move no longer
regresses to the legacy layout.
- R1-6 Non-string `context.fileName` entries are dropped instead of throwing
out of a half-committed relocation; the apply step is also wrapped so
`complete()` (which resumes the watcher) is always reached.
- R1-7 Session-scoped context names threaded through the remaining
consumers: `/directory add`, the TUI memory refresh, the ACP
`getMemoryPaths` request (answered from the session owning the cwd),
and optional parameters on the two daemon memory helpers.
- R1-8 `applyProjectRuntimeConfig` clears the legacy `hooks` fallback so a
hook-less target cannot revive the previous project's hooks.
- R1-9 The CwdChanged fire site checks `getDisableAllHooks()` like every
other fire site.
- R1-10 `clearProjectRuntimeTools` removes command-discovered tools only;
core tools and factories survive a failed refresh.
- R1-11 Command discovery skips names the session owns.
Suggestions with a runtime effect:
- R1-14 the reloader replicates startup's `tool_search` denial (explicit
setting, or the session model captured once at startup)
- R1-15 `agents` goes through the same projection startup uses
(`projectAgentsSettings`), so `team.*` no longer appears after /cd
- R1-17 the resumed workspace watcher reconciles against disk
- R1-23 the cron work the swap cancelled is reported (`cronExitSummary`)
by both consumers instead of vanishing with the destroyed scheduler
- R1-26 runtime-added directories are not absorbed into the managed set
when an intermediate project happens to list them
Coverage the review asked for: R1-12 (session-owned registration), R1-13
(a real-file reloader suite), R1-16, R1-18, R1-19, R1-20, R1-22 (both
rollback sites), R1-24. Left as-is: R1-21 (the wider settings-parity audit
is a separate change) and R1-25 (refresh helpers' own unit tests).
Mutation-verified in three batches, 19 reverts against the full affected
suites: each revert reddens exactly its own test(s) and nothing else.
Claude-Session: https://claude.ai/code/session_01VXsC4f71S6U6YkW82NRw7m
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": verifying the settings merge layer ( mergeSettings in packages/cli/src/config/settings.ts) passes a non-array security.allowedHttpHookUrls value through to …; "agent reverse-audit (round 2)": checking whether resolveDisabledSlashCommands and the remaining applyProjectRuntimeConfig spreads share the same unguarded shape.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/config/config.ts:1922 — [probe] complete() success-path watcher resume is never executed by any testpackages/cli/src/config/config.ts:1708 — [probe] cronEnabledOverride wired but never exercised through the real reloaderpackages/core/src/config/config.test.ts:242 (+3 locations) — [review] Three relocate call sites unpinned: reload failClosed, tool-swap catch, client refreshpackages/cli/src/config/settings.ts:757 — [review] Target-project settings warnings collected on /cd but never surfaced
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：verifying the settings merge layer ( mergeSettings in packages/cli/src/config/settings.ts) passes a non-array security.allowedHttpHookUrls value through to …；"agent reverse-audit (round 2)"：checking whether resolveDisabledSlashCommands and the remaining applyProjectRuntimeConfig spreads share the same unguarded shape。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| this.enableTeamMemorySync = runtime.enableTeamMemorySync ?? false; | ||
| this.enableAutoSkill = runtime.enableAutoSkill ?? false; | ||
| this.autoSkillConfirm = runtime.autoSkillConfirm ?? true; | ||
| this.agentsSettings = runtime.agents ?? {}; |
There was a problem hiding this comment.
[Critical] R2-1: /cd stores the target project's agents settings here, but the background-agent concurrency caps derived from them (agents.maxParallelAgents / maxParallelAgentsByModel) are baked into BackgroundTaskRegistry at Config construction — private readonly fields with no setter, and the registry field itself is readonly. A session starting in project A (default cap 10) that /cds into project B with agents.maxParallelAgents: 1 (e.g. to bound parallel background agents against a rate-limited model) keeps admitting up to project A's cap: getAgentsSettings() reports 1 while tryReserveBackgroundSlot / waitForBackgroundSlot enforce 10; symmetrically a restrictive startup cap keeps throttling a project that loosens it. Before this PR agentsSettings was never mutated post-construction, so this staleness is newly introduced. Same class as round 1's R1-21.
Witness (probe against the real relocateWorkingDirectory):
capBefore=10 capAfter=10 agentsSettings.maxParallelAgents=1 registryIdentityPreserved=true
Adding a registry setter + sync in applyProjectRuntimeConfig flips the probe to capAfter=1.
Suggested fix: add a BackgroundTaskRegistry.setConcurrencyLimits(options) mirroring the constructor's validation (re-pumping waitQueue when a cap rises) and call it here after the agentsSettings assignment.
Fix witness: extend the relocation suite to prepare a runtime with agents: { maxParallelAgents: 2 } and assert the registry's max-concurrent value is 2 after relocate — it must go red when the sync is removed.
中文说明
/cd 在此存储目标项目的 agents 设置，但由其派生的后台智能体并发上限（agents.maxParallelAgents / maxParallelAgentsByModel）在 Config 构造时就已固化进 BackgroundTaskRegistry——这些是无 setter 的 private readonly 字段，registry 字段本身也是 readonly。在项目 A（默认上限 10）启动的会话 /cd 到设置了 agents.maxParallelAgents: 1 的项目 B（例如为限速模型约束并行后台智能体数量）后，仍会按项目 A 的上限放行：getAgentsSettings() 报告 1，而 tryReserveBackgroundSlot / waitForBackgroundSlot 实际执行 10；反向同理，启动时的严格上限会继续限制放宽了设置的项目。本 PR 之前 agentsSettings 从不在构造后被修改，因此这一过期状态是本 PR 新引入的。与第 1 轮的 R1-21 同类。
证据（针对真实 relocateWorkingDirectory 的探针）：迁移后 capBefore=10 capAfter=10 agentsSettings.maxParallelAgents=1；为 registry 增加 setter 并在 applyProjectRuntimeConfig 中同步后，探针翻转为 capAfter=1。
建议修复：为 BackgroundTaskRegistry 增加 setConcurrencyLimits(options)（复用构造函数的校验，上限放宽时重新唤醒 waitQueue），并在此处 agentsSettings 赋值后调用。
修复见证：在迁移测试套件中准备 agents: { maxParallelAgents: 2 } 的运行时，断言迁移后 registry 的最大并发值为 2；移除同步逻辑后该测试应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| mcpServerCommand?: string; | ||
| mcpToolIdleTimeoutMs?: number; | ||
| disabledSkillLevels?: readonly SkillLevel[]; | ||
| customSkillDirs?: readonly string[]; |
There was a problem hiding this comment.
[Critical] R2-15: ProjectRuntimeConfig — the reload surface this PR defines — omits worktree.symlinkDirectories, a project-scoped setting read into Config at construction only (private readonly worktreeSettings) yet consumed dynamically at tool-call time by enter_worktree and agent worktree isolation. The schema marks it requiresRestart: false, and this reloader already applies settings marked requiresRestart: true (cron, plansDirectory), so no restart-flag rationale justifies the omission. Trigger: project A sets worktree.symlinkDirectories: ["shared-secrets"]; after /cd to project B (["node_modules"]), enter_worktree in B symlinks B's shared-secrets directory into the new worktree — against B's declared settings — while B's node_modules entry is silently ignored. Same class as R2-1 / R1-21. (The sibling memory.agentTimeoutMinutes/agentMaxTurns knobs declare requiresRestart: true and are excluded from this finding.)
Witness (probe against the real relocateWorkingDirectory):
AssertionError: expected [ 'shared-secrets' ] to deeply equal [ 'node_modules' ]
With worktree added to ProjectRuntimeConfig and assigned in applyProjectRuntimeConfig the same probe passes.
Suggested fix: add worktree?: WorktreeSettings to the interface, populate it in createProjectRuntimeReloader mirroring startup (runtimeSettings.worktree ? { symlinkDirectories: runtimeSettings.worktree.symlinkDirectories } : undefined, bare/safe → undefined), drop readonly from worktreeSettings and assign this.worktreeSettings = runtime.worktree ?? {} in applyProjectRuntimeConfig.
Fix witness: a relocation test constructing Config with worktree: { symlinkDirectories: ['a'] }, relocating with a runtime overriding ['b'], asserting getWorktreeSymlinkDirectories() is ['b'] — red without the assignment.
中文说明
本 PR 定义的重载面 ProjectRuntimeConfig 遗漏了 worktree.symlinkDirectories：该项目级设置只在构造时读入 Config（private readonly worktreeSettings），却在工具调用时被 enter_worktree 和智能体 worktree 隔离动态读取。Schema 将其标为 requiresRestart: false，且本 reloader 已经会应用标为 requiresRestart: true 的设置（cron、plansDirectory），因此没有重启标志上的理由可以解释这一遗漏。触发场景：项目 A 设置 worktree.symlinkDirectories: ["shared-secrets"]，/cd 到项目 B（["node_modules"]）后，在 B 中执行 enter_worktree 会把 B 的 shared-secrets 目录软链进新 worktree——与 B 的声明设置相悖——而 B 的 node_modules 条目被静默忽略。与 R2-1 / R1-21 同类。（同类兄弟字段 memory.agentTimeoutMinutes/agentMaxTurns 声明为 requiresRestart: true，不在本条范围内。）
证据（针对真实 relocateWorkingDirectory 的探针）：AssertionError: expected [ 'shared-secrets' ] to deeply equal [ 'node_modules' ]；在接口中加入 worktree 并在 applyProjectRuntimeConfig 赋值后同一探针通过。
建议修复：接口增加 worktree?: WorktreeSettings，在 createProjectRuntimeReloader 中按启动逻辑填充（runtimeSettings.worktree ? { symlinkDirectories: runtimeSettings.worktree.symlinkDirectories } : undefined，bare/safe 下为 undefined），去掉 worktreeSettings 的 readonly 并在 applyProjectRuntimeConfig 中赋值 this.worktreeSettings = runtime.worktree ?? {}。
修复见证：迁移测试中以 worktree: { symlinkDirectories: ['a'] } 构造 Config，用覆盖为 ['b'] 的运行时迁移，断言 getWorktreeSymlinkDirectories() 为 ['b']；移除该赋值后变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| this.webSearchSettings = runtime.webSearch; | ||
| this.webSearchNoticeEmitted = false; | ||
| this.imageModel = runtime.imageModel || undefined; | ||
| this.allowedHttpHookUrls = [...runtime.allowedHttpHookUrls]; |
There was a problem hiding this comment.
[Critical] R2-23: this spread is unguarded: settings load performs no type validation and the CLI builder passes runtimeSettings.security?.allowedHttpHookUrls through raw, so a target project whose .qwen/settings.json has "security": { "allowedHttpHookUrls": 42 } makes [...42] throw TypeError here, mid-apply. The error-collection wrapper (added by the round-1 R1-6 fix) catches it, records it in projectRuntimeRefreshErrors, and relocation continues — silently skipping every later assignment in this method: permissionsDeny, contextFileNames, disableAllHooks, userHooks/projectHooks, and the this.hooks = undefined legacy reset. The session then runs in the target directory under the SOURCE project's permission and hook rules: the target's permissions.deny is never enforced, the previous project's legacy hooks are never cleared, and the context file names never update. The PR's own test anchored near here defends context.fileName from this exact failure class, leaving this sibling unguarded. A sibling sweep confirms allowedHttpHookUrls is the only field that passes through prepare() raw and spreads at apply time — the others (explicitIncludeDirectories, disabledSlashCommands, customSkillDirs) are constructed inside prepare() and fail closed before any commit.
Witness (probes):
merged.security.allowedHttpHookUrls = 42 isArray: false
prepared.config.allowedHttpHookUrls = 42
spread THREW -> TypeError: ... is not iterable
full relocate: move completes; getPermissionsDeny() still ['source-project-deny'];
getContextFileNames() still ['PROJECT-A.md']; legacy hooks intact
With Array.isArray(...) ? [...] : [] the same probe passes with the target deny applied, PROJECT-B.md active, and hooks cleared.
| this.allowedHttpHookUrls = [...runtime.allowedHttpHookUrls]; | |
| this.allowedHttpHookUrls = Array.isArray(runtime.allowedHttpHookUrls) | |
| ? [...runtime.allowedHttpHookUrls] | |
| : []; |
Fix witness: a sibling of the context.fileName test — relocate with allowedHttpHookUrls: 42 and permissions: { deny: ['target-project-deny'] }, assert the target deny applies with no refresh errors; red without the coercion.
中文说明
此展开未加防护：设置加载不做类型校验，CLI 构建端原样透传 runtimeSettings.security?.allowedHttpHookUrls，因此目标项目的 .qwen/settings.json 若含 "security": { "allowedHttpHookUrls": 42 }，[...42] 会在 apply 中途抛出 TypeError。（第 1 轮 R1-6 修复加入的）错误收集包装会捕获它、记入 projectRuntimeRefreshErrors 并继续迁移——从而静默跳过本方法之后的全部赋值：permissionsDeny、contextFileNames、disableAllHooks、userHooks/projectHooks 以及 this.hooks = undefined 的 legacy 重置。会话随后在目标目录中按“源”项目的权限与 hooks 规则运行：目标的 permissions.deny 永不生效，旧项目的 legacy hooks 永不清除，上下文文件名也不更新。本 PR 在附近为 context.fileName 加的测试恰好防御了同一失败类别，却遗漏了这个兄弟字段。兄弟字段排查确认 allowedHttpHookUrls 是唯一原样穿过 prepare() 并在 apply 时展开的字段——其余（explicitIncludeDirectories、disabledSlashCommands、customSkillDirs）都在 prepare() 内构造，会在 commit 前失败关闭。
证据（探针）：merged.security.allowedHttpHookUrls = 42 isArray: false，展开抛出 TypeError: ... is not iterable；完整迁移后 getPermissionsDeny() 仍为 ['source-project-deny']、getContextFileNames() 仍为 ['PROJECT-A.md']、legacy hooks 仍在。改为 Array.isArray(...) ? [...] : [] 后同一探针通过：目标 deny 生效、PROJECT-B.md 激活、hooks 被清除。
修复见证：仿照 context.fileName 测试新增用例——以 allowedHttpHookUrls: 42 和 permissions: { deny: ['target-project-deny'] } 迁移，断言目标 deny 生效且无刷新错误；去掉该防护后变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| loadedSettings: LoadedSettings | undefined, | ||
| cwd: string, | ||
| ): LoadedSettings { | ||
| return loadedSettings?.user.path ? loadedSettings : loadSettings(cwd); |
There was a problem hiding this comment.
[Critical] R1-4: (fix-induced) The round-1 crash — bare-mode "Always allow" persisting through createMinimalSettings()'s '' paths — is closed by this fallback, but the fallback opened a new defect at the same site: onPersistPermissionRule (config.ts:2684) is a construction-time closure capturing the startup cwd, never refreshed by /cd (Config.onPersistPermissionRuleCallback is readonly). In bare mode loadedSettings.user.path is always '', so every persistence takes this loadSettings(cwd) branch with the STALE startup cwd, and setValue(Workspace, ...) writes the rule into the project that was LEFT, not the relocation target. Concretely: qwen --bare in project A (bare mode keeps manual approval, so prompts fire), /cd to project B, approve a tool with "Always allow" — the grant lands in project A's .qwen/settings.json, never takes effect in project B (the user is re-prompted in every future session there), and project A's settings file is silently modified. Non-bare sessions are unaffected (replaceWith swaps scopes in place on the same instance).
Witness (probe):
projectA permissions: {"allow":["run_shell_command(ls *)"]}
projectB permissions: null
The flip arm resolving against the session's actual directory puts the rule in project B.
Suggested fix: make the persistence path relocation-aware — re-supply onPersistPermissionRule through ProjectRuntimeConfig (passing the live loadedSettings instance commit() already swaps and the target cwd), or have the closure resolve the directory lazily via a Config-bound getter instead of the captured startup cwd.
Fix witness: build a CLI config in bare mode, relocate via relocateWorkingDirectory, invoke config.getOnPersistPermissionRule() with scope 'project', and assert the rule lands in <targetDir>/.qwen/settings.json — red without the live-cwd rewiring.
中文说明
（修复引入）第 1 轮的崩溃——bare 模式下“始终允许”经由 createMinimalSettings() 的 '' 路径持久化——已被此回退分支修复，但该回退在同一位置引入了新缺陷：onPersistPermissionRule（config.ts:2684）是构造时捕获启动 cwd 的闭包，/cd 从不刷新它（Config.onPersistPermissionRuleCallback 为 readonly）。bare 模式下 loadedSettings.user.path 恒为 ''，因此每次持久化都会走这个 loadSettings(cwd) 分支，而 cwd 是过期的启动目录，setValue(Workspace, ...) 会把规则写进“已离开”的项目，而不是迁移目标。具体场景：在项目 A 中 qwen --bare（bare 模式保留人工审批，确认弹窗会出现），/cd 到项目 B，用“始终允许”批准某工具——授权落进项目 A 的 .qwen/settings.json，在项目 B 永不生效（此后每个会话都会重复询问），且项目 A 的设置文件被静默修改。非 bare 会话不受影响（replaceWith 在同一实例上原位切换作用域）。
证据（探针）：projectA permissions: {"allow":["run_shell_command(ls *)"]} / projectB permissions: null；将会话实际目录作为解析目标的对照分支则把规则写入项目 B。
建议修复：让持久化路径感知迁移——通过 ProjectRuntimeConfig 重新提供 onPersistPermissionRule（传入 commit() 已在原位切换的活 loadedSettings 实例与目标 cwd），或让闭包通过绑定到 Config 的 getter 惰性解析目录，而不是捕获的启动 cwd。
修复见证：以 bare 模式构建 CLI 配置，执行 relocateWorkingDirectory 迁移，调用 config.getOnPersistPermissionRule()（scope 为 'project'），断言规则落入 <targetDir>/.qwen/settings.json；去掉活 cwd 改造后变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| agents: | ||
| bareMode || safeMode | ||
| ? undefined | ||
| : projectAgentsSettings(nextSettings.merged.agents), |
There was a problem hiding this comment.
[Critical] R2-25: the reloader gates agents on bareMode || safeMode, but startup's configParams passes agents: projectAgentsSettings(settings.agents) with NO safe-mode gate — the one field where the two projections diverge, despite projectAgentsSettings' docblock claiming the two "cannot drift". In safe mode the gate is operative (user-scope agents survive skipWorkspaceSettings); in bare mode it is redundant (createMinimalSettings() already yields undefined). A safe-mode session whose user settings define agents: { allowedGrades: ['fast'], modelGrades: {...}, arena: { worktreeBaseDir: ... } } starts with those applied; after /cd ../other (allowed — cdCommand has no safe-mode restriction), prepare() emits agents: undefined, applyProjectRuntimeConfig sets agentsSettings = {}, and SubagentManager.getAvailableModelGrades() (reads config.getAgentsSettings() at spawn) returns an empty map — the user's grade restriction is silently LOOSENED to all grades mid-session, and arena worktrees relocate to the default base dir.
Witness (probe, real loadCliConfig + real reloader):
startup safe-mode getAgentsSettings() = {modelGrades..., allowedGrades:['fast'], arena:{worktreeBaseDir...}} (isSafeMode()=true)
safe-mode prepared.config.agents = undefined <- after /cd
normal-mode prepared.config.agents = {full projection} <- control arm
| agents: | |
| bareMode || safeMode | |
| ? undefined | |
| : projectAgentsSettings(nextSettings.merged.agents), | |
| agents: | |
| bareMode | |
| ? undefined | |
| : projectAgentsSettings(nextSettings.merged.agents), |
Fix witness: a reloader test with user settings agents: { allowedGrades: ['fast'] } and makeReloader(loaded, { safeMode: true }) asserting prepared.config.agents?.allowedGrades equals ['fast'] — red if the safeMode gate is re-added.
中文说明
reloader 用 bareMode || safeMode 门控 agents，但启动时的 configParams 传递 agents: projectAgentsSettings(settings.agents) 时并没有 safe 模式门控——尽管 projectAgentsSettings 的文档注释声称两者“不会漂移”，这正是两个投影唯一分叉的字段。safe 模式下该门控实际生效（用户级 agents 在 skipWorkspaceSettings 下仍然存在）；bare 模式下则是冗余的（createMinimalSettings() 本来就得到 undefined）。用户设置定义了 agents: { allowedGrades: ['fast'], ... } 的 safe 模式会话启动时这些设置会生效；执行 /cd ../other（允许——cdCommand 不限制 safe 模式）后，prepare() 输出 agents: undefined，applyProjectRuntimeConfig 置 agentsSettings = {}，SubagentManager.getAvailableModelGrades()（spawn 时读取 config.getAgentsSettings()）返回空映射——用户的等级限制在会话中途被静默“放宽”为全部等级，arena worktree 也会回落到默认基础目录。
证据（探针，真实 loadCliConfig + 真实 reloader）：启动 safe 模式下 getAgentsSettings() 含完整投影（allowedGrades:['fast'] 等，isSafeMode()=true）；/cd 后 safe 模式 prepared.config.agents = undefined；非 safe 对照分支得到完整投影。
修复见证：reloader 测试中用户设置含 agents: { allowedGrades: ['fast'] }，makeReloader(loaded, { safeMode: true })，断言 prepared.config.agents?.allowedGrades 等于 ['fast']；重新加上 safeMode 门控后变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| return this.targetDir; | ||
| } | ||
|
|
||
| private applyProjectRuntimeConfig(runtime: ProjectRuntimeConfig): void { |
There was a problem hiding this comment.
[Suggestion] R1-21: round 1 flagged that applyProjectRuntimeConfig re-applies ~50 settings-derived knobs but silently skips other settings-derived requiresRestart: false fields (maxToolCallsPerTurn, gitCoAuthor, …), so after /cd the session keeps the OLD project's values. The answer was "Not in this commit — closing it means auditing every requiresRestart: false settings-derived field"; the thread was left open and nothing changed this round, so the entry still stands. The class remains open and grew two probe-confirmed Critical members this round: R2-1 (agents concurrency caps stored but never enforced by BackgroundTaskRegistry) and R2-15 (worktree.symlinkDirectories not even stored), plus chatCompression, autoCompactThreshold, skipLoopDetection, preventSystemSleep in the same omission class. Any /cd into a project that tunes one of the skipped fields keeps the source project's value mid-session, contradicting the reload's stated goal of aligning the session with its current directory.
Suggested fix: audit the requiresRestart: false, settings-derived fields and either add them to ProjectRuntimeConfig/applyProjectRuntimeConfig or document each exclusion with a restart-flag rationale.
Fix witness: per-field relocation assertions mirroring the existing "apply the prepared project runtime" suite — each red without the corresponding assignment.
中文说明
第 1 轮曾指出 applyProjectRuntimeConfig 重新应用约 50 个派生自设置的旋钮，却静默跳过其他派生自设置且 requiresRestart: false 的字段（maxToolCallsPerTurn、gitCoAuthor 等），导致 /cd 后会话保留“旧”项目的取值。当时的答复是“本次提交不处理——收尾意味着审计所有 requiresRestart: false 的设置派生字段”；线程保持开放，本轮也没有任何变化，因此该条目仍然存在。这个类别依然开放，且本轮新增了两个经探针确认的 Critical 成员：R2-1（agents 并发上限被存储但 BackgroundTaskRegistry 从不执行）与 R2-15（worktree.symlinkDirectories 甚至未被存储），另有 chatCompression、autoCompactThreshold、skipLoopDetection、preventSystemSleep 同属遗漏类别。任何 /cd 进入调整了被跳过字段的项目，都会在会话中途沿用源项目的取值，与重载“让会话与当前目录对齐”的声明目标相悖。
建议修复：审计 requiresRestart: false 的设置派生字段，要么加入 ProjectRuntimeConfig/applyProjectRuntimeConfig，要么为每个排除项给出重启标志层面的理由。
修复见证：仿照现有“应用准备好的项目运行时”套件逐字段添加迁移断言——缺少对应赋值时各自变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| * caller acting for a session whose `/cd` scoped the names should pass | ||
| * that session's primary name. | ||
| */ | ||
| contextFileName?: string; |
There was a problem hiding this comment.
[Suggestion] R2-9: this new contextFileName option is a dead switch — declared, documented, and read by resolveContextFilePath, but no production caller sets it: all three writeWorkspaceContextFile call sites (serve/workspace-memory.ts:303 and :444, serve/acp-http/dispatch.ts:3849) pass object literals without contextFileName; the only setter in the tree is the new unit test. Meanwhile applyProjectRuntimeConfig updates the session's contextFileNames but never calls setMemoryFilename (deliberately, pinned by test), so after /cd into a project with context.fileName: PROJECT-B.md, discovery, write-detection, the /memory dialog, and collectWorkspaceMemoryStatus all think in session-scoped names, while every daemon/ACP memory-write route still resolves the stale process-global getCurrentMemoryFilename() — a client that saves memory through those routes after /cd writes to the wrong file: the exact GET-vs-POST filename divergence the comment block above this option warns about.
Suggested fix: pass the session/workspace runtime's primary context filename at the three write call sites (the workspace runtime already carries contextFilename, workspace-service/types.ts), or delete the option until a consumer exists.
Fix witness: a route-level test asserting a memory-write request with a session-scoped context filename resolves and writes <projectRoot>/<session filename> — none exists today; the only pin is on the unwired helper.
中文说明
这个新的 contextFileName 选项是死开关——已声明、有文档、被 resolveContextFilePath 读取，但没有任何生产调用方设置它：writeWorkspaceContextFile 的全部三个调用点（serve/workspace-memory.ts:303 与 :444、serve/acp-http/dispatch.ts:3849）传入的对象字面量都不含 contextFileName；整棵树中唯一的设置者是新的单元测试。与此同时 applyProjectRuntimeConfig 会更新会话的 contextFileNames 但从不调用 setMemoryFilename（刻意为之，有测试钉住），因此 /cd 进入 context.fileName: PROJECT-B.md 的项目后，发现、写入检测、/memory 对话框、collectWorkspaceMemoryStatus 都按会话作用域名称思考，而每个守护进程/ACP 记忆写入路由仍解析过期的进程级全局 getCurrentMemoryFilename()——客户端在 /cd 后通过这些路由保存记忆会写错文件：正是本选项上方注释块所警告的 GET 与 POST 文件名分叉。
建议修复：在三个写入调用点传入会话/工作区运行时的主上下文文件名（工作区运行时本就携带 contextFilename，见 workspace-service/types.ts），或在出现消费者之前删除该选项。
修复见证：路由级测试断言携带会话作用域上下文文件名的记忆写入请求解析并写入 <projectRoot>/<会话文件名>——目前不存在这样的测试，唯一的钉点在这个未接线的助手函数上。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (contextFileName) { | ||
| return scope === 'workspace' | ||
| ? path.join(projectRoot, contextFileName) |
There was a problem hiding this comment.
[Suggestion] R2-10: this new contextFileName join has no containment validation, and the option's doc comment invites callers to pass session names that originate in unvalidated project settings: resolveContextFileNames (core config.ts) only trims and type-filters, so '../../evil' and absolute paths from a target project's context.fileName survive. Once any caller wires this option (as the doc comment directs), resolveContextFilePath returns path.join(projectRoot, '../../evil'), and runWrite then does fs.mkdir(dirname, recursive) + fs.writeFile outside the workspace — a write-what-where primitive fed by a semi-trusted repo's settings. The daemon facade already validates the same input class for init (workspace-service/index.ts rejects with "resolves outside the bound workspace", pinned by facade.test.ts), but this helper bypasses that check. Not reachable by production callers today (see the companion dead-switch finding) — the risk materializes exactly when the documented wiring lands.
Witness (probe): writeWorkspaceContextFile({scope:'workspace', mode:'append', projectRoot: <tmp>/workspace, contextFileName: '../escape.md'}) →
ESCAPE: filePath=/tmp/qwen-r210-.../escape.md projectRoot=/tmp/qwen-r210-.../workspace
The file landed outside the workspace; with a containment guard patched in, the same probe is refused and all 18 existing tests stay green.
Suggested fix: before returning, resolve the candidate path and assert it stays inside the base directory (mirroring the facade's "resolves outside" check) for both scopes; alternatively validate in resolveContextFileNames so every consumer is covered.
Fix witness: a writeContextFile.test.ts case asserting contextFileName: '../escape.md' rejects (writing nothing outside projectRoot) — red if the containment check is removed.
中文说明
这个新的 contextFileName 拼接没有包含性校验，而该选项的文档注释正邀请调用方传入源自未校验项目设置的会话名称：resolveContextFileNames（core config.ts）只做 trim 和类型过滤，因此来自目标项目 context.fileName 的 '../../evil' 和绝对路径可以存活。一旦有调用方接上该选项（文档注释正是这么指引的），resolveContextFilePath 会返回 path.join(projectRoot, '../../evil')，runWrite 随即执行 fs.mkdir(dirname, recursive) + fs.writeFile 写到工作区之外——一个由半信任仓库设置喂入的任意写原语。守护进程 facade 在 init 路径上已对同类输入做校验（workspace-service/index.ts 以 "resolves outside the bound workspace" 拒绝，有 facade.test.ts 钉住），但本助手函数绕过了该校验。当前生产调用方不可达（见配套的死开关发现）——风险恰好在文档所述接线落地时成形。
证据（探针）：writeWorkspaceContextFile({scope:'workspace', mode:'append', projectRoot: <tmp>/workspace, contextFileName: '../escape.md'}) → ESCAPE: filePath=/tmp/qwen-r210-.../escape.md projectRoot=/tmp/qwen-r210-.../workspace，文件落在工作区之外；打上包含性守卫后同一探针被拒绝，且现有 18 个测试保持为绿。
建议修复：返回前解析候选路径并断言其保持在基础目录内（两种 scope 都套用，仿照 facade 的 "resolves outside" 检查）；或在 resolveContextFileNames 中校验，使所有消费者都被覆盖。
修复见证：writeContextFile.test.ts 用例断言 contextFileName: '../escape.md' 被拒绝（projectRoot 外无写入）；移除包含性检查后变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| async refreshForProjectChange(): Promise<void> { | ||
| try { | ||
| await this.refreshCache(); |
There was a problem hiding this comment.
[Suggestion] R1-25: round 1 flagged that SubagentManager.refreshForProjectChange (here) and SkillManager.refreshForProjectChange (skill-manager.ts:516) have no direct tests — they appear in test code only as bare vi.fn() mocks in config.test.ts. The answer was "Not in this commit — these helpers' own unit tests are coverage work on code this PR did not change; leaving the thread open"; nothing changed this round, so the entry still stands. Verified by mutation this round: (a) deleting the updateWatchersFromCache() branch in the skill manager survives the whole suite — after /cd the skill watcher would keep watching the previous project's skill directories, so skill additions/changes in the new project never surface until restart; (b) dropping this catch body survives too — on a /cd whose target subagent directory read fails, the manager would keep serving the previous project's subagents while relocateWorkingDirectory records the error and moves on. Both new probes flip (pristine code green; under mutation the skill probe finds no watcher for the new project's skills dir and the subagent probe finds change listeners unnotified).
Fix witness: add direct unit tests — subagent-manager: refreshCache rejects, assert the 'project' cache entry is evicted, listeners are notified, and the error propagates; skill-manager: start watching, call refreshForProjectChange, assert watchers are rebuilt from the refreshed cache (and a not-started watcher stays not-started). Each must go red when the corresponding branch is removed.
中文说明
第 1 轮曾指出 SubagentManager.refreshForProjectChange（此处）与 SkillManager.refreshForProjectChange（skill-manager.ts:516）没有直接测试——它们在测试代码中仅以 config.test.ts 里的裸 vi.fn() mock 出现。当时的答复是“本次提交不做——这两个助手函数自身的单元测试属于对本 PR 未改动代码的覆盖工作；线程保持开放”；本轮没有任何变化，因此该条目仍然存在。本轮经变异验证：(a) 删除 skill manager 中的 updateWatchersFromCache() 分支，整个套件仍为绿——/cd 后 skill 监视器会继续监视上一个项目的 skill 目录，新项目中的 skill 新增/变更在重启前永不浮现；(b) 删除此处 catch 体，同样存活——当 /cd 的目标子智能体目录读取失败时，管理器会继续提供上一个项目的子智能体，而 relocateWorkingDirectory 仅记录错误并继续。两个新探针均可翻转（原始代码为绿；变异后 skill 探针发现新项目 skills 目录没有监视器、子智能体探针发现变更监听器未被通知）。
修复见证：补充直接单元测试——subagent-manager：refreshCache 拒绝时断言 'project' 缓存条目被清除、监听器被通知、错误向外传播；skill-manager：先启动监视，调用 refreshForProjectChange，断言监视器按刷新后的缓存重建（未启动的监视器保持未启动）。移除对应分支后各自必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| async refreshForProjectChange(): Promise<void> { | ||
| await this.refreshCache(); |
There was a problem hiding this comment.
[Suggestion] R1-25 (second location): round 1 flagged that this refreshForProjectChange and its twin in SubagentManager (subagent-manager.ts:665) have no direct tests; the author deferred ("Not in this commit … leaving the thread open") and nothing changed this round. This location is the watcher half: deleting the updateWatchersFromCache() branch survives the whole suite (verified by mutation), leaving the skill watcher on the previous project's directories after /cd — skill additions/changes in the new project never surface until restart. See the companion comment on subagent-manager.ts for the full entry and fix witness.
中文说明
R1-25（第二处）：第 1 轮曾指出本 refreshForProjectChange 与其在 SubagentManager 中的孪生方法（subagent-manager.ts:665）没有直接测试；作者推迟处理（“本次提交不做……线程保持开放”），本轮没有任何变化。本处是监视器侧：删除 updateWatchersFromCache() 分支后整个套件仍为绿（已经变异验证），/cd 后 skill 监视器仍停留在上一个项目的目录上——新项目中的 skill 新增/变更在重启前永不浮现。完整条目与修复见证见 subagent-manager.ts 上的配套评论。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
已修复本轮 5 条 Critical：目录切换后会同步后台智能体并发限制、worktree 软链设置和权限持久化目标，safe mode 保留用户级 agents 设置；非法 allowedHttpHookUrls 会安全降级且不再中断后续运行时配置应用。验证通过：全仓 build、typecheck；core 747 项相关测试、CLI 366 项相关测试；目标 ESLint、Prettier 与 git diff --check。修复提交 afbf358。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
已修复 upstream 合并后的编译回归：项目运行时刷新改用当前 LlmClient 实例，消除 core 中两处 geminiClient 不存在错误。 验证：Node 22 下仓库 build、typecheck 通过；core config.test.ts 599/599；目标 ESLint、Prettier 与独立 relocation 路径验证通过。提交 6d1bdd6。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-18 bare-mode persistence fallback re-enters loadSettings(cwd) with env loading enabled — already reported (comment 3875850392)
- cronEnabledOverride never exercised through the real reloader (test gap) — already recorded in the round-2 deferral list (review 5045650144)
- complete() success-path watcher resume never executed by any test — already recorded in the round-2 deferral list (review 5045650144)
- tool-swap catch / reload failClosed / client-refresh call sites unpinned — already recorded in the round-2 deferral list (review 5045650144)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Convergence: round 3 posted 26 inline comment(s), 15 of them reported for the first time; the previous round posted 21 (18 new). Findings keep coming back to the same files: packages/cli/src/config/config.ts (findings in rounds 1, 2; 4 more now); packages/core/src/config/config.ts (findings in rounds 1, 2; 1 more now); packages/cli/src/config/config.projectRuntimeReloader.test.ts (findings in round 2; 1 more now), and 1 more file(s). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛情况：第 3 轮发布了 26 条行内评论，其中 15 条是首次提出；上一轮发布了 21 条（其中 18 条首次提出）。发现反复回到同一批文件：packages/cli/src/config/config.ts（第 1、2 轮已出过发现，本轮又有 4 条）；packages/core/src/config/config.ts（第 1、2 轮已出过发现，本轮又有 1 条）；packages/cli/src/config/config.projectRuntimeReloader.test.ts（第 2 轮已出过发现，本轮又有 1 条），另有 1 个文件。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| loadedSettings: LoadedSettings | undefined, | ||
| cwd: string, | ||
| ): LoadedSettings { | ||
| return loadedSettings?.user.path ? loadedSettings : loadSettings(cwd); |
There was a problem hiding this comment.
[Critical] R1-4: (fix-induced) The round-2 fix for the stale-cwd persistence closure (afbf358) re-created the callback per move, but switched it to read currentRules from the captured in-memory nextSettings instead of fresh-loading from disk — and that opens two holes at this site. In --safe-mode, prepare() loads the target with skipWorkspaceSettings, so the workspace scope is empty in memory while user.path stays real and resolvePersistenceSettings returns that instance: after /cd, approving a tool with "Always allow in this project" writes setValue(Workspace, 'permissions.allow', [newRule]) over the empty in-memory array, so the target project's existing allow rules are silently wiped from its .qwen/settings.json — deterministic data loss for every future session in that project. In normal mode the same in-memory read leaves a ~300 ms race: two sessions in one project, session A persists a rule, session B persists before its settings watcher refreshes, and B's stale read writes [...[], ruleB], silently deleting A's rule. The round-1 crash this site originally fixed stays fixed; both holes above are introduced by the callback extraction.
Witness (probe, real reloader + settings stack at head 6d1bdd6):
safe-mode arm: disk after onPersistPermissionRule('project','allow',…) = ["run_shell_command(ls *)"]
(fixture had ["rule-A","rule-B"]); workspace-active-gated flip keeps all three
race arm: window open -> disk = ["run_shell_command(rule-b)"] (rule-a deleted before any watcher event)
watcher catch-up measured at 305 ms; fresh-load-per-persist mutant keeps both rules
Re-read the target scope from disk before the read-modify-write — at the top of createPermissionRulePersistenceCallback:
currentSettings.reloadScopeFromDisk(settingScope);
const currentRules: string[] =
currentSettings.forScope(settingScope).settings.permissions?.[ruleType] ??
[];(or gate resolvePersistenceSettings on workspaceSettingsActive so a skipWorkspaceSettings load falls back to a fresh loadSettings(targetDir) — that also fixes the safe-mode arm without touching the callback).
Fix witness: a reloader test — write project B with existing permissions.allow, prepare/commit with { safeMode: true }, invoke onPersistPermissionRule('project','allow','R'), assert the file on disk contains the existing rules AND R; removing the disk reload must turn it red.
中文说明
（修复引入）第 2 轮对"持久化闭包捕获过期 cwd"的修复（afbf358）把回调改为每次迁移重建，但 currentRules 从捕获的内存态 nextSettings 读取，而不再从磁盘新鲜加载——在同一位置引入了两个漏洞。--safe-mode 下 prepare() 以 skipWorkspaceSettings 加载目标，工作区作用域在内存中为空而 user.path 仍为真实路径，resolvePersistenceSettings 返回该实例：/cd 后用"始终允许（本项目）"批准工具会以 setValue(Workspace, 'permissions.allow', [newRule]) 覆盖空数组，把目标项目 .qwen/settings.json 里已有的 allow 规则静默清空——对该项目后续所有会话都是确定性的数据丢失。普通模式下同一内存读取留下约 300ms 竞态：同项目两个会话，A 持久化一条规则，B 在其设置监听刷新前持久化，B 的过期读到 [...[], ruleB]，静默删除 A 的规则。该位置原先修复的第 1 轮崩溃仍然保持修复；以上两个漏洞均由回调抽取引入。
证据（探针，真实 reloader + 设置栈，head 6d1bdd6）：safe 模式分支——持久化后磁盘 = ["run_shell_command(ls *)"]（fixture 原有 ["rule-A","rule-B"]）；按工作区激活态门控的对照保留全部三条。竞态分支——窗口打开时磁盘 = ["run_shell_command(rule-b)"]（rule-a 在任何监听事件前被删除）；监听追平实测 305ms；"每次持久化新鲜加载"的突变体在两个分支都保留两条规则。
建议：在 createPermissionRulePersistenceCallback 的读-改-写之前先从磁盘重读目标作用域（currentSettings.reloadScopeFromDisk(settingScope)）；或让 resolvePersistenceSettings 以 workspaceSettingsActive 门控，使 skipWorkspaceSettings 的加载回退到新鲜的 loadSettings(targetDir)。修复见证：reloader 测试——目标项目预置 permissions.allow，以 { safeMode: true } prepare/commit 后调用 onPersistPermissionRule，断言磁盘同时包含原有规则与新规则；去掉磁盘重读后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // and expand their commands against it. Same step serve's | ||
| // `workspaceReload` performs; bare mode never loads env at all. | ||
| if (!bareMode) { | ||
| reloadEnvironment(nextSettings.merged, targetDir, effectiveTrust); |
There was a problem hiding this comment.
[Critical] R3-1: commit()'s reloadEnvironment(nextSettings.merged, targetDir, effectiveTrust) rewrites process-global process.env from a per-session /cd — deleting previously-tracked keys absent from the target's env files and force-writing the target's keys. The ACP child under qwen serve is long-lived and hosts multiple sessions (the this.sessions map; the PR's own contextFileNamesForCwd only makes sense when sessions in one child have different cwds), and sessionCd has no sibling-session gate. A sibling session still bound to the old workspace then spawns an MCP stdio server or a shell child (mcp-client inherits process.env at spawn) and inherits the relocated session's environment while missing its own keys — cross-project secret crossover that did not exist pre-PR (only the child-wide workspaceReload could touch env before, so sibling divergence was impossible).
Witness (probe driving the real reloader + environment modules at head 6d1bdd6):
PROBE afterCommit: {"KEY_X":"secret-from-x"} (session B's /cd deleted boot-workspace KEY_W process-wide)
PROBE sibling child inherited: {"KEY_X":"KEY_X=secret-from-x"} (sibling bound to W inherits X's secret, misses KEY_W)
Do not mutate process.env from a per-session /cd when the process can host sibling sessions — keep the target env in a per-session snapshot used when expanding that session's MCP/shell spawn env, or gate relocation on single-session processes; if the shared-process env churn is an accepted design decision, document it at the reloader.
Fix witness: a two-session ACP-child test — session A bound to W with KEY_W in env, session B /cds to X, then A spawns a child; assert the child's env still carries KEY_W and no X-only keys; removing the per-session isolation must turn it red.
中文说明
commit() 的 reloadEnvironment(nextSettings.merged, targetDir, effectiveTrust) 以"每会话 /cd"为粒度改写进程级全局 process.env——删除目标 env 文件中不存在的既有跟踪键、强制写入目标的键。qwen serve 的 ACP 子进程长期存活并承载多个会话（this.sessions 映射；本 PR 自己的 contextFileNamesForCwd 只有在同一子进程内会话 cwd 各不相同时才有意义），而 sessionCd 没有兄弟会话门控。仍绑定旧工作区的兄弟会话随后启动 MCP stdio 服务器或 shell 子进程（mcp-client 在 spawn 时继承 process.env），会继承被迁移会话的环境、丢失自己的键——跨项目密钥串扰。PR 之前不存在该问题：只有整个子进程级的 workspaceReload 能触碰 env，兄弟分歧不可能发生。
证据（探针驱动真实 reloader + environment 模块，head 6d1bdd6）：迁移提交后 {"KEY_X":"secret-from-x"}（会话 B 的 /cd 在进程范围删除了启动工作区的 KEY_W）；兄弟会话启动的子进程继承 {"KEY_X":"KEY_X=secret-from-x"}（仍绑定 W 的兄弟继承了 X 的密钥、丢失 KEY_W）。
建议：当进程可能承载兄弟会话时，不要以每会话 /cd 改写 process.env——把目标 env 存为每会话快照、在展开该会话的 MCP/shell spawn env 时使用；或将迁移限制在单会话进程。若接受共享进程的 env 变动为设计决策，请在 reloader 处文档化。修复见证：双会话 ACP 子进程测试——A 绑定 W（env 含 KEY_W），B /cd 到 X 后 A 启动子进程，断言子进程 env 仍含 KEY_W 且无 X 专属键；去掉每会话隔离后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| expect(prepare).toHaveBeenCalledWith( | ||
| newDir, | ||
| true, | ||
| ApprovalMode.AUTO, | ||
| expect.any(String), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-2: the new relocation suite leaves three apply/prepare contracts unpinned — this is one of its three locations. (1) prepare's 4th argument (the previous directory) is asserted only as expect.any(String) while cwdSpy is installed to return newDir before the move, so at test runtime oldDir === newDir: a future edit passing expected instead of oldDir stays green, and rollback() would then call reloadEnvironment(previousSettings.merged, previousDir=wrong) — reloading the TARGET's .env while settings restore to the old project. (2) The fixture sets loadMemoryFromIncludeDirectories: true (constructor default is false) but no test asserts getLoadMemoryFromIncludeDirectories(): deleting the assignment in applyProjectRuntimeConfig keeps all 599 core config tests green, and after /cd into a project with memory.loadFromIncludeDirectories: true the include-dir memory silently does not load. (3) Every fixture passes trustedFolder: true — exactly the constructor default — and nothing asserts isTrustedFolder(): deleting the trustedFolder re-application keeps the session trusted in a workspace the runtime declared untrusted.
Mutations verified green at head 6d1bdd6: oldDir → expected in the prepare call ('Tests 3 passed | 596 skipped'); deleted loadMemoryFromIncludeDirectories assignment ('Tests 599 passed').
Tighten the happy-path test: make cwdSpy return a distinct old directory first (mockReturnValueOnce(oldDir).mockReturnValue(newDir)) and assert expect(prepare).toHaveBeenCalledWith(newDir, true, ApprovalMode.AUTO, oldDir); add expect(config.getLoadMemoryFromIncludeDirectories()).toBe(true); add a trustedFolder: false fixture with expect(config.isTrustedFolder()).toBe(false).
Fix witness: the tightened test — each of the three mutations goes red against it.
中文说明
新增的迁移测试套件留下三个 apply/prepare 契约未固定——本条是其中一处。(1) prepare 的第 4 个参数（上一目录）只断言为 expect.any(String)，且 cwdSpy 在迁移前就被设为返回 newDir，测试运行时 oldDir === newDir：未来把 expected 当作 oldDir 传入也不会变红，rollback() 会以错误的 previousDir 调 reloadEnvironment——设置已回滚到旧项目、env 却重载了目标项目的 .env。(2) fixture 设了 loadMemoryFromIncludeDirectories: true（构造默认 false）但没有测试断言 getLoadMemoryFromIncludeDirectories()：删除 applyProjectRuntimeConfig 中的赋值，599 个测试依旧全绿；/cd 到开启该选项的项目后，include 目录记忆静默不加载。(3) 所有 fixture 传 trustedFolder: true——恰为构造默认值——也没有断言 isTrustedFolder()：删除 trustedFolder 再应用后，会话在运行时声明为不受信任的工作区里仍被视为可信。
在 head 6d1bdd6 验证突变均为绿：oldDir → expected（'Tests 3 passed | 596 skipped'）；删除 loadMemoryFromIncludeDirectories 赋值（'Tests 599 passed'）。
建议：收紧 happy-path 测试——cwdSpy 首次返回不同的旧目录并断言 prepare 第 4 参的精确值；补 getLoadMemoryFromIncludeDirectories()).toBe(true)；以 trustedFolder: false fixture 断言 isTrustedFolder()).toBe(false)。修复见证：收紧后的测试使上述三个突变全部变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| config: { | ||
| trustedFolder: true, | ||
| includeDirectories: ['/path/to/project-b-include'], | ||
| loadMemoryFromIncludeDirectories: true, |
There was a problem hiding this comment.
[Suggestion] R3-2: the new relocation suite leaves three apply/prepare contracts unpinned — this is one of its three locations. (1) prepare's 4th argument (the previous directory) is asserted only as expect.any(String) while cwdSpy is installed to return newDir before the move, so at test runtime oldDir === newDir: a future edit passing expected instead of oldDir stays green, and rollback() would then call reloadEnvironment(previousSettings.merged, previousDir=wrong) — reloading the TARGET's .env while settings restore to the old project. (2) The fixture sets loadMemoryFromIncludeDirectories: true (constructor default is false) but no test asserts getLoadMemoryFromIncludeDirectories(): deleting the assignment in applyProjectRuntimeConfig keeps all 599 core config tests green, and after /cd into a project with memory.loadFromIncludeDirectories: true the include-dir memory silently does not load. (3) Every fixture passes trustedFolder: true — exactly the constructor default — and nothing asserts isTrustedFolder(): deleting the trustedFolder re-application keeps the session trusted in a workspace the runtime declared untrusted.
Mutations verified green at head 6d1bdd6: oldDir → expected in the prepare call ('Tests 3 passed | 596 skipped'); deleted loadMemoryFromIncludeDirectories assignment ('Tests 599 passed').
Tighten the happy-path test: make cwdSpy return a distinct old directory first (mockReturnValueOnce(oldDir).mockReturnValue(newDir)) and assert expect(prepare).toHaveBeenCalledWith(newDir, true, ApprovalMode.AUTO, oldDir); add expect(config.getLoadMemoryFromIncludeDirectories()).toBe(true); add a trustedFolder: false fixture with expect(config.isTrustedFolder()).toBe(false).
Fix witness: the tightened test — each of the three mutations goes red against it.
中文说明
新增的迁移测试套件留下三个 apply/prepare 契约未固定——本条是其中一处。(1) prepare 的第 4 个参数（上一目录）只断言为 expect.any(String)，且 cwdSpy 在迁移前就被设为返回 newDir，测试运行时 oldDir === newDir：未来把 expected 当作 oldDir 传入也不会变红，rollback() 会以错误的 previousDir 调 reloadEnvironment——设置已回滚到旧项目、env 却重载了目标项目的 .env。(2) fixture 设了 loadMemoryFromIncludeDirectories: true（构造默认 false）但没有测试断言 getLoadMemoryFromIncludeDirectories()：删除 applyProjectRuntimeConfig 中的赋值，599 个测试依旧全绿；/cd 到开启该选项的项目后，include 目录记忆静默不加载。(3) 所有 fixture 传 trustedFolder: true——恰为构造默认值——也没有断言 isTrustedFolder()：删除 trustedFolder 再应用后，会话在运行时声明为不受信任的工作区里仍被视为可信。
在 head 6d1bdd6 验证突变均为绿：oldDir → expected（'Tests 3 passed | 596 skipped'）；删除 loadMemoryFromIncludeDirectories 赋值（'Tests 599 passed'）。
建议：收紧 happy-path 测试——cwdSpy 首次返回不同的旧目录并断言 prepare 第 4 参的精确值；补 getLoadMemoryFromIncludeDirectories()).toBe(true)；以 trustedFolder: false fixture 断言 isTrustedFolder()).toBe(false)。修复见证：收紧后的测试使上述三个突变全部变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const complete = vi.fn().mockResolvedValue(undefined); | ||
| const prepare = vi.fn().mockResolvedValue({ | ||
| config: { | ||
| trustedFolder: true, |
There was a problem hiding this comment.
[Suggestion] R3-2: the new relocation suite leaves three apply/prepare contracts unpinned — this is one of its three locations. (1) prepare's 4th argument (the previous directory) is asserted only as expect.any(String) while cwdSpy is installed to return newDir before the move, so at test runtime oldDir === newDir: a future edit passing expected instead of oldDir stays green, and rollback() would then call reloadEnvironment(previousSettings.merged, previousDir=wrong) — reloading the TARGET's .env while settings restore to the old project. (2) The fixture sets loadMemoryFromIncludeDirectories: true (constructor default is false) but no test asserts getLoadMemoryFromIncludeDirectories(): deleting the assignment in applyProjectRuntimeConfig keeps all 599 core config tests green, and after /cd into a project with memory.loadFromIncludeDirectories: true the include-dir memory silently does not load. (3) Every fixture passes trustedFolder: true — exactly the constructor default — and nothing asserts isTrustedFolder(): deleting the trustedFolder re-application keeps the session trusted in a workspace the runtime declared untrusted.
Mutations verified green at head 6d1bdd6: oldDir → expected in the prepare call ('Tests 3 passed | 596 skipped'); deleted loadMemoryFromIncludeDirectories assignment ('Tests 599 passed').
Tighten the happy-path test: make cwdSpy return a distinct old directory first (mockReturnValueOnce(oldDir).mockReturnValue(newDir)) and assert expect(prepare).toHaveBeenCalledWith(newDir, true, ApprovalMode.AUTO, oldDir); add expect(config.getLoadMemoryFromIncludeDirectories()).toBe(true); add a trustedFolder: false fixture with expect(config.isTrustedFolder()).toBe(false).
Fix witness: the tightened test — each of the three mutations goes red against it.
中文说明
新增的迁移测试套件留下三个 apply/prepare 契约未固定——本条是其中一处。(1) prepare 的第 4 个参数（上一目录）只断言为 expect.any(String)，且 cwdSpy 在迁移前就被设为返回 newDir，测试运行时 oldDir === newDir：未来把 expected 当作 oldDir 传入也不会变红，rollback() 会以错误的 previousDir 调 reloadEnvironment——设置已回滚到旧项目、env 却重载了目标项目的 .env。(2) fixture 设了 loadMemoryFromIncludeDirectories: true（构造默认 false）但没有测试断言 getLoadMemoryFromIncludeDirectories()：删除 applyProjectRuntimeConfig 中的赋值，599 个测试依旧全绿；/cd 到开启该选项的项目后，include 目录记忆静默不加载。(3) 所有 fixture 传 trustedFolder: true——恰为构造默认值——也没有断言 isTrustedFolder()：删除 trustedFolder 再应用后，会话在运行时声明为不受信任的工作区里仍被视为可信。
在 head 6d1bdd6 验证突变均为绿：oldDir → expected（'Tests 3 passed | 596 skipped'）；删除 loadMemoryFromIncludeDirectories 赋值（'Tests 599 passed'）。
建议：收紧 happy-path 测试——cwdSpy 首次返回不同的旧目录并断言 prepare 第 4 参的精确值；补 getLoadMemoryFromIncludeDirectories()).toBe(true)；以 trustedFolder: false fixture 断言 isTrustedFolder()).toBe(false)。修复见证：收紧后的测试使上述三个突变全部变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // known. Same split as `collectWorkspaceMemoryStatus`. | ||
| if (contextFileName) { | ||
| return scope === 'workspace' | ||
| ? path.join(projectRoot, contextFileName) |
There was a problem hiding this comment.
[Suggestion] R2-10: still stands — this contextFileName join has no containment validation, and the option's doc comment invites callers to pass session names that originate in unvalidated project settings: resolveContextFileNames trims and filters non-strings but rejects no separators. A project's .qwen/settings.json sets context.fileName: '../escape.md' (settings load performs no schema validation); any wiring of the session name into writeWorkspaceContextFile (the wiring R2-9 asks for) resolves path.join(projectRoot, '../escape.md') outside the workspace root. Round-2 probe: writeWorkspaceContextFile({scope:'workspace', mode:'append', projectRoot: <tmp>/workspace, contextFileName: '../escape.md'}) → ESCAPE: filePath=<tmp>/escape.md. Validate containment before the join — reject (or strip to basename) any name containing a separator or resolving outside projectRoot, and apply the same check in resolveContextFileNames where settings-sourced names enter; fix witness: a test passing '../escape.md' asserting the write is refused or stays inside projectRoot — red without the check.
中文说明
仍然成立——该 contextFileName 拼接没有包含性校验，且选项的文档注释邀请调用方传入源自未校验项目设置的会话名：resolveContextFileNames 会 trim 并过滤非字符串，但不拒绝路径分隔符。项目的 .qwen/settings.json 设 context.fileName: '../escape.md'（设置加载不做 schema 校验）；任何把会话名接入 writeWorkspaceContextFile 的接线（正是 R2-9 所要求的）都会把 path.join(projectRoot, '../escape.md') 解析到工作区根之外。第 2 轮探针：writeWorkspaceContextFile({scope:'workspace', mode:'append', projectRoot: <tmp>/workspace, contextFileName: '../escape.md'}) → ESCAPE: filePath=<tmp>/escape.md。建议在拼接前做包含性校验——拒绝（或剥成 basename）任何含分隔符或解析到 projectRoot 之外的名字，并在设置来源名字进入的 resolveContextFileNames 处做同样检查；修复见证：传入 '../escape.md' 的测试断言写入被拒绝或保持在 projectRoot 内——没有该校验时应为红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| return; | ||
| } | ||
| toolRegistry.registerTool(new CreateSubSessionTool(config)); | ||
| toolRegistry.registerSessionTool(new CreateSubSessionTool(config)); |
There was a problem hiding this comment.
[Suggestion] R1-12: still stands — Session's switch from registerTool/registerPermissionDeferredFactory to the session-owned variants is pinned only for capture_screen_context; create_sub_session (here and the deferred branch), speak_to_user, and the live-voice tool batch assert only the plain delegate mocks (Session.test.ts maps registerSessionTool/registerSessionPermissionDeferredFactory to their delegates). Reverting create_sub_session or speak_to_user to plain registerTool keeps Session.test.ts green, yet the next /cd's replaceCoreToolsFrom would then dispose the tool mid-session and drop the model's speak_to_user/create_sub_session surface. Mirror the capture-screen-context assertion for these registrations, asserting the session variants directly (not their delegates); fix witness: reverting Session.ts to the plain registrations at those sites must turn the new assertions red.
中文说明
仍然成立——Session 从 registerTool/registerPermissionDeferredFactory 切换到会话自有变体，目前只为 capture_screen_context 固定；create_sub_session（此处及延迟分支）、speak_to_user 与实时语音工具批只断言了普通的委托 mock（Session.test.ts 把 registerSessionTool/registerSessionPermissionDeferredFactory 映射到其委托）。把 create_sub_session 或 speak_to_user 回退为普通 registerTool，Session.test.ts 仍为绿，而下一次 /cd 的 replaceCoreToolsFrom 会在会话中途处置该工具，模型将失去 speak_to_user/create_sub_session 入口。建议仿照 capture-screen-context 的断言为这些注册直接断言会话变体（而非委托）；修复见证：把 Session.ts 这些位置回退为普通注册后，新断言应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| return this.targetDir; | ||
| } | ||
|
|
||
| private applyProjectRuntimeConfig(runtime: ProjectRuntimeConfig): void { |
There was a problem hiding this comment.
[Suggestion] R1-21: still stands — class: applyProjectRuntimeConfig re-applies ~50 settings-derived knobs but silently skips other settings-derived requiresRestart: false fields (maxToolCallsPerTurn, gitCoAuthor, and siblings), so after /cd the session keeps the OLD project's values for them — the user sees project B as active while behavior still comes from project A, the exact mixed runtime issue #10173 asks to avoid. The author acknowledged the class is real and deferred the audit ('Not in this commit… leaving the thread open'). Audit the requiresRestart: false, settings-derived fields and either add them to ProjectRuntimeConfig/applyProjectRuntimeConfig or document each deliberate exclusion at the interface; fix witness: per-field relocation assertions mirroring the 'apply the prepared project runtime' suite — each red without the corresponding assignment.
中文说明
仍然成立——类级问题：applyProjectRuntimeConfig 重新应用约 50 个来自设置的旋钮，但静默跳过其他来自设置且 requiresRestart: false 的字段（maxToolCallsPerTurn、gitCoAuthor 等），/cd 后这些字段仍是"旧"项目的值——用户看到项目 B 已生效、部分行为却仍来自项目 A，正是 issue #10173 要求避免的混合运行时。作者已确认该类问题真实存在并推迟审计（"本提交不做……保留线程"）。建议审计所有 requiresRestart: false 的设置派生字段，要么加入 ProjectRuntimeConfig/applyProjectRuntimeConfig，要么在接口处逐一文档化有意的排除；修复见证：仿照 'apply the prepared project runtime' 套件的逐字段迁移断言——缺少对应赋值时各自为红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| this.notifyChangeListeners(); | ||
| } | ||
|
|
||
| async refreshForProjectChange(): Promise<void> { |
There was a problem hiding this comment.
[Suggestion] R1-25: still stands — SubagentManager.refreshForProjectChange and its twin SkillManager.refreshForProjectChange (skill-manager.ts:516) have no direct tests — they appear in test code only as bare vi.fn().mockResolvedValue(undefined) mocks in the relocation suite, which never asserts either is called. Deleting either call from relocateWorkingDirectory (or breaking a method body — e.g. dropping the subagent failure branch that deletes the stale 'project' cache) leaves every existing test green, and a session that /cds keeps project A's skills/subagents launchable in project B. The author deferred ('coverage work on code this PR did not change… leaving the thread open'). Add direct unit tests — subagent-manager: refreshCache rejects, assert the 'project' cache entry is evicted, listeners notified, and the error re-thrown; skill-manager: cache re-scan for the new project and watcher re-rooting — plus expect(...refreshForProjectChange).toHaveBeenCalledOnce() in the relocation test; deleting either call site must turn them red.
中文说明
仍然成立——SubagentManager.refreshForProjectChange 与其孪生方法 SkillManager.refreshForProjectChange（skill-manager.ts:516）没有直接测试——在迁移测试套件中它们只以裸 vi.fn().mockResolvedValue(undefined) mock 出现，且从不断言它们被调用。从 relocateWorkingDirectory 删除任一调用（或破坏方法体——例如丢弃 subagent 失败分支中删除过期 'project' 缓存的逻辑）都不会让任何现有测试变红，/cd 的会话将继续在项目 B 中启动项目 A 的 skills/subagents。作者已推迟（"属于本 PR 未改动代码的覆盖工作……保留线程"）。建议补直接单测——subagent-manager：refreshCache 拒绝时断言 'project' 缓存条目被清除、监听者被通知、错误被重新抛出；skill-manager：新项目的缓存重扫与 watcher 重定根——并在迁移测试中补 expect(...refreshForProjectChange).toHaveBeenCalledOnce()；删除任一调用点后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| await this.notifyChangeListeners(); | ||
| } | ||
|
|
||
| async refreshForProjectChange(): Promise<void> { |
There was a problem hiding this comment.
[Suggestion] R1-25: still stands — SubagentManager.refreshForProjectChange and its twin SkillManager.refreshForProjectChange (skill-manager.ts:516) have no direct tests — they appear in test code only as bare vi.fn().mockResolvedValue(undefined) mocks in the relocation suite, which never asserts either is called. Deleting either call from relocateWorkingDirectory (or breaking a method body — e.g. dropping the subagent failure branch that deletes the stale 'project' cache) leaves every existing test green, and a session that /cds keeps project A's skills/subagents launchable in project B. The author deferred ('coverage work on code this PR did not change… leaving the thread open'). Add direct unit tests — subagent-manager: refreshCache rejects, assert the 'project' cache entry is evicted, listeners notified, and the error re-thrown; skill-manager: cache re-scan for the new project and watcher re-rooting — plus expect(...refreshForProjectChange).toHaveBeenCalledOnce() in the relocation test; deleting either call site must turn them red.
中文说明
仍然成立——SubagentManager.refreshForProjectChange 与其孪生方法 SkillManager.refreshForProjectChange（skill-manager.ts:516）没有直接测试——在迁移测试套件中它们只以裸 vi.fn().mockResolvedValue(undefined) mock 出现，且从不断言它们被调用。从 relocateWorkingDirectory 删除任一调用（或破坏方法体——例如丢弃 subagent 失败分支中删除过期 'project' 缓存的逻辑）都不会让任何现有测试变红，/cd 的会话将继续在项目 B 中启动项目 A 的 skills/subagents。作者已推迟（"属于本 PR 未改动代码的覆盖工作……保留线程"）。建议补直接单测——subagent-manager：refreshCache 拒绝时断言 'project' 缓存条目被清除、监听者被通知、错误被重新抛出；skill-manager：新项目的缓存重扫与 watcher 重定根——并在迁移测试中补 expect(...refreshForProjectChange).toHaveBeenCalledOnce()；删除任一调用点后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
3 participants
What this PR does
This PR reloads project-scoped runtime state when an existing session changes its working directory with
/cd. The switch is transactional for settings and file watching, then refreshes context files, permissions, tools, hooks, skills, subagents, MCP servers, system instructions, and durable cron scheduling for the destination project. Session-owned tools remain registered, while stale resources from the previous project are removed.Why it's needed
Previously,
/cdchanged the process working directory but left runtime capabilities derived from the original project active until restart. That could expose the new project to permissions, hooks, tools, context, or background work that belonged to the previous project. Reloading the complete project runtime keeps the active session aligned with its current directory.Reviewer Test Plan
How to verify
/cd <project-b>where project B defines different values, and verify project-B capabilities become active without starting a new conversation.Evidence (Before & After)
Before:
/cdchanged the working directory while retaining project-scoped runtime capabilities from the original project.After: the destination project runtime is loaded transactionally; blocking preparation failures roll back the switch, and non-blocking refresh failures report warnings without restoring stale capabilities.
Tested on
Environment (optional)
Node.js 22 on macOS. Full build, typecheck, lint, and 4,114 focused and downstream tests passed after rebasing onto the latest upstream main.
Risk & Scope
Linked Issues
Closes #10173
中文说明
本 PR 做了什么
本 PR 在现有会话通过
/cd更改工作目录时重新加载项目级运行时状态。切换过程会以事务方式更新设置和文件监听，然后刷新目标项目的上下文文件、权限、工具、Hooks、Skills、子智能体、MCP 服务、系统指令和持久化定时任务。会话级工具会继续保留，旧项目遗留的项目级资源会被移除。为什么需要
此前
/cd只会改变进程工作目录，启动时从原项目加载的运行时能力会一直保留到重启。这可能让新项目继续使用旧项目的权限、Hooks、工具、上下文或后台任务。完整重载项目运行时后，当前会话的能力会与当前目录保持一致。Reviewer Test Plan
如何验证
/cd <project-b>，让项目 B 提供不同配置，确认无需新建会话即可启用项目 B 的能力。证据（修改前后）
修改前：
/cd改变工作目录后，原项目的项目级运行时能力仍然保留。修改后：目标项目运行时以事务方式加载；阻断性的准备失败会回滚切换，非阻断刷新失败会报告警告且不会恢复旧项目能力。
测试平台
环境（可选）
macOS、Node.js 22。在变基到最新上游主干后，全量构建、类型检查、Lint 以及 4,114 项重点和下游测试均通过。
风险与范围
关联 Issue
Closes #10173