Skip to content

fix(cli): restore green main CI after the live slash-command submit change (#10967) - #10968

Closed
qwen-code-dev-bot wants to merge 2 commits into
mainfrom
autofix/issue-10967
Closed

qwen-code-dev-bot wants to merge 2 commits into
mainfrom
autofix/issue-10967

Conversation

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

What this PR does

Restores main to green by repairing the two CI jobs that have failed on every post-merge run since the live slash-command submit change landed. The submit path that reads the available slash commands now declares that input in its memoization dependencies, satisfying the lint gate CI enforces. The two regression tests that stopped passing are made to describe a real perfect match again: their mocked command was a container with no executable action, a state the completion hook can never report as a perfect match, so the mock now carries an action the way the real command does. Two lines change in total; no assertion is weakened and no runtime behavior is altered.

Why it's needed

Qwen Code CI on main has been red since the commit that made Enter decide a slash-command perfect match from the live input buffer instead of the render-derived completion state. That commit read the slash-command list inside the keypress callback but did not add it to the dependency array, which the react-hooks/exhaustive-deps rule flags; CI's ESLint step runs with --max-warnings 0, so a single warning fails the whole Lint & Static job and skips every static check after it. The same commit broke two pre-existing tests that drive /memory through a mocked completion claiming a perfect match while their mocked memory command has subcommands and no action. Production never produces that combination — the real perfect-match logic requires an executable action, and every real command that can be typed exactly defines one, including the real memory command and MCP prompt parents — so the tests were asserting against a mock artifact rather than catching a user-facing regression. Because both jobs are required contexts, main reports failure on every merge until this is fixed.

Reviewer Test Plan

How to verify

  1. Check out main at cca376f6aa and run npm run lint:ci. Expect exit code 1 with exactly one warning: packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' react-hooks/exhaustive-deps. Note that npm run lint (without --max-warnings 0) only warns, which is why this looked clean locally.
  2. Still on that commit, run cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx. Expect 2 failed | 213 passed (215), failing 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, both with expected "spy" to be called with arguments: [ '/memory', …(1) ] / Number of calls: 0.
  3. Check out this branch and re-run both commands. Expect lint:ci exit code 0 with no output, and 215 passed (215).
  4. Confirm the diff is exactly two added lines: slashCommands, in the keypress callback's dependency array, and action: vi.fn(), on the mocked memory command.
  5. Mutation-check both lines. Removing the mock action must fail those same two tests again (npx vitest run src/ui/components/InputPrompt.test.tsx -t "perfect match"2 failed | 1 passed). Removing the dependency must fail npx eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 with exit code 1.
  6. Confirm the neighbouring test should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list still passes: with an actionable /memory, arrow-navigating and pressing Enter must still autocomplete the highlighted subcommand rather than submit, which is the behavior that test pins.
  7. Behaviorally, nothing user-visible should change: typing an exact slash command and pressing Enter submits it, a partial one autocompletes, and a navigated suggestion list autocompletes the highlighted entry.

Evidence (Before & After)

N/A — no user-visible/TUI change. The evidence is the two CI gates themselves:

Check Before (at cca376f6aa) After (this branch)
npm run lint:ci exit 1 — ESLint found too many warnings (maximum: 0), 1 react-hooks/exhaustive-deps warning exit 0, no findings
vitest run src/ui/components/InputPrompt.test.tsx 2 failed | 213 passed (215) 215 passed (215)
CI job Lint & StaticRun ESLint failure (run 33797332027) expected success
CI job TestRun tests and generate reports failure (run 33797332027) expected success

Tested on

OS Status
🍏 macOS ⚠️ not tested
🪟 Windows ⚠️ not tested
🐧 Linux ✅ tested

Environment (optional)

Linux self-hosted runner checkout, Node v22.23.2, npm 10.9.8, unit tests only (no CLI run, no sandbox, no live model calls). Test runs were additionally repeated under a CI-like environment (SANDBOX unset, temporary HOME, all API keys emptied) to rule out environment dependence: InputPrompt.test.tsx is 215/215 in both.

Risk & Scope

  • Main risk or tradeoff: effectively none at runtime. The added dependency cannot change behavior — the completion object already in that dependency array is rebuilt on every render, so the callback was already recreated each render; the entry only makes the memoization honest and the lint gate pass. The test change touches a mock, not an assertion.
  • Not validated / out of scope: a full local packages/cli workspace run in this sandbox reports 33 failures in 9 unrelated files (settings/config, Windows-style home paths, browser-open commands, serve live-host discovery, Footer snapshots). These are sandbox artifacts, not main failures: the Footer snapshot diff is an extra docker | pill because the shell runs inside Docker with SANDBOX set, and re-running that file with SANDBOX unset and a clean HOME gives 34/34 instead of 4 failures. CI history corroborates — the Test job was green at 678ac2e1ec, the commit immediately before the one that touched only these two files. Also out of scope: the earlier Lint & Static job-level failure on run 33773880468 (678ac2e1ec), which reported no failed step and is a separate transient infrastructure event, not the failure tracked by Main CI failed: Qwen Code CI on cca376f6aa4b #10967.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #10967

中文说明

这个 PR 做了什么

main 恢复绿色:修复自“实时斜杠命令提交”改动落地以来、每次合并后运行都失败的两个 CI job。读取可用斜杠命令的提交路径现在把这一输入声明进了它的 memo 依赖数组,从而满足 CI 强制执行的 lint 门禁。两个不再通过的回归测试被重新改为描述一个真实的“完全匹配”:它们 mock 的命令是一个没有可执行 action 的容器命令,而 completion hook 永远不可能把这种状态报告为完全匹配,因此该 mock 现在像真实命令一样带上了 action。总共只改动两行;没有削弱任何断言,也没有改变任何运行时行为。

为什么需要

自那个让 Enter 依据实时输入缓冲区(而不是渲染派生的 completion 状态)来判定斜杠命令完全匹配的 commit 起,main 上的 Qwen Code CI 一直是红的。该 commit 在按键回调内部读取了斜杠命令列表,却没有把它加入依赖数组,react-hooks/exhaustive-deps 规则会标记这一点;而 CI 的 ESLint 步骤以 --max-warnings 0 运行,所以一条警告就会让整个 Lint & Static job 失败,并跳过其后的所有静态检查。同一个 commit 还破坏了两既有测试:它们用 /memory 驱动,其 mock 的 completion 声称完全匹配,而它们 mock 的 memory 命令有子命令却没有 action。生产环境永远不会产生这种组合 —— 真实的完全匹配逻辑要求存在可执行的 action,而所有可以被完整输入的真实命令都定义了 action(包括真实的 memory 命令和 MCP prompt 父命令)—— 所以这些测试断言的是 mock 造成的假象,而不是捕获了面向用户的回归。由于这两个 job 都是必需检查项,在修复之前每次合并 main 都会报告失败。

审阅者测试计划(Reviewer Test Plan)

如何验证

  1. 检出 maincca376f6aa,运行 npm run lint:ci。预期退出码为 1,并且恰好只有一条警告:packages/cli/src/ui/components/InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' react-hooks/exhaustive-deps。注意 npm run lint(没有 --max-warnings 0)只会警告,这正是它在本地看起来“干净”的原因。
  2. 仍在该 commit 上,运行 cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx。预期 2 failed | 213 passed (215),失败的是 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,报错均为 expected "spy" to be called with arguments: [ '/memory', …(1) ] / Number of calls: 0
  3. 检出本分支,重新运行上述两条命令。预期 lint:ci 退出码为 0 且无输出,测试为 215 passed (215)
  4. 确认 diff 恰好是两行新增:按键回调依赖数组中的 slashCommands,,以及 mock 的 memory 命令上的 action: vi.fn(),
  5. 对这两行分别做变异验证。移除 mock 的 action 必须让同样的两个测试再次失败(npx vitest run src/ui/components/InputPrompt.test.tsx -t "perfect match"2 failed | 1 passed)。移除依赖项必须让 npx eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 以退出码 1 失败。
  6. 确认相邻的测试 should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list 依然通过:当 /memory 可执行时,用方向键导航后按 Enter 仍必须自动补全高亮的子命令,而不是提交 —— 这正是该测试钉住的行为。
  7. 从行为上看,用户可见的部分不应有任何变化:输入一个完整的斜杠命令后按 Enter 会提交它,输入不完整的会触发补全,而在已导航的建议列表中按 Enter 会补全高亮项。

证据(改动前与改动后)

N/A —— 没有用户可见/TUI 变化。证据就是这两个 CI 门禁本身:

检查项 改动前(cca376f6aa 改动后(本分支)
npm run lint:ci 退出码 1 —— ESLint found too many warnings (maximum: 0),1 条 react-hooks/exhaustive-deps 警告 退出码 0,无任何问题
vitest run src/ui/components/InputPrompt.test.tsx 2 failed | 213 passed (215) 215 passed (215)
CI job Lint & StaticRun ESLint 失败(运行 33797332027) 预期成功
CI job TestRun tests and generate reports 失败(运行 33797332027) 预期成功

测试环境(Tested on)

操作系统 状态
🍏 macOS ⚠️ 未测试
🪟 Windows ⚠️ 未测试
🐧 Linux ✅ 已测试

环境(可选)

Linux 自托管 runner 检出,Node v22.23.2,npm 10.9.8,仅单元测试(未运行 CLI、未使用沙箱、未发起真实模型调用)。为排除环境依赖,测试还在类 CI 环境(取消 SANDBOX、使用临时 HOME、清空所有 API key)下重复运行:InputPrompt.test.tsx 在两种环境下均为 215/215。

风险与范围

  • 主要风险或取舍:运行时实际上没有风险。新增的依赖项不可能改变行为 —— 依赖数组中已有的 completion 对象在每次渲染时都会重建,所以该回调本来每次渲染都会重建;这一条目只是让 memo 语义诚实、并让 lint 门禁通过。测试侧的改动只涉及 mock,不涉及断言。
  • 未验证 / 范围之外:在这个沙箱中对 packages/cli workspace 跑全量测试会报出 9 个无关文件中的 33 个失败(settings/config、Windows 风格的 home 路径、拉起浏览器的命令、serve live-host 发现、Footer 快照)。它们是沙箱产物,而非 main 的失败:Footer 快照差异是多出一个 docker | 标记,因为这个 shell 运行在 Docker 内部且设置了 SANDBOX;取消 SANDBOX 并使用干净 HOME 重跑该文件,结果是 34/34 而不是 4 个失败。CI 历史也佐证了这一点 —— Test job 在 678ac2e1ec(紧接在只改动这两个文件的那个 commit 之前)是绿色的。同样超出范围:运行 33773880468(678ac2e1ec)中更早的那次 Lint & Static job 级失败,它没有报告任何失败的步骤,属于另一次瞬时的基础设施事件,而不是 Main CI failed: Qwen Code CI on cca376f6aa4b #10967 跟踪的失败。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

Fixes #10967

…hange (#10967)

Commit 661f41e (#10929) reddened both post-merge CI jobs on main, and
every run since has repeated the same two failures.

Lint & Static / "Run ESLint": the submit path now reads `slashCommands`
inside the keypress `useCallback`, but the dependency array was not
updated. CI runs `lint:ci` (`--max-warnings 0`), so the single
react-hooks/exhaustive-deps warning fails the job even though
`npm run lint` only warns. Add the dependency.

Test / "Run tests and generate reports": Enter on a slash buffer now
decides "perfect match" from the live buffer and the real command list
instead of the render-derived completion state. Two pre-existing
regression tests drive `/memory` through a mocked completion that claims
a perfect match while their mocked `memory` command is a container with
no `action` — a state the real hook cannot produce, because
`usePerfectMatch` requires an action and every real command that can be
typed exactly (including the real `memoryCommand` and MCP prompt parents)
defines one. Give the mock an `action` so both tests exercise a genuine
perfect match again; no assertion is weakened, and the neighbouring
"navigated perfect match autocompletes" test now covers the branch it
previously reached only by accident.

Fixes #10967.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

Autofix report — issue #10967 (main CI failure on cca376f6aa)

What CI reported

Run 33797332027 (Qwen Code CI, push to main, commit cca376f6aa). Job logs need authentication (the public log endpoint returned HTTP 403), so the failure was located from the public job/step conclusions and then reproduced locally on the exact commit:

  • Lint & Static (ubuntu-latest, Node 22.x) — step Run ESLint failed; every later step (actionlint, shellcheck, yamllint, Prettier, settings schema, companion notices, helper tests) was skipped.
  • Test (ubuntu-latest, Node 22.x) — step Run tests and generate reports failed.
  • All other jobs skipped (post-merge push runs only these two product jobs).

Root cause

Both failures trace to commit 661f41eef0 (#10929), which changed how Enter decides a slash-command "perfect match". CI history pins the boundary: the Test job was green at 678ac2e1ec (the commit immediately before) and red from 661f41eef0 onward — runs 33776698676, 33795521802 and 33797332027 all fail the same two steps.

1. Run ESLint. The new submit path reads the slashCommands prop inside the keypress useCallback (parseSlashCommand(buffer.text, slashCommands)) without adding it to the dependency array, producing a react-hooks/exhaustive-deps warning at InputPrompt.tsx:1892. CI's gate is node scripts/lint.js --eslintnpm run lint:cieslint . --ext .ts,.tsx --max-warnings 0, so a single warning fails the job. npm run lint (no --max-warnings) only warns — which is why the change looks clean locally and in a lint-only check.

2. Run tests and generate reports. For a buffer starting with /, Enter no longer consults the render-derived completion.isPerfectMatch; it recomputes from the live buffer and the real command list, and requires the resolved command to have an action. Two pre-existing #3701 regression tests drive /memory through a mocked completion that claims isPerfectMatch: true, while their mocked memory command is a container with subCommands and no action — a combination the real hook cannot produce, because usePerfectMatch (useSlashCompletion.ts) requires leafCommand.action or an actionable match. Both tests therefore stopped submitting:

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

Reproduced on the exact failing commit: Test Files 1 failed (1) / Tests 2 failed | 213 passed (215).

No production regression hides behind these test failures. Every real command that can be typed exactly defines an action — including memoryCommand and the MCP prompt parents built by McpPromptLoader (which declare both subCommands and an action), and all 14 command files that declare subCommands. So the new action !== undefined condition matches both production reality and the previous usePerfectMatch semantics; only the mock disagreed. Relaxing the condition was also ruled out because it would contradict #10929's own pinned test — should not submit a live partial slash command when completion is stale mocks isPerfectMatch: true for /cle and requires that Enter does not submit.

Fix (2 lines, no behavior change)

  • packages/cli/src/ui/components/InputPrompt.tsx — add slashCommands to the handleInput dependency array (also consistent with the two other callbacks in this file that already list it).
  • packages/cli/src/ui/components/InputPrompt.test.tsx — give the mocked memory command an action, matching the real memoryCommand and the mock's own export entry (which already pairs action with subCommands), so the two tests exercise a genuine perfect match again. No assertion was changed, weakened, or removed. Side benefit: the neighbouring should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list test previously reached the generic suggestion path only because /memory was not actionable; it now genuinely covers the navigated branch inside the perfect-match path, and still passes.

Mutation probes

  • Removed the mock action → the two tests FAIL (2 failed | 1 passed | 212 skipped under -t "perfect match"); restored → pass. So the test change is witnessed by the tests this round commits.
  • Removed slashCommands from the dependency array → npx eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 exits 1 with the exhaustive-deps warning; restored → exit 0. No Vitest witness is possible for this line: useCommandCompletion returns a fresh object literal every render, so completion (already a dependency) recreates handleInput on every render and a stale closure is unreachable from a test. The deterministic witness is the repository's own lint:ci gate, which both CI and the autofix verification gate re-run.

Local full-suite note (environment artifacts, with evidence)

A full packages/cli Vitest run inside this sandbox reports 33 failures across 9 files unrelated to this change (config.test.ts, settings.test.ts, cdCommand, directoryCommand, docsCommand, extensionsCommand, ideCommand, Footer.test.tsx, run-qwen-serve-live.test.ts). They are artifacts of the sandboxed runner, not main CI failures:

  • The Footer snapshot diff is an extra docker | pill: this shell runs inside Docker (/.dockerenv present) with SANDBOX=qwen-code-a4649b7e set. Re-running that one file with SANDBOX unset and a clean HOME gives 34/34 passed instead of 4 failed. The remaining files are the HOME-dependent, browser-spawning and network-listening tests, which CI runs with a clean HOME and no sandbox.
  • CI history corroborates: the Test job was green one commit before fix(cli): submit exact slash commands from the live input #10929, and fix(cli): submit exact slash commands from the live input #10929 touched only the two InputPrompt files, so those 9 files pass in CI.
  • InputPrompt.test.tsx is 215/215 both in this sandbox and under a CI-like environment (SANDBOX unset, temporary HOME, all API keys emptied).

Verification

  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0, no errors)
  • npm run lint — passed (exit 0; 0 errors, 0 warnings — the pre-fix run reported 1 warning)
  • npm run lint:ci (the exact CI ESLint gate, --max-warnings 0) — failed before the fix (exit 1, ESLint found too many warnings), passed after (exit 0)
  • npx prettier --check packages/cli/src/ui/components/InputPrompt.tsx packages/cli/src/ui/components/InputPrompt.test.tsx — passed ("All matched files use Prettier code style!")
  • npx vitest run src/ui/components/InputPrompt.test.tsx (packages/cli) — 215 passed (was 213 passed / 2 failed)
  • npx vitest run on InputPrompt.test.tsx, InputPrompt.suggestionMouse.test.tsx, Composer.test.tsx, useCommandCompletion.test.ts, useSlashCompletion.test.ts, useExportCompletion.test.ts6 files, 336 tests passed
  • npx vitest run src/ui/components/InputPrompt.test.tsx under a CI-like env (SANDBOX unset, temp HOME, API keys empty) — 215 passed
  • npx vitest run on the whole packages/cli workspace — 28109 passed, 33 failed in 9 unrelated files, all shown above to be sandbox artifacts (the same files pass with a CI-like env; InputPrompt.test.tsx is not among them)
  • npm run test:scripts — passed (exit 0)
  • Mutation probes (previous section) — both behaved as required
  • Not run: full-monorepo npm run test:ci:workspaces (only packages/cli and scripts were exercised), integration tests after npm run bundle (the change is fully covered by unit tests, not only reachable through the bundled CLI), npm run generate:settings-schema (no settings source touched)
中文说明

Autofix 报告 —— issue #10967cca376f6aa 上 main CI 失败)

CI 报告的内容

运行 33797332027Qwen Code CI,push 到 main,commit cca376f6aa)。作业日志需要鉴权(公开的日志接口返回 HTTP 403),因此先通过公开的 job/step 结论定位失败点,然后在完全相同的 commit 上本地复现:

  • Lint & Static (ubuntu-latest, Node 22.x) —— Run ESLint 步骤失败;其后所有步骤(actionlint、shellcheck、yamllint、Prettier、settings schema、companion notices、helper tests)均被跳过。
  • Test (ubuntu-latest, Node 22.x) —— Run tests and generate reports 步骤失败
  • 其余 job 全部跳过(合并后的 push 运行只跑这两个产品 job)。

根因

两个失败都源自 commit 661f41eef0#10929),它改变了 Enter 判定斜杠命令“完全匹配(perfect match)”的方式。CI 历史精确划出了边界:Test job 在 678ac2e1ec(紧邻的前一个 commit)上是绿色的,从 661f41eef0 起变红 —— 运行 337766986763379552180233797332027 都失败在同样的两个步骤上。

1. Run ESLint 新的提交路径在按键 useCallback 内部读取了 slashCommands 这个 prop(parseSlashCommand(buffer.text, slashCommands)),却没有把它加入依赖数组,于是在 InputPrompt.tsx:1892 产生 react-hooks/exhaustive-deps 警告。CI 的门禁是 node scripts/lint.js --eslintnpm run lint:cieslint . --ext .ts,.tsx --max-warnings 0,所以一条警告就会让 job 失败。而 npm run lint(没有 --max-warnings)只是警告 —— 这正是它在本地和只跑 lint 的检查看起来“干净”的原因。

2. Run tests and generate reports 对于以 / 开头的输入缓冲区,Enter 不再参考由渲染派生的 completion.isPerfectMatch,而是根据实时缓冲区和真实命令列表重新计算,并要求解析到的命令必须有 action。两个既有的 #3701 回归测试用 /memory 驱动,其被 mock 的 completion 声称 isPerfectMatch: true,而它们 mock 的 memory 命令是一个只有 subCommands没有 action 的容器命令 —— 真实的 hook 不可能产生这种组合,因为 usePerfectMatchuseSlashCompletion.ts)要求 leafCommand.action 或一个可执行的匹配项。于是这两个测试不再提交:

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

在完全相同的失败 commit 上复现结果为:Test Files 1 failed (1) / Tests 2 failed | 213 passed (215)

这些测试失败背后并不存在生产环境回归。 所有可以被完整输入的真实命令都定义了 action —— 包括 memoryCommand,以及 McpPromptLoader 构造的 MCP prompt 父命令(它们同时声明了 subCommandsaction),还有全部 14 个声明了 subCommands 的命令文件。因此新增的 action !== undefined 条件既符合生产实际,也与原先 usePerfectMatch 的语义一致;只有 mock 与之不符。放宽该条件的方案也被排除,因为它会与 #10929 自己钉住的测试冲突 —— should not submit a live partial slash command when completion is stale/cle mock 了 isPerfectMatch: true,并要求 Enter 提交。

修复(2 行,不改变行为)

  • packages/cli/src/ui/components/InputPrompt.tsx —— 把 slashCommands 加入 handleInput 的依赖数组(与该文件中另外两个已列出它的 callback 保持一致)。
  • packages/cli/src/ui/components/InputPrompt.test.tsx —— 给 mock 的 memory 命令加上 action,与真实的 memoryCommand 以及该 mock 自己的 export 条目(本来就是 action + subCommands)保持一致,使这两个测试重新覆盖真正的“完全匹配”。没有修改、削弱或删除任何断言。附带收益:相邻的 should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list 测试此前只是因为 /memory 不可执行才走到通用建议分支;现在它真正覆盖了完全匹配路径内部的“已导航”分支,并且依然通过。

变异探针(mutation probes)

  • 移除 mock 的 action → 两个测试失败(在 -t "perfect match" 过滤下为 2 failed | 1 passed | 212 skipped);恢复后 → 通过。因此测试侧的改动由本轮提交的测试自身作证。
  • 从依赖数组中移除 slashCommandsnpx eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 退出码为 1,并报出 exhaustive-deps 警告;恢复后 → 退出码 0。这一行无法用 Vitest 作证:useCommandCompletion 每次渲染都返回一个全新的对象字面量,因此 completion(已在依赖数组中)会让 handleInput 每次渲染都重建,测试中根本无法构造出过期闭包。确定性的见证者是仓库自身的 lint:ci 门禁 —— CI 和 autofix 验证门禁都会重新运行它。

本地全量测试说明(环境导致的假失败,附证据)

在这个沙箱里对 packages/cli 跑全量 Vitest,会报出 33 个失败,分布在 9 个与本次改动无关的文件中(config.test.tssettings.test.tscdCommanddirectoryCommanddocsCommandextensionsCommandideCommandFooter.test.tsxrun-qwen-serve-live.test.ts)。它们是沙箱运行环境的产物,而不是 main CI 的失败:

  • Footer 的快照差异是多出一个 docker | 标记:这个 shell 运行在 Docker 内部(存在 /.dockerenv),并且设置了 SANDBOX=qwen-code-a4649b7e。仅重跑该文件、取消 SANDBOX 并使用干净的 HOME,结果是 34/34 通过,而不是 4 个失败。其余文件分别依赖 HOME、需要拉起浏览器、需要监听网络端口,而 CI 使用干净的 HOME 且没有沙箱。
  • CI 历史同样佐证:Test job 在 fix(cli): submit exact slash commands from the live input #10929 前一个 commit 上是绿色的,而 fix(cli): submit exact slash commands from the live input #10929 只改动了两个 InputPrompt 文件,所以那 9 个文件在 CI 中是通过的。
  • InputPrompt.test.tsx 在这个沙箱中以及在类 CI 环境下(取消 SANDBOX、使用临时 HOME、清空所有 API key)都是 215/215 通过。

验证(Verification)

  • npm run build —— 通过(退出码 0)
  • npm run typecheck —— 通过(退出码 0,无错误)
  • npm run lint —— 通过(退出码 0;0 errors、0 warnings —— 修复前那次运行报出 1 条警告)
  • npm run lint:ci(即 CI 的 ESLint 门禁,--max-warnings 0)—— 修复前失败(退出码 1,ESLint found too many warnings),修复后通过(退出码 0)
  • npx prettier --check packages/cli/src/ui/components/InputPrompt.tsx packages/cli/src/ui/components/InputPrompt.test.tsx —— 通过(“All matched files use Prettier code style!”)
  • npx vitest run src/ui/components/InputPrompt.test.tsx(packages/cli)—— 215 通过(此前为 213 通过 / 2 失败)
  • npx vitest run 覆盖 InputPrompt.test.tsxInputPrompt.suggestionMouse.test.tsxComposer.test.tsxuseCommandCompletion.test.tsuseSlashCompletion.test.tsuseExportCompletion.test.ts —— 6 个文件、336 个测试全部通过
  • 在类 CI 环境下(取消 SANDBOX、临时 HOME、清空 API key)运行 npx vitest run src/ui/components/InputPrompt.test.tsx —— 215 通过
  • 对整个 packages/cli workspace 运行 npx vitest run —— 28109 通过,9 个无关文件中 33 个失败;上文已逐一证明其为沙箱环境产物(同样的文件在类 CI 环境下通过;InputPrompt.test.tsx 不在其中)
  • npm run test:scripts —— 通过(退出码 0)
  • 变异探针(见上一节)—— 两者均符合预期
  • 未运行:全仓库的 npm run test:ci:workspaces(只跑了 packages/cliscripts);npm run bundle 之后的集成测试(本次改动完全由单元测试覆盖,并非只能通过打包后的 CLI 触达);npm run generate:settings-schema(未改动任何 settings 源文件)

🧠 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

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 43dd35e did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 43dd35e 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is present, including the reviewer test plan and the Chinese translation.

Problem: observed, not theoretical — and I confirmed it from the CI logs rather than taking the description's word for it. Run 33797332027 on cca376f6aa failed exactly two required jobs. Lint & Static logged InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' react-hooks/exhaustive-deps under eslint . --max-warnings 0, and Test logged InputPrompt.test.tsx (215 tests | 2 failed) naming the two perfect-match tests, ending at Tests 2 failed | 28134 passed | 90 skipped (28226). One failed test file, two failed tests, one lint warning — the scope of the breakage and the scope of this PR line up exactly.

Direction: restoring the two required checks on main is about as clearly aligned as it gets; every merge reports red until this lands. Nothing here touches auth, sandboxing, model selection, telemetry, release, or a public contract, so no escalation on direction.

Size: not applicable. packages/cli/src/ui/components/** is not a core path under the Stage 0 definition and this is not a cross-package change. For the record: 2 added lines, 0 deleted, 1 production line + 1 test line.

Approach: the scope feels right, and I could not find a smaller one. I considered the three obvious alternatives and each is worse: an inline eslint-disable would silence the rule instead of satisfying it; reading slashCommands through a ref adds machinery for no gain (see the review — the callback is already recreated every render); and rewriting the two tests' expectations would have traded a fixture bug for lost coverage. No unrelated changes, no drive-by refactor, no formatting churn.

One thing I want to be explicit about, because it is the only way this PR could have been wrong: editing a test fixture to make a test pass is usually a red flag. Here it is the correct fix, and the reason is that the mocked state was unreachable in production — the details are in the code review below.

Risk: no elevated risk signals — no Stage 1e high-risk path matched.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需小标题都在,包括审阅者测试计划和中文翻译。

问题: 是已观测到的失败,不是理论性加固 —— 而且我是从 CI 日志里自己确认的,没有只采信 PR 描述。cca376f6aa 上的运行 33797332027 恰好失败了两条必需检查。Lint & Staticeslint . --max-warnings 0 下记录了 InputPrompt.tsx 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' react-hooks/exhaustive-depsTest 记录了 InputPrompt.test.tsx (215 tests | 2 failed),点名的正是那两个 perfect-match 测试,最终汇总为 Tests 2 failed | 28134 passed | 90 skipped (28226)。一个测试文件失败、两个测试失败、一条 lint 警告 —— 故障范围与本 PR 的改动范围完全吻合。

方向: 恢复 main 上两条必需检查,方向上再明确不过;在修好之前每次合并都会报红。这里没有触及鉴权、沙箱、模型选择、遥测、发布或对外契约,因此方向上无需上升处理。

规模: 不适用。按 Stage 0 的定义,packages/cli/src/ui/components/** 不是核心路径,本 PR 也不是跨包改动。记录一下:新增 2 行、删除 0 行,其中 1 行生产代码 + 1 行测试代码。

方案: 范围合理,我也找不到更小的方案。我考虑过三种显而易见的替代做法,都更差:行内 eslint-disable 是压掉规则而不是满足规则;用 ref 读取 slashCommands 会平白增加机制而没有收益(见下方代码审查 —— 该回调本来每次渲染都会重建);改写那两个测试的预期则会用fixture的bug换来覆盖率的损失。没有无关改动,没有顺手重构,没有格式化噪音。

有一点我想明确说明,因为这是本 PR 唯一可能出错的地方:为了让测试通过而去改测试 fixture,通常是个危险信号。这里却是正确的修法,原因在于被 mock 出来的那个状态在生产环境中根本不可达 —— 细节在下面的代码审查里。

风险: 无升级风险信号 —— Stage 1e 的高风险路径没有命中。

进入代码审查 🔍

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

Reviewed at 43dd35eb5d6e9f34df0e4832dd43c0525e8fc1b8 · 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

Before reading the diff I wrote down what I would have done from the title and the "why" alone: add the missing dependency to the callback's dependency array (an inline eslint-disable would silence the rule rather than satisfy it, and a ref would be machinery for nothing), and for the two tests, decide which of two worlds we are in — either the mocked state is reachable in production, in which case the tests caught a real regression and the submit logic is what needs fixing, or it is unreachable, in which case the fixture is wrong and the tests are asserting a mock artifact. That second question is the whole review, so I went and answered it from the source rather than from the description.

The PR's approach matches mine, and the diagnosis holds up under checking:

  • The lint warning is real. The keypress callback reads slashCommands at InputPrompt.tsx:1435-1437, and the dependency array at 1893-1957 did not list it. Adding it is the fix, and it is the one the rule itself suggests.
  • It genuinely cannot change behavior, which is the claim worth pinning down before waving a runtime edit through. completion is already in that array, and useCommandCompletion returns a plain unmemoized object literal (useCommandCompletion.tsx:437-457), so it is a new reference on every render and the callback was already being recreated every render. One more entry changes nothing observable; it only makes the memoization honest.
  • The mocked state was unreachable. Both perfect-match gates require an executable action: the live one at InputPrompt.tsx:1441 (commandToExecute?.action !== undefined && args.length === 0 && canonicalPath.length === commandPartCount) and the render-derived one at useSlashCompletion.ts:587,591-593. The real memoryCommand defines an action (memoryCommand.ts:18). A command that is a pure container with subcommands and no action can therefore never be reported as a perfect match, so the two tests were asserting against a fixture that production cannot produce. That is the case where fixing the mock is right.
  • No assertion was weakened. Both tests' expectations are untouched — onSubmit called with '/memory', handleAutocomplete not called. I traced each through the branch it now takes: with an actionable /memory, Enter reaches isSubmit && isCurrentPerfectMatch, the navigated guard fails (test 2860 never arrow-navigated; test 2798 remounts, so navigatedRef is fresh), and it lands on handleSubmitAndClear('/memory'). Before the change, isCurrentPerfectMatch was false, the block was skipped, and Enter fell through to the generic ACCEPT_SUGGESTION path at 1525-1540 — which autocompleted and never submitted, matching the Number of calls: 0 in the CI log exactly.
  • A neighbouring test quietly got better. should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list (2765) was written to cover the perfect-match + navigated + Enter branch, but with an unactionable mock it never entered that branch — it passed by accident through the generic accept path. With the fixture fixed it now goes through acceptActiveCompletionSuggestion() inside the branch it was written for, same assertions, real coverage.

Collateral check on the shared fixture, since mockSlashCommands is module-level and one edit could move unrelated tests: it is local to this file (InputPrompt.suggestionMouse.test.tsx and useSlashCompletion.test.ts each define their own). Within the file, only three tests press Enter on a bare /memory buffer — the three above. The Tab test at 1899-1909 uses /memory with a trailing space, which the !/\s$/.test(buffer.text) guard excludes, and the whole cursor-based completion trigger block (3258-3679) never sends a keystroke that reaches the submit path. Nothing else moves.

No critical blockers and no AGENTS.md violations. Two non-blocking observations for a follow-up, neither of which belongs in this PR:

  • For any buffer starting with /, the ternary at 1449-1451 always takes the live branch, so completion.isPerfectMatch no longer participates in the submit decision for slash input — which means the isPerfectMatch: true these three tests mock is now inert. Worth a cleanup pass so the fixture does not imply a dependency that is not there.
  • parseSlashCommand matches names case-sensitively (commands.ts:90-96, though it does resolve altNames) while matchesCommand lowercases both sides. That divergence predates this PR and is arguably the safer of the two, since parseSlashCommand is what the executor actually runs — Enter now submits precisely what would execute.

Testing

This is an unattended CI run, so per the gate's rules I did not build or execute anything from this PR. The evidence below is the PR's own CI, read through the API, plus the base-commit logs I quoted in Stage 1 — not the author's local results. I have not re-run the suite myself and am not presenting the description's local numbers as verification.

The two gates this PR targets, Lint & Static (ubuntu-latest, Node 22.x) and Test (ubuntu-latest, Node 22.x), were still in progress when I fetched this snapshot; one pull_request workflow run remains open. Everything already settled is green, including the surfaces closest to this change — TUI parity snapshots (ink vs opentui) and OpenTUI no-flicker gate, which is the check I would have expected to complain if the ink submit path and its OpenTUI port had drifted. Test (macos/windows) and the CLI integration matrix are skipped on this PR by workflow design, not failing. I am not guessing the outcome of the two pending gates — the table below is refreshed in place by the finalize job once they land.

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

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
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
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,失败项排在最前。

Real-scenario testing: N/A — nothing user-visible changes, and on an unattended CI run the live-behaviour lane is not driven from here. No sandboxed verification lane applies either: the claim on trial is "the two required gates go green", which is exactly what the CI signal above settles, and there is no behavioural delta for /verify to A/B or TUI surface for /tmux to capture. The one thing CI cannot tell you — whether these tests pin the change — I checked by reading instead: at the base commit the fixture had no action, and the base run shows both tests failing with Number of calls: 0. That is the mutation already run for us, so the tests are load-bearing on the line this PR adds.

中文说明

在看 diff 之前,我先只根据标题和「为什么需要」写下了自己的做法:把缺失的依赖加进回调的依赖数组(行内 eslint-disable 是压掉规则而不是满足规则,用 ref 则是白费机制);对那两个测试,先判断我们处在两种情况中的哪一种 —— 要么被 mock 的状态在生产中可达,那么测试抓到的是真实回归、需要修的是提交逻辑;要么不可达,那么错的是 fixture、测试断言的只是 mock 造成的假象。第二个问题就是整场审查的核心,所以我去源码里自己回答了它,而不是采信 PR 描述。

本 PR 的做法与我的一致,诊断经得起核对:

  • lint 警告是真的。 按键回调在 InputPrompt.tsx:1435-1437 读取 slashCommands,而 1893-1957 的依赖数组没有列出它。补上就是修复,也正是规则本身给出的建议。
  • 它确实不可能改变行为 —— 在放行一处运行时代码改动之前,这一点值得钉死。completion 已经在数组里,而 useCommandCompletion 返回的是一个未做 memo 的普通对象字面量(useCommandCompletion.tsx:437-457),每次渲染都是新引用,所以该回调本来每次渲染都会重建。多加一项不会改变任何可观测行为,只是让 memo 语义变得诚实。
  • 被 mock 出来的状态不可达。 两条 perfect-match 判定都要求存在可执行 action:实时那条在 InputPrompt.tsx:1441commandToExecute?.action !== undefined && args.length === 0 && canonicalPath.length === commandPartCount),渲染派生那条在 useSlashCompletion.ts:587,591-593。真实的 memoryCommand 定义了 action(memoryCommand.ts:18)。因此一个只有子命令、没有 action 的纯容器命令永远不可能被报告为 perfect match —— 那两个测试断言的是一个生产环境产生不出来的 fixture。这正是「改 mock 才是对的」那种情况。
  • 没有削弱任何断言。 两个测试的预期原封不动 —— onSubmit'/memory' 被调用、handleAutocomplete 未被调用。我把它们各自现在走的分支都跟了一遍:/memory 可执行后,Enter 进入 isSubmit && isCurrentPerfectMatch,navigated 守卫不成立(2860 从未按方向键;2798 重新挂载,navigatedRef 是全新的),于是落到 handleSubmitAndClear('/memory')。改动之前 isCurrentPerfectMatch 为 false,整个块被跳过,Enter 落到 1525-1540 的通用 ACCEPT_SUGGESTION 路径 —— 它会补全而不提交,与 CI 日志里的 Number of calls: 0 完全对应。
  • 相邻的一个测试悄悄变好了。 should autocomplete on Enter when user arrow-navigated a perfect-match suggestion list(2765)本来就是为覆盖 perfect-match + 已导航 + Enter 这个分支而写的,但在不可执行的 mock 下它从未进入该分支 —— 它是通过通用补全路径「碰巧」通过的。fixture 修好后,它现在走的是该分支内部的 acceptActiveCompletionSuggestion(),断言不变,覆盖变真。

共享 fixture 的连带影响检查mockSlashCommands 是模块级的,一处改动可能牵动无关测试):它只属于本文件(InputPrompt.suggestionMouse.test.tsxuseSlashCompletion.test.ts 各自定义了自己的)。文件内只有三个测试会在裸 /memory 缓冲区上按 Enter —— 就是上面那三个。1899-1909 的 Tab 测试用的是带尾随空格的 /memory ,被 !/\s$/.test(buffer.text) 守卫排除;整个 cursor-based completion trigger 块(3258-3679)也不会送出任何抵达提交路径的按键。其余测试不受影响。

没有阻断性问题,也没有违反 AGENTS.md。两点不阻断的后续观察,都不属于本 PR 的范围:

  • 对任何以 / 开头的缓冲区,1449-1451 的三元表达式总是走实时分支,因此 completion.isPerfectMatch 已不再参与斜杠输入的提交判定 —— 也就是说这三个测试 mock 的 isPerfectMatch: true 现在是失效的。值得后续清理一下,免得 fixture 暗示出一个并不存在的依赖。
  • parseSlashCommand 对命令名是大小写敏感的(commands.ts:90-96,但它确实解析 altNames),而 matchesCommand 两侧都转小写。这个差异早于本 PR,而且可以说两者之中它反而更安全,因为 parseSlashCommand 正是执行器真正调用的那个 —— 现在 Enter 提交的恰好就是会被执行的内容。

测试证据: 这是一次无人值守的 CI 运行,按门禁规则我没有构建或执行本 PR 的任何代码。下面的证据是 PR 自己的 CI(通过 API 读取),加上我在 Stage 1 引用的基线 commit 日志 —— 不是作者的本地结果。我没有自己重跑测试套件,也不会把描述里的本地数字当作验证呈现。

本 PR 针对的两条门禁 Lint & Static (ubuntu-latest, Node 22.x)Test (ubuntu-latest, Node 22.x) 在我抓取快照时仍在运行,还有一个 pull_request workflow run 未结束。已经出结果的检查全部为绿,包括与本次改动最接近的两个面 —— TUI parity snapshots (ink vs opentui)OpenTUI no-flicker gate;如果 ink 的提交路径与它的 OpenTUI 端口发生了漂移,我预期正是这两条会报错。Test (macos/windows) 和 CLI 集成矩阵在本 PR 上是按 workflow 设计 skipped,不是失败。我不去猜这两条未完成门禁的结果 —— 它们跑完后,下方表格会由 finalize 任务就地刷新。

真实场景测试:N/A —— 没有用户可见的变化,而且无人值守 CI 运行不会从这里驱动实时行为验证。也不适用任何沙箱验证通道:本次待判定的主张就是「两条必需门禁变绿」,而这正是上面 CI 信号能够判定的内容;不存在可供 /verify 做 A/B 的行为差异,也没有可供 /tmux 采集的 TUI 界面。CI 唯一无法告诉你的是「这些测试是否真的钉住了改动」—— 这一点我改用读代码来核对:在基线 commit 上 fixture 没有 action,而基线运行显示两个测试都以 Number of calls: 0 失败。这等于变异验证已经替我们跑过了,所以这两个测试确实承重于本 PR 新增的这一行。

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

Reviewed at 43dd35eb5d6e9f34df0e4832dd43c0525e8fc1b8 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean across every stage and I could not break it; the two reservations I have are follow-ups that predate this PR, and the one thing still outstanding is CI itself.

Stepping back: this is the rare two-line PR where the honest question is not "is the code good" but "is it allowed to be this small". A red main blocks every merge and buries real regressions in noise, so the goal is worth exactly this much effort and no more — one line per red gate, nothing beside it. My independent proposal was the same two edits, and I did not find a smaller or safer path.

The place a PR like this does damage, if it does damage, is the test-fixture edit — "make the mock satisfy the test" is how a real regression gets buried. So that is where I spent the review, and it holds: the state the mock described (a command with subcommands, no action, reported as a perfect match) cannot occur in production, because both perfect-match gates require an executable action and the real memory command has one. The base commit's own CI run is the mutation proof — with the fixture as it was, both tests failed with Number of calls: 0, which is exactly the fall-through path I traced. Nothing was weakened; one neighbouring test went from passing by accident to covering the branch it was written for.

The runtime line is the other place to be suspicious, and it is inert: the callback already carried a fresh-every-render object in its dependency array, so it was never actually memoized. Adding the entry satisfies the linter without changing a single observable behavior. Six months from now this reads as the fix the lint rule was asking for, not as a workaround.

What keeps this at 4 rather than 5: the two follow-ups named in the review — the render-derived perfect-match flag is now dead for slash buffers, so the mocked isPerfectMatch: true in these three tests no longer means anything, and the two name-matching helpers disagree on case sensitivity. Both predate this PR, neither is a reason to hold it, and folding either in here would only widen a diff whose virtue is its size.

Verdict: approve — deferred until CI lands green. The two gates this PR exists to fix, Lint & Static and Test (ubuntu-latest), were still in progress on 43dd35eb5d6e9f34df0e4832dd43c0525e8fc1b8 when I wrote this, so I am not posting an approval now: approving against a result that does not exist yet would attest to something I have not seen. Approval is therefore pinned to that commit and deferred until every check on it completes green; if either gate lands red, or the head moves, the approval is withheld and the status comment says so. I would want that to be treated as a real signal, not a formality — my read is that both should pass, but the base run is the only test evidence that exists so far.

中文说明

信心:4/5 —— 每个阶段都干净,我没能把它找出破绽;我保留的两点都是早于本 PR 的后续事项,而唯一还悬着的就是 CI 本身。

退一步看:这是那种少见的两行 PR,真正该问的不是「代码好不好」,而是「它是否可以小到这个程度」。main 变红会阻塞每一次合并,并把真正的回归埋在噪音里,所以这个目标恰好值得这么多工作量、不多也不少 —— 每条红门禁一行,此外别无他物。我独立想到的方案就是这两处改动,也没有找到更小或更安全的路径。

这类 PR 如果真会造成损害,损害点就在测试 fixture 的那处改动上 —— 「让 mock 去满足测试」正是真实回归被埋掉的方式。所以我把审查的力气花在了那里,而它站得住:mock 描述的那个状态(一个有子命令、没有 action、却被报告为 perfect match 的命令)在生产中不可能出现,因为两条 perfect-match 判定都要求存在可执行 action,而真实的 memory 命令有。基线 commit 自己的 CI 运行就是变异验证 —— 在 fixture 原样的情况下,两个测试都以 Number of calls: 0 失败,正好对应我跟出来的那条 fall-through 路径。没有任何断言被削弱;相邻的一个测试则从「碰巧通过」变成了真正覆盖它本来要覆盖的分支。

运行时那一行是另一个该保持怀疑的地方,而它是惰性的:该回调的依赖数组里本来就有一个每次渲染都重建的对象,所以它从未真正被 memo 住。加上这一项只是满足了 linter,不改变任何可观测行为。半年后再看,这就是 lint 规则本来要求的那个修复,而不是一个绕法。

之所以是 4 而不是 5:审查里点名的两个后续事项 —— 对斜杠缓冲区而言,渲染派生的 perfect-match 标志已经失效,所以这三个测试里 mock 的 isPerfectMatch: true 不再有任何意义;以及两个命令名匹配函数在大小写敏感性上不一致。两者都早于本 PR,都不构成压住它的理由,而把任何一个塞进来都只会扩大这个以「小」为优点的 diff。

结论:批准 —— 但推迟到 CI 转绿。 本 PR 存在的意义就是修好那两条门禁,而在我写下这段时,Lint & StaticTest (ubuntu-latest)43dd35eb5d6e9f34df0e4832dd43c0525e8fc1b8 上仍在运行,所以我现在不提交批准:对一个尚不存在的结果批准,等于为我没见过的东西背书。因此批准被钉在该 commit 上,并推迟到它上面的所有检查全部转绿之后;如果任一门禁变红、或者 head 发生移动,批准将被撤回,状态评论会说明原因。我希望这一点被当作真实信号而不是走过场 —— 我的判断是两条都应该通过,但到目前为止,唯一存在的测试证据是基线那次运行。

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

Reviewed at 43dd35eb5d6e9f34df0e4832dd43c0525e8fc1b8 · 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.

⚠️ Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). 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.

中文说明

⚠️ 已从批准降级为评论:CI failing: Test (ubuntu-latest, Node 22.x)。 仅完成部分审查,审查缺口已披露。

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

Test Plan(非阻断):src/ui/components/InputPrompt.test.tsxno such file or directory

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

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), review-pr] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x), review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@wenshao

wenshao commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Local verification report — PR #10968

Verdict: the change is correct and every claim in the PR body reproduces, but this PR is now an empty diff against main — recommend closing it as superseded by #10961 rather than merging.

1. The PR no longer changes anything

Everything below verifies the change itself, so the record is complete whichever way it is closed.

2. Environment

Fresh git worktree at the PR head 0dfc6a8b58, npm ci (Node v22.22.2, npm 10.9.7, Linux). For the real-TUI runs I built dist/cli.js with npm run bundle and launched it in tmux with an isolated HOME/QWEN_HOME, a pre-trusted workspace and OpenAI-compatible env vars pointing at a dead port (slash commands never call the model). I also built the pre-#10929 base 678ac2e1ec the same way for an A/B; the lockfile is identical across every commit used here, so one node_modules served all of them.

3. Gate replay (Reviewer Test Plan steps 1–4)

Commit eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 vitest run src/ui/components/InputPrompt.test.tsx
cca376f6aa (before) exit 1 — 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' 2 failed | 213 passed (215) — exactly the two named tests, both expected "spy" to be called with arguments: [ '/memory', …(1) ] / Number of calls: 0
43dd35eb5d (PR fix commit) exit 0 215 passed (215)
0dfc6a8b58 (PR head = main) exit 0; full npm run lint:ci also exit 0 with no findings 215 passed (215)

Diff confirmed as exactly two added lines (slashCommands, in the handleInput dependency array; action: vi.fn(), on the mocked memory command).

4. Mutation probes (step 5) — both lines are load-bearing

Run at the PR head, file restored with git checkout after each probe:

  • Remove action: vi.fn(), from the memory mock → npx vitest run … -t "perfect match"2 failed | 1 passed | 212 skipped, same two tests, same Number of calls: 0.
  • Remove slashCommands, from the dependency array → single-file eslint gate → exit 1 with the same react-hooks/exhaustive-deps warning at 1892:5.
  • The "cannot change runtime behaviour" argument holds: useCommandCompletion returns an un-memoised object literal (packages/cli/src/ui/hooks/useCommandCompletion.tsx:437), so completion is a new reference on every render and handleInput was already rebuilt every render before this dependency was added.

5. Real TUI behaviour (steps 6–7), with an A/B against the pre-#10929 build

Four probes, each in a fresh CLI session, run on the PR-head bundle and again on the 678ac2e1ec bundle:

Probe Input Expected PR head Base 678ac2e1ec
P1 /stats (exact; real command with action and subCommands) + Enter submits ✅ stats panel rendered, input cleared ✅ identical
P2 /agents (exact; real container without action) + Enter must not submit ✅ autocompletes to /agents manage ✅ identical
P3 /sta (partial) + Enter autocompletes /stats ✅ identical
P4 /stats + ↓ + Enter (navigated list) autocompletes highlighted entry /stats tools , no submit ✅ identical

Seven pane captures per arm were diffed after stripping blanks: all identical except the random tip line and the session UUID inside the /stats panel. So the user-visible behaviour the PR describes is real, and it is unchanged from before #10929.

P1: /stats typed with its subcommand popup, then Enter submits and the stats panel renders

P2: /agents typed, Enter autocompletes to /agents manage instead of submitting

P4: /stats with the list navigated to tools, Enter autocompletes to /stats tools

A/B: /agents + Enter on the pre-#10929 base (left) and on the PR head (right) — identical

A/B: /stats + down + Enter on the pre-#10929 base (left) and on the PR head (right) — identical

6. One correction to the PR narrative (does not change the conclusion)

The body says every real command that can be typed exactly defines an action, and the autofix report extends that to "all 14 command files that declare subCommands". That is not accurate: /agents (agentsCommand.ts), /arena (arenaCommand.ts) and /ide when an IDE is detected (ideCommand.ts, subCommands assigned later) are containers with no top-level action. It does not undermine the fix, because the pre-#10929 usePerfectMatch (useSlashCompletion.ts:575) already required action, so those commands were never a perfect match under either implementation — probe P2 above shows the identical outcome on both builds. Also worth noting: the real memoryCommand is now a dialog leaf with no subCommands; the test fixture's memory with show/add/refresh is a legacy shape, which is fine for what the tests pin.

7. CI on this PR, as of posting

  • Lint & Static — pass.
  • Test (ubuntu-latest, Node 22.x)fail, but not from this change. packages/cli passed 1003 files / 28136 tests; the only failure is packages/web-shell/client/components/MessageList.dom.test.tsx › drops the anchor instead of re-expanding when the user collapsed the anchored turn (expected "spy" to be called 2 times, but got 1 times, 3/3 attempts under --retry=2) after 65 minutes on ecs-qwen-hk4-26. The identical tree passed this file on main's green run 33813966420 (✓ … (163 tests)), it passes 3/3 locally here, and it is the wall-clock-sensitive class tracked in ci: Test (ubuntu) fails non-deterministically on shared runners — a different test set each run #10490, which lists this exact file.
  • Dependency CVE auditfail from the npm registry (npm warn audit 503 Service Unavailable), infrastructure only; main's Security Checks run on the same tree is green.

8. Not covered

  • I did not re-run the full packages/cli suite locally; CI already ran it twice on this exact tree (this PR's run and main's run 33813966420), both green for that workspace.
  • macOS and Windows were not exercised.
中文版

PR #10968 本地验证报告

结论:改动本身正确,PR 描述中的每一条声明都能复现;但这个 PR 现在相对 main 已经是空 diff —— 建议按“已被 #10961 取代”关闭,而不是合并。

1. 这个 PR 已经不再改变任何东西

下面的内容用于完整地验证改动本身,无论最终怎么关闭都留有记录。

2. 环境

在 PR head 0dfc6a8b58 上新建 git worktree,npm ci(Node v22.22.2,npm 10.9.7,Linux)。真实 TUI 验证用 npm run bundle 构建 dist/cli.js,在 tmux 中以隔离的 HOME/QWEN_HOME、预先信任的工作目录、以及指向死端口的 OpenAI 兼容环境变量启动(斜杠命令不会调用模型)。同时用同样方式构建了 #10929 之前的基线 678ac2e1ec 做 A/B;所涉及的所有 commit 的 lockfile 完全一致,因此一份 node_modules 通用。

3. 门禁复现(Reviewer Test Plan 第 1–4 步)

Commit eslint packages/cli/src/ui/components/InputPrompt.tsx --max-warnings 0 vitest run src/ui/components/InputPrompt.test.tsx
cca376f6aa(改动前) 退出码 1 —— 1892:5 warning React Hook useCallback has a missing dependency: 'slashCommands' 2 failed | 213 passed (215) —— 正是那两个测试,均为 expected "spy" to be called with arguments: [ '/memory', …(1) ] / Number of calls: 0
43dd35eb5d(PR 修复 commit) 退出码 0 215 passed (215)
0dfc6a8b58(PR head = main 退出码 0;完整的 npm run lint:ci 同样退出码 0、无任何问题 215 passed (215)

已确认 diff 恰好是两行新增(handleInput 依赖数组中的 slashCommands,,以及 mock 的 memory 命令上的 action: vi.fn(),)。

4. 变异验证(第 5 步)—— 两行都是起作用的

在 PR head 上执行,每次探针后用 git checkout 恢复文件:

  • 删除 memory mock 上的 action: vi.fn(),npx vitest run … -t "perfect match"2 failed | 1 passed | 212 skipped,同样的两个测试,同样的 Number of calls: 0
  • 删除依赖数组中的 slashCommands, → 单文件 eslint 门禁 → 退出码 1,同样的 react-hooks/exhaustive-deps 警告出现在 1892:5
  • “不可能改变运行时行为”的论证成立:useCommandCompletion 返回的是未 memo 的对象字面量(packages/cli/src/ui/hooks/useCommandCompletion.tsx:437),所以 completion 每次渲染都是新引用,handleInput 在加入这个依赖之前本来就每次渲染重建。

5. 真实 TUI 行为(第 6–7 步),并与 #10929 之前的构建做 A/B

四个探针,每个都在全新的 CLI 会话中执行,分别跑在 PR head 的 bundle 和 678ac2e1ec 的 bundle 上:

探针 输入 预期 PR head 基线 678ac2e1ec
P1 /stats(完整;真实命令,同时有 action subCommands)+ Enter 提交 ✅ 渲染出 stats 面板,输入框清空 ✅ 完全一致
P2 /agents(完整;真实容器命令,没有 action)+ Enter 不得提交 ✅ 自动补全为 /agents manage ✅ 完全一致
P3 /sta(不完整)+ Enter 自动补全 /stats ✅ 完全一致
P4 /stats + ↓ + Enter(已导航的列表) 补全高亮项 /stats tools ,未提交 ✅ 完全一致

每个分支各 7 张终端截屏,去掉空行后逐一 diff:除随机 tip 行和 /stats 面板里的 session UUID 外全部一致。因此 PR 描述的用户可见行为是真实的,并且与 #10929 之前完全相同。

截图见上方英文部分(图 1–5)。

6. 对 PR 叙述的一处更正(不影响结论)

PR 正文称“所有可以被完整输入的真实命令都定义了 action”,autofix 报告进一步扩展为“所有 14 个声明 subCommands 的命令文件”都如此。这并不准确:/agentsagentsCommand.ts)、/arenaarenaCommand.ts),以及检测到 IDE 时的 /ideideCommand.tssubCommands 稍后赋值)都是没有顶层 action 的容器命令。这不影响修复本身,因为 #10929 之前的 usePerfectMatchuseSlashCompletion.ts:575)同样要求 action,所以这些命令在新旧两套实现下都从来不是完全匹配 —— 上面的探针 P2 在两个构建上结果完全一致。另外值得一提:真实的 memoryCommand 现在是一个没有 subCommands 的对话框叶子命令;测试 fixture 里带 show/add/refreshmemory 是旧形态,对这些测试要钉住的行为来说没有问题。

7. 发布本评论时该 PR 的 CI 状态

  • Lint & Static —— 通过。
  • Test (ubuntu-latest, Node 22.x) —— 失败,但与本改动无关。packages/cli 通过了 1003 个文件 / 28136 个测试;唯一的失败是 packages/web-shell/client/components/MessageList.dom.test.tsx › drops the anchor instead of re-expanding when the user collapsed the anchored turnexpected "spy" to be called 2 times, but got 1 times--retry=2 下 3/3 次失败),在 ecs-qwen-hk4-26 上跑了 65 分钟。完全相同的树在 main 的绿色运行 33813966420 中通过了该文件(✓ … (163 tests)),在本机本地 3/3 通过,并且它属于 ci: Test (ubuntu) fails non-deterministically on shared runners — a different test set each run #10490 跟踪的对墙钟敏感的一类,该 issue 明确列出了这个文件。
  • Dependency CVE audit —— 失败,来自 npm registry(npm warn audit 503 Service Unavailable),纯基础设施问题;同一棵树上 main 的 Security Checks 是绿的。

8. 未覆盖的部分

  • 没有在本地重跑完整的 packages/cli 套件;CI 已经在这棵完全相同的树上跑过两次(本 PR 的运行和 main 的运行 33813966420),该 workspace 均为绿。
  • 未在 macOS 和 Windows 上验证。

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 cca376f6aa4b

3 participants