fix(cli): hide unavailable external editor option - #10746
Conversation
github-actions
Bot
added
the
review/self-reported
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical: this PR breaks the repo's own unit suite on any machine without VS Code installed (incl. all CI runners).
ToolConfirmationMessage.test.tsx (unchanged by this PR) contains the pre-existing test external editor option > should show "Modify with external editor" when preferredEditor is set (line 595), which sets preferredEditor: 'vscode' and asserts the option renders. It does NOT mock @qwen-code/qwen-code-core (only useLaunchEditor). With the gate changed to isEditorAvailable(preferredEditor), that test now depends on the host environment: isEditorAvailable('vscode') → checkHasEditorType → commandExists('code') (execSync('command -v code')), which is false on CI runners, so the option disappears and the test fails.
Reproduced locally at this head (62b1788): existing suite is 40/40 green at base; after applying only this PR's two hunks, exactly that 1 test fails — AssertionError: expected ... to contain 'Modify with external editor' (the select renders only options 1-3). Failure is deterministic anywhere code is not on PATH; it is masked only on dev machines that have VS Code installed. Note the new ToolConfirmationMessage.externalEditor.test.tsx passes because it explicitly mocks isEditorAvailable — the old file was not given the same treatment. The PR's product CI lanes have not run yet (triage ended early / action_required), so this is not yet visible in checks.
Fix direction: mock isEditorAvailable in ToolConfirmationMessage.test.tsx the same way the new test file does (or otherwise make the existing assertion environment-independent).
Critical：本 PR 会在任何未安装 VS Code 的机器上（包括所有 CI runner）打破仓库自身的单元测试。
ToolConfirmationMessage.test.tsx（本 PR 未修改）中已存在的测试 external editor option > should show "Modify with external editor" when preferredEditor is set（第 595 行）设置 preferredEditor: 'vscode' 并断言该选项出现。该文件没有 mock @qwen-code/qwen-code-core（只 mock 了 useLaunchEditor）。门槛改为 isEditorAvailable(preferredEditor) 后，该测试的结果取决于运行环境：isEditorAvailable('vscode') → checkHasEditorType → commandExists('code')（execSync('command -v code')），在 CI runner 上为 false，选项不再渲染，测试失败。
已在该 head（62b1788）本地复现：base 下 40/40 全绿；仅应用本 PR 的两处 hunk 后，恰好这 1 个测试失败。在没有 code 命令的机器上确定性失败，只在装了 VS Code 的开发机上被掩盖。新增的 ToolConfirmationMessage.externalEditor.test.tsx 之所以通过，是因为它显式 mock 了 isEditorAvailable——旧文件没有做同样处理。该 PR 的产品 CI lane 尚未运行（triage 提前结束），所以 checks 里还看不到。
修复方向：在 ToolConfirmationMessage.test.tsx 中以与新增测试文件相同的方式 mock isEditorAvailable（或让该断言与环境无关）。
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": running ToolConfirmationMessage.externalEditor.test.tsx / ToolConfirmationMessage.test.tsx to confirm they pass — the worktree and the parent checkout have ….
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /review |
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 explored to full depth (tool budget reached): "agent 1a": did not mutation-verify that the new "configured editor is unavailable" test goes RED when editorAvailable is reverted to the old preferredEditor gate (I ve…; "agent 1a": did not determine empirically whether the provider-less rerender surfaces a swallowed React uncaught error (no console output appeared with --silent=false , ….
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
|
@qwen-code /review |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": did not execute npx vitest run src/ui/components/messages/ToolConfirmationMessage.test.tsx myself — relied on R2-1's confirmed measurement (43 tests, 1 failed…; "agent reverse-audit (round 2)": did not run tsc --noEmit or ESLint over the two changed files — verified the new fixture and prop types by comparison against the identical pre-existing fixtu…; "agent 1c": could not execute ToolConfirmationMessage.test.tsx — the review worktree has no node_modules and no packages/core/dist , so the Critical's red/green state …; "agent 1d": none — but I could not execute the test to observe the failure directly: this review worktree has no node_modules and no packages/core/dist , so npx vitest ….
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
|
@qwen-code /review |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx:630 — [review] Rerender test hand-copies provider tree; use withProviderspackages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx:75 — [review] editorAvailable memo encodes full gate; hideModify checked twice
— qwen3.8-max via Qwen Code /review (v0.23.0)
Withdrawing my 2026-09-01 Critical — verified fixed at
|
| file | sha256 |
|---|---|
ToolConfirmationMessage.tsx |
f940c552a994f4aa24535e840b97960a55c7dd3288a4eadd4d747672c407eaa8 |
ToolConfirmationMessage.test.tsx |
f8d8b2472d15e46ab269752feeef68be7fe53e2e2e648ce0f808878b3e9bf20c |
Unit matrix, cd packages/cli && npx vitest run src/ui/components/messages/ToolConfirmationMessage.test.tsx:
| arm | product file | test file | result |
|---|---|---|---|
base (main 7a3b6cb6bd) |
main | main | 40 passed, exit 0 |
PR head b4fa9ccb6b |
head | head | 43 passed, exit 0 |
| M1 (mutant) | head | main, i.e. the isEditorAvailable mock removed |
1 failed | 39 passed, exit 1 |
M1 fails on exactly the test my Critical named, with exactly the assertion I reported:
× ToolConfirmationMessage > external editor option > should show "Modify with external editor" when preferredEditor is set
→ AssertionError: expected '\n ╭──────────────────────╮\n │ …' to contain 'Modify with external editor'
So the defect was real, the PR closes it, and the new mock is what closes it — removing it brings the failure straight back. This is not a false pass.
2. tmux E2E — real TUI, real build, A/B against main
I built the head tree (packages/cli compiled from the patched sources) and ran main's existing build as the control. Build witness, counted in the compiled dist/src/ui/components/messages/ToolConfirmationMessage.js:
- head build:
editorAvailable2×,isEditorAvailable2× - main build:
editorAvailable0×
Each arm boots the real TUI in tmux (200×50) against a local stub OpenAI-compatible server that returns a write_file tool call, so a genuine edit confirmation renders. The single variable is whether code resolves on PATH.
| arm | build | code on PATH |
options rendered | "Modify with external editor" |
|---|---|---|---|---|
| E1 | PR head | no | 3 | absent ✅ |
| E2 | PR head | yes (planted stub code) |
4 | present ✅ |
| E3 | main base | no | 4 | present ❌ ← the reported defect |
E3 is #10745 reproduced on main: the dialog offers an action that cannot be launched. E1 is the same environment at PR head, correctly hiding it. E2 proves the fix does not over-suppress — the option still appears when the editor really is available.
E1, as captured from the pane:
? WriteFile Writing to target.txt ←
1 hello from the stub model
Apply this change?
› 1. Yes, allow once
2. Yes, allow always
3. No, suggest changes (esc)
⠏ Waiting for user confirmation...
E2 and E3 render the same dialog with 3. Modify with external editor inserted before the cancel row (4 options).
Harness notes, so the result can be judged: env -i with an isolated HOME and QWEN_RUNTIME_DIR (verified afterwards — nothing was written into the real ~/.qwen), a pre-trusted workspace, and tools.approvalMode: "default" set explicitly because the repo default is now ApprovalMode.AUTO, under which the classifier would auto-approve and the dialog would never render. The dialog was dismissed with Escape; target.txt was not written in any arm.
I also checked whether the memo's two extra short-circuit clauses change behavior. Neither does:
confirmationDetails.type === 'edit'— the option push already sits insideif (confirmationDetails.type === 'edit'), so a non-edit confirmation never reached it at base either.!compactMode—renderedOptionsdiscards theoptionsarray entirely and builds a fixed set whencompactModeis true, so the editor option was never rendered in compact mode at base either. The clause only avoids a wasted synchronous probe, which is what the newshould NOT probe editor availability in compactModetest pins.
No Critical found. I am dismissing my stale CHANGES_REQUESTED (it was anchored at 62b17884d9) so it stops blocking a defect that no longer exists.
3. ⚠️ Not merge-ready yet: product CI has never executed on this head
I am deliberately not certifying this PR as mergeable, and the reason is not the code.
At b4fa9ccb6b these workflow runs are all completed / action_required, i.e. they were never allowed to start:
Qwen Code CI :: action_required
Security Checks :: action_required
Qwen Autofix :: action_required
tui-parity :: action_required
Comment Attachment Guard:: action_required (×2)
🧐 Qwen Pull Request Review :: action_required (×1)
The 12 check-runs that did complete are all triage/automation — 7 success, 5 skipped, 0 failure:
review-pr, precheck-pr / precheck, assign, label, authorize,
delay-automatic-review, Remind on force-push,
fallback-comment, publish-resolution, ack-review-request,
resolve-pr, review-config (the last five skipped)
Because those are green, the commit's statusCheckRollup reads SUCCESS. That rollup is automation-only. There is no Test (ubuntu/windows/macos), no Lint & Static, no Integration Tests, and no web-shell E2E Smoke result at this head — the lanes that would actually exercise the changed component and its test file have not run at all. This is the same condition I noted on 2026-09-01, and it is still true three head-commits later.
For contrast, #10902 at the same moment shows Qwen Code CI :: success across 35 runs, so this is specific to this PR needing a maintainer to approve the fork's workflow run.
Action needed from a maintainer: approve the pending workflow run on this head. My local measurements above cover the component and its suite, but they are not a substitute for the product lanes — in particular nothing upstream has yet run the full packages/cli suite with this change, which is precisely the risk my original Critical was about. Once those lanes are green I have no objection to this PR.
撤回我 2026-09-01 的 Critical —— 已在 b4fa9ccb6b 验证修复；但本 head 上产品 CI 从未运行
1. 我的 Critical 已修复，且修复是有效的。 head 的测试文件加上了我要求的那个 mock（vi.hoisted + vi.mock('@qwen-code/qwen-code-core', importOriginal)），独立的 ToolConfirmationMessage.externalEditor.test.tsx 也已合并进现有 suite。两个改动文件与 GitHub 上 b4fa9ccb6b 的 blob 逐字节一致。单元测试矩阵：base（main）40 passed / exit 0；PR head 43 passed / exit 0；变异体 M1（head 产品代码 + 去掉 mock 的 main 测试文件）1 failed | 39 passed / exit 1，失败的正是我当初点名的那个测试，断言信息也与我当初报告的完全一致。所以缺陷真实存在、本 PR 确实修好了、而且就是这个 mock 在起作用。
2. tmux E2E（真实 TUI + 真实构建 + 与 main 做 A/B）。 编译产物见证：head 构建里 editorAvailable 出现 2 次，main 构建里 0 次。每个 arm 在 tmux(200×50) 里启动真实 TUI，由本地 stub OpenAI 服务返回一个 write_file tool call，从而渲染出真实的编辑确认框；唯一变量是 code 是否在 PATH 上。结果：E1（head，无 code）3 个选项、不含"Modify with external editor"；E2（head，植入 code）4 个选项、包含；E3（main，无 code）4 个选项、包含 —— 这正是 #10745 在 main 上的复现。E3↔E1 证明修复生效，E2 证明没有过度抑制。另外我核对了 memo 新增的两个短路条件（type === 'edit'、!compactMode）：两者都不改变行为，选项 push 本就在 type === 'edit' 分支内，而 compactMode 为真时 renderedOptions 会整体丢弃 options 数组，所以 base 下紧凑模式本来也不会渲染该选项；这两个条件只是避免一次无用的同步探测。
3. b4fa9ccb6b 上，Qwen Code CI、Security Checks、Qwen Autofix、tui-parity、Comment Attachment Guard 等 run 全部是 completed / action_required，即从未被允许启动。真正跑完的 12 个 check-run 全是 triage/自动化（7 success / 5 skipped / 0 failure），因此该 commit 的 statusCheckRollup 显示 SUCCESS —— 但这个 rollup 只覆盖自动化，本 head 上没有任何 Test、Lint & Static、Integration Tests、web-shell E2E Smoke 结果。作为对照，同一时刻 #10902 是 Qwen Code CI :: success（共 35 个 run），所以这是本 PR（fork）需要 maintainer 批准 workflow 运行的问题。
需要 maintainer 处理： 批准该 head 上待运行的 workflow。我上面的本地测量覆盖了该组件及其测试，但不能替代产品 lane —— 尤其上游至今没有在这个改动下跑过完整的 packages/cli suite，而这恰好就是我当初那个 Critical 所针对的风险。那些 lane 变绿之后，我对本 PR 没有异议。
我已 dismiss 自己那条陈旧的 CHANGES_REQUESTED（锚定在 62b17884d9），以免它继续阻塞一个已不存在的缺陷。
Withdrawing my 2026-09-01 Critical: it is verified FIXED at head b4fa9cc. The test file now mocks isEditorAvailable via vi.hoisted + vi.mock('@qwen-code/qwen-code-core', importOriginal), which is exactly the fix direction I asked for. Evidence: base 40 passed / head 43 passed / mutant M1 (head product code + the unmocked main test file) reproduces my original failure byte-for-byte (1 failed | 39 passed, 'expected ... to contain Modify with external editor'), so the mock is load-bearing. A tmux A/B on real builds also confirms the behaviour: main offers 'Modify with external editor' when code is absent from PATH (3 options vs 4), PR head hides it, and still shows it when a code stub is on PATH. No Critical found. Full measurements in issuecomment-5545711481. NOTE: I am NOT certifying mergeability - product CI (Qwen Code CI / Security Checks / tui-parity) is still action_required at this head and has never executed; the green statusCheckRollup is triage automation only.
Local verification at
|
| file | sha256 |
|---|---|
ToolConfirmationMessage.tsx |
f940c552a994f4aa24535e840b97960a55c7dd3288a4eadd4d747672c407eaa8 |
ToolConfirmationMessage.test.tsx |
f8d8b2472d15e46ab269752feeef68be7fe53e2e2e648ce0f808878b3e9bf20c |
Two full bundles were built from source, PR head and merge-base 419e8d57b2. Build witness, read out of the emitted chunk that carries the option label:
PR head : ... !confirmationDetails.hideModify && (!config.getIdeMode() || !isDiffingEnabled) && editorAvailable) {
base : ... !confirmationDetails.hideModify && (!config.getIdeMode() || !isDiffingEnabled) && preferredEditor) {
1. Real TUI A/B — 9 arms
Each arm boots the real bundled CLI under the repo's own integration-tests/terminal-capture harness (node-pty + xterm.js in headless Chromium) against a scripted local OpenAI-compatible server that returns a write_file tool call, so a genuine edit confirmation renders. Isolated HOME/QWEN_RUNTIME_DIR, tools.approvalMode: "default" set explicitly (the repo default is now AUTO, under which the dialog never renders), and a synthetic PATH whose only editor binaries are the ones each arm plants.
| arm | build | preferredEditor |
editor on PATH |
SANDBOX |
option offered | options |
|---|---|---|---|---|---|---|
| A1 | base | vscode |
no | – | yes ❌ (#10745) | 4 |
| A2 | head | vscode |
no | – | no ✅ | 3 |
| A3 | head | vscode |
yes | – | yes ✅ | 4 |
| A4 | head | vscode |
yes | set | no ✅ | 3 |
| A5 | base | vscode |
yes | set | yes ❌ | 4 |
| A6 | head | sublime (invalid) |
– | – | no ✅ | 3 |
| A7 | base | sublime (invalid) |
– | – | yes ❌ | 4 |
| A8 | head | vim |
yes | – | yes ✅ | 4 |
| A9 | base | vscode |
no | – | yes, then selected | 4 |
A1↔A2 is the bug and the fix. A3 and A8 prove the gate does not over-suppress — the option still appears whenever the editor is genuinely launchable, for a GUI editor and for a terminal editor.
2. Two behaviour changes the PR body does not mention (both improvements)
isEditorAvailable() is isValidEditorType() && checkHasEditorType() && allowEditorTypeInSandbox(), so swapping it in for a truthiness check also closes two cases beyond "editor uninstalled":
- A6/A7 — an invalid
preferredEditorvalue."sublime"is not a supportedEditorType; the old gate accepted any non-empty string and offered a permanently dead action. Head hides it. - A4/A5 — sandbox mode. With
SANDBOXset, GUI editors cannot be launched, andallowEditorTypeInSandbox()says so. Head hides the option; base offers it even thoughcodeis onPATH. This matches whatusePreferredEditor()already does for theCtrl+Xcomposer editor, so it is consistent with existing repo policy — but it is a behaviour change and deserves a line in the PR description.
3. What the offered option actually did on base (A9)
I did not just check that the row disappears — I selected it on the base build. The scheduler's ModifyWithEditor branch only early-returns when getPreferredEditor() is falsy (AppContainer.tsx:2313 hands it the raw setting), so a stale "vscode" sails through to modifyWithEditor → openDiff → spawn:
x WriteFile {"file_path":".../ws/target.txt","content":"hello from the scripted model\n"}
spawn code ENOENT
The pending edit is cancelled, not just "the editor didn't open" — the user loses the turn and has to ask again. That is the concrete harm this PR removes.
4. Mutation matrix — every new guard is load-bearing and pinned
cd packages/cli && npx vitest run src/ui/components/messages/ToolConfirmationMessage.test.tsx, PR tests kept, product file mutated:
| mutant | result | test that flips RED |
|---|---|---|
| M0 control | 43 passed | – |
| M1 product file reverted to base | 2 failed | should show … when preferredEditor is set + should NOT show … when the configured editor is unavailable |
M2 drop !compactMode from the memo |
1 failed | should NOT probe editor availability in compactMode |
M3 drop type === 'edit' from the memo |
1 failed | should NOT probe editor availability for a non-edit confirmation |
M4 drop !hideModify from the memo |
1 failed | should NOT show … when hideModify is true |
M5 defeat memoization (Math.random() in deps) |
1 failed | should show … when preferredEditor is set |
Every clause the PR adds is killed by exactly one test, and no test is vacuous. M1 in particular is the RED/GREEN that had not been run before: reverting only the product file, keeping the new tests, flips exactly the two tests that describe the fix.
5. The memoization claim, measured at the syscall level
isEditorAvailable() ends in execSync("command -v code"), so I counted the actual subprocess spawns with strace -f -e trace=execve around a real confirmation dialog left on screen for 12 s:
| arm | /bin/sh -c "command -v code" spawns |
at boot | at dialog time |
|---|---|---|---|
| base | 1 | 1 | 0 |
| PR head (memoized) | 2 | 1 | 1 |
| PR head, memo defeated | 3 | 1 | 2 |
Three things follow, and I think they resolve the earlier performance concern in both directions:
- The PR costs exactly one extra synchronous probe per edit-confirmation dialog. Measured cost of one probe on this box: ~1.2 ms.
- The memo is real but modest: without it this scenario spends 2 probes instead of 1, not "one per Ink re-render" — Ink simply does not re-render this component that often. The PR body slightly oversells it.
- For scale: every arm, base included, already spawns 11 of these probes at startup (
code,codium,cursor,emacs,nvim,trae,vim,windsurf,zed,zeditor, plusrg). One more probe per dialog is well inside the noise of what boot already pays.
6. Static checks, full suites, and the red CI lane
npx eslint <both changed files> --max-warnings 0→ exit 0.packages/clitsc --noEmit→ exit 0.- IDE-diff behaviour is unchanged, verified directly rather than by reading: with
getIdeMode() === trueandisDiffingEnabled() === truethe option is hidden on both arms; with diffing disabled it is shown on both arms. - Full
packages/clisuite, both arms (28 588 tests). Head: 11 failures, of which 10 are identical on the base arm (environment: root-owned housekeeping tests, ink cursor rendering, serve) and the 11th (src/ui/opentui/dialogs-modes.test.tsx, an OpenTUI output-style dialog, unrelated to this change) passes 3/3 in isolation on both arms — a flake under a 4 480-suite parallel run. - The red
Test (ubuntu-latest, Node 22.x)lane on this head is not this PR. It fails onsrc/acp-integration/acpAgent.test.ts > QwenAgent runtime-root pinning choke point, an architectural guard asserting thatacpAgent.tsnever namesrunWithAcpRuntimeOutputDirdirectly. Line 9200 does exactly that, and it was introduced by feat(serve): add session turn navigation protocol #10751 (8a0a9c6614, merged intomainon 2026-09-04), which this branch inherited through itsmainmerge. I reproduced the identical failure locally on the merge-base419e8d57b2with this PR's changes absent, andmain's own CI run 33932327186 fails on the same test. Nothing to do here beyond waiting for themainfix.
7. Non-blocking observations
- The memo omits the IDE-diff clause. The render gate is
!hideModify && (!getIdeMode() || !isDiffingEnabled) && editorAvailable, but the memo covers only!compactMode,type === 'edit',!hideModify,preferredEditor !== undefined. Measured: underideMode + diffingthe head arm still callsisEditorAvailableonce (base: 0) for an option that can never be rendered. Cosmetic at ~1.2 ms, but it is the one remaining gap in the "only probe when it can matter" property that the newcompactModeand non-edit tests otherwise pin. withProvidersalready exists for the rerender case.src/test-utils/render.tsxexports it with the comment "Use this to rebuild the exact same root structure forrerender()calls". The new memoization test hand-builds its owntree(height)with onlySettingsContext+KeypressProvider, silently droppingConfigContextandShellFocusContext. It works today, but it diverges from the suite's shared harness for no gain.hideModifyis evaluated twice — once into thehideModifylocal for the memo, once again as!confirmationDetails.hideModifyin the render gate. M4 shows the memo copy is deliberate (probe avoidance), so this is fine; a one-line comment saying so would stop the next reader from "simplifying" it away.- A second, parallel editor-resolution path.
usePreferredEditor()already does validity + sandbox filtering and is whatAppContainer,AgentComposerandTextInputuse. This component keeps reading the raw setting and now adds its own availability call. ComposingusePreferredEditor()with an availability check would leave one resolution path instead of two. - The scheduler-side gate is still a truthiness check.
AppContainer.tsx:2313passes the raw setting intocoreToolScheduler, whoseModifyWithEditorbranch only bails on a falsy value — the exact path that produces the A9spawn code ENOENT. After this PR the TUI no longer offers the only route that reaches it, so this is not a live bug; but the defence is now UI-only, and it would come back for any non-TUI client that emitsModifyWithEditor, or if the editor disappears between render and Enter. A one-lineisEditorAvailable(raw) ? raw : undefinedthere would make it defence-in-depth. Out of scope for this PR.
Harness. Two full bundles built from source (head + merge-base); real bundled CLI driven under integration-tests/terminal-capture against a scripted local OpenAI-compatible server; isolated HOME/QWEN_RUNTIME_DIR per arm (verified afterwards — every per-arm artifact landed in the isolated home, and no file under the real ~/.qwen references any arm's workspace path); strace -f -e trace=execve for the probe counts. target.txt was never actually written in any arm. Screenshots are real terminal renders (xterm.js), hosted at wenshao/qwen-code@902897240d.
中文版本
在 b4fa9ccb6b 上的本地验证 —— 真实 TUI A/B、变异矩阵、系统调用级探测
我为这个 PR 搭了一套完整的本地验证环境并跑通了全流程。结论：修复正确且完整，我没有阻塞意见。 本 head 上唯一变红的 lane（Test (ubuntu-latest)）是 main 上已有的问题，与本 PR 无关，详见 §6。
本轮是在此前那轮之上做增量，不重复既有结论；下面全部是新证据（9 个 TUI arm 而非 3 个、产品文件的变异矩阵、用系统调用数量核实 memo 的说法、IDE-diff 分支、lint/typecheck、两个 arm 的完整 packages/cli 套件，以及 CI 定性）。
被测对象的一致性。 我 worktree 里两个改动文件与 b4fa9ccb6b 上的 GitHub blob 逐字节一致：
| 文件 | sha256 |
|---|---|
ToolConfirmationMessage.tsx |
f940c552a994f4aa24535e840b97960a55c7dd3288a4eadd4d747672c407eaa8 |
ToolConfirmationMessage.test.tsx |
f8d8b2472d15e46ab269752feeef68be7fe53e2e2e648ce0f808878b3e9bf20c |
从源码构建了两份完整 bundle：PR head 与 merge-base 419e8d57b2。构建见证（取自承载该选项文案的 chunk）：
PR head : ... !confirmationDetails.hideModify && (!config.getIdeMode() || !isDiffingEnabled) && editorAvailable) {
base : ... !confirmationDetails.hideModify && (!config.getIdeMode() || !isDiffingEnabled) && preferredEditor) {
1. 真实 TUI A/B —— 9 个 arm
每个 arm 都用仓库自带的 integration-tests/terminal-capture（node-pty + 无头 Chromium 里的 xterm.js）启动真实打包后的 CLI，由本地脚本化 OpenAI 兼容服务返回一个 write_file tool call，从而渲染出真实的编辑确认框。每个 arm 使用隔离的 HOME/QWEN_RUNTIME_DIR，显式设置 tools.approvalMode: "default"（仓库默认已是 AUTO，那样确认框根本不会出现），并构造 PATH，其中唯一的编辑器可执行文件就是该 arm 自己植入的。
| arm | 构建 | preferredEditor |
PATH 上有编辑器 |
SANDBOX |
是否提供该选项 | 选项数 |
|---|---|---|---|---|---|---|
| A1 | base | vscode |
否 | – | 是 ❌（#10745） | 4 |
| A2 | head | vscode |
否 | – | 否 ✅ | 3 |
| A3 | head | vscode |
是 | – | 是 ✅ | 4 |
| A4 | head | vscode |
是 | 已设 | 否 ✅ | 3 |
| A5 | base | vscode |
是 | 已设 | 是 ❌ | 4 |
| A6 | head | sublime（非法） |
– | – | 否 ✅ | 3 |
| A7 | base | sublime（非法） |
– | – | 是 ❌ | 4 |
| A8 | head | vim |
是 | – | 是 ✅ | 4 |
| A9 | base | vscode |
否 | – | 是，并实际选中 | 4 |
A1↔A2 就是缺陷与修复本身。A3、A8 证明没有过度抑制 —— 只要编辑器真的可启动，选项依旧出现，GUI 编辑器和终端编辑器都是如此。
2. PR 描述未提及的两处行为变化（都是改进）
isEditorAvailable() 等于 isValidEditorType() && checkHasEditorType() && allowEditorTypeInSandbox()，因此用它替换真值判断，顺带还关掉了「编辑器被卸载」之外的两种情形：
- A6/A7 —— 非法的
preferredEditor取值。"sublime"不是受支持的EditorType；旧的判断只要非空字符串就放行，于是长期提供一个永远无法执行的动作。head 会隐藏它。 - A4/A5 —— sandbox 模式。 设置了
SANDBOX时 GUI 编辑器无法启动，allowEditorTypeInSandbox()已经这么规定。head 隐藏该选项；base 即使code在PATH上也照样提供。这与usePreferredEditor()对Ctrl+X编辑器的既有处理一致，因此符合仓库既有策略 —— 但确实是行为变化，值得在 PR 描述里补一句。
3. 在 base 上选中该选项究竟会发生什么（A9）
我不只是确认那一行消失了 —— 我在 base 构建上真的选中了它。scheduler 的 ModifyWithEditor 分支只在 getPreferredEditor() 为假值时提前返回（AppContainer.tsx:2313 传进去的是原始设置），所以一个失效的 "vscode" 会一路走到 modifyWithEditor → openDiff → spawn：
x WriteFile {"file_path":".../ws/target.txt","content":"hello from the scripted model\n"}
spawn code ENOENT
待确认的编辑被取消了，而不仅仅是「编辑器没打开」—— 用户丢掉了这一轮，必须重新提问。这就是本 PR 消除的具体损害。
4. 变异矩阵 —— 新增的每个条件都有效且被单一测试钉住
cd packages/cli && npx vitest run src/ui/components/messages/ToolConfirmationMessage.test.tsx，保留 PR 的测试，只变异产品文件：
| 变异体 | 结果 | 变红的测试 |
|---|---|---|
| M0 对照 | 43 passed | – |
| M1 产品文件回退到 base | 2 failed | should show … when preferredEditor is set + should NOT show … when the configured editor is unavailable |
M2 去掉 memo 里的 !compactMode |
1 failed | should NOT probe editor availability in compactMode |
M3 去掉 memo 里的 type === 'edit' |
1 failed | should NOT probe editor availability for a non-edit confirmation |
M4 去掉 memo 里的 !hideModify |
1 failed | should NOT show … when hideModify is true |
M5 令 memo 失效（deps 里加 Math.random()） |
1 failed | should show … when preferredEditor is set |
PR 新增的每个条件都恰好被一个测试杀死，没有空转的测试。其中 M1 正是此前一直没做的 RED/GREEN：只回退产品文件、保留新测试，恰好翻红描述该修复的那两个测试。
5. 用系统调用核实 memo 的说法
isEditorAvailable() 最终落到 execSync("command -v code")，因此我用 strace -f -e trace=execve 统计了真实确认框在屏幕上停留 12 秒期间的实际子进程数：
| arm | /bin/sh -c "command -v code" 次数 |
启动时 | 弹框期间 |
|---|---|---|---|
| base | 1 | 1 | 0 |
| PR head（有 memo） | 2 | 1 | 1 |
| PR head（memo 失效） | 3 | 1 | 2 |
由此得到三点，我认为它从正反两面都了结了此前的性能顾虑：
- 本 PR 每个编辑确认框恰好多出一次同步探测。本机实测单次成本约 1.2 ms。
- memo 的作用真实但有限：没有它，本场景是 2 次而不是 1 次，并非「每次 Ink 重渲染一次」—— Ink 本来就不会那么频繁地重渲染这个组件。PR 描述这一句略有夸大。
- 作为量级参照：所有 arm（含 base）在启动时本来就会做 11 次同类探测（
code、codium、cursor、emacs、nvim、trae、vim、windsurf、zed、zeditor，外加rg）。每个弹框多一次，完全落在启动本身已付出的噪声之内。
6. 静态检查、完整套件与变红的 CI lane
npx eslint <两个改动文件> --max-warnings 0→ exit 0。packages/clitsc --noEmit→ exit 0。- IDE-diff 行为未变，这是直接验证而非阅读得出的：
getIdeMode() === true且isDiffingEnabled() === true时，两个 arm 都隐藏该选项；关闭 diffing 后，两个 arm 都显示。 - 两个 arm 的完整
packages/cli套件（28 588 个测试）。head 侧 11 个失败，其中 10 个在 base arm 上完全相同（环境因素：以 root 运行的 housekeeping 测试、ink 光标渲染、serve），第 11 个（src/ui/opentui/dialogs-modes.test.tsx，OpenTUI 输出样式对话框，与本改动无关）在两个 arm 上单独跑 3/3 全绿 —— 是 4 480 个 suite 并行下的抖动。 - 本 head 上变红的
Test (ubuntu-latest, Node 22.x)不是本 PR 造成的。 它挂在src/acp-integration/acpAgent.test.ts > QwenAgent runtime-root pinning choke point—— 一个断言acpAgent.ts不得直接引用runWithAcpRuntimeOutputDir的架构守卫。第 9200 行正是这么写的，而它由 feat(serve): add session turn navigation protocol #10751（8a0a9c6614，2026-09-04 合入main）引入，本分支通过合并main继承了它。我在 merge-base419e8d57b2（不含本 PR 改动）上本地复现了完全相同的失败，且main自身的 CI run 33932327186 也挂在同一个测试上。这里除了等main修好之外无需处理。
7. 非阻塞观察
- memo 没有包含 IDE-diff 条件。 渲染判断是
!hideModify && (!getIdeMode() || !isDiffingEnabled) && editorAvailable，而 memo 只覆盖了!compactMode、type === 'edit'、!hideModify、preferredEditor !== undefined。实测：在ideMode + diffing下 head arm 仍会调用一次isEditorAvailable（base 为 0），而该选项此时根本不可能被渲染。1.2 ms 级别只是观感问题，但它是「只在可能有用时才探测」这一性质仅存的缺口 —— 新增的compactMode与非 edit 测试恰恰是在钉这条性质。 withProviders本来就是为 rerender 场景准备的。src/test-utils/render.tsx导出了它，注释写着 "Use this to rebuild the exact same root structure forrerender()calls"。新增的 memo 测试自己手搭了tree(height)，只包了SettingsContext+KeypressProvider，悄悄少了ConfigContext和ShellFocusContext。目前能跑通，但无谓地偏离了套件共用的测试脚手架。hideModify被判断了两次 —— 一次算进 memo 用的hideModify局部变量，一次又在渲染判断里写!confirmationDetails.hideModify。M4 说明 memo 里那一份是刻意保留的（用于避免探测），所以没问题；加一行注释说明这点，可以避免下一个读者把它「简化」掉。- 出现了第二条并行的编辑器解析路径。
usePreferredEditor()已经在做合法性 + sandbox 过滤，AppContainer、AgentComposer、TextInput用的都是它。而本组件依旧读原始设置，如今又自己加了一次可用性调用。用usePreferredEditor()组合一次可用性检查，就只剩一条解析路径。 - scheduler 那一侧仍然是真值判断。
AppContainer.tsx:2313把原始设置传进coreToolScheduler，其ModifyWithEditor分支只在假值时退出 —— 正是产生 A9 中spawn code ENOENT的那条路径。本 PR 之后 TUI 不再提供通往它的唯一入口，所以这不是活跃缺陷；但防线现在只在 UI 层，任何非 TUI 客户端发出ModifyWithEditor、或编辑器在渲染与回车之间消失，问题都会回来。在那里加一行isEditorAvailable(raw) ? raw : undefined就能做成纵深防御。不属于本 PR 范围。
验证环境说明。 从源码构建了两份完整 bundle（head + merge-base）；用 integration-tests/terminal-capture 驱动真实打包 CLI，对接本地脚本化 OpenAI 兼容服务；每个 arm 使用隔离的 HOME/QWEN_RUNTIME_DIR（事后已核对：每个 arm 的产物都落在隔离 home 内，真实 ~/.qwen 下没有任何文件引用 arm 的工作区路径）；探测计数使用 strace -f -e trace=execve。所有 arm 中 target.txt 都没有被真正写入。截图是 xterm.js 的真实终端渲染，托管在 wenshao/qwen-code@902897240d。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
4 participants
Summary
hideModifybehavior unchangedToolConfirmationMessage.test.tsxsuiteWhy
A stale
general.preferredEditorsetting can remain after an editor is uninstalled or removed fromPATH. The confirmation UI previously checked only whether the setting was present, so it could offer an action that could not be launched.The core package already exposes
isEditorAvailable(), which checks editor validity, executable availability, and sandbox allowance. Reusing it keeps the UI aligned with the existing editor-selection logic. Because that check can synchronously probe the executable, the result is memoized per confirmation type/preferred editor instead of running on every render.Fixes #10745
Testing
Updated the existing
external editor optiontests inToolConfirmationMessage.test.tsxto cover:hideModify-> option remains hidden