test(acp-bridge): request response-mode replay in the transport-failure test - #8984
Merged
Conversation
…re test 'transport failure marks the channel dying before process exit' (added in #8947) fails deterministically on merged main: it calls loadSession with historyPageSize but without historyReplay, and since #8933 historyPageSize only takes effect when historyReplay is 'response' — the request silently degrades to stream replay, no sessionTranscript page is ever requested, and the extMethodCalls vi.waitFor times out. This was a logical merge conflict, not a flake: #8947 branched before #8933 landed, so both PRs were green on their own branches and red only once combined. Passing historyReplay: 'response' restores the intended path — the transcript page request hangs, the transport failure rejects it, and the second assertAttachableSessionEntry maps the torn-down session to SessionNotFoundError exactly as the test asserts.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
yiliang114
approved these changes
Aug 12, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — this is the right minimal fix for the main-branch red.
- Verified on main: bridge.ts gates the bounded transcript refresh behind
historyReplay === 'response'(req.historyReplay ?? 'stream'), so #8947's test calling loadSession with only historyPageSize silently degrades to stream replay and its waitFor on the sessionTranscript ext call can never succeed — a genuine logical merge conflict with #8933, not a flake. - Adding
historyReplay: 'response'restores the intended scenario (hanging transcript page request rejected by transport failure → in-memory replay fallback → SessionNotFoundError), matching what the test asserts. - Test-only, +6 lines, no production code touched; author verified 3/3 locally and the full acp-bridge suite green (1277 passed).
Collaborator
|
Thanks for the quick turnaround on this one! Template: the body uses its own headings rather than the PR template's, but everything the template asks for is present — what changed, why, how to verify, evidence, and a full bilingual translation. Passing on substance. Problem: observed, and verified independently. CI run 31565801863 (an unrelated autofix PR) failed Direction: aligned. main is red and breaking unrelated PRs' CI; this is the unblock. Size: not applicable — test-only change, +6/−0 in one file, 0 production lines. Approach: minimal and correct. Passing Risk: no elevated risk signals (test-only change; the high-risk-path check excludes test files). Moving on to code review. 🔍 中文说明感谢快速跟进！ 模板：正文没有使用 PR 模板的小节标题，但模板要求的内容齐全——改了什么、为什么、如何验证、证据、完整中文翻译。按实质内容通过。 问题：已观测到，且已独立核实。CI run 31565801863（一个无关的 autofix PR）的 方向：对齐。main 当前是红的，正在打挂无关 PR 的 CI；此 PR 用于解堵。 规模：不适用——仅测试改动，单文件 +6/−0，生产代码 0 行。 方案：最小且正确。补上 风险：无升级风险信号（仅测试改动；高风险路径检查不计测试文件）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewBefore reading the diff, my own proposal for this root cause was exactly what this PR does: pass No blockers. One non-blocking observation: the silent degradation (a Test evidence (the PR's own CI — per policy I never run PR code myself)Baseline, verified independently: on merged main this test fails deterministically. CI run 31565801863, job CI on this PR's head at review time — the decisive Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The author reports the full Real-scenario testing: N/A — test-only change with nothing user-visible (and this is an unattended CI run). 中文说明代码审查看 diff 之前，我对这个根因的独立方案就是本 PR 的做法：补上 无阻塞项。一个非阻塞观察：静默降级（不传 response 模式时 测试证据（来自本 PR 自己的 CI——按规则我从不自行运行 PR 代码）基线已独立核实：合并后的 main 上该测试确定性失败。CI run 31565801863 的 审查时点头部提交上的 CI：关键检查 真实场景测试：N/A——仅测试改动，无用户可见行为（且本次为无人值守 CI 运行）。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — every claim in this PR checked out independently, and the diff is the minimal correct fix. This is exactly the kind of PR you want after a logical merge conflict: one file, one functional line, no production changes, and a comment making sure nobody trips over the same gating again. I verified the failure myself in CI run 31565801863 (the exact assertion — Approval deferred until CI lands green on 中文说明置信度：5/5 —— PR 中的每项声明都已独立核实，diff 是最小的正确修复。 这正是逻辑合并冲突之后想要的 PR：单文件、一行功能改动、不碰生产代码，并附注释确保没人再被同一个门控绊倒。我在 CI run 31565801863 中亲自核实了失败（确切的断言—— 待 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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. |
qwen-code-ci-bot
approved these changes
Aug 12, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Collaborator
|
Released in v0.21.11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
createAcpSessionBridge > transport failure marks the channel dying before process exit(added in #8947) fails deterministically on merged main — first observed breaking an unrelated PR's CI (#8981,Test (ubuntu-latest, Node 22.x)), and reproducible locally onorigin/main3/3:Root cause: a logical merge conflict, not a flake
962dc8eadc, merged 2026-08-12 01:20) gatedhistoryPageSizebehind response-mode replay: inrestoreSession,historyPageSizenow only takes effect whenhistoryReplay === 'response'; otherwise the request degrades to'stream'and the bounded transcript refresh (refreshedReplayFieldsFor→requestSessionTranscriptPage) is never entered.962dc8eadcis not an ancestor of its heade2c6af25). Its new test callsloadSession({ historyPageSize: 10 })withouthistoryReplay, which triggered the transcript page request under pre-fix(serve): Keep restore request shapes distinct #8933 semantics.sessionTranscriptext call ever happens, and thevi.waitForonextMethodCallstimes out at its 1s default. Instrumented replay confirms the mechanism:historyReplay=stream historyPageSize=undefined, replay contains a singlesession_update, nohistory_truncatedmarker, so the anchor-backfill path does not fire either.Fix
Pass
historyReplay: 'response'in the test'sloadSessioncall, restoring the intended scenario under post-#8933 semantics: the transcript page request hangs against the fake agent, the transport failure rejects it,refreshedReplayFieldsForfalls back to the in-memory replay, and the secondassertAttachableSessionEntrymaps the torn-down session toSessionNotFoundError— exactly what the test asserts. One test-file change, no production code touched.Verification
packages/acp-bridgesuite: 26 files, 1277 passed (previously 1 failed / 1276 passed in CI run 31565801863).中文说明
问题
createAcpSessionBridge > transport failure marks the channel dying before process exit（#8947 新增）在合并后的 main 上确定性失败——最先打红了无关 PR（#8981）的 CI（Test (ubuntu-latest, Node 22.x)），并可在本地origin/main上 3/3 复现：根因：逻辑合并冲突，不是 flake
962dc8eadc，2026-08-12 01:20 合入）为historyPageSize增加了 response 模式门控：restoreSession中historyPageSize仅在historyReplay === 'response'时生效；否则请求降级为'stream'，有界 transcript 刷新（refreshedReplayFieldsFor→requestSessionTranscriptPage）根本不会进入。962dc8eadc不是其 heade2c6af25的祖先）。其新测试调用loadSession({ historyPageSize: 10 })且未传historyReplay，在 fix(serve): Keep restore request shapes distinct #8933 之前的语义下会触发 transcript 分页请求。sessionTranscriptext 调用永不发生，extMethodCalls上的vi.waitFor在默认 1 秒后超时。插桩回放证实了这一机制：historyReplay=stream historyPageSize=undefined，回放仅含一条session_update，无history_truncated标记，锚点回填路径同样不会触发。修复
在测试的
loadSession调用中补上historyReplay: 'response'，在 #8933 之后的语义下恢复测试预期场景：transcript 分页请求挂在 fake agent 上，transport failure 将其拒绝，refreshedReplayFieldsFor回退到内存回放，随后第二个assertAttachableSessionEntry把已拆除的会话映射为SessionNotFoundError——与测试断言完全一致。仅改动一个测试文件，不触碰生产代码。验证
packages/acp-bridge全套：26 个文件，1277 全部通过（CI run 31565801863 中此前为 1 失败 / 1276 通过）。