Skip to content

fix(cli): complete the live slash-submit deps and fixture (#10944) - #10961

Merged
wenshao merged 1 commit into
mainfrom
autofix/issue-10944
Sep 3, 2026
Merged

fix(cli): complete the live slash-submit deps and fixture (#10944)#10961
wenshao merged 1 commit into
mainfrom
autofix/issue-10944

Conversation

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

What this PR does

Repairs the post-merge Qwen Code CI lane on main, which is red on its ESLint step, and the two unit tests that fail alongside it. Both trace to the change that made Enter resolve a /-prefixed input buffer against the live command list rather than the render-derived completion state.

Two lines change. The keypress callback that reads the live command list now names it in its own dependency list, which is what react-hooks/exhaustive-deps asks for and what the callback needs to avoid resolving commands from a list captured on an earlier render. And the test fixture's memory command gains the action the real command has always had, so the two tests that assert "Enter on a perfect match submits" describe a state the real completion hook can actually produce.

Why it's needed

main is red now. The post-merge run for the current tip fails Lint & Static at its Run ESLint step with a recorded conclusion, and the lane runs ESLint with --max-warnings 0, so the single missing dependency: 'slashCommands' warning is a hard failure rather than noise.

Beyond the gate, the missing dependency is the exact staleness the original change was written to remove: a submit handler holding an older command list resolves the user's live input against data from a previous render.

The two failing tests matter for a different reason. They stopped passing because the submit path began consulting the real command list instead of a mocked isPerfectMatch, and the fixture declared memory as a parent with subcommands but no action — while the production command is runnable, and both the hook the old path used and the predicate the new path mirrors require an action before calling a match perfect. The tests were therefore pinning a combination the product can never produce. Leaving them red keeps the Test lane failing on every merge and hides real regressions in the surrounding submit logic.

Reviewer Test Plan

How to verify

  1. On main without this PR, run npm run lint:ci from the repository root. Expect exit 1 with packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands'. With this PR the same command exits 0.
  2. Still on main without this PR, run npx vitest run src/ui/components/InputPrompt.test.tsx inside packages/cli. Expect 213 passed / 2 failed, both failures being AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ] with Number of calls: 0, on should submit directly on Enter after arrow-navigate + backspace + retype to perfect match and should submit directly on Enter for a perfect match without prior arrow navigation. With this PR the file is 215/215.
  3. Confirm no assertion was touched: the diff to the test file is a single added line in the fixture, and the diff to the component is a single added entry in a dependency array.
  4. Confirm the fixture now matches production: the real memory command is runnable and opens the memory dialog, and the fixture's export entry in the same list already carried both an action and subCommands, so a runnable parent is the established shape here.
  5. Behavior to check by hand if you want product-level confidence: type /memory in the TUI and press Enter. It should submit and open the memory manager, not autocomplete to the first subcommand. Arrow-navigating the suggestion list first and then pressing Enter should still accept the highlighted suggestion rather than submit.

Evidence (Before & After)

Non-UI change (a React hook dependency list and a test fixture), so no TUI capture. The observable before/after is the two gates:

  • Before: npm run lint:ci → exit 1, ✖ 1 problem (0 errors, 1 warning) / ESLint found too many warnings (maximum: 0). InputPrompt.test.tsx → 213 passed, 2 failed.
  • After: npm run lint:ci → exit 0. InputPrompt.test.tsx + InputPrompt.suggestionMouse.test.tsx → 2 files, 226 passed, 0 failed.
  • Upstream confirmation that this is the live failure and not a local one: the post-merge run for the current main tip reports Lint & Static → step 22 Run ESLint = failure.

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Unit tests and lint only — no CLI run, no sandbox, no container. Verified in a Linux container on Node v22.23.2 against a clean npm ci install.

Risk & Scope

  • Main risk or tradeoff: naming the command list in the dependency array means the keypress handler is recreated when that prop's identity changes, rather than holding a stale one. That is the intended semantics and matches what the sibling render-highlighting callback in the same component already does with the same value. The fixture change adds one property to one entry; it does not relax or delete any assertion.
  • Not validated / out of scope: the issue this closes was filed for an earlier post-merge run whose Lint & Static job died inside Run ESLint with no step conclusion recorded — a self-hosted runner that stopped reporting mid-step, at 37 minutes against a 90-minute cap. That is infrastructure, has no repository-side repair, and is not addressed here; what this PR fixes is the reproducible failure of the same lane and step on the current tip. Also out of scope and only reported: node scripts/lint.js --sensitive-keywords has no matching branch in the lint driver's main(), so that step exits 0 without linting anything, and --prettier runs prettier --write ., which cannot fail on unformatted input. Both are false greens in CI machinery; changing either could redden main on violations nobody has been measuring.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #10944

中文说明

这个 PR 做了什么

修复 main 上合并后的 Qwen Code CI 通道 —— 它当前在 ESLint 步骤上是红的 —— 以及与之一起失败的两个单元测试。两者都源自那次「让 Enter 键针对以 / 开头的输入缓冲区,改为对照实时命令列表解析,而不再依赖由渲染派生的补全状态」的改动。

改动共两行。读取实时命令列表的那个按键回调,现在在自己的依赖列表里声明了该列表:这既是 react-hooks/exhaustive-deps 的要求,也是该回调避免用「上一次渲染时捕获的列表」去解析命令所必需的。另外,测试夹具中的 memory 命令补上了真实命令一直具有的 action,于是那两个断言「完全匹配时按 Enter 应当提交」的测试,描述的是真实补全 hook 确实可能产生的状态。

为什么需要它

main 现在是红的。当前 tip 的合并后运行在 Lint & StaticRun ESLint 步骤上失败,并记录了明确结论;而该通道以 --max-warnings 0 运行 ESLint,所以那一条 missing dependency: 'slashCommands' warning 是硬性失败,不是噪音。

除了关卡本身,这个缺失的依赖恰恰是原改动想要消除的那种陈旧状态:一个持有旧命令列表的提交处理函数,会用上一次渲染的数据去解析用户的实时输入。

那两个失败的测试之所以重要,原因不同。它们不再通过,是因为提交路径开始查询真实命令列表而不是被 mock 的 isPerfectMatch;而夹具把 memory 声明为一个只有 subcommands、没有 action 的父命令 —— 但生产环境中该命令是可执行的,并且旧路径使用的 hook 与新路径所镜像的判定函数,都要求存在 action 才认定「完全匹配」。因此这两个测试固定住的是产品永远不可能产生的组合。让它们继续红着,会使每次合并都让 Test 通道失败,并掩盖周边提交逻辑中的真实回归。

审阅者测试计划

如何验证

  1. 在不含本 PR 的 main 上,从仓库根目录运行 npm run lint:ci。预期退出码 1,并报出 packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands'。带上本 PR 后,同一命令退出码为 0。
  2. 仍在不含本 PR 的 main 上,进入 packages/cli 运行 npx vitest run src/ui/components/InputPrompt.test.tsx。预期 213 通过 / 2 失败,两个失败都是 AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ]Number of calls: 0,分别位于 should submit directly on Enter after arrow-navigate + backspace + retype to perfect matchshould submit directly on Enter for a perfect match without prior arrow navigation。带上本 PR 后该文件为 215/215。
  3. 确认没有触碰任何断言:测试文件的 diff 是夹具中新增的一行,组件的 diff 是依赖数组中新增的一项。
  4. 确认夹具现在与生产一致:真实的 memory 命令是可执行的、会打开 memory 管理器;而同一列表中夹具的 export 条目本来就同时带有 actionsubCommands,所以「可执行的父命令」正是这里既有的形态。
  5. 如果你想在产品层面确认行为:在 TUI 中输入 /memory 并按 Enter。它应当提交并打开 memory 管理器,而不是自动补全成第一个子命令。而先用方向键在建议列表中导航、再按 Enter,仍应接受高亮的建议,而不是提交。

证据(改动前与改动后)

非 UI 改动(一个 React hook 依赖列表和一个测试夹具),因此没有 TUI 录制。可观察的前后差异就是这两个关卡:

  • 改动前:npm run lint:ci → 退出码 1,✖ 1 problem (0 errors, 1 warning) / ESLint found too many warnings (maximum: 0)InputPrompt.test.tsx → 213 通过,2 失败。
  • 改动后:npm run lint:ci → 退出码 0。InputPrompt.test.tsx + InputPrompt.suggestionMouse.test.tsx → 2 个文件,226 通过,0 失败。
  • 上游佐证,说明这是真实存在的失败而非本地现象:当前 main tip 的合并后运行报告 Lint & Static → 第 22 步 Run ESLint = failure

测试环境

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

运行环境(可选)

仅单元测试与 lint —— 没有运行 CLI,没有 sandbox,没有容器。在 Linux 容器内、Node v22.23.2、基于干净的 npm ci 安装验证。

风险与范围

  • 主要风险或取舍:在依赖数组中声明命令列表,意味着该按键处理函数会在这个 prop 的身份变化时被重建,而不是持有一个陈旧的副本。这正是期望的语义,也与同一组件中相邻的渲染高亮回调对同一个值的既有做法一致。夹具改动只是给一个条目增加一个属性;它没有放宽或删除任何断言。
  • 未验证 / 超出范围:本 PR 关闭的 issue 是针对更早一次合并后运行提出的,那次的 Lint & Static job 死在 Run ESLint 内部,且没有记录步骤结论 —— 这是一个自托管 runner 在步骤中途停止上报,发生在 37 分钟处,而上限是 90 分钟。那属于基础设施问题,仓库侧无从修复,本 PR 也不处理它;本 PR 修复的是当前 tip 上同一通道、同一步骤的可复现失败。同样超出范围、仅作上报:node scripts/lint.js --sensitive-keywords 在 lint 驱动程序的 main() 中没有对应分支,所以该步骤什么都不检查就以退出码 0 结束;而 --prettier 运行的是 prettier --write .,对未格式化的输入不可能失败。两者都是 CI 机制中的「假绿」;改动任一处都可能让 main 因为从未被度量过的违规而变红。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

Fixes #10944

Main's post-merge lane is red on its ESLint step, and two unit tests in the
same area fail next to it. Both trace to #10929, which made Enter resolve a
`/`-prefixed buffer against the live command list instead of the
render-derived completion that can still describe the previous keystroke.

The keypress callback that reads that list never added it to its
dependencies, so `react-hooks/exhaustive-deps` reports a missing
`slashCommands` and the lane's `--max-warnings 0` turns the single warning
into a failed step. The omission is also the very staleness the commit set
out to remove: a handler captured against an older list resolves the live
command from stale data.

The same commit exposed a fixture that never matched production. The test
command list declared `memory` with subcommands but no action, while the
real command is runnable, and the hook both the old and the new path
consult requires an action before it calls a match perfect. Two tests
therefore mocked a perfect match for a buffer the live list called
unrunnable — a state the real hook cannot produce — and Enter stopped
submitting. Giving the fixture the action the real command has restores
both with their assertions untouched; a third test sharing the fixture had
been passing only because the perfect-match branch was dead for that
buffer, and now exercises the path it was written for.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

Autofix report — issue #10944 (Main CI failed: Qwen Code CI on 678ac2e1ec2d)

What the issue reported

Issue #10944 tracks main-branch run 33773880468 of Qwen Code CI at commit 678ac2e1ec2d, filed with the per-commit marker because "a main-branch CI run failed before any test result was reported".

What the run actually did

Read from the run's own job metadata over the public, unauthenticated REST API (read-only; this agent holds no GitHub credentials and performed no write). No job log was downloadable without authentication, so the step timeline is the evidence:

Job Conclusion Window
Classify PR success 15:57:42 → 15:57:46
Test (ubuntu-latest, Node 22.x) success 15:57:49 → 16:29:15
Lint & Static (ubuntu-latest, Node 22.x) failure 15:57:50 → 16:34:58

Inside Lint & Static (self-hosted runner ecs-qwen-hk3-3), steps 1–21 are all success — including Install dependencies (15:58:15 → 16:19:23) and Install linters. Step 22 Run ESLint started at 16:19:50 and recorded no conclusion and no completion time; steps 23–34 never started. A step that starts and never concludes, with the job then failing roughly 15 minutes later, is a runner that stopped reporting mid-step — not an assertion that failed.

That also explains the issue's wording. Lint & Static runs ESLint, actionlint, shellcheck, yamllint, Prettier, the generated-artifact freshness gates and node --test helper suites; none of them print the vitest FAIL / pytest FAILED lines the filer dedupes on, so this lane can only ever produce a per-commit issue.

The commit under test changed two timeout-minutes values plus their comments in npm-cache.yml and sdk-java.yml. Neither file is read by ci.yml, and every repository-side check that lane runs was re-executed locally against this tree and passed (listed under Verification). There is no content in that commit that can fail this lane, and a runner that dies mid-step is not repairable from the repository.

The reproducible defect in the same lane and the same step

The next post-merge run, 33776698676 at commit 661f41eef0 (current main), fails the same job on the same step, this time with a recorded conclusion:

  • Lint & Static → step 22 Run ESLint = failure (16:39:11 → 16:41:01)

Reproduced locally with the exact command that step runs (node scripts/lint.js --eslintnpm run lint:ci):

packages/cli/src/ui/components/InputPrompt.tsx
  1892:5  warning  React Hook useCallback has a missing dependency: 'slashCommands'.
                   Either include it or remove the dependency array  react-hooks/exhaustive-deps

✖ 1 problem (0 errors, 1 warning)
ESLint found too many warnings (maximum: 0).        → exit 1

Two unit tests in the same area fail on the merged tree as well, which is the Test lane's business:

FAIL src/ui/components/InputPrompt.test.tsx > InputPrompt > should submit directly on Enter
     after arrow-navigate + backspace + retype to perfect match
FAIL src/ui/components/InputPrompt.test.tsx > InputPrompt > should submit directly on Enter
     for a perfect match without prior arrow navigation
AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ]
Number of calls: 0

Root cause

Both come from #10929, which made Enter resolve a /-prefixed buffer against the live command list instead of the render-derived completion, because that completion can still describe the previous keystroke when Enter arrives.

  1. The lint gate. The keypress callback that now reads the command list never added it to its own dependency array. react-hooks/exhaustive-deps reports the missing slashCommands, and this lane runs ESLint with --max-warnings 0, so one warning is a failed step. The omission is not only a lint problem: a handler captured against an older list resolves the live command from stale data, which is precisely the staleness fix(cli): submit exact slash commands from the live input #10929 set out to remove.

  2. The two tests. The new path decides a perfect match by requiring the resolved command to have an action. That mirrors production exactly — usePerfectMatch in useSlashCompletion.ts requires leafCommand.action too, and isPerfectSlashMatch in the OpenTUI port is documented as a port of it. The test fixture, however, declared memory with subCommands and no action, while the real memoryCommand is runnable (action: async () => ({ type: 'dialog', dialog: 'memory' })). So both tests mocked isPerfectMatch: true for a buffer whose live command list said "not runnable" — a state the real hook cannot produce. While the submit path trusted the mock the discrepancy was invisible; once it consulted the fixture, Enter stopped submitting.

The fix (two lines)

  • packages/cli/src/ui/components/InputPrompt.tsx: add slashCommands to the handleKeypress dependency array, next to the buffer and completion the same branch reads.
  • packages/cli/src/ui/components/InputPrompt.test.tsx: give the fixture's memory command the action the real command has.

No assertion was changed, weakened, or deleted. A third test sharing that fixture — should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list — had been passing only because the perfect-match branch was dead for /memory; with the fixture corrected it exercises the path it was written for and still passes.

Mutation probes (witness for each added line)

Both changes were probed in the failing direction on the real tree, not inferred:

  • Probe A — dependency entry. With slashCommands absent from the array (the tree as merged), npm run lint:ci exits 1 with 1892:5 warning … missing dependency: 'slashCommands'. With it present, npm run lint:ci exits 0. Observed both ways.
  • Probe B — fixture action. With action: vi.fn() removed from the fixture's memory entry, the two named tests fail with Number of calls: 0 on onSubmit. With it present, the file runs 215/215. Observed both ways on an otherwise byte-identical tree (git diff empty at the revert point).

Reported, deliberately not changed

  • The 678ac2e1ec runner death itself. Infrastructure: a self-hosted runner stopped reporting inside Run ESLint. There is no repository-side defect to repair and no evidence supporting a workflow change, so none was made. This is the same shared-pool contention that ci: size two job caps for a contended install, not a quiet one #10931 (the commit under test) was sizing caps against; ci.yml's two lanes already carry 120/90-minute ECS caps and this job died at 37 minutes, well inside them.
  • 44 failures across 12 files when running the whole packages/cli suite in this sandbox are environmental, not regressions. They are byte-for-byte identical with the fix reverted (same 12 files, same per-file counts: llm.test.tsx 21, config.test.ts 5, Footer.test.tsx 4, and 9 files with 1–3 each). The causes are visible in the output: Error: EACCES: permission denied, mkdir '/home/github-runner/.qwen', and sandbox-image assertions defeated by the SANDBOX / QWEN_CODE_CUSTOM_SANDBOX_IMAGE variables this agent's own container injects. CI does not set those.
  • Two false greens in this lane, outside this issue's scope and outside the areas this bot may modify. node scripts/lint.js --sensitive-keywords (the Run sensitive keyword linter step) has no matching branch in scripts/lint.js's main(), so it exits 0 without linting anything; and --prettier runs prettier --write ., which rewrites the runner's checkout and cannot fail on unformatted input. Both make the lane greener than it looks. Flagging rather than fixing: changing either could turn main red on violations nobody has been measuring, and both live in CI machinery.

Verification

Commands actually run in this checkout, on the committed tree unless stated:

  • npm run buildpassed (exit 0)
  • npm run typecheckpassed (exit 0)
  • npm run lintpassed (exit 0)
  • npm run lint:ci (the exact command the failing CI step runs) — passed (exit 0). Before the fix: exit 1 with the warning quoted above.
  • npx vitest run src/ui/components/InputPrompt.test.tsx src/ui/components/InputPrompt.suggestionMouse.test.tsx in packages/cli226 passed, 0 failed (2 files). Before the fix the first file ran 213 passed / 2 failed.
  • Whole packages/cli suite (npx vitest run) — 28095 passed, 44 failed; the identical 44 fail with the fix reverted, and are sandbox artifacts as documented above. Reported for honesty, not as a pass.
  • node --test --test-concurrency=1 $HELPER_TESTS (all 23 helper suites) — 511 passed, 0 failed (run while diagnosing; untouched by this change).
  • Also run while diagnosing, all passing: bash .github/scripts/check-workflow-size.sh (with the push lane's base SHA), npm run audit:runtime:critical, npm run check:lockfile, npm run check:desktop-isolation, npm run check:tui-dep-direction, npm run check-i18n, npm run generate:settings-schema and npm run generate:notices --workspace=qwen-code-vscode-ide-companion (both left the tree clean, so both freshness gates hold), and npx prettier --check on the two workflow files the issue's commit touched.
  • npm run generate:settings-schema — not required (no settings source changed), but run anyway as above; packages/vscode-ide-companion/schemas/settings.schema.json stayed clean, so no regenerated artifact needed committing.
  • Integration tests after npm run bundlenot run. The touched behavior is unit-level React keypress handling and a test fixture, both exercised directly by the suites above; nothing here is reachable only through the bundled CLI or the integration harness.
  • actionlint / yamllint / shellchecknot run: none of the three binaries is installed in this container and there is no pip3 to install yamllint. They are unaffected by this change, which touches only a .tsx dependency array and a test fixture.
中文说明

自动修复报告 —— issue #10944Main CI failed: Qwen Code CI on 678ac2e1ec2d

该 issue 报告了什么

Issue #10944 跟踪的是 commit 678ac2e1ec2d 上的 main 分支运行 33773880468(工作流 Qwen Code CI)。它以「按 commit 归档」的标记创建,原因是「一次 main 分支 CI 运行在报告任何测试结果之前就失败了」。

这次运行实际发生了什么

以下信息来自该运行自身的 job 元数据,通过公开的、无需认证的 REST API 读取(只读;本 agent 不持有任何 GitHub 凭据,也没有执行任何写操作)。没有认证就无法下载 job 日志,因此步骤时间线就是本次的证据:

Job 结论 时间窗口
Classify PR success 15:57:42 → 15:57:46
Test (ubuntu-latest, Node 22.x) success 15:57:49 → 16:29:15
Lint & Static (ubuntu-latest, Node 22.x) failure 15:57:50 → 16:34:58

Lint & Static 内部(自托管 runner ecs-qwen-hk3-3),第 1–21 步全部为 success,包括 Install dependencies(15:58:15 → 16:19:23)和 Install linters。第 22 步 Run ESLint 于 16:19:50 开始,但既没有记录结论、也没有记录完成时间;第 23–34 步从未开始。一个已经开始却永远没有结论的步骤、随后 job 在约 15 分钟后失败,这是 runner 在步骤中途停止上报的特征 —— 而不是某个断言失败。

这也解释了 issue 的措辞。Lint & Static 运行的是 ESLint、actionlint、shellcheck、yamllint、Prettier、生成产物的新鲜度校验以及 node --test 辅助测试套件;它们都不会打印归档脚本用于去重的 vitest FAIL / pytest FAILED 行,所以这条通道只可能产出「按 commit 归档」的 issue。

被测试的那个 commit 只修改了 npm-cache.ymlsdk-java.yml 中的两个 timeout-minutes 取值及其注释。ci.yml 并不读取这两个文件,而该通道运行的每一项仓库侧检查都已在本地针对当前代码树重新执行并通过(见「验证」一节)。那个 commit 的内容中没有任何东西能让这条通道失败,而一个在步骤中途死掉的 runner 也无法从仓库侧修复。

同一通道、同一步骤上可复现的缺陷

下一次合并后运行 33776698676(commit 661f41eef0,即当前 main)在同一个 job 的同一个步骤上失败,而且这次记录了明确的结论:

  • Lint & Static → 第 22 步 Run ESLint = failure(16:39:11 → 16:41:01)

用该步骤实际执行的命令(node scripts/lint.js --eslintnpm run lint:ci)在本地复现:

packages/cli/src/ui/components/InputPrompt.tsx
  1892:5  warning  React Hook useCallback has a missing dependency: 'slashCommands'.
                   Either include it or remove the dependency array  react-hooks/exhaustive-deps

✖ 1 problem (0 errors, 1 warning)
ESLint found too many warnings (maximum: 0).        → exit 1

同一区域还有两个单元测试在合并后的代码树上失败,那属于 Test 通道的范围:

FAIL src/ui/components/InputPrompt.test.tsx > InputPrompt > should submit directly on Enter
     after arrow-navigate + backspace + retype to perfect match
FAIL src/ui/components/InputPrompt.test.tsx > InputPrompt > should submit directly on Enter
     for a perfect match without prior arrow navigation
AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ]
Number of calls: 0

根因

两者都源自 #10929。该改动让 Enter 键针对以 / 开头的输入缓冲区,改为对照「实时命令列表」解析,而不再依赖由渲染派生的补全状态 —— 因为 Enter 到达时,后者可能仍在描述上一次按键。

  1. Lint 关卡。 那个现在会读取命令列表的按键回调,从未把它加进自己的依赖数组。react-hooks/exhaustive-deps 因此报告缺失 slashCommands,而这条通道以 --max-warnings 0 运行 ESLint,所以一条 warning 就等于步骤失败。这不仅仅是 lint 问题:一个捕获了旧列表的处理函数会用过期数据去解析实时命令,而这恰恰是 fix(cli): submit exact slash commands from the live input #10929 想要消除的那种陈旧状态。

  2. 那两个测试。 新路径判定「完全匹配」的方式是要求解析到的命令带有 action。这与生产代码完全一致 —— useSlashCompletion.ts 中的 usePerfectMatch 同样要求 leafCommand.action,而 OpenTUI 移植版中的 isPerfectSlashMatch 在文档里就写明是它的移植。但测试夹具把 memory 声明为只有 subCommands没有 action,而真实的 memoryCommand 是可执行的(action: async () => ({ type: 'dialog', dialog: 'memory' }))。于是这两个测试为一个「实时命令列表判定为不可执行」的缓冲区 mock 出了 isPerfectMatch: true —— 这是真实 hook 不可能产生的状态。当提交路径还信任 mock 时,这个不一致是隐形的;一旦它改为查询夹具,Enter 就不再提交了。

修复(两行)

  • packages/cli/src/ui/components/InputPrompt.tsx:把 slashCommands 加入 handleKeypress 的依赖数组,位置紧邻同一分支读取的 buffercompletion
  • packages/cli/src/ui/components/InputPrompt.test.tsx:给夹具中的 memory 命令补上真实命令所具有的 action

没有任何断言被修改、削弱或删除。共享该夹具的第三个测试 —— should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list —— 此前之所以通过,只是因为对 /memory 而言完全匹配分支是死代码;夹具修正后,它真正走到了自己原本要验证的路径,并且依然通过。

变异探针(为每一行新增代码提供见证)

两处改动都在真实代码树上朝「失败方向」做过探针验证,而不是靠推断:

  • 探针 A —— 依赖项。slashCommands 不在数组中时(即已合并的树),npm run lint:ci 以退出码 1 失败,并报出 1892:5 warning … missing dependency: 'slashCommands'。加上它之后,npm run lint:ci 退出码为 0。两个方向都已实测。
  • 探针 B —— 夹具的 action 当夹具中 memory 条目的 action: vi.fn() 被移除时,上述两个测试以 onSubmitNumber of calls: 0 失败。加上它之后,该文件 215/215 全部通过。两个方向都在其余内容逐字节相同的树上实测(回退点上 git diff 为空)。

已报告但刻意未改动的部分

  • 678ac2e1ec 那次 runner 死亡本身。 属于基础设施问题:一个自托管 runner 在 Run ESLint 内部停止上报。仓库侧没有可修复的缺陷,也没有任何证据支持修改工作流,因此没有改动。这与 ci: size two job caps for a contended install, not a quiet one #10931(即被测试的那个 commit)正在为之调整超时时长的共享资源池争用是同一类现象;ci.yml 的两条通道在 ECS 上已分别配有 120/90 分钟的超时上限,而这个 job 在 37 分钟就死了,远在上限之内。
  • 在本沙箱中运行整个 packages/cli 套件时出现的 12 个文件、44 个失败属于环境问题,不是回归。 在回退本次修复后它们逐字节完全一致(同样的 12 个文件、同样的每文件计数:llm.test.tsx 21 个、config.test.ts 5 个、Footer.test.tsx 4 个,另有 9 个文件各 1–3 个)。原因在输出中可直接看到:Error: EACCES: permission denied, mkdir '/home/github-runner/.qwen',以及 sandbox 镜像相关断言被本 agent 自身容器注入的 SANDBOX / QWEN_CODE_CUSTOM_SANDBOX_IMAGE 变量破坏。CI 不会设置这些变量。
  • 本通道中的两处「假绿」,超出该 issue 范围,也超出本 bot 允许修改的区域。 node scripts/lint.js --sensitive-keywords(即 Run sensitive keyword linter 步骤)在 scripts/lint.jsmain() 中没有对应分支,因此它什么都不检查就以退出码 0 结束;而 --prettier 运行的是 prettier --write .,它会改写 runner 上的检出内容,对未格式化的输入不可能失败。两者都让这条通道看起来比实际更绿。选择上报而非修复:改动任一处都可能让 main 因为从未被度量过的违规而变红,而且两者都属于 CI 机制。

验证

以下命令确实在本检出中执行过;除特别说明外,都是在已提交的代码树上运行:

  • npm run build —— 通过(退出码 0)
  • npm run typecheck —— 通过(退出码 0)
  • npm run lint —— 通过(退出码 0)
  • npm run lint:ci(失败 CI 步骤实际执行的命令)—— 通过(退出码 0)。修复前:退出码 1,并报出上文引用的 warning。
  • packages/cli 中执行 npx vitest run src/ui/components/InputPrompt.test.tsx src/ui/components/InputPrompt.suggestionMouse.test.tsx —— 226 通过,0 失败(2 个文件)。修复前第一个文件为 213 通过 / 2 失败。
  • 整个 packages/cli 套件(npx vitest run)—— 28095 通过,44 失败;这 44 个在回退修复后完全相同地失败,属于上文所述的沙箱环境问题。为诚实起见列出,不计为通过。
  • node --test --test-concurrency=1 $HELPER_TESTS(全部 23 个辅助套件)—— 511 通过,0 失败(在诊断过程中运行;本次改动不涉及它们)。
  • 诊断过程中还运行了以下命令,全部通过:bash .github/scripts/check-workflow-size.sh(使用 push 通道的 base SHA)、npm run audit:runtime:criticalnpm run check:lockfilenpm run check:desktop-isolationnpm run check:tui-dep-directionnpm run check-i18nnpm run generate:settings-schemanpm run generate:notices --workspace=qwen-code-vscode-ide-companion(两者都没有改动代码树,说明两个新鲜度关卡均成立),以及对该 issue 的 commit 所触碰的两个工作流文件执行 npx prettier --check
  • npm run generate:settings-schema —— 并非必需(没有改动任何 settings 源),但仍如上执行过;packages/vscode-ide-companion/schemas/settings.schema.json 保持干净,因此没有需要一并提交的再生成产物。
  • npm run bundle 之后的集成测试 —— 未运行。本次触碰的行为是单元级的 React 按键处理和一个测试夹具,上面的套件已直接覆盖;这里没有任何内容是只能通过打包后的 CLI 或集成测试框架才能触达的。
  • actionlint / yamllint / shellcheck —— 未运行:这三个二进制在本容器中都没有安装,也没有可用于安装 yamllint 的 pip3。本次改动只涉及一个 .tsx 依赖数组和一个测试夹具,与它们无关。

🧠 Handled by Qwen Code · model/模型 qwen3.8-max-2026-09-02

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Sep 3, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 6d023e9 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 6d023e9 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is filled in, plus the Chinese section.

Problem: observed, and I confirmed it independently rather than taking the description's word for it. main at 661f41ee (the merge of #10929) is red on two jobs in the same lane:

  • Lint & Static (ubuntu-latest, Node 22.x)failure. The job log carries exactly one problem: packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' … react-hooks/exhaustive-deps, then ✖ 1 problem (0 errors, 1 warning) / ESLint found too many warnings (maximum: 0). The root lint:ci script does run with --max-warnings 0, so one warning is a hard failure, not noise.
  • Test (ubuntu-latest, Node 22.x)failure. src/ui/components/InputPrompt.test.tsx (215 tests | 2 failed), the two being should submit directly on Enter after arrow-navigate + backspace + retype to perfect match and should submit directly on Enter for a perfect match without prior arrow navigation, both AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ] at :2852 and :2880. Suite total: 2 failed | 28131 passed | 90 skipped.

So both halves of the stated problem are real and reproducible from main's own recorded runs. The review/self-reported label is accurate (#10944 and this PR share an author), but nothing here rests on a self-report.

Direction: aligned — main is red, this is the repair, and it repairs it at the cause rather than muting the lint rule. One honest scope note, as a question and not a blocker: Fixes #10944 will auto-close an issue that tracks the run on 678ac2e1ec2d, which is the parent of the commit that introduced the warning. I checked that run — its Lint & Static job went 15:57:50 → 16:34:58Z and its Run ESLint step recorded no conclusion at all, i.e. the runner hang the Risk & Scope section describes, while Test (ubuntu) on that same commit passed. The lint warning this PR fixes did not exist yet at 678ac2e1. The PR states that plainly and does not claim to repair the hang, which is the right call; I only flag it because #10944 was labeled for the autofix lifecycle, so closing it here is presumably intended. If a maintainer would rather keep the infra hang tracked on its own, say so before merge.

Size: not applicable — no core paths (packages/cli/src/ui/components/** is outside the Stage 0 list, single package). 1 production line + 1 test line, 0 generated/schema.

Approach: scope feels exactly right, and I'd have written the same two lines. I tried to think of a smaller or safer edit and there isn't one: an eslint-disable or a ref-mirror would silence the rule while keeping the stale capture, and re-pointing the two tests at an already-runnable fixture command (/quit, /clear) would change what they cover instead of fixing a fixture that never matched production. No drive-by edits, no formatting churn, no assertion relaxed or removed — the test-side diff is one added property.

Two observations for the record, neither blocking:

  • The fixture's memory entry still declares show/add/refresh subcommands that production memoryCommand no longer has (it's a bare runnable command that opens the dialog). That staleness predates this PR and is harmless here — the suggestion list in these tests comes from the mocked completion hook, not the fixture — and the shape the fix lands on (runnable parent with subcommands) does exist in production via exportCommand, whose fixture entry already carried both.
  • Making memory runnable also revives a third test that shares the fixture, should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list. Worth a look in Stage 2 that it still asserts what it was written for.

Risk: no elevated risk signals — neither changed file matches the high-risk path set, so standard review depth.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需小标题都填了,中文部分也在。

问题: 是已观测到的问题,而且我没有只采信 PR 描述,自己独立确认过。main661f41ee(即 #10929 的合并提交)上,同一条流水线有两个 job 是红的:

  • Lint & Static (ubuntu-latest, Node 22.x)failure。日志里只有一个问题:packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' … react-hooks/exhaustive-deps,随后是 ✖ 1 problem (0 errors, 1 warning) / ESLint found too many warnings (maximum: 0)。根目录的 lint:ci 确实带 --max-warnings 0,所以一条 warning 就是硬失败,不是噪音。
  • Test (ubuntu-latest, Node 22.x)failuresrc/ui/components/InputPrompt.test.tsx (215 tests | 2 failed),失败的正是 should submit directly on Enter after arrow-navigate + backspace + retype to perfect matchshould submit directly on Enter for a perfect match without prior arrow navigation,报错都是 AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ],位置 :2852:2880。整个套件:2 failed | 28131 passed | 90 skipped

所以 PR 所说的两半问题都真实存在,并且能从 main 自己记录的运行结果里复现。review/self-reported 标签是准确的(#10944 与本 PR 同一作者),但这里的结论并不依赖自述。

方向: 对齐 —— main 现在是红的,这个 PR 就是修复,而且是从根因修,不是把 lint 规则消音。有一点需要如实说明,作为提问而不是阻拦:Fixes #10944 会自动关闭一个跟踪 678ac2e1ec2d 那次运行的 issue,而该提交是引入这条 warning 的提交的父提交。我查了那次运行 —— 它的 Lint & Static job 从 15:57:50 跑到 16:34:58Z,Run ESLint 这一步完全没有记录结论,也就是 Risk & Scope 里说的 runner 挂起;而同一提交上的 Test (ubuntu) 是通过的。本 PR 修的 lint warning 在 678ac2e1 时还不存在。PR 对此说得很清楚,也没有声称修好了那次挂起,这个处理是对的;我之所以提出来,是因为 #10944 本身打了 autofix 流程标签,所以在这里关闭它应该就是预期行为。如果维护者更希望把基础设施挂起单独跟踪,请在合并前说明。

规模: 不适用 —— 未触及核心路径(packages/cli/src/ui/components/** 不在 Stage 0 清单内,且只涉及单个 package)。生产代码 1 行 + 测试 1 行,生成/schema 0 行。

方案: 范围拿捏得刚好,我自己也会写这两行。我尝试想过有没有更小或更安全的改法,结论是没有:eslint-disable 或用 ref 镜像只能让规则闭嘴,同时保留过期捕获;把两个测试改成指向 fixture 里本来就可执行的命令(/quit/clear)则会改变它们的覆盖面,而不是修正一个从未与生产一致的 fixture。没有夹带改动,没有格式化噪音,没有放松或删除任何断言 —— 测试侧 diff 就是加了一个属性。

两点记录在案的观察,都不构成阻拦:

  • fixture 里的 memory 仍然声明了生产 memoryCommand 已经没有的 show/add/refresh 子命令(生产中它是一个直接打开对话框的可执行命令)。这个陈旧早于本 PR,在此处也无害 —— 这些测试里的候选列表来自被 mock 的补全 hook,而不是 fixture —— 并且修复后落到的形态(既有子命令又可执行的父命令)在生产中确实存在,即 exportCommand,其 fixture 条目本来就同时带这两者。
  • memory 变为可执行,同时也会「激活」共用该 fixture 的第三个测试 should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list。Stage 2 值得确认它仍在断言它当初要断言的东西。

风险: 无升级风险信号 —— 两个改动文件都不匹配高风险路径集合,按常规深度审查即可。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6d023e9f042cfbad0556e79f3fb8903f123d6a8c · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Code review

The production line is the right fix, and it's load-bearing rather than lint appeasement. slashCommands is genuinely read inside the keypress callback — parseSlashCommand(buffer.text, slashCommands) at line 1437 — while the dependency array starting at 1890 doesn't list it, so react-hooks/exhaustive-deps is reporting a real omission, not a false positive.

The staleness is also concretely reachable. slashCommands arrives as uiState.slashCommands, which is the commands state in slashCommandProcessor: initialised to [] and replaced once by setCommands(commandService.getCommandsForMode('interactive')) inside the async load() in an effect (and again on a reloadTrigger). So the identity changes from empty to loaded, and a callback captured before that replacement resolves the user's live /… input against an empty list — commandToExecute comes back undefined, isLiveSlashCommand is false, and Enter falls through instead of submitting. Narrow window, but it is exactly the failure mode #10929 was written to remove, so the dependency belongs there.

The cost is nil, which is what I'd want to check before accepting a new dep on a hot keypress path: because the value is state and not a per-render literal, the callback is recreated only when the command list is (re)loaded, not on every render. It's also passed down as a plain onKeypress prop that BaseTextInput invokes per keystroke from its own [buffer, onSubmit, onKeypress] callback — no subscription is torn down and no keystroke can be dropped by a fresh identity. And the sibling renderLineWithHighlighting in the same file already depends on [slashCommands], so the two are now consistent.

The test line makes the fixture describe a state production can actually produce. With action present, parseSlashCommand('/memory', mockSlashCommands) satisfies all three clauses of the live predicate — commandToExecute.action !== undefined, args === '', canonicalPath.length === commandPartCount — so isLiveSlashCommand is true and Enter submits again. Production memoryCommand does have an action (it opens the memory dialog), so the fixture was the wrong side of the mismatch. No assertion was touched; the test-side diff is that one added property.

Blast radius of the fixture change — checked, not assumed:

  • The third test sharing the fixture, should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list, does switch branches, and it still asserts what it was written for. Its arrow-down goes through the showCompletionSuggestions block, which sets navigatedRef.current = true and navigatedTextRef.current = buffer.text; on Enter the perfect-match branch now opens, the navigated guard matches, and acceptActiveCompletionSuggestion() calls handleAutocomplete(0) and returns before handleSubmitAndClear. Both assertions hold — via the intended branch instead of the fallthrough ACCEPT_SUGGESTION path. That's a coverage gain, not a silent weakening.
  • Nothing else in the file can observe the change. The other /memory tests only assert useCommandCompletion was called with the fixture by identity; the /memory case at line 1909 has a trailing space, which the live path excludes via !/\s$/.test(buffer.text); parseInputForHighlighting never reads .action or .subCommands; and useExportCompletion only ever looks up export.

No correctness, security, or convention problems, and nothing here that reads as over-abstraction or scope creep. Two lines doing one job.

Test evidence

This is an unattended CI run, so nothing was built or executed here — the review is static, and the evidence below is the PR's own CI read through the API plus main's recorded failures.

Final CI results for 6d023e9 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
OpenTUI no-flicker gate ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
TUI parity snapshots (ink vs opentui) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Nothing red on this head so far, and TUI parity snapshots (ink vs opentui) passing is a useful early signal that an ink-side keypress change didn't diverge from the OpenTUI implementation. I did not poll the two pending jobs — the suite runs far longer than any sensible wait here, so the table above is a snapshot and the finalize job rewrites it once CI settles.

The baseline this PR has to clear is recorded, and it is the failure I quoted in the gate comment: on main at 661f41ee, Lint & Static fails on the single missing dependency: 'slashCommands' warning under --max-warnings 0, and Test (ubuntu) fails InputPrompt.test.tsx (215 tests | 2 failed) with AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ] at :2852 and :2880. Those are the two jobs to watch in the table above.

One pre-existing red to not misread later: main at the same tip also carries E2E Test (Linux) - sandbox:docker - shard 3/3failure. Nothing in this diff touches sandbox or E2E paths, so if that shard goes red on this PR too it is inherited, not caused here.

Not verified, and why:

  • Lint and unit results on this head — pending, per the table. Not verified: the jobs had not reported at review time.
  • The staleness window itself — not verified: no test pins "Enter pressed before the command list finishes loading". The restored tests pin the steady-state path (loaded list, runnable exact match, submit), and the dep addition is what makes the early-window path correct, but that window is only argued from the code above, never executed by a test.
  • Product-level behaviour — not verified: this run never drives the TUI. The PR's own step 5 (type /memory, press Enter, expect the memory manager rather than an autocomplete to the first subcommand) is the author's suggested manual check, not evidence I reproduced.

Sandboxed verification would settle both gaps, and the author has write access so neither lane needs sponsoring: @qwen-code /tmux — that Enter on a bare /memory submits and opens the memory dialog instead of accepting the first suggestion, and that arrow-navigating first still accepts the highlight, which is the behaviour the revived third test only asserts through mocks; @qwen-code /verify — that the added dependency is load-bearing rather than cosmetic, since this PR's suite would pass identically with the slashCommands dep removed and only the lint job would notice.

中文说明

代码审查

生产代码这一行是正确的修法,而且是真正起作用的,不只是让 lint 闭嘴。 slashCommands 确实在按键回调里被读取 —— 第 1437 行的 parseSlashCommand(buffer.text, slashCommands) —— 而从 1890 行开始的依赖数组没有列出它,所以 react-hooks/exhaustive-deps 报的是真实遗漏,不是误报。

这个过期捕获也是具体可达的。slashCommands 来自 uiState.slashCommands,也就是 slashCommandProcessor 里的 commands state:初始为 [],随后在 effect 内的异步 load() 里被 setCommands(commandService.getCommandsForMode('interactive')) 替换一次(reloadTrigger 时还会再替换)。所以它的标识会从「空」变成「已加载」,而在替换之前捕获的回调会拿空列表去解析用户实时输入的 /… —— commandToExecute 返回 undefinedisLiveSlashCommand 为 false,Enter 就直接漏下去而不提交。窗口很窄,但这正是 #10929 想要消除的失效模式,所以这个依赖本来就该在那里。

代价为零,这也是我在接受热按键路径上新增依赖前想确认的:因为该值是 state 而不是每次渲染新建的字面量,回调只在命令列表(重新)加载时才重建,不是每次渲染都重建。它也是作为普通 onKeypress prop 往下传,由 BaseTextInput 在自己的 [buffer, onSubmit, onKeypress] 回调里逐键调用 —— 标识变新不会拆掉任何订阅,也不可能丢按键。同文件里的 renderLineWithHighlighting 本来就依赖 [slashCommands],现在两者一致了。

测试这一行让 fixture 描述了一个生产确实可能出现的状态。 有了 actionparseSlashCommand('/memory', mockSlashCommands) 就满足实时判定的三个条件 —— commandToExecute.action !== undefinedargs === ''canonicalPath.length === commandPartCount —— 于是 isLiveSlashCommand 为 true,Enter 重新会提交。生产里的 memoryCommand 确实带 action(打开 memory 对话框),所以这次不匹配里错的一方是 fixture。没有动任何断言;测试侧 diff 就是新增的这一个属性。

fixture 改动的影响范围 —— 是查过的,不是假设的:

  • 共用该 fixture 的第三个测试 should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list 确实换了分支,而且它断言的仍是它当初要断言的东西。它的下方向键走 showCompletionSuggestions 分支,会设置 navigatedRef.current = truenavigatedTextRef.current = buffer.text;按 Enter 时完美匹配分支现在会打开,navigated 守卫命中,acceptActiveCompletionSuggestion() 调用 handleAutocomplete(0) 并在 handleSubmitAndClear 之前返回。两个断言都成立 —— 走的是预期分支,而不是原来漏下去的 ACCEPT_SUGGESTION 路径。这是覆盖面的收益,不是悄悄放松。
  • 文件里其他测试观察不到这个改动。其余 /memory 测试只断言 useCommandCompletion 被以标识相等的方式传入了该 fixture;1909 行的 /memory 带尾随空格,实时路径通过 !/\s$/.test(buffer.text) 把它排除;parseInputForHighlighting 从不读 .action.subCommandsuseExportCompletion 只查 export

没有正确性、安全性或约定问题,也看不出过度抽象或范围蔓延。两行,做一件事。

测试证据

这是无人值守的 CI 运行,所以本地没有构建或执行任何东西 —— 审查是静态的,下面的证据来自通过 API 读取的本 PR 自身 CI,以及 main 上已记录的失败。

(上表是审查时刻 6d023e9f 上的 check run 快照,CI 结束后会自动原地重写。)

目前这个 head 上没有红项,而 TUI parity snapshots (ink vs opentui) 通过是个有用的早期信号:ink 侧的按键改动没有和 OpenTUI 实现产生分歧。我没有轮询那两个 pending job —— 套件运行时间远超此处合理的等待预算,所以上表只是快照。

本 PR 需要跨过的基线是有记录的,也就是我在 gate 评论里引用的失败:main661f41ee 上,Lint & Static--max-warnings 0 下唯一的 missing dependency: 'slashCommands' warning 而失败,Test (ubuntu)InputPrompt.test.tsx (215 tests | 2 failed) 失败,报错为 AssertionError: expected "spy" to be called with arguments: [ '/memory', …(1) ],位置 :2852:2880。上表里要盯的就是这两个 job。

有一个既有红项,稍后别误读:同一 main tip 上还带着 E2E Test (Linux) - sandbox:docker - shard 3/3failure。本 diff 完全不碰 sandbox 或 E2E 路径,所以如果该 shard 在本 PR 上也变红,那是继承来的,不是这里造成的。

未验证的部分及原因:

  • 本 head 上的 lint 与单测结果 —— pending,见上表。未验证:审查时这两个 job 还没出结果。
  • 过期窗口本身 —— 未验证:没有测试钉住「命令列表加载完成前按下 Enter」这一路径。被恢复的两个测试钉住的是稳态路径(列表已加载、可执行的精确匹配、提交),依赖新增正是让早期窗口这条路正确的原因,但那个窗口只在上面的代码推理里成立,从未被测试执行过。
  • 产品级行为 —— 未验证:本次运行完全不驱动 TUI。PR 自己的第 5 步(输入 /memory 后按 Enter,期望打开 memory 管理器而不是补全到第一个子命令)是作者建议的手工检查,不是我复现出来的证据。

沙箱化验证可以把这两个缺口都补上,而且作者有 write 权限,两条通道都不需要赞助:@qwen-code /tmux —— 验证在裸 /memory 上按 Enter 会提交并打开 memory 对话框,而不是接受第一个候选项,以及先用方向键导航后仍会接受高亮项;这正是那个被重新激活的第三个测试只能通过 mock 断言的行为。@qwen-code /verify —— 验证新增依赖是真正起作用而非装饰性的,因为把 slashCommands 这个依赖去掉后,本 PR 的测试套件会完全一样地通过,只有 lint job 会发现差别。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6d023e9f042cfbad0556e79f3fb8903f123d6a8c · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean two-line repair of a red main that I confirmed independently; the only open items are the two CI jobs still running and two non-blocking notes.

I wrote my own proposal for this before opening the diff, from the title and the motivation alone: add the missing dependency to the keypress callback (not a suppression, not a ref mirror), and decide which side of the test mismatch is wrong — the fixture or the assertions — by checking whether production /memory is actually runnable. That is exactly what this PR does, in the same two places, and I could not find a smaller or safer edit. So the approach matches my baseline rather than merely surviving it.

The one thing that would normally make me hesitate is the shape of the test fix: making a failing test pass by changing test data is how real regressions get buried. I pressed on it rather than accepting the framing. Production memoryCommand does carry an action, exportCommand establishes that a runnable parent with subcommands is a legitimate shape (and the fixture's export entry already had both), no assertion was relaxed or removed, and the third test that shares the fixture switches to the branch it was originally written for instead of quietly losing meaning. That resolves the hesitation with evidence. If any of those four had come back the other way, this would have been a request-changes.

The production line is also more than lint hygiene, which is what lifts this above a chore: the command list starts as [] and is replaced once after an async load, so a handler captured before the replacement resolves live /… input against an empty list and Enter silently fails to submit. Narrow, but it is the exact staleness #10929 set out to remove, and the value is state rather than a per-render literal, so declaring it costs nothing on a hot keypress path.

What is still open, and why I'm not approving in this run: Lint & Static and Test (ubuntu) on this head had not reported at review time — they are the two jobs whose failure on main this PR exists to clear, so approving before they land would attest to a result that does not exist yet. Approval is deferred until CI lands green on 6d023e9f042cfbad0556e79f3fb8903f123d6a8c; if either job goes red, or the head moves, nothing gets approved and the status comment says so.

Two notes for whoever merges, neither blocking:

  • Fixes #10944 closes an issue that tracks the run on 678ac2e1ec2d, the parent of the commit that introduced the warning. That run's Run ESLint step recorded no conclusion at all — the runner hang the description is upfront about — and its Test (ubuntu) passed. So this PR repairs the same lane and step on the current tip, not the failure that issue was filed for. Closing it is presumably the intended autofix lifecycle; if a maintainer would rather track the hang separately, do that before merge.
  • The fixture's memory still declares show/add/refresh subcommands that production dropped. Pre-existing, harmless here because the suggestion list is mocked, and out of scope for a CI repair — just don't read the fixture as a mirror of today's /memory.

On volume: this author has ten open PRs, so I checked that I was judging this one on its own evidence and not on a plausible-sounding description. The red main is confirmed from main's own recorded check runs and job logs, quoted in the gate comment, not taken on trust.

Six months from now this reads fine: an honest dependency array and a fixture that describes a state the product can produce.

中文说明

Confidence: 4/5 —— 一个干净的两行修复,修的是我自己独立确认过的红 main;唯一未决的是还在跑的两个 CI job,以及两点不构成阻拦的备注。

在看 diff 之前,我只根据标题和动机写了自己的方案:给按键回调补上缺失的依赖(不是消音,也不是用 ref 镜像),然后判断测试不匹配里错的是哪一边 —— fixture 还是断言 —— 办法是确认生产里的 /memory 到底是否可执行。这正是本 PR 做的事,位置也一样,我没能找到更小或更安全的改法。所以这个方案是与我的基线相符,而不只是勉强过关。

正常情况下会让我犹豫的一点,是这个测试修复的形态:靠改测试数据让失败的测试通过,正是真实回归被埋掉的常见方式。我没有接受它的说法,而是压上去查了。生产里的 memoryCommand 确实带 actionexportCommand 证明了「既有子命令又可执行的父命令」是合法形态(而 fixture 里的 export 条目本来就两者都有);没有任何断言被放松或删除;共用该 fixture 的第三个测试转到了它当初为之编写的分支,而不是悄悄失去意义。这些证据把犹豫解掉了。这四点里只要有一点是反的,这里就该是 request-changes。

生产代码那一行也不只是 lint 卫生问题,这正是它高于一次杂活的地方:命令列表初始是 [],异步加载完成后被替换一次,所以在替换之前捕获的 handler 会拿空列表去解析实时的 /… 输入,Enter 就静默地不提交。窗口很窄,但这正是 #10929 想消除的过期状态;而且该值是 state 而非每次渲染新建的字面量,所以在热按键路径上声明它没有代价。

仍未决的部分,也是我这轮不批准的原因:审查时本 head 上的 Lint & StaticTest (ubuntu) 还没出结果 —— 而它们正是本 PR 存在的意义所要清掉的、main 上失败的那两个 job,所以在它们落地前批准,等于为一个还不存在的结果背书。批准将推迟到 CI 在 6d023e9f042cfbad0556e79f3fb8903f123d6a8c 上全绿之后;如果任一 job 变红,或者 head 发生移动,就不会有任何批准,并且状态评论会说明情况。

给合并者的两点备注,都不构成阻拦:

  • Fixes #10944 会关闭一个跟踪 678ac2e1ec2d 那次运行的 issue,而该提交是引入这条 warning 的提交的父提交。那次运行的 Run ESLint 步骤完全没有记录结论 —— 也就是描述里坦白说明的 runner 挂起 —— 而它的 Test (ubuntu) 是通过的。所以本 PR 修的是当前 tip 上的同一条流水线和同一步骤,不是那个 issue 被创建时对应的失败。关闭它大概就是 autofix 生命周期的预期行为;如果维护者更想单独跟踪那次挂起,请在合并前处理。
  • fixture 里的 memory 仍声明了生产已经去掉的 show/add/refresh 子命令。这是既有问题,在此处无害(候选列表是 mock 的),也超出一次 CI 修复的范围 —— 只是别把这个 fixture 当成今天 /memory 的镜像来读。

关于数量:这位作者有十个开放 PR,所以我特意确认了自己是在按它自身的证据判断,而不是被一段听起来合理的描述带走。main 变红这件事是从 main 自己记录的 check run 和 job 日志确认的,已在 gate 评论里引用,不是采信来的。

六个月后再看这段代码是舒服的:一个诚实的依赖数组,和一个描述了产品确实可能出现的状态的 fixture。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6d023e9f042cfbad0556e79f3fb8903f123d6a8c · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: reverse audit — stopped before round 1 by the review time budget.

Test Plan (not a blocker): src/ui/components/InputPrompt.test.tsxno such file or directory; 213 passed — this review observed 28134 passed; 226 passed — this review observed 28134 passed.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:反向审计——评审时间预算不足,未能开始第 1 轮。

Test Plan(非阻断):src/ui/components/InputPrompt.test.tsxno such file or directory; 213 passed — this review observed 28134 passed; 226 passed — this review observed 28134 passed

— qwen3.8-max via Qwen Code /review (v0.23.0)

@qwen-code-dev-bot

qwen-code-dev-bot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

AutoFix round 1 finishedview run. See this round's report below.

中文说明

AutoFix 第 1 轮已完成 —— 查看运行。本轮报告见下方。

@qqqys

qqqys commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

E2E verification report (tmux, A/B against broken main)

Verified head 6d023e9f04 against base 661f41eef0 (current main, red since the #10929 merge) on a Linux host, driving the real toolchain in a tmux session with no build (codeload tarballs + the checkout's node_modules/package dists symlinked in, so both sides share identical dependencies and the A/B is controlled to the 2-line diff).

Leg 1 — npx eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 on base → exit 1, reproducing the red Lint & Static lane exactly:

1892:5  warning  React Hook useCallback has a missing dependency: 'slashCommands'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
✖ 1 problem (0 errors, 1 warning)
ESLint found too many warnings (maximum: 0).

Leg 2 — same eslint command on head → exit 0, clean.

Leg 3 — npx vitest run src/ui/components/InputPrompt.test.tsx on base → exit 1, Tests 2 failed | 213 passed (215), and the two failures are exactly the ones red on main's Test lane:

  • InputPrompt > should submit directly on Enter after arrow-navigate + backspace + retype to perfect match
  • InputPrompt > should submit directly on Enter for a perfect match without prior arrow navigation

Leg 4 — same vitest file on head → exit 0, Tests 215 passed (215).

Independent review of the diff: the added slashCommands entry is genuinely read inside the callback (parseSlashCommand(buffer.text, slashCommands) at line 1437, dep array at 1892), so this is a real stale-closure fix, not lint appeasement; and the fixture change aligns the mock with the real memoryCommand, which has always had an action (packages/cli/src/ui/commands/memoryCommand.ts:18). No Critical issues found.

Conclusion: the PR repairs both red lanes on main as claimed — verified end to end. Looks mergeable.

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix round — no code change

PR #10961 (issue #10944), head 6d023e9f04. This round made no commit and left the working tree byte-identical to the pushed head (git status --short empty, HEAD still 6d023e9f04).

What the round received

  • 1 review body — [rv:5105766848], COMMENTED, automated reviewer round 1, ledger findings: [], posted: 0.
  • 0 inline comments, 0 issue-level comments, 0 failed checks, 0 still-red checks.
  • CI on this exact SHA: 71 checks → 23 SUCCESS, 48 SKIPPED, 0 failures. An APPROVED review (5105171437, "LGTM, looks ready to ship — CI landed green") is already on the same commit.
  • Diff growth this window: source 0 / test 0 net lines (budgets 400/400). No Deferred non-Critical feedback section, no Growth audit required section, --conflict false.

So the only content to triage is the three clauses of that one review body.

Disposition of [rv:5105766848]

1. "Not reviewed: reverse audit — stopped before round 1 by the review time budget" — informational, nothing to implement

This is the reviewer disclosing its own unfinished coverage, not a claim about the code. There is no defect asserted and therefore nothing to fix. Since the gap was disclosed, I re-read the whole two-line diff myself rather than leaving it unexamined:

  • InputPrompt.tsxslashCommands is genuinely read inside handleKeypress (parseSlashCommand(buffer.text, slashCommands), line 1437), so the new dependency-array entry is load-bearing and not lint appeasement; it sits next to the buffer and completion the same branch reads.
  • InputPrompt.test.tsx — the fixture's memory entry gains action: vi.fn(). Production memoryCommand is runnable (action: async () => ({ type: 'dialog', dialog: 'memory' })), and the perfect-match branch keys on exactly that (commandToExecute?.action !== undefined). The added line also brings memory in line with the adjacent export fixture, which already declares both an action and subCommands. No assertion was changed, weakened, or deleted.

The fixture's subCommands on memory do not exist on the real command, but they are pre-existing (not added by this PR) and tests depend on /memory show completion, so touching them would be out-of-scope diff growth for no finding.

2. "src/ui/components/InputPrompt.test.tsxno such file or directory" — Decline (refuted)

The path is package-relative, which is the form AGENTS.md prescribes for focused runs ("Tests must be run from within the specific package directory, not the project root", cd packages/cli && npx vitest run src/path/to/file.test.tsx), and the report line it was extracted from says "in packages/cli". Measured:

$ test -f src/ui/components/InputPrompt.test.tsx        # repo root
MISSING at root (no such file or directory)
$ ls -l src                                             # repo root
ls: cannot access 'src': No such file or directory
$ ls -l packages/cli/src/ui/components/InputPrompt.test.tsx
-rw-r--r-- 1 node node 209169 ... InputPrompt.test.tsx

There is no src/ directory at the repository root at all, so a verifier resolving that string from the root cannot find it — the file exists and the command runs when the documented package cwd is used (see clause 3). The claim is about path context, not about a defect in the change.

3. "213 passed / 226 passed — this review observed 28134 passed" — Decline (refuted by measurement)

I re-ran the exact focused command from the documented directory on the committed tree:

$ cd packages/cli && npx vitest run \
    src/ui/components/InputPrompt.test.tsx \
    src/ui/components/InputPrompt.suggestionMouse.test.tsx

 ✓ src/ui/components/InputPrompt.suggestionMouse.test.tsx (11 tests) 193ms
 ✓ src/ui/components/InputPrompt.test.tsx (215 tests) 57369ms

 Test Files  2 passed (2)
      Tests  226 passed (226)
FOCUSED_EXIT=0

226 passed, 0 failed — reproduces exactly. The two figures the review compared against are not both current-state claims:

  • 226 passed is the focused two-file total, and it reproduces (215 + 11).
  • 213 passed was explicitly labeled the before-fix count in a before/after sentence ("Before the fix the first file ran 213 passed / 2 failed"). 213 + the 2 then-failing tests = the 215 measured now, so the two numbers corroborate each other rather than conflicting.
  • 28134 passed is a whole-suite total. It can never equal a focused two-file count; the comparison is a category mismatch. It appears to be the fallback the reviewer ran after clause 2's path lookup failed, so clause 2's path context is what invalidated the comparison.

Both declined clauses are refuted by direct measurement, and the review came from the automated reviewer rather than a maintainer, so per the source-blind verification rule this is a recorded Decline rather than an escalation. No thread exists to reply on (a review body carries no inline thread), which is why the disposition is recorded here.

One note for whoever next edits the PR description

Not a code issue and not something this round can change: the PR-side verification text cites the test file by its package-relative path. Quoting it repo-root-relative — packages/cli/src/ui/components/InputPrompt.test.tsx, run from packages/cli — would let an automated verifier that resolves paths from the repository root find the file instead of falling back to a whole-suite run. address-review has no PR-body output and this agent holds no GitHub credentials, so the body is left as-is; the correction is recorded here instead. Nothing in the repository needs to change for it.

Verification

Commands actually run this round, in this checkout, on the committed tree at 6d023e9f04:

  • npm run buildpassed (BUILD_EXIT=0). Required first: the workspace dist/ outputs were absent, and the vitest globalSetup guard stops package-local runs until they exist.
  • cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx src/ui/components/InputPrompt.suggestionMouse.test.tsx226 passed, 0 failed (2 files: 215 + 11), exit 0. This is the probe that refutes clause 3.
  • test -f src/ui/components/InputPrompt.test.tsx from the repository root — missing, confirming clause 2 is a path-context artifact.
  • git status --shortempty; git rev-parse HEAD6d023e9f042cfbad0556e79f3fb8903f123d6a8c, unchanged. Build output is gitignored, so the build left no tracked modification.
  • npm run typecheck, npm run lint, integration tests — not re-run. No source changed this round, so the tree is byte-identical to the one CI already validated green on this SHA (23 SUCCESS, 0 failures); re-running them would measure the same bytes. The previous round's report records both as passing on this commit.
中文说明

自动修复轮次 —— 未改动代码

PR #10961(issue #10944),head 6d023e9f04 本轮没有提交任何 commit,工作树与已推送的 head 逐字节一致(git status --short 为空,HEAD 仍为 6d023e9f04)。

本轮收到的内容

  • 1 条 review body —— [rv:5105766848],状态 COMMENTED,自动审查器第 1 轮,ledger 中 findings: []posted: 0
  • 0 条行内评论、0 条 issue 级评论、0 个失败检查、0 个持续失败的检查。
  • 该 SHA 上的 CI:71 个检查 → 23 个 SUCCESS、48 个 SKIPPED0 个失败。同一个 commit 上已有一条 APPROVED 审查(5105171437,「LGTM, looks ready to ship — CI landed green」)。
  • 本计数窗口的 diff 增长:源码净 0 行 / 测试净 0 行(预算 400/400)。没有 Deferred non-Critical feedback 段落,没有 Growth audit required 段落,--conflict false

因此唯一需要分诊的内容,就是那一条 review body 中的三个子句。

[rv:5105766848] 的处置

1.「未审查:反向审计 —— 评审时间预算不足,未能开始第 1 轮」—— 信息性说明,无需实现

这是审查器在披露自身未完成的覆盖范围,而不是对代码提出主张。其中没有断言任何缺陷,因此没有可修之处。既然该缺口已被披露,我自己把这两行的完整 diff 重新读了一遍,而不是放着不看:

  • InputPrompt.tsx —— slashCommands 确实在 handleKeypress 内部被读取(parseSlashCommand(buffer.text, slashCommands),第 1437 行),所以新增的依赖数组条目是承载实际作用的,而不是为了讨好 lint;它的位置紧邻同一分支读取的 buffercompletion
  • InputPrompt.test.tsx —— 夹具中的 memory 条目新增了 action: vi.fn()。生产环境的 memoryCommand 是可执行的(action: async () => ({ type: 'dialog', dialog: 'memory' })),而完全匹配分支判定的正是这一点(commandToExecute?.action !== undefined)。这一行还让 memory 与相邻的 export 夹具保持一致 —— 后者本来就同时声明了 actionsubCommands。没有任何断言被修改、削弱或删除。

夹具中 memorysubCommands 在真实命令上并不存在,但它们是既有内容(不是本 PR 新增),且有测试依赖 /memory show 的补全行为,因此去动它们只会在没有任何 finding 的情况下扩大 diff。

2.「src/ui/components/InputPrompt.test.tsx —— no such file or directory」—— Decline(已证伪)

该路径是相对于包的,而这正是 AGENTS.md 为聚焦测试运行所规定的形式(「Tests must be run from within the specific package directory, not the project root」,即 cd packages/cli && npx vitest run src/path/to/file.test.tsx),并且它被摘取时所依据的那行报告文字写明了「in packages/cli」。实测:

$ test -f src/ui/components/InputPrompt.test.tsx        # 仓库根目录
MISSING at root (no such file or directory)
$ ls -l src                                             # 仓库根目录
ls: cannot access 'src': No such file or directory
$ ls -l packages/cli/src/ui/components/InputPrompt.test.tsx
-rw-r--r-- 1 node node 209169 ... InputPrompt.test.tsx

仓库根目录下根本不存在 src/ 目录,所以从根目录解析这个字符串的校验器不可能找到它 —— 而只要使用文档所规定的包内工作目录,该文件确实存在、命令也确实能运行(见子句 3)。这条主张讲的是路径上下文,不是改动本身的缺陷。

3.「213 passed / 226 passed —— 本次审查观察到 28134 passed」—— Decline(经实测证伪)

我在已提交的代码树上,从文档规定的目录重新执行了那条完全相同的聚焦命令:

$ cd packages/cli && npx vitest run \
    src/ui/components/InputPrompt.test.tsx \
    src/ui/components/InputPrompt.suggestionMouse.test.tsx

 ✓ src/ui/components/InputPrompt.suggestionMouse.test.tsx (11 tests) 193ms
 ✓ src/ui/components/InputPrompt.test.tsx (215 tests) 57369ms

 Test Files  2 passed (2)
      Tests  226 passed (226)
FOCUSED_EXIT=0

226 通过,0 失败 —— 完全复现。 被拿来比较的那两个数字,并不都是对当前状态的主张:

  • 226 passed 是聚焦两个文件的总数,它复现了(215 + 11)。
  • 213 passed 在一个「修复前 / 修复后」的句子中被明确标注为修复前的计数(「Before the fix the first file ran 213 passed / 2 failed」)。213 加上当时失败的 2 个测试,正好等于现在实测的 215,所以这两个数字互相印证,而非互相矛盾。
  • 28134 passed整套测试的总数。它永远不可能等于聚焦两个文件的计数;这个比较属于类别错配。它看起来是审查器在子句 2 的路径查找失败之后所跑的兜底命令,也就是说,正是子句 2 的路径上下文让这个比较失去了意义。

两条被 decline 的子句都由直接实测证伪,而且该审查来自自动审查器而非维护者,因此按照「来源无关」的验证规则,这里记录为 Decline,而不是上报请示。没有可供回复的讨论串(review body 不带行内讨论串),这也是处置结论记录在此处的原因。

给下一位编辑 PR 描述者的一条说明

这不是代码问题,也不是本轮能改的东西:PR 侧的验证文字用相对于包的路径引用了该测试文件。若改为相对于仓库根目录书写 —— packages/cli/src/ui/components/InputPrompt.test.tsx,并在 packages/cli 下运行 —— 那么从仓库根目录解析路径的自动校验器就能找到该文件,而不必退回去跑整套测试。address-review 模式没有修改 PR 正文的输出通道,且本 agent 不持有任何 GitHub 凭据,因此正文保持原样,修正建议记录在此。仓库里没有任何内容需要为此改动。

验证

本轮实际执行的命令,均在本检出中、于 6d023e9f04 的已提交代码树上运行:

  • npm run build —— 通过BUILD_EXIT=0)。这是必须先做的一步:工作区的 dist/ 产物原本缺失,而 vitest 的 globalSetup 守卫会在其存在之前阻止包内测试运行。
  • cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx src/ui/components/InputPrompt.suggestionMouse.test.tsx —— 226 通过,0 失败(2 个文件:215 + 11),退出码 0。这是证伪子句 3 的探针。
  • 在仓库根目录执行 test -f src/ui/components/InputPrompt.test.tsx —— 不存在,确认子句 2 属于路径上下文造成的假象。
  • git status --short —— 为空git rev-parse HEAD —— 6d023e9f042cfbad0556e79f3fb8903f123d6a8c,未变化。构建产物受 gitignore 忽略,因此构建没有留下任何被跟踪的修改。
  • npm run typechecknpm run lint、集成测试 —— 未重跑。本轮没有改动任何源码,代码树与 CI 已在该 SHA 上验证为全绿的那一份逐字节相同(23 个 SUCCESS,0 个失败);重跑只会度量同样的字节。上一轮的报告记录了这两项在该 commit 上均通过。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-2026-09-02

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving per the standing gate for this main-repair PR:

  • ci-bot APPROVED this head (6d023e9f04) at 18:00Z; its later 19:06Z pass was a partial re-review with an empty findings ledger (time-budget disclosure only, no findings).
  • CI at head: 22 success / 0 failure / 48 path-skips, nothing in flight.
  • Our tmux A/B e2e report above (against broken main 661f41eef0) reproduced both red lanes exactly and showed both green at this head — conclusion: mergeable.
  • No critical issues found in our review.

批准理由(按既定门禁):

  • ci-bot 已在当前 head(6d023e9f04)于 18:00Z 批准;其 19:06Z 的后续评审为零发现的补审(仅披露时间预算缺口)。
  • 当前 head CI 全绿:22 成功 / 0 失败 / 48 路径跳过。
  • 上方 tmux A/B e2e 报告已在损坏的 main(661f41eef0)上精确复现两条红色通道,并验证本 head 两条均转绿——结论:可合入。
  • 我们的评审未发现 Critical 问题。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

No action this round

PR: #10961 · Issue: #10944 · Head: 6d023e9f04 · Base: main

No code change was made and no commit was created. The feedback in this window contains nothing actionable.

What the feedback said

Source Content Disposition
Issue-level comment [ic:5531088503] (@qqqys) E2E verification report (tmux, A/B against broken main): both previously-red lanes reproduce on base and are clean on head. Concludes "Looks mergeable." No action — a confirmation, not a request. It asks for no change and poses no question.
Reviews None in this window.
Inline comments None (rc.json is empty).
Failed checks None.
Still-red checks None.

There was no Deferred non-Critical feedback section (so this is not a critical-only round) and no Growth audit required section (so no growth audit was owed). Reported diff growth this window: source 0 / test 0 lines.

Independent verification of the report's claims

I did not take the verification report on trust. Both of its load-bearing code claims were checked against the exact reviewed commit, and both hold:

  1. The added dependency is genuinely read inside the callback — this is a real stale-closure fix, not lint appeasement. InputPrompt.tsx:1438-1439 calls parseSlashCommand(buffer.text, slashCommands) inside the useCallback whose dependency array begins at line 1892; the PR adds slashCommands to that array at line 1896. Omitting it meant the callback could capture a stale slashCommands binding, which is exactly the defect the two failing tests pinned.
  2. The fixture change aligns the mock with the real command. The real memoryCommand has always declared an action (packages/cli/src/ui/commands/memoryCommand.ts:19); the test mock omitted it, so commandToExecute?.action !== undefined in the live-submit path could not behave as it does in production. Adding action: vi.fn() removes that divergence.

Commands run (all green)

  • npm run buildpassed (exit 0)
  • npm run typecheckpassed (exit 0)
  • npm run lint (repo-wide, eslint . --ext .ts,.tsx && eslint integration-tests) — passed (exit 0)
  • npx eslint packages/cli/src/ui/components/InputPrompt.tsx packages/cli/src/ui/components/InputPrompt.test.tsx --max-warnings 0passed (exit 0), confirming the react-hooks/exhaustive-deps warning that reddened the Lint & Static lane on base is gone
  • cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx215 passed (215) (exit 0), matching the count the report observed on head

CI corroborates: checks.json for this head records 0 failures — 23 SUCCESS, 48 SKIPPED out of 71 checks. Both lanes the report names as red on base are green on head.

No mutation probes were run because this round added no guard, branch, or behavior — there is nothing new to witness.

Two notes for the reviewer

  • main has moved on; the PR is unaffected. Since the merge base (661f41eef0), main advanced by two commits (81de29d36e, cca376f6aa) touching acp-integration, git-branches, web-shell/BranchPickerPopover, and sdk-typescript — none of which touch InputPrompt.tsx or InputPrompt.test.tsx. The 2-line diff still applies cleanly, so no merge was performed (consistent with --conflict false).
  • The earlier automated review's Test Plan remark was a wrong-directory artifact, not a missing test. That review (submitted at the evaluation boundary, ledger "findings":[], 0 findings posted) reported src/ui/components/InputPrompt.test.tsxno such file or directory. The file exists at packages/cli/src/ui/components/InputPrompt.test.tsx and passes 215 tests when run from packages/cli, as the repo's own test instructions require; running the bare relative path from the repo root is what produces that error. The review itself marked the remark "not a blocker", and no action followed from it. The stale pass-counts it compared against (213 / 226) came from earlier PR-body prose; the current figure is 215.

Conclusion

The PR is a 2-line change (1 source, 1 test) that repairs both lanes red on main since the #10929 merge. It is verified green locally, has 0 outstanding review findings, an APPROVED review, and an independent maintainer E2E verification. Nothing further to address — the round closes with no change and no commit.

中文说明

本轮无需改动

PR: #10961 · Issue: #10944 · Head: 6d023e9f04 · Base: main

本轮未做任何代码改动,也未创建任何提交。本窗口内的反馈没有任何需要处理的内容。

反馈内容

来源 内容 处理结论
Issue 级评论 [ic:5531088503]@qqqys E2E 验证报告(tmux,与损坏的 main 做 A/B 对比):两条此前变红的检查在 base 上可复现、在 head 上干净通过。结论为 "Looks mergeable."(可以合并) 无需处理 —— 这是一份确认,而非要求。它没有请求任何改动,也没有提出任何问题。
Reviews 本窗口内没有。 ——
行内评论 没有(rc.json 为空)。 ——
失败的检查 没有。 ——
仍然变红的检查 没有。 ——

反馈中没有 Deferred non-Critical feedback(因此本轮不是 critical-only 模式),也没有 Growth audit required 章节(因此不欠一份增长审计)。本窗口报告的 diff 增长为:源码 0 行 / 测试 0 行。

对报告结论的独立核实

我没有直接采信这份验证报告。它两条关键性的代码论断都对照被审查的确切提交做了核实,两条都成立:

  1. 新增的依赖确实在回调内部被读取 —— 这是一个真实的陈旧闭包(stale-closure)修复,而非为了讨好 lint。 InputPrompt.tsx:1438-1439 在那个 useCallback 内部调用了 parseSlashCommand(buffer.text, slashCommands),而该回调的依赖数组从第 1892 行开始;本 PR 在第 1896 行把 slashCommands 加进了这个数组。此前遗漏它意味着回调可能捕获一个陈旧的 slashCommands 绑定,而这正是那两个失败测试所钉住的缺陷。
  2. fixture 的改动让 mock 与真实命令对齐。 真实的 memoryCommand 一直都声明了 actionpackages/cli/src/ui/commands/memoryCommand.ts:19);测试 mock 却遗漏了它,导致实时提交路径中的 commandToExecute?.action !== undefined 无法表现得与生产环境一致。加上 action: vi.fn() 消除了这一偏差。

已执行的命令(全部通过)

  • npm run build —— 通过(exit 0)
  • npm run typecheck —— 通过(exit 0)
  • npm run lint(全仓库,eslint . --ext .ts,.tsx && eslint integration-tests)—— 通过(exit 0)
  • npx eslint packages/cli/src/ui/components/InputPrompt.tsx packages/cli/src/ui/components/InputPrompt.test.tsx --max-warnings 0 —— 通过(exit 0),确认在 base 上让 Lint & Static 通道变红的 react-hooks/exhaustive-deps 警告已消失
  • cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx —— 215 passed (215)(exit 0),与报告在 head 上观察到的数量一致

CI 亦相互印证:本 head 的 checks.json 记录 0 个失败 —— 71 项检查中 23 项 SUCCESS、48 项 SKIPPED。报告指出的在 base 上变红的两条通道,在 head 上均为绿色。

本轮未运行变异探针(mutation probe),因为本轮没有新增任何守卫、分支或行为 —— 没有新东西需要见证。

给审查者的两点说明

  • main 已向前推进;本 PR 不受影响。 自合并基点(661f41eef0)以来,main 前进了两个提交(81de29d36ecca376f6aa),涉及 acp-integrationgit-branchesweb-shell/BranchPickerPopoversdk-typescript —— 均未触及 InputPrompt.tsxInputPrompt.test.tsx。这 2 行 diff 仍可干净应用,因此没有执行合并(与 --conflict false 一致)。
  • 此前自动审查的 Test Plan 备注是目录错误造成的假象,而非测试缺失。 那次审查(提交于评估边界时刻,账本为 "findings":[],发布 0 条发现)报告 src/ui/components/InputPrompt.test.tsx —— no such file or directory(无此文件或目录)。该文件实际存在于 packages/cli/src/ui/components/InputPrompt.test.tsx,并按仓库自身的测试要求在 packages/cli 下运行时通过 215 个测试;从仓库根目录直接运行那个裸相对路径才会产生该错误。审查本身已把该备注标为 "not a blocker"(非阻断项),也未据此采取任何行动。它所对比的过期通过数(213 / 226)来自更早的 PR 正文描述;当前数字为 215

结论

本 PR 是一个 2 行改动(1 行源码、1 行测试),修复了自 #10929 合并以来 main 上变红的两条通道。它已在本地验证通过,0 条未处理的审查发现,拥有一个 APPROVED 审查,以及一份独立的维护者 E2E 验证。没有更多需要处理的事项 —— 本轮以无改动、无提交收尾。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-2026-09-02

@wenshao
wenshao added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 69c4f1e Sep 3, 2026
90 checks passed
qwen-code-dev-bot pushed a commit that referenced this pull request Sep 3, 2026
Main's 69c4f1e (#10961) landed the same two changes this branch carries:
the missing `slashCommands` dependency on the keypress callback, and the
`action` the mock `memory` command needs to satisfy the live-slash submit
gate. Both are fallout from #10929, which is what this branch had been
reduced to.

Git reported a conflict only in the test fixture. In InputPrompt.tsx the two
sides inserted `slashCommands` at adjacent but different positions, so the
auto-merge silently kept both and produced a duplicate entry in the
dependency array. Neither eslint's exhaustive-deps rule, which reports
missing dependencies rather than duplicated ones, nor tsc flags that, so it
would have landed unreviewed. Resolved to main's content, leaving one entry.

The fixture resolves to the union: main's `action: vi.fn()` plus the comment
recording why that never-invoked, never-asserted fn is load-bearing. The
comment stays because it implements an accepted review suggestion and is
still accurate against the merged gate, which remains
`commandToExecute?.action !== undefined && args.length === 0 &&
canonicalPath.length === commandPartCount`.

The functional fix now belongs entirely to main; this branch's remaining
delta is that one comment line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Main CI failed: Qwen Code CI on 678ac2e1ec2d

4 participants