Skip to content

test(serve): mock attachment cleanup in batch delete test - #9554

Closed
ytahdn wants to merge 1 commit into
mainfrom
codex/fix-session-delete-attachment-mock
Closed

test(serve): mock attachment cleanup in batch delete test#9554
ytahdn wants to merge 1 commit into
mainfrom
codex/fix-session-delete-attachment-mock

Conversation

@ytahdn

@ytahdn ytahdn commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Updates the case-variant batch-delete regression test to provide the session attachment cleanup dependency and verifies that attachment cleanup runs exactly once for duplicate case spellings.

Why it's needed

Two closely timed merges left main with a semantic integration mismatch: the batch-delete API began requiring attachment cleanup after another PR had added a test mock containing only session close. The resulting TypeScript error blocks CLI builds and downstream CI jobs.

Reviewer Test Plan

How to verify

Run the session archive test and CLI typecheck. The archive suite should pass all 48 tests, the case-variant delete test should call both session close and attachment cleanup once, and TypeScript should no longer report TS2741 for the bridge mock.

Evidence (Before & After)

N/A — test-only change. Before: CLI compilation fails because the bridge mock lacks deleteSessionAttachments. After: the targeted suite and CLI typecheck pass.

Tested on

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

Environment (optional)

Node.js 22 workspace checkout.

Risk & Scope

  • Main risk or tradeoff: Minimal; this changes only a test mock and its assertion.
  • Not validated / out of scope: No UI or runtime behavior changes; Windows and Linux were not tested locally.
  • Breaking changes / migration notes: None.

Linked Issues

Follow-up integration fix for #9477 after #9341 merged a new caller shortly before it.

中文说明

本 PR 的改动

更新批量删除中大小写变体的回归测试,为其补充 session 附件清理依赖,并验证同一 session 的重复大小写拼写只会触发一次附件清理。

修改原因

两个时间非常接近的合并在 main 中形成了语义集成不一致:一个 PR 新增了仅包含 session close 的测试 mock,随后批量删除接口开始要求附件清理依赖,最终导致 TypeScript 编译错误,并阻塞 CLI 构建及下游 CI。

Reviewer Test Plan

验证方法

运行 session archive 测试和 CLI typecheck。archive 测试应当 48 项全部通过;大小写变体删除用例应分别只调用一次 session close 和附件清理;TypeScript 不应再为 bridge mock 报告 TS2741。

前后证据

N/A——仅测试改动。修改前:CLI 因 bridge mock 缺少 deleteSessionAttachments 而编译失败。修改后:定向测试和 CLI typecheck 均通过。

已测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

Node.js 22 workspace checkout。

风险与范围

  • 主要风险或权衡:极低,仅修改测试 mock 及其断言。
  • 未验证或不在范围内:没有 UI 或运行时行为改动;未在本地测试 Windows 和 Linux。
  • 破坏性改动或迁移说明:无。

关联事项

这是 #9341#9477 合并前不久新增调用点后产生的集成 follow-up 修复。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@ytahdn

ytahdn commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as duplicate of #9551, which has already merged the same fix into main.

@ytahdn ytahdn closed this Aug 20, 2026

@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.

The problem this PR fixes no longer exists on main — which makes this one superseded rather than fixable. @ytahdn

#9551 (fix(cli): give the case-variant delete test the attachments mock, merged as 43aea24 at 06:57 UTC) landed the same fix about five minutes before this PR was opened (07:02 UTC): the case-variant batch-delete test on current main already passes deleteSessionAttachments in its bridge mock, and the TS2741 compile error is gone. That merge is also why this branch now shows as conflicting — both PRs edited the same lines of the same test in session-archive.test.ts.

The only substantive delta versus #9551 is one extra assertion (expect(deleteSessionAttachments).toHaveBeenCalledTimes(1)) plus hoisting the mock into a named const. Reasonable test strengthening, but not enough to keep a conflicting, superseded PR open. Closing in favor of #9551 is the clean path; if that assertion is wanted, a fresh one-line PR rebased on current main would apply cleanly.

中文说明

这个 PR 要解决的问题在 main 上已经不存在了——所以本 PR 属于被取代,而不是需要修改。

#9551fix(cli): give the case-variant delete test the attachments mock,已于 06:57 UTC 以 43aea24 合入)在本 PR 提出(07:02 UTC)前约五分钟已经合入了相同的修复:当前 mainsession-archive.test.ts 中大小写变体批量删除测试的 bridge mock 已经包含 deleteSessionAttachments,TS2741 编译错误已消除。也正是那次合入导致本分支与 main 冲突——两个 PR 修改了同一个测试的同一段代码。

#9551 相比唯一的实质差异是一行额外断言(expect(deleteSessionAttachments).toHaveBeenCalledTimes(1))以及把 mock 提成命名常量。这是合理的测试加强,但不足以支撑一个已冲突、已被取代的 PR 继续存在。建议直接关闭(由 #9551 取代);如果确实想要这条断言,可以在最新 main 上重新提一个单行 PR。

Qwen Code · qwen3.8-max

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants