fix(web-shell): keep workspace picker suggestions closed - #8844
Conversation
E2E Test ReportScope: Add Workspace native folder selection and directory autocomplete interaction on macOS. Baseline: With the existing Web Shell, selecting a complete directory updated the input and the debounced lookup subsequently reopened the suggestions with that directory as another choice. Post-fix verification: The focused Web Shell behavioral suite passed 23/23. It verifies that the list remains closed after advancing beyond the debounce interval following picker selection, and that selecting the current input path does not disable autocomplete on the next manual edit. Web Shell TypeScript typecheck also passed. Not covered: A post-fix live-browser run of the native macOS picker, non-macOS native pickers, and workspace registration authentication. The bearer-token behavior is intentionally outside this PR. |
|
Re-running the full gate on the current head Template looks good ✓ Problem: observed bug with solid evidence. The suggestion list reopening after blur/Browse was visible directly in the base code (the path-change effect ran with Direction: aligned. Web Shell is an actively maintained surface — the CHANGELOG carries a steady stream of recent Web Shell UX fixes (#8930, #8915, #8914, #8924, #8891) — and a double-selection papercut in Add Workspace is squarely in scope. Size: not applicable — no core paths; Approach: the scope is right and matches what this race class needs — an arrival gate (only open the list if the input is still focused when a result lands), blur dismissal that invalidates in-flight lookups through the sequence counter instead of the one-shot suppress flag, a Browse flow that resets that flag on every non-path-change outcome, and the duplicate-fetch root cause fixed by passing the memoized workspace action directly instead of a per-render closure. No unrelated changes spotted. Risk: no elevated risk signals — no changed file matches a revert-correlated path. Moving on to code review. 🔍 中文说明在当前 head 模板完整 ✓ 问题：已观测到的 bug，证据充分。失焦/浏览后建议列表重新打开的问题在基线代码中直接可见（选择后 path-change effect 以 方向：对齐。Web Shell 是活跃维护的界面——CHANGELOG 中有持续的近期 Web Shell UX 修复（#8930、#8915、#8914、#8924、#8891）——Add Workspace 中的重复选择问题完全在范围内。 规模：不适用——未触及核心路径，仅 方案：范围恰当，与这类竞态所需的修复一致——到达门控（结果到达时仅当输入框仍聚焦才打开列表）、失焦关闭通过序号计数器作废在途查询（而非依赖一次性抑制标志）、"浏览"流程在所有未产生路径变化的结局上重置该标志，以及通过直接传递 memoize 的 workspace action（而非逐 render 闭包）修复重复查询的根因。未发现无关改动。 风险：无升级风险信号——变更文件均未命中与回滚相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal before reading the diff: gate the open decision in the fetch callback on "is the input still focused when the result arrives" (reading focus from the input's own root node — The PR does all four, and goes one step further on two of them. I verified the previously blocking findings against the head code myself rather than taking the review thread's word:
Non-blocking, for the record: No Critical findings remain. TestingUnattended CI run — I did not build or execute any PR code; the evidence below is the PR's own CI at the reviewed commit, read through the API. Everything green, no failures:
Since the last pass this PR also carries a maintainer verification report (@wenshao posted it in this thread — his verification, run on this same head): a real Not verified: native directory-picker behavior on Windows/Linux — OS file dialogs are not scriptable in CI, the PR declares them out of scope, and no sandboxed lane can exercise a native GUI picker, so this remains reported for macOS only. This is the one residual gap, and no Real-scenario testing: not driven here (CI path); the live-behavior signal comes from the maintainer verification above plus the green web-shell E2E Smoke check. 中文说明代码审查读 diff 之前我独立的方案是：在 fetch 回调处以"结果到达时输入框是否仍聚焦"门控打开决策（焦点需从输入框自身的根节点读取——Web Shell portal 模式下 PR 四点全部做到，其中两点还更进一步。此前阻塞合并的两个发现，我没有采信评审线程的结论，而是对照 head 代码逐一独立验证：
非阻塞，仅记录在案： 已无 Critical 发现。 测试无人值守 CI 运行——我没有构建或执行任何 PR 代码；以下证据是在被审查提交上通过 API 读取的 PR 自身 CI。全部通过，无失败（上方表格；macOS/Windows 单测与 CLI 集成为 merge_group 专用任务，按 ci.yml 设计跳过）。 自上次审查以来，本 PR 还附带了一份维护者验证报告（@wenshao 在本线程发布——是他的验证，针对同一 head 运行）：真实 未验证：Windows/Linux 上的原生目录选择器行为——操作系统文件对话框在 CI 中无法脚本化，PR 声明其为范围外，且没有任何沙箱通道能驱动原生 GUI 选择器，因此该部分仍仅为 macOS 上的报告。这是唯一遗留的缺口， 真实场景测试：本次未驱动（CI 路径）；实机行为信号来自上述维护者验证与通过的 web-shell E2E Smoke 检查。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every prior blocking finding is fixed and independently verified against the head code; the two reservations are non-blocking caveats, not code issues (native picker behavior on Windows/Linux remains untested anywhere, and the PR's before-state wording about Browse holding focus turned out slightly imprecise under the maintainer's reproduction — the symptom that actually reproduces, the list staying open and reopening, is the one that got fixed). Stepping back: this is the pass I wanted to write last time. The focus-gate plus sequence-invalidation shape is exactly what I would have proposed independently, and the PR lands all four pieces — the arrival gate, blur invalidation, Browse flag hygiene on every outcome, and the memoized-callback root cause. What moved this from "looks right" to "verified": I re-read every state path in the head code (cancelled / same-value / error picks, timer stacking, refocus before the dismiss fires, stale-sequence drops) and each one lands correctly; and the maintainer's real-stack before/after with the mutation check answers the question CI alone could not — the green suite now fails without the fix, so it pins the behavior instead of passing vacuously. The diff is 90 production lines with no drive-bys, and it retires the earlier duplication note properly instead of leaving it as debt. CI fully green on the reviewed commit, nothing pending. Approving, pinned to the reviewed commit. The standing 中文说明置信度：4/5 —— 此前所有阻塞性发现均已修复，并已对照 head 代码独立验证；两点保留意见是非阻塞的注意事项，而非代码问题（Windows/Linux 上的原生选择器行为在任何环境下都仍未实测；PR 对修改前状态的描述中"Browse 会让输入框保持焦点"在维护者的复现下略有不精确——真正可复现的症状，即列表保持打开及重新弹出，正是本次修复掉的行为）。 退一步看：这正是我上次想给出的结论。焦点门控 + 序号作废的形态与我独立提出的方案完全一致，且 PR 四部分全部落地——到达门控、失焦作废、"浏览"在所有结局上的标志卫生、以及 memoize 回调这一根因修复。从"看起来对"到"已验证"的关键在于：我把 head 代码中的每条状态路径重新读了一遍（取消/同值/出错选择、定时器叠加、关闭触发前回焦、陈旧序号丢弃），每条都正确收敛；而维护者在真实栈上的 before/after 加变异测试，回答了单靠 CI 无法回答的问题——现在的绿色套件在去掉修复后会失败，因此它钉住了行为，而不是空转通过。diff 只有 90 行生产代码、无顺手改动，并且把早前的重复代码记录项妥善解决而非留成债务。CI 在被审查提交上全面通过，无待决任务。 批准，并钉在被审查的提交上。本 PR 上现存的 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
46e44d4 to
b3d4802
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
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): This PR fixes the web-shell AddWorkspaceDialog: it closes...: (none); This PR fixes the web-shell AddWorkspaceDialog: it closes...: (none).
中文说明
未探索到全部深度（达到工具调用预算）：This PR fixes the web-shell AddWorkspaceDialog: it closes...：(none)；This PR fixes the web-shell AddWorkspaceDialog: it closes...：(none)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| if ( | ||
| inputRef.current === document.activeElement && | ||
| (openOnResult || listOpenRef.current) | ||
| ) { |
There was a problem hiding this comment.
[Critical] R1-1: The new focus gate compares inputRef.current against document.activeElement, but when this dialog is portaled into the Web Shell shadow-DOM root (embedders with shadowDom: true or { portals: true }), document.activeElement retargets to the shadow host — the focused input never matches, so suggestions can never auto-open in that entire embedding mode. The codebase already documents this retargeting gotcha and uses the getRootNode() pattern for it (see the comment at App.tsx ~L3862-3865). — Failure scenario: an embedder mounts Web Shell with shadow-DOM portals → the user focuses the path input and types an absolute prefix → the lookup succeeds, but document.activeElement is the host element while shadowRoot.activeElement is the input → the gate is always false → setListOpen is never called → directory suggestions never appear for the whole shadow-DOM mode. Verified by probe in both jsdom and real Chromium (gate false, listbox never rendered); the getRootNode() fix below makes the same probe render the listbox, and all 23 existing tests still pass with it.
| if ( | |
| inputRef.current === document.activeElement && | |
| (openOnResult || listOpenRef.current) | |
| ) { | |
| if ( | |
| (inputRef.current?.getRootNode() as | |
| | Document | |
| | ShadowRoot | |
| | undefined)?.activeElement === inputRef.current && | |
| (openOnResult || listOpenRef.current) | |
| ) { |
中文说明
新的焦点门控把 inputRef.current 和 document.activeElement 比较，但当该对话框被 portal 到 Web Shell 的 shadow-DOM 根时（即嵌入方启用 shadowDom: true 或 { portals: true }），document.activeElement 会被重定向（retarget）到 shadow host——聚焦的输入框永远无法匹配，因此在整个该嵌入模式下建议列表永远不会自动打开。代码库中已经记录了这个 retarget 陷阱，并使用了 getRootNode() 模式（见 App.tsx ~L3862-3865 处的注释）。— 失败场景：嵌入方以 shadow-DOM portals 挂载 Web Shell → 用户聚焦路径输入框并输入绝对路径前缀 → 查询成功，但 document.activeElement 是宿主元素，而 shadowRoot.activeElement 才是输入框 → 门控恒为 false → setListOpen 永远不会被调用 → 整个 shadow-DOM 模式下目录建议都不会出现。已在 jsdom 和真实 Chromium 中通过探针验证（门控为 false，listbox 从未渲染）；应用下方的 getRootNode() 修复后，同一探针可以渲染出 listbox，且全部 23 个现有测试仍然通过。
— qwen3.8-max via Qwen Code /review (v0.21.8)
ytahdn
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback round — PR #8844Feedback triage
Changes
No conflicts: Verification
Commit: 中文说明评审反馈轮次 — PR #8844反馈分类
变更内容
无冲突： 验证
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
| if (!onPick) return; | ||
| inputRef.current?.blur(); | ||
| setBrowsing(true); |
There was a problem hiding this comment.
[Critical] R2-1: Browse leaves a stale suppressNextFetchOpenRef, so the first keystroke after a cancelled (or same-path) pick never opens the suggestion list.
The inputRef.current?.blur() added here routes Browse through the input's onBlur, whose 100 ms timer sets suppressNextFetchOpenRef.current = true. That flag is consumed only by the path-change effect's absolute-like branch (openOnResult = !suppressNextFetchOpenRef.current; suppressNextFetchOpenRef.current = false;, lines 98-99). When the picker is cancelled (onPick resolves undefined -> no setPath) or the user selects the already-typed path (React state bail-out -> the effect does not re-run), nothing consumes the flag.
- Failure scenario: open Add Workspace -> click Browse (the blur schedules the 100 ms timer, flag is set) -> cancel the native picker -> click back into the input and type an absolute prefix such as
/h: the lookup resolves with the input focused, butopenOnResultisfalseandlistOpenRef.currentisfalse, so the gate skipssetListOpenand the listbox stays closed; suggestions appear only from the second keystroke. Reproduced deterministically by probe in jsdom (listboxnullon keystroke 1, open on keystroke 2, for both the cancel and same-path variants); pre-diff, the removedonMouseDownpreventDefault kept focus in the input, so this path never firedonBlurin the test harness. This also contradicts the PR's own E2E evidence ("selecting the current input path does not disable autocomplete on the next manual edit"); the earlier iteration carried a same-path regression test that this iteration dropped.
Suggested fix - reset the flag once the picker outcome is known (a few lines below in pickDirectory), and add a regression test for typing after a cancelled picker:
try {
const selectedPath = await onPick();
suppressNextFetchOpenRef.current = false;
if (selectedPath) {The result-arrival focus gate above already keeps the list closed for a delayed lookup while the input is unfocused, so this reset does not re-open the race this PR fixes.
中文说明
"浏览"会留下一个过期的 suppressNextFetchOpenRef 标志，导致取消选择器（或选择了相同路径）之后，第一次按键不会打开建议列表。
此处新增的 inputRef.current?.blur() 使"浏览"流程经过输入框的 onBlur，其 100 ms 定时器会设置 suppressNextFetchOpenRef.current = true。该标志只在路径变化 effect 的绝对路径分支中被消费（openOnResult = !suppressNextFetchOpenRef.current; suppressNextFetchOpenRef.current = false;，第 98-99 行）。当用户取消系统选择器（onPick 返回 undefined → 没有 setPath）或选择了与已输入内容相同的目录（React 状态未变化 → effect 不会重新运行）时，没有任何逻辑消费该标志。
- 失败场景：打开"添加工作区" → 点击"浏览"（blur 触发 100 ms 定时器，标志被置位）→ 取消系统目录选择器 → 重新点击输入框并输入绝对路径前缀（如
/h）：查询在输入框聚焦时返回结果，但openOnResult为false且listOpenRef.current为false，门控跳过setListOpen，列表保持关闭；建议要到第二次按键才出现。已在 jsdom 中通过探针确定性复现（取消和相同路径两种变体均为：第一次按键 listbox 为null，第二次按键才打开）。修改前，被移除的onMouseDownpreventDefault 会让输入框保持聚焦，该路径在测试框架中从未触发onBlur。这也与本 PR 自己的 E2E 证据相矛盾（"选择当前输入路径不会禁用下一次手动编辑时的自动补全"）；之前的迭代版本带有相同路径的回归测试，本次迭代删除了它。
建议修复 —— 在选择器结果确定后重置该标志（位于下方 pickDirectory 中，代码见上），并补充"取消选择后继续输入"的回归测试。上方的结果到达焦点门控已经保证：输入框失焦时延迟返回的查询不会打开列表，因此该重置不会重新引入本 PR 要修复的竞争。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| )?.activeElement === inputRef.current && | ||
| (openOnResult || listOpenRef.current) |
There was a problem hiding this comment.
[Suggestion] R2-2: The shadow-DOM-aware focus query is now hand-inlined a third time - extract a shared helper.
The getRootNode()-based activeElement read added here is the third copy: App.tsx:3864-3865 and App.tsx:3925-3927 already carry the same cast with a near-identical retargeting comment, and no shared helper exists (utils/dom.ts has only isEditableTarget, portalRoot.ts only the context). The copies have already diverged on null handling - the App.tsx sites cast without undefined and dereference directly, while this site adds | undefined and optional chaining.
- Concrete cost: a fix to the focus-resolution rule (nested shadow trees, a new root kind) must be applied to three sites found only by grepping a comment; a fix landing in one copy but not the others yields different focus behaviour between the fullscreen-surface focus trap, the Tab trap, and this dialog's autocomplete. The divergence already bites elsewhere:
AtMentionPanel.tsx:75still uses the naivedocument.activeElement === searchInputRef.currentform (never true in shadow-DOM portal mode, so its "already focused" guard is dead there), andDialogShell.tsx:84records the retargeteddocument.activeElementas the focus-restore target.
Suggested fix:
// e.g. in client/utils/dom.ts or portalRoot.ts
export function getRootActiveElement(
node: Node | null | undefined,
): Element | null {
const root = node?.getRootNode();
return root instanceof Document || root instanceof ShadowRoot
? root.activeElement
: null;
}and route this site plus the two App.tsx sites through it.
中文说明
shadow-DOM 感知的焦点查询现在是第三处手写内联 —— 建议抽取共享辅助函数。
此处新增的基于 getRootNode() 的 activeElement 读取是第三份拷贝：App.tsx:3864-3865 和 App.tsx:3925-3927 已经有相同的类型转换和几乎一致的 retarget 注释，且目前不存在共享辅助函数（utils/dom.ts 只有 isEditableTarget，portalRoot.ts 只有 context）。这些拷贝在空值处理上已经出现分歧 —— App.tsx 处的转换不含 undefined 且直接解引用，而本处增加了 | undefined 和可选链。
- 具体代价：一旦焦点解析规则需要修复（嵌套 shadow 树、新的根类型），就必须把修复应用到三处只能通过 grep 注释才能找到的位置；如果修复只落在其中一份拷贝上，全屏 surface 焦点陷阱、Tab 陷阱和本对话框的自动补全将表现出不同的焦点行为。这种分歧已经在其他地方造成问题：
AtMentionPanel.tsx:75仍在使用朴素的document.activeElement === searchInputRef.current形式（在 shadow-DOM portal 模式下永远不为真，其"已聚焦"守卫在那里是死代码），DialogShell.tsx:84则把 retarget 后的document.activeElement记录为焦点还原目标。
建议修复（代码见上）：在 client/utils/dom.ts 或 portalRoot.ts 中抽取类似 getRootActiveElement 的辅助函数，并让本处与 App.tsx 的两处都改用它。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31381560750 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31383566918 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 86 passed · 0 failed · 86 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：86 通过 · 0 失败 · 86 总计 Verification reportPR 8844 deep verification —
|
| cell | oracle | base 0d56e50b |
commit 1 b3d4802e |
head 67c0e3db |
|---|---|---|---|---|
| C0 focused input opens suggestions | listbox present | opens | opens | opens |
| C1d lookup resolves after blur | listbox present | reopens (bug) | closed | closed |
| C2b/d focus retained during/after Browse | activeElement === input |
retained (bug) | released | released |
| C2Bb list visible 100 ms into open picker | listbox present | visible (bug) | closed | closed |
| C2e/f list reopens after fill, offers the already-selected dir | listbox + code/ option |
reopens (bug) | closed | closed |
| C3c shadow-DOM portal root, focused input | listbox inside shadow root | opens | never opens (bug) | opens |
| C4a–d refocus-after-blur A/A controls | 4 checks | identical | identical | identical |
Result: 23/23 checks matched prediction in all three builds (69 assertions) — base reproduces every symptom the PR describes (including the "redundant option": the reopened list offers /Users/me/code right after the user selected it); commit 1 flips all five document-mode cells but breaks shadow-DOM mode (document.activeElement retargets to the shadow host — control C3b proves the retargeting in the same cell); commit 2 flips exactly that cell back while keeping every commit-1 fix. Both commits are load-bearing. jsdom semantics that make the focus cells discriminating were measured directly first (disabling a focused input neither moves focus nor fires blur in jsdom 26, so on base nothing ever blurs the input through Browse).
Witness: evidence/01-ab-base-inter-head-cells.png (live re-run of all three arms printing the full CHECK grid).
2. Corrections to the PR description
- The description's evidence line says "focused Web Shell tests (23/23)". At head the file contains 24 tests: commit 1's file had 23, and the follow-up commit 2 added the shadow-DOM test. All 24 pass (measured, M0 below). This is a description correction only — no code change implied.
3. Vacuity check and mutation matrix (PR's own tests)
Unmutated control is green (M0: 24/24). Single-point mutants of the production file, run against the PR's unmodified test file (swap-and-restore with git restore, verified clean; junit scored programmatically):
| mutant | change | killed by | failure message (behavioural) |
|---|---|---|---|
| M1 | gate reverted to if (openOnResult || listOpenRef.current) |
exactly 2 tests: keeps suggestions closed when a lookup finishes after blur, opens the system picker and fills the selected absolute path | expected <ul …(4)>…(2)</ul> to be null (both) |
| M2 | gate reads document.activeElement (recreates commit 1) |
exactly 1 test: opens suggestions for a focused input in a shadow-DOM portal root | expected null not to be null |
| M3 | inputRef.current?.blur() removed from pickDirectory |
exactly 1 test: opens the system picker… | expected <input …> not to be <input …> |
| M4 | onMouseDown={(e) => e.preventDefault()} restored on Browse |
— | survives (24/24 still green) |
Every revert fails the intended behavioural assertion (list visibility / focus), not an import or fixture break, and each new guard is pinned by exactly the test the commit says it strengthens. M2's kill is the positive control proving commit 2's one-line change is what the shadow-DOM test pins.
M4 survivor classification — redundant-clause removal, not a coverage gap on the fix path: jsdom's HTMLElement.click() fires no mousedown, so no jsdom test can express the clause's production effect (preventing focus loss on pointer-down). In production the removal is redundant next to the new explicit blur() — the input is blurred before the picker either way — so the surviving mutant confirms an unpinnable-in-jsdom clause, not an untested fix.
Witness: evidence/02-mutation-matrix-live.png (live M0–M4 run + encoded scoring, 11/11).
4. Findings
- Pre-existing quirk, not caused or worsened by this PR (informational). After the list closes on blur, the first keystroke once the input is refocused (>100 ms later) fetches suggestions but does not show them; the second keystroke does. Cause: the pre-existing blur handler sets
suppressNextFetchOpenRef = trueat +100 ms and the next path-change effect consumes it. Measured A/A: cells C4a–d are byte-identical across base, commit 1 and head, so the PR neither introduced nor changed it; the PR's risk note ("cannot become visible until existing input interactions open them again") is slightly optimistic about the very first interaction. Worth a separate issue if maintainers care; not a merge condition. - M4 survivor (informational) — classified above; no action needed.
5. Not covered
- Real native directory picker on any OS:
onPickis a host-provided callback and was driven with controlled promises at the prop seam. Picker UX was verified only by the author on macOS. - Real-browser pointer semantics (mousedown-driven focus movement, click-outside targeting): jsdom does not simulate them; cells drive
.click()/.blur()directly. This is why M4 survives and is also the reason the Reviewer Test Plan's "click outside" step was exercised as ablur()event — the wire shape is reproduced, the pointer mechanism is not. - Repo-wide gates were not run — the effective diff touches only
packages/web-shell, so gates were scoped to that workspace (3071-test suite,tsc -p tsconfig.json --noEmit,eslint packages/web-shell), each proven live with a planted violation first (evidence/03-gates-and-liveness.png). - Playwright e2e suites (
test:e2e*) not run — unit-level A/B was chosen instead. - Commit 1's own diff vs its parent (
2e3d297e, not fetchable at depth 2) was not measured; commit 1 was verified as its faithful tree state. The aggregateHEAD^1..HEADdiff is fully covered. - Dialog closed mid-flight: if the dialog unmounts while a lookup is pending, React nulls the ref and the gate evaluates
undefined === null→ false (fail-closed), and the state setters are no-ops on an unmounted component — static analysis only, no cell.
6. Methodology
Environment: CI verify container (node:22-bookworm), Node 22.23.2, vitest 3.2.4, jsdom 26.1.0; repo pre-installed and pre-built at the merge commit. A/B arms ran in git worktrees (HEAD^1 and b3d4802e) reusing the root node_modules (lockfile untouched by the PR; AddWorkspaceDialog's import closure contains no @qwen-code/* packages — grep-verified — so no internal symlink could leak head code into a control arm). The harness renders the real component through real react-dom into jsdom (plain document and shadow-root portal modes), drives it with fake timers and controlled prop promises, and records every check as CHECK|variant|id|PASS/FAIL|… lines plus a JSON tally per arm. Mutations were applied by swapping the single production file and restoring via git restore (clean status verified after each run); scoring parses the junit XML written by the repo's own vitest config. Gates were scored the same way. Raw per-cell logs, harness sources, mutants, junit files and tallies live under logs/ and harness/ in this artifact directory.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31399630023 🧠 Handled by Qwen Code · model/模型 |
…og (#8844) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8844Feedback dispositions
Changes
Conflict notesNone ( Verification
Commit: 中文说明Autofix 审查轮次 — PR #8844反馈处理
变更内容
冲突说明无（ 验证
提交： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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): 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 planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| blurTimeoutRef.current = setTimeout(() => { | ||
| blurTimeoutRef.current = undefined; |
There was a problem hiding this comment.
[Suggestion] The newly tracked blur timer is cancelled on re-blur and in pickDirectory, but never when focus returns to the input — there is no onFocus handler. If the user blurs and refocuses within the 100 ms window, the pending timer fires on a focused input: closeList() visibly dismisses an open list mid-interaction and suppressNextFetchOpenRef.current = true persists, so the next edit's path-change effect consumes the stale flag (openOnResult = false) and that edit's lookup never opens the list — only the second keystroke does. This PR took explicit ownership of this timer's lifecycle (tracking ref, two cancellation sites, and a test asserting cancellation semantics), leaving focus-return as the one schedule site the new lifecycle never clears. Probe-verified at the reviewed commit: list open → blur → refocus within 100 ms → timer fires → first edit CLOSED / second edit OPEN; an onFocus that clears blurTimeoutRef flips the probe to OPEN / OPEN. — Failure scenario: suggestion list open → user clicks another element and clicks back into the path input within 100 ms → the timer fires while the input is focused → the open list visibly closes and the next keystroke shows no suggestions (recovers on the following keystroke).
Suggested fix — cancel the pending timer when focus returns:
onFocus={() => {
if (blurTimeoutRef.current !== undefined) {
clearTimeout(blurTimeoutRef.current);
blurTimeoutRef.current = undefined;
}
}}中文说明
新增的受跟踪 blur 定时器会在重复失焦时以及 pickDirectory 中被取消，但当焦点回到输入框时从未被取消——组件没有 onFocus 处理器。如果用户在 100 ms 窗口内先失焦再重新聚焦，挂起的定时器会在输入框已聚焦时触发：closeList() 会把正在显示的列表关闭（用户可见），并且 suppressNextFetchOpenRef.current = true 残留；下一次编辑的路径变化 effect 会消费这个过期标志（openOnResult = false），导致该次编辑的查询不打开列表——要到第二次按键才恢复。本 PR 已明确接管了这个定时器的生命周期（跟踪 ref、两处取消点、以及一个断言取消语义的测试），焦点回归是新生命周期唯一没有清理的调度点。已在被审 commit 上通过探针验证：列表打开 → 失焦 → 100 ms 内重新聚焦 → 定时器触发 → 第一次按键列表关闭 / 第二次按键打开；增加一个清除 blurTimeoutRef 的 onFocus 后探针翻转为 打开 / 打开。— 失败场景：建议列表已打开 → 用户点击其他元素并在 100 ms 内点回路径输入框 → 定时器在输入框聚焦时触发 → 打开的列表被关闭，且下一次按键没有建议（再下一次按键恢复）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const blurTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>( | ||
| undefined, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The tracked blur timer has no unmount cleanup: closing the dialog within 100 ms of a blur leaves the pending timer to fire on the dead component — it writes suppressNextFetchOpenRef.current = true on a dead ref and runs closeList() (setListOpen/setHighlight) against an unmounted component. Probe-verified on this package's React 19: a silent no-op today (no exception, no state-update warning) with ≤100 ms closure retention — but the codebase's own precedent for this exact delayed-close pattern pairs the timer ref with a dedicated unmount cleanup (ArtifactPanel.tsx clears sideTaskMenuCloseTimerRef in a mount-effect cleanup), and the dialog's other timers/listeners (suggest debounce, window keydown capture) all clean up on unmount. This PR otherwise carefully tracks the timer through its schedules; unmount is the one site nothing clears. — Concrete cost: post-unmount state writes plus divergence from the sibling pattern; a future edit or a harness that surfaces post-unmount updates would turn the silent no-op into noise.
Suggested fix, next to the existing mount-focus effect:
useEffect(
() => () => {
if (blurTimeoutRef.current !== undefined) {
clearTimeout(blurTimeoutRef.current);
}
},
[],
);中文说明
受跟踪的 blur 定时器没有卸载清理：在失焦后 100 ms 内关闭对话框，挂起的定时器会在已卸载的组件上触发——向已失效的 ref 写入 suppressNextFetchOpenRef.current = true，并对已卸载组件执行 closeList()（setListOpen/setHighlight）。已在本包的 React 19 上通过探针验证：目前是静默无操作（无异常、无 state 更新警告），闭包保留不超过 100 ms——但代码库中对同一"延迟关闭"模式的既有先例是为定时器 ref 配套专门的卸载清理（ArtifactPanel.tsx 在挂载 effect 的清理中清除 sideTaskMenuCloseTimerRef），且本对话框的其他定时器/监听器（建议 debounce、window keydown 捕获）都在卸载时清理。本 PR 在其他方面都小心地跟踪了这个定时器的各个调度点；卸载是唯一没有任何清理的调度点。— 具体代价：卸载后的状态写入，以及与兄弟模式的偏离；未来某次修改或某个会暴露卸载后更新的测试框架，会把这个静默无操作变成噪音。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(listbox()).toBeNull(); | ||
| }); | ||
|
|
||
| it('keeps suggestions closed when a lookup finishes after blur', async () => { |
There was a problem hiding this comment.
[Suggestion] No test opens the suggestion list before blurring the input, so closeList() inside the tracked blur timer is a surviving mutant: deleting only that line keeps the whole 28-test suite green (verified by a mutant run at the reviewed commit) — the two blur-based tests blur while their fetch is still pending (the list is never open before the blur), and the picker paths call closeList() directly. The PR's headline behavior — "close workspace path suggestions whenever the path input loses focus" — is therefore pinned by nothing. — Failure scenario: a future edit to this timer body (which this PR already restructured once) could ship with the list staying open after the user clicks away, suite green; the untested path is the PR's own stated purpose, and the regression would be user-visible yet invisible to CI.
Suggested regression test:
it('closes the suggestion list when the input blurs', async () => {
const onSuggest = vi.fn().mockResolvedValue(SUGGESTIONS);
mount(
<AddWorkspaceDialog onClose={vi.fn()} onAdd={vi.fn()} onSuggest={onSuggest} />,
);
type('/home/me/co');
await settle();
expect(listbox()).not.toBeNull();
act(() => input().blur());
await act(async () => {
await vi.advanceTimersByTimeAsync(100);
});
expect(listbox()).toBeNull();
});中文说明
没有任何测试先把建议列表打开再让输入框失焦，因此受跟踪 blur 定时器中的 closeList() 是一个存活的变异体：只删除这一行，全部 28 个测试仍然通过（已在被审 commit 上通过变异运行验证）——两个基于 blur 的测试在查询尚未返回时就失焦（blur 前列表从未打开），而选择器路径会直接调用 closeList()。因此本 PR 的核心行为——"路径输入框失焦时关闭工作区路径建议"——没有被任何测试钉住。— 失败场景：未来对这个定时器主体的修改（本 PR 已经重构过一次）可能让"用户点击别处后列表仍然打开"的回归被发布，而测试套件依然全绿；未被测试的路径正是本 PR 自己声明的目标，该回归用户可见、CI 不可见。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Cancelled, failed, or same-value pick: the first edit must open. | ||
| suppressNextFetchOpenRef.current = false; |
There was a problem hiding this comment.
[Suggestion] A same-value pick no longer invalidates an in-flight suggestion lookup. Pre-diff, every truthy pick bumped suggestSeqRef.current, discarding in-flight lookups; this branch clears the suppress flag but does not bump the seq. A lookup that was debounced/in flight when Browse was clicked therefore passes the seq check when it resolves; if the user has refocused the input, the focus gate passes and the stale lookup pops the list open on mere refocus with zero edits — violating the invariant the adjacent test encodes ("Refocusing ... must not pop the list open; only a real edit may"). Probe reproduced this deterministically at the reviewed commit (type → Browse inside the debounce window → same-value pick → refocus → listbox OPEN), and bumping the seq for a truthy same-value pick flips it to CLOSED. — Failure scenario: user types an absolute path, clicks Browse while the lookup is still debounced/in flight (a window that includes daemon latency), confirms the same path in the native picker, and clicks back into the input: the listbox pops open unprompted; with exactly one suggestion, the single-suggestion Tab-accept path can then rewrite the typed path.
| // Cancelled, failed, or same-value pick: the first edit must open. | |
| suppressNextFetchOpenRef.current = false; | |
| // Cancelled, failed, or same-value pick: the first edit must open. | |
| // A same-value pick keeps the typed path; invalidate any lookup | |
| // already in flight from before Browse was clicked. | |
| if (pickedPath) ++suggestSeqRef.current; | |
| suppressNextFetchOpenRef.current = false; |
中文说明
选择与已输入内容相同的路径时，不再使正在进行的建议查询失效。修改前，任何非空选择都会递增 suggestSeqRef.current 以丢弃在途查询；该分支现在清除了抑制标志，但没有递增 seq。因此，点击"浏览"时已处于 debounce 或在途的查询在返回时仍能通过 seq 检查；若用户此时已重新聚焦输入框，焦点门控通过，过期查询会在零次编辑时仅因重新聚焦就弹出列表——违反了相邻测试所编码的不变量（"重新聚焦……不得弹出列表；只有真正的编辑才可以"）。已在被审 commit 上通过探针确定性复现（输入 → 在 debounce 窗口内点击浏览 → 选择相同路径 → 重新聚焦 → listbox 打开），为相同值的非空选择递增 seq 后翻转为 关闭。— 失败场景：用户输入绝对路径，在查询仍在 debounce/在途时（该窗口包含 daemon 延迟）点击"浏览"，在系统选择器中确认了相同路径，然后点回输入框：列表在零次编辑时意外弹出；若恰好只有一条建议，单条建议的 Tab 接受路径还会改写已输入的路径。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix review response — PR #8844 (round 5)All four inline suggestions were verified against the head commit and addressed in one commit ( Feedback points and dispositions
Changes
Verification
中文说明Autofix 审查响应 — PR #8844（第 5 轮）四条行内建议均已在 head commit 上核实，并在一次提交中处理完毕（ 反馈点与处理结果
变更内容
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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): Context: PR #8844 (web-shell AddWorkspaceDialog focus/blu...: none — all checks above were completed within budget.; Context: PR #8844 (web-shell AddWorkspaceDialog focus/blu...: none — I finished within the tool budget with no checks left unfinished.; Context: PR #8844 (web-shell AddWorkspaceDialog) closes w...: none — I completed all checks I opened; no check was left unfinished at the tool ceiling.; Context: PR #8844 (web-shell AddWorkspaceDialog) closes w...: none — all checks I started completed within budget..
中文说明
未探索到全部深度（达到工具调用预算）：Context: PR #8844 (web-shell AddWorkspaceDialog focus/blu...：none — all checks above were completed within budget.；Context: PR #8844 (web-shell AddWorkspaceDialog focus/blu...：none — I finished within the tool budget with no checks left unfinished.；Context: PR #8844 (web-shell AddWorkspaceDialog) closes w...：none — I completed all checks I opened; no check was left unfinished at the tool ceiling.；Context: PR #8844 (web-shell AddWorkspaceDialog) closes w...：none — all checks I started completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| blurTimeoutRef.current = setTimeout(() => { | ||
| blurTimeoutRef.current = undefined; | ||
| suppressNextFetchOpenRef.current = true; |
There was a problem hiding this comment.
[Critical] R6-1: When the 100 ms blur-dismiss timer fires (user stays blurred >100 ms), it arms suppressNextFetchOpenRef and nothing clears it until the next path change — which consumes it (openOnResult = !flag), so the user's first edit after returning to the field never opens the suggestion list; only the second keystroke does. This PR's App.tsx change makes it live: pre-PR the inline onSuggest arrow gave the suggest effect fresh identity on every App render, so churn re-runs consumed the stale flag; the stable memoized workspaceActions.suggestWorkspacePaths reference removes that incidental consumption, making the suppression deterministic. — Failure scenario: open Add Workspace → type /home/me/co (suggestions appear) → click the Persist switch → after 100 ms the timer fires (list closes, flag armed) → click back into the input and type one character → the lookup resolves with the input focused, but openOnResult || listOpenRef.current is false → the listbox stays closed on keystroke 1 and opens only on keystroke 2. Probe-reproduced at the reviewed commit in both "typed prefix then blurred" and "blur from empty then type" variants (the fetch happens, only the open is suppressed); the fix below flips the probe with all 31 existing tests still green. Same first-edit-suppressed family as R2-1/R3-1, on the timer-fired path the picker fixes don't reach.
| blurTimeoutRef.current = setTimeout(() => { | |
| blurTimeoutRef.current = undefined; | |
| suppressNextFetchOpenRef.current = true; | |
| blurTimeoutRef.current = setTimeout(() => { | |
| blurTimeoutRef.current = undefined; | |
| ++suggestSeqRef.current; |
Invalidating the dismissed lookup via the sequence counter (instead of poisoning the next edit) also subsumes R6-2: an in-flight lookup then fails the seq check on resolve, while the first edit opens normally. The existing "keeps suggestions closed when a lookup finishes after blur" test still passes under it. Add a regression test: blur → advance 100 ms → focus → type → expect the listbox open on the first edit.
中文说明
当 100 ms 的 blur 关闭定时器真正触发时（用户失焦超过 100 ms），它会置位 suppressNextFetchOpenRef，且没有任何逻辑在下一次路径变化前清除它——而下一次路径变化恰恰会消费该标志（openOnResult = !flag），因此用户回到输入框后的第一次编辑永远不会打开建议列表，要到第二次按键才打开。本 PR 在 App.tsx 中的改动使该缺陷确定性地生效：修改前内联的 onSuggest 箭头函数在每次 App 渲染时都会产生新标识，churn 导致的 effect 重跑会顺带消费过期标志；改为稳定的记忆化 workspaceActions.suggestWorkspacePaths 引用后，这一意外消费被移除，抑制行为变为确定性发生。— 失败场景：打开"添加工作区" → 输入 /home/me/co（建议出现）→ 点击 Persist 开关 → 100 ms 后定时器触发（列表关闭、标志置位）→ 重新点击输入框并输入一个字符 → 查询在输入框聚焦时返回，但 openOnResult || listOpenRef.current 为 false → 第一次按键时列表保持关闭，第二次按键才打开。已在评审提交上通过探针复现（"先输入再失焦"与"空输入失焦后再输入"两种变体：查询会发出，只有打开被抑制）；应用下方修复后探针翻转通过，且全部 31 个现有测试仍然通过。这与 R2-1/R3-1 属于同一类"首次编辑被抑制"缺陷，只是发生在选择器相关修复覆盖不到的"定时器已触发"路径上。
通过序列计数器使被关闭的查询失效（而不是毒化下一次编辑）同时也覆盖了 R6-2：未决查询在返回时会因 seq 检查失败而被丢弃，而第一次编辑可以正常打开列表。现有的"keeps suggestions closed when a lookup finishes after blur"测试在该修复下仍然通过。请补充回归测试：blur → 推进 100 ms → focus → 输入 → 断言第一次编辑时列表打开。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| blurTimeoutRef.current = setTimeout(() => { | ||
| blurTimeoutRef.current = undefined; | ||
| suppressNextFetchOpenRef.current = true; |
There was a problem hiding this comment.
[Critical] R6-2: The blur-dismiss timer arms the suppress flag but never bumps suggestSeqRef, so a lookup in flight when the user blurs survives invalidation. Once the timer has fired, refocusing is a no-op (onFocus only cancels a pending timer), and when the stale lookup resolves it passes the seq check and the new focus gate (the input is focused again) with its pre-blur captured openOnResult=true — popping the list open on a bare refocus with zero edits. That violates the invariant this PR's own same-value race test documents ("so a bare refocus cannot pop the list open"), implemented for the picker-dismiss path but omitted on the blur-dismiss path. — Failure scenario: type /home/me/co → wait past the 150 ms debounce so the daemon lookup is in flight (routine for a remote daemon) → click another field → the 100 ms timer fires (closeList, flag armed, no seq bump) → click back into the input before the lookup resolves → the lookup resolves → the list pops open with zero edits; with the list unexpectedly open, handleInputKeyDown then treats subsequent keys as listbox navigation (Tab with exactly one suggestion is accepted immediately). Probe-reproduced at the reviewed commit: the listbox rendered with both options on a bare refocus; adding ++suggestSeqRef.current; in the timer callback flips the probe; no existing test pins this window ("keeps suggestions closed when a lookup finishes after blur" never refocuses; "cancels the pending blur dismiss…" refocuses before the timer fires). Distinct from R6-1 (which suppresses the next edit; this one opens the list with no edit), but R6-1's fix — seq bump instead of arming the flag — subsumes this finding, so no separate suggestion block here: one defect family, one fix.
中文说明
blur 关闭定时器会置位抑制标志，但从不递增 suggestSeqRef，因此用户失焦时仍在飞行中的查询不会被作废。定时器触发之后，重新聚焦是空操作（onFocus 只取消未决的定时器），当这个过期查询返回时，它能通过 seq 检查和新的焦点门控（此时输入框已重新聚焦），并凭借 blur 之前捕获的 openOnResult=true 调用 setListOpen(true) —— 在没有任何编辑的情况下，仅凭重新聚焦就弹出列表。这违反了本 PR 自己的相同值竞争测试所声明的不变量（"so a bare refocus cannot pop the list open"）：该不变量在选择器关闭路径上有实现，在 blur 关闭路径上却被遗漏。— 失败场景：输入 /home/me/co → 等待超过 150 ms 去抖使守护进程查询进入飞行状态（对远程守护进程很常见）→ 点击其他字段 → 100 ms 定时器触发（closeList、标志置位、未递增 seq）→ 在查询返回前重新点击输入框 → 查询返回 → 列表在零编辑时弹出；列表意外打开后，handleInputKeyDown 会把后续按键当作列表导航处理（恰好只有一条建议时 Tab 会立即接受）。已在评审提交上通过探针复现：仅重新聚焦即渲染出包含两个选项的列表框；在定时器回调中加入 ++suggestSeqRef.current; 即可翻转探针；现有测试均未钉住该窗口（"keeps suggestions closed when a lookup finishes after blur" 从不重新聚焦；"cancels the pending blur dismiss…" 在定时器触发前就重新聚焦）。与 R6-1 不同（那个抑制下一次编辑；这个在无编辑时打开列表），但 R6-1 的修复——用 seq 递增替代置位标志——同样覆盖本发现，因此此处不再给出独立的 suggestion 块：同一缺陷家族，一个修复即可。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| closeList(); | ||
| suppressNextFetchOpenRef.current = true; |
There was a problem hiding this comment.
[Suggestion] R6-3: No test pins that the suggestion list closes when Browse is clicked — deleting this closeList() keeps all 31 tests green (mutation run at the reviewed commit; comparator aliveness verified: a sanity mutation in the resolve handler fails 14 of the same 31 tests, so the green mutant run is meaningful, not a dead comparator). — Concrete cost: with the list open (user typed /home/me/co, suggestions visible), a future refactor dropping this call leaves the listbox rendered under the disabled input while the native picker is up, and still open after a cancelled or same-value pick (those branches don't clear suggestions/listOpen) — a git bisect-able regression of this PR's stated "close on picker open" behaviour shipping green. All existing picker tests either start with the list closed or assert nothing between the Browse click and the next edit. Suggested fix: in opens suggestions on the first edit after picking the typed path (the one test whose list is open when Browse is clicked), add expect(listbox()).toBeNull(); immediately after the act(() => { browseButton().click(); }) block.
中文说明
没有测试钉住"点击'浏览'时建议列表会关闭"这一行为——删除这里的 closeList() 后全部 31 个测试仍然通过（已在评审提交上运行变异验证；比较器活性也已确认：在结果处理器中注入一个健全性变异会使同样 31 个测试中的 14 个失败，因此该绿色变异结果是有效的，而非死比较器）。— 具体代价：当列表处于打开状态（用户已输入 /home/me/co，建议可见）时，未来某次重构若删掉该调用，系统选择器弹出期间列表框会继续渲染在被禁用的输入框下方，并且在取消选择或选择相同路径后仍然保持打开（这些分支不会清除 suggestions/listOpen）——本 PR 声明的"打开选择器时关闭列表"行为发生可被 git bisect 定位的回归，却能一路绿灯地合入。现有的选择器测试要么从列表关闭状态开始，要么在"浏览"点击与下一次编辑之间不做任何断言。建议修复：在 opens suggestions on the first edit after picking the typed path（唯一一个点击"浏览"时列表处于打开状态的测试）中，紧跟 act(() => { browseButton().click(); }) 代码块之后加上 expect(listbox()).toBeNull();。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| onFocus={() => { | ||
| if (blurTimeoutRef.current !== undefined) { | ||
| clearTimeout(blurTimeoutRef.current); | ||
| blurTimeoutRef.current = undefined; | ||
| } | ||
| }} |
There was a problem hiding this comment.
[Suggestion] R6-4: The blur-timer cancel block is hand-copied four times in this one component, all introduced by this PR: pickDirectory (lines 173-176), this onFocus handler, onBlur's clear-before-rearm (324-326), and the unmount cleanup effect (88-90) — and one copy has already drifted: the unmount effect clears the timeout without resetting blurTimeoutRef.current = undefined. — Concrete cost: the next semantic change to "cancel the pending dismiss" (e.g. also clearing suppressNextFetchOpenRef on cancel) must locate and update all four copies by hand; missing one reintroduces the exact timer-race class this round fixed — the class that took five review rounds to close. Suggested fix (spans the declaration and three call sites, so no one-click block):
const cancelBlurDismiss = useCallback(() => {
if (blurTimeoutRef.current !== undefined) {
clearTimeout(blurTimeoutRef.current);
blurTimeoutRef.current = undefined;
}
}, []);called from the unmount effect, pickDirectory, and onFocus.
中文说明
blur 定时器的取消代码块在这一个组件中被手写了四份，且全部由本 PR 引入：pickDirectory（第 173-176 行）、这个 onFocus 处理器、onBlur 中先清除再重置的部分（324-326），以及卸载清理 effect（88-90）——其中一份已经出现漂移：卸载 effect 清除了定时器却没有把 blurTimeoutRef.current 重置为 undefined。— 具体代价：下一次对"取消待执行的关闭"的语义修改（例如取消时同时清除 suppressNextFetchOpenRef）必须手工找到并更新全部四处拷贝；漏掉任何一处都会重新引入本轮刚修复的定时器竞争——这类缺陷花了五轮评审才收敛。建议修复（跨声明与三个调用点，因此不使用一键 suggestion 块）：提取如上所示的 cancelBlurDismiss 辅助函数，并在卸载 effect、pickDirectory 与 onFocus 中调用。
— qwen3.8-max via Qwen Code /review (v0.21.9)
…dWorkspaceDialog (#8844)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #8844Commit: Feedback points and dispositionsAll four findings were verified against the reviewed code and addressed. R6-1 [Critical] (rc:3756750996) — fixed. Confirmed: when the 100 ms blur-dismiss timer fired, it armed R6-2 [Critical] (rc:3756751014) — fixed (subsumed by the R6-1 fix, as the finding predicted). Confirmed: the timer never bumped R6-3 [Suggestion] (rc:3756751017) — addressed. Added R6-4 [Suggestion] (rc:3756751030) — addressed. Extracted a single Also refreshed one test comment that referenced the old timer mechanism ("re-set the suppress flag") and the The review-level CHANGES_REQUESTED entry carried only the reviewer's budget-accounting context lines ("all checks completed within budget") — no additional actionable content beyond the four inline findings above. Conflict notes
VerificationCommands actually run this round (repo root unless noted):
Integration tests after 中文说明Autofix 评审处理总结 — PR #8844提交： 反馈点及处理结果四个发现均已对照被评审的代码核实，并全部处理。 R6-1 [Critical]（rc:3756750996）— 已修复。 确认属实：当 100 ms 的 blur 关闭定时器真正触发时，它会置位 R6-2 [Critical]（rc:3756751014）— 已修复（被 R6-1 的修复一并覆盖，与该发现的预判一致）。 确认属实：定时器从不递增 R6-3 [Suggestion]（rc:3756751017）— 已处理。 在 R6-4 [Suggestion]（rc:3756751030）— 已处理。 抽取了单一的 同时刷新了一处引用旧定时器机制的测试注释（"re-set the suppress flag"）以及 评审层面的 CHANGES_REQUESTED 条目只包含评审者的工具预算核算上下文（"all checks completed above were completed within budget"）——除上述四条行内发现外没有其他可操作内容。 冲突说明
验证本轮实际执行的命令（未注明时均在仓库根目录）：
未运行 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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 explored to full depth (tool budget reached): Context: PR #8844 (review round 7) makes the web-shell Ad...: none — did not run the web-shell unit test suite (not needed for the perf judgment; no perf-specific harness exists). Should I mention this? The brief says to d….
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：Context: PR #8844 (review round 7) makes the web-shell Ad...：none — did not run the web-shell unit test suite (not needed for the perf judgment; no perf-specific harness exists). Should I mention this? The brief says to d…。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Cross the blur timer's deadline: a still-pending timer would close | ||
| // the list and leave a stale suppress flag for the next edit. |
There was a problem hiding this comment.
[Suggestion] R7-2: New test comments contradict the shipped blur-timer mechanism. The timer body this PR ships is ++suggestSeqRef.current; closeList(); and never touches suppressNextFetchOpenRef (the handler's own comment says: "Invalidate in-flight lookups via the sequence counter rather than suppressing the next fetch"). But this comment class describes the pre-fix behaviour — a pending timer that "would … leave a stale suppress flag for the next edit" (here and at lines 432-433) / "would have set the suppress flag by now" — and, in the failed-picker test, "Cross the blur timer's deadline before the first edit below" (~line 773) plus "settling before the input's 100 ms blur timer" (~line 762) assert a pending blur timer that does not exist there: pickDirectory calls blur() then synchronously cancelBlurDismiss() before await onPick(), so that 100 ms advance drains nothing blur-related. Probe-verified at the reviewed commit: after an instantly rejected picker, a lookup flown through the 100 ms window is applied (no timer fired); deleting cancelBlurDismiss() flips the probe. — Failure scenario: a maintainer debugging this file's three-ref interplay (suppress flag, sequence counter, blur timer) reads these comments and either chases suppress-flag state the timer never touches, or concludes the failed-picker path leaves a live blur timer that must be crossed — steering a refactor toward re-arming the suppress flag and reintroducing the R6-1-class first-edit leak.
Suggested fix: reword to the shipped mechanism, e.g. "a still-pending timer would close the list and invalidate the next lookup via the sequence counter", "an uncancelled timer would have bumped the sequence counter by now", and for the failed-picker test "No blur timer is pending here — pickDirectory cancels it synchronously, so this advance only drains the debounce window before the first edit below."
中文说明
新增的测试注释与实际交付的 blur 定时器机制相矛盾。本 PR 交付的定时器逻辑是 ++suggestSeqRef.current; closeList();，从不写 suppressNextFetchOpenRef（处理器自己的注释写明："通过序列计数器使在飞行中的查询失效，而不是抑制下一次查询"）。但这类注释描述的是修复前的行为——一个仍未决的定时器"会给下一次编辑留下一个过期的 suppress 标志"（此处及第 432-433 行）/"此刻本应已置位 suppress 标志"；此外在"选择器失败"测试中，"在下方第一次编辑前越过 blur 定时器的截止时间"（约第 773 行）与"在输入框 100 ms blur 定时器之前结束"（约第 762 行）断言了一个实际不存在的未决 blur 定时器：pickDirectory 在 blur() 之后、await onPick() 之前就同步调用了 cancelBlurDismiss()，因此那 100 ms 的推进不会排空任何 blur 相关逻辑。已在评审提交上通过探针验证：立即 reject 的选择器之后，飞过 100 ms 窗口的查询会被应用（定时器未触发）；删除 cancelBlurDismiss() 可翻转探针。— 失败场景：维护者调试本文件中三个 ref 的交互（suppress 标志、序列计数器、blur 定时器）时读到这些注释，要么去追查定时器从不写入的 suppress 标志状态，要么误以为"选择器失败"路径留下了一个必须越过的存活 blur 定时器——从而引导重构重新置位 suppress 标志，重新引入 R6-1 类的"首次编辑被抑制"泄漏。
建议修复：按实际机制改写注释，例如"仍未决的定时器会关闭列表并通过序列计数器使下一次查询失效"、"未被取消的定时器此刻本应已递增序列计数器"，以及"选择器失败"测试中的"此处没有未决的 blur 定时器——pickDirectory 已同步取消它，这次推进只是为下方第一次编辑前排空去抖窗口"。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Cross the first timer's original deadline: an uncancelled timer | ||
| // would have set the suppress flag by now. |
There was a problem hiding this comment.
[Suggestion] R7-2: New test comments contradict the shipped blur-timer mechanism. The timer body this PR ships is ++suggestSeqRef.current; closeList(); and never touches suppressNextFetchOpenRef (the handler's own comment says: "Invalidate in-flight lookups via the sequence counter rather than suppressing the next fetch"). But this comment class describes the pre-fix behaviour — a pending timer that "would … leave a stale suppress flag for the next edit" (here and at lines 432-433) / "would have set the suppress flag by now" — and, in the failed-picker test, "Cross the blur timer's deadline before the first edit below" (~line 773) plus "settling before the input's 100 ms blur timer" (~line 762) assert a pending blur timer that does not exist there: pickDirectory calls blur() then synchronously cancelBlurDismiss() before await onPick(), so that 100 ms advance drains nothing blur-related. Probe-verified at the reviewed commit: after an instantly rejected picker, a lookup flown through the 100 ms window is applied (no timer fired); deleting cancelBlurDismiss() flips the probe. — Failure scenario: a maintainer debugging this file's three-ref interplay (suppress flag, sequence counter, blur timer) reads these comments and either chases suppress-flag state the timer never touches, or concludes the failed-picker path leaves a live blur timer that must be crossed — steering a refactor toward re-arming the suppress flag and reintroducing the R6-1-class first-edit leak.
Suggested fix: reword to the shipped mechanism, e.g. "a still-pending timer would close the list and invalidate the next lookup via the sequence counter", "an uncancelled timer would have bumped the sequence counter by now", and for the failed-picker test "No blur timer is pending here — pickDirectory cancels it synchronously, so this advance only drains the debounce window before the first edit below."
中文说明
新增的测试注释与实际交付的 blur 定时器机制相矛盾。本 PR 交付的定时器逻辑是 ++suggestSeqRef.current; closeList();，从不写 suppressNextFetchOpenRef（处理器自己的注释写明："通过序列计数器使在飞行中的查询失效，而不是抑制下一次查询"）。但这类注释描述的是修复前的行为——一个仍未决的定时器"会给下一次编辑留下一个过期的 suppress 标志"（此处及第 432-433 行）/"此刻本应已置位 suppress 标志"；此外在"选择器失败"测试中，"在下方第一次编辑前越过 blur 定时器的截止时间"（约第 773 行）与"在输入框 100 ms blur 定时器之前结束"（约第 762 行）断言了一个实际不存在的未决 blur 定时器：pickDirectory 在 blur() 之后、await onPick() 之前就同步调用了 cancelBlurDismiss()，因此那 100 ms 的推进不会排空任何 blur 相关逻辑。已在评审提交上通过探针验证：立即 reject 的选择器之后，飞过 100 ms 窗口的查询会被应用（定时器未触发）；删除 cancelBlurDismiss() 可翻转探针。— 失败场景：维护者调试本文件中三个 ref 的交互（suppress 标志、序列计数器、blur 定时器）时读到这些注释，要么去追查定时器从不写入的 suppress 标志状态，要么误以为"选择器失败"路径留下了一个必须越过的存活 blur 定时器——从而引导重构重新置位 suppress 标志，重新引入 R6-1 类的"首次编辑被抑制"泄漏。
建议修复：按实际机制改写注释，例如"仍未决的定时器会关闭列表并通过序列计数器使下一次查询失效"、"未被取消的定时器此刻本应已递增序列计数器"，以及"选择器失败"测试中的"此处没有未决的 blur 定时器——pickDirectory 已同步取消它，这次推进只是为下方第一次编辑前排空去抖窗口"。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| 'Unable to open the system folder picker', | ||
| ); | ||
|
|
||
| // Cross the blur timer's deadline before the first edit below. |
There was a problem hiding this comment.
[Suggestion] R7-2: New test comments contradict the shipped blur-timer mechanism. The timer body this PR ships is ++suggestSeqRef.current; closeList(); and never touches suppressNextFetchOpenRef (the handler's own comment says: "Invalidate in-flight lookups via the sequence counter rather than suppressing the next fetch"). But this comment class describes the pre-fix behaviour — a pending timer that "would … leave a stale suppress flag for the next edit" (here and at lines 432-433) / "would have set the suppress flag by now" — and, in the failed-picker test, "Cross the blur timer's deadline before the first edit below" (~line 773) plus "settling before the input's 100 ms blur timer" (~line 762) assert a pending blur timer that does not exist there: pickDirectory calls blur() then synchronously cancelBlurDismiss() before await onPick(), so that 100 ms advance drains nothing blur-related. Probe-verified at the reviewed commit: after an instantly rejected picker, a lookup flown through the 100 ms window is applied (no timer fired); deleting cancelBlurDismiss() flips the probe. — Failure scenario: a maintainer debugging this file's three-ref interplay (suppress flag, sequence counter, blur timer) reads these comments and either chases suppress-flag state the timer never touches, or concludes the failed-picker path leaves a live blur timer that must be crossed — steering a refactor toward re-arming the suppress flag and reintroducing the R6-1-class first-edit leak.
Suggested fix: reword to the shipped mechanism, e.g. "a still-pending timer would close the list and invalidate the next lookup via the sequence counter", "an uncancelled timer would have bumped the sequence counter by now", and for the failed-picker test "No blur timer is pending here — pickDirectory cancels it synchronously, so this advance only drains the debounce window before the first edit below."
中文说明
新增的测试注释与实际交付的 blur 定时器机制相矛盾。本 PR 交付的定时器逻辑是 ++suggestSeqRef.current; closeList();，从不写 suppressNextFetchOpenRef（处理器自己的注释写明："通过序列计数器使在飞行中的查询失效，而不是抑制下一次查询"）。但这类注释描述的是修复前的行为——一个仍未决的定时器"会给下一次编辑留下一个过期的 suppress 标志"（此处及第 432-433 行）/"此刻本应已置位 suppress 标志"；此外在"选择器失败"测试中，"在下方第一次编辑前越过 blur 定时器的截止时间"（约第 773 行）与"在输入框 100 ms blur 定时器之前结束"（约第 762 行）断言了一个实际不存在的未决 blur 定时器：pickDirectory 在 blur() 之后、await onPick() 之前就同步调用了 cancelBlurDismiss()，因此那 100 ms 的推进不会排空任何 blur 相关逻辑。已在评审提交上通过探针验证：立即 reject 的选择器之后，飞过 100 ms 窗口的查询会被应用（定时器未触发）；删除 cancelBlurDismiss() 可翻转探针。— 失败场景：维护者调试本文件中三个 ref 的交互（suppress 标志、序列计数器、blur 定时器）时读到这些注释，要么去追查定时器从不写入的 suppress 标志状态，要么误以为"选择器失败"路径留下了一个必须越过的存活 blur 定时器——从而引导重构重新置位 suppress 标志，重新引入 R6-1 类的"首次编辑被抑制"泄漏。
建议修复：按实际机制改写注释，例如"仍未决的定时器会关闭列表并通过序列计数器使下一次查询失效"、"未被取消的定时器此刻本应已递增序列计数器"，以及"选择器失败"测试中的"此处没有未决的 blur 定时器——pickDirectory 已同步取消它，这次推进只是为下方第一次编辑前排空去抖窗口"。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Set when a suggestion is accepted or the list is dismissed, so the | ||
| // path-change effect knows whether to reopen the list for that update. | ||
| const suppressNextFetchOpenRef = useRef(false); |
There was a problem hiding this comment.
[Suggestion] R7-3: The suppressNextFetchOpenRef declaration comment still names blur dismissal as a setter ("Set when a suggestion is accepted or the list is dismissed"), but this diff removed that setter — the blur timer now bumps suggestSeqRef instead. The only remaining = true write is pickDirectory's Browse flow, and acceptSuggestion clears the flag rather than setting it. The comment now contradicts the blur handler's comment twenty lines below. — Failure scenario: a maintainer adding a future dismiss path on the strength of this ref comment would set the flag and reintroduce exactly the first-edit suppress leak (R6-1 class) the blur handler's comment warns against.
| // Set when a suggestion is accepted or the list is dismissed, so the | |
| // path-change effect knows whether to reopen the list for that update. | |
| const suppressNextFetchOpenRef = useRef(false); | |
| // Set while Browse is in flight, so the path-change effect keeps the | |
| // pick-triggered lookup closed until the first edit; blur dismissal | |
| // invalidates in-flight lookups via suggestSeqRef instead. | |
| const suppressNextFetchOpenRef = useRef(false); |
中文说明
suppressNextFetchOpenRef 的声明注释仍把 blur 关闭列为置位方（"当建议被接受或列表被关闭时置位"），但本次 diff 已移除该置位方——blur 定时器现在改为递增 suggestSeqRef。仅剩的 = true 写入位于 pickDirectory 的"浏览"流程中，且 acceptSuggestion 是清除而非置位该标志。该注释现在与下方约二十行处 blur 处理器的注释直接矛盾。— 失败场景：维护者依据此 ref 注释新增一条关闭路径时会置位该标志，从而重新引入 blur 处理器注释所警告的"首次编辑被抑制"泄漏（R6-1 类）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| onSuggest={(prefix) => | ||
| workspaceActions.suggestWorkspacePaths(prefix) | ||
| } | ||
| onSuggest={workspaceActions.suggestWorkspacePaths} |
There was a problem hiding this comment.
[Suggestion] R7-4: The switch to the stable onSuggest reference is load-bearing but unpinned by any App-level test. The dialog's fetch effect lists onSuggest in its deps [path, onSuggest, closeList], bumps suggestSeqRef on every run, and clears the 150 ms debounce in its cleanup — so a fresh per-render identity here restarts the debounce and invalidates in-flight lookups on every App re-render. Stability holds today only because DaemonWorkspaceProvider memoizes the actions object on [baseUrl, token]; App.test.tsx fully mocks the dialog, captures its props, and asserts the onPick/onClose/onAdd wiring, but never onSuggest. — Failure scenario: a future change re-introduces a per-render closure (the exact shape this diff removes) or widens the provider memo's deps → during streaming, when App re-renders constantly, every re-render while the dialog is open re-runs the fetch effect, so workspace suggestions churn and effectively never appear; the suite cannot detect the regression in either direction.
Suggested fix: in the existing App-level add-workspace test, assert latestAddWorkspaceDialogProps.onSuggest is mockWorkspaceActions.suggestWorkspacePaths and remains referentially identical across a re-render.
中文说明
改用稳定的 onSuggest 引用是承重的，但没有任何 App 层测试将其钉住。对话框的 fetch effect 把 onSuggest 列在依赖 [path, onSuggest, closeList] 中，每次运行都递增 suggestSeqRef，并在清理函数中清除 150 ms 去抖定时器——因此此处若重新出现每次渲染都变化的引用，App 每次重渲染都会重启去抖并使在飞行中的查询失效。当前的稳定性仅因 DaemonWorkspaceProvider 以 [baseUrl, token] 为依赖记忆化 actions 对象而成立；App.test.tsx 完整 mock 了该对话框、捕获其 props，并断言了 onPick/onClose/onAdd 的接线，却从未断言 onSuggest。— 失败场景：未来某次改动重新引入每次渲染都变化的闭包（正是本 diff 移除的形态）或放宽 provider 记忆化的依赖 → 流式输出期间 App 持续重渲染，对话框打开时的每次重渲染都会重跑 fetch effect，工作区建议将不断抖动、实际上永远无法出现；且现有测试套件无法从任何方向检测到该回归。
建议修复：在现有的 App 层"添加工作区"测试中，断言 latestAddWorkspaceDialogProps.onSuggest 即 mockWorkspaceActions.suggestWorkspacePaths，且在一次重渲染后引用保持同一。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| ++suggestSeqRef.current; | ||
| closeList(); | ||
| }, 100); |
There was a problem hiding this comment.
[Suggestion] R7-5: Blur dismiss leaves stale suggestions that ArrowDown reopens. The blur timer bumps suggestSeqRef and closes the list, but the fetch callback's seq check returns BEFORE setSuggestions(...), so suggestions keeps the previous prefix's entries until the next edit. handleInputKeyDown's ArrowDown branch reopens a closed list whenever suggestions.length > 0 — so a bare refocus + ArrowDown pops entries that don't match the typed path, and Enter/Tab then accepts a wrong directory into the field. Diff-introduced: pre-diff, this blur sequence let the lookup resolve and refresh suggestions (at the cost of the pop-open bug this PR fixes); the seq-based invalidation is what makes the stale entries persist. — Failure scenario: type /home/me/co (list shows results A) → type /home/me/cod (lookup in flight) → blur past the 100 ms window (timer fires) → the stale lookup early-returns before setSuggestions → refocus without typing → ArrowDown reopens the list showing A against the input /home/me/cod → Enter accepts /home/me/code/ into the field. Probe-reproduced end-to-end at the reviewed commit; adding setSuggestions([]) flips the probe.
| ++suggestSeqRef.current; | |
| closeList(); | |
| }, 100); | |
| ++suggestSeqRef.current; | |
| setSuggestions([]); | |
| closeList(); | |
| }, 100); |
中文说明
blur 关闭会留下过期的建议数据，ArrowDown 可据此重新打开列表。blur 定时器递增 suggestSeqRef 并关闭列表，但 fetch 回调中的 seq 检查在 setSuggestions(...) 之前就 return，因此 suggestions 会保留上一个前缀的条目直到下一次编辑。handleInputKeyDown 的 ArrowDown 分支在 suggestions.length > 0 时会重新打开已关闭的列表——于是仅重新聚焦 + ArrowDown 就会弹出与当前输入不匹配的条目，Enter/Tab 随即把一个错误的目录接受进输入框。该问题由本 diff 引入：修改前，这一 blur 时序会让查询正常返回并刷新 suggestions（代价正是本 PR 要修复的自动弹出缺陷）；基于序列计数器的作废机制才使过期条目得以长期残留。— 失败场景：输入 /home/me/co（列表显示结果 A）→ 输入 /home/me/cod（查询在飞行中）→ 失焦超过 100 ms 窗口（定时器触发）→ 过期查询在 setSuggestions 前提前返回 → 不做任何编辑直接重新聚焦 → ArrowDown 重新打开列表，输入框为 /home/me/cod 却显示 A → Enter 把 /home/me/code/ 接受进输入框。已在评审提交上端到端探针复现；加入 setSuggestions([]) 可翻转探针。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #8844 (review round 7)All four distinct findings (six inline comments) were verified against the reviewed commit and resolved in one commit: Feedback points and dispositions
Conflict notes
Verification
中文说明Autofix 评审处理总结 — PR #8844（评审第 7 轮）全部四条独立发现（六条行内评论）均已在被评审提交上核实，并通过一次提交解决： 反馈点及处理
冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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): Context: PR #8844 (web-shell) closes workspace path sugge...: none — all checks I opened were completed within budget.; Context: PR #8844 (web-shell) closes workspace path sugge...: none — all checks I started were completed within budget.; Context: PR #8844 (web-shell) closes workspace path sugge...: none — all checks above were completed within budget.; Context: PR #8844 (web-shell) closes workspace path sugge...: the exact trigger of the flaky shadow DOM tests has not been identified (could not be reproduced in 11 subsequent runs).; Context: PR #8844 (web-shell) closes workspace path sugge...: Windows FolderBrowserDialog's UNC output has not been empirically verified (based on code reading only)., and 3 more.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：Context: PR #8844 (web-shell) closes workspace path sugge...：none — all checks I opened were completed within budget.；Context: PR #8844 (web-shell) closes workspace path sugge...：none — all checks I started were completed within budget.；Context: PR #8844 (web-shell) closes workspace path sugge...：none — all checks above were completed within budget.；Context: PR #8844 (web-shell) closes workspace path sugge...：the exact trigger of the flaky shadow DOM tests has not been identified (could not be reproduced in 11 subsequent runs).；Context: PR #8844 (web-shell) closes workspace path sugge...：Windows FolderBrowserDialog's UNC output has not been empirically verified (based on code reading only).，另有 3 条。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| getShadowAwareActiveElement(input) === input && | ||
| (openOnResult || listOpenRef.current) |
There was a problem hiding this comment.
[Suggestion] The new input-focus gate has no isolating test — deleting this conjunct leaves all 34 tests green (mutant verified by running the suite), yet the gate is load-bearing. — Failure scenario: user types an absolute path and clicks Browse while the lookup is debounced/in flight; pickDirectory closes the list and cancels the blur timer without bumping the sequence counter, so when the lookup resolves while the native picker is open (input blurred), only this gate prevents setListOpen(true). A future refactor that drops the gate ships a reopen-under-the-picker bug with every test still green. Second trigger: Escape closes the list without bumping seq, then blur, then a fast resolve inside the 100 ms window.
Suggested regression test (inverse order of the existing race test): type /home/me/co, click Browse immediately (lookup still debounced), advance timers so the lookup is in flight while the picker is up, resolve the suggestions while the picker is still open and assert listbox() is null; then resolve the pick with undefined and assert it is still null.
中文说明
新的输入框焦点门控没有专门的隔离测试——删除这个条件后全部 34 个测试仍然通过（已通过运行该变异体验证），但该门控是承重的。— 失败场景：用户输入绝对路径后，在查询处于去抖/飞行状态时点击“浏览”；pickDirectory 会关闭列表并取消 blur 定时器，但不递增序列计数器，因此当查询在系统选择器打开期间返回（输入框已失焦）时，只有这个门控能阻止 setListOpen(true)。未来某次删除该门控的重构会带着所有测试仍然绿灯 shipped 一个“选择器打开期间列表被重新打开”的缺陷。第二个触发路径：Escape 关闭列表（未递增 seq）→ blur → 查询在 100 ms 窗口内快速返回。
建议补充回归测试（与现有竞争测试相反的顺序）：输入 /home/me/co，立即点击“浏览”（查询仍在去抖中），推进定时器使查询在选择器打开期间处于飞行状态，在选择器仍打开时让建议返回并断言 listbox() 为 null；再以 undefined 结束选择并断言列表仍为 null。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| if (pickedPath && pickedPath !== path) { | ||
| // Leave the suppress flag set: the path-change effect consumes it, | ||
| // keeping the pick-triggered lookup closed until the first edit. |
There was a problem hiding this comment.
[Suggestion] pickDirectory leaves suppressNextFetchOpenRef set for the path-change effect to consume, but the fetch effect only consumes it after the isAbsoluteLike(path) guard — a picked path that fails that check (e.g. a Windows UNC \\server\share from FolderBrowserDialog) early-returns with the flag still set, leaking it into the user's next edit. Probe-verified on this tree: the first edit does not open the list; adding the reset flips the probe with all 34 tests still green. Diff-introduced (pre-diff pickDirectory never set the flag). — Failure scenario: on Windows, Browse → pick a network share → setPath runs the effect's early-return branch, leaving the flag true → the user clears the field and types an absolute prefix → the first fetch computes openOnResult=false, so the suggestion list does not open until the second edit, violating the "first edit must open" invariant this PR establishes for every other picker outcome. Mitigation: handleSubmit rejects non-absolute-like paths, so this is a self-healing one-time UX paper cut.
Fix — consume the flag in the fetch effect's early-return branch (~line 104):
if (!isAbsoluteLike(path)) {
suppressNextFetchOpenRef.current = false;
setSuggestions([]);
closeList();
return undefined;
}(or reset it in pickDirectory when !isAbsoluteLike(pickedPath))
中文说明
pickDirectory 把 suppressNextFetchOpenRef 置位后留给路径变化 effect 去消费，但 fetch effect 只在通过 isAbsoluteLike(path) 守卫之后才消费该标志——如果所选路径不满足该检查（例如 Windows 上 FolderBrowserDialog 返回的 UNC 路径 \\server\share），effect 会在标志仍为 true 时提前返回，使它泄漏到用户的下一次编辑。已在本代码树上通过探针验证：第一次编辑不会打开列表；补上重置后探针翻转，且全部 34 个现有测试仍然通过。此问题由本 diff 引入（修改前 pickDirectory 从不置位该标志）。— 失败场景：在 Windows 上点击“浏览” → 选择一个网络共享 → setPath 走 effect 的提前返回分支，标志保持 true → 用户清空输入框并输入绝对路径前缀 → 第一次查询算出 openOnResult=false，建议列表要到第二次编辑才打开，违反了本 PR 为其他所有选择器结果确立的“首次编辑必须打开”不变量。缓解因素：handleSubmit 同样拒绝非绝对路径形式的值，因此这是一次性且可自愈的轻微体验问题。
修复——在 fetch effect 的提前返回分支（约第 104 行）中消费该标志（代码见上），或在 pickDirectory 中当 !isAbsoluteLike(pickedPath) 时重置。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // A same-value pick keeps the typed path, so invalidate any lookup | ||
| // already in flight from before Browse was clicked. | ||
| if (pickedPath) ++suggestSeqRef.current; |
There was a problem hiding this comment.
[Suggestion] The else branch invalidates a pre-Browse in-flight lookup only for a same-value pick; the sibling outcomes grouped in the same comment (cancelled or failed picker) get the identical state transition but no seq invalidation. Probe-verified: cancelled picker + refocus pops the list open on zero edits; making the bump unconditional flips the probe with all 34 tests still green. — Failure scenario: type /home/me/co → lookup in flight → click Browse → cancel the picker (pickedPath undefined, no seq bump) → refocus → the stale lookup resolves with the pre-Browse captured openOnResult=true and passes the focus gate → setListOpen(true) pops the list open on a bare refocus with zero edits — exactly the outcome the PR's own same-value race test prevents for the sibling branch. Outcome is benign (entries match the unchanged path), but the guard is asymmetric between branches the comment treats as one family.
| // A same-value pick keeps the typed path, so invalidate any lookup | |
| // already in flight from before Browse was clicked. | |
| if (pickedPath) ++suggestSeqRef.current; | |
| // Cancelled, failed, and same-value picks keep the typed path, so | |
| // invalidate any lookup already in flight from before Browse. | |
| ++suggestSeqRef.current; |
中文说明
else 分支只在“选择相同路径”时作废“浏览”之前已在飞行中的查询；同一段注释归为一类的兄弟结果（取消或失败的选择器）经历完全相同的状态转换，却没有序列作废。已通过探针验证：取消选择器 + 重新聚焦会在零编辑时弹出列表；把递增改为无条件后探针翻转，且全部 34 个现有测试仍然通过。— 失败场景：输入 /home/me/co → 查询进入飞行 → 点击“浏览” → 取消选择器（pickedPath 为 undefined，未递增 seq）→ 重新聚焦 → 过期查询带着“浏览”前捕获的 openOnResult=true 返回并通过焦点门控 → setListOpen(true) 在零编辑时仅凭重新聚焦就弹出列表——这正是本 PR 自己的相同值竞争测试为兄弟分支所阻止的结果。后果本身无害（条目与未改变的路径一致），但注释视为同一族的两个分支之间，守卫是不对称的。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no actionThis round found nothing actionable on PR #8844:
The PR is in Critical-only mode after five change-producing rounds. The remaining non-Critical feedback (one automated review and three inline suggestions on No commit was created; the branch stays at 中文说明Autofix 轮次：无需操作本轮在 PR #8844 上未发现任何可处理的内容：
该 PR 在经过 5 个产生改动的轮次后已进入仅处理 Critical 的模式。剩余的非 Critical 反馈（一条自动化评审，以及针对 未创建任何提交；分支保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Maintainer verification report (local, real stack)Verified head 1. Unit tests & build (PR head)
2. Scenario A — list must close on blur and stay closedTyped
The before-arm reopen is driven by the per-render
3. Scenario B — Browse fills the path and keeps the list closedTyped
4. Side observation — duplicate daemon queries eliminatedFor the identical interaction sequence, the before arm issued 12 Notes / caveats
Conclusion: works as described on a real daemon + real browser; before/after difference is exactly the fixed behavior; new tests fail on the un-fixed code. LGTM from a verification standpoint. 中文版本（Chinese version）维护者本地真实环境验证报告在 macOS 上对 head 1. 单元测试与构建(PR head)
2. 场景 A——失焦后列表必须关闭且保持关闭输入
before 的重开由逐 render 新建的 3. 场景 B——Browse 填入路径且列表保持关闭输入
4. 附带发现——重复 daemon 查询被消除相同交互序列下,before arm 发出 12 次 备注
结论:在真实 daemon + 真实浏览器下行为与描述一致,before/after 差异恰为所修复的行为,新测试在未修复代码上失败。验证角度 LGTM。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no action neededThis round's feedback contains no actionable items:
No code changes were made; the branch remains at 中文说明Autofix 本轮：无需处理本轮反馈中没有任何可执行条目：
未做任何代码改动；分支保持在 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 189 passed · 0 failed · 189 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：189 通过 · 0 失败 · 189 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at head Ran the PR in an isolated, token-free container: A/B against the base build and the round-1 head, mock-free harness assertions, mutation matrix, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 189 passed · 0 failed · 189 total 中文 — 判定：✅ 通过 · 可合入（agent 判定，第二轮）沙箱验证在隔离、无凭证的容器中复测了该 PR 的新 head（
Verification reportPR 8844 deep verification (round 2) —
|
| # | finding (round 1) | severity | status at new head |
|---|---|---|---|
| R1-1 | After a blur dismiss, the first keystroke once the input is refetched fetches but does not show suggestions (pre-existing suppress-flag quirk) | informational | fixed — cell B1: base red, prev red, head green; pinned by the PR's new test opens suggestions on the first edit after the blur dismiss fired; the new guards that fix it are pinned (M5/M6/M7 kills) |
| R1-2 | Restoring onMouseDown preventDefault on Browse survives the suite (jsdom .click() fires no mousedown) |
informational | stands — clause still present; mutant M10 still survives (34/34); classification unchanged (unpinnable in jsdom), and at the new head it is additionally superseded: pickDirectory blurs and cancels the dismiss timer explicitly, so the clause's removal is behaviorally inert in the production flow too |
| R1-3 (correction) | Description said "focused Web Shell tests (23/23)" while the file had 24 tests | correction | stands, now wider — the description still says 23/23; the file now contains 34 tests (the five delta commits added 10). All 34 pass (measured). Description correction only |
Declined/deferred rows from round 1: none.
1. Central claim and A/B proof
Central claim. Once the workspace-path input loses focus, the suggestion list closes and stays closed — a delayed lookup completing after blur must not reopen it, and the Browse flow must fill the selected path with the input unfocused and no list.
Delta under test (5 commits since round 1). 0e80df09 cancels the 100 ms blur-dismiss timer inside pickDirectory and applies close+suppress synchronously, with outcome-dependent suppress handling; 2ff92ea2 adds the re-blur cancellation test; b5037580 cancels the timer on focus and unmount; 797a75f8 replaces the blur-time suppressNextFetchOpenRef=true with a sequence-counter bump (invalidating in-flight lookups instead of poisoning the next fetch); ff555e0e clears stale suggestions on blur dismiss and extracts getShadowAwareActiveElement into utils/dom.ts (also used by two App.tsx focus sites), plus pins onSuggest to the memoized workspace action so the dialog's fetch effect no longer restarts on every App render.
The same 21-cell harness (harness/zz-verify-ab.test.tsx, fake timers, controlled prop promises, plain-document and shadow-root portal modes) ran in three trees — base worktree (HEAD^1), round-1-head worktree (67c0e3db), and the merge head — with per-arm expectations encoded, so every bug cell is an assertion that the build misbehaves as predicted (predicted reds count as passing assertions). 63/63 matched:
| cell | oracle | base 16ff42bd |
prev 67c0e3db |
head ff555e0e |
|---|---|---|---|---|
| C00 input focused on mount | activeElement | focused | focused | focused |
| A1 typing opens suggestions | listbox | opens | opens | opens |
| A2 lookup resolves after blur | listbox | reopens | closed | closed |
| A3 Browse releases focus | activeElement | retained | released | released |
| A4 list visible while picker open | listbox | visible | closed | closed |
| A5 list reopens after fill | listbox | reopens | closed | closed |
| A6 shadow-DOM portal, focused input | listbox | opens | opens | opens |
| B1 first edit after dismiss opens | listbox | closed | closed | opens |
| B2a blurred when late lookup lands | listbox | reopens | closed | closed |
| B2b ArrowDown on refocus | listbox | reopens | reopens | closed |
| B3a quick pick (50 ms), before edit | listbox | opens | closed | closed |
| B3b first edit after quick pick | listbox | opens | closed | opens |
| B4 re-blur within window, then edit | listbox | closed | closed | opens |
| B5 unmount with pending dismiss | no throw | ok | ok | ok |
| B6 pre-blur lookup resolves after refocus | listbox | pops | pops | closed |
| B7 first edit after same-value pick | listbox | opens | closed | opens |
| B8 first edit after cancelled picker | listbox | opens | closed | opens |
| B9a picker error surfaced | alert | shown | shown | shown |
| B9b first edit after picker error | listbox | opens | closed | opens |
| G1 lookup resolves in 100 ms grace window | listbox | reopens | closed | closed |
| G2 in-flight lookup resolves during picker | listbox | reopens | closed | closed |
Witness: evidence/01-ab-three-arm-cells.png (live re-run of all three arms printing the full CHECK grid).
Attribution. Base reproduces every symptom the PR describes. Round-1 head fixed the original five document-mode cells but left six residual defects, all visible as red prev cells: the leaked dismiss timer set suppress=true after cancel/error/same-value outcomes (B3b, B7, B8, B9b), stale entries survived dismissal (B2b), a pre-blur lookup could pop the list open after refocus (B6), and stacked re-blur timers were never cancelled (B4). The new head turns all six green while keeping every round-1 fix green. Each delta commit's diff (object-level, trees available despite the shallow checkout) matches its headline; git rev-list HEAD^1..HEAD^2 returns 1 (shallow-boundary artifact) while all 11 commit objects from the snapshot exist locally with intact parent chains — per-commit attribution for the five delta commits was exercised at the object level.
2. Corrections
- The description's evidence line still says "focused Web Shell tests (23/23)". At this head the file contains 34 tests, all passing (measured, §4). Description correction only — no code change implied.
3. Vacuity check and mutation matrix (PR's own tests, at head)
Unmutated control M0 green (34/34). Single-point mutants of the production files, scored against the PR's unmodified test file by harness/score.mjs (116/116 scripted expectations) and re-run live for the capture:
| mutant | change | result | killed by (behavioural) |
|---|---|---|---|
| M1 | focus gate reverted to if (openOnResult || listOpenRef.current) |
survives (34/34) | — (see finding 1) |
| M2 | gate reads document.activeElement |
killed (1) | opens suggestions for a focused input in a shadow-DOM portal root — expected null not to be null |
| M3 | inputRef.current?.blur() removed from pickDirectory |
killed (1) | opens the system picker and fills the selected absolute path — focus assertion |
| M4 | cancelBlurDismiss() removed from pickDirectory |
survives (34/34) | — (classified §5) |
| M5 | ++suggestSeqRef.current removed from blur timer |
killed (2) | keeps suggestions closed when a pre-blur lookup resolves after refocus + drops stale suggestions… — expected <ul …> to be null |
| M6 | setSuggestions([]) removed from blur timer |
killed (1) | drops stale suggestions on blur dismiss… |
| M7 | onFocus={cancelBlurDismiss} removed |
killed (2) | cancels the pending blur dismiss when focus returns… + …first edit after a re-blur… |
| M8 | unmount cleanup effect removed | survives (34/34) | — (classified §5) |
| M9 | input !== null && guard removed |
survives (34/34) | — (classified §5) |
| M10 | Browse onMouseDown preventDefault removed |
survives (34/34) | — (R1-2 stands) |
| M11 | getShadowAwareActiveElement → document.activeElement |
killed (1) | shadow-DOM test (positive control at the shared seam) |
| M12 | App.tsx onSuggest back to a per-render closure |
killed (1) | opens one App-owned Add workspace dialog… — expected [Function onSuggest] to be [Function spy] |
Witness: evidence/03-mutation-matrix-live.png (live M0–M12 run), evidence/02-cross-arm-suite-matrix.png (scripted scoring of the cross-arm suite runs and the matrix).
Cross-arm suite oracle. The head test file run against all three arms: head 34/34 green; base fails 9, prev fails 9 — same count, different sets (6 shared): base-only reds (keeps suggestions closed when a lookup finishes after blur, keeps the pick-triggered lookup closed until the first edit, opens the system picker and fills the selected absolute path) are exactly the round-1 fixes; prev-only reds (does not pop suggestions open on refocus when a same-value pick raced an in-flight lookup, opens suggestions on the first edit after a cancelled picker, shows an error when the system picker fails) are exactly the delta commits' fixes. Every new guard is pinned by the test its commit says it strengthens; no mutant regressed from killed to survived versus round 1's matrix.
4. Findings
- The focus gate is load-bearing but not pinned by the suite (Suggestion). Removing it (M1) leaves the 34-test suite green, yet the 21-cell harness flips cells G1 and G2 red under M1 (19/21): a lookup resolving inside the 100 ms blur grace window, or an in-flight lookup resolving while the picker is open, would pop the list open while the input is unfocused. The sequence counter covers post-dismiss resolutions; the gate covers these two windows. The code is correct — this is a coverage gap. A pinning fixture: type a prefix, let the debounce fire, blur, advance 50 ms (inside the grace window), resolve the lookup, and assert
listbox()is null. Not a merge condition. - Surviving mutants classified (informational). M4 (
cancelBlurDismissinpickDirectory): behaviorally equivalent — the harness runs 21/21 under it, because at this head the leaked timer no longer poisonssuppressNextFetchOpenRef(the author's own test comment states this); redundant defense-in-depth. M8 (unmount cleanup) and M9 (input !== null): React 18+ makes post-unmountsetStatea no-op, so the consequence is unobservable in any environment; defensive hygiene. M10: see R1-2. - Description drift (informational) — see Corrections.
No blocking findings. The PR's risk note ("suggestions continue to be fetched and updated while unfocused, but cannot become visible until existing input interactions open them again") holds at head for every probed window except the intended ones (focus returned within the grace window; refocus after a pick, where ArrowDown shows the picked path's suggestions for fine-tuning).
5. Not covered
- Real native directory picker:
onPickis host-provided and was driven with controlled promises at the prop seam; picker UX verified only by the author on macOS. - Real-browser pointer semantics (mousedown-driven focus movement, click-outside): jsdom does not simulate them; cells drive
.click()/.blur()directly. This is why M10 survives and why the Reviewer Test Plan's "click outside" step was exercised as ablur()event — the wire shape is reproduced, the pointer mechanism is not. - Repo-wide gates not run; gates scoped to
packages/web-shell(the effective diff's only workspace): 181 files / 3281/3281 tests (junit-verified),tsc -p tsconfig.json --noEmit,eslinton the five changed files,prettier --check— each proven live with a planted violation first (evidence/04-gates-live.png). The M12 filter run skipped the other 385 App.test tests; the full file ran green inside the workspace gate. - Playwright e2e suites not run — unit-level A/B chosen instead.
- Per-commit attribution for commits 1–6 comes from round 1 plus the prev arm; the five delta commits were diffed at the object level (all trees present locally;
rev-listundercounts at the shallow boundary).
6. Methodology
Environment: CI verify container (node:22-bookworm), Node 22.23.2, vitest 3.2.4, jsdom 26.1.0; repo pre-installed and pre-built at the merge commit. A/B arms ran in git worktrees (HEAD^1, 67c0e3db) reusing the root node_modules — clean control: the effective diff leaves package-lock.json untouched, and the dialog's import closure contains no @qwen-code/* packages (grep-verified; the internal symlinks in node_modules/@qwen-code/* point at the head tree but are never resolved by the harness). The harness renders the real component through real react-dom into jsdom (plain document and shadow-root portal modes), drives it with fake timers and controlled prop promises, and records every check as CHECK|arm|cell|PASS/FAIL lines plus a per-arm tally; a predicted-red control cell counts as a passing assertion. Cross-arm suite runs and the mutation matrix were scored programmatically by harness/score.mjs (116/116), which parses the saved vitest logs (ANSI-stripped) and compares per-test outcomes to encoded expectations. Raw logs, harness sources, mutants and captures live under logs/, harness/, evidence/ in this artifact directory. The dom-probe capture's shadow-mode|false console line was an over-strict compound print in the probe (it wrongly expected the host element to resolve through its own shadow root); the two real assertions of that case passed (utility resolves the inner element for nodes inside the shadow root; document.activeElement retargets to the host).
Evidence images
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — PR #8844 (no action)Nothing to address this round; no changes were made.
中文说明Autofix 轮次总结 — PR #8844（无操作）本轮没有需要处理的内容，未做任何改动。
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Released in v0.21.11. |
5 participants
What this PR does
Closes workspace path suggestions whenever the path input loses focus. Directory suggestions are allowed to open only while the input is still active, so a delayed lookup cannot reopen the list after the user clicks elsewhere. The Browse action now releases input focus before opening the native directory picker and keeps the list closed after filling the selected path.
Why it's needed
After a user typed a workspace path, the suggestion popover could remain visible even after the user clicked outside the input. The Browse button also prevented focus from leaving the path input, so selecting a directory could leave the popover visible or allow a delayed lookup to reopen it with the selected directory as a redundant option. The UI therefore looked as if the user still needed to choose another directory after already entering or selecting the intended path.
Reviewer Test Plan
How to verify
Open Add Workspace, type an absolute path prefix, and wait for directory suggestions to appear. Click outside the path input and confirm the input loses focus and the list closes; it must remain closed if a pending lookup completes afterward. Then click Browse…, select a directory, and confirm the selected absolute path is filled while the input remains unfocused and no suggestion list opens.
Evidence (Before & After)
Before: clicking Browse kept the path input focused, and a delayed autocomplete result could reopen the list with the selected directory. A lookup completing after an ordinary blur could also reopen the list.
After: blur closes the list, delayed results cannot open it while the input is inactive, and Browse fills the selected path with focus outside the input.
Tested on
Environment (optional)
macOS; focused Web Shell tests (23/23), Web Shell production build, TypeScript typecheck, ESLint, and Prettier.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 的改动
工作区路径输入框失去焦点时关闭目录建议。只有输入框仍处于激活状态时，目录建议才允许打开，因此延迟返回的查询不会在用户点击其他位置后重新打开列表。“浏览”操作现在会在打开系统目录选择器前释放输入框焦点，并在填入所选路径后保持列表关闭。
为什么需要
用户输入工作区路径后，即使点击输入框外部，目录建议浮窗仍可能持续显示。“浏览”按钮还会阻止焦点离开路径输入框，因此选择目录后，浮窗可能继续显示，或者被延迟返回的查询重新打开，并把已经选中的目录再次显示成一个多余选项。这会让界面看起来像是用户已经输入或选择目标路径后，仍然需要再选择一次目录。
Reviewer 测试计划
验证方式
打开“添加工作区”，输入绝对路径前缀并等待目录建议出现。点击路径输入框外部，确认输入框失去焦点且列表关闭；即使仍在等待的查询随后完成，列表也必须保持关闭。然后点击“浏览…”，选择目录，并确认所选绝对路径已填入、输入框保持失焦且建议列表没有打开。
证据（修改前后）
修改前：点击“浏览”后路径输入框仍保持焦点，延迟返回的自动补全结果可能重新打开列表并显示已选择的目录。普通失焦后的查询也可能重新打开列表。
修改后：失焦会关闭列表；输入框未激活时，延迟结果不能打开列表；“浏览”填入路径后，焦点位于输入框之外。
测试平台
环境（可选）
macOS；Web Shell 定向测试（23/23）、Web Shell 生产构建、TypeScript 类型检查、ESLint 和 Prettier。
风险与范围
关联 Issue
无