fix(core): retry leaked protocol turns - #6603
Merged
Merged
Conversation
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. |
wenshao
reviewed
Jul 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/core/src/services/postCompactAttachments.ts:525 |
stripAnalysisBlock is dead code — still exported but has zero importers after this PR migrated all callers |
Remove the function body and export, or add @deprecated JSDoc |
packages/core/src/utils/protocol-tag-sanitizer.ts |
TopLevelProtocolTagStreamFilter lacks direct unit tests — only covered via integration test in turn.test.ts; key scenarios untested: partial tag splits across chunks, non-protocol passthrough, flush() after partial prefix, reset() |
Add a dedicated protocol-tag-sanitizer.test.ts |
packages/core/src/core/geminiChat.ts:3544 |
Missing negative test for !hasToolCall guard — no test verifies tool-call responses with protocol-looking text are NOT sanitized in history |
Add a test with functionCall + protocol text, verify raw tags preserved |
packages/core/src/core/turn.ts:561 |
flush() silently drops all content when model response is only an unclosed `` tag (e.g., MAX_TOKENS truncation) — user sees blank response with no diagnostic |
Emit a debug log when flush produces empty output from non-empty buffer |
packages/core/src/core/geminiChat.ts:3556 |
Persistent history now strips analysis from main model's own responses (behavioral change beyond original subagent-only scope) — may affect multi-turn reasoning quality | Document this intentional change, or consider keeping raw content in history |
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jul 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/core/src/utils/protocol-tag-sanitizer.ts |
No dedicated unit tests for the 198-line sanitizer module. Complex parsing logic (depth tracking, self-closing tags, streaming state machine) is only tested indirectly through integration tests. | Add protocol-tag-sanitizer.test.ts with direct unit tests for each exported function and TopLevelProtocolTagStreamFilter. |
packages/core/src/core/geminiChat.ts:~3685 |
Misleading error message when sanitizer strips to empty. InvalidStreamError('Model stream ended with empty response text.') fires when the model produced content inside protocol tags that was entirely stripped. |
Use a distinct error code/message, e.g., 'Model response contained only protocol scratchpad tags; visible content was empty after stripping.' |
packages/core/src/core/turn.ts:~537 + geminiChat.ts:~3543 |
Stream filter and persistence layer sanitize independently with divergent conditions. The !hasToolCall guard in the persistence layer means tool-call responses with protocol-wrapped text leak tags into history while the TUI shows clean text. |
Extract a shared shouldStripProtocolTags predicate or let the persistence layer inspect already-filtered content events. |
packages/core/src/utils/protocol-tag-sanitizer.ts |
Mid-response protocol tags bypass both sanitizers. Both guards only activate when the response begins with protocol tags. The design assumption is undocumented. | Add a JSDoc comment on TopLevelProtocolTagStreamFilter documenting that only top-level protocol wrappers are stripped. |
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jul 10, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
…ummary-tags-main # Conflicts: # packages/core/src/core/geminiChat.test.ts
Address review suggestions: - Add edge case tests: single-char chunk splitting, adversarial prefixes (<analyze>, <suitor>), whitespace-only flush, reset+reuse, nested analysis-inside-summary-inside-analysis - Add inline comments on bidirectional prefix check in classifyTagStart to prevent future maintainers from mistaking one direction for a typo - Add debug log on first protocol tag suppression in stream processing to aid diagnosing 'model response disappeared' incidents
…-main' into fix/leaked-analysis-summary-tags-main
yiliang114
marked this pull request as ready for review
July 10, 2026 06:53
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: This addresses a real concern — Gemini-family models emit Direction: Aligned. Stripping internal model protocol wrappers from user-visible output and persisted history is a legitimate correctness fix. The approach of filtering at the stream boundary (before TUI/history/recording) rather than at each consumer is the right architectural choice. Size: 680 production logic lines (466 additions + 214 deletions) across 5 core files, plus 695 test lines (excluded from count). The bulk is a new 369-line sanitizer module ( Approach: Scope feels appropriate. The PR consolidates previously duplicated sanitization logic from Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：解决了一个真实存在的问题——Gemini 系列模型会将 方向：一致。在流边界处（TUI/历史/记录之前）而非在各消费端过滤内部模型协议包装器，是正确的架构选择。 规模：680 行生产逻辑（466 行新增 + 214 行删除）分布在 5 个核心文件中，另有 695 行测试代码（不计入）。主要是一个新的 369 行清理器模块和在 方案：范围适当。PR 将 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before reading the diff): Given the problem of protocol tags leaking into visible output, I would have created a shared stripping utility, a streaming filter for the chat path, wired it into Comparison with the PR: The PR's approach matches my independent proposal almost exactly. The streaming filter ( No critical blockers found. The code is well-structured, follows project conventions, and the consolidation of duplicated sanitization logic is a clear improvement. Testing ResultsUnit Tests (worktree, PR code)Total: 442 unit tests passed, 0 failed. Build & TypecheckIntegration TestsInteractive PTY TestThe core assertion ( This is a test-environment issue, not a code correctness issue. The unit tests (216 geminiChat tests, 17 sanitizer tests) thoroughly cover the core protocol tag filtering logic at the stream boundary. Before/After tmuxSkipped — this bug requires specific model behavior (Gemini model emitting — Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis is a well-scoped fix for a real problem. Protocol tags from Gemini-family models leaking into visible terminal output, persisted history, and JSONL recordings is a legitimate correctness issue — users should never see internal model scratchpad text. The implementation matches my independent proposal and goes further in useful ways: the streaming filter handles arbitrary SSE chunk boundaries (which I'd have underestimated the complexity of), the 442 unit tests passing gives me high confidence. The one interactive PTY test failure (4 requests vs expected 3) is an environmental issue — the core assertion (visible summary renders correctly) passes, and the request-count sensitivity to startup API calls is a test-design limitation, not a code bug. Every change in the diff is necessary for the stated goal. No drive-by refactors, no scope creep. The consolidation of duplicated sanitization logic from Approved. ✅ 中文说明这是一个范围合理、针对真实问题的修复。Gemini 系列模型的协议标签泄漏到终端输出、持久化历史和 JSONL 记录中，是一个正当的正确性问题——用户不应看到内部模型草稿文本。 实现方案与我的独立提案一致，并在有用的方面更进一步：流式过滤器处理任意 SSE 分片边界（这比我预想的复杂度更高）， 442 个单元测试全部通过，给了我高度信心。唯一的交互式 PTY 测试失败（4 个请求 vs 预期 3 个）是环境问题——核心断言（可见摘要正确渲染）通过了，请求计数对启动 API 调用的敏感性是测试设计的局限，不是代码缺陷。 diff 中的每一处改动都是实现目标所必需的。没有顺手重构，没有范围蔓延。将 已批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jul 10, 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. ✅
carffuca
reviewed
Jul 10, 2026
carffuca
reviewed
Jul 10, 2026
carffuca
reviewed
Jul 10, 2026
carffuca
previously requested changes
Jul 10, 2026
carffuca
left a comment
carffuca
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for tackling this — the overall direction makes sense, and I like that the sanitization is shared across the main stream path, subagent results, and compaction-related flows. The retry / persistence intent is also good, and the new interactive coverage is valuable.
That said, I do not think this is ready to merge yet. The main blocker is that the new sanitizer semantics are still inconsistent across important cases:
- The streaming filter only recognizes protocol tags at the very beginning of visible output, so later top-level
<summary>wrappers can still leak once the filter has switched to passthrough. - The batch sanitizer now treats literal
<analysis>text inside visible content as a real scratchpad opener, which can truncate legitimate user-visible text and appears to regress an existing caller expectation. - The stream and non-stream sanitizers disagree on nested / literal
<summary>markup inside visible summary content.
So my main recommendation is: treat the sanitizer as the true merge gate here. If the goal is “top-level protocol wrappers are removed, but literal tag-shaped text inside visible payload is preserved”, that contract needs to hold consistently for both streaming and non-streaming paths before this lands.
I left inline comments on the concrete spots I think need attention.
— Claude Fable 5 via Claude Code /review
Collaborator
Author
|
Handled the current sanitizer review round in Fixed:
Verification run locally:
|
Collaborator
Runtime verification reportI built this PR locally and exercised it against a real The core fix works. However, I found two reproducible defects in Verification setup (click to expand)
Results
Persistence was checked independently by reading the chat-recording JSONL after each run: row 1 persists exactly
Finding 1 — an unmatched
|
| # | 模型输出（流式） | BEFORE（main） |
AFTER（本 PR） | |
|---|---|---|---|---|
| 1 | <analysis>SECRET…</analysis><summary>VISIBLE_ANSWER_OK</summary> |
全部泄漏 | VISIBLE_ANSWER_OK |
✅ 已修复 |
| 2 | 纯文本，无标签 | 不变 | 不变 | ✅ 无回归 |
| 3 | 工具调用后跟带标签的文本 | 标签泄漏 | Tool output confirmed. |
✅ 无回归 |
| 4 | <analysis>only scratchpad</analysis>（只有隐藏内容） |
泄漏，exit 0 | 3 次上游请求 → 报错，JSONL 0 条记录 | ✅ 符合设计 |
| 5 | length → 提额重试 → 续写 |
标签泄漏 | 过滤状态保留，标签被剥离 | ✅ 符合设计 |
| 6 | <analysis>ok</analysis><summary>Because 3 < 5, pick the smaller one.</summary> |
全部泄漏 | Because 3 < 5, pick the smaller one.</summary> |
❌ 标签泄漏 |
| 7 | <analysis>3 < 5 so proceed</analysis>Here are your 3 files: … |
正常显示答案 | 空响应 → 3 次请求 → 硬报错 | ❌ 答案丢失 |
持久化行为是单独读取每次运行后的 chat-recording JSONL 验证的：第 1 行只持久化了 "VISIBLE_ANSWER_OK"；第 4、7 行持久化了 0 条 assistant 记录（PR 声称的「重试前不落盘」成立）；第 6 行则把泄漏的 "Because 3 < 5, pick the smaller one.</summary>" 写进了历史。
问题 1 —— summary 正文中出现未配对的 < 会把 </summary> 泄漏到终端和历史里
这正是 PR 所针对的标准协议形态，只不过 summary 正文里恰好有一个普通的 <。闭合标签既渲染给了用户，也被写进了持久化的 chat JSONL，因此恢复会话时它会重新进入模型上下文。
问题 2 —— analysis 正文中出现未配对的 < 会摧毁整个可见答案
过滤器再也没能退出 analysis 模式，于是 </analysis> 之后的所有内容都被吞掉。响应变成空的，而新增的校验逻辑会（正确地）把它判为 NO_RESPONSE_TEXT——于是 CLI 耗尽 INVALID_STREAM_RETRY_CONFIG.maxRetries（3 次上游请求、约 6 秒退避），最后抛出 [API Error: Model stream ended with empty response text.]。而在 main 上用户是能拿到答案的。这是从「标签噪音」退化成了「答案丢失」。
根因（一处代码，两个问题）
packages/core/src/utils/protocol-tag-sanitizer.ts:334-338：当一个 < 未能被识别成协议标签时，第 367 行会设置 literalTag = true，随后 literalTag 分支会一直跳过字符，直到遇到下一个 >：
if (this.literalTag) {
if (this.analysisDepth === 0) out += char;
if (char === '>') this.literalTag = false; // <-- 这个 '>' 其实是协议标签自己的 '>'
continue;
}如果文本里有一个未配对的 <（即在闭合协议标签之前没有对应的 >），那么这个「下一个 >」就是 </summary> 或 </analysis> 的那个 >，协议标签会被当作普通文本吃掉：
- 在
<summary>内部（analysisDepth === 0）→ 闭合标签被当作可见文本输出 → 问题 1 - 在
<analysis>内部（analysisDepth === 1）→</analysis>被吞掉，深度不再归零 → 问题 2
成对的尖括号是安全的（Array<string>、<b>bold</b>），这也是它躲过 review 的原因——protocol-tag-sanitizer.test.ts 里所有含 < 的正文都是配对的 HTML（<details>、<p>）。而未配对的 < 非常常见：3 < 5、if (x<3)、cmd < input.txt、`a < b`。此外问题 2 只在 analysis 块后面没有 <summary> 时才会发作——一旦有 summary，recoveryBuffer 路径会悄悄用批处理 sanitizer 兜底，这是现有测试仍然全绿的第二个原因。
建议的修复
在 literalTag 分支里遇到 < 就重新开始标签识别，两个问题一起解决：
if (this.literalTag) {
+ // '<' 永远开启一个新的标签候选。否则正文里未配对的 '<'（例如 "3 < 5"）
+ // 会让我们一直扫到下一个 '>'，而那个 '>' 属于协议标签本身。
+ if (char === '<') {
+ this.literalTag = false;
+ this.tagCandidate = char;
+ continue;
+ }
if (this.analysisDepth === 0) out += char;
if (char === '>') this.literalTag = false;
continue;
}我在本地打上这个补丁后重跑了全部验证：
- 上表第 6、7 行在真实 CLI 中分别变成
Because 3 < 5, pick the smaller one.和Here are your 3 files: a.txt b.txt c.txt； - 其他各行行为不变；
- PR 改动的 5 个 core 测试套件中的全部 412 个测试依然通过（加上下面 5 个回归测试后是 417 个）。
建议补充的回归测试（在当前 head 上失败，打补丁后通过）：
it('does not leak </summary> when the summary body has an unmatched "<"', () => {
const f = new TopLevelProtocolTagStreamFilter();
expect(f.accept('<analysis>ok</analysis><summary>Because 3 < 5, pick the smaller.</summary>') + f.flush())
.toBe('Because 3 < 5, pick the smaller.');
});
it('does not drop the visible answer when the analysis body has an unmatched "<"', () => {
const f = new TopLevelProtocolTagStreamFilter();
expect(f.accept('<analysis>3 < 5 so proceed</analysis>Here are your 3 files.') + f.flush())
.toBe('Here are your 3 files.');
});问题 3 —— 新增的集成测试在 Linux 上失败（上游请求是 4 次，不是 3 次）
integration-tests/interactive/protocol-tags-interactive.test.ts:103 断言 expect(fakeServer.requests).toHaveLength(3)，但在 Linux 上稳定地得到 4。多出来的那一个是后台的记忆抽取子代理（memory-extraction subagent），它在这一轮结束后向同一个 fake server 发了请求：
RQ0 :: You are Qwen Code, an interactive CLI agent developed by Alibaba Group
RQ1 :: You are Qwen Code, an interactive CLI agent developed by Alibaba Group
RQ2 :: You are Qwen Code, an interactive CLI agent developed by Alibaba Group
RQ3 :: You are now acting as the managed memory extraction subagent for an AI…
用干净的 HOME 也能复现，所以不是本地配置问题——TestRig 直接传 env: process.env，没有隔离 HOME。而且 startFakeOpenAIServer 是按 requestIndex 在所有请求上选响应的，后台的 side-query 还可能把场景映射整体错位，所以这本质上是有竞态的，而不只是差一个数。建议只统计 system prompt 为交互式 agent prompt 的请求（或者在 rig 的 settings 里关掉记忆抽取），而不是断言总数。另外注意这个 PR 上 Integration Tests (CLI, No Sandbox) 这个 job 显示为 skipping，所以 CI 抓不到它。
次要问题
PR 描述里把「17,759 次穷举三段切分检查」列为风险缓解措施。但 diff 和整个仓库里都不存在这样的测试——最接近的只是对一个输入做了一次逐字符切分（protocol-tag-sanitizer.test.ts 的 handles single-character chunk splitting）。建议修正描述，因为正是这句话让「切分边界风险」看起来已经被覆盖了。（我自己对上面每个场景做了穷举的两段切分扫描：过滤器对切分是稳定的——两个问题在任意 chunk 边界都以完全相同的方式复现，因此它们是真正的逻辑 bug，而不是流式竞态。）
结论
流边界的整体方案、重试/持久化的顺序调整、以及续写/提额重试时的状态保持，在运行时测试下都是站得住的。（invalid-stream 分支新增的 suppressNextRetryEvent 从代码上看也是对的——它消除了该分支此前与循环头部重复发出的一次 RETRY 事件。）问题 1 和 2 需要上面那个 5 行修复（或等效方案）加上测试之后再合入——尤其是问题 1，意味着这个 PR 在它所针对的那个形态上并没有完全达成目标。问题 3 建议一并修掉，否则新增的集成测试在 Linux 上没有意义。
…nalysis The streaming protocol-tag filter dropped visible text after a literal `<analysis>` mention inside a visible `<summary>`, e.g. `<summary>Fix: replace <analysis> tag ...</summary>` streamed as `Fix: replace ` while the batch sanitizer correctly kept the literal mention. Distinguishing a paired analysis block (hidden scratchpad) from an unmatched literal opener needs look-ahead, which a per-char stream cannot do inline. Buffer the summary tail once an analysis-shaped token appears inside a visible summary and resolve it through the same batch stripper, so the streaming and non-streaming paths always agree.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
An unmatched '<' in prose (e.g. "3 < 5" or "if (x<3)") put the streaming filter into literal-tag scanning that only ended at the next '>' — which was the '>' of the tag closing the current protocol block. Inside a summary this leaked the raw `</summary>`; inside an analysis block it swallowed the visible answer that followed, so a valid response was discarded and retried instead of surfaced. Treat every '<' as the start of a fresh tag candidate while scanning literal text, keeping the streaming and batch sanitizers aligned.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
yiliang114
changed the title
Collaborator
Author
|
Reworked the implementation in The batch/stream sanitizer has been removed. Main-assistant output that begins with leaked The PR is now 4 files, +362/-8 (down from 11 files, +1676/-224). The previous sanitizer review threads are obsolete because that file and recovery behavior no longer exist. Local verification: 212 GeminiChat tests, focused ESLint, full typecheck, build + bundle, and the three-attempt PTY/fake-SSE regression all pass. |
Collaborator
✅ Local build & real-test verification (maintainer)I built this PR from source and ran the full reviewer test plan plus an end-to-end run of the real Environment: macOS (Darwin 24.6) · Node v22.23.1 · npm 10.9.8 · fresh
Results
Functional verification — the fix itself works ✅Driving the built CLI against the fake OpenAI server through the exact leak scenario (SSE-disconnect mid-
|
| # | 步骤 | 命令 | 结果 |
|---|---|---|---|
| 1 | 核心单元测试 | vitest run src/core/geminiChat.test.ts |
✅ 212 通过（含 2 个新增协议标签用例） |
| 2 | Lint（改动文件） | eslint --max-warnings 0 … |
✅ 0 warning |
| 3 | 类型检查 | npm run typecheck |
✅ 通过（5 个包） |
| 4 | 构建 + bundle | npm run build && npm run bundle |
✅ 通过 |
| 5 | 交互式集成测试 | vitest run interactive/protocol-tags-interactive.test.ts |
功能验证 —— 修复本身工作正常 ✅
用构建出的 CLI 对接 fake server，跑完整的泄漏场景（<analysis> 中途 SSE 断连 → 完整的 <analysis>…</analysis><summary>WRONG…</summary> → 正常回复）：
- 断连的半截尝试不会渲染并触发重试；
- 完整的带协议标签的这轮被丢弃、不入历史、并重试；
- 只有之后的正常回复进入屏幕与历史；
- 真实渲染的 TUI 中没有任何
<analysis>/<summary>/WRONG_FIRST_ATTEMPT/WRONG_COMPLETED_SUMMARY，只有VISIBLE_TMUX_RETRY_RESPONSE_DONE。
⚠️ 为什么 protocol-tags-interactive.test.ts 在本地失败（及修复）
断言 expect(fakeServer.requests).toHaveLength(3) 实际看到 4 个请求。我给请求日志加了埋点：
TOTAL REQUESTS = 4
request[0..2] = 用户这一轮的 3 次相同重放（符合预期）
request[3] last="Managed memory has TWO directories…" ← 并非该轮的重放
body[0]==body[3]? false
request[3] 是托管自动记忆抽取 agent（extractionAgentPlanner.ts）：它在每次 UserQuery 后发起一次后台模型调用，打到了同一个 fake server。该功能默认开启 —— memory.enableManagedAutoMemory 默认为 true。
这不是生产逻辑的 bug（上面已验证功能正确），而是新测试不够 hermetic：集成测试框架既不隔离 $HOME 也不关闭后台记忆 agent，于是多出第 4 个请求。我用干净的 $HOME 复现了它（排除我本地配置的影响）；而且它对时序敏感（后台调用是否在断言前到达取决于机器快慢），因此即使目前某些环境侥幸通过，也是潜在 flaky。
已验证的修复 —— 在测试的 rig.setup settings 中加入以下配置，请求数即降为 3、测试通过：
memory: {
enableManagedAutoMemory: false,
enableManagedAutoDream: false,
},（或者在断言前把记录到的请求过滤成“该轮的 payload”再计数。关闭后台 agent 是更 hermetic、也更贴合测试意图的做法。）
次要说明（不阻塞）
检测会有意地把开头的 <summary> / <analysis> 视为协议泄漏，因此若某轮主回复合法地以 HTML <summary> 开头，会被丢弃并重试。新增测试正确地覆盖了 <details><summary>…（非开头）与正文中的标签提及会被保留，属于 PR 声明的风险范围之内，仅提示知悉。
结论
核心改动 LGTM，已验证可用。建议合并前按上面的一行修复把新增交互式测试改成 hermetic；生产代码无需改动。
Collaborator
Author
Updated tmux runtime verification (current head
|
| Scenario | Mock response sequence | Requests | TUI / durable history |
|---|---|---|---|
| S1: complete leaked turn | complete tagged response -> normal response | 2 | Only VISIBLE_AFTER_WHOLE_TURN_RETRY was rendered and persisted. |
| S2: transport failure plus leaked turn | tagged partial SSE + disconnect -> complete tagged response -> normal response | 3 | Only VISIBLE_AFTER_TWO_RETRIES was rendered and persisted. |
| S3: retry exhaustion | three complete tagged responses | 3 exactly | TUI showed the bounded API error; no assistant turn was persisted and no fourth request occurred. |
| S4: false-positive guard | valid <details><summary>... plus a literal <analysis> mention |
1 | The response was preserved verbatim with no retry. |
The runtime JSONL assistant records were exactly:
[{"text":"VISIBLE_AFTER_WHOLE_TURN_RETRY"}]
[{"text":"VISIBLE_AFTER_TWO_RETRIES"}]
// S3: no assistant record
[{"text":"<details><summary>VALID_HTML_TITLE</summary></details> Use the literal <analysis> tag in prose."}]
Actual tmux captures
S1 server
MAIN#1 sha=b2cc838d275948ab -> TAGGED_COMPLETE_DISCARD
MAIN#2 sha=b2cc838d275948ab -> NORMAL_SUCCESS
S1 TUI
◆ VISIBLE_AFTER_WHOLE_TURN_RETRY
S2 server
MAIN#1 sha=b2cc838d275948ab -> TAGGED_PARTIAL_DISCONNECT
MAIN#1 socket destroyed after content chunk 1
MAIN#2 sha=b2cc838d275948ab -> TAGGED_COMPLETE_DISCARD
MAIN#3 sha=b2cc838d275948ab -> NORMAL_SUCCESS
S2 TUI
◆ VISIBLE_AFTER_TWO_RETRIES
S3 server
MAIN#1 sha=b2cc838d275948ab -> TAGGED_FAILURE_1
MAIN#2 sha=b2cc838d275948ab -> TAGGED_FAILURE_2
MAIN#3 sha=b2cc838d275948ab -> TAGGED_FAILURE_3
S3 TUI
✕ [API Error: Model response started with leaked protocol tags.]
S4 server
MAIN#1 sha=b2cc838d275948ab -> VALID_LITERAL_RESPONSE
S4 TUI
◆ <details><summary>VALID_HTML_TITLE</summary></details> Use the literal <analysis> tag in prose.
S1: discard the whole tagged turn, then replay
S2: retry a disconnected SSE attempt and a completed tagged attempt
S3: stop after the existing three-attempt budget
S4: preserve valid HTML and later literal tag names
Automated-test isolation follow-up
The existing interactive regression initially allowed the default managed-memory extraction agent to send an unrelated fourth request to the same fake server. Commit eb89c035d disables managed auto-memory and auto-dream only inside this test. The focused interactive test then passed 3/3 consecutive runs with the expected three-request sequence. Focused ESLint and full typecheck also pass.
Intentional boundary
The guard applies only to a main-assistant pure-text response whose first non-whitespace output is <analysis> or <summary>. Tool-call turns are not replayed, avoiding duplicate side effects. Compaction/subagent behavior remains separate and unchanged.
wenshao
approved these changes
Jul 10, 2026
qwen-code-ci-bot
approved these changes
Jul 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
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
4 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.
What this PR does
This PR treats a main-assistant response that starts with leaked
<analysis>or<summary>protocol tags as an invalid turn. The entire pure-text attempt is suppressed, excluded from chat-recording JSONL and in-memory history, and retried through the existing bounded invalid-stream retry path. A retry is successful only when the model returns a normal response without the protocol wrapper.The detector buffers only the leading tag prefix so it works across arbitrary SSE chunk boundaries without parsing or recovering the protocol payload. Once ordinary response text is identified, streaming continues unchanged. Responses that contain tool calls are not retried, avoiding duplicate side effects.
Why it is needed
The content inside a leaked
<summary>is compaction protocol output, not the requested task result. Unwrapping that summary and presenting it as a successful assistant response stops the task before the expected follow-up tool calls and contaminates later context. The correct recovery is to discard the abnormal turn and replay the original request from clean history.Reviewer Test Plan
How to verify
Verify that a split
<analysis>response followed by an SSE disconnect retries without rendering the partial response, a completed<analysis>...<summary>...</summary>response is discarded and retried without persistence, and only a later ordinary response reaches the terminal and history. Also verify that normal HTML such as<details><summary>...</summary></details>and protocol-tag names appearing later in prose are not rejected.Commands run locally:
Evidence (Before & After)
Before: the leaked summary payload could be stripped from its wrapper and treated as the final user-visible answer.
After: the complete tagged turn is rejected, never persisted, and retried; only an ordinary successful response is emitted.
Tested on
Risk & Scope
Linked Issues
Closes #6595
Related to #6023 and #6027.