fix(acp): isolate workspace settings and context file resolution for worktree sessions - #8152
fix(acp): isolate workspace settings and context file resolution for worktree sessions#8152Aleks-0 wants to merge 23 commits into
Conversation
…worktree sessions When a session operates inside a git worktree, settings and context file paths were resolved against the project root instead of the worktree. Add worktree-aware fallback in extMethodInternal (defaultSettingsCwd), optional resolveContextFile callback in workspace-service, and worktreeRootPath on desktop BackendHostRuntimeContext. All changes are additive with null/undefined defaults — existing behavior is preserved when no worktree is involved. Fixes QwenLM#8138 QwenCode QwenLLM
github-actions
Bot
added
the
review/self-reported
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with a clear root cause. Issue #8138 identifies the exact code path ( Direction: aligned. Worktree isolation is a core qwen-code feature ( Size: cross-package change (cli + desktop) → core infrastructure by the cross-package criterion. 75 production lines (22+1 acpAgent, 8 Session, 4 run-qwen-serve, 13+5 workspace-service/index, 11 types, 2 desktop types, 8+1 qwen-agent) vs. 57 test lines (14 Session.worktree.test, 43 facade.test). Well under any threshold. Approach: the scope feels right. Three small extension points — a fallback in the ACP settings handler, an optional resolver callback in the workspace service, and a forward-looking field on the desktop runtime context — each falls through to existing behavior when unset. One question worth thinking about: the Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，根因清晰。Issue #8138 指出了确切的代码路径（ 方向：对齐。Worktree 隔离是 qwen-code 的核心功能（ 规模：跨包变更（cli + desktop）→ 按跨包标准属于核心基础设施。75 行生产代码 vs. 57 行测试代码，远低于任何阈值。 方案：范围合理。三个小扩展点——ACP 设置处理器中的回退、工作区服务中的可选解析回调、桌面运行时上下文上的前瞻字段——未设置时均回退到现有行为。一个值得思考的问题： 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewIndependent proposal: given the root cause (settings handler falls back to Comparison: the PR does exactly this. The approach matches — No critical blockers found. Two observations:
Convention check: ESM ✓, no CI test evidenceCI is still running on the reviewed commit. Key checks: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 macOS and Windows tests are skipped (fork PR — expected). The ubuntu unit suite and serve A/B are still in flight; the finalize workflow will update this table when they land. Sandboxed verification would settle this: 中文说明代码审查独立方案： 鉴于根因（设置处理器回退到 对比： PR 完全这样做了。方案匹配——agent 上的 未发现关键阻塞项。两个观察：
CI 测试证据CI 仍在运行。macOS 和 Windows 测试被跳过（fork PR——预期行为）。ubuntu 单元测试和 serve A/B 仍在进行中；finalize 工作流会在完成后更新此表。 沙箱验证可以确认： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal fix that matches the problem exactly; the untested core path and desktop dead switch are non-blocking nits. This is a well-scoped bug fix. The approach is what I'd have proposed independently — store the worktree path at session creation, inject it into the settings fallback chain, clean up on close. Three small extension points, each falling through to existing behavior when unset. The code reads cleanly, the limitations are honestly stated (agent-global state, deferred desktop wiring), and every downstream consumer is accounted for. The two nits: the Approval deferred until CI lands green on 中文说明置信度：4/5 —— 干净、最小化的修复，精确匹配问题；未测试的核心路径和桌面端死开关是非阻塞的小问题。 这是一个范围良好的 bug 修复。方案与我的独立提案一致——在会话创建时存储 worktree 路径，注入设置回退链，关闭时清理。三个小扩展点，未设置时均回退到现有行为。代码清晰，限制诚实说明（agent 全局状态、推迟的桌面端接线），每个下游消费者都有交代。 两个小问题：实际修复 #8138 的 批准推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
— qwen3.8-max-preview via Qwen Code /review
| const targetDir = config.getTargetDir(); | ||
| if (targetDir !== process.cwd()) { |
There was a problem hiding this comment.
[Critical] The new config.getTargetDir() call here breaks the existing acpAgent.worktree.test.ts suite. Its mock config does not implement getTargetDir, so the three 'Phase C worktree context restore VP1/VP2/VP2b' tests throw TypeError: config.getTargetDir is not a function. Verified: these pass 3/3 on the base commit and fail 3/3 with this PR — a regression introduced by this diff, and the cause of the red 'Test (ubuntu-latest, Node 22.x)' CI check. — Failure scenario: npm test --workspace=packages/cli → 3 tests in acpAgent.worktree.test.ts fail because createAndStoreSession now calls a method the test's hand-built config mock lacks.
Fix: add the method to the mock config in acpAgent.worktree.test.ts (and any other suite that drives createAndStoreSession with a hand-built config):
getTargetDir: vi.fn().mockReturnValue(process.cwd()),— qwen3.8-max-preview via Qwen Code /review
| if (targetDir !== process.cwd()) { | ||
| session.worktreeCwd = targetDir; | ||
| this.defaultSettingsCwd = targetDir; | ||
| } else { | ||
| this.defaultSettingsCwd = null; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The core behaviour this PR adds — settings resolving against the worktree via the defaultSettingsCwd lifecycle (set-on-create here, the fallback at acpAgent.ts:7252, and the conditional clear-on-close near line 3909) — has no behavioural test. The only test added for it (Session.worktree.test.ts) is a trivial getter/setter on worktreeCwd. — Concrete cost: if the clear-on-close condition regresses (e.g. inverted), a later regular session whose client omits cwd would read/write the stale worktree's settings.json with no test failing. This is testable with existing infra — acpAgent.test.ts already drives agent.newSession({ cwd }) and agent.extMethod(...).
Add a test: create a session whose config.getTargetDir() differs from process.cwd(), call a settings ext method (e.g. qwen/settings/getMemory) with no cwd, and assert loadSettings received the worktree dir; then close it and assert a subsequent no-cwd call resolves against process.cwd() again.
— qwen3.8-max-preview via Qwen Code /review
| resolveContextFile: (filename, ws) => ({ | ||
| target: path.resolve(ws, filename), | ||
| effectiveWorkspace: ws, | ||
| }), |
There was a problem hiding this comment.
[Suggestion] This resolveContextFile is a no-op: it receives ws === boundWorkspace and returns exactly what the default fallback in workspace-service/index.ts already computes ({ target: path.resolve(boundWorkspace, filename), effectiveWorkspace: boundWorkspace }). The other two production call sites (run-qwen-serve.ts:4093, :4595) omit the parameter entirely. — Concrete cost: the indirection changes no behaviour, but a reader concludes worktree-aware context-file resolution is wired up here when it is not, and a future edit to the default fallback would silently diverge from this bypassing resolver.
Either omit resolveContextFile at this call site until a real worktree-aware resolver exists, or mark the placeholder intent:
// TODO(#8138): replace with a worktree-aware resolver
resolveContextFile: (filename, ws) => ({
target: path.resolve(ws, filename),
effectiveWorkspace: ws,
}),— qwen3.8-max-preview via Qwen Code /review
| // TODO(#8138): populate worktreeRootPath in buildBackendHostRuntimeContext | ||
| // so desktop worktree sessions resolve settings against the worktree. | ||
| return ( | ||
| hostRuntime.worktreeRootPath || |
There was a problem hiding this comment.
[Suggestion] worktreeRootPath is declared (backend/types.ts:202) and read here as the highest-priority cwd source, but has no write site anywhere — grep finds exactly three hits (declaration, this TODO, this read), and neither buildBackendHostRuntimeContext sets it. So qwenSettingsCwd always falls through to appRootPath (pre-PR behaviour) and the desktop half of this feature is inert. — Concrete cost: a developer debugging desktop settings resolution sees worktreeRootPath at the top of the chain and assumes it is active. The TODO documents the gap, but shipping a dead read site that looks live misleads grep-based investigation. (Already noted as deferred follow-up in the PR description.)
Either populate worktreeRootPath in buildBackendHostRuntimeContext in the PR that relies on it, or defer this read site until the write site exists (leave only the TODO comment).
— qwen3.8-max-preview via Qwen Code /review
| const withinWorkspace = | ||
| target === boundWorkspace || | ||
| target.startsWith(boundWorkspace + path.sep); | ||
| target === effectiveWorkspace || | ||
| target.startsWith(effectiveWorkspace + path.sep); |
There was a problem hiding this comment.
[Suggestion] The path-escape boundary check is only exercised through the no-resolver branch (facade.test.ts 'throws for escaping filename' passes no resolveContextFile). The two new resolver tests are happy-path only. — Concrete cost: if a future resolver returns a target outside its declared effectiveWorkspace, initWorkspace would write the context file outside the workspace instead of throwing WorkspaceInitPathEscapeError, and no test would catch the regression in this security check.
Add a facade test passing a resolveContextFile whose target lies outside its returned effectiveWorkspace, and assert initWorkspace rejects with /resolves outside/.
— qwen3.8-max-preview via Qwen Code /review
- Add getTargetDir to mock config in acpAgent.worktree.test.ts (fixes 3 CI test failures) - Add behavioral test for defaultSettingsCwd lifecycle (worktree set, regular reset) - Add facade test for path-escape through resolveContextFile branch - Add TODO(QwenLM#8138) to identity resolveContextFile in run-qwen-serve.ts QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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-preview via Qwen Code /review
| const requestedCwd = | ||
| typeof params['cwd'] === 'string' ? params['cwd'] : undefined; | ||
| const cwd = requestedCwd || process.cwd(); | ||
| const cwd = requestedCwd || this.defaultSettingsCwd || process.cwd(); |
There was a problem hiding this comment.
[Suggestion] defaultSettingsCwd is an agent-level scalar that every createAndStoreSession overwrites, but this.sessions holds multiple concurrent sessions — so the last session created wins settings resolution for all of them. This read site feeds cwd into loadSettings(cwd) for every qwen/settings/* handler, including the write handlers (setCoreValue, setMcpServer, setMemory).
Failure scenario: if a client ever holds two concurrent sessions with differing worktrees and issues a settings call without an explicit cwd, the write lands in the wrong workspace's .qwen/settings.json — the exact mix-up the codebase warns about in the this.settings concurrency comment. No current client triggers it (the desktop client always passes an explicit cwd, so this chain short-circuits at requestedCwd), and the PR description documents the tradeoff — hence Suggestion, not blocker.
The long-term path (per-session resolution via Session.worktreeCwd) is already introduced by this PR. Consider pulling it forward: have the settings handlers accept a sessionId and resolve this.sessions.get(sessionId)?.worktreeCwd || process.cwd() (as PROMPT_CANCEL_METHOD already does), dropping the agent-global mirror. At minimum, a one-line note on the field documenting the single-worktree-session assumption would stop a future maintainer extending multi-worktree support on top of it.
// e.g. resolve per-session instead of via the agent-global mirror:
const cwd =
requestedCwd ||
this.sessions.get(sessionId ?? '')?.worktreeCwd ||
process.cwd();中文说明
defaultSettingsCwd 是一个代理级标量，每次 createAndStoreSession 都会覆盖它，但 this.sessions 可容纳多个并发会话——因此最后创建的会话会赢得所有会话的设置解析。此读取点将 cwd 传入每个 qwen/settings/* 处理器的 loadSettings(cwd)，包括写入处理器（setCoreValue、setMcpServer、setMemory）。
失败场景：如果某个客户端同时持有两个 worktree 不同的并发会话，并在不传显式 cwd 的情况下发起设置调用，写入会落到错误工作区的 .qwen/settings.json——正是代码库在 this.settings 并发注释中警告的混淆。当前没有客户端会触发（桌面客户端总是传递显式 cwd，因此该链在 requestedCwd 处短路），且 PR 描述已记录此权衡——故为建议而非阻塞项。
长期方案（通过 Session.worktreeCwd 按会话解析）已由本 PR 引入。可考虑提前实施：让设置处理器接受 sessionId 并解析 this.sessions.get(sessionId)?.worktreeCwd || process.cwd()（如同 PROMPT_CANCEL_METHOD 已有的做法），移除代理级镜像。至少，在字段上加一行注释说明单 worktree 会话假设，可避免未来维护者在其之上扩展多 worktree 支持。
— qwen3.8-max-preview via Qwen Code /review
…ingsCwd QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 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-preview via Qwen Code /review
| if ( | ||
| session.worktreeCwd && | ||
| session.worktreeCwd === this.defaultSettingsCwd | ||
| ) { | ||
| this.defaultSettingsCwd = null; | ||
| } |
There was a problem hiding this comment.
[Suggestion] No test covers this session-close path that clears defaultSettingsCwd. The new test in acpAgent.test.ts exercises set-on-create and use-on-read, but never closes the worktree session and re-checks the fallback. — Failure scenario: if this clearing condition regresses (removed or inverted), defaultSettingsCwd stays pinned to a stale worktree path after the owning session closes; a later qwen/settings/getCore without an explicit cwd then resolves settings against the defunct worktree dir instead of process.cwd(), loading the wrong settings.json — and no test catches it.
Suggested fix — add a test that creates a worktree session, closes it, then asserts the fallback:
await agent.newSession({ cwd: '/fake/project', mcpServers: [] });
await agent.closeSession({ sessionId: worktreeSessionId });
vi.mocked(loadSettings).mockClear();
await agent.extMethod('qwen/settings/getCore', {});
expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(process.cwd());中文说明
[Suggestion] 没有测试覆盖这条清除 defaultSettingsCwd 的会话关闭路径。acpAgent.test.ts 中的新测试覆盖了创建时设置与读取时使用，但从未关闭 worktree 会话并复查回退行为。— 失败场景：如果此清除条件发生回归（被删除或取反），defaultSettingsCwd 会在所属会话关闭后仍停留在过期的 worktree 路径上；之后一次不带显式 cwd 的 qwen/settings/getCore 会解析到已失效的 worktree 目录而非 process.cwd()，加载错误的 settings.json——且没有测试能捕获。
建议修复——添加一个测试：创建 worktree 会话、关闭它，然后断言回退行为（见上方代码块）。
— qwen3.8-max-preview via Qwen Code /review
QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. 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.7-max via Qwen Code /review
Local runtime verification reportBuilt two real artifacts — Verdict: the mechanism behaves exactly as its unit tests describe, but the flow issue #8138 actually describes is unchanged, and no shipped client can reach the new code path today. I'd hold merging it as Fixes #8138 until the 1. What the PR does fix ✅S1 — session created with
The context file moves with it in this path too — S5 — a caller that passes an explicit 2. Finding 1 — the worktree is entered after the session exists (blocking for "Fixes #8138")
At the moment S2 (screenshot above) — Same story end-to-end at the daemon level —
Verified fix. Patching the built artifact so the session.worktreeCwd = canonicalPath;
this.defaultSettingsCwd = canonicalPath === process.cwd() ? null : canonicalPath;Note this does not cover the daemon's REST surface: 3. Finding 2 — no shipped client reaches the new fallback today
So the new path is currently reachable only by a client that both omits 4. Finding 3 — the trigger is "cwd ≠ launch dir", not "worktree"S3 — a plain, non-worktree project directory: BEFORE writes to the agent's launch dir, AFTER writes to that project dir. Related: the fallback was added to the shared 5. Finding 4 — close order leaves a live worktree session un-isolatedS4 — sessions A ( The clear-on-close resets to 6. Tests — all pass, 4 of 5 mutants killedAll four changed test files pass locally: 466/466. Test-teeth check — revert one production line at a time, keep the PR's own tests:
M5 survives because the new escape test asserts Suggestions before merge
How this was verified (repro)# two arms, same tree, only packages/cli/src differs
git worktree add /root/git/wt8152 main
git apply pr8152.patch # AFTER
npm run build -w @qwen-code/qwen-code-core -w @qwen-code/acp-bridge -w @qwen-code/qwen-code
cp -a packages/cli/dist dist-cli-AFTER
git checkout -- packages/cli/src # BEFORE
npm run build -w @qwen-code/qwen-code && cp -a packages/cli/dist dist-cli-BEFORE
# test bed: isolated HOME, git repo + two real worktrees under .qwen/worktrees/
# ACP probe: spawn `node packages/cli/dist/index.js --acp`, drive it with a real
# ClientSideConnection (initialize → newSession → extMethod), observe which
# .qwen/settings.json file appears on disk
# daemon probe: `qwen serve --port 0 --token … --workspace <project>` then
# POST /session {"worktree":{}} → GET /session/:id/status → POST /workspace/settings
# → POST /workspace/initScenarios: S1 session-created-in-worktree · S2 cd-into-worktree-mid-session · S3 plain session in another dir · S4 two concurrent worktree sessions · S5 explicit 中文说明本地运行时验证报告我在本地构建了两份真实产物 —— 结论： 机制本身与单测描述一致，但 issue #8138 真正描述的那条链路没有变化，而且当前没有任何已发布的客户端能走到新代码路径。建议在补上 1. 本 PR 确实修好的部分 ✅S1 —— 会话以 S5 —— 显式传 2. 发现 1 —— worktree 是在会话创建之后才进入的（阻塞 “Fixes #8138”）
S2 与守护进程级别的验证都显示：两个 arm 完全一致，设置仍写到项目根目录， 已验证的修法：在 3. 发现 2 —— 当前没有客户端会走到新回退
4. 发现 3 —— 触发条件是“cwd ≠ 启动目录”，而不是“worktree”S3：一个普通的、非 worktree 的项目目录也会触发重定向， 5. 发现 4 —— 关闭顺序会让仍在运行的 worktree 会话失去隔离S4：A（alpha）、B（beta）两个 worktree 会话并存时写入 beta（文档已说明“后创建者胜出”）；关闭 B 之后，尽管 A 仍然活着并位于 alpha，写入却回落到了项目根目录。clear-on-close 只置 6. 测试 —— 全部通过，5 个变异中杀死 4 个4 个改动文件的测试本地全绿：466/466。变异测试（逐条回退一行生产代码、保留 PR 自带测试）：M1 读取端回退、M2 创建时赋值、M3 关闭时清理、M4 resolver 注入 —— 全部 KILLED；M5（把边界检查里的 合入前建议
|
…s cwd, daemon worktree-awareness - Update sessionCd handler to set session.worktreeCwd and defaultSettingsCwd after relocateWorkingDirectory (fixes S2 scenario) - Add resolveSettingsCwd with per-session worktree resolution via sessionId param; remove defaultSettingsCwd from shared cwd to scope fallback to settings/permissions handlers only - Wire daemon resolveContextFile and POST /workspace/settings to session worktree via findEffectiveWorkspace helper - Tighten escape test to WorkspaceInitPathEscapeError class (kills M5 mutant) - Extract shared findEffectiveWorkspace helper (dedup 3 copies) QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
— qwen3.8-max-preview via Qwen Code /review
| const session = new Session(sessionId, config, this.connection, settings); | ||
| this.sessions.set(sessionId, session); | ||
|
|
||
| const targetDir = config.getTargetDir(); |
There was a problem hiding this comment.
[Critical] This assignment only fires when config.getTargetDir() !== process.cwd(), but the enter_worktree tool — explicitly named in issue #8138 — never changes getTargetDir(): it creates a worktree and returns its path without calling relocateWorkingDirectory, triggering qwen/control/session/cd, or mutating the config target dir. For an enter_worktree session, session.worktreeCwd and defaultSettingsCwd therefore stay null, and resolveSettingsCwd falls through to process.cwd() (the project root). — Failure scenario: a user follows the issue's own repro — enter a worktree via enter_worktree, then change a workspace-scoped setting with no explicit cwd → the setting still lands in <projectRoot>/.qwen/settings.json, not the worktree's. The issue's named scenario remains broken, so "Fixes #8138" is overstated. (The session/cd handler at ~acpAgent.ts:9050 does set both fields, so the cd-into-worktree path works; only the enter_worktree path is uncovered.)
Fix direction (spans files, so not a one-click suggestion): have enter_worktree relocate the session — call the session/cd ext-method, or set session.worktreeCwd + defaultSettingsCwd after worktree creation — or drop "Fixes #8138" and land this as scaffolding with a follow-up for the enter_worktree path.
中文说明
[Critical] 此赋值仅在 config.getTargetDir() !== process.cwd() 时触发，但 issue #8138 明确点名的 enter_worktree 工具从不改变 getTargetDir()：它创建 worktree 并返回路径，却不调用 relocateWorkingDirectory、不触发 qwen/control/session/cd、也不修改 config 的 target dir。因此对 enter_worktree 会话，session.worktreeCwd 与 defaultSettingsCwd 始终为 null，resolveSettingsCwd 最终回退到 process.cwd()（项目根目录）。— 失败场景：用户按 issue 自身的复现路径操作——用 enter_worktree 进入 worktree，再在不传显式 cwd 的情况下更改工作区范围设置 → 设置仍写入 <projectRoot>/.qwen/settings.json，而非 worktree 自身。issue 点名的场景依旧未修复，因此 "Fixes #8138" 言过其实。（session/cd 处理器在约 acpAgent.ts:9050 处确实同时设置了两个字段，所以 cd 进入 worktree 的路径是通的；只有 enter_worktree 路径未被覆盖。）
修复方向（跨文件，故非一键 suggestion）：让 enter_worktree 迁移会话——调用 session/cd 扩展方法，或在创建 worktree 后设置 session.worktreeCwd + defaultSettingsCwd；或者去掉 "Fixes #8138"，将本 PR 作为脚手架合入，并为 enter_worktree 路径开一个后续 issue。
— qwen3.8-max-preview via Qwen Code /review
| const sessions = bridge.listWorkspaceSessions(boundWorkspace); | ||
| const relocated = sessions.find((s) => s.workspaceCwd !== boundWorkspace); | ||
| return relocated?.workspaceCwd ?? boundWorkspace; |
There was a problem hiding this comment.
[Critical] findEffectiveWorkspace can never return a relocated worktree from the production bridge, so every boundWorkspace → effectiveWorkspace substitution this PR makes (in server.ts, run-qwen-serve.ts, and workspace-settings.ts) is a no-op in production. The real bridge's resolveWorkspaceKey (bridge.ts:3228) throws WorkspaceMismatchError for any workspace other than boundWorkspace, so every registered session has workspaceCwd === boundWorkspace; listWorkspaceSessions (bridge.ts:6563) then filters to entry.workspaceCwd === key. Thus sessions.find((s) => s.workspaceCwd !== boundWorkspace) is always undefined, and this function always returns boundWorkspace. The worktree location is actually carried in BridgeSessionSummary.worktree.path (bridgeTypes.ts:98), which this function never reads. — Failure scenario: with an active qwen serve worktree session, POST /workspace/settings and POST /workspace/init still resolve against the project root, so the serve-side half of this PR never fires. The new worktree-workspace.test.ts mocks listWorkspaceSessions with summaries whose workspaceCwd differs from the queried workspace — a shape the production bridge cannot emit (it would throw WorkspaceMismatchError at registration) — so the tests pass while the behavior is dead.
Fix direction (requires widening the SessionLister session shape, so not a one-click suggestion): resolve from the worktree metadata the bridge actually exposes, e.g.
const relocated = sessions.find((s) => s.worktree);
return relocated?.worktree?.path ?? boundWorkspace;and add at least one integration-level assertion that a real bridge with a worktree session produces the relocated path.
中文说明
[Critical] findEffectiveWorkspace 在生产 bridge 下永远无法返回已迁移的 worktree，因此本 PR 所做的每一处 boundWorkspace → effectiveWorkspace 替换（位于 server.ts、run-qwen-serve.ts、workspace-settings.ts）在生产环境中都是空操作。真实 bridge 的 resolveWorkspaceKey（bridge.ts:3228）对任何非 boundWorkspace 的 workspace 都会抛 WorkspaceMismatchError，所以每个已注册会话都满足 workspaceCwd === boundWorkspace；listWorkspaceSessions（bridge.ts:6563）随后按 entry.workspaceCwd === key 过滤。于是 sessions.find((s) => s.workspaceCwd !== boundWorkspace) 恒为 undefined，本函数恒返回 boundWorkspace。worktree 位置实际存放在 BridgeSessionSummary.worktree.path（bridgeTypes.ts:98），而本函数从不读取它。— 失败场景：当存在活跃的 qwen serve worktree 会话时，POST /workspace/settings 与 POST /workspace/init 仍解析到项目根目录，因此本 PR 的 serve 端那一半根本不生效。新增的 worktree-workspace.test.ts 用 workspaceCwd 与被查询 workspace 不同的 summary 来 mock listWorkspaceSessions —— 这种形状是生产 bridge 不可能发出的（注册时就会抛 WorkspaceMismatchError）—— 所以测试通过，但行为是死的。
修复方向（需要扩展 SessionLister 的会话形状，故非一键 suggestion）：从 bridge 实际暴露的 worktree 元数据解析，例如：
const relocated = sessions.find((s) => s.worktree);
return relocated?.worktree?.path ?? boundWorkspace;并至少增加一个集成级断言：真实 bridge 在存在 worktree 会话时能产生迁移后的路径。
— qwen3.8-max-preview via Qwen Code /review
| if (session?.worktreeCwd) { | ||
| return session.worktreeCwd; | ||
| } | ||
| } | ||
| return this.defaultSettingsCwd || process.cwd(); |
There was a problem hiding this comment.
[Suggestion] When a sessionId is supplied for a regular (non-worktree) session, the session is found but session.worktreeCwd is null, so this guard fails and execution falls through to return this.defaultSettingsCwd || process.cwd(). If a concurrent worktree session has set defaultSettingsCwd, the regular session's settings resolve to that worktree directory — contradicting the doc comment on defaultSettingsCwd, which says it is the fallback "when the client supplies neither an explicit cwd nor a sessionId" and that passing sessionId "avoid[s] the last-wins ambiguity". This branch also has no test coverage (both new tests call getCore with no sessionId). — Failure scenario: create regular session B, then worktree session A (defaultSettingsCwd = <worktree>), then call qwen/settings/getCore with { sessionId: B } → B's settings load from / persist to A's worktree dir instead of process.cwd().
| if (session?.worktreeCwd) { | |
| return session.worktreeCwd; | |
| } | |
| } | |
| return this.defaultSettingsCwd || process.cwd(); | |
| if (session) { | |
| return session.worktreeCwd ?? process.cwd(); | |
| } | |
| } | |
| return this.defaultSettingsCwd || process.cwd(); |
中文说明
[Suggestion] 当为一个常规（非 worktree）会话提供 sessionId 时，会话能找到，但 session.worktreeCwd 为 null，于是此守卫失败，执行落到 return this.defaultSettingsCwd || process.cwd()。如果此时有另一个并发的 worktree 会话设置了 defaultSettingsCwd，常规会话的设置就会解析到那个 worktree 目录——这与 defaultSettingsCwd 的文档注释相矛盾：注释称它是"当客户端既不提供显式 cwd 也不提供 sessionId 时"的回退，并称传 sessionId 能"避免 last-wins 歧义"。此分支也没有测试覆盖（两个新测试都在不带 sessionId 的情况下调用 getCore）。— 失败场景：先创建常规会话 B，再创建 worktree 会话 A（defaultSettingsCwd = <worktree>），然后用 { sessionId: B } 调用 qwen/settings/getCore → B 的设置会从 A 的 worktree 目录加载/写入，而非 process.cwd()。
（一键 suggestion 见英文半部的代码块：找到会话时短路返回 session.worktreeCwd ?? process.cwd()。）
— qwen3.8-max-preview via Qwen Code /review
| case 'qwen/settings/getMemory': { | ||
| const settings = loadSettings(cwd); | ||
| const settings = loadSettings(settingsCwd); | ||
| this.settings = settings; |
There was a problem hiding this comment.
[Suggestion] The cwd → settingsCwd substitution spans ~15 ext-method handlers (getMemory, getMemoryPaths, setCoreValue, setMcpServer, removeMcpServer, setHook, removeHook, setExtensionSetting, permissions/getSettings, permissions/setRules, reloadEnvironment, …), but the new tests exercise only qwen/settings/getCore with a worktree cwd. The existing tests for these other handlers use the default cwd, so a regression reverting any single one of them to cwd would not be caught. — Concrete cost: if getMemory (or any other handler) were left reading loadSettings(cwd), a worktree session would load settings from the project root and the bug would surface as stale/wrong settings in the client, with every test still green. Fix: extend at least one new worktree test to also call a second handler (e.g. qwen/settings/getMemory or qwen/permissions/getSettings) and assert loadSettings / loadPermissionSettings received the worktree dir.
中文说明
[Suggestion] cwd → settingsCwd 的替换横跨约 15 个扩展方法处理器（getMemory、getMemoryPaths、setCoreValue、setMcpServer、removeMcpServer、setHook、removeHook、setExtensionSetting、permissions/getSettings、permissions/setRules、reloadEnvironment 等），但新测试只在 worktree cwd 下覆盖了 qwen/settings/getCore。这些其它处理器的既有测试都用默认 cwd，因此其中任何一个被回退成 cwd 的回归都不会被发现。— 具体代价：如果 getMemory（或任何其它处理器）仍在读 loadSettings(cwd)，worktree 会话就会从项目根目录加载设置， bug 会以客户端中过期/错误的设置呈现，而所有测试依旧全绿。修复：至少在一个新 worktree 测试中再调用第二个处理器（如 qwen/settings/getMemory 或 qwen/permissions/getSettings），并断言 loadSettings / loadPermissionSettings 收到的是 worktree 目录。
— qwen3.8-max-preview via Qwen Code /review
| const effectiveWorkspace = | ||
| resolveEffectiveWorkspace?.() ?? boundWorkspace; |
There was a problem hiding this comment.
[Suggestion] This new dynamic workspace resolution has no test coverage — workspace-settings.test.ts contains zero references to resolveEffectiveWorkspace. If the wiring is broken (the dep not passed in, or it throws and the ?. swallows the error), settings writes silently fall back to boundWorkspace with no test catching it. — Concrete cost: a worktree session's settings changes would appear not to take effect, with no failing test to point at the cause. Fix: add a test that provides a resolveEffectiveWorkspace returning a different directory, issues a PUT, and asserts persistSetting / prepareSettingWrite received the resolved directory rather than boundWorkspace. (Note: per the findEffectiveWorkspace no-op flagged above, this resolver currently never relocates anyway, so this is the test gap that will matter once that is fixed.)
中文说明
[Suggestion] 这个新的动态 workspace 解析没有测试覆盖——workspace-settings.test.ts 中对 resolveEffectiveWorkspace 零引用。如果接线坏了（依赖没传进来，或它抛错而被 ?. 吞掉），设置写入会静默回退到 boundWorkspace，没有任何测试能发现。— 具体代价：worktree 会话的设置更改会看起来不生效，且没有失败的测试指明原因。修复：增加一个测试，提供一个返回不同目录的 resolveEffectiveWorkspace，发起 PUT，并断言 persistSetting / prepareSettingWrite 收到的是解析后的目录而非 boundWorkspace。（注意：根据上面指出的 findEffectiveWorkspace 空操作问题，此解析器目前本来就不会迁移，所以这是在那个问题修复之后才会显现的测试缺口。）
— qwen3.8-max-preview via Qwen Code /review
| { "path": "packages/desktop" }, | ||
| { "path": "packages/mobile-mcp" }, |
There was a problem hiding this comment.
[Suggestion] The new solution-style references array names several projects that are not composite: true and do not extend the root config — packages/desktop, packages/webui, packages/vscode-ide-companion, packages/chrome-extension, packages/mobile-mcp, packages/sdk-typescript, and packages/web-shell. TypeScript requires every referenced project to be composite (TS6306), so a root tsc --build — the canonical use of a files: [] + references solution tsconfig — fails immediately for those seven. No npm script or CI step runs a root tsc --build today (build is per-workspace via scripts/build.js; typecheck is npm run typecheck --workspaces --if-present), so this does not break CI — hence Suggestion, not Critical. — Concrete cost: the solution config this hunk adds is non-functional and surfaces TS6306 errors in reference-aware IDEs. Fix: either restrict references to the projects that are actually composite build targets (the ones that extend the root and inherit composite: true), or add composite: true + outDir to those seven.
中文说明
[Suggestion] 新的 solution 风格 references 数组点名了若干并非 composite: true、也不继承根 config 的项目——packages/desktop、packages/webui、packages/vscode-ide-companion、packages/chrome-extension、packages/mobile-mcp、packages/sdk-typescript、packages/web-shell。TypeScript 要求每个被引用的项目都是 composite（TS6306），因此在根目录运行 tsc --build（files: [] + references solution tsconfig 的标准用法）会立即在这七个项目上报错。目前没有任何 npm 脚本或 CI 步骤会在根目录运行 tsc --build（build 是通过 scripts/build.js 按 workspace 执行；typecheck 是 npm run typecheck --workspaces --if-present），所以这不会破坏 CI——因此是 Suggestion 而非 Critical。— 具体代价：本 hunk 添加的 solution config 不可用，并在支持 project references 的 IDE 中暴露 TS6306 错误。修复：要么把 references 限制为真正是 composite 构建目标的项目（那些继承根 config 并 inherit composite: true 的），要么给这七个项目加上 composite: true + outDir。
— qwen3.8-max-preview via Qwen Code /review
… code - Add Config.setActiveWorktree/getActiveWorktree; enter_worktree sets it on creation, exit_worktree clears it - resolveSettingsCwd: check config.getActiveWorktree() as fallback; fix fallthrough for regular sessions with sessionId (return process.cwd() instead of leaking another session's worktree) - findEffectiveWorkspace: use session.worktree.path instead of workspaceCwd (bridge never changes workspaceCwd) QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
— qwen3.8-max-preview via Qwen Code /review
| const activeWorktree = this.config?.getActiveWorktree?.(); | ||
| if (activeWorktree) { | ||
| return activeWorktree; |
There was a problem hiding this comment.
[Critical] config.getActiveWorktree() here is dead code in the ACP daemon. setActiveWorktree is only ever called on per-session Config objects (by enter-worktree.ts:214 and exit-worktree.ts:223,366), but this method reads from the agent-level Config (this.config at the QwenAgent constructor, acpAgent.ts:4155) — a different object that never has activeWorktreePath set. — Failure scenario: user invokes enter_worktree in an ACP session (the exact trigger named in issue #8138), then changes a workspace-scoped setting without explicit cwd. The tool sets activeWorktreePath on the session's Config; this fallback reads the agent's Config → null. defaultSettingsCwd is also null (set at session creation when getTargetDir() === process.cwd(), before the worktree existed). Resolution falls through to process.cwd() — the project root. The issue's named scenario remains broken. Wenshao's A/B test (comment 5142259790) independently confirmed: "the flow issue #8138 actually describes is unchanged."
Fix direction: have enter_worktree relocate the session via the session/cd ext-method (which already sets session.worktreeCwd and defaultSettingsCwd correctly at acpAgent.ts:9054-9056), or propagate the worktree path from the session's Config to the agent after the tool completes. If neither is done, drop "Fixes #8138" and land as scaffolding.
中文说明
[Critical] 此处的 config.getActiveWorktree() 在 ACP 守护进程中是死代码。setActiveWorktree 只会在逐会话的 Config 对象上被调用（enter-worktree.ts:214 和 exit-worktree.ts:223,366），但本方法读取的是代理级 Config（QwenAgent 构造函数 acpAgent.ts:4155 处的 this.config）——这是一个不同的对象，其 activeWorktreePath 从未被设置。— 失败场景：用户在 ACP 会话中调用 enter_worktree（issue #8138 点名的触发路径），然后在不传显式 cwd 的情况下更改工作区设置。工具在会话的 Config 上设置了 activeWorktreePath；本回退读取代理的 Config → null。defaultSettingsCwd 同样为 null（在会话创建时 getTargetDir() === process.cwd() 时被置空，此时 worktree 尚不存在）。解析最终回退到 process.cwd()——项目根目录。issue 点名的场景依旧未修复。Wenshao 的 A/B 测试（评论 5142259790）独立确认：“issue #8138 描述的那条链路没有变化。”
修复方向：让 enter_worktree 通过 session/cd 扩展方法迁移会话（该方法已在 acpAgent.ts:9054-9056 正确设置了 session.worktreeCwd 和 defaultSettingsCwd），或在工具完成后将 worktree 路径从会话的 Config 传播到代理。若两者都不做，请去掉 "Fixes #8138"，将本 PR 作为脚手架合入。
— qwen3.8-max-preview via Qwen Code /review
| const effectiveWorkspace = | ||
| resolveEffectiveWorkspace?.() ?? boundWorkspace; |
There was a problem hiding this comment.
[Critical] GET /workspace/settings reads from boundWorkspace (line ~302: buildSettingsResponse(boundWorkspace, …)) while POST writes to effectiveWorkspace (via resolveEffectiveWorkspace), creating a read/write split when a worktree session exists. — Failure scenario: worktree session active → POST persists to <worktree>/.qwen/settings.json → GET calls loadSettings(boundWorkspace) → reads <projectRoot>/.qwen/settings.json → returns stale values. A write-then-read round trip returns old data; a settings UI reload shows the pre-write value even though the worktree session uses the new one. Before this diff both paths used boundWorkspace, so the divergence is introduced by this change.
Apply the same resolveEffectiveWorkspace resolution in the GET handler so reads and writes target the same settings file.
中文说明
[Critical] GET /workspace/settings 从 boundWorkspace 读取（约第 302 行：buildSettingsResponse(boundWorkspace, …)），而 POST 写入 effectiveWorkspace（通过 resolveEffectiveWorkspace），当 worktree 会话存在时造成读写分裂。— 失败场景：worktree 会话活跃 → POST 持久化到 <worktree>/.qwen/settings.json → GET 调用 loadSettings(boundWorkspace) → 读取 <projectRoot>/.qwen/settings.json → 返回旧值。写后读的往返返回旧数据；设置 UI 重新加载显示写入前的值，尽管 worktree 会话使用的是新值。此差异由本变更引入。
在 GET 处理器中应用相同的 resolveEffectiveWorkspace 解析，使读写目标为同一设置文件。
— qwen3.8-max-preview via Qwen Code /review
|
|
||
| // Symlink check on parent path: canonicalize and verify. | ||
| const wsCanonical = await fs.realpath(boundWorkspace); | ||
| const wsCanonical = await fs.realpath(effectiveWorkspace); |
There was a problem hiding this comment.
[Critical] initWorkspace now calls fs.realpath(effectiveWorkspace) where effectiveWorkspace can be a dangling worktree path. findEffectiveWorkspace returns the first session's worktree.path with no existence check; the bridge's setSessionWorktree API does not accept null (bridgeTypes.ts:899), so the field is never cleared while the session lives. exit_worktree action='remove' deletes the directory but does not clear the bridge entry. This realpath call is outside any try/catch. — Failure scenario: worktree session existed → worktree removed (via exit_worktree or git worktree prune) → bridge entry persists → any initWorkspace call → findEffectiveWorkspace returns the dangling path → fs.realpath throws ENOENT → initWorkspace crashes for the whole workspace. Before this diff, line 1000 was fs.realpath(boundWorkspace) — the always-present project root.
Fix: make the resolver fall back when the worktree is gone — e.g. in findEffectiveWorkspace, prefer sessions whose worktree.path passes fs.existsSync, falling back to boundWorkspace; and/or wrap this realpath to catch ENOENT and retry against boundWorkspace.
中文说明
[Critical] initWorkspace 现在调用 fs.realpath(effectiveWorkspace)，而 effectiveWorkspace 可能是一个已不存在的 worktree 路径。findEffectiveWorkspace 返回第一个会话的 worktree.path 且不做存在性检查；bridge 的 setSessionWorktree API 不接受 null（bridgeTypes.ts:899），因此该字段在会话存活期间永远不会被清除。exit_worktree action='remove' 删除目录但不清除 bridge 条目。此 realpath 调用不在任何 try/catch 内。— 失败场景：worktree 会话曾存在 → worktree 被删除 → bridge 条目持续存在 → 任何 initWorkspace 调用 → fs.realpath 抛出 ENOENT → initWorkspace 对整个 workspace 崩溃。变更前第 1000 行是 fs.realpath(boundWorkspace)。
修复：让解析器在 worktree 不存在时回退——例如在 findEffectiveWorkspace 中优先选择 worktree.path 通过 fs.existsSync 检查的会话，回退到 boundWorkspace。
— qwen3.8-max-preview via Qwen Code /review
| return ( | ||
| hostRuntime.worktreeRootPath || | ||
| hostRuntime.appRootPath || |
There was a problem hiding this comment.
[Suggestion] worktreeRootPath is declared (backend/types.ts:202) and read here as the highest-priority cwd source, but has no write site — both buildBackendHostRuntimeContext definitions (SessionManager.ts:288, handlers/utils.ts:20) omit the field. The desktop worktree-settings path is dead code; qwenSettingsCwd always falls through to appRootPath. — Concrete cost: a desktop user entering a worktree session gets no settings isolation; the fix this PR delivers for CLI/serve does not reach desktop users. The TODO at line 595 acknowledges this.
中文说明
[Suggestion] worktreeRootPath 已声明并在此处作为最高优先级 cwd 源读取，但没有写入点——两个 buildBackendHostRuntimeContext 定义都未包含该字段。桌面端 worktree 设置路径是死代码。— 具体代价：桌面用户进入 worktree 会话时无法获得设置隔离。第 595 行的 TODO 已确认此问题。
— qwen3.8-max-preview via Qwen Code /review
| const sessionId = params['sessionId']; | ||
| if (typeof sessionId === 'string' && sessionId.length > 0) { | ||
| const session = this.sessions.get(sessionId); |
There was a problem hiding this comment.
[Suggestion] Three new behavioral paths have zero test assertions — mutations removing any of them would not turn any test red: (1) the per-session resolveSettingsCwd path via sessionId (here — both new tests call extMethod with empty params, so sessionId is always undefined); (2) enter-worktree.ts:214 setActiveWorktree call (mocks added solely to prevent crashes, no assertions); (3) the session/cd handler's worktreeCwd assignment at acpAgent.ts:9054-9056 (no test exercises session/cd followed by a settings extMethod). — Concrete cost: a bug in the session lookup, the worktreeCwd ?? process.cwd() fallback, or the setActiveWorktree wiring would ship undetected.
中文说明
[Suggestion] 三条新行为路径的测试断言为零——删除其中任何一条的变异不会使任何测试变红：(1) 通过 sessionId 的按会话 resolveSettingsCwd 路径；(2) enter-worktree.ts:214 的 setActiveWorktree 调用；(3) session/cd 处理器在 acpAgent.ts:9054-9056 的 worktreeCwd 赋值。— 具体代价：会话查找、worktreeCwd ?? process.cwd() 回退或 setActiveWorktree 接线中的 bug 将不被检测地发布。
— qwen3.8-max-preview via Qwen Code /review
…, guard dangling worktree - resolveSettingsCwd: read getActiveWorktree() from session.getConfig() (per-session object), not agent-level this.config which is a different instance - GET /workspace/settings: use resolveEffectiveWorkspace (was hardcoded boundWorkspace, causing read/write split with POST) - findEffectiveWorkspace: add existsSync guard via injectable pathExists param (prevents realpath crash on deleted worktree) QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
— qwen3.8-max-preview via Qwen Code /review
| if (this.config.getActiveWorktree?.() === worktreePath) { | ||
| this.config.setActiveWorktree?.(null); | ||
| } |
There was a problem hiding this comment.
[Critical] The branchPreserved early-return path (line ~349–362) removes the worktree directory via service.removeUserWorktree() but returns before reaching this new setActiveWorktree(null) call, leaving activeWorktreePath pointing at a deleted directory. — Failure scenario: user enters a worktree via enter_worktree (sets activeWorktreePath), then calls exit_worktree with action: 'remove'. The directory is removed, but git refuses the branch delete (branchPreserved === true). The code enters the branchPreserved block and returns at line ~360. This clearing code is never reached. getActiveWorktree() now returns a path to a non-existent directory. Downstream, resolveSettingsCwd at acpAgent.ts:7262 uses this stale path to resolve workspace settings against a deleted directory.
| if (this.config.getActiveWorktree?.() === worktreePath) { | |
| this.config.setActiveWorktree?.(null); | |
| } | |
| if (this.config.getActiveWorktree?.() === worktreePath) { | |
| this.config.setActiveWorktree?.(null); | |
| } |
Move this clearing check before the branchPreserved early return, or duplicate it inside that block.
中文说明
[Critical] branchPreserved 提前返回路径（约第 349–362 行）通过 service.removeUserWorktree() 删除了 worktree 目录，但在到达此新增的 setActiveWorktree(null) 调用之前就已返回，导致 activeWorktreePath 仍指向已删除的目录。— 失败场景：用户通过 enter_worktree 进入 worktree（设置 activeWorktreePath），然后调用 exit_worktree（action: 'remove'）。目录被删除，但 git 拒绝删除分支（branchPreserved === true）。代码进入 branchPreserved 分支并在约第 360 行返回。此清理代码永远不会被执行。getActiveWorktree() 现在返回一个不存在目录的路径。下游 resolveSettingsCwd（acpAgent.ts:7262）使用此过期路径解析工作区设置。
修复：将此清理检查移到 branchPreserved 提前返回之前，或在该分支内复制一份。
— qwen3.8-max-preview via Qwen Code /review
| const configWt = session.getConfig().getActiveWorktree?.(); | ||
| return session.worktreeCwd ?? configWt ?? process.cwd(); |
There was a problem hiding this comment.
[Critical] session.worktreeCwd is never cleared after exit_worktree removes the worktree directory. The ?? short-circuits on the non-null string, so the correctly-cleared configWt (null after exit_worktree) is never consulted. — Failure scenario: session created in worktree → worktreeCwd = '/wt/A'. Model calls exit_worktree action='remove' → directory deleted, config.setActiveWorktree(null) called, but session.worktreeCwd untouched (core tool has no access to the ACP Session object — grep confirms worktreeCwd = null is never assigned anywhere). Subsequent qwen/settings/* with sessionId returns the deleted path; loadSettings finds no settings file and returns defaults instead of the project root's settings.
Guard the per-session return with an existence check, mirroring the pathExists guard that findEffectiveWorkspace already applies:
| const configWt = session.getConfig().getActiveWorktree?.(); | |
| return session.worktreeCwd ?? configWt ?? process.cwd(); | |
| const configWt = session.getConfig().getActiveWorktree?.(); | |
| if (session.worktreeCwd && existsSync(session.worktreeCwd)) { | |
| return session.worktreeCwd; | |
| } | |
| return configWt ?? process.cwd(); |
中文说明
[Critical] session.worktreeCwd 在 exit_worktree 删除 worktree 目录后从未被清除。?? 在非 null 字符串处短路，因此被正确清除的 configWt（exit_worktree 后为 null）永远不会被查询。— 失败场景：会话在 worktree 中创建 → worktreeCwd = '/wt/A'。模型调用 exit_worktree action='remove' → 目录被删除，config.setActiveWorktree(null) 被调用，但 session.worktreeCwd 未被触及（核心工具无法访问 ACP Session 对象——grep 确认 worktreeCwd = null 在任何地方都没有被赋值）。后续带 sessionId 的 qwen/settings/* 返回已删除的路径；loadSettings 找不到设置文件，返回默认值而非项目根目录的设置。
修复：用存在性检查保护按会话返回，模仿 findEffectiveWorkspace 已有的 pathExists 守卫。
— qwen3.8-max-preview via Qwen Code /review
| for (const session of this.sessions.values()) { | ||
| const wt = session.getConfig().getActiveWorktree?.(); | ||
| if (wt) return wt; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The no-sessionId fallback loop checks only Config.getActiveWorktree() (set by the enter-worktree tool) but not Session.worktreeCwd (set by this PR's createAndStoreSession and session/cd paths), making it inconsistent with the per-session branch directly above (session.worktreeCwd ?? configWt ?? process.cwd()). — Concrete cost: session A created targeting worktree /wt/A → worktreeCwd = '/wt/A'. Session B created as regular → defaultSettingsCwd = null. Neither used enter-worktree, so getActiveWorktree() returns null for all. A call without sessionId falls through to process.cwd() even though session A has worktreeCwd = '/wt/A'.
| for (const session of this.sessions.values()) { | |
| const wt = session.getConfig().getActiveWorktree?.(); | |
| if (wt) return wt; | |
| } | |
| for (const session of this.sessions.values()) { | |
| const wt = | |
| session.worktreeCwd ?? session.getConfig().getActiveWorktree?.(); | |
| if (wt) return wt; | |
| } |
中文说明
[Suggestion] 无 sessionId 的回退循环仅检查 Config.getActiveWorktree()（由 enter-worktree 工具设置），而不检查 Session.worktreeCwd（由本 PR 的 createAndStoreSession 和 session/cd 路径设置），使其与上方的按会话分支（session.worktreeCwd ?? configWt ?? process.cwd()）不一致。— 具体代价：会话 A 创建时指向 worktree /wt/A → worktreeCwd = '/wt/A'。会话 B 作为常规会话创建 → defaultSettingsCwd = null。两者都未使用 enter-worktree，所以所有 getActiveWorktree() 返回 null。不带 sessionId 的调用回退到 process.cwd()，即使会话 A 的 worktreeCwd = '/wt/A'。
— qwen3.8-max-preview via Qwen Code /review
| case 'qwen/settings/getMemory': { | ||
| const settings = loadSettings(cwd); | ||
| const settings = loadSettings(settingsCwd); |
There was a problem hiding this comment.
[Suggestion] The cwd→settingsCwd switch spans ~13 settings/permissions handlers, but the only worktree-resolution test exercises qwen/settings/getCore. A regression reverting settingsCwd→cwd in any sibling handler (e.g. setMcpServer, permissions/setRules, getMemory) stays green. — Concrete cost: a regression writing MCP server config or permission rules to the project root instead of the active worktree would ship undetected — the exact bug (#8138) this PR fixes, but on a sibling handler.
Parametrize one of the new worktree tests to call a representative mutating handler (e.g. qwen/settings/setMcpServer) after createAndStoreSession sets a worktree cwd, and assert the write targets the worktree directory.
中文说明
[Suggestion] cwd→settingsCwd 切换涉及约 13 个设置/权限处理器，但唯一的 worktree 解析测试仅验证 qwen/settings/getCore。在任何兄弟处理器中将 settingsCwd 回退为 cwd 的回归仍会通过测试。— 具体代价：将 MCP 服务器配置或权限规则写入项目根目录而非活跃 worktree 的回归将不被检测到——正是本 PR 修复的 bug (#8138)，但发生在兄弟处理器上。
— qwen3.8-max-preview via Qwen Code /review
| const sessionId = params['sessionId']; | ||
| if (typeof sessionId === 'string' && sessionId.length > 0) { | ||
| const session = this.sessions.get(sessionId); |
There was a problem hiding this comment.
[Suggestion] resolveSettingsCwd's per-session branches (explicit sessionId → session.worktreeCwd / getActiveWorktree()) and the session/cd setter are untested; the new tests only hit the agent-level defaultSettingsCwd fallback. — Concrete cost: a bug in the recommended multi-session path (e.g. returning the wrong session's worktree, or the ?? ordering) would ship with zero test signal. The session/cd handler that also sets worktreeCwd/defaultSettingsCwd (~line 9054) is likewise uncovered.
Add a test that creates two worktree sessions and calls qwen/settings/getCore with { sessionId: <second> }, asserting loadSettings receives the second session's worktreeCwd.
中文说明
[Suggestion] resolveSettingsCwd 的按会话分支（显式 sessionId → session.worktreeCwd / getActiveWorktree()）和 session/cd 设置器未被测试；新测试仅验证了代理级 defaultSettingsCwd 回退。— 具体代价：推荐的多会话路径中的 bug（例如返回错误会话的 worktree，或 ?? 排序错误）将在零测试信号下发布。
— qwen3.8-max-preview via Qwen Code /review
| ); | ||
| } | ||
|
|
||
| this.config.setActiveWorktree?.(result.worktree.path); |
There was a problem hiding this comment.
[Suggestion] The Config active-worktree lifecycle (enter sets it, exit clears it) has no asserting test. enter-worktree.test.ts contains no reference to setActiveWorktree; exit-worktree.test.ts stubs getActiveWorktree: () => null (lines 34, 192), so the clearing guard is never taken. — Concrete cost: dropping the enter-set call, or breaking the exit clear (leaving a stale active worktree that resolveSettingsCwd would resolve settings against after exit), would ship undetected.
In enter-worktree.test.ts, spy setActiveWorktree and assert it is called with the created worktree path; in exit-worktree.test.ts, stub getActiveWorktree to return the live worktree path and assert setActiveWorktree(null) is called.
中文说明
[Suggestion] Config 活跃 worktree 生命周期（enter 设置，exit 清除）没有断言测试。enter-worktree.test.ts 不包含对 setActiveWorktree 的引用；exit-worktree.test.ts 将 getActiveWorktree 存根为 () => null（第 34、192 行），因此清理守卫永远不会被触发。— 具体代价：删除 enter 设置调用或破坏 exit 清除（留下一个过期的活跃 worktree，resolveSettingsCwd 在 exit 后仍会据此解析设置）将不被检测到。
— qwen3.8-max-preview via Qwen Code /review
| const effectiveWorkspace = | ||
| resolveEffectiveWorkspace?.() ?? boundWorkspace; |
There was a problem hiding this comment.
[Suggestion] POST persist-error log at line ~458 still interpolates boundWorkspace instead of effectiveWorkspace. Before this diff both were the same value; now they diverge when a worktree session is active. — Concrete cost: a worktree session is active (effectiveWorkspace = /repo/.qwen/worktrees/feat), a settings persist fails (ENOSPC, EACCES). The catch block logs workspace=/repo instead of workspace=/repo/.qwen/worktrees/feat. An engineer debugging the failure inspects the wrong directory.
| const effectiveWorkspace = | |
| resolveEffectiveWorkspace?.() ?? boundWorkspace; | |
| const effectiveWorkspace = | |
| resolveEffectiveWorkspace?.() ?? boundWorkspace; |
(Also change workspace=${boundWorkspace} to workspace=${effectiveWorkspace} in the writeStderrLine template at line ~458.)
中文说明
[Suggestion] 约第 458 行的 POST 持久化错误日志仍然插入 boundWorkspace 而非 effectiveWorkspace。在此变更之前两者是同一个值；现在当 worktree 会话活跃时它们会分歧。— 具体代价：worktree 会话活跃时设置持久化失败，日志记录错误的目录路径，延迟问题诊断。
— qwen3.8-max-preview via Qwen Code /review
| resolveEffectiveWorkspace: () => | ||
| findEffectiveWorkspace(primaryBridge, primaryBoundWorkspace), |
There was a problem hiding this comment.
[Suggestion] The settings route resolves its target workspace from global live-session state (first worktree session in Map insertion order wins), with no pinning to the requesting client/session. — Concrete cost: with two concurrent worktree sessions (feat-a, feat-b) under the same daemon, every client's GET and POST /workspace/settings is silently redirected to whichever worktree appears first — not the worktree the client is working in. The resolution is also recomputed independently per request, so a GET can read feat-a's settings, then feat-a closes, and the follow-up POST writes to feat-b. The ACP agent path has per-session resolution via sessionId; the serve routes lack an equivalent.
Scope the resolution to the requesting client's session (pass clientId into resolveEffectiveWorkspace and prefer that session's worktree.path), or pin the effective workspace for the lifetime of a client connection.
中文说明
[Suggestion] 设置路由从全局活跃会话状态解析目标工作区（Map 插入顺序中第一个 worktree 会话胜出），不绑定到请求客户端/会话。— 具体代价：同一守护进程下有两个并发 worktree 会话时，每个客户端的设置读写被静默重定向到第一个 worktree，而非客户端实际工作的 worktree。
— qwen3.8-max-preview via Qwen Code /review
| target === effectiveWorkspace || | ||
| target.startsWith(effectiveWorkspace + path.sep); |
There was a problem hiding this comment.
[Suggestion] The path-escape guard now anchors to the callback-supplied effectiveWorkspace instead of the immutable boundWorkspace, and nothing validates that effectiveWorkspace is contained within boundWorkspace. findEffectiveWorkspace returns relocated?.worktree?.path with only an existsSync check — no containment check. — Concrete cost: if a session's worktree.path is set to a directory outside the daemon's workspace (e.g. via a corrupted sidecar file or a future code path calling setSessionWorktree with an unvalidated path), then effectiveWorkspace becomes that external directory. The boundary check and symlink canonicalization both validate against it, so initWorkspace writes QWEN.md and the settings routes read/write .qwen/settings.json outside the daemon's workspace. Before this diff the anchor was boundWorkspace, set once at daemon startup.
Add a containment guard in findEffectiveWorkspace:
| target === effectiveWorkspace || | |
| target.startsWith(effectiveWorkspace + path.sep); | |
| target === effectiveWorkspace || | |
| target.startsWith(effectiveWorkspace + path.sep); |
(In worktree-workspace.ts, add s.worktree.path.startsWith(boundWorkspace + path.sep) to the find predicate.)
中文说明
[Suggestion] 路径逃逸守卫现在锚定到回调提供的 effectiveWorkspace 而非不可变的 boundWorkspace，且没有验证 effectiveWorkspace 是否包含在 boundWorkspace 内。— 具体代价：如果会话的 worktree.path 被设置为守护进程工作区外的目录，边界检查和符号链接规范化都会针对该外部目录验证，导致 initWorkspace 和设置路由在守护进程工作区外读写文件。
— qwen3.8-max-preview via Qwen Code /review
…r worktreeCwd on cd-to-root Two fixes from PR review:
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Aleks-0
left a comment
Aleks-0
left a comment
There was a problem hiding this comment.
Addressed both findings in ec57679:
R1-1 (desktop/ACP path): settingsCwd now prefers resolveSettingsCwd() over explicit requestedCwd when a worktree is active. Desktop always sends cwd=projectRoot, but resolveSettingsCwd finds the worktree via config.getActiveWorktree() and overrides it.
R1-2 (session/cd guard): session/cd back to project root now clears worktreeCwd = null instead of setting it to process.cwd(), preventing poisoning of the no-sessionId fallback loop.
两个发现已在 ec57679 中修复：
R1-1 (desktop/ACP 路径): settingsCwd 现在在 worktree 活跃时优先使用 resolveSettingsCwd() 而非显式的 requestedCwd。Desktop 总是发送 cwd=projectRoot，但 resolveSettingsCwd 通过 config.getActiveWorktree() 找到 worktree 并覆盖它。
R1-2 (session/cd 守卫): session/cd 返回项目根目录时现在清除 worktreeCwd = null，而非设为 process.cwd()，避免污染无 sessionId 的回退循环。
QwenCode QwenLLM
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
— qwen3.8-max-preview via Qwen Code /review
| * Resolve the cwd for `qwen/settings/*` handlers with per-session | ||
| * worktree awareness. Priority: explicit `cwd` param (handled by | ||
| * caller) > session.worktreeCwd > session config's activeWorktree > | ||
| * agent-level defaultSettingsCwd > process.cwd(). |
There was a problem hiding this comment.
[Suggestion] Doc comment states the wrong priority order — says "explicit cwd param (handled by caller) > session.worktreeCwd > …" but the caller at line 7375 intentionally gives worktree resolution priority over explicit cwd.
A future developer reading the comment would build a client expecting cwd to override worktree resolution — it will not. Update the comment to match the implemented order: session.worktreeCwd > configWt > defaultSettingsCwd > explicit cwd > process.cwd().
| * Resolve the cwd for `qwen/settings/*` handlers with per-session | |
| * worktree awareness. Priority: explicit `cwd` param (handled by | |
| * caller) > session.worktreeCwd > session config's activeWorktree > | |
| * agent-level defaultSettingsCwd > process.cwd(). | |
| * Resolve the cwd for `qwen/settings/*` handlers with per-session | |
| * worktree awareness. Priority: session.worktreeCwd > session | |
| * config's activeWorktree > agent-level defaultSettingsCwd > | |
| * explicit `cwd` param > process.cwd(). |
中文说明
[Suggestion] 文档注释的优先级顺序有误——写的是"显式 cwd 参数（由调用者处理）> session.worktreeCwd > …"，但第 7375 行的调用者故意让 worktree 解析优先于显式 cwd。
未来阅读此注释的开发者会以为 cwd 能覆盖 worktree 解析——实际不会。请更新注释以匹配实现的顺序。
— qwen3.8-max-preview via Qwen Code /review
| const session = this.sessions.get(sessionId); | ||
| if (session) { |
There was a problem hiding this comment.
[Suggestion] When sessionId is provided but the session is not found (closed or stale ID), resolveSettingsCwd silently falls through to the multi-session fallback loop, returning another session's worktree path.
Failure scenario: sessions A (/wt/a) and B (closed). A call with { sessionId: 'B' } skips the if (session) block, the fallback loop finds session A's worktreeCwd, and the setting lands in /wt/a — the wrong worktree — with no error. The defaultSettingsCwd JSDoc says passing sessionId avoids "the last-wins ambiguity", but the fallthrough reintroduces it for stale IDs.
Consider returning process.cwd() (or throwing) when sessionId is a non-empty string but the session is not found, instead of falling through to the loop.
中文说明
[Suggestion] 当提供了 sessionId 但会话未找到（已关闭或过期 ID）时，resolveSettingsCwd 会静默回退到多会话循环，返回另一个会话的 worktree 路径。
失败场景：会话 A（/wt/a）和 B（已关闭）。带 { sessionId: 'B' } 的调用跳过 if (session) 块，回退循环找到会话 A 的 worktreeCwd，设置写入 /wt/a——错误的 worktree——且无错误提示。
— qwen3.8-max-preview via Qwen Code /review
| const targetDir = config.getTargetDir(); | ||
| if (targetDir !== process.cwd()) { | ||
| session.worktreeCwd = targetDir; | ||
| this.defaultSettingsCwd = targetDir; |
There was a problem hiding this comment.
[Suggestion] The targetDir !== process.cwd() heuristic misclassifies every serve/daemon session as a worktree session, because the daemon's process.cwd() (e.g. /home/user) always differs from the workspace targetDir (e.g. /repo/project).
Failure scenario: serve session created → worktreeCwd = '/repo/project' (the plain workspace). Agent runs enter_worktree → activeWorktreePath = '/repo/project/.qwen/worktrees/feat'. A qwen/settings/getCore call with sessionId returns session.worktreeCwd (/repo/project) immediately — configWt (the actual worktree) is never checked. Settings resolve against the project root instead of the worktree.
Consider a more precise worktree detection — e.g. only set worktreeCwd when the session is explicitly created with a worktree flag, or check whether targetDir is inside a .qwen/worktrees/ subtree.
中文说明
[Suggestion] targetDir !== process.cwd() 启发式会将每个 serve/daemon 会话误判为 worktree 会话，因为守护进程的 process.cwd()（如 /home/user）总是与工作区 targetDir（如 /repo/project）不同。
失败场景：创建 serve 会话 → worktreeCwd = '/repo/project'（普通工作区）。代理运行 enter_worktree → activeWorktreePath = '/repo/project/.qwen/worktrees/feat'。带 sessionId 的 qwen/settings/getCore 调用立即返回 session.worktreeCwd（/repo/project）——configWt（实际 worktree）永远不会被检查。设置解析到项目根目录而非 worktree。
— qwen3.8-max-preview via Qwen Code /review
| for (const session of this.sessions.values()) { | ||
| if (session.worktreeCwd && existsSync(session.worktreeCwd)) { | ||
| return session.worktreeCwd; | ||
| } | ||
| const wt = session.getConfig().getActiveWorktree?.(); | ||
| if (wt && existsSync(wt)) return wt; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The no-sessionId fallback loop interleaves worktreeCwd and activeWorktree checks per session, so an earlier session's config-level activeWorktree wins over a later session's explicit worktreeCwd (set by session/cd), inverting the documented priority across sessions.
Failure scenario: session 1 (created first) has no worktreeCwd but activeWorktree = '/config/wt'; session 2 has worktreeCwd = '/explicit/wt' from session/cd. The loop hits session 1 first, finds activeWorktree, and returns it — session 2's deliberate cd target is silently overridden.
| for (const session of this.sessions.values()) { | |
| if (session.worktreeCwd && existsSync(session.worktreeCwd)) { | |
| return session.worktreeCwd; | |
| } | |
| const wt = session.getConfig().getActiveWorktree?.(); | |
| if (wt && existsSync(wt)) return wt; | |
| } | |
| for (const session of this.sessions.values()) { | |
| if (session.worktreeCwd && existsSync(session.worktreeCwd)) { | |
| return session.worktreeCwd; | |
| } | |
| } | |
| for (const session of this.sessions.values()) { | |
| const wt = session.getConfig().getActiveWorktree?.(); | |
| if (wt && existsSync(wt)) return wt; | |
| } |
中文说明
[Suggestion] 无 sessionId 的回退循环在每个会话内交替检查 worktreeCwd 和 activeWorktree，因此较早会话的 config 级 activeWorktree 会胜过较晚会话的显式 worktreeCwd（由 session/cd 设置），跨会话地反转了文档化的优先级。
建议拆分为两趟循环：先扫描所有会话的 worktreeCwd，再扫描 activeWorktree。
— qwen3.8-max-preview via Qwen Code /review
| path.normalize(s.worktree.path).startsWith(normalizedBound + path.sep) && | ||
| pathExists(s.worktree.path), | ||
| ); | ||
| return relocated?.worktree?.path ?? boundWorkspace; |
There was a problem hiding this comment.
[Suggestion] findEffectiveWorkspace returns the raw s.worktree.path while the boundary check one line above uses path.normalize(s.worktree.path), so the returned value can disagree with the normalized form that downstream consumers compare against.
If a worktree path contains lexically resolvable segments (e.g. /repo/.qwen/worktrees/../worktrees/feat), the boundary check passes but the downstream target.startsWith(effectiveWorkspace + path.sep) in initWorkspace fails, throwing a false WorkspaceInitPathEscapeError.
| return relocated?.worktree?.path ?? boundWorkspace; | |
| return relocated ? path.normalize(relocated.worktree.path) : boundWorkspace; |
中文说明
[Suggestion] findEffectiveWorkspace 返回原始的 s.worktree.path，而上方的边界检查使用 path.normalize(s.worktree.path)，因此返回值可能与下游消费者比较的规范化形式不一致。
如果 worktree 路径包含可词法解析的段（如 ..），边界检查通过但下游的 target.startsWith(effectiveWorkspace + path.sep) 失败，抛出错误的 WorkspaceInitPathEscapeError。
— qwen3.8-max-preview via Qwen Code /review
…onId, loop split, path normalization
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
— qwen3.8-max-preview via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| /** Worktree root path when the session operates inside a git worktree */ | ||
| worktreeRootPath?: string; |
There was a problem hiding this comment.
[Suggestion] worktreeRootPath is declared and read but never set by any caller — a dead switch in the desktop path. Neither buildBackendHostRuntimeContext implementation (handlers/utils.ts:20, SessionManager.ts:288) populates it. — Concrete cost: a reader of qwenSettingsCwd sees a worktree-aware branch and may believe the desktop half of #8138 is fixed when it is not; the dead field and its 13 call-site reads add maintenance surface until the follow-up lands or is abandoned.
| /** Worktree root path when the session operates inside a git worktree */ | |
| worktreeRootPath?: string; | |
| // TODO(#8138): add worktreeRootPath?: string to BackendHostRuntimeContext | |
| // and populate it in buildBackendHostRuntimeContext before reading it here. |
中文说明
[Suggestion] worktreeRootPath 已声明并被读取，但没有任何调用方设置它——桌面路径中的死开关。两个 buildBackendHostRuntimeContext 实现（handlers/utils.ts:20、SessionManager.ts:288）均未填充该字段。— 具体代价：qwenSettingsCwd 的读者会看到一个 worktree 感知分支，可能误以为 #8138 的桌面部分已修复，但实际上并未修复；该死字段及其 13 个调用点读取增加了维护面，直到后续工作合入或被放弃。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| if (canonicalPath !== process.cwd()) { | ||
| session.worktreeCwd = canonicalPath; | ||
| this.defaultSettingsCwd = canonicalPath; |
There was a problem hiding this comment.
[Suggestion] The session/cd handler's new worktreeCwd/defaultSettingsCwd assignment has no test; a mutation removing these three lines would not turn any test red. — Failure scenario: a user runs session/cd into a worktree directory, then calls qwen/settings/getCore. Without the assignment, resolveSettingsCwd finds no session with worktreeCwd set and falls back to process.cwd() — settings read from the project root instead of the worktree.
中文说明
[Suggestion] session/cd 处理器新增的 worktreeCwd/defaultSettingsCwd 赋值没有测试；删除这三行的突变不会让任何测试变红。— 失败场景：用户运行 session/cd 进入 worktree 目录，然后调用 qwen/settings/getCore。没有该赋值，resolveSettingsCwd 找不到设置了 worktreeCwd 的会话，回退到 process.cwd()——设置从项目根目录而非 worktree 读取。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const getWorkspace = resolveEffectiveWorkspace?.() ?? boundWorkspace; | ||
| const response = buildSettingsResponse( | ||
| boundWorkspace, | ||
| getWorkspace, |
There was a problem hiding this comment.
[Suggestion] The workspace-settings GET and POST routes' resolveEffectiveWorkspace integration is untested; existing workspace-settings.test.ts never passes resolveEffectiveWorkspace in deps. — Failure scenario: if the wiring in server.ts is wrong (e.g., resolveEffectiveWorkspace not passed, or findEffectiveWorkspace called with the wrong bridge), settings reads and writes silently fall back to boundWorkspace (the project root). A mutation replacing resolveEffectiveWorkspace?.() ?? boundWorkspace with just boundWorkspace would not turn any test red.
中文说明
[Suggestion] workspace-settings GET 和 POST 路由的 resolveEffectiveWorkspace 集成未被测试；现有的 workspace-settings.test.ts 从未在 deps 中传递 resolveEffectiveWorkspace。— 失败场景：如果 server.ts 中的接线错误（例如未传递 resolveEffectiveWorkspace，或 findEffectiveWorkspace 使用了错误的 bridge），设置读写会静默回退到 boundWorkspace（项目根目录）。将 resolveEffectiveWorkspace?.() ?? boundWorkspace 替换为仅 boundWorkspace 的突变不会让任何测试变红。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| it('qwen/settings handlers resolve against worktree cwd set by createAndStoreSession', async () => { | ||
| const WORKTREE_DIR = os.tmpdir(); | ||
| const innerConfig = await setupSessionMocks('wt-settings-session'); |
There was a problem hiding this comment.
[Suggestion] No test replays the issue's actual end-to-end shape: enter_worktree tool called mid-session → workspace-scoped setting change → setting lands in worktree. The two halves are tested independently (enter-worktree.test.ts asserts setActiveWorktree is called; acpAgent.test.ts mocks getActiveWorktree to return a preset path) but the Config identity wiring is never exercised. — Failure scenario: a refactor that breaks Config identity (tools receiving a cloned Config, or Session.getConfig() returning a wrapper) would pass both test halves independently while silently re-breaking issue #8138.
中文说明
[Suggestion] 没有测试复现 issue 的实际端到端形态：会话中调用 enter_worktree 工具 → 更改工作区范围设置 → 设置写入 worktree。两半被独立测试（enter-worktree.test.ts 断言 setActiveWorktree 被调用；acpAgent.test.ts mock getActiveWorktree 返回预设路径），但 Config 身份接线从未被验证。— 失败场景：一个破坏 Config 身份的重构（工具收到克隆的 Config，或 Session.getConfig() 返回包装器）会让两半测试独立通过，同时静默地重新破坏 issue #8138。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| "files": [], | ||
| "references": [ | ||
| { "path": "packages/acp-bridge" }, |
There was a problem hiding this comment.
[Suggestion] Seven of the 22 referenced packages lack composite: true, so tsc --build from the root — the standard way to use a solution-style tsconfig — fails with TS6306 errors. No existing CI or npm script is broken today (root typecheck and build delegate to workspaces), but the solution-style structure is inert for its primary purpose and will error on first use. Affected: packages/desktop, packages/chrome-extension, packages/mobile-mcp, packages/sdk-typescript, packages/vscode-ide-companion, packages/web-shell, packages/webui.
中文说明
[Suggestion] 22 个被引用的包中有 7 个缺少 composite: true，因此从根目录运行 tsc --build（使用 solution-style tsconfig 的标准方式）会以 TS6306 错误失败。当前没有 CI 或 npm 脚本受影响（根目录的 typecheck 和 build 委托给各工作区），但 solution-style 结构对其主要用途是无效的，首次使用即会报错。受影响：packages/desktop、packages/chrome-extension、packages/mobile-mcp、packages/sdk-typescript、packages/vscode-ide-companion、packages/web-shell、packages/webui。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
…overage, tsconfig cleanup
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] worktree-workspace.ts findEffectiveWorkspace (existing comment 3695238316): cannot tell — whether the production bridge populates BridgeSessionSummary.worktree for enter_worktree sessions (and thus whether the boundWorkspace->effectiveWorkspace substitution is a no-op in production) lives outside this diff and could not be verified from the code. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未决，请确认：[Critical] worktree-workspace.ts findEffectiveWorkspace (existing comment 3695238316): cannot tell — whether the production bridge populates BridgeSessionSummary.worktree for enter_worktree sessions (and thus whether the boundWorkspace->effectiveWorkspace substitution is a no-op in production) lives outside this diff and could not be verified from the code. 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
| const newMerged = this.settings.merged; | ||
|
|
||
| const envResult = reloadEnvironment(newMerged, cwd); | ||
| const envResult = reloadEnvironment(newMerged, settingsCwd); |
There was a problem hiding this comment.
[Suggestion] workspaceReload now reads its two halves from different directories. this.settings.reloadScopeFromDisk(...) re-reads from the directory this.settings was constructed with, while reloadEnvironment(newMerged, settingsCwd) scans settingsCwd. Before this PR both used cwd; the PR migrated the env half to settingsCwd but not the settings half. — Failure scenario: a session created in a worktree whose this.settings was last loaded from the project root (e.g. workspaceReload is the first settings-related ext method, before any qwen/settings/* handler re-points this.settings via loadSettings(settingsCwd)). The reload then merges the project root's workspace settings with the worktree's .env — e.g. the worktree's DATABASE_URL lands on top of the root's feature flags — so the returned snapshot mixes two directories. Suggested fix (spans the top of the case, so not a one-click suggestion):
case SERVE_CONTROL_EXT_METHODS.workspaceReload: {
this.settings = loadSettings(settingsCwd); // re-point before reloading
const oldMerged = structuredClone(this.settings.merged);
this.settings.reloadScopeFromDisk(SettingScope.User);
this.settings.reloadScopeFromDisk(SettingScope.Workspace);
const newMerged = this.settings.merged;
const envResult = reloadEnvironment(newMerged, settingsCwd);中文说明
[Suggestion] workspaceReload 现在从两个不同的目录读取其两半。this.settings.reloadScopeFromDisk(...) 从 this.settings 构造时所用的目录重新读取，而 reloadEnvironment(newMerged, settingsCwd) 扫描 settingsCwd。本 PR 之前两者都用 cwd；PR 将 env 一半迁移到了 settingsCwd，却没有迁移 settings 一半。— 失败场景：一个在 worktree 中创建的会话，其 this.settings 最后一次是从项目根目录加载的（例如 workspaceReload 是第一个与设置相关的 ext 方法，在任何 qwen/settings/* 处理器通过 loadSettings(settingsCwd) 重新指向 this.settings 之前）。此时 reload 会把项目根目录的工作区设置与 worktree 的 .env 合并——例如 worktree 的 DATABASE_URL 叠加到根目录的特性开关之上——于是返回的快照混合了两个目录。修复建议（跨 case 顶部，故非一键 suggestion）：在 case 顶部、oldMerged 之前加入 this.settings = loadSettings(settingsCwd);。
— qwen3.8-max-preview via Qwen Code /review
| const { mockExistsSync } = vi.hoisted(() => ({ | ||
| mockExistsSync: vi.fn().mockReturnValue(true), | ||
| })); |
There was a problem hiding this comment.
[Suggestion] The existsSync stub is installed file-wide (via vi.mock('node:fs')) and defaults to true for the entire 17,507-line test file, with no per-test reset. Two consequences: (1) every existsSync call in every unrelated test now reports every path as existing, so a regression that breaks missing-file handling would pass silently here; (2) the two worktree tests that override the mock inline (~9236, ~9268) restore it only as their last statement — not in try/finally or afterEach — so if an assertion throws mid-block, the override leaks into every later test in the file (vitest runs in file order), producing a cascade of misleading failures or masking a real regression. The diff already uses try/finally for cleanup in the session/cd test (~4068), so the safer pattern is established. Suggested direction:
// default to real behavior; stub true/false only within the tests that need it
const { mockExistsSync } = vi.hoisted(() => ({
mockExistsSync: vi.fn().mockImplementation((...a) => actualExistsSync(...a)),
}));
// plus afterEach(() => mockExistsSync.mockReset()), or try/finally around inline overrides中文说明
[Suggestion] existsSync 桩通过 vi.mock('node:fs') 被安装为全文件生效，并在整个 17,507 行的测试文件中默认返回 true，且没有按测试重置。两个后果：(1) 每个无关测试中的 existsSync 调用现在都报告所有路径都存在，因此破坏"文件缺失处理"的回归在此处会静默通过；(2) 两个在内联覆盖该 mock 的 worktree 测试（约 9236、约 9268）只在其最后一条语句恢复它——而非在 try/finally 或 afterEach 中——所以若某个断言在块中途抛出，该覆盖会泄漏到文件中所有后续测试（vitest 按文件顺序运行），产生一连串误导性失败或掩盖真实回归。本 diff 在 session/cd 测试（约 4068）中已使用 try/finally 做清理，因此更安全的模式已经确立。修复方向：默认调用真实实现，仅在需要的测试内 stub true/false，并配合 afterEach 重置或 try/finally。
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /resolve |
…overage, tsconfig cleanup
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
[Critical] R2-17: Resume gap — activeWorktreePath is only ever populated by a live enter_worktree invocation (enter-worktree.ts:214); #restoreWorktreeOnResume (acpAgent.ts:4953, 5046) restores only the context notice and discards restored.session, never calling config.setActiveWorktree(...) nor setting session.worktreeCwd. After exit_worktree(action='keep') (which deliberately preserves the sidecar "so --resume can restore it") and a session resume via the direct ACP paths — SDK ACP transport and desktop resume are shipped flows; the daemon REST restore routes compensate at the daemon layer, but session/load/session/resume in the ACP-HTTP dispatch contain zero worktree/sidecar logic — every qwen/settings/* call resolves to process.cwd() (project root) while the restored notice instructs the model to operate in the worktree. Persistent, with no self-heal: re-running enter_worktree with the same slug fails with "Worktree already exists" (gitWorktreeService.ts:442). (Could not be anchored inline: the only added line in enter-worktree.ts:214 already carries an unrelated existing comment.) Fix: in #restoreWorktreeOnResume, when restored.session is non-null, repopulate the state this PR keys on — config.setActiveWorktree(restored.session.worktreePath) (or set session.worktreeCwd) alongside pendingWorktreeNotice.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
[Critical] R2-17: Resume gap — activeWorktreePath is only ever populated by a live enter_worktree invocation (enter-worktree.ts:214); #restoreWorktreeOnResume (acpAgent.ts:4953, 5046) restores only the context notice and discards restored.session, never calling config.setActiveWorktree(...) nor setting session.worktreeCwd. After exit_worktree(action='keep') (which deliberately preserves the sidecar "so --resume can restore it") and a session resume via the direct ACP paths — SDK ACP transport and desktop resume are shipped flows; the daemon REST restore routes compensate at the daemon layer, but session/load/session/resume in the ACP-HTTP dispatch contain zero worktree/sidecar logic — every qwen/settings/* call resolves to process.cwd() (project root) while the restored notice instructs the model to operate in the worktree. Persistent, with no self-heal: re-running enter_worktree with the same slug fails with "Worktree already exists" (gitWorktreeService.ts:442). (Could not be anchored inline: the only added line in enter-worktree.ts:214 already carries an unrelated existing comment.) Fix: in #restoreWorktreeOnResume, when restored.session is non-null, repopulate the state this PR keys on — config.setActiveWorktree(restored.session.worktreePath) (or set session.worktreeCwd) alongside pendingWorktreeNotice.
— qwen3.8-max via Qwen Code /review (v0.21.7)
| this.settings = loadSettings(settingsCwd); | ||
| const oldMerged = structuredClone(this.settings.merged); |
There was a problem hiding this comment.
[Critical] R2-1: The newly inserted this.settings = loadSettings(settingsCwd) runs BEFORE the oldMerged snapshot. loadSettings is uncached (settings.ts re-reads every scope from disk on each call), so diffSettingsKeys(oldMerged, newMerged) compares two consecutive fresh-from-disk reads and is always empty — every changed-gated reaction below (switchModel, reloadModelProvidersConfig/refreshAuth, setApprovalMode incl. plan-guard clearing, setDisabledTools, and the changedKeys broadcast in settings_reloaded) is dead. Pre-change, oldMerged came from the cached (possibly stale) this.settings, so external edits were detected. Verified by runtime probe: the base flow reports changedKeys: ["tools","$version"], this flow reports [] while the new value is present — flips precisely on the added line. — Failure scenario: user edits settings.json (approval mode / model / providers) → daemon workspace-service.reload() → changedKeys: [] broadcast; live sessions silently keep the stale model/approval mode until restart. The two existing workspaceReload tests mask this because their loadSettings mock returns the same mutable object whose merged only changes when reloadScopeFromDisk runs.
| this.settings = loadSettings(settingsCwd); | |
| const oldMerged = structuredClone(this.settings.merged); | |
| const oldMerged = structuredClone(this.settings.merged); | |
| this.settings = loadSettings(settingsCwd); |
(the subsequent reloadScopeFromDisk calls become redundant once the fresh load is kept)
中文说明
[Critical] R2-1：新插入的 this.settings = loadSettings(settingsCwd) 在 oldMerged 快照之前执行。loadSettings 无缓存（settings.ts 每次调用都从磁盘重读所有 scope），因此 diffSettingsKeys(oldMerged, newMerged) 比较的是两次连续的磁盘新读取，结果恒为空 —— 下方所有由 changed 控制的联动（switchModel、reloadModelProvidersConfig/refreshAuth、setApprovalMode（含 plan 守卫清除）、setDisabledTools，以及 settings_reloaded 事件中的 changedKeys 广播）全部失效。变更前 oldMerged 来自缓存的（可能过期的）this.settings，因此外部编辑能被检测到。已用运行时探针验证：base 流程报告 changedKeys: ["tools","$version"]，本流程报告 []（而新值确实已读入）——翻转点正是新增的这一行。— 失败场景：用户编辑 settings.json（审批模式/模型/providers）→ 守护进程 workspace-service.reload() → 广播 changedKeys: []；运行中的会话静默保留旧的模型/审批模式直到重启。现有两个 workspaceReload 测试掩盖了此问题，因为它们的 loadSettings mock 返回同一个可变对象，其 merged 只在 reloadScopeFromDisk 时才变化。
修复：先快照再替换（见上方 suggestion）；保留新加载的话，后续 reloadScopeFromDisk 调用即为冗余。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const resolvedWt = this.resolveSettingsCwd(params); | ||
| const settingsCwd = | ||
| (resolvedWt !== process.cwd() ? resolvedWt : undefined) || |
There was a problem hiding this comment.
[Critical] R2-2: "Fixes #8138" remains overstated for the desktop client. The desktop Settings panel sends qwen/settings/* through a dedicated session-less shared ACP process: buildSharedAcpProcessKey never matches the session-hosting process (the session agent's key includes envOverrides = { CRAFT_WORKSPACE_PATH } — SessionManager.ts:4909-4912 — while getQwenWorkspaceAcpOptions passes none), so in that process this.sessions is empty and defaultSettingsCwd is null → resolveSettingsCwd returns process.cwd() and this precedence collapses to the requested project root. The R1-1 fix (worktree resolution outranking explicit cwd) only helps processes that host a session. — Failure scenario: the issue's own repro — session runs enter_worktree, user changes the model in the desktop Settings panel → the write lands in <projectRoot>/.qwen/settings.json, not the worktree's; the issue closes unfixed for its primary desktop surface. No test exercises a session-less caller with explicit project-root cwd, which is why the gap ships green. — Fix direction: complete the desktop wiring (populate worktreeRootPath per the TODO(#8138), or make the settings-panel path reuse the session-hosting process via matching envOverrides), or rescope the closing reference to "Partially addresses #8138" with a follow-up.
中文说明
[Critical] R2-2："Fixes #8138" 对桌面客户端仍然言过其实。桌面设置面板通过一个无会话的共享 ACP 进程发送 qwen/settings/*：buildSharedAcpProcessKey 永远不会与承载会话的进程匹配（会话代理的 key 包含 envOverrides = { CRAFT_WORKSPACE_PATH }（SessionManager.ts:4909-4912），而 getQwenWorkspaceAcpOptions 不传任何 envOverrides），因此该进程中 this.sessions 为空、defaultSettingsCwd 为 null → resolveSettingsCwd 返回 process.cwd()，此优先级表达式退回为请求的项目根目录。R1-1 的修复（worktree 解析优先于显式 cwd）只对承载会话的进程生效。— 失败场景：按 issue 自身的复现——会话运行 enter_worktree，用户在桌面设置面板更改模型 → 写入落在 <projectRoot>/.qwen/settings.json 而非 worktree；issue 在其主要桌面场景上未修复即被关闭。没有任何测试覆盖"带显式项目根 cwd 的无会话调用方"，因此该缺口绿灯合入。— 修复方向：补全桌面接线（按 TODO(#8138) 填充 worktreeRootPath，或让设置面板路径通过匹配的 envOverrides 复用承载会话的进程），或将关闭引用调整为 "Partially addresses #8138" 并开后续 issue。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const relocated = sessions.find( | ||
| (s) => | ||
| s.worktree && |
There was a problem hiding this comment.
[Critical] R2-3: This lookup only sees worktrees via BridgeSessionSummary.worktree, which the production bridge populates only at REST worktree-session creation (routes/session.ts:1743 → bridge.ts:3097/5900) and load/resume restore (routes/session.ts:2391 → setSessionWorktree). A worktree entered via the enter_worktree tool mid-session — the trigger issue #8138 explicitly names — never reaches the bridge (the tool only mutates in-child Config state; grep confirms no setSessionWorktree wiring in packages/cli/src/serve), so GET/POST /workspace/settings and POST /workspace/init keep resolving against the project root for that session shape. This resolves the previous round's open "cannot tell" (review 4837609086): the bridge does NOT populate worktree for tool-entered sessions. — Failure scenario: web-shell session bound to the project root; the model runs enter_worktree; the user changes a setting via the web shell → findEffectiveWorkspace sees no s.worktree on any summary → returns boundWorkspace → the write lands in <projectRoot>/.qwen/settings.json while the session works in the worktree. — Fix direction: have the tool-driven path notify the bridge (e.g. call setSessionWorktree after enter_worktree / worktree-targeted session/cd), or explicitly document that the serve HTTP surface only follows REST-created worktree sessions and file the tool-driven gap as a follow-up; add a test covering the tool-entered shape.
中文说明
[Critical] R2-3：此查找只能通过 BridgeSessionSummary.worktree 看到 worktree，而生产 bridge 只在 REST 创建 worktree 会话（routes/session.ts:1743 → bridge.ts:3097/5900）与加载/恢复（routes/session.ts:2391 → setSessionWorktree）时填充该字段。会话中途通过 enter_worktree 工具进入的 worktree——issue #8138 明确点名的触发方式——永远不会到达 bridge（该工具只修改子进程内的 Config 状态；grep 确认 packages/cli/src/serve 中没有 setSessionWorktree 接线），因此对这类会话形态，GET/POST /workspace/settings 与 POST /workspace/init 仍解析到项目根目录。这解决了上一轮未决的 "cannot tell"（review 4837609086）：bridge 确实不会为工具进入的会话填充 worktree。— 失败场景：绑定项目根的 web-shell 会话；模型运行 enter_worktree；用户经 web shell 更改设置 → findEffectiveWorkspace 在任何 summary 上看不到 s.worktree → 返回 boundWorkspace → 写入落在 <projectRoot>/.qwen/settings.json，而会话实际工作在 worktree 中。— 修复方向：让工具路径通知 bridge（如在 enter_worktree / 目标为 worktree 的 session/cd 之后调用 setSessionWorktree），或明确文档说明 serve HTTP 面只跟随 REST 创建的 worktree 会话并将工具路径缺口开为后续 issue；补充覆盖工具进入形态的测试。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| path.normalize(s.worktree.path).startsWith(normalizedBound + path.sep) && | ||
| pathExists(s.worktree.path), |
There was a problem hiding this comment.
[Critical] R2-4: The containment check rejects worktrees under the git repo top-level, but worktrees always anchor at <repoRoot>/.qwen/worktrees/ (enter-worktree.ts:115-119 — projectRoot = getRepoTopLevel() ?? cwd; config.ts:3051-3068 explains why a subdirectory anchor would never exist), which is OUTSIDE a monorepo subdirectory workspace. The session-restore route (routes/session.ts:2326-2350) explicitly supports this layout — it validates against two candidate roots, with a comment about monorepo subdirectory workspaces, then stores the repo-top-level path — which this check then rejects. — Failure scenario: daemon bound to /repo/packages/api; a session entered/restored from a worktree at /repo/.qwen/worktrees/feat → startsWith('/repo/packages/api/') is false → fall back to boundWorkspace → settings and QWEN.md init silently target the workspace root instead of the worktree, in exactly the configuration the restore route's candidate-roots logic exists to support. REST-created worktrees under the workspace dir pass; tool-created/restored repo-top worktrees (the issue's named flow) do not. — Fix direction: mirror the restore route's candidateRoots — accept the worktree when contained in boundWorkspace OR in its git repo top-level (both canonicalized via realpath, as session.ts does).
中文说明
[Critical] R2-4：包含性检查会拒绝位于 git 仓库顶层目录下的 worktree，但 worktree 始终锚定在 <repoRoot>/.qwen/worktrees/（enter-worktree.ts:115-119 —— projectRoot = getRepoTopLevel() ?? cwd；config.ts:3051-3068 解释了为何子目录锚点永远不存在），而该位置在 monorepo 子目录 workspace 之外。会话恢复路由（routes/session.ts:2326-2350）明确支持这种布局——它针对两个候选根做校验，并附有 monorepo 子目录 workspace 的注释，然后保存仓库顶层路径——而本检查随后却拒绝了它。— 失败场景：守护进程绑定 /repo/packages/api；会话进入/恢复自 /repo/.qwen/worktrees/feat → startsWith('/repo/packages/api/') 为 false → 回退到 boundWorkspace → 设置与 QWEN.md 初始化静默指向 workspace 根目录而非 worktree，恰好是恢复路由的 candidateRoots 逻辑存在所要支持的配置。REST 创建的、位于 workspace 目录下的 worktree 能通过；工具创建/恢复的仓库顶层 worktree（issue 点名的流程）不能。— 修复方向：复刻恢复路由的 candidateRoots——worktree 包含于 boundWorkspace 或其 git 仓库顶层目录时均接受（两者都经 realpath 规范化，与 session.ts 一致）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| resolveEffectiveWorkspace: () => | ||
| findEffectiveWorkspace(primaryBridge, primaryBoundWorkspace), |
There was a problem hiding this comment.
[Critical] R2-6: Settings reads now follow the active worktree, but bridge-driven workspace-scope writes still target boundWorkspace: bridge.applyApprovalMode persists via persistApprovalMode?.(boundWorkspace, mode) (bridge.ts:3515; the callback in run-qwen-serve.ts:3940-3959 writes tools.approvalMode at workspace scope of the root), and persistDisabledToolsFn/persistDisabledSkillsFn (workspace-service/index.ts:791-796) have the same shape — while resolveContextFile two doors down (run-qwen-serve.ts:3972) IS worktree-aware. The inconsistency is inside the same wiring function. — Failure scenario: worktree session active; user changes approval mode with persist: true (web shell POST /session/:id/approval-mode or ACP-HTTP session/set_config_option with configId: 'mode') → the write goes to the root .qwen/settings.json; the settings UI reading the worktree file keeps showing the old value; workspaceReload reloading from the worktree never picks it up; the persisted mode only reappears after the worktree session closes. Before this PR, reads and writes were both anchored at boundWorkspace (coherent). — Fix direction: route these persists through findEffectiveWorkspace(bridge, boundWorkspace) instead of boundWorkspace, or deliberately document and test the split.
中文说明
[Critical] R2-6：设置读取现在跟随活跃 worktree，但 bridge 驱动的工作区范围写入仍以 boundWorkspace 为目标：bridge.applyApprovalMode 通过 persistApprovalMode?.(boundWorkspace, mode) 持久化（bridge.ts:3515；run-qwen-serve.ts:3940-3959 中的回调把 tools.approvalMode 写入根目录的 workspace scope），persistDisabledToolsFn/persistDisabledSkillsFn（workspace-service/index.ts:791-796）形状相同——而紧邻的 resolveContextFile（run-qwen-serve.ts:3972）却已具备 worktree 感知。不一致就出现在同一个接线函数内部。— 失败场景：worktree 会话活跃时，用户以 persist: true 更改审批模式（web shell POST /session/:id/approval-mode 或 ACP-HTTP session/set_config_option（configId: 'mode'））→ 写入落在根目录 .qwen/settings.json；读取 worktree 文件的设置 UI 继续显示旧值；从 worktree 重新加载的 workspaceReload 永远取不到它；持久化的模式只在 worktree 会话关闭后才"重新出现"。本 PR 之前读写都锚定 boundWorkspace（一致）。— 修复方向：让这些持久化改经 findEffectiveWorkspace(bridge, boundWorkspace) 而非 boundWorkspace，或有意地文档化并测试此分裂。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| } | ||
|
|
||
| if (canonicalPath !== process.cwd()) { | ||
| session.worktreeCwd = canonicalPath; |
There was a problem hiding this comment.
[Suggestion] R2-9: session/cd stamps worktree state (worktreeCwd/defaultSettingsCwd) for ANY target directory ≠ process.cwd() — not only git worktrees; createAndStoreSession has the same raw comparison (that site sits under an existing thread). This contradicts Session.worktreeCwd's own doc ("null for regular sessions"). — Concrete cost: cd-ing a session into a workspace subdirectory (POST /session/:id/cd is a public route; ordinary usage) makes every sessionless qwen/settings/* call resolve against that subdirectory: loadSettings(<subdir>) finds no workspace settings (UI shows defaults), and write handlers create a stray <subdir>/.qwen/settings.json while the running session's effective settings (loaded from the workspace root at session start) diverge from what the UI displays. — Fix direction: only treat a relocation as settings-relevant when the target is actually a worktree (e.g. under <repoTop>/.qwen/worktrees/, matching the restore route's containment logic); otherwise leave settings resolution at the workspace root.
中文说明
[Suggestion] R2-9：session/cd 会为任何 ≠ process.cwd() 的目标目录打上 worktree 状态（worktreeCwd/defaultSettingsCwd）——而不只是 git worktree；createAndStoreSession 有同样的裸比较（该处已有现存讨论线程）。这与 Session.worktreeCwd 自身的文档（"常规会话为 null"）相矛盾。— 具体代价：将会话 cd 进 workspace 子目录（POST /session/:id/cd 是公开路由，属常规操作）后，所有无 sessionId 的 qwen/settings/* 调用都会解析到该子目录：loadSettings(<子目录>) 找不到工作区设置（UI 显示默认值），写入处理器还会创建游离的 <子目录>/.qwen/settings.json，而运行中会话的有效设置（会话启动时从 workspace 根加载）与 UI 显示发生偏离。— 修复方向：仅当目标是真实 worktree（如位于 <repoTop>/.qwen/worktrees/ 下，与恢复路由的包含逻辑一致）时才视其为设置相关；否则设置解析保持在 workspace 根。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| getDebugMode: vi.fn().mockReturnValue(false), | ||
| getMcpServers: vi.fn().mockReturnValue({}), | ||
| setMcpBudgetEventCallback: vi.fn(), | ||
| getTargetDir: vi.fn().mockReturnValue(process.cwd()), |
There was a problem hiding this comment.
[Suggestion] R2-12: The test-efficacy probe (harness validated) reverted this hunk — the only change in this file, added to fix the earlier TypeError blocker — and all 3 Phase C tests still passed: because the mock returns process.cwd(), the tests never leave the non-worktree branch, so nothing gates the getTargetDir-based worktree resolution this PR adds. — Concrete cost: a regression breaking or removing that resolution ships green with this suite. — Fix direction: make at least one test drive the worktree branch — have getTargetDir return a directory distinct from process.cwd() and assert the worktree-specific restore behavior, so reverting the change fails the suite.
中文说明
[Suggestion] R2-12：测试有效性探针（测试框架已验证）回退了此 hunk——本文件唯一的改动，当初为修复早先的 TypeError 阻断而添加——3 个 Phase C 测试依然全部通过：因为 mock 返回 process.cwd()，测试从未离开非 worktree 分支，所以本 PR 新增的基于 getTargetDir 的 worktree 解析没有任何测试把关。— 具体代价：破坏或移除该解析的回归可以在此测试套件全绿的情况下合入。— 修复方向：让至少一个测试驱动 worktree 分支——使 getTargetDir 返回一个不同于 process.cwd() 的目录并断言 worktree 特有的恢复行为，这样回退该改动时测试会失败。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if (this.defaultSettingsCwd && existsSync(this.defaultSettingsCwd)) { | ||
| return this.defaultSettingsCwd; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-13: This fallback branch is provably unreachable dead code. The field's only writers (createAndStoreSession at 11979-11985, session/cd at 9800-9806) always assign defaultSettingsCwd in lockstep with a registered session's worktreeCwd, and the only removal path (removeStoredSessionEntry) clears it when the owner session leaves — so defaultSettingsCwd !== null implies some live session carries the same worktreeCwd, which the scan loop above (7685-7689) returns first (same value ⇒ same existsSync result, including the deleted-directory case). The JSDoc documents a priority level that can never fire, and the close-time clearing hunk exists solely to preserve the invariant that keeps this branch dead (AGENTS.md: "for every added field, grep its read sites"). — Concrete cost: the next maintainer debugging settings resolution chases a fallback with no observable effect; a future writer that sets defaultSettingsCwd without a paired worktreeCwd silently behaves differently than the documented priority claims. — Fix direction: remove the field, the close-time clearing block, and the doc clause — or, if the agent-level fallback is wanted, let it mean something and add a test that actually reaches it.
中文说明
[Suggestion] R2-13：此回退分支是可证明不可达的死代码。该字段仅有的两个写入点（createAndStoreSession（11979-11985）与 session/cd（9800-9806））总是将 defaultSettingsCwd 与已注册会话的 worktreeCwd 同步赋值，而唯一的移除路径（removeStoredSessionEntry）在属主会话离开时将其清除——因此 defaultSettingsCwd !== null 必然意味着某个存活会话携带相同的 worktreeCwd，而上方的扫描循环（7685-7689）会先返回它（相同值 ⇒ 相同的 existsSync 结果，目录被删除的情形亦然）。JSDoc 记录了一个永远不会生效的优先级层级，而关闭时清理的 hunk 存在的唯一作用就是维持令本分支死掉的不变量（AGENTS.md："对每个新增字段，grep 其读取点"）。— 具体代价：下一位调试设置解析的维护者会追查一个没有任何可观测效果的回退；未来若有写入点单独设置 defaultSettingsCwd 而不成对设置 worktreeCwd，其行为会与文档声称的优先级悄然不符。— 修复方向：移除该字段、关闭时清理块与文档条款——或者若确实需要代理级回退，让它真正有意义并补充能到达它的测试。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const assertGenerationOpen = | ||
| deps.captureGenerationAssertion?.() ?? (() => {}); | ||
| assertGenerationOpen(); | ||
| const getWorkspace = resolveEffectiveWorkspace?.() ?? boundWorkspace; |
There was a problem hiding this comment.
[Suggestion] R2-15: The worktree-aware resolver was wired into the singular route family only; the plural /workspaces/:workspace/settings routes (registerWorkspaceQualifiedSettingsRoutes, same file, ~520-690) still read/write runtime.workspaceCwd in buildSettingsResponse, prepareSettingWrite, and persistSetting. The two route families now disagree about the same workspace whenever a worktree session is active. No in-tree consumer exhibits this today (the webui settings panel uses the singular route via the primary DaemonClient), but WorkspaceDaemonClient in sdk-typescript exposes the plural family. — Concrete cost: an external SDK consumer calling the plural route against the primary workspace while a worktree session is active reads/writes the root, while the singular route and the ACP child resolve the worktree — same settings, different answers depending on which route the client hit. — Fix direction: pass an equivalent resolver (from runtime.bridge) into the plural routes and use it in buildSettingsResponse/persist there.
中文说明
[Suggestion] R2-15：worktree 感知解析器只接入了单数路由族；复数 /workspaces/:workspace/settings 路由（registerWorkspaceQualifiedSettingsRoutes，同文件 ~520-690）在 buildSettingsResponse、prepareSettingWrite、persistSetting 中仍读写 runtime.workspaceCwd。一旦存在活跃 worktree 会话，两个路由族对同一 workspace 就会给出不同结果。当前仓库内没有消费者触发此差异（webui 设置面板通过主 DaemonClient 走单数路由），但 sdk-typescript 的 WorkspaceDaemonClient 暴露了复数族。— 具体代价：外部 SDK 消费者在 worktree 会话活跃时对主 workspace 调用复数路由，读写的是根目录，而单数路由与 ACP 子进程解析的是 worktree——同样的设置，因客户端命中的路由不同而得到不同答案。— 修复方向：把等价的解析器（来自 runtime.bridge）传入复数路由，并在其 buildSettingsResponse/持久化处使用。
— qwen3.8-max via Qwen Code /review (v0.21.7)
…, worktree containment, persist routing, dead code removal
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — packages/desktop suites were not run by this review (separate toolchain/lockfile; the two changed files there are TODO-comment-only, logic byte-identical).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within the tool budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: did not determine whether applyProviderInstallPlan writes workspace-scope settings — installAuthProvider (run-qwen-serve.ts:5774) still locks/loads boundWo…, and 1 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
[Critical] R3-24: [Critical] Voice-settings writes were not converted to effective-workspace resolution: setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:738-760, not touched by the diff) while this same diff converts the settings GET routes and every sibling write (approval mode ×3 factories, tool/skill toggles, generic setting writes) to findEffectiveWorkspace. Secondary/dynamic factories force voiceSettingsScope: WORKSPACE_SETTING_SCOPE (run-qwen-serve.ts:4577, 5139). — Failure scenario: workspace X with a REST-created/restored session inside worktree W: workspace/voice/set writes X/.qwen/settings.json; the worktree session's child loads W/.qwen/settings.json → the change never applies to the running session, and GET /workspace/settings (rewired to read W) returns the stale voiceModel right after the 200. Could not be anchored inline: the write site is unchanged code outside the diff. 中文说明：语音设置写入未转换为 effective-workspace 解析——同一 diff 把设置 GET 路由与所有兄弟写入都转到了 findEffectiveWorkspace，setWorkspaceVoiceSettings 却仍写 boundWorkspace；secondary/dynamic 工厂还强制 workspace scope。结果：worktree 会话的子进程读 W 的设置，语音变更写入 X，变更对运行中的会话永不生效，GET 在 200 之后返回过期的 voiceModel。写入点在 diff 之外的未改动代码中，无法锚定行内。
[Critical] R2-2: [Critical] Still stands (re-checked against b5a8b2a): "Fixes #8138" remains overstated for the desktop client. The desktop Settings panel sends qwen/settings/* through a dedicated session-less shared ACP process (buildSharedAcpProcessKey never matches the session-hosting process — the session agent's key includes envOverrides = { CRAFT_WORKSPACE_PATH } while getQwenWorkspaceAcpOptions passes none), so in that process this.sessions is empty and resolveSettingsCwd returns process.cwd() — the issue's own repro (change the model in the Settings panel after enter_worktree) still writes to <projectRoot>/.qwen/settings.json. The diff's own TODO(#8138) in resolveSettingsCwd (acpAgent.ts:7858-7863) concedes this. Fix direction: complete the desktop wiring (populate worktreeRootPath, or route the panel through the session-hosting process), or rescope to "Partially addresses #8138". 中文说明：仍然成立（已对 b5a8b2a 复核）——对桌面客户端 "Fixes #8138" 仍言过其实。桌面设置面板经无会话的共享 ACP 进程发送 qwen/settings/*，该进程 this.sessions 为空，resolveSettingsCwd 返回 process.cwd()——issue 自身的复现（enter_worktree 后在设置面板改模型）仍写入项目根。diff 自己的 TODO(#8138) 也承认这一点。
[Critical] R2-3: [Critical] Still stands (re-checked against b5a8b2a): findEffectiveWorkspace only sees worktrees via BridgeSessionSummary.worktree, which the production bridge populates only at REST worktree-session creation and load/resume restore. A worktree entered via the enter_worktree tool mid-session — the trigger issue #8138 explicitly names — never reaches the bridge (the tool only mutates in-child Config state; no setSessionWorktree wiring in packages/cli/src/serve), so GET/POST /workspace/settings and POST /workspace/init keep resolving against the project root for that session shape. The diff's TODO at worktree-workspace.ts:26-30 acknowledges it. Fix direction: have the tool-driven path notify the bridge (call setSessionWorktree after enter_worktree / worktree-targeted session/cd), or document the limit and file a follow-up. 中文说明：仍然成立（已对 b5a8b2a 复核）——findEffectiveWorkspace 只能通过 BridgeSessionSummary.worktree 看到 worktree，而生产 bridge 只在 REST 创建与加载/恢复时填充它；会话中途经 enter_worktree 进入的 worktree 永远到不了 bridge，该会话形态下 GET/POST /workspace/settings 与 POST /workspace/init 仍解析到项目根。diff 的 TODO 也承认此缺口。
中文说明
未审查：build-and-test — packages/desktop suites were not run by this review (separate toolchain/lockfile; the two changed files there are TODO-comment-only, logic byte-identical)。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above were completed within the tool budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：did not determine whether applyProviderInstallPlan writes workspace-scope settings — installAuthProvider (run-qwen-serve.ts:5774) still locks/loads boundWo…，另有 1 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
[Critical] R3-24: [Critical] Voice-settings writes were not converted to effective-workspace resolution: setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:738-760, not touched by the diff) while this same diff converts the settings GET routes and every sibling write (approval mode ×3 factories, tool/skill toggles, generic setting writes) to findEffectiveWorkspace. Secondary/dynamic factories force voiceSettingsScope: WORKSPACE_SETTING_SCOPE (run-qwen-serve.ts:4577, 5139). — Failure scenario: workspace X with a REST-created/restored session inside worktree W: workspace/voice/set writes X/.qwen/settings.json; the worktree session's child loads W/.qwen/settings.json → the change never applies to the running session, and GET /workspace/settings (rewired to read W) returns the stale voiceModel right after the 200. Could not be anchored inline: the write site is unchanged code outside the diff. 中文说明：语音设置写入未转换为 effective-workspace 解析——同一 diff 把设置 GET 路由与所有兄弟写入都转到了 findEffectiveWorkspace，setWorkspaceVoiceSettings 却仍写 boundWorkspace；secondary/dynamic 工厂还强制 workspace scope。结果：worktree 会话的子进程读 W 的设置，语音变更写入 X，变更对运行中的会话永不生效，GET 在 200 之后返回过期的 voiceModel。写入点在 diff 之外的未改动代码中，无法锚定行内。
[Critical] R2-2: [Critical] Still stands (re-checked against b5a8b2a): "Fixes #8138" remains overstated for the desktop client. The desktop Settings panel sends qwen/settings/* through a dedicated session-less shared ACP process (buildSharedAcpProcessKey never matches the session-hosting process — the session agent's key includes envOverrides = { CRAFT_WORKSPACE_PATH } while getQwenWorkspaceAcpOptions passes none), so in that process this.sessions is empty and resolveSettingsCwd returns process.cwd() — the issue's own repro (change the model in the Settings panel after enter_worktree) still writes to <projectRoot>/.qwen/settings.json. The diff's own TODO(#8138) in resolveSettingsCwd (acpAgent.ts:7858-7863) concedes this. Fix direction: complete the desktop wiring (populate worktreeRootPath, or route the panel through the session-hosting process), or rescope to "Partially addresses #8138". 中文说明：仍然成立（已对 b5a8b2a 复核）——对桌面客户端 "Fixes #8138" 仍言过其实。桌面设置面板经无会话的共享 ACP 进程发送 qwen/settings/*，该进程 this.sessions 为空，resolveSettingsCwd 返回 process.cwd()——issue 自身的复现（enter_worktree 后在设置面板改模型）仍写入项目根。diff 自己的 TODO(#8138) 也承认这一点。
[Critical] R2-3: [Critical] Still stands (re-checked against b5a8b2a): findEffectiveWorkspace only sees worktrees via BridgeSessionSummary.worktree, which the production bridge populates only at REST worktree-session creation and load/resume restore. A worktree entered via the enter_worktree tool mid-session — the trigger issue #8138 explicitly names — never reaches the bridge (the tool only mutates in-child Config state; no setSessionWorktree wiring in packages/cli/src/serve), so GET/POST /workspace/settings and POST /workspace/init keep resolving against the project root for that session shape. The diff's TODO at worktree-workspace.ts:26-30 acknowledges it. Fix direction: have the tool-driven path notify the bridge (call setSessionWorktree after enter_worktree / worktree-targeted session/cd), or document the limit and file a follow-up. 中文说明：仍然成立（已对 b5a8b2a 复核）——findEffectiveWorkspace 只能通过 BridgeSessionSummary.worktree 看到 worktree，而生产 bridge 只在 REST 创建与加载/恢复时填充它；会话中途经 enter_worktree 进入的 worktree 永远到不了 bridge，该会话形态下 GET/POST /workspace/settings 与 POST /workspace/init 仍解析到项目根。diff 的 TODO 也承认此缺口。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| type ToolInvocationGuard, | ||
| findGitRoot, | ||
| } from '@qwen-code/qwen-code-core'; |
There was a problem hiding this comment.
[Critical] The new findGitRoot import is not declared in acpAgent.test.ts's vi.mock('@qwen-code/qwen-code-core', …) factory (line ~192 — it enumerates exports manually without spreading importOriginal()), and createAndStoreSession now calls isWorktreePath() → findGitRoot() for every session whose getTargetDir() !== process.cwd(). — Failure scenario: npm test --workspace="packages/cli" → 174 failed | 227 passed (401) in acpAgent.test.ts, every failure [vitest] No "findGitRoot" export is defined on the "@qwen-code/qwen-code-core" mock — including all 6 tests this PR adds and pre-existing tests (e.g. defers MCP discovery for a worktree session until relocation). Measured net-new against the merge base (the base passes this file).
| type ToolInvocationGuard, | |
| findGitRoot, | |
| } from '@qwen-code/qwen-code-core'; | |
| type ToolInvocationGuard, | |
| findGitRoot, | |
| } from '@qwen-code/qwen-code-core'; |
(add findGitRoot: vi.fn().mockReturnValue(null), to the core mock factory in acpAgent.test.ts, mirroring acpAgent.worktree.test.ts, then re-run the file — note this alone does not green the new tests, see the fixture-shape comment)
中文说明
[Critical] 新增的 findGitRoot 导入未在 acpAgent.test.ts 的 vi.mock('@qwen-code/qwen-code-core', …) 工厂（约第 192 行——该工厂逐项列举导出、未展开 importOriginal()）中声明，而 createAndStoreSession 现在会对每个 getTargetDir() !== process.cwd() 的会话调用 isWorktreePath() → findGitRoot()。— 失败场景：npm test --workspace="packages/cli" → acpAgent.test.ts 174 失败 | 227 通过（共 401），每个失败都是 [vitest] No "findGitRoot" export is defined on the "@qwen-code/qwen-code-core" mock——包括本 PR 新增的全部 6 个测试和既有测试（如 defers MCP discovery for a worktree session until relocation）。已相对合并基线测量为净新增（基线上该文件通过）。
修复：在 acpAgent.test.ts 的 core mock 工厂中加入 findGitRoot: vi.fn().mockReturnValue(null),（与 acpAgent.worktree.test.ts 一致），然后重跑该文件——注意仅此一项无法让新测试变绿，见关于 fixture 形状的评论。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| function isWorktreePath(target: string): boolean { | ||
| const repoRoot = findGitRoot(target); | ||
| if (!repoRoot) return false; |
There was a problem hiding this comment.
[Critical] isWorktreePath can never return true for a real git worktree. findGitRoot (gitUtils.ts:100) walks up with existsSync and stops at the worktree's own .git gitfile (git worktree add places a gitfile, not a directory, at each worktree root), so it returns the worktree itself as the "repo root"; worktreesDir becomes <worktree>/.qwen/worktrees and the startsWith check fails. Verified by probe against a real git worktree add fixture: findGitRoot(W) = W, isWorktreePath(W) = false. — Failure scenario: a session cd'd into a worktree (session/cd handler) or created inside one (createAndStoreSession) never gets worktreeCwd set; additionally the daemon cold-restore path's follow-up changeSessionCwd INTO the worktree runs the else branch and wipes the worktreeCwd that #restoreWorktreeOnResume set one step earlier (the MUST-1/R2-17 test stops before the wipe). So resolveSettingsCwd falls through to process.cwd() and qwen/settings/* resolves against the project root — issue #8138 remains unfixed for these flows; only the sidecar-restore path and enter_worktree's config-level setActiveWorktree survive.
| function isWorktreePath(target: string): boolean { | |
| const repoRoot = findGitRoot(target); | |
| if (!repoRoot) return false; | |
| function isWorktreePath(target: string): boolean { | |
| const repoRoot = findGitRoot(target); | |
| if (!repoRoot) return false; |
(compute the repo root via the gitfile/common dir — e.g. git rev-parse --git-common-dir, or resolve <wt>/.git's gitdir: pointer to the common dir — before building worktreesDir; then extend MUST-1 to issue the daemon's post-load changeSessionCwd and re-assert)
中文说明
[Critical] isWorktreePath 对真实 git worktree 永远返回 false。findGitRoot（gitUtils.ts:100）用 existsSync 向上查找，会停在 worktree 自身的 .git gitfile 上（git worktree add 在每个 worktree 根目录放置的是 gitfile 而非目录），于是把 worktree 本身当作"仓库根"返回；worktreesDir 变成 <worktree>/.qwen/worktrees，startsWith 检查必然失败。已用真实 git worktree add fixture 探针验证：findGitRoot(W) = W、isWorktreePath(W) = false。— 失败场景：cd 进 worktree 的会话（session/cd 处理器）或在 worktree 内创建的会话（createAndStoreSession）永远不会设置 worktreeCwd；此外守护进程冷恢复路径随后的"cd 进 worktree"（changeSessionCwd）会走 else 分支，抹掉上一步 #restoreWorktreeOnResume 刚设置的 worktreeCwd（MUST-1/R2-17 测试在抹除之前就停止了）。于是 resolveSettingsCwd 回退到 process.cwd()，qwen/settings/* 解析到项目根目录——对这些链路，issue #8138 依旧未修复；只有 sidecar 恢复路径和 enter_worktree 的 config 级 setActiveWorktree 幸存。
修复方向：通过 gitfile/common dir 计算仓库根（如 git rev-parse --git-common-dir，或解析 <wt>/.git 的 gitdir: 指向的 common dir），再构造 worktreesDir；随后扩展 MUST-1，使其发出守护进程加载后的 changeSessionCwd 并再次断言。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const oldMerged = structuredClone(this.settings.merged); | ||
|
|
||
| this.settings.reloadScopeFromDisk(SettingScope.User); | ||
| this.settings.reloadScopeFromDisk(SettingScope.Workspace); | ||
| this.settings = loadSettings(settingsCwd); | ||
| const newMerged = this.settings.merged; |
There was a problem hiding this comment.
[Critical] The workspaceReload swap from in-place reloadScopeFromDisk(User/Workspace) to this.settings = loadSettings(settingsCwd) breaks reload semantics in three verified ways: (1) the two pre-existing tests (clears removed providerProtocol mappings and refreshes auth on workspace reload ~18025, clears Todo Stop Guard trust when workspace reload enters plan mode ~18099) drive their change via a reloadScopeFromDisk mock the handler no longer calls — probe-verified: they still fail after the unrelated findGitRoot mock fix (diffSettingsKeys empty → reloadModelProvidersConfig/setApprovalMode('plan') never called); (2) session-owned LoadedSettings instances stay stale — Session reads ui.enableFollowupSuggestions/ui.enableCacheSharing per turn from its own instance and buildDisabledSkillNamesProvider closes over it, while the propagation loop only refreshes config-level state, yet the response reports sessionsRefreshed; (3) oldMerged is captured from this.settings, which every settings handler now rebinds to a request-scoped settingsCwd — so the baseline can come from a different directory than newMerged (e.g. previous request bound the root, this reload resolves a worktree), fabricating changedKeys that fire switchModel/refreshAuth/setApprovalMode/setDisabledTools on every idle session, or masking real on-disk changes when the two dirs agree. — Failure scenario: plain session created at the root binds this.settings to the root; a later workspaceReload resolving to worktree W diffs root-vs-W with nothing changed on disk → W's model/approval-mode/disabled tools applied to all idle sessions; the inverse masks real updates. Pre-diff, reloadScopeFromDisk guaranteed old and new came from the same directory.
| const oldMerged = structuredClone(this.settings.merged); | |
| this.settings.reloadScopeFromDisk(SettingScope.User); | |
| this.settings.reloadScopeFromDisk(SettingScope.Workspace); | |
| this.settings = loadSettings(settingsCwd); | |
| const newMerged = this.settings.merged; | |
| const oldMerged = structuredClone(this.settings.merged); | |
| this.settings = loadSettings(settingsCwd); | |
| const newMerged = this.settings.merged; |
(keep a disk-resync for the instances sessions hold — reload each session's own scopes in the propagation loop, or adopt the freshly loaded scopes into the shared cached instance; capture the baseline from the same directory being reloaded; rework the two orphaned tests onto the new mechanism)
中文说明
[Critical] workspaceReload 从就地 reloadScopeFromDisk(User/Workspace) 换成 this.settings = loadSettings(settingsCwd)，以三种已验证的方式破坏了 reload 语义：（1）两个既有测试（约 18025 的 clears removed providerProtocol mappings and refreshes auth on workspace reload、约 18099 的 clears Todo Stop Guard trust when workspace reload enters plan mode）通过处理器已不再调用的 reloadScopeFromDisk mock 驱动变更——探针验证：即便修掉无关的 findGitRoot mock 问题后它们仍然失败（diffSettingsKeys 为空 → reloadModelProvidersConfig/setApprovalMode('plan') 永不被调用）；（2）会话自持的 LoadedSettings 实例保持过期——Session 每轮从自己的实例读取 ui.enableFollowupSuggestions/ui.enableCacheSharing，buildDisabledSkillNamesProvider 也闭包引用它，而逐会话传播循环只刷新 config 级状态，响应却报告 sessionsRefreshed；（3）oldMerged 取自 this.settings，而本 diff 的每个 settings 处理器都会把 this.settings 重绑定到请求级的 settingsCwd——基线可能与 newMerged 来自不同目录（例如上一个请求绑定了根目录，本次 reload 解析到 worktree），凭空产生 changedKeys，对每个空闲会话触发 switchModel/refreshAuth/setApprovalMode/setDisabledTools；反之当两个目录在某键上一致时，真实的磁盘变更又被掩盖。— 失败场景：在根目录创建的普通会话把 this.settings 绑定到根；随后解析到 worktree W 的 workspaceReload 在磁盘毫无变化时对 root 与 W 做 diff → W 的 model/审批模式/禁用工具被应用到所有空闲会话；反向则掩盖真实更新。变更前 reloadScopeFromDisk 保证新旧来自同一目录。
修复方向：为会话持有的实例保留磁盘重同步（在传播循环中重载每个会话自己的 scope，或把新加载的 scope 合并进共享缓存实例）；从被 reload 的同一目录捕获基线；把两个被孤立的测试改造到新机制上。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
|
||
| this.settings.reloadScopeFromDisk(SettingScope.User); | ||
| this.settings.reloadScopeFromDisk(SettingScope.Workspace); | ||
| this.settings = loadSettings(settingsCwd); |
There was a problem hiding this comment.
[Critical] workspaceReload consumes settingsCwd, whose priority chain prefers the no-sessionId worktree scan over the caller's explicit cwd — but workspace-service's reload() invokes this method with { cwd: boundWorkspace } and no sessionId (workspace-service/index.ts:1466), intending to reload THAT workspace. The scan returns the first session's worktree W and (resolvedWt !== process.cwd() ? resolvedWt : undefined) || requestedCwd || … discards requestedCwd. — Failure scenario: session A resumed inside worktree W, plain session B at the workspace root; workspace-service issues workspaceReload {cwd: boundWorkspace} → settingsCwd = W → the handler loads W's settings and its per-session loop (only filter: isIdle()) applies W's tools.approvalMode, tools.disabled, model, modelProviders to every session — B silently inherits the worktree's security posture — and reloadEnvironment(newMerged, settingsCwd) rebuilds the daemon child's process.env from W's .env/settings.env. The R1-1 rationale (worktree beats explicit cwd) was argued for the desktop Settings panel, not for a workspace-wide reload that names its target workspace.
| this.settings = loadSettings(settingsCwd); | |
| this.settings = loadSettings(settingsCwd); |
(for serve-control methods whose params.cwd identifies the target workspace, prefer the explicit cwd — e.g. use cwd here as pre-diff — or make resolveSettingsCwd not override a provided cwd param, or have workspace-service pass the owning sessionId)
中文说明
[Critical] workspaceReload 消费 settingsCwd，其优先级链让无 sessionId 的 worktree 扫描优先于调用方的显式 cwd——但 workspace-service 的 reload() 调用本方法时传 { cwd: boundWorkspace } 且不带 sessionId（workspace-service/index.ts:1466），意图是 reload 该 workspace。扫描返回第一个会话的 worktree W，(resolvedWt !== process.cwd() ? resolvedWt : undefined) || requestedCwd || … 丢弃了 requestedCwd。— 失败场景：会话 A 恢复在 worktree W 内，普通会话 B 在 workspace 根；workspace-service 发起 workspaceReload {cwd: boundWorkspace} → settingsCwd = W → 处理器加载 W 的设置，其逐会话循环（唯一过滤条件是 isIdle()）把 W 的 tools.approvalMode、tools.disabled、model、modelProviders 应用到每个会话——B 静默继承 worktree 的安全姿态——且 reloadEnvironment(newMerged, settingsCwd) 用 W 的 .env/settings.env 重建守护子进程的 process.env。R1-1 的理由（worktree 优先于显式 cwd）是为桌面设置面板论证的，不是为"点名了目标 workspace 的整 workspace reload"论证的。
修复方向：对 params.cwd 用于标识目标 workspace 的 serve-control 方法，优先使用显式 cwd（如此处按变更前使用 cwd），或让 resolveSettingsCwd 不覆盖已提供的 cwd 参数，或让 workspace-service 传入所属 sessionId。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| } else { | ||
| session.worktreeCwd = null; | ||
| } |
There was a problem hiding this comment.
[Critical] session/cd out of a worktree clears session.worktreeCwd but never clears the PR-introduced Config.activeWorktreePath; the per-session branch of resolveSettingsCwd then falls back to getActiveWorktree(), so settings keep resolving against the worktree the session just left. Only exit_worktree ever calls setActiveWorktree(null); relocateWorkingDirectory never touches the field. Probe-verified with flip: adding the clear below makes resolution fall through correctly. — Failure scenario: a session resumed inside worktree W (#restoreWorktreeOnResume sets both halves), then the client relocates it via session/cd to a plain directory D → else branch nulls worktreeCwd, activeWorktreePath stays W → next qwen/settings/* with that sessionId finds configWt = W still on disk and returns W — settings are read from and persisted to the abandoned worktree instead of the session's actual directory D. Also reachable via enter_worktree followed by a plain session/cd out.
| } else { | |
| session.worktreeCwd = null; | |
| } | |
| } else { | |
| session.worktreeCwd = null; | |
| this.config.setActiveWorktree?.(null); | |
| } |
(and mirror setActiveWorktree(canonicalPath) in the if branch, consistent with enter_worktree and #restoreWorktreeOnResume)
中文说明
[Critical] session/cd 离开 worktree 时清除了 session.worktreeCwd，却从不清除本 PR 引入的 Config.activeWorktreePath；resolveSettingsCwd 的按会话分支随后回退到 getActiveWorktree()，于是设置继续解析到会话刚离开的 worktree。只有 exit_worktree 会调用 setActiveWorktree(null)；relocateWorkingDirectory 从不触碰该字段。已用探针验证并翻转：加上如下清除后解析正确回退。— 失败场景：会话恢复在 worktree W 内（#restoreWorktreeOnResume 同时设置两部分），随后客户端通过 session/cd 把它迁到普通目录 D → else 分支把 worktreeCwd 置 null，activeWorktreePath 仍为 W → 下一次带该 sessionId 的 qwen/settings/* 发现 configWt = W 仍在磁盘上，返回 W——设置从被遗弃的 worktree 读取并写入，而不是会话实际所在的目录 D。也可经 enter_worktree 后普通 session/cd 离开触达。
修复：在 else 分支加 this.config.setActiveWorktree?.(null);（并在 if 分支对应地 setActiveWorktree(canonicalPath)，与 enter_worktree 和 #restoreWorktreeOnResume 保持一致）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const effective = findEffectiveWorkspace( | ||
| runtime.bridge, | ||
| runtime.workspaceCwd, | ||
| ); | ||
| const response = buildSettingsResponse(effective, allowedKeys); |
There was a problem hiding this comment.
[Suggestion] The workspace-qualified settings GET/POST routes now redirect reads/writes to a worktree via findEffectiveWorkspace, but no test exercises these routes (or the run-qwen-serve.ts persistApprovalMode callbacks) with a bridge that has a worktree session — exhaustively checked: workspace-qualified-rest.test.ts mocks listWorkspaceSessions: vi.fn(() => []) and contains zero "worktree"; server.test.ts's only worktree listImpl override is in an unrelated POST /session test; run-qwen-serve.test.ts has no worktree mention. — Concrete cost: the mutation "pass runtime.workspaceCwd instead of effective" stays green in every test; a wiring regression ships with the exact write mis-direction #8138 describes untested at route level.
| const effective = findEffectiveWorkspace( | |
| runtime.bridge, | |
| runtime.workspaceCwd, | |
| ); | |
| const response = buildSettingsResponse(effective, allowedKeys); | |
| const effective = findEffectiveWorkspace( | |
| runtime.bridge, | |
| runtime.workspaceCwd, | |
| ); | |
| const response = buildSettingsResponse(effective, allowedKeys); |
(add a qualified-route test whose runtime.bridge.listWorkspaceSessions returns a session with an existing worktree.path, asserting loadSettings/persistSetting receive the worktree path)
中文说明
[Suggestion] workspace-qualified settings GET/POST 路由现在通过 findEffectiveWorkspace 把读写重定向到 worktree，但没有任何测试在"bridge 带有 worktree 会话"的前提下验证这些路由（或 run-qwen-serve.ts 的 persistApprovalMode 回调）——已穷尽核查：workspace-qualified-rest.test.ts mock listWorkspaceSessions: vi.fn(() => []) 且全文无 "worktree"；server.test.ts 唯一返回 worktree 的 listImpl 覆盖在一个无关的 POST /session 测试里；run-qwen-serve.test.ts 无 worktree 字样。— 具体代价：变异"把 effective 换回 runtime.workspaceCwd"在所有测试中保持绿色；接线回归会在路由层面毫无测试的情况下，带着 #8138 所描述的写入错位合入。
修复：新增一个 qualified 路由测试，让 runtime.bridge.listWorkspaceSessions 返回带有存在 worktree.path 的会话，断言 loadSettings/persistSetting 收到 worktree 路径。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect( | ||
| (lastSessionMock as Record<string, unknown>)?.['worktreeCwd'], | ||
| ).toBe(canonicalTargetDir); |
There was a problem hiding this comment.
[Suggestion] The new tests cannot establish real worktree state with plain mkdtemp/os.tmpdir() fixtures — production resolution can never classify those targets as worktrees. Probe-verified: after the missing-findGitRoot mock export is fixed, 4 tests still fail on fixture shape — this assertion (session/cd sets worktreeCwd…), resolve against worktree cwd set by createAndStoreSession both phases (~9918/9941), the pre-close phase of fall back to process.cwd() after worktree session closes (~9966), and resolves per-session worktreeCwd via sessionId param (~10003): all assert loadSettings(os.tmpdir()) / worktreeCwd = <tmpdir>, which resolveSettingsCwd can never return. Additionally 2 tests pass vacuously: the worktreeCwd dir is deleted tests (~9930-9961, ~9964) can never set worktreeCwd either, so the existsSync guards at acpAgent.ts:7871/7882 have zero effective coverage (probes also showed the mockExistsSync knob never reaches acpAgent.ts — it calls the real fs.existsSync). — Concrete cost: once the author patches the mock export, these tests either stay red or get "fixed" in a way that bypasses the containment logic entirely — leaving the #8138 mis-routing path without real coverage.
| expect( | |
| (lastSessionMock as Record<string, unknown>)?.['worktreeCwd'], | |
| ).toBe(canonicalTargetDir); | |
| expect( | |
| (lastSessionMock as Record<string, unknown>)?.['worktreeCwd'], | |
| ).toBe(canonicalTargetDir); |
(build worktree-shaped fixtures — <repo>/.qwen/worktrees/<slug> with findGitRoot mocked to return <repo>, mirroring acpAgent.worktree.test.ts — or inject worktreeCwd/getActiveWorktree directly into the session mock)
中文说明
[Suggestion] 新测试无法用普通 mkdtemp/os.tmpdir() fixture 建立真实 worktree 状态——生产解析永远不会把这些目标识别为 worktree。已探针验证：修好缺失的 findGitRoot mock 导出后，仍有 4 个测试因 fixture 形状失败——本断言（session/cd sets worktreeCwd…）、resolve against worktree cwd set by createAndStoreSession 两个阶段（约 9918/9941）、fall back to process.cwd() after worktree session closes 的关闭前阶段（约 9966）、resolves per-session worktreeCwd via sessionId param（约 10003）：都断言 loadSettings(os.tmpdir()) / worktreeCwd = <tmpdir>，而 resolveSettingsCwd 永远不可能返回它。另外 2 个测试空转通过：worktreeCwd dir is deleted 测试（约 9930-9961、9964）同样无法设置 worktreeCwd，于是 acpAgent.ts:7871/7882 的 existsSync 守卫零有效覆盖（探针还表明 mockExistsSync 旋钮根本到不了 acpAgent.ts——它调用的是真实 fs.existsSync）。— 具体代价：作者修好 mock 导出后，这些测试要么继续红，要么以完全绕过包含逻辑的方式被"修好"——使 #8138 的错位路径失去真实覆盖。
修复：构造 worktree 形状的 fixture（<repo>/.qwen/worktrees/<slug>，findGitRoot mock 返回 <repo>，比照 acpAgent.worktree.test.ts），或向会话 mock 直接注入 worktreeCwd/getActiveWorktree。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const candidateRoots = [normalizedBound]; | ||
| const repoTop = findGitRoot(normalizedBound); |
There was a problem hiding this comment.
[Suggestion] The repo-top containment branch — explicitly documented as supporting monorepo subdirectory workspaces (the R2-4 fix) — is never exercised by any of the 8 unit tests: worktree-workspace.test.ts never mocks findGitRoot, and with the fake non-existent workspace /repo/project the real findGitRoot returns null, so candidateRoots is always just [normalizedBound]. Probe-verified: deleting the repoTop push leaves 8/8 green. — Concrete cost: a regression breaking monorepo resolution (boundWorkspace = /repo/monorepo/pkg, session worktree under /repo/.qwen/worktrees/feat) ships with every test green — settings/context writes silently land in the project root, the original #8138 symptom.
| const candidateRoots = [normalizedBound]; | |
| const repoTop = findGitRoot(normalizedBound); | |
| const candidateRoots = [normalizedBound]; | |
| const repoTop = findGitRoot(normalizedBound); |
(add a test with findGitRoot mocked to return a repo root different from boundWorkspace, and a session whose worktree.path sits under <repoRoot>/.qwen/worktrees/, asserting the worktree is returned)
中文说明
[Suggestion] 仓库顶层包含分支——明确注释为支持 monorepo 子目录 workspace（R2-4 的修复）——在全部 8 个单测中从未被执行：worktree-workspace.test.ts 从不 mock findGitRoot，而伪造的不存在 workspace /repo/project 让真实 findGitRoot 返回 null，于是 candidateRoots 恒为 [normalizedBound]。已探针验证：删掉 repoTop 推入后 8/8 仍全绿。— 具体代价：破坏 monorepo 解析的回归（boundWorkspace = /repo/monorepo/pkg，会话 worktree 在 /repo/.qwen/worktrees/feat 下）会在所有测试绿色时合入——设置/上下文写入静默落在项目根，即 #8138 的原始症状。
修复：新增测试：findGitRoot mock 返回不同于 boundWorkspace 的仓库根，会话的 worktree.path 位于 <repoRoot>/.qwen/worktrees/ 下，断言返回该 worktree。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| * Read by `removeStoredSessionEntry` to clear the agent-level | ||
| * `defaultSettingsCwd` when the owning session closes. |
There was a problem hiding this comment.
[Suggestion] This doc comment describes a lifecycle that does not exist at this commit: removeStoredSessionEntry (acpAgent.ts:4172-4219) never reads worktreeCwd, and defaultSettingsCwd has zero occurrences anywhere in the codebase — the field was removed in this revision while the comment survived. — Concrete cost: a maintainer debugging stale-worktree settings resolution (this PR's history includes three rounds of exactly that bug class) follows the comment to a clearing step and a symbol that do not exist; the real mechanism is just this.sessions.delete(sessionId) in removeStoredSessionEntry, after which the resolveSettingsCwd scan loops stop seeing the session.
| * Read by `removeStoredSessionEntry` to clear the agent-level | |
| * `defaultSettingsCwd` when the owning session closes. | |
| * Read by `removeStoredSessionEntry` to clear the agent-level | |
| * `defaultSettingsCwd` when the owning session closes. |
(reword to the real lifecycle, e.g. "Set by acpAgent on session creation, session/cd, and worktree restore; read by resolveSettingsCwd; cleared implicitly when the session is removed from the agent's session map.")
中文说明
[Suggestion] 该文档注释描述的生命周期在本提交中不存在：removeStoredSessionEntry（acpAgent.ts:4172-4219）从不读取 worktreeCwd，而 defaultSettingsCwd 在整个代码库中出现零次——该字段在本轮修订中已被移除，注释却留了下来。— 具体代价：调试 worktree 设置过期解析的维护者（本 PR 的历史包含三轮恰恰这类 bug）会按注释去找一个不存在的清理步骤和符号；真实机制只是 removeStoredSessionEntry 中的 this.sessions.delete(sessionId)，之后 resolveSettingsCwd 的扫描循环就不再看到该会话。
修复：按真实生命周期改写，例如"由 acpAgent 在会话创建、session/cd 与 worktree 恢复时设置；由 resolveSettingsCwd 读取；会话从代理会话表中移除时隐式清除。"
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const { findGitRoot } = await import('@qwen-code/qwen-code-core'); | ||
| vi.mocked(findGitRoot).mockReturnValue('/repo'); |
There was a problem hiding this comment.
[Suggestion] The VP4 test mocks findGitRoot to return the parent repo ('/repo') for a worktree path — a value production findGitRoot can never return there: it stops at the worktree's own .git gitfile (verified on a real git worktree add fixture: findGitRoot(W) = W, isWorktreePath(W) = false, while under this mock contract isWorktreePath = true). — Concrete cost: the test passes while the code path it covers never fires in production (the isWorktreePath finding above): it neither catches the live defect nor validates any future fix of it.
| const { findGitRoot } = await import('@qwen-code/qwen-code-core'); | |
| vi.mocked(findGitRoot).mockReturnValue('/repo'); | |
| const { findGitRoot } = await import('@qwen-code/qwen-code-core'); | |
| vi.mocked(findGitRoot).mockReturnValue('/repo'); |
(mock findGitRoot to honor the production contract — return the worktree path itself when a .git entry exists there — or build the test on a real git worktree add fixture)
中文说明
[Suggestion] VP4 测试把 findGitRoot mock 成对 worktree 路径返回父仓库（'/repo'）——这是生产 findGitRoot 在那里永远不可能返回的值：它停在 worktree 自身的 .git gitfile 上（已在真实 git worktree add fixture 上验证：findGitRoot(W) = W、isWorktreePath(W) = false，而按此 mock 约定 isWorktreePath = true）。— 具体代价：测试通过，但它覆盖的代码路径在生产中从不触发（见上文 isWorktreePath 发现）：既抓不到现网缺陷，也无法验证对它的任何未来修复。
修复：让 findGitRoot 的 mock 遵守生产约定（当 .git 条目存在时返回 worktree 路径本身），或把测试建立在真实 git worktree add fixture 上。
— qwen3.8-max via Qwen Code /review (v0.21.9)
…solation # Conflicts: # packages/cli/src/acp-integration/acpAgent.test.ts # packages/cli/src/serve/run-qwen-serve.ts
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution for #8152Root cause. Two main-side changes collided with this PR:
Textual or semantic. Both are positional overlaps of independent changes — no logic was modified by both sides. Resolution keeps both changes in full: fileSystem: createBridgeFileSystemAdapter(fsFactory, {
allowSameHostToolWritesOutsideWorkspace: deps.fsFactory === undefined,
}),
persistApprovalMode: (workspace, mode) => {
const effective = findEffectiveWorkspace(bridge, workspace);
return withSettingsLock(effective, async () => { ... });
},( What is load-bearing.
What I could not verify. No build/typecheck/tests were run. One behavior coupling worth CI's attention: main's new test 中文说明冲突根因：main 上 #8852 给三处 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and did not run locally; R3-11 predicts the macOS leg fails on the two new exit-worktree tests.
Not reviewed: reverse audit — reached the 5-round cap without two consecutive dry rounds; round-5 reports were fully adjudicated (one absorbed into R3-2, one rejected by the bounded-tail verifier).
Not explored to full depth (tool budget reached): PR #8152 adds worktree-aware resolution of workspace sett...: did not run the full unfiltered acpAgent.test.ts suite (402 tests) end-to-end, so I cannot state the exact total failure count beyond the worktree-filtered ru…; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all reads completed within budget..
[Critical] R3-3: The workspaceReload swap from in-place reloadScopeFromDisk(User/Workspace) to this.settings = loadSettings(settingsCwd) breaks reload semantics in three verified ways: (1) the two pre-existing tests (clears removed providerProtocol mappings and refreshes auth on workspace reload ~18216, clears Todo Stop Guard trust when workspace reload enters plan mode ~18300) drive their change via a reloadScopeFromDisk mock the handler no longer calls — verified still failing after the R3-1 mock fix (reloadModelProvidersConfig/setApprovalMode('plan') call count 0); (2) session-owned LoadedSettings instances stay stale (Session reads ui.enableFollowupSuggestions/ui.enableCacheSharing per turn from its own instance; buildDisabledSkillNamesProvider closes over it) while the response reports sessionsRefreshed; (3) oldMerged captured from this.settings can come from a different directory than newMerged (every settings handler rebinds this.settings to a request-scoped settingsCwd) — fabricating changedKeys that fire switchModel/refreshAuth/setApprovalMode/setDisabledTools on every idle session, or masking real on-disk changes. Failure scenario: plain session binds this.settings to the root; a later workspaceReload resolving to worktree W diffs root-vs-W with nothing changed on disk → W's settings applied to all idle sessions; the inverse masks real updates. Pre-diff, reloadScopeFromDisk guaranteed old and new came from the same directory. Fix: keep a disk-resync for the instances sessions hold; capture the baseline from the same directory being reloaded; rework the two orphaned tests onto the new mechanism. (Not re-anchored inline this round: its position overlaps the live R3-4 comment on the same line.)
中文说明
[Critical] R3-3：workspaceReload 从就地 reloadScopeFromDisk(User/Workspace) 换成 this.settings = loadSettings(settingsCwd)，以三种已验证的方式破坏 reload 语义：（1）两个既有测试（约 18216 的 clears removed providerProtocol mappings and refreshes auth on workspace reload、约 18300 的 clears Todo Stop Guard trust when workspace reload enters plan mode）通过处理器已不再调用的 reloadScopeFromDisk mock 驱动变更——已验证在修复 R3-1 的 mock 问题后仍然失败（reloadModelProvidersConfig/setApprovalMode('plan') 调用次数为 0）；（2）会话自持的 LoadedSettings 实例保持过期（Session 每轮从自己的实例读取 ui.enableFollowupSuggestions/ui.enableCacheSharing；buildDisabledSkillNamesProvider 闭包引用它），而响应却报告 sessionsRefreshed；（3）oldMerged 取自 this.settings，而每个 settings 处理器都会把 this.settings 重绑定到请求级 settingsCwd——基线可能与 newMerged 来自不同目录，凭空产生 changedKeys，对每个空闲会话触发 switchModel/refreshAuth/setApprovalMode/setDisabledTools；反之则掩盖真实的磁盘变更。失败场景：普通会话把 this.settings 绑定到根；随后解析到 worktree W 的 workspaceReload 在磁盘毫无变化时对 root 与 W 做 diff → W 的设置被应用到所有空闲会话；反向则掩盖真实更新。变更前 reloadScopeFromDisk 保证新旧来自同一目录。修复：为会话持有的实例保留磁盘重同步；从被 reload 的同一目录捕获基线；把两个被孤立的测试改造到新机制上。（本轮未重新锚定行内：其位置与同一行上存活的 R3-4 评论重叠。）
[Critical] R4-1 (new this round): The workspaceReload rewrite calls loadSettings(settingsCwd) without skipLoadEnvironment; loadSettings internally runs loadEnvironment (settings.ts:1062) in no-override mode and applies newly added .env/settings.env keys to process.env before reloadEnvironment computes its diff. The force-write loop (environment.ts:718-730) then sees process.env[key] === value and never adds the key to updatedKeys → envChanged=false → config.refreshAuth(authType) is never called for idle sessions. Pre-diff, reloadScopeFromDisk never touched process.env, so brand-new keys were detected. Probe-verified: both arms flip on exactly the one-line difference. Failure scenario: user adds DASHSCOPE_API_KEY via settings/.env and the settings panel issues qwen/control/workspace/reload; the key reaches process.env, but env.updatedKeys misreports and long-lived sessions keep auth state captured at start. Fix: pass { skipLoadEnvironment: true } to loadSettings here — the handler already applies env itself via reloadEnvironment immediately after. (Not re-anchored inline this round: its position overlaps a live pre-existing comment on the same line.)
中文说明
[Critical] R4-1（本轮新发现）：workspaceReload 重写调用 loadSettings(settingsCwd) 时未带 skipLoadEnvironment；loadSettings 内部以非覆盖模式运行 loadEnvironment（settings.ts:1062），在 reloadEnvironment 计算差量之前就把新增的 .env/settings.env 键应用到 process.env。强制写入循环（environment.ts:718-730）随后看到 process.env[key] === value，不会把该键加入 updatedKeys → envChanged=false → 空闲会话永远不会收到 config.refreshAuth(authType)。变更前 reloadScopeFromDisk 从不触碰 process.env，因此全新键能被检测到。已探针验证：两个 arm 恰在这一行之差上翻转。失败场景：用户经 settings/.env 添加 DASHSCOPE_API_KEY，设置面板发起 qwen/control/workspace/reload；键进入了 process.env，但 env.updatedKeys 误报，长期存活的会话保留启动时捕获的认证状态。修复：此处给 loadSettings 传 { skipLoadEnvironment: true }——处理器随后立即通过 reloadEnvironment 自行应用 env。（本轮未重新锚定行内：其位置与同一行上存活的既有评论重叠。）
[Critical] R3-24: Voice-settings writes were not converted to effective-workspace resolution: setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:739, 758 — not touched by the diff) and the REST route workspace-voice.ts:234-271 persists deps.boundWorkspace, while this same diff converts the settings GET routes (whose response includes the voice keys) and every sibling write (approval mode ×3 factories, tool/skill toggles, generic setting writes) to findEffectiveWorkspace. Secondary/dynamic factories force voiceSettingsScope: WORKSPACE_SETTING_SCOPE (run-qwen-serve.ts:4602, 5167). Failure scenario: workspace X with a REST-created/restored session inside worktree W — workspace/voice/set writes X/.qwen/settings.json; the worktree session's child loads W/.qwen/settings.json → the change never applies to the running session, and GET /workspace/settings (rewired to read W) returns the stale voiceModel right after the 200. Fix: route the voice-settings persistence through findEffectiveWorkspace like the sibling writers. Could not be anchored inline: the write site is unchanged code outside the diff.
中文说明
[Critical] R3-24：语音设置写入未转换为 effective-workspace 解析：setWorkspaceVoiceSettings 经 persistSettings(boundWorkspace, …) 持久化（workspace-service/index.ts:739、758——diff 未触碰），REST 路由 workspace-voice.ts:234-271 持久化 deps.boundWorkspace，而同一 diff 把设置 GET 路由（其响应包含语音键）与所有兄弟写入（审批模式 ×3 工厂、工具/技能开关、通用设置写入）都转到了 findEffectiveWorkspace。secondary/dynamic 工厂强制 voiceSettingsScope: WORKSPACE_SETTING_SCOPE（run-qwen-serve.ts:4602、5167）。失败场景：workspace X 有一个在 worktree W 内的 REST 创建/恢复会话——workspace/voice/set 写入 X/.qwen/settings.json；worktree 会话的子进程加载 W/.qwen/settings.json → 变更对运行中的会话永不生效，GET /workspace/settings（已改读 W）在 200 之后返回过期的 voiceModel。修复：让语音设置持久化像兄弟写入一样经 findEffectiveWorkspace。无法锚定行内：写入点在 diff 之外的未改动代码中。
[Critical] R2-2: Still stands (re-checked against 2b2bec8): "Fixes #8138" remains overstated for the desktop client. The desktop Settings panel sends qwen/settings/* through a dedicated session-less shared ACP process: buildSharedAcpProcessKey never matches the session-hosting process (the session agent's key includes envOverrides = { CRAFT_WORKSPACE_PATH } — SessionManager.ts:4910 — while getQwenWorkspaceAcpOptions passes none), so in that process this.sessions is empty and resolveSettingsCwd returns process.cwd(). The diff's own TODO(#8138) (acpAgent.ts:7885-7893) concedes this; qwenSettingsCwd still returns appRootPath; the PR body even mis-describes the state ("desktop worktreeRootPath is declared and read" — at this commit it is neither; both sites carry TODO comments only). Failure scenario: the issue's own repro — session runs enter_worktree, user changes the model in the desktop Settings panel → the write lands in <projectRoot>/.qwen/settings.json, not the worktree's; #8138 closes unfixed for its primary desktop surface. Fix direction: complete the desktop wiring (populate worktreeRootPath per the TODO, or route the panel through the session-hosting process), or rescope to "Partially addresses #8138" with a follow-up.
中文说明
[Critical] R2-2：仍然成立（已对 2b2bec8 复核）："Fixes #8138" 对桌面客户端仍言过其实。桌面设置面板经一个无会话的共享 ACP 进程发送 qwen/settings/*：buildSharedAcpProcessKey 永远不会与承载会话的进程匹配（会话代理的 key 包含 envOverrides = { CRAFT_WORKSPACE_PATH }（SessionManager.ts:4910），而 getQwenWorkspaceAcpOptions 不传任何 envOverrides），因此该进程中 this.sessions 为空、resolveSettingsCwd 返回 process.cwd()。diff 自己的 TODO(#8138)（acpAgent.ts:7885-7893）也承认这一点；qwenSettingsCwd 仍返回 appRootPath；PR 正文甚至描述了与事实不符的状态（"desktop worktreeRootPath is declared and read"——在本提交两者皆非；两处只有 TODO 注释）。失败场景：issue 自身的复现——会话运行 enter_worktree，用户在桌面设置面板更改模型 → 写入落在 <projectRoot>/.qwen/settings.json 而非 worktree；#8138 在其主要桌面场景上未修复即被关闭。修复方向：补全桌面接线（按 TODO 填充 worktreeRootPath，或让设置面板复用承载会话的进程），或将关闭引用调整为 "Partially addresses #8138" 并开后续 issue。
[Critical] R2-3: Still stands (re-checked against 2b2bec8): findEffectiveWorkspace only sees worktrees via BridgeSessionSummary.worktree, which the production bridge populates only at REST worktree-session creation and load/resume restore (routes/session.ts:2366 is the only setSessionWorktree wiring in packages/cli/src/serve). A worktree entered via the enter_worktree tool mid-session — the trigger issue #8138 explicitly names — never reaches the bridge (the tool only mutates in-child Config state), so GET/POST /workspace/settings and POST /workspace/init keep resolving against the project root for that session shape. The diff's TODO (worktree-workspace.ts:26-30) acknowledges it. Failure scenario: web-shell session bound to the project root; the model runs enter_worktree; the user changes a setting via the web shell → findEffectiveWorkspace sees no s.worktree on any summary → returns boundWorkspace → the write lands in <projectRoot>/.qwen/settings.json while the session works in the worktree. Fix direction: have the tool-driven path notify the bridge (call setSessionWorktree after enter_worktree / worktree-targeted session/cd), or document the limit and file a follow-up.
中文说明
[Critical] R2-3：仍然成立（已对 2b2bec8 复核）：findEffectiveWorkspace 只能通过 BridgeSessionSummary.worktree 看到 worktree，而生产 bridge 只在 REST 创建 worktree 会话与加载/恢复时填充该字段（routes/session.ts:2366 是 packages/cli/src/serve 中唯一的 setSessionWorktree 接线）。会话中途经 enter_worktree 工具进入的 worktree——issue #8138 明确点名的触发方式——永远到不了 bridge（该工具只修改子进程内的 Config 状态），因此对该会话形态，GET/POST /workspace/settings 与 POST /workspace/init 仍解析到项目根目录。diff 的 TODO（worktree-workspace.ts:26-30）也承认此缺口。失败场景：绑定项目根的 web-shell 会话；模型运行 enter_worktree；用户经 web shell 更改设置 → findEffectiveWorkspace 在任何 summary 上看不到 s.worktree → 返回 boundWorkspace → 写入落在 <projectRoot>/.qwen/settings.json，而会话实际工作在 worktree 中。修复方向：让工具路径通知 bridge（在 enter_worktree / 目标为 worktree 的 session/cd 之后调用 setSessionWorktree），或明确文档化此限制并开后续 issue。
[Critical] R4-10 (new this round): The daemon auth-install writer was missed by the effective-workspace conversion: installAuthProvider (run-qwen-serve.ts:5801-5846, serving POST /workspace/auth/provider via workspace-auth.ts:296-345) still does withSettingsLock(boundWorkspace) + loadSettingsForPersistence(boundWorkspace), while every sibling primary writer in this diff was converted. When X's file owns modelProviders the adapter's persist scope resolves to Workspace (modelProvidersScope.ts:28-30) → the plan's env.*, modelProviders.<authType>, security.auth.selectedType and model selection persist to X and the route returns 200 — but the worktree session loads W/.qwen/settings.json and every converted read surface resolves to W. Masking caveat ruled: env-key injection only partially masks env-key-only providers in newly spawned children — the running worktree child, modelProviders config, model selection and selectedType never propagate via env. Failure scenario: workspace X (trusted, owning workspace-level modelProviders) with a session in worktree W — user installs a provider via POST /workspace/auth/provider → everything persists to X and the API reports success, while the active worktree session keeps failing auth / using the old provider, and the settings panel reading W shows none of it. Fix: mirror the siblings — const effective = findEffectiveWorkspace(bridge, boundWorkspace); then withSettingsLock(effective, …) / loadSettingsForPersistence(effective). Could not be anchored inline: the write site (run-qwen-serve.ts:5801) is outside every diff hunk.
中文说明
[Critical] R4-10（本轮新发现）：守护进程认证安装写入器被 effective-workspace 转换遗漏：installAuthProvider（run-qwen-serve.ts:5801-5846，经 workspace-auth.ts:296-345 服务 POST /workspace/auth/provider）仍执行 withSettingsLock(boundWorkspace) + loadSettingsForPersistence(boundWorkspace)，而本 diff 转换了所有兄弟 primary 写入器。当 X 的文件拥有 modelProviders 时，适配器的持久化 scope 解析为 Workspace（modelProvidersScope.ts:28-30）→ 安装计划的 env.*、modelProviders.<authType>、security.auth.selectedType 与模型选择都持久化到 X，路由返回 200——但 worktree 会话加载 W/.qwen/settings.json，且所有已转换的读取面都解析到 W。掩蔽注意已裁定：env 键注入只能部分掩盖纯 env 键 provider（且仅对新拉起的子进程）——运行中的 worktree 子进程、modelProviders 配置、模型选择与 selectedType 从不经过 env 传播。失败场景：workspace X（受信任、拥有 workspace 级 modelProviders）有一个在 worktree W 内的会话——用户经 POST /workspace/auth/provider 安装 provider → 一切持久化到 X 且 API 报告成功，而活跃的 worktree 会话继续认证失败/使用旧 provider，读取 W 的设置面板什么都看不到。修复：比照兄弟——const effective = findEffectiveWorkspace(bridge, boundWorkspace); 然后 withSettingsLock(effective, …) / loadSettingsForPersistence(effective)。无法锚定行内：写入点（run-qwen-serve.ts:5801）在所有 diff hunk 之外。
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and did not run locally; R3-11 predicts the macOS leg fails on the two new exit-worktree tests。
未审查：reverse audit — reached the 5-round cap without two consecutive dry rounds; round-5 reports were fully adjudicated (one absorbed into R3-2, one rejected by the bounded-tail verifier)。
未探索到全部深度（达到工具调用预算）：PR #8152 adds worktree-aware resolution of workspace sett...：did not run the full unfiltered acpAgent.test.ts suite (402 tests) end-to-end, so I cannot state the exact total failure count beyond the worktree-filtered ru…；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all reads completed within budget.。
[Critical] R3-3: The workspaceReload swap from in-place reloadScopeFromDisk(User/Workspace) to this.settings = loadSettings(settingsCwd) breaks reload semantics in three verified ways: (1) the two pre-existing tests (clears removed providerProtocol mappings and refreshes auth on workspace reload ~18216, clears Todo Stop Guard trust when workspace reload enters plan mode ~18300) drive their change via a reloadScopeFromDisk mock the handler no longer calls — verified still failing after the R3-1 mock fix (reloadModelProvidersConfig/setApprovalMode('plan') call count 0); (2) session-owned LoadedSettings instances stay stale (Session reads ui.enableFollowupSuggestions/ui.enableCacheSharing per turn from its own instance; buildDisabledSkillNamesProvider closes over it) while the response reports sessionsRefreshed; (3) oldMerged captured from this.settings can come from a different directory than newMerged (every settings handler rebinds this.settings to a request-scoped settingsCwd) — fabricating changedKeys that fire switchModel/refreshAuth/setApprovalMode/setDisabledTools on every idle session, or masking real on-disk changes. Failure scenario: plain session binds this.settings to the root; a later workspaceReload resolving to worktree W diffs root-vs-W with nothing changed on disk → W's settings applied to all idle sessions; the inverse masks real updates. Pre-diff, reloadScopeFromDisk guaranteed old and new came from the same directory. Fix: keep a disk-resync for the instances sessions hold; capture the baseline from the same directory being reloaded; rework the two orphaned tests onto the new mechanism. (Not re-anchored inline this round: its position overlaps the live R3-4 comment on the same line.)
中文说明
[Critical] R3-3：workspaceReload 从就地 reloadScopeFromDisk(User/Workspace) 换成 this.settings = loadSettings(settingsCwd)，以三种已验证的方式破坏 reload 语义：（1）两个既有测试（约 18216 的 clears removed providerProtocol mappings and refreshes auth on workspace reload、约 18300 的 clears Todo Stop Guard trust when workspace reload enters plan mode）通过处理器已不再调用的 reloadScopeFromDisk mock 驱动变更——已验证在修复 R3-1 的 mock 问题后仍然失败（reloadModelProvidersConfig/setApprovalMode('plan') 调用次数为 0）；（2）会话自持的 LoadedSettings 实例保持过期（Session 每轮从自己的实例读取 ui.enableFollowupSuggestions/ui.enableCacheSharing；buildDisabledSkillNamesProvider 闭包引用它），而响应却报告 sessionsRefreshed；（3）oldMerged 取自 this.settings，而每个 settings 处理器都会把 this.settings 重绑定到请求级 settingsCwd——基线可能与 newMerged 来自不同目录，凭空产生 changedKeys，对每个空闲会话触发 switchModel/refreshAuth/setApprovalMode/setDisabledTools；反之则掩盖真实的磁盘变更。失败场景：普通会话把 this.settings 绑定到根；随后解析到 worktree W 的 workspaceReload 在磁盘毫无变化时对 root 与 W 做 diff → W 的设置被应用到所有空闲会话；反向则掩盖真实更新。变更前 reloadScopeFromDisk 保证新旧来自同一目录。修复：为会话持有的实例保留磁盘重同步；从被 reload 的同一目录捕获基线；把两个被孤立的测试改造到新机制上。（本轮未重新锚定行内：其位置与同一行上存活的 R3-4 评论重叠。）
[Critical] R4-1 (new this round): The workspaceReload rewrite calls loadSettings(settingsCwd) without skipLoadEnvironment; loadSettings internally runs loadEnvironment (settings.ts:1062) in no-override mode and applies newly added .env/settings.env keys to process.env before reloadEnvironment computes its diff. The force-write loop (environment.ts:718-730) then sees process.env[key] === value and never adds the key to updatedKeys → envChanged=false → config.refreshAuth(authType) is never called for idle sessions. Pre-diff, reloadScopeFromDisk never touched process.env, so brand-new keys were detected. Probe-verified: both arms flip on exactly the one-line difference. Failure scenario: user adds DASHSCOPE_API_KEY via settings/.env and the settings panel issues qwen/control/workspace/reload; the key reaches process.env, but env.updatedKeys misreports and long-lived sessions keep auth state captured at start. Fix: pass { skipLoadEnvironment: true } to loadSettings here — the handler already applies env itself via reloadEnvironment immediately after. (Not re-anchored inline this round: its position overlaps a live pre-existing comment on the same line.)
中文说明
[Critical] R4-1（本轮新发现）：workspaceReload 重写调用 loadSettings(settingsCwd) 时未带 skipLoadEnvironment；loadSettings 内部以非覆盖模式运行 loadEnvironment（settings.ts:1062），在 reloadEnvironment 计算差量之前就把新增的 .env/settings.env 键应用到 process.env。强制写入循环（environment.ts:718-730）随后看到 process.env[key] === value，不会把该键加入 updatedKeys → envChanged=false → 空闲会话永远不会收到 config.refreshAuth(authType)。变更前 reloadScopeFromDisk 从不触碰 process.env，因此全新键能被检测到。已探针验证：两个 arm 恰在这一行之差上翻转。失败场景：用户经 settings/.env 添加 DASHSCOPE_API_KEY，设置面板发起 qwen/control/workspace/reload；键进入了 process.env，但 env.updatedKeys 误报，长期存活的会话保留启动时捕获的认证状态。修复：此处给 loadSettings 传 { skipLoadEnvironment: true }——处理器随后立即通过 reloadEnvironment 自行应用 env。（本轮未重新锚定行内：其位置与同一行上存活的既有评论重叠。）
[Critical] R3-24: Voice-settings writes were not converted to effective-workspace resolution: setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:739, 758 — not touched by the diff) and the REST route workspace-voice.ts:234-271 persists deps.boundWorkspace, while this same diff converts the settings GET routes (whose response includes the voice keys) and every sibling write (approval mode ×3 factories, tool/skill toggles, generic setting writes) to findEffectiveWorkspace. Secondary/dynamic factories force voiceSettingsScope: WORKSPACE_SETTING_SCOPE (run-qwen-serve.ts:4602, 5167). Failure scenario: workspace X with a REST-created/restored session inside worktree W — workspace/voice/set writes X/.qwen/settings.json; the worktree session's child loads W/.qwen/settings.json → the change never applies to the running session, and GET /workspace/settings (rewired to read W) returns the stale voiceModel right after the 200. Fix: route the voice-settings persistence through findEffectiveWorkspace like the sibling writers. Could not be anchored inline: the write site is unchanged code outside the diff.
中文说明
[Critical] R3-24：语音设置写入未转换为 effective-workspace 解析：setWorkspaceVoiceSettings 经 persistSettings(boundWorkspace, …) 持久化（workspace-service/index.ts:739、758——diff 未触碰），REST 路由 workspace-voice.ts:234-271 持久化 deps.boundWorkspace，而同一 diff 把设置 GET 路由（其响应包含语音键）与所有兄弟写入（审批模式 ×3 工厂、工具/技能开关、通用设置写入）都转到了 findEffectiveWorkspace。secondary/dynamic 工厂强制 voiceSettingsScope: WORKSPACE_SETTING_SCOPE（run-qwen-serve.ts:4602、5167）。失败场景：workspace X 有一个在 worktree W 内的 REST 创建/恢复会话——workspace/voice/set 写入 X/.qwen/settings.json；worktree 会话的子进程加载 W/.qwen/settings.json → 变更对运行中的会话永不生效，GET /workspace/settings（已改读 W）在 200 之后返回过期的 voiceModel。修复：让语音设置持久化像兄弟写入一样经 findEffectiveWorkspace。无法锚定行内：写入点在 diff 之外的未改动代码中。
[Critical] R2-2: Still stands (re-checked against 2b2bec8): "Fixes #8138" remains overstated for the desktop client. The desktop Settings panel sends qwen/settings/* through a dedicated session-less shared ACP process: buildSharedAcpProcessKey never matches the session-hosting process (the session agent's key includes envOverrides = { CRAFT_WORKSPACE_PATH } — SessionManager.ts:4910 — while getQwenWorkspaceAcpOptions passes none), so in that process this.sessions is empty and resolveSettingsCwd returns process.cwd(). The diff's own TODO(#8138) (acpAgent.ts:7885-7893) concedes this; qwenSettingsCwd still returns appRootPath; the PR body even mis-describes the state ("desktop worktreeRootPath is declared and read" — at this commit it is neither; both sites carry TODO comments only). Failure scenario: the issue's own repro — session runs enter_worktree, user changes the model in the desktop Settings panel → the write lands in <projectRoot>/.qwen/settings.json, not the worktree's; #8138 closes unfixed for its primary desktop surface. Fix direction: complete the desktop wiring (populate worktreeRootPath per the TODO, or route the panel through the session-hosting process), or rescope to "Partially addresses #8138" with a follow-up.
中文说明
[Critical] R2-2：仍然成立（已对 2b2bec8 复核）："Fixes #8138" 对桌面客户端仍言过其实。桌面设置面板经一个无会话的共享 ACP 进程发送 qwen/settings/*：buildSharedAcpProcessKey 永远不会与承载会话的进程匹配（会话代理的 key 包含 envOverrides = { CRAFT_WORKSPACE_PATH }（SessionManager.ts:4910），而 getQwenWorkspaceAcpOptions 不传任何 envOverrides），因此该进程中 this.sessions 为空、resolveSettingsCwd 返回 process.cwd()。diff 自己的 TODO(#8138)（acpAgent.ts:7885-7893）也承认这一点；qwenSettingsCwd 仍返回 appRootPath；PR 正文甚至描述了与事实不符的状态（"desktop worktreeRootPath is declared and read"——在本提交两者皆非；两处只有 TODO 注释）。失败场景：issue 自身的复现——会话运行 enter_worktree，用户在桌面设置面板更改模型 → 写入落在 <projectRoot>/.qwen/settings.json 而非 worktree；#8138 在其主要桌面场景上未修复即被关闭。修复方向：补全桌面接线（按 TODO 填充 worktreeRootPath，或让设置面板复用承载会话的进程），或将关闭引用调整为 "Partially addresses #8138" 并开后续 issue。
[Critical] R2-3: Still stands (re-checked against 2b2bec8): findEffectiveWorkspace only sees worktrees via BridgeSessionSummary.worktree, which the production bridge populates only at REST worktree-session creation and load/resume restore (routes/session.ts:2366 is the only setSessionWorktree wiring in packages/cli/src/serve). A worktree entered via the enter_worktree tool mid-session — the trigger issue #8138 explicitly names — never reaches the bridge (the tool only mutates in-child Config state), so GET/POST /workspace/settings and POST /workspace/init keep resolving against the project root for that session shape. The diff's TODO (worktree-workspace.ts:26-30) acknowledges it. Failure scenario: web-shell session bound to the project root; the model runs enter_worktree; the user changes a setting via the web shell → findEffectiveWorkspace sees no s.worktree on any summary → returns boundWorkspace → the write lands in <projectRoot>/.qwen/settings.json while the session works in the worktree. Fix direction: have the tool-driven path notify the bridge (call setSessionWorktree after enter_worktree / worktree-targeted session/cd), or document the limit and file a follow-up.
中文说明
[Critical] R2-3：仍然成立（已对 2b2bec8 复核）：findEffectiveWorkspace 只能通过 BridgeSessionSummary.worktree 看到 worktree，而生产 bridge 只在 REST 创建 worktree 会话与加载/恢复时填充该字段（routes/session.ts:2366 是 packages/cli/src/serve 中唯一的 setSessionWorktree 接线）。会话中途经 enter_worktree 工具进入的 worktree——issue #8138 明确点名的触发方式——永远到不了 bridge（该工具只修改子进程内的 Config 状态），因此对该会话形态，GET/POST /workspace/settings 与 POST /workspace/init 仍解析到项目根目录。diff 的 TODO（worktree-workspace.ts:26-30）也承认此缺口。失败场景：绑定项目根的 web-shell 会话；模型运行 enter_worktree；用户经 web shell 更改设置 → findEffectiveWorkspace 在任何 summary 上看不到 s.worktree → 返回 boundWorkspace → 写入落在 <projectRoot>/.qwen/settings.json，而会话实际工作在 worktree 中。修复方向：让工具路径通知 bridge（在 enter_worktree / 目标为 worktree 的 session/cd 之后调用 setSessionWorktree），或明确文档化此限制并开后续 issue。
[Critical] R4-10 (new this round): The daemon auth-install writer was missed by the effective-workspace conversion: installAuthProvider (run-qwen-serve.ts:5801-5846, serving POST /workspace/auth/provider via workspace-auth.ts:296-345) still does withSettingsLock(boundWorkspace) + loadSettingsForPersistence(boundWorkspace), while every sibling primary writer in this diff was converted. When X's file owns modelProviders the adapter's persist scope resolves to Workspace (modelProvidersScope.ts:28-30) → the plan's env.*, modelProviders.<authType>, security.auth.selectedType and model selection persist to X and the route returns 200 — but the worktree session loads W/.qwen/settings.json and every converted read surface resolves to W. Masking caveat ruled: env-key injection only partially masks env-key-only providers in newly spawned children — the running worktree child, modelProviders config, model selection and selectedType never propagate via env. Failure scenario: workspace X (trusted, owning workspace-level modelProviders) with a session in worktree W — user installs a provider via POST /workspace/auth/provider → everything persists to X and the API reports success, while the active worktree session keeps failing auth / using the old provider, and the settings panel reading W shows none of it. Fix: mirror the siblings — const effective = findEffectiveWorkspace(bridge, boundWorkspace); then withSettingsLock(effective, …) / loadSettingsForPersistence(effective). Could not be anchored inline: the write site (run-qwen-serve.ts:5801) is outside every diff hunk.
中文说明
[Critical] R4-10（本轮新发现）：守护进程认证安装写入器被 effective-workspace 转换遗漏：installAuthProvider（run-qwen-serve.ts:5801-5846，经 workspace-auth.ts:296-345 服务 POST /workspace/auth/provider）仍执行 withSettingsLock(boundWorkspace) + loadSettingsForPersistence(boundWorkspace)，而本 diff 转换了所有兄弟 primary 写入器。当 X 的文件拥有 modelProviders 时，适配器的持久化 scope 解析为 Workspace（modelProvidersScope.ts:28-30）→ 安装计划的 env.*、modelProviders.<authType>、security.auth.selectedType 与模型选择都持久化到 X，路由返回 200——但 worktree 会话加载 W/.qwen/settings.json，且所有已转换的读取面都解析到 W。掩蔽注意已裁定：env 键注入只能部分掩盖纯 env 键 provider（且仅对新拉起的子进程）——运行中的 worktree 子进程、modelProviders 配置、模型选择与 selectedType 从不经过 env 传播。失败场景：workspace X（受信任、拥有 workspace 级 modelProviders）有一个在 worktree W 内的会话——用户经 POST /workspace/auth/provider 安装 provider → 一切持久化到 X 且 API 报告成功，而活跃的 worktree 会话继续认证失败/使用旧 provider，读取 W 的设置面板什么都看不到。修复：比照兄弟——const effective = findEffectiveWorkspace(bridge, boundWorkspace); 然后 withSettingsLock(effective, …) / loadSettingsForPersistence(effective)。无法锚定行内：写入点（run-qwen-serve.ts:5801）在所有 diff hunk 之外。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| type ToolInvocationGuard, | ||
| findGitRoot, | ||
| } from '@qwen-code/qwen-code-core'; |
There was a problem hiding this comment.
[Critical] R3-1: The new findGitRoot import is not declared in acpAgent.test.ts's vi.mock('@qwen-code/qwen-code-core', …) factory (line ~199 — manually enumerated exports, no importOriginal() spread), and createAndStoreSession now calls isWorktreePath() → findGitRoot() for every session whose getTargetDir() !== process.cwd(). Re-measured at this commit — still stands.
Failure scenario: npm test --workspace="packages/cli" → acpAgent.test.ts 175 failed | 227 passed (402), every failure [vitest] No "findGitRoot" export is defined on the "@qwen-code/qwen-code-core" mock — including all six tests this PR adds and pre-existing tests; the identical run passes on the merge base. The PR description's "All 108 tests in changed files pass" does not hold at this commit.
Fix (mirror acpAgent.worktree.test.ts); note this alone does not green the new tests — see the fixture-shape comment:
// in the vi.mock('@qwen-code/qwen-code-core', …) factory:
findGitRoot: vi.fn().mockReturnValue(null),中文说明
[Critical] R3-1：新增的 findGitRoot 导入未在 acpAgent.test.ts 的 vi.mock('@qwen-code/qwen-code-core', …) 工厂（约第 199 行——逐项列举导出、未展开 importOriginal()）中声明，而 createAndStoreSession 现在会对每个 getTargetDir() !== process.cwd() 的会话调用 isWorktreePath() → findGitRoot()。已在本提交重新测量——仍然成立。
失败场景：npm test --workspace="packages/cli" → acpAgent.test.ts 175 失败 | 227 通过（共 402），每个失败都是 [vitest] No "findGitRoot" export is defined on the "@qwen-code/qwen-code-core" mock——包括本 PR 新增的全部 6 个测试和既有测试；同样的运行在合并基线上通过。PR 描述中"All 108 tests in changed files pass"在本提交不成立。
修复（比照 acpAgent.worktree.test.ts）；注意仅此一项无法让新测试变绿——见关于 fixture 形状的评论：
// 在 vi.mock('@qwen-code/qwen-code-core', …) 工厂中：
findGitRoot: vi.fn().mockReturnValue(null),— qwen3.8-max via Qwen Code /review (v0.21.10)
| function isWorktreePath(target: string): boolean { | ||
| const repoRoot = findGitRoot(target); | ||
| if (!repoRoot) return false; |
There was a problem hiding this comment.
[Critical] R3-2: isWorktreePath can never return true for a real git worktree. findGitRoot (gitUtils.ts:100) walks up with existsSync and stops at the worktree's own .git gitfile (git worktree add places a gitfile, not a directory, at each worktree root), so it returns the worktree itself as the "repo root"; worktreesDir becomes <worktree>/.qwen/worktrees and the startsWith check fails. Re-probed at this commit against a real git worktree add: findGitRoot(W) = W, isWorktreePath(W) = false; the predicate flips when the root comes from git rev-parse --git-common-dir. Still stands.
Failure scenario: createAndStoreSession (~12210) and the session/cd handler (~10026) never set session.worktreeCwd for real worktrees → resolveSettingsCwd falls through to process.cwd() → settings resolve against the project root — issue #8138 remains unfixed for the create/cd flows (including subagent isolation:'worktree'). Additionally the daemon's post-restore changeSessionCwd INTO the worktree runs the else branch and wipes the worktreeCwd that #restoreWorktreeOnResume set one step earlier (the MUST-1/R2-17 test stops before the wipe).
Fix direction: compute the repo root via the gitfile/common dir — e.g. git rev-parse --git-common-dir, or resolve <wt>/.git's gitdir: pointer — before building worktreesDir; then extend MUST-1 to issue the daemon's post-load changeSessionCwd and re-assert.
中文说明
[Critical] R3-2：isWorktreePath 对真实 git worktree 永远返回 false。findGitRoot（gitUtils.ts:100）用 existsSync 向上查找，会停在 worktree 自身的 .git gitfile 上（git worktree add 在每个 worktree 根目录放置的是 gitfile 而非目录），于是把 worktree 本身当作"仓库根"返回；worktreesDir 变成 <worktree>/.qwen/worktrees，startsWith 检查必然失败。已在本提交用真实 git worktree add 重新探针验证：findGitRoot(W) = W、isWorktreePath(W) = false；当仓库根改由 git rev-parse --git-common-dir 计算时谓词翻转。仍然成立。
失败场景：createAndStoreSession（约 12210）与 session/cd 处理器（约 10026）永远不会为真实 worktree 设置 session.worktreeCwd → resolveSettingsCwd 回退到 process.cwd() → 设置解析到项目根目录——对创建/cd 链路（包括 subagent isolation:'worktree'），issue #8138 依旧未修复。此外守护进程恢复后"cd 进 worktree"（changeSessionCwd）会走 else 分支，抹掉上一步 #restoreWorktreeOnResume 刚设置的 worktreeCwd（MUST-1/R2-17 测试在抹除之前就停止了）。
修复方向：通过 gitfile/common dir 计算仓库根（如 git rev-parse --git-common-dir，或解析 <wt>/.git 的 gitdir: 指向），再构造 worktreesDir；随后扩展 MUST-1，使其发出守护进程加载后的 changeSessionCwd 并再次断言。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const resolvedWt = this.resolveSettingsCwd(params); | ||
| const settingsCwd = |
There was a problem hiding this comment.
[Critical] R3-4: settingsCwd's priority chain prefers the no-sessionId worktree scan over the caller's explicit cwd — but serve-control methods receive params.cwd as the target workspace. workspace-service's reload() invokes workspaceReload with { cwd: boundWorkspace } and no sessionId (workspace-service/index.ts:1466-1470); the scan returns the first session's worktree W and discards requestedCwd. Verified at this commit — still stands.
Failure scenario: session A resumed inside worktree W, plain session B at the workspace root; workspace-service issues workspaceReload {cwd: boundWorkspace} → settingsCwd = W → W's tools.approvalMode, tools.disabled, model, modelProviders applied to every idle session — B silently inherits the worktree's security posture — and reloadEnvironment(newMerged, settingsCwd) rebuilds the daemon child's env from W's .env/settings.env.
Additional unconverted consumers of the this.settings rebind verified this round: qwen/providers/connect (~8023) persists workspace-scope provider/auth config via createLoadedSettingsAdapter(this.settings, …) into whatever this.settings was last rebound to — after any worktree-resolving settings call, provider setup lands in W/.qwen/settings.json (never read from X, destroyed by exit_worktree action='remove'); and session/cd's folder-trust gate reads isFolderTrustEnabled(this.settings.merged) (~9953) — a plain session's cd trust decision can use W's security posture. A narrow fix of only workspaceReload leaves these consumers broken.
Fix direction: for serve-control methods whose params.cwd identifies the target workspace, prefer the explicit cwd (as pre-diff), or make resolveSettingsCwd not override a provided cwd param, or have workspace-service pass the owning sessionId; stop adopting request-scoped loads into the agent-global this.settings.
中文说明
[Critical] R3-4：settingsCwd 的优先级链让无 sessionId 的 worktree 扫描优先于调用方的显式 cwd——但 serve-control 方法收到的 params.cwd 就是目标 workspace。workspace-service 的 reload() 以 { cwd: boundWorkspace } 且不带 sessionId 调用 workspaceReload（workspace-service/index.ts:1466-1470）；扫描返回第一个会话的 worktree W 并丢弃 requestedCwd。已在本提交验证——仍然成立。
失败场景：会话 A 恢复在 worktree W 内，普通会话 B 在 workspace 根；workspace-service 发起 workspaceReload {cwd: boundWorkspace} → settingsCwd = W → W 的 tools.approvalMode、tools.disabled、model、modelProviders 被应用到每个空闲会话——B 静默继承 worktree 的安全姿态——且 reloadEnvironment(newMerged, settingsCwd) 用 W 的 .env/settings.env 重建守护子进程的环境变量。
本轮额外验证了两个未转换的 this.settings 重绑定消费者：qwen/providers/connect（约 8023）经 createLoadedSettingsAdapter(this.settings, …) 把 workspace 范围的 provider/auth 配置持久化到 this.settings 最近一次重绑定到的目录——任何解析到 worktree 的设置调用之后，provider 配置会落入 W/.qwen/settings.json（X 侧从不读取，且被 exit_worktree action='remove' 销毁）；session/cd 的文件夹信任门槛读取 isFolderTrustEnabled(this.settings.merged)（约 9953）——普通会话的 cd 信任判定可能使用 W 的安全姿态。仅修 workspaceReload 无法修复这些消费者。
修复方向：对 params.cwd 用于标识目标 workspace 的 serve-control 方法，优先使用显式 cwd（如此前），或让 resolveSettingsCwd 不覆盖已提供的 cwd 参数，或让 workspace-service 传入所属 sessionId；停止把请求级加载采纳进代理全局的 this.settings。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| } else { | ||
| session.worktreeCwd = null; | ||
| } |
There was a problem hiding this comment.
[Critical] R3-5: session/cd out of a worktree clears session.worktreeCwd but never clears the PR-introduced Config.activeWorktreePath; the per-session branch of resolveSettingsCwd then falls back to session.getConfig().getActiveWorktree?.(), so settings keep resolving against the worktree the session just left. Only exit_worktree ever calls setActiveWorktree(null); relocateWorkingDirectory never touches the field. Verified at this commit — still stands.
Failure scenario: a session resumed inside worktree W (#restoreWorktreeOnResume ~5278 sets both halves), then relocated via session/cd to a plain directory D → else branch nulls worktreeCwd, activeWorktreePath stays W → next qwen/settings/* with that sessionId finds configWt = W still on disk and returns W — settings are read from and persisted to the abandoned worktree instead of D; the sessionId-less scan is poisoned the same way.
| } else { | |
| session.worktreeCwd = null; | |
| } | |
| } else { | |
| session.worktreeCwd = null; | |
| session.getConfig().setActiveWorktree?.(null); | |
| } |
(and mirror setActiveWorktree(canonicalPath) in the if branch, consistent with enter_worktree and #restoreWorktreeOnResume)
中文说明
[Critical] R3-5：session/cd 离开 worktree 时清除了 session.worktreeCwd，却从不清除本 PR 引入的 Config.activeWorktreePath；resolveSettingsCwd 的按会话分支随后回退到 session.getConfig().getActiveWorktree?.()，于是设置继续解析到会话刚离开的 worktree。只有 exit_worktree 会调用 setActiveWorktree(null)；relocateWorkingDirectory 从不触碰该字段。已在本提交验证——仍然成立。
失败场景：会话恢复在 worktree W 内（#restoreWorktreeOnResume 约 5278 同时设置两部分），随后经 session/cd 迁到普通目录 D → else 分支把 worktreeCwd 置 null，activeWorktreePath 仍为 W → 下一次带该 sessionId 的 qwen/settings/* 发现 configWt = W 仍在磁盘上，返回 W——设置从被遗弃的 worktree 读取并写入，而不是 D；无 sessionId 的扫描同样被污染。
（并在 if 分支对应地 setActiveWorktree(canonicalPath)，与 enter_worktree 和 #restoreWorktreeOnResume 保持一致）
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const effective = findEffectiveWorkspace(bridge, workspace); | ||
| return withSettingsLock(effective, async () => { |
There was a problem hiding this comment.
[Critical] R3-9: The converted tool/skill toggle closures pass the resolved worktree path to loadSettingsForPersistence, whose trust lookup isWorkspaceTrustedForPersistence (~3787) keys on the workspace registry by exact cwd (byCwd.get(workspaceCwd), workspace-registry.ts:423) with a fallback requiring W === boundWorkspace — a worktree path satisfies neither, so trust = false and loadSettings runs with skipWorkspaceSettings: true: the worktree's own workspace scope is never loaded ({}). Probe-verified with flip (trusted arm preserves existing entries). Sibling persistApprovalMode converted in the same diff passes explicit workspaceTrusted: trustedWorkspace — the asymmetry is internal to this diff. Still stands.
Failure scenario: W/.qwen/settings.json holds tools.disabled: ["shell","web_fetch"]; user toggles edit off → persistDisabledTools(X, …) → effective = W → trust false → current = [] → next = ["edit"] → persisted, silently re-enabling shell and web_fetch in W — the disabled list is truncated to the single toggled entry; toggling two tools in sequence leaves only the last one disabled. persistDisabledSkillsFn loses skill entries the same way (and resolveSkillSettings(fresh) resolves against a merged view with the untrusted workspace scope filtered, skewing hard-lock detection).
Fix direction (mirror persistApprovalMode): derive trust from the original bound workspace argument — const trusted = isWorkspaceTrustedForPersistence(workspace); — then load effective with { skipLoadEnvironment: true, skipWorkspaceSettings: !trusted, workspaceTrusted: trusted }.
中文说明
[Critical] R3-9：已转换的工具/技能开关闭包把解析出的 worktree 路径传给 loadSettingsForPersistence，其信任查询 isWorkspaceTrustedForPersistence（约 3787）以精确 cwd 为键查注册表（byCwd.get(workspaceCwd)，workspace-registry.ts:423），回退分支要求 W === boundWorkspace——worktree 路径两者都不满足，于是 trust = false、loadSettings 以 skipWorkspaceSettings: true 运行：worktree 自身的 workspace scope 永不被加载（{}）。已探针验证并翻转（trusted 分支保留既有条目）。同一 diff 转换的兄弟 persistApprovalMode 显式传 workspaceTrusted: trustedWorkspace——不对称就在本 diff 内部。仍然成立。
失败场景：W/.qwen/settings.json 中有 tools.disabled: ["shell","web_fetch"]；用户关闭 edit → persistDisabledTools(X, …) → effective = W → trust false → current = [] → next = ["edit"] → 持久化，静默重新启用 shell 和 web_fetch——禁用列表被截断为单个被切换项；连续切换两个工具只会留下最后一个被禁用。persistDisabledSkillsFn 以同样方式丢失技能条目（且 resolveSkillSettings(fresh) 基于过滤掉未信任 workspace scope 的合并视图解析，使 hard-lock 检测失真）。
修复方向（比照 persistApprovalMode）：从原始 bound workspace 参数推导信任——const trusted = isWorkspaceTrustedForPersistence(workspace);——然后以 { skipLoadEnvironment: true, skipWorkspaceSettings: !trusted, workspaceTrusted: trusted } 加载 effective。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| resolveEffectiveWorkspace: () => | ||
| findEffectiveWorkspace(primaryBridge, primaryBoundWorkspace), |
There was a problem hiding this comment.
[Critical] R4-7 (new this round): The models surface was missed by the effective-workspace conversion: DELETE /workspace/models still reads and persists against boundWorkspace (workspace-models.ts:163, 255, 257 — unconverted; registerWorkspaceModelsRoutes at ~2251 gets no resolver), while this diff wires resolveEffectiveWorkspace into the settings routes here and — per R3-4 (confirmed) — workspaceReload applies W's model/modelProviders to sessions.
Failure scenario: workspace X with a session whose worktree W is visible (restored sidecar or enter_worktree — live now; REST-created once R3-2 is fixed): user deletes a model via Web Shell → the route loads X → a model configured only in W/.qwen/settings.json is not found → 404 model_not_found, or it is removed from X while W's copy keeps overriding the session; the tombstone loop writes clearing tombstones to X's scopes, so the deleted model stays selected for exactly the session it was deleted for.
Fix: resolve the effective workspace in the models routes the same way as the settings routes (pass a resolveEffectiveWorkspace dep and use it for both the loadSettings read and the persistSettings write).
中文说明
[Critical] R4-7（本轮新发现）：模型面被 effective-workspace 转换遗漏：DELETE /workspace/models 仍对 boundWorkspace 读写（workspace-models.ts:163、255、257——未转换；约 2251 处的 registerWorkspaceModelsRoutes 没有得到解析器），而本 diff 在此为设置路由接入了 resolveEffectiveWorkspace，且——按已确认的 R3-4——workspaceReload 会把 W 的 model/modelProviders 应用到会话。
失败场景：workspace X 有一个 worktree W 可见的会话（恢复的 sidecar 或 enter_worktree——当前即可触发；R3-2 修复后 REST 创建的也可以）：用户经 Web Shell 删除模型 → 路由加载 X → 只配置在 W/.qwen/settings.json 的模型找不到 → 404 model_not_found；或者从 X 删除了而 W 的副本继续覆盖会话；tombstone 循环把清除标记写入 X 的各 scope，被删除的模型对"正是为它而删"的那个会话仍然保持选中。
修复：比照设置路由，在模型路由中解析 effective workspace（传入 resolveEffectiveWorkspace 依赖，loadSettings 读取与 persistSettings 写入都使用它）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const effective = findEffectiveWorkspace( | ||
| runtime.bridge, | ||
| runtime.workspaceCwd, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-13: The workspace-qualified settings GET/POST routes now redirect reads/writes via findEffectiveWorkspace, but no test creates a worktree-session state for them — the qualified-rest harness's bridge mock returns listWorkspaceSessions: [], so the new calls always fall back to bound — and the five rewired persistence closures in run-qwen-serve.ts have no coverage (serve.test.ts mocks the whole module away). Still stands.
Concrete cost: if findEffectiveWorkspace misresolves (e.g. a stale session worktree entry pointing outside the workspace), settings are silently read from / persisted to the wrong tree and no test can catch it.
Fix: add tests for the qualified routes with a fake runtime.bridge whose listWorkspaceSessions returns a worktree session, asserting buildSettingsResponse/persistSetting receive the effective path; add at least one test per converted closure shape.
中文说明
[Suggestion] R3-13：workspace-qualified 设置 GET/POST 路由现在经 findEffectiveWorkspace 重定向读写，但没有任何测试为它们构造 worktree 会话状态——qualified-rest 测试框架的 bridge mock 返回 listWorkspaceSessions: []，新调用总是回退到 bound——且 run-qwen-serve.ts 中五个改线的持久化闭包没有覆盖（serve.test.ts 把整个模块 mock 掉了）。仍然成立。
具体代价：若 findEffectiveWorkspace 解析错误（如过期的会话 worktree 条目指向 workspace 之外），设置会被静默地从错误的树读取/持久化，而没有任何测试能捕获。
修复：用 listWorkspaceSessions 返回 worktree 会话的假 runtime.bridge 为 qualified 路由补测试，断言 buildSettingsResponse/persistSetting 收到 effective 路径；每种改线闭包至少补一个测试。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await agentPromise; | ||
| }); | ||
|
|
||
| it('session/cd sets worktreeCwd and getCore resolves against the worktree path', async () => { |
There was a problem hiding this comment.
[Suggestion] R3-14: The new tests cannot establish real worktree state with plain mkdtemp/os.tmpdir() fixtures — production resolution only sets worktreeCwd for paths under <gitRoot>/.qwen/worktrees/ (via isWorktreePath) or via getActiveWorktree()/sidecar restore. Probe-verified at this commit: after adding the missing findGitRoot mock export (R3-1), 4 tests still fail on fixture shape — this one, resolve against worktree cwd set by createAndStoreSession (both phases), fall back to process.cwd() after worktree session closes (pre-close phase), and resolves per-session worktreeCwd via sessionId param — and the 2 worktreeCwd dir is deleted tests pass vacuously, so the existsSync guards (acpAgent.ts:7902/7913) get zero effective coverage. Still stands.
Concrete cost: the PR's only test evidence for the #8138 fix can never exercise the isolation logic; once R3-1 is fixed these tests either stay red or get "fixed" in a way that bypasses the containment logic — leaving the mis-routing path without real coverage.
Fix: build worktree-shaped fixtures — <repo>/.qwen/worktrees/<slug> with findGitRoot mocked to return <repo> (mirroring acpAgent.worktree.test.ts) — or inject worktreeCwd/getActiveWorktree directly into the session mock.
中文说明
[Suggestion] R3-14：新测试无法用普通 mkdtemp/os.tmpdir() fixture 建立真实 worktree 状态——生产解析只会为 <gitRoot>/.qwen/worktrees/ 之下的路径（经 isWorktreePath）或经 getActiveWorktree()/sidecar 恢复设置 worktreeCwd。已在本提交探针验证：补上缺失的 findGitRoot mock 导出（R3-1）后，仍有 4 个测试因 fixture 形状失败——本测试、resolve against worktree cwd set by createAndStoreSession（两个阶段）、fall back to process.cwd() after worktree session closes（关闭前阶段）、resolves per-session worktreeCwd via sessionId param——且 2 个 worktreeCwd dir is deleted 测试空转通过，existsSync 守卫（acpAgent.ts:7902/7913）零有效覆盖。仍然成立。
具体代价：本 PR 对 #8138 修复的唯一测试证据永远无法执行隔离逻辑；R3-1 修复后这些测试要么继续红，要么以绕过包含逻辑的方式被"修好"——错位路径失去真实覆盖。
修复：构造 worktree 形状的 fixture——<repo>/.qwen/worktrees/<slug>，findGitRoot mock 返回 <repo>（比照 acpAgent.worktree.test.ts）——或向会话 mock 直接注入 worktreeCwd/getActiveWorktree。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| * Read by `removeStoredSessionEntry` to clear the agent-level | ||
| * `defaultSettingsCwd` when the owning session closes. |
There was a problem hiding this comment.
[Suggestion] R3-16: This doc comment describes a lifecycle that does not exist at this commit: removeStoredSessionEntry (acpAgent.ts:4184-4232) never reads worktreeCwd, and defaultSettingsCwd has zero occurrences anywhere in the codebase except this comment (repo-wide grep). Still stands.
Concrete cost: a maintainer tracing session-close cleanup hunts for a nonexistent clearing path or adds unnecessary code against a fabricated identifier; the comment also implies an intended close-time reset that was never written.
| * Read by `removeStoredSessionEntry` to clear the agent-level | |
| * `defaultSettingsCwd` when the owning session closes. | |
| * Read by `resolveSettingsCwd` to route `qwen/settings/*` handlers to this | |
| * session's worktree; cleared implicitly when the session is removed from | |
| * the agent's session map. |
中文说明
[Suggestion] R3-16：本文档注释描述了一个在本提交不存在的生命周期：removeStoredSessionEntry（acpAgent.ts:4184-4232）从不读取 worktreeCwd，defaultSettingsCwd 在整个代码库中除本注释外零出现（全仓库 grep）。仍然成立。
具体代价：追踪会话关闭清理的维护者会去寻找不存在的清理路径，或对着虚构的标识符添加不必要的代码；注释还暗示了一个从未写出的关闭时重置。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const prepared = prepareSettingWrite( | ||
| runtime.workspaceCwd, | ||
| effective, |
There was a problem hiding this comment.
[Suggestion] R4-8 (new this round): This qualified POST persists to effective, but its error path still logs runtime.workspaceCwd (~695), while the primary route's equivalent message was updated in this same diff to log effectiveWorkspace (~484) — the asymmetry misreports the file the write targeted. (effective is const-declared inside the try, so the catch cannot currently name it; the primary route avoids this only by resolving outside the try.) Distinct from the earlier open comment about the primary route's log — that one is fixed.
Concrete cost: worktree session active (effective = W ≠ runtime.workspaceCwd = X) and the persist fails (generation closes mid-write, disk error) → stderr prints the persist error with workspace=X even though the write targeted W/.qwen/settings.json; the operator inspects the wrong settings file while diagnosing.
Fix: hoist const effective = findEffectiveWorkspace(runtime.bridge, runtime.workspaceCwd) above the try, mirroring the primary route, and log effective in the catch.
中文说明
[Suggestion] R4-8（本轮新发现）：本 qualified POST 持久化到 effective，但其错误路径仍记录 runtime.workspaceCwd（约 695），而主路由的等价消息在同一 diff 中已更新为记录 effectiveWorkspace（约 484）——不对称会误报写入目标文件。（effective 在 try 内以 const 声明，catch 目前无法引用它；主路由只是因为在 try 外解析才避免了这一点。）与早先关于主路由日志的开放评论不同——那一条已修复。
具体代价：worktree 会话活跃（effective = W ≠ runtime.workspaceCwd = X）且持久化失败（写入中途生成关闭、磁盘错误）→ stderr 打印 workspace=X 的持久化错误，而实际写入目标是 W/.qwen/settings.json；运维诊断时会检查错误的设置文件。
修复：比照主路由，把 const effective = findEffectiveWorkspace(runtime.bridge, runtime.workspaceCwd) 提到 try 之上，并在 catch 中记录 effective。
— qwen3.8-max via Qwen Code /review (v0.21.10)
- Fix isWorktreePath to detect .git directory vs gitfile (cycle 3, 24 repeats) - Add setActiveWorktree clear on session/cd out of worktree (cycle 5, 28 repeats) - Convert persist closures to per-factory bridge with trust fix (cycles 6+7) - Fix qualified route lock key and error log to use effective workspace (cycles 9, 16) - Add skipLoadEnvironment to workspaceReload (cycle 17) - Add resolveContextFile to secondary/dynamic factories (cycle 10) - Update Session.ts doc comment (cycle 15) - Update test fixtures to use worktree-shaped directory structure
…o worktree-settings-isolation
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped (the packages/cli test phase could not run: the build fails — see R5-1).
Not explored to full depth (tool budget reached): chunk 6: none — all planned checks completed. Actually, checks I skipped: did not run the tests (review-only, static verification); did not verify per-package composite …; chunk 6: none — all planned checks completed within budget (tests not executed; static verification only).; PR #8152 (QwenLM/qwen-code): adds worktree-aware resoluti...: none — all checks above were completed within budget.; chunk 3: did not execute workspace-settings.test.ts / acpAgent.worktree.test.ts (worktree checkout has no installed node_modules; relied on code reading — CI will ru…; chunk 3: did not inspect the Web Shell/desktop client code that round-trips redacted mcpServers (Finding 1's client-side half is inferred from the redact/restore contrac…, and 4 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
[Critical] R5-2 (re-check of existing blocker comment 3752416550 — still stands at HEAD, probe-verified): the pre-existing makeRuntimeBridge() fake in run-qwen-serve.test.ts lacks listWorkspaceSessions, which findEffectiveWorkspace (worktree-workspace.ts:37) unconditionally calls; the pre-existing 'workspace skill settings persistence' tests fail at HEAD with TypeError: bridge.listWorkspaceSessions is not a function (2 tests). Already reported on this commit — not re-posted inline.
[Critical] R3-4 (still stands, re-checked at HEAD): settingsCwd's priority chain prefers the no-sessionId worktree scan over the caller's explicit cwd — workspace-service reload() invokes workspaceReload with { cwd: boundWorkspace } and no sessionId, and the scan silently redirects it to the first session's worktree (feeds R4-1's cross-session fan-out). Live thread at acpAgent.ts:7941.
[Critical] R3-11 (still stands, re-checked at HEAD): the exit-worktree clear-checks use strict === across heterogeneous path spellings (git-toplevel vs fs.realpath vs path.resolve) and the new tests construct both sides from the same raw mkdtemp string, so the divergence (symlinked ancestors on macOS, case-insensitive volumes) is untested and the clear silently fails. Live thread at exit-worktree.test.ts:316.
[Critical] R4-1 / R3-3 (still stands, re-checked at HEAD): the orphaned-tests half is addressed (both reload tests adapted), but session-owned LoadedSettings instances still stay stale while the response reports sessionsRefreshed, and oldMerged captured from the process-wide this.settings cache can come from a DIFFERENT directory than newMerged — fabricating changedKeys that fire switchModel/refreshAuth/setApprovalMode/setDisabledTools on every idle session, or masking real on-disk changes. Live thread at acpAgent.ts:11516.
[Critical] R4-4 (still stands, re-checked at HEAD): the REST permissions surface is split across two trees — this diff routes qwen/permissions/setRules writes through worktree-aware settingsCwd, while GET /workspace/permissions and the POST route's validation read still load boundWorkspace (workspace-permissions.ts, untouched by this diff). Live thread at acpAgent.ts:11478.
[Critical] R4-6 / R2-2 (still stands, re-checked at HEAD): 'Fixes #8138' remains overstated for the desktop client — the Settings panel talks to a session-less shared ACP process where this.sessions is empty and resolveSettingsCwd returns process.cwd(); qwenSettingsCwd is unchanged and the desktop diff is comment-only (TODO(#8138)). The issue's own repro (change the model in the panel after enter_worktree) still writes to the project root; merging closes #8138 with the desktop half unwired. Complete the desktop wiring or rescope to 'Partially addresses #8138'.
[Critical] R4-5 / R3-24 (still stands, re-checked at HEAD): voice-settings writes were not converted — setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:738-760, untouched) and workspace-voice.ts persists deps.boundWorkspace, while this same diff converts the settings GET routes (whose response includes the voice keys) and every sibling write → workspace/voice/set writes X while the worktree child loads W, and GET returns the stale voiceModel right after the 200.
[Critical] R4-7 (still stands, re-checked at HEAD): the models surface was missed by the effective-workspace conversion — DELETE /workspace/models and its persist path still read/write boundWorkspace (workspace-models.ts:163/255/257, untouched; no resolver wired), while the settings routes in the same file are converted. Live thread at server.ts:2194.
[Critical] R4-9 / R2-3 (still stands, re-checked at HEAD): findEffectiveWorkspace only sees REST-populated BridgeSessionSummary.worktree; worktrees entered mid-session via the enter_worktree tool — the trigger issue #8138 explicitly names — never reach the bridge (no setSessionWorktree wiring in packages/cli/src/serve), so GET/POST /workspace/settings, /workspace/init and the converted persist callbacks keep resolving against the project root for that session shape. The diff's own TODO in worktree-workspace.ts concedes it. Live thread at worktree-workspace.ts:48.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test legs were also skipped (the packages/cli test phase could not run: the build fails — see R5-1)。
未探索到全部深度（达到工具调用预算）：chunk 6：none — all planned checks completed. Actually, checks I skipped: did not run the tests (review-only, static verification); did not verify per-package composite …；chunk 6：none — all planned checks completed within budget (tests not executed; static verification only).；PR #8152 (QwenLM/qwen-code): adds worktree-aware resoluti...：none — all checks above were completed within budget.；chunk 3：did not execute workspace-settings.test.ts / acpAgent.worktree.test.ts (worktree checkout has no installed node_modules; relied on code reading — CI will ru…；chunk 3：did not inspect the Web Shell/desktop client code that round-trips redacted mcpServers (Finding 1's client-side half is inferred from the redact/restore contrac…，另有 4 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
[Critical] R5-2 (re-check of existing blocker comment 3752416550 — still stands at HEAD, probe-verified): the pre-existing makeRuntimeBridge() fake in run-qwen-serve.test.ts lacks listWorkspaceSessions, which findEffectiveWorkspace (worktree-workspace.ts:37) unconditionally calls; the pre-existing 'workspace skill settings persistence' tests fail at HEAD with TypeError: bridge.listWorkspaceSessions is not a function (2 tests). Already reported on this commit — not re-posted inline.
[Critical] R3-4 (still stands, re-checked at HEAD): settingsCwd's priority chain prefers the no-sessionId worktree scan over the caller's explicit cwd — workspace-service reload() invokes workspaceReload with { cwd: boundWorkspace } and no sessionId, and the scan silently redirects it to the first session's worktree (feeds R4-1's cross-session fan-out). Live thread at acpAgent.ts:7941.
[Critical] R3-11 (still stands, re-checked at HEAD): the exit-worktree clear-checks use strict === across heterogeneous path spellings (git-toplevel vs fs.realpath vs path.resolve) and the new tests construct both sides from the same raw mkdtemp string, so the divergence (symlinked ancestors on macOS, case-insensitive volumes) is untested and the clear silently fails. Live thread at exit-worktree.test.ts:316.
[Critical] R4-1 / R3-3 (still stands, re-checked at HEAD): the orphaned-tests half is addressed (both reload tests adapted), but session-owned LoadedSettings instances still stay stale while the response reports sessionsRefreshed, and oldMerged captured from the process-wide this.settings cache can come from a DIFFERENT directory than newMerged — fabricating changedKeys that fire switchModel/refreshAuth/setApprovalMode/setDisabledTools on every idle session, or masking real on-disk changes. Live thread at acpAgent.ts:11516.
[Critical] R4-4 (still stands, re-checked at HEAD): the REST permissions surface is split across two trees — this diff routes qwen/permissions/setRules writes through worktree-aware settingsCwd, while GET /workspace/permissions and the POST route's validation read still load boundWorkspace (workspace-permissions.ts, untouched by this diff). Live thread at acpAgent.ts:11478.
[Critical] R4-6 / R2-2 (still stands, re-checked at HEAD): 'Fixes #8138' remains overstated for the desktop client — the Settings panel talks to a session-less shared ACP process where this.sessions is empty and resolveSettingsCwd returns process.cwd(); qwenSettingsCwd is unchanged and the desktop diff is comment-only (TODO(#8138)). The issue's own repro (change the model in the panel after enter_worktree) still writes to the project root; merging closes #8138 with the desktop half unwired. Complete the desktop wiring or rescope to 'Partially addresses #8138'.
[Critical] R4-5 / R3-24 (still stands, re-checked at HEAD): voice-settings writes were not converted — setWorkspaceVoiceSettings persists via persistSettings(boundWorkspace, …) (workspace-service/index.ts:738-760, untouched) and workspace-voice.ts persists deps.boundWorkspace, while this same diff converts the settings GET routes (whose response includes the voice keys) and every sibling write → workspace/voice/set writes X while the worktree child loads W, and GET returns the stale voiceModel right after the 200.
[Critical] R4-7 (still stands, re-checked at HEAD): the models surface was missed by the effective-workspace conversion — DELETE /workspace/models and its persist path still read/write boundWorkspace (workspace-models.ts:163/255/257, untouched; no resolver wired), while the settings routes in the same file are converted. Live thread at server.ts:2194.
[Critical] R4-9 / R2-3 (still stands, re-checked at HEAD): findEffectiveWorkspace only sees REST-populated BridgeSessionSummary.worktree; worktrees entered mid-session via the enter_worktree tool — the trigger issue #8138 explicitly names — never reach the bridge (no setSessionWorktree wiring in packages/cli/src/serve), so GET/POST /workspace/settings, /workspace/init and the converted persist callbacks keep resolving against the project root for that session shape. The diff's own TODO in worktree-workspace.ts concedes it. Live thread at worktree-workspace.ts:48.
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
|
||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||
| vi.mocked(loadSettings).mockImplementation((...args) => { | ||
| if (args[1] && (args[1] as Record<string, unknown>).skipLoadEnvironment) { |
There was a problem hiding this comment.
[Critical] R5-1: PR-added test code accesses an index-signature property with dot notation, violating the repo's pre-existing noPropertyAccessFromIndexSignature: true. npm run build --workspace=packages/cli fails with TS4111 here and at line 18302 — the build gate is red as-is. — Failure scenario: any full build (npm run build, npm run preflight, CI) compiles packages/cli with tsc --build and emits error TS4111: Property 'skipLoadEnvironment' comes from an index signature, so it must be accessed with ['skipLoadEnvironment'] twice → build exits non-zero. Reproduced twice at HEAD; these are the only two type errors in the repo.
Fix (both lines 18221 and 18302):
if (args[1] && (args[1] as Record<string, unknown>)['skipLoadEnvironment']) {中文说明
[Critical] R5-1：本 PR 新增的测试代码用点号访问索引签名属性，违反仓库既有的 noPropertyAccessFromIndexSignature: true。npm run build --workspace=packages/cli 在此处与第 18302 行报 TS4111 失败——构建门禁当前为红。— 失败场景：任何完整构建（npm run build、npm run preflight、CI）用 tsc --build 编译 packages/cli 时会两次报出 error TS4111: Property 'skipLoadEnvironment' comes from an index signature, so it must be accessed with ['skipLoadEnvironment'] → 构建非零退出。已在 HEAD 复现两次；这是仓库中仅有的两个类型错误。
修复（第 18221 与 18302 两处）：
if (args[1] && (args[1] as Record<string, unknown>)['skipLoadEnvironment']) {— qwen3.8-max via Qwen Code /review (v0.21.10)
| persistDisabledSkills: createPersistDisabledSkillsFn(bridge), | ||
| persistDisabledSkillsBatch: persistDisabledSkillsBatchFn, |
There was a problem hiding this comment.
[Critical] R5-3: Batch skill-toggle persistence was not converted to worktree-aware resolution while its siblings were. createPersistDisabledToolsFn/createPersistDisabledSkillsFn now resolve findEffectiveWorkspace and lock/load/write the effective (worktree) path, but persistDisabledSkillsBatchFn (~3923) still locks/loads/writes the raw bound workspace, and it is wired unconverted at all four sites (~4206, ~4628, ~5200, ~5822). — Failure scenario: workspace with a REST-created/restored session inside worktree W → a single tool/skill toggle writes skills.disabled/skills.enabled to W/.qwen/settings.json, while a batch toggle (setWorkspaceSkillsEnabled → persistDisabledSkillsBatch(boundWorkspace)) writes to boundWorkspace/.qwen/settings.json: the running worktree session never sees batch changes, the two files accumulate split-brain skills state, and the lock keys diverge. Fix: convert the batch path the same way — createPersistDisabledSkillsBatchFn(localBridge: SessionLister) computing const effective = findEffectiveWorkspace(localBridge, workspace) and using withSettingsLock(effective, ...) + loadSettingsForPersistence(effective, workspace), wired at all four sites.
中文说明
[Critical] R5-3：批量技能开关持久化没有像其兄弟路径一样转换为 worktree 感知解析。createPersistDisabledToolsFn/createPersistDisabledSkillsFn 现在会解析 findEffectiveWorkspace 并对 effective（worktree）路径加锁/加载/写入，但 persistDisabledSkillsBatchFn（约 3923 行）仍对原始 bound workspace 加锁/加载/写入，且在全部四处接线点（约 4206、4628、5200、5822）原样接入。— 失败场景：某 workspace 有一个经 REST 创建/恢复、位于 worktree W 内的会话 → 单个工具/技能开关把 skills.disabled/skills.enabled 写入 W/.qwen/settings.json，而批量开关（setWorkspaceSkillsEnabled → persistDisabledSkillsBatch(boundWorkspace)）写入 boundWorkspace/.qwen/settings.json：运行中的 worktree 会话永远看不到批量变更，两个文件累积脑裂的技能状态，锁键也不一致。修复：以同样方式转换批量路径——createPersistDisabledSkillsBatchFn(localBridge: SessionLister)，内部计算 const effective = findEffectiveWorkspace(localBridge, workspace) 并使用 withSettingsLock(effective, ...) + loadSettingsForPersistence(effective, workspace)，在四处接线点全部接入。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| let publicValue: unknown = value; | ||
| const effectiveWorkspace = | ||
| resolveEffectiveWorkspace?.() ?? boundWorkspace; |
There was a problem hiding this comment.
[Critical] R5-4: The settings target is re-resolved from live session state on every request, breaking the GET→POST same-file invariant that MCP-secret redaction/restore depends on. Pre-diff, GET and POST both used the fixed boundWorkspace, so the redacted view the client received and the existing source used by restoreRedactedMcpServersSetting (in prepareSettingWrite) always came from the same file. The qualified routes have the same shape (~544, ~660). — Failure scenario: client GETs /workspace/settings (no worktree session yet → reads bound workspace, mcpServers secrets redacted to __redacted__); a worktree session is then created/restored; client POSTs the edited mcpServers back with unmodified placeholders; POST resolves to the worktree; existing = loadSettings(worktree)…mcpServers lacks those servers; restoreRecord (mcp-server-secrets.ts) silently drops every __redacted__ key with no prior string and sets oauth clientSecret to undefined → servers persisted with secrets stripped, POST returns 200. Ordinary states trigger it (a session created or closed while the settings dialog is open) — no microsecond race needed. Fix: pin the resolution across the round trip — restore redacted secrets against the same path buildSettingsResponse served (carry the resolved path in the GET response and echo it back, or fall back to the bound workspace's existing values when the effective one lacks the entry), or refuse the write (409) when resolution changed between read and write.
中文说明
[Critical] R5-4：设置目标在每次请求时都从实时会话状态重新解析，破坏了 MCP 密钥脱敏/还原所依赖的 GET→POST 同文件不变量。变更前 GET 与 POST 都使用固定的 boundWorkspace，客户端收到的脱敏视图与 prepareSettingWrite 中 restoreRedactedMcpServersSetting 使用的 existing 来源永远来自同一文件。带限定路由同样是这个形态（约 544、660）。— 失败场景：客户端 GET /workspace/settings（此时无 worktree 会话 → 读 bound workspace，mcpServers 密钥被脱敏为 __redacted__）；随后一个 worktree 会话被创建/恢复；客户端把带有未修改占位符的编辑后 mcpServers POST 回来；POST 解析到 worktree；existing = loadSettings(worktree)…mcpServers 中没有这些服务器；restoreRecord（mcp-server-secrets.ts）会静默丢弃所有没有 prior 字符串的 __redacted__ 键，并把 oauth clientSecret 置为 undefined → 服务器在密钥被剥掉的情况下被持久化，POST 却返回 200。普通状态即可触发（设置面板打开期间会话被创建或关闭）——不需要微秒级竞态。修复：在整个往返中钉住解析结果——从 buildSettingsResponse 提供读取的同一路径还原脱敏密钥（在 GET 响应中携带解析路径并要求回传，或当 effective 文件中缺少该条目时回退到 bound workspace 的既有值），或在读写之间解析发生变化时拒绝写入（409）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if (restored.session?.worktreePath) { | ||
| session.worktreeCwd = restored.session.worktreePath; | ||
| config.setActiveWorktree?.(restored.session.worktreePath); | ||
| } |
There was a problem hiding this comment.
[Critical] R5-5: A stale worktree sidecar resurrects worktreeCwd/activeWorktreePath on resume. session/cd OUT of a worktree clears the in-memory state (worktreeCwd, setActiveWorktree(null)) but never clears the persisted sidecar — there is no clearWorktreeSession/writeWorktreeSession call site anywhere in packages/cli/src/acp-integration. On the next loadSession/unstable_resumeSession this new restore hunk reads the stale sidecar (the directory still exists and passes containment; nothing compares the session's persisted cwd against it) and re-pins the session to the exited worktree — the restore runs AFTER createAndStoreSession's fresh isWorktreePath assignment and wins. Pre-diff, the restore only emitted a context notice. — Failure scenario: session enters worktree W (the serve route writes the sidecar) → user session/cds out to a regular dir B → session saved → resume: worktreeCwd = W and activeWorktreePath = W while cwd is B → every qwen/settings/* resolves against W and the model receives the "still in worktree W" notice. Materializes on direct ACP session/load (the desktop embedded backend does exactly this — qwen-agent.ts:3152/3226) and on the REST edges that skip re-relocation. Fix: in the session/cd handler's non-worktree branch also clear the sidecar (mirror exit_worktree's maybeClearWorktreeSession), or gate this restore on the session's persisted cwd still being under the restored worktree path.
中文说明
[Critical] R5-5：过期 worktree sidecar 会在 resume 时复活 worktreeCwd/activeWorktreePath。session/cd 离开 worktree 时只清除内存状态（worktreeCwd、setActiveWorktree(null)），从不清除持久化的 sidecar——packages/cli/src/acp-integration 中没有任何 clearWorktreeSession/writeWorktreeSession 调用点。下一次 loadSession/unstable_resumeSession 时，本新增恢复片段会读取过期 sidecar（目录仍存在且通过包含性校验；没有任何逻辑把会话持久化的 cwd 与它比较），把会话重新钉回已离开的 worktree——恢复逻辑在 createAndStoreSession 基于 isWorktreePath 的新赋值之后运行并覆盖它。变更前，恢复只发一条上下文提示。— 失败场景：会话进入 worktree W（serve 路由写入 sidecar）→ 用户 session/cd 离开到普通目录 B → 会话保存 → resume：cwd 在 B 而 worktreeCwd = W、activeWorktreePath = W → 每个 qwen/settings/* 都解析到 W，模型还会收到"仍在 worktree W"的提示。在直接 ACP session/load（桌面内嵌后端正是这样做——qwen-agent.ts:3152/3226）以及跳过重新定位的 REST 分支上都会出现。修复：在 session/cd 处理器的非 worktree 分支同时清除 sidecar（比照 exit_worktree 的 maybeClearWorktreeSession），或者在本恢复逻辑中校验会话持久化的 cwd 仍在被恢复的 worktree 路径之下。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| persistApprovalMode: (workspace, mode) => { | ||
| const effective = findEffectiveWorkspace(bridge, workspace); | ||
| return withSettingsLock(effective, async () => { |
There was a problem hiding this comment.
[Critical] R5-6: Approval-mode persistence was converted to write the effective worktree file, but the workspace-level approval-mode read was not — the write→read round trip is split across two files. All three persistApprovalMode closures (~4157, ~4572, ~5137) now write tools.approvalMode into the effective worktree, while getWorkspaceProvidersStatus → workspaceProvidersStatusProvider(boundWorkspace) → buildWorkspaceProvidersStatus → loadSettings(B) (workspace-providers-status.ts:63/141/232) still reads the bound workspace. tools.approvalMode is SECURITY_SENSITIVE, so the converted settings GET does not expose it — there is no alternative read. — Failure scenario: with a live worktree session, a persisted approval-mode change (daemon-MCP session_set_approval_mode tool, SDK setSessionApprovalMode, webui setApprovalMode({persist})) lands in W/.qwen/settings.json; every workspace providers-status read keeps loading B → webui/desktop show the stale mode (mapProviderStatus → connection.currentMode); the mode is not inherited by new sessions at B and is destroyed if the worktree is removed. Fix: resolve the effective workspace on the read side too — pass findEffectiveWorkspace(bridge, boundWorkspace) into the providers-status provider call in workspace-service, mirroring resolveEffectiveWorkspace in the settings routes.
中文说明
[Critical] R5-6：审批模式持久化已转换为写 effective worktree 文件，但 workspace 级审批模式读取没有转换——写→读往返被拆在两个文件上。三个 persistApprovalMode 闭包（约 4157、4572、5137）现在把 tools.approvalMode 写入 effective worktree，而 getWorkspaceProvidersStatus → workspaceProvidersStatusProvider(boundWorkspace) → buildWorkspaceProvidersStatus → loadSettings(B)（workspace-providers-status.ts:63/141/232）仍读 bound workspace。tools.approvalMode 属于 SECURITY_SENSITIVE，转换后的 settings GET 不会暴露它——没有替代读取路径。— 失败场景：存在活跃 worktree 会话时，一次带持久化的审批模式变更（daemon-MCP session_set_approval_mode 工具、SDK setSessionApprovalMode、webui setApprovalMode({persist})）落入 W/.qwen/settings.json；而所有 workspace providers-status 读取仍加载 B → webui/桌面显示过期模式（mapProviderStatus → connection.currentMode）；该模式不会被 B 上新建的会话继承，worktree 被删除时还会丢失。修复：读取侧也解析 effective workspace——在 workspace-service 的 providers-status provider 调用中传入 findEffectiveWorkspace(bridge, boundWorkspace)，比照设置路由中的 resolveEffectiveWorkspace。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const resolved = resolveContextFile | ||
| ? resolveContextFile(filename, boundWorkspace) |
There was a problem hiding this comment.
[Critical] R5-7: The context-file surface is now split across two trees. initWorkspace creates/validates QWEN.md in the effective worktree (resolveContextFile → findEffectiveWorkspace — the PR's stated design), but every daemon memory surface stays bound-anchored and was not converted: POST /workspace/memory writes with projectRoot: deps.boundWorkspace (workspace-memory.ts:307), GET /workspace/memory status reads collectStatus(deps.boundWorkspace) (:201 — discovery never lists the worktree QWEN.md), the qualified GET/POST read/write runtime.workspaceCwd (:362/:448), and the ACP-HTTP workspace/memory/write handler writes with projectRoot: this.boundWorkspace (dispatch.ts:3457). Same missed-conversion family as the already-accepted R4-5/R4-7/R4-10 findings. — Failure scenario: with a REST-created/restored worktree session, the user appends/replaces memory from the webui memory dialog or the SDK MCP workspaceWrite tool → bytes land in <bound>/QWEN.md while the session loads <worktree>/QWEN.md → the memory never enters the session's context; replace mode falsely reports success; GET status shows the root file while initWorkspace.result.path points at the worktree file. Pre-diff, init and memory agreed on boundWorkspace. Fix: route the memory surfaces through the same effective-workspace resolution (pass a resolveContextFile-equivalent into mountWorkspaceMemoryRoutes / the qualified mounts / dispatch.ts), or explicitly document and reject the divergence.
中文说明
[Critical] R5-7：上下文文件表面现在被拆在两棵树上。initWorkspace 在 effective worktree 中创建/校验 QWEN.md（resolveContextFile → findEffectiveWorkspace——本 PR 声明的设计），但所有 daemon memory 表面仍锚定 bound workspace 且未转换：POST /workspace/memory 以 projectRoot: deps.boundWorkspace 写入（workspace-memory.ts:307），GET /workspace/memory 状态读取 collectStatus(deps.boundWorkspace)（:201——发现逻辑永远不会列出 worktree 的 QWEN.md），带限定 GET/POST 读写 runtime.workspaceCwd（:362/:448），ACP-HTTP workspace/memory/write 处理器以 projectRoot: this.boundWorkspace 写入（dispatch.ts:3457）。与已被接受的 R4-5/R4-7/R4-10 同属漏转换家族。— 失败场景：存在 REST 创建/恢复的 worktree 会话时，用户从 webui memory 对话框或 SDK MCP workspaceWrite 工具追加/替换 memory → 字节落入 <bound>/QWEN.md，而会话加载的是 <worktree>/QWEN.md → memory 永远进不了会话上下文；replace 模式虚报成功；GET status 显示根目录文件而 initWorkspace.result.path 指向 worktree 文件。变更前 init 与 memory 都一致使用 boundWorkspace。修复：让 memory 表面走同样的 effective-workspace 解析（向 mountWorkspaceMemoryRoutes / 带限定挂载 / dispatch.ts 传入等价于 resolveContextFile 的解析器），或明确记录并拒绝这种分叉。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| clientMcpSenderRegistry, | ||
| persistDisabledTools: persistDisabledToolsFn, | ||
| persistDisabledSkills: persistDisabledSkillsFn, | ||
| persistDisabledTools: createPersistDisabledToolsFn(bridge), |
There was a problem hiding this comment.
[Critical] R4-10: Still stands (re-checked against HEAD 10dad438). The daemon auth-install writer was missed by the effective-workspace conversion: installAuthProvider (~5843) still locks/loads the bound workspace (loadSettingsForPersistence single-arg), while this same wiring block converts the tool/skill toggle factories around it to findEffectiveWorkspace. — Failure scenario: workspace with a session in worktree W — provider auth install (applyProviderInstallPlan) persists provider API keys/modelProviders into boundWorkspace/.qwen/settings.json while the session child loads W/.qwen/settings.json → the installed provider never reaches the running session, and the converted read surfaces show the worktree file lacking it. Fix: convert installAuthProvider the same way as its siblings (resolve findEffectiveWorkspace before lock/load).
中文说明
[Critical] R4-10：仍然成立（已对 HEAD 10dad438 复核）。daemon 认证安装写入器被 effective-workspace 转换遗漏：installAuthProvider（约 5843）仍对 bound workspace 加锁/加载（loadSettingsForPersistence 单参），而同一接线块中它周围的工具/技能开关工厂都已转换为 findEffectiveWorkspace。— 失败场景：workspace 有一个位于 worktree W 内的会话——provider 认证安装（applyProviderInstallPlan）把 provider API 密钥/modelProviders 持久化到 boundWorkspace/.qwen/settings.json，而会话子进程加载的是 W/.qwen/settings.json → 安装的 provider 永远到不了运行中的会话，转换过的读取表面也会显示 worktree 文件中缺少它。修复：像兄弟路径一样转换 installAuthProvider（在加锁/加载前解析 findEffectiveWorkspace）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const resolvedWt = this.resolveSettingsCwd(params); | ||
| const settingsCwd = | ||
| (resolvedWt !== process.cwd() ? resolvedWt : undefined) || |
There was a problem hiding this comment.
[Suggestion] R5-17: resolveSettingsCwd runs unconditionally before the ext-method switch, so every ext method — including polled status methods that never read settingsCwd — pays its filesystem cost. The session-less path is two loops over this.sessions with existsSync per session (up to 2N synchronous stats per call on the long-lived ACP child's event loop); workspaceResource polls at ~5s cadence, workspaceSkills on UI subscription. — Concrete cost: N sessions × ~2 stats × every poll, paid forever on long-lived children, for a value the handler never reads; any future throw inside the scan would also break methods unrelated to settings. Magnitude is small today (N typically 1-3), but the fix is one line: compute settingsCwd lazily inside the ~13 cases that need it (or gate the call on the settings/permissions method set).
中文说明
[Suggestion] R5-17：resolveSettingsCwd 在 ext-method switch 之前无条件运行，因此每个 ext 方法——包括从不读取 settingsCwd 的轮询状态方法——都要支付其文件系统开销。无 sessionId 路径是对 this.sessions 的两轮循环、逐会话 existsSync（在长驻 ACP 子进程的事件循环上每次调用最多 2N 次同步 stat）；workspaceResource 约 5 秒一轮，workspaceSkills 随 UI 订阅触发。— 具体代价：N 个会话 × 约 2 次 stat × 每次轮询，在长驻子进程上永久支付，只为一个处理器根本不读的值；扫描中未来任何抛错还会波及与设置无关的方法。当前量级很小（N 通常为 1-3），但修复只需一行：把 settingsCwd 的计算推迟到真正需要它的约 13 个 case 内部（或按设置/权限方法集合门控该调用）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if (!withinWorkspace) { | ||
| throw new WorkspaceInitPathEscapeError(filename, boundWorkspace); | ||
| throw new WorkspaceInitPathEscapeError(filename, effectiveWorkspace); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R5-18: The symlink-escape guard was re-anchored from boundWorkspace to the resolver's effectiveWorkspace, but findEffectiveWorkspace validates worktree containment only textually (path.normalize + startsWith + existsSync, which follows symlinks) — so the guard now checks the target against the same session-derived root on both sides and passes self-consistently even when that root resolves outside the bound workspace. Probe-verified: a repo committing .qwen/worktrees as a symlink to an outside directory passes git worktree add (the creation gate does NOT sanitize), and the guard flips — post-diff the QWEN.md write proceeds OUTSIDE realpath(boundWorkspace); pre-diff it threw WorkspaceInitSymlinkError. The sibling restore gate deliberately canonicalizes against server-derived roots. — Concrete cost: a malicious repo with a committed .qwen/worktrees symlink + a user starting a worktree session + POST /workspace/init → the context file is created/truncated outside the canonical bound workspace. Impact is bounded (one context file; needs a hostile repo), but this is reachable with the current create path — not only via the future writers the TODO announces. Fix: in findEffectiveWorkspace, realpath the session worktree path and compare against realpath of the candidate roots (dropping non-resolving candidates), as the restore gate does; or additionally assert realpath(effectiveWorkspace) is contained in realpath(boundWorkspace) here before writing.
中文说明
[Suggestion] R5-18：符号链接逃逸守卫从 boundWorkspace 重新锚定到解析器的 effectiveWorkspace，但 findEffectiveWorkspace 只做文本层面的包含性校验（path.normalize + startsWith + existsSync，且 existsSync 跟随符号链接）——于是守卫两侧都拿目标与同一个会话派生的根比较，即使该根解析到 bound workspace 之外也会自洽地通过。已探针验证：把 .qwen/worktrees 作为指向外部目录的符号链接提交进仓库后，git worktree add 能成功（创建关口不做净化），守卫行为翻转——变更后 QWEN.md 写入会落在 realpath(boundWorkspace) 之外；变更前会抛 WorkspaceInitSymlinkError。兄弟恢复关口则刻意对服务端派生的根做规范化。— 具体代价：恶意仓库提交 .qwen/worktrees 符号链接 + 用户启动 worktree 会话 + POST /workspace/init → 上下文文件被创建/截断在规范化的 bound workspace 之外。影响有限（只是一个上下文文件，且需要恶意仓库），但用当前创建路径即可触达——不只是 TODO 所预告的未来写入器才能触发。修复：在 findEffectiveWorkspace 中对会话 worktree 路径做 realpath 并与各候选根的 realpath 比较（丢弃无法解析的候选），如恢复关口所做；或在此处写入前额外断言 realpath(effectiveWorkspace) 包含于 realpath(boundWorkspace)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
5 participants
What this PR does
When a session operates inside a git worktree, workspace-scoped settings (
settings.json) and the context file (QWEN.md) were resolved against the project root instead of the worktree directory. File operations (read, write, shell) already worked correctly because they useconfig.getTargetDir(), but the settings and context-file paths were hardcoded to the daemon'sprocess.cwd()or the desktop client'sappRootPath.This PR adds worktree-aware resolution for both paths without modifying the core settings or workspace-service logic. The changes bolt onto the existing extension points: a new fallback in the ACP extension-method dispatcher, an optional resolver callback in the workspace service, and a new optional field on the desktop host-runtime context. Existing behavior is fully preserved when no worktree is involved — every new code path falls through to the original resolution when the worktree fields are unset.
Why it's needed
Users working inside a git worktree (created via
enter_worktreeor subagentisolation: "worktree") expect settings changes to land in the worktree's.qwen/settings.json, not the main project's. Without this fix, changing a model or toggling an MCP server inside a worktree silently writes to the project root, and the context file is always created relative to the project root. This makes worktree-based workflows unreliable for isolated experimentation.Fixes #8138
Reviewer Test Plan
How to verify
qwen servewith a worktree-bound session).cwd.<worktree>/.qwen/settings.json, not<projectRoot>/.qwen/settings.json.process.cwd()(no stale worktree path).POST /workspace/initwith aresolveContextFilethat points to a worktree subdirectory — confirm the file is created there.Unit tests cover the
resolveContextFileinjectable (facade.test.ts, 2 new tests) and theSession.worktreeCwdfield lifecycle (Session.worktree.test.ts, 1 new test). All 108 tests in changed files pass.Evidence (Before & After)
N/A — non-UI change (ACP/serve internals).
Tested on
Environment (optional)
npm run build && npm run typecheckon Windows. Unit tests vianpx vitest runinpackages/cli.Risk & Scope
defaultSettingsCwdonQwenAgentis agent-global state shared across sessions. It is reset when a regular session is created and when the owning worktree session closes, but two concurrent worktree sessions will last-write-wins. This is acceptable for the current single-focus-UI usage pattern; per-session resolution (via the already-presentSession.worktreeCwdfield) is the long-term path.worktreeRootPathis declared and read but not yet populated bybuildBackendHostRuntimeContext(marked withTODO(#8138)). The desktop-side wiring requires Electron host changes and is a follow-up. Integration tests for theextMethodInternalfallback chain require the fullrunAcpAgentmock harness and are deferred.Linked Issues
Fixes #8138
中文说明
本 PR 做了什么
当会话在 git worktree 内运行时，工作区设置（
settings.json）和上下文文件（QWEN.md）被解析到项目根目录而非 worktree 目录。文件操作（读、写、shell）已经正确工作（使用config.getTargetDir()），但设置和上下文文件路径被硬编码为守护进程的process.cwd()或桌面客户端的appRootPath。本 PR 为两个路径添加了 worktree 感知解析，且未修改核心设置或工作区服务逻辑。变更通过现有扩展点接入：ACP 扩展方法分发器中的新回退、工作区服务中的可选解析回调、以及桌面宿主运行时上下文上的新可选字段。当不涉及 worktree 时，现有行为完全保留——所有新代码路径在 worktree 字段未设置时回退到原始解析。
为什么需要
在 git worktree 内工作的用户（通过
enter_worktree或子代理isolation: "worktree"创建）期望设置变更写入 worktree 的.qwen/settings.json，而非主项目的。没有此修复，在 worktree 内更改模型或切换 MCP 服务器会静默写入项目根目录，上下文文件也始终相对于项目根目录创建。这使得基于 worktree 的工作流对于隔离实验不可靠。修复 #8138
审阅者测试计划
如何验证
qwen serve绑定 worktree 的会话）。cwd。<worktree>/.qwen/settings.json，而非<projectRoot>/.qwen/settings.json。process.cwd()（无残留 worktree 路径）。resolveContextFile调用POST /workspace/init——确认文件在该处创建。单元测试覆盖了
resolveContextFile可注入项（facade.test.ts，2 个新测试）和Session.worktreeCwd字段生命周期（Session.worktree.test.ts，1 个新测试）。变更文件中全部 108 个测试通过。证据（变更前/后）
N/A — 非 UI 变更（ACP/serve 内部）。
测试环境
环境（可选）
Windows 上
npm run build && npm run typecheck。单元测试通过packages/cli中的npx vitest run。风险与范围
QwenAgent上的defaultSettingsCwd是跨会话共享的代理全局状态。当创建常规会话时和所属 worktree 会话关闭时会重置，但两个并发 worktree 会话会最后写入生效。这对于当前单焦点 UI 使用模式是可接受的；按会话解析（通过已存在的Session.worktreeCwd字段）是长期方案。worktreeRootPath已声明和读取，但尚未由buildBackendHostRuntimeContext填充（标记为TODO(#8138)）。桌面端接线需要 Electron 宿主变更，属于后续工作。extMethodInternal回退链的集成测试需要完整的runAcpAgentmock 框架，已推迟。关联 Issue
修复 #8138
QwenCode QwenLLM