fix(cli): give the case-variant delete test the attachments mock - #9551
Conversation
`deleteDaemonSessions` takes `bridge: Pick<AcpSessionBridge, 'closeSession' | 'deleteSessionAttachments'>` since #9477, which updated every mock that existed when it was written. #9341 landed in parallel and added one more — "collapses case-variant spellings in one batch to a single delete" — with a `{ closeSession }` bridge. Each PR was green on its own merge ref; main is red combined, so `npm ci` fails the build for every branch cut from it: src/serve/server/session-archive.test.ts(1069,7): error TS2741: Property 'deleteSessionAttachments' is missing in type '{ closeSession: Mock<Procedure> }' Adds the same `vi.fn().mockResolvedValue(undefined)` its neighbours already pass. The test asserts on the delete result, not on the spy, so its meaning is unchanged: 48/48 still pass.
|
Thanks — this is the "main doesn't build" repair, so I ran the gate against the base tree rather than taking the description's word for it. Template: free-form headings (What / Why it happened / The fix / Verification) instead of the template's sections. Normally a bounce, but — same call as on #9517 — the content covers everything the template asks for, including before/after verification, so this is a substance pass with the deviation noted, not a block. Problem: real, verified in the base tree. At Direction: aligned — Size: not a core path ( Approach: minimal, and the only correct minimal — loosening the Risk: no high-risk path matches (the only changed file is a Moving on to code review. 🔍 中文说明感谢——这是修复「main 无法构建」的 PR,所以我对照 base 树独立核实,而不是只看描述。 模板:自由格式标题(What / Why it happened / The fix / Verification),而非模板章节。按惯例应打回,但和 #9517 一样,内容覆盖了模板要求的一切(含 before/after 验证),因此按实质通过处理,只记录偏差,不阻塞。 问题:真实存在,已在 base 树验证。 方向:对齐—— 规模:非核心路径( 方案:最小,且是唯一正确的最小方案——放宽 风险:无高风险路径命中(唯一改动文件是 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first: with The diff is the minimum set: one mock site, +4/−1. The test asserts on the delete result ( Test evidence — this PR's own CI, read via the API (no PR code run here)The oracle for this PR is the macOS/Windows
( Real-scenario testing
中文说明代码审查先独立给出方案: diff 就是最小集:单个 mock 位置,+4/−1。该测试断言删除结果( 测试证据——本 PR 自己的 CI,通过 API 读取(本地未运行任何 PR 代码)本 PR 的判据是 macOS/Windows 的 ( 真实场景测试
— Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — problem verified in the base tree, the fix is the only correct minimal one, and the CI step that has been failing repo-wide just passed on the reviewed commit; approval is deferred only because CI is still in flight. Stepping back: this is exactly the repair Verdict: approve. Two 中文说明信心:5/5——问题已在 base 树核实,修复是唯一正确的最小方案,而全仓库一直失败的 CI 步骤刚在被审 commit 上通过;批准仅因 CI 仍在运行而推迟。 退一步看:这正是 结论:批准。被审 commit 上还有两个 — Qwen Code · qwen3.8-max Reviewed at |
The case-variant delete test fix carried by the previous merge is superseded by QwenLM#9551, which landed the same mock on main; this merge takes main's version of session-archive.test.ts.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Released in v0.21.15. |
What
maindoes not build.npm cirunsprepare→build, andtsc --buildfails:Every branch cut from current
maininherits it — the Test, web-shell E2E Smoke and Coverage jobs all fail in Install dependencies, before a single test runs.Why it happened
Two PRs that were each green on their own merge ref:
a41d5ec058) wideneddeleteDaemonSessions' parameter toPick<AcpSessionBridge, 'closeSession' | 'deleteSessionAttachments'>and updated every mock that existed at the time.a659539bc7) landed in parallel and added one more test — "collapses case-variant spellings in one batch to a single delete" — passingbridge: { closeSession }.Neither could see the other. There is no post-merge CI run on
main, so nothing caught the combination.The fix
One mock site, given the same
vi.fn().mockResolvedValue(undefined)its neighbours at lines 1048/1096/1126 already pass.Verification
origin/mainwith per-worktree@qwen-code/*symlinks (sopackages/cliresolves siblings to the worktree, not a stale checkout): base reproduces CI'sTS2741verbatim; with the patch,session-archiveerrors go to 0.vitest run src/serve/server/session-archive.test.ts→ 48/48 pass. The test asserts on the delete result, not on the spy, so its meaning is unchanged.中文说明
main目前构建失败:npm ci的prepare会跑build,tsc --build在session-archive.test.ts:1069报TS2741。所有从当前main切出的分支都会继承这个失败,CI 在「Install dependencies」阶段就挂掉,一个测试都跑不到。成因是两个 PR 的语义冲突:#9477 把
deleteDaemonSessions的bridge参数收紧为Pick<AcpSessionBridge, 'closeSession' | 'deleteSessionAttachments'>,并更新了当时存在的所有 mock;#9341 并行合入,新增了一个只传{ closeSession }的用例。两者各自的 merge ref 都是绿的,而main合并后没有 CI 复跑,于是无人发现。修复只动一处 mock,补上与相邻用例(1048/1096/1126 行)完全一致的
vi.fn().mockResolvedValue(undefined)。验证:A/B 类型检查确认 base 能复现 CI 的原始报错、打上补丁后归零;该测试文件 48/48 通过,断言的是删除结果而非该 spy,语义不变。