fix(core): keep Responses reasoning replay data off foreign wires - #11567
Conversation
`Part.thoughtSignature` doubles as the cross-provider store for reasoning replay data but carries no origin marker, so each wire interprets the same opaque string in its own way. Since #8169 landed, `responses-converter.ts` writes `JSON.stringify({ id, encrypted_content })` into that field for every Responses reasoning item. After a provider switch the Anthropic converter forwards any string-valued `thoughtSignature` as a native `thinking.signature`, and the Gemini wire passes it through untouched, so a Responses replay payload goes out as if it were another provider's native signature. Guard both request-build points with a shared recognizer for that payload shape, `isResponsesReasoningSignature` in `thoughtUtils.ts`. The Anthropic converter drops the payload and leaves the thinking block unsigned; the Gemini wire deletes it from the shallow copy that `stripPartFields` already builds, so the caller's history keeps the payload and a later switch back to Responses can still replay it. Both keep the visible reasoning text. This completes the fallback `responses-converter.ts` already applies in the other direction -- drop the unreplayable payload, preserve the human-readable summary, log the drop -- rather than adding a new heuristic. A native Anthropic or Gemini signature is an opaque token that never takes this JSON shape, so legitimate same-wire round-trips stay lossless; the tests assert both sides on both wires. Not addressed here: treating *unknown* metadata as incompatible by default. With no origin marker on the field, "unknown" can only be decided by guessing at string shapes, and guessing wrong drops legitimate native signatures. That needs the provenance contract discussed in #8533. Tests: packages/core converter.test.ts, llm-content-generator.test.ts and thoughtUtils.test.ts pass 158/158. Both new leak assertions fail before the guard with `expected '{"id":"rs_68c6c0c9ff5c8191a29b2e78c1a…' to be undefined`. Adjacent wire suites (responses-converter, anthropic generator, openai converter) pass 499/499. Fixes #9453 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-issue-patrol/jmtvd2hbyvd
|
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. #9453 is a real user report (filed by @netbrah, Direction: aligned. A shared Size: all five files sit under Approach: the scope feels right, and it matches what I'd have written independently before reading the diff — a shape recognizer in a leaf util, applied at the two request-build points, keeping the visible text and stripping wire-only so history stays replayable on a switch back. No drive-by refactors, no unrelated churn, and the four out-of-scope items are named explicitly rather than quietly dropped. One question worth thinking about, not a blocker: the new Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需小标题都在，包括 Reviewer Test Plan 和中文翻译。 问题： 是已观测到的问题，不是理论性加固。#9453 是真实用户报告（由 @netbrah 提出，带 方向： 对齐。共享的 规模： 五个文件全部位于 方案： 范围合理，而且与我读 diff 之前独立想到的写法一致 —— 在叶子 util 里放一个形状识别函数，在两个请求构建点应用，保留可见文本，并且只剥离发往线路的拷贝，这样切回原提供商时历史仍可重放。没有顺手重构，没有无关改动，四项「不在范围内」也明确列出而不是悄悄丢掉。有一个值得考虑的问题（不是阻塞项）：新增的 风险： 无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewNo critical blockers. The change does what it says, and I checked the load-bearing claims against
One thing worth a second look (non-blocking). The Anthropic half leaves the thinking block unsigned, and there is already machinery downstream that keys off exactly that.
My read is that this is coherent integration rather than a defect — that pass is documented for exactly "cross-provider history where non-Anthropic generators only set Two smaller notes: the new helper is a verbatim logic duplicate of the private Test evidenceThis is an unattended CI run, so per the gate rules I did not build or execute any PR-derived code. Everything below is this PR's own CI, read through the API for the reviewed commit
So the fix here is to merge or rebase
Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 (Bot orchestration jobs — Sandboxed verification would settle this: 中文说明代码审查没有发现阻塞性问题。改动与描述一致，而且我是对照
有一处值得再看一眼（非阻塞）。 Anthropic 这一半让 thinking 块保持无签名，而下游已有专门针对这一点的机制。
我的判断是这属于合理的机制复用，而不是缺陷 —— 那段逻辑的文档说明它正是为「非 Anthropic 生成器只设置 两个较小的点：新增的 helper 与 测试证据这是无人值守的 CI 运行，因此按门禁规则我没有构建或执行任何来自 PR 的代码。下面全部内容都是通过 API 读取的、针对被审查提交
这也意味着需要明确说明描述中「门禁」那一节的性质：其中引用的 eslint/prettier/
（表格中的 CI 结论由上方英文表格给出，此处不重复；机器人编排 job 未计入。） 沙箱验证可以把这件事定死： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the change itself is right and I'd merge it, but a required check is red on a staleness gate, the unit suite hasn't reported on this commit, and one downstream interaction is untested. Going back to what I'd have written before opening the diff: this is essentially it. A shape recognizer in a leaf util, applied at the two points that would forward a foreign payload, wire-only on the Gemini side so history stays replayable. I did not find a simpler path that I think the PR missed, and I looked for one — the obvious "just add an origin marker to the field" answer is the right long-term shape and is exactly what #8533 is for, so pulling it into a bugfix would have been the wrong call here. It solves something real. #9453 came out of a dogfood session that actually failed after a provider switch, and since #8169 merged the payload is being written to real history rather than only proposed, so this stopped being hypothetical. The 67 production lines are all in service of the stated goal — no drive-by refactors, no formatting churn, and the four things deliberately left out are named in the description instead of quietly skipped. That's a PR I can review in one pass, which is the point. In six months I'd thank you for the comments at both guards — they say why the field is ambiguous and link the issue, so nobody has to re-derive this. The one thing that would annoy me is the third copy of the same predicate; you've flagged the dedup as a follow-up and I agree with leaving it out of this diff, but it should actually happen, because three private copies of one shape check is how the next person adds a fourth. Where I landed short of approving:
None of that is a reason to request changes, so I haven't: the design is sound and the only hard blocker is mechanical. I'm also not posting an approval this run, and deliberately not leaving a deferred-approval instruction behind — with a required check already red, CI cannot go green on this commit, and the rebase that fixes it moves the head the approval would be pinned to. A standing approve that can only ever be withheld is worse than none. I'm not escalating to another maintainer or reassigning either. The open question above is answerable by you, and you have admin on the repo, so a second name in the thread would be noise rather than signal. Next step: rebase or merge 中文说明Confidence: 3/5 —— 改动本身是对的，我愿意合入；但有一个必需检查因门禁过期而变红，单元测试尚未在该提交上出结果，另外有一处下游交互没有被测试覆盖。 回到我在看 diff 之前会写的方案：本 PR 基本就是那个方案。在叶子 util 里放一个形状识别函数，在两个会把外来负载转发出去的点应用它，Gemini 侧只作用于线路拷贝以保留历史的可重放性。我没有找到更简单而本 PR 遗漏的路径，而且我确实找过 —— 最明显的「给字段加一个来源标记」是正确的长期形态，也正是 #8533 要做的事，所以把它拉进一个 bugfix 里反而是错的。 它解决的是真实问题。#9453 来自一次在切换提供商后确实失败的 dogfood 会话；而且自 #8169 合并后，该负载是在写入真实历史，而不只是提案，所以这件事已经不再是假设性的。67 行生产代码全部服务于既定目标 —— 没有顺手重构，没有格式化噪音，刻意排除的四项也在描述里点名，而不是悄悄跳过。这样的 PR 一遍就能审完，这正是关键。 半年后回看，我会感谢你在两个守卫处写的注释 —— 它们说明了字段为何存在歧义并链接了 issue，因此没人需要重新推导一遍。唯一会让我不舒服的是同一个判定出现了第三份拷贝；你已经把去重列为后续工作，我也同意不要放进这个 diff，但它应该真的被做掉 —— 同一个形状判定存在三份私有拷贝，正是下一个人加出第四份的原因。 我没有直接批准的原因：
以上都不构成 request changes 的理由，所以我没有这么做：设计是站得住的，唯一的硬阻塞是机械性的。本次运行我也不发批准，并且刻意不留下延迟批准的指令 —— 既然已有必需检查变红，CI 不可能在该提交上转绿，而修复它的 rebase 会移动批准所要绑定的 head。一个只能被撤回的常设批准，比不发更糟。 我也不会升级给另一位 maintainer 或重新指派。上面那个未决问题你自己就能回答，而且你在本仓库有 admin 权限，因此在线程里再加一个名字只会是噪音而非信号。 下一步：rebase 或 merge — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at c7087ee. The direction is right and the scope is narrow. I verified the Gemini side really is wire-only: stripPartFields copies at thoughtUtils.ts:354 before the delete at :378, so history stays replayable. One blocking issue.
[Critical] packages/core/src/core/anthropicContentGenerator/converter.ts:623 — dropping the payload but keeping the thinking block produces a block that this same converter classifies as unsigned, and one of its own passes throws on that shape.
converter.ts:1178-1184—isUnsignedThinkingistype === 'thinking' && (typeof value.signature !== 'string' || value.signature.length === 0).converter.ts:1225-1231— when such a block sits in a turn belonging to the unbroken activetool_use/tool_resultchain at the end of history, it throws'Anthropic-compatible proxy omitted the thinking signature for a tool-use turn that is still in progress…'.- That pass is enabled at
anthropicContentGenerator.ts:793-797:dropUnsignedAssistantThinking = !isDeepSeek && !!thinking && this.modelSupportsAdaptiveThinking() && !isAnthropicNativeBaseUrl(...)— proxy-hosted Claude 4.6+/5.x with thinking on. That is exactly the configuration where the pre-PR request succeeded, because the payload was assigned unconditionally assignatureand the block therefore counted as signed.
So on the lax-proxy path this turns a silent foreign-blob leak into a hard request failure whose message blames the proxy for something the client just did.
Secondary effect on the same pass: on a turn that is not in the active chain the unsigned block is filtered at converter.ts:1234, and if it was the turn's only block the whole assistant message is dropped at :1237 — so "keeps the visible reasoning text" does not hold there either.
Suggest mirroring what responses-converter.ts:544-550 does in the other direction — downgrade or drop the block rather than un-sign it. Worth noting converter.ts:1146-1147 already states that filling signature: '' is what "keeps the wire shape spec-compliant", so an unsigned thinking block is a shape this codebase treats as non-compliant.
[Suggestion] converter.test.ts:4466,4485 — both new Anthropic cases pass only { enableCacheControl: false }, so dropUnsignedAssistantThinking, normalizeAssistantThinkingSignature and injectThinkingOnToolUseTurns are all false and the interaction above is untested. A case with dropUnsignedAssistantThinking: true would pin it.
[Nit] thoughtUtils.ts:56 — third copy of this JSON shape check; llm-chat.ts:1178-1193 is equivalent minus the falsy guard. Fine to defer, flagging only because the new function is the designated shared one.
Checked and clean: both Gemini send paths route through stripUnsupportedFields (llm-content-generator.ts:281,297); openaiContentGenerator/** never reads thoughtSignature; loggingContentGenerator.ts:1165 is logging-only; the recognizer matches the producer exactly, since responses-converter.ts:299-318 returns null when encrypted_content is absent, so every emitted payload carries both keys — no false negatives.
CI note: Lint & Static is red purely on a lint-gate freshness check after ci.yml moved on main, so a rebase rather than a code change. I did not run the suite locally.
…wire-signature Refresh the branch onto main so the Lint & Static lane passes the `Check lint gate freshness` gate (main changed .github/workflows/ci.yml in 17990c3). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtvkxeirvp
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 duplicate Responses-payload shape check at thoughtUtils.ts:56 — already reported (review 5166618982, @doudouOUC)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more。
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
An unsigned `thinking` block is exactly the shape the downstream `dropUnsignedThinkingFromAssistantMessages` pass treats as a proxy protocol violation, so a foreign Responses reasoning replay payload that gets dropped must not leave an unsigned thinking block on the wire. Demote it to a plain text block (keeping the summary when present) instead, mirroring responses-converter.ts's fallback for unreplayable signatures. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtvoi040vv
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- T2-3 duplicate Responses-payload shape check at thoughtUtils.ts:51 — already reported (review 5166618982, @doudouOUC)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/thoughtUtils.ts:56 — [probe] No test for the new shared predicate at its own home
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan（非阻断）：src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.2)
…nking Demoting a foreign Responses reasoning replay payload to a visible text block leaked hidden reasoning as assistant prose under stripAssistantThinking (DeepSeek + thinking disabled), because stripThinkingFromAssistantMessages only removes thinking blocks. Thread dropUnsignedAssistantThinking into processContents/processContent and take the text-demotion branch only when it is set; otherwise emit the thinking block unsigned so strip/normalize passes handle it instead of ever attaching the foreign payload as a native signature. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtvz7syqwc
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short by the tool ceiling.; "agent 6c": could not confirm the Anthropic SDK's ThinkingBlockParam type (no @anthropic-ai/sdk .d.ts declaring it is installed in this worktree), so the "signature i….
Test Plan (not a blocker): src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more.
Convergence: round 3 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/core/anthropicContentGenerator/converter.test.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short by the tool ceiling.；"agent 6c"：could not confirm the Anthropic SDK's ThinkingBlockParam type (no @anthropic-ai/sdk .d.ts declaring it is installed in this worktree), so the "signature i…。
Test Plan（非阻断）：src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more。
收敛情况：第 3 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/core/src/core/anthropicContentGenerator/converter.test.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.3)
Local verification at
|
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 102 passed · 0 failed · 102 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：102 通过 · 0 失败 · 102 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11567 — deep verification reportVerdict: 中文摘要
Central claim and A/BCentral claim. After a provider switch, an OpenAI Responses reasoning-replay payload ( Oracles. Anthropic: the Arms.
No-regression arm (identical on all three arms, byte-for-byte): Why the third arm matters. Corrections to the PR descriptionThese are statements about the description, not requests to change code.
FindingsF1 (Suggestion) — non-string
|
| input | base | head |
|---|---|---|
thoughtSignature: 1 |
forwarded to the socket, request proceeds | THREW signature.startsWith is not a function, request never reaches the peer |
thoughtSignature: true |
forwarded | THREW |
thoughtSignature: {id, encrypted_content} |
forwarded | THREW |
thoughtSignature: ['{"id":…}'] |
forwarded | THREW |
4 of 8 hostile runtime types throw on head; 0 of 8 on base (base has no recognizer). The Anthropic path is identical on all arms (the typeof guard absorbs the value), which is what makes the asymmetry visible.
Reachability, bounded. No in-app writer emits a non-string except one: anthropicContentGenerator.ts:1426-1430 does const signature = (event.delta as { signature?: string }).signature || ''; and passes signature (not blockState.signature, which is stringified by +=) into the emitted chunk — so a non-conforming proxy returning a numeric/boolean signature in signature_delta puts a non-string into history, exactly the cross-provider scenario this PR addresses. The other route is corrupted or hand-edited persisted session history (session restore performs no Part shape validation; sessionService.corruption.test.ts exists precisely because these files are not trusted). A conforming provider cannot produce this. So: not a security issue, not reachable in the happy path — but it is a crash regression introduced by this diff on a request path, where base merely forwarded the garbage.
Minimal suggested fix (measured, not eyeballed)
Make the recognizer honest about its input — it already is the trust boundary for both call sites:
export function isResponsesReasoningSignature(
- signature: string | undefined,
+ signature: unknown,
): boolean {
- if (!signature || !signature.startsWith('{')) return false;
+ if (typeof signature !== 'string') return false;
+ if (!signature.trimStart().startsWith('{')) return false;(trimStart() also closes F2; it keeps the cheap fast-path that avoids JSON.parse on every native base64 signature.) Measured in tmp/fix-tree, same harness, same 27 cells:
- hostile fixtures go clean: 4/8 throwing inputs → 0/8; all four
H-gemini-nonstring-*cells go fromTHREWto "request reached the peer" (base-equivalent behaviour). - benign fixtures byte-identical: 19 of 27 cells identical head↔fix, including all 13 central-claim and native-signature cells (
A1–A8,G1–G5); exactly the 8 targeted cells changed. - suite unchanged: 161 passed / 0 failed on fix-tree vs 161 / 0 on head;
tsc --noEmitin the main tree with the patch applied = 0 errors (same tree unpatched = 0). - divergence vs
decodeReasoningSignature: 2 → 0.
Because the suite is green both with and without the patch, it pins nothing along this axis: the fixtures that would go red are a llm-content-generator.test.ts case with thoughtSignature: 1 as unknown as string asserting the request still reaches the peer, and a thoughtUtils.test.ts describe for the recognizer over the boundary set below. The fix should ship with them.
F2 (Suggestion) — the guard and the Responses-side decoder disagree on leading whitespace, so a replayable payload still reaches both foreign wires
decodeReasoningSignature (responses-converter.ts:105-121) has no startsWith('{') precondition; JSON.parse tolerates leading whitespace. The new recognizer requires the value to begin with {. Measured over 21 boundary inputs, 2 disagree (foreignLeadingSpace, foreignLeadingNewline), and the disagreement has a real wire consequence on head:
| input | head Anthropic | head Gemini | fix |
|---|---|---|---|
' ' + payload |
FORWARDED verbatim as thinking.signature |
FORWARDED verbatim as thoughtSignature |
stripped |
'\n' + payload |
FORWARDED verbatim |
FORWARDED verbatim |
stripped |
payload + ' ' |
stripped | stripped | stripped (trailing whitespace was always caught) |
Reproducing command: node --import tsx …/probe.mjs --tree <tree> --arm head --observe, cells W-*; captured in 03-recognizer-boundaries-head-vs-fix.png.
Reachability, bounded. The only producer is encodeReasoningSignature = JSON.stringify, which never emits leading whitespace; llm-chat.ts's episode concatenation only ever appends to an existing string. So this is unreachable from the app's own output today and becomes reachable only through hand-edited or migrated persisted sessions (#9452's territory). The reason to care is consistency, not exploitability: the value the Responses side would happily replay is precisely the value the foreign-wire guard lets through, which is the opposite of the invariant the PR states it establishes. The same one-line trimStart() in F1 closes it (measured above: divergence 2 → 0).
F3 (Suggestion, coverage gap from the mutation matrix) — typeof payload.id === 'string' is load-bearing for the PR's own false-positive argument and is pinned by no test
Mutant M8 deletes that clause; the unmutated control is 161/0 and M8 is 161/0 — it survives. It is not dead code (it decides an outcome: {"id":123,"encrypted_content":"g"} is false at head, true under M8) and not redundant defence (no sibling hunk closes the same hazard). It is an ordinary coverage gap. It matters because the PR body's false-positive argument leans on exactly this clause: "the recognizer requires both keys to be strings, so this is theoretical rather than observed". The over-broad direction is well pinned — mutant M4 (recognizer always true) turns 12 tests red, including both native-preservation tests and 10 pre-existing ones — so the risk the author cares about is largely covered; only this one clause is not. Fixture that would pin it: expect(isResponsesReasoningSignature('{"id":123,"encrypted_content":"g"}')).toBe(false).
Mutation matrix (vacuity of the PR's new tests)
Run in tmp/mut-tree (worktree at head), three test files, restored with git checkout between mutants; captured as 02-mutation-matrix-8-mutants-2-controls.png. Both positive controls are landed in the same files as the mutants they validate.
| mutant | result | classification |
|---|---|---|
| control, unmutated | 161 passed / 0 failed | suite green |
| M1 Anthropic guard off | 3 red (both leak tests + the signature-only test) | killed |
| M2 Gemini guard off | 1 red (the Gemini leak test) | killed |
M3 demote gate off (== a1f7f68f) |
1 red ("does not throw on an active tool-use turn") | killed — the third commit is pinned by a test |
| M4 recognizer always true | 12 red (2 native-preservation + 10 pre-existing) | killed |
| M5 recognizer always false | 4 red (3 Anthropic + 1 Gemini) | killed |
| M6 Gemini strip deletes from the caller part | 1 red ("does not mutate the caller-owned history part") | killed — that test is non-vacuous |
| M7 demote path drops the summary text | 1 red (the tool-use test asserts the demoted text) | killed |
M8 id string check removed |
0 red | survivor → coverage gap (F3) |
| PC1 strip pass no-op (converter.ts) | 4 red | control live in the mutated file |
| PC2 displayName strip no-op (llm-content-generator.ts) | 2 red | control live in the mutated file |
No combination row was needed: M1/M2 close different hazards on different wires, and M3 is the single-hunk row that already shows the layered pair (recognizer + gate) is load-bearing — reverting the recognizer alone (M1/M5) leaks, reverting the gate alone (M3) throws.
Reviewer Test Plan, walked step by step
- "Both new leak tests failing before the guard" — reproduced equivalently: M1 (Anthropic guard off) turns exactly the named test red, M2 turns the named Gemini test red. Holds.
- "After the guard: 158 passed (158)" — green, but the count is stale: 161 (119 + 27 + 15). Holds modulo the count (correction 4).
- "Adjacent wire regression: 499 passed" — green at 502 (93 + 175 + 234). Holds modulo the count.
- "
npm run typecheckin packages/core passes with zero errors" — measured:tsc --noEmitexit 0, 0 errors. Holds. - "
prettier --checkandeslintpass on all five changed files" — not run by this round (listed under Not covered). - "A third test asserts the Gemini strip does not mutate the caller-owned history part, holding it by object identity" — M6 proves the assertion can fail and fails for the right reason. Holds.
- "Wire coverage is complete with these two guards: the OpenAI Chat converter never reads
thoughtSignature" — census overpackages/*/src(non-test): the only request-build readers areconverter.ts:618-650(guarded) andllm-content-generator.ts:377(guarded);openaiContentGeneratorhas zero reads;sessionRecap/sessionTitlefilter any part carryingthoughtSignatureout;loggingContentGeneratoris a decorator over the guarded generators;responses-converterhandles its own direction.convertLlmRequestToAnthropichas exactly one non-test caller and both Gemini entry points funnel throughstripPartFields. Holds.
Targeted gates
packages/corevitest, 6 affected files (converter,llm-content-generator,thoughtUtils,responses-converter,anthropicContentGenerator,openaiContentGenerator/converter): 663 passed (663), 6 files, exit 0 (logs/gate-head-vitest.log).packages/corenpm run typecheck: 0 errors (logs/typecheck-head.log).- Gate liveness: the mutation matrix's PC1/PC2 rows are the planted violations — the same command that reports 663/0 turns red on a one-line break in each mutated file.
Not covered
- Per-commit attribution. The checkout is shallow (
git rev-parse --is-shallow-repository= true);git rev-list HEAD^1..HEAD^2returns 1 while the snapshot lists 4 commits. All measurements are against the aggregateHEAD^1..HEADdiff; the intermediate arm approximates commita1f7f68f's state by reverting one hunk, which is an approximation, not that commit's tree. - No live multi-provider E2E capture (needs two provider credentials plus a mid-session switch) — the same boundary the issue itself declares. The Gemini oracle is a real socket but a synthetic peer; this reproduces the wire shape the issue reports, not a live provider-switch session.
- The Anthropic→foreign direction and the "unknown by default" half are unchanged by this PR and remain deferred to Foundational problem: Content[]/Part[] cannot safely encode per-provider reasoning-replay contracts #8533. Verified as a scope statement, not as a defect: the recognizer returns
falsefor both native signature shapes (measured), so an Anthropic signature still travels on the Gemini wire exactly as before. prettier --check/eslinton the changed files were not run (the PR's own CI covers them).- Full-repo test suite not run; only the 6 affected files.
- No scaling ladder was run: the changed code adds no regex or scanner over untrusted text — one
JSON.parseper part, gated behind astartsWith('{')fast path that native base64 signatures never enter, so the added cost per request is bounded by the number of thought parts and is linear. - Worktree typechecks are environmental, proven by A/A: any worktree under
tmp/lackspackages/core/node_modulesand reports 63tscerrors (@opentelemetry/*,@lydell/node-pty); the pristine unpatched base worktree reports the identical 63 (logs/typecheck-aa-control.log), none in a file this PR touches, so the patch was typechecked in the main tree instead (0 errors, then restored — finalgit statusempty). countTokenshas no Gemini path inLlmContentGenerator(no such method), so there is no third Gemini request-build point to guard.
Methodology
Environment: CI verify container at refs/pull/11567/merge (depth 2); HEAD^1 = base, HEAD^2 = head; the snapshot's baseRefOid (1961e974…) had drifted and was not used. Four trees were used, all nested under the repo so third-party deps resolve identically from the root node_modules: the head checkout, tmp/base-tree (HEAD^1), tmp/mid-tree (head with the demote gate reverted), tmp/mut-tree / tmp/fix-tree (head, mutated/patched per run, restored via git checkout after each). Each harness arm loaded the units under test from its own tree through tsx, with a node:module registerHooks resolve-trace asserting zero modules from any foreign tree; the Gemini oracle is a real node:http loopback server receiving the real SDK's request bodies (SSE for the stream endpoint). Assertions live in harness/assert.mjs and read only recorded evidence (logs/probe-*.json, logs/mut-*.json, gate logs); raw per-arm output is in logs/observe-*.txt, the matrix in logs/mutation-matrix.txt. Scratch worktrees are removed below.
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/core/src/core/anthropicContentGenerator/converter.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/anthropicContentGenerator/converter.test.ts
file packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/llm-content-generator/llm-content-generator.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/core/anthropicContentGenerator/converter.test.ts: PPPPP
packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/core/anthropicContentGenerator/converter.test.ts: P (exit 0)
round 1 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 2 · packages/core/src/core/anthropicContentGenerator/converter.test.ts: P (exit 0)
round 2 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 3 · packages/core/src/core/anthropicContentGenerator/converter.test.ts: P (exit 0)
round 3 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 4 · packages/core/src/core/anthropicContentGenerator/converter.test.ts: P (exit 0)
round 4 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 5 · packages/core/src/core/anthropicContentGenerator/converter.test.ts: P (exit 0)
round 5 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…n tests
- Guard isResponsesReasoningSignature against non-string input so a Gemini
signature_delta number/boolean no longer throws startsWith and crashes.
- Tolerate leading whitespace in the '{' pre-check (matching JSON.parse) so
whitespace-prefixed replay payloads are still recognized and kept off wires.
- Pin the Anthropic demote/tool-use blocks with exact toEqual and add a
non-empty summary demote assertion to distinguish demote from drop.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtwe7xfux1
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- converter.ts:642 manual-thinking active tool-loop shape — already reported (@wenshao, comment 5628170560, non-blocking observation 2)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more.
Convergence: round 4 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan（非阻断）：src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more。
收敛情况：第 4 轮发布了 3 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 1 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.3)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtwknetexb
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 3b": could not confirm api.anthropic.com's distinct handling of a missing vs. an invalid thinking.signature — node_modules/@anthropic-ai/sdk type definitions wer….
Test Plan (not a blocker): src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/core/anthropicContentGenerator/converter.test.ts:4434 — [probe] No test couples the recognizer to the producer's encoderpackages/core/src/core/anthropicContentGenerator/converter.test.ts:4652 — [probe] DeepSeek thinking-on option pair never exercised with a…
中文说明
无阻断问题。LGTM！✅
未探索到全部深度（达到工具调用预算）："agent 3b"：could not confirm api.anthropic.com's distinct handling of a missing vs. an invalid thinking.signature — node_modules/@anthropic-ai/sdk type definitions wer…。
Test Plan（非阻断）：src/core/anthropicContentGenerator/converter.test.ts — no such file or directory; src/core/llm-content-generator/llm-content-generator.test.ts — no such file or directory; src/utils/thoughtUtils.test.ts — no such file or directory; src/core/openaiResponsesContentGenerator/responses-converter.test.ts — no such file or directory; src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts — no such file or directory; and 4 more。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.3)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Agent-assisted review at 11b347100ce92d653931da3382992ca6e335a268 — no confirmed Critical in the full current-diff static review.
Previous Criticals rechecked against source (not thread flags):
- Our review 5166618982 / first-round R1-1, foreign replay becoming unsigned thinking and throwing on the active proxy tool chain: fixed.
packages/core/src/core/anthropicContentGenerator/converter.ts:268-272,629-655now forwards the realdropUnsignedAssistantThinkingdecision and demotes recognized foreign summaries before the unsigned-block pass at:310-312. The production producer isanthropicContentGenerator.ts:793-797,829-845; this is not an unpopulated switch. - Second-round R1-1 at the old demotion anchor, hidden reasoning escaping DeepSeek's
stripAssistantThinking: fixed for that production path. Demotion is conditional, and DeepSeek's mutually exclusive option set leaves a thinking block for the stripping pass (converter.ts:274-280,1142-1157). The regression fixture also checks that only the visible answer survives.
Coverage: all six changed files and all new test cases; Responses encoder/decoder and signature-only emission, history episode consolidation, both Gemini send paths and nested-part traversal, Anthropic native/proxy/DeepSeek option production and post-processing. The recognizer matches the actual {id, encrypted_content} producer. Gemini removes the field from a copied Part (llm-content-generator.ts:354,377-378), preserving history for a later Responses replay. Chat Completions does not consume thoughtSignature. The new unknown-input guard and the actual later-model-turn test fixture address the subsequent test/robustness concerns.
Boundaries: this isolates the recognized Responses payload; it is not a general signature-authenticity check or a promise that every cross-provider history is accepted by every server. Native/older-Claude paths can still produce unsigned thinking, while manual-thinking proxies have their existing leading-thinking constraints. I did not establish a new regression on those paths and am not elevating the already-discussed limitations. Existing producer-coupling/DeepSeek test and deduplication suggestions remain deferred after the repeated review rounds; no new suggestions.
The author is verified repository admin; maintainer core-gate exemption applies (98 changed non-test production-file lines, including comments). No daemon routes change. Head/base match selection. Validation was static only: no tests, builds, PR code or live provider requests executed. Comment only; no approval implied.
Independent verification at
|
| arm | converter.ts |
llm-content-generator.ts |
thoughtUtils.ts |
|---|---|---|---|
| base | c4d383781494 |
b811296da5f8 |
523d455694c1 |
| head | a45afabb82e7 |
d5d65e657e53 |
52ef4d65e3ae |
All three base blobs are byte-identical to main, and all three head blobs match the shas GitHub reports for this head, so the only difference between the two arms is this PR's own production delta. A third configuration forces the new predicate to return false as a sensitivity witness.
Results — 14 arms × 3 configurations
ENC_MARKER_9f3a7c was planted inside encrypted_content; "leak" means the marker reached the serialized body.
| arm | base | head | mutant |
|---|---|---|---|
| W1 foreign sig, no options | LEAK thinking/SIG:FOREIGN-PAYLOAD |
clean thinking/NOSIG |
LEAK |
W2 foreign, dropUnsignedAssistantThinking |
LEAK | clean text(summary text) |
LEAK |
| W4 foreign, empty text, dropUnsigned | LEAK | clean (turn emits no block) | LEAK |
| W5 foreign + closed tool chain, dropUnsigned | LEAK | clean text,tool_use + tool_result |
LEAK |
W6 foreign + dangling tool_use, dropUnsigned |
LEAK | clean text,tool_use |
LEAK |
| W8 foreign + DeepSeek pair, tool chain | LEAK | clean thinking/SIG:EMPTY,tool_use |
LEAK |
W9 foreign + stripAssistantThinking |
LEAK | clean thinking/NOSIG |
LEAK |
| W14 foreign + visible answer, DeepSeek pair | LEAK | clean thinking/SIG:EMPTY,text(visible answ) |
LEAK |
W12 foreign + visible answer, stripAssistantThinking |
clean | clean | clean |
| W3 / W7 / W10 / W11 / W13 — native Anthropic signature controls | clean | clean | clean |
- The leak reproduces at base on 8 of 9 foreign arms and is gone at head on all 9. W12 is already clean at base because the strip pass removes the block wholesale, payload included.
- The five native-signature controls are byte-identical across base, head and mutant. A legitimate Anthropic
thinking.signaturestill goes on the wire untouched, so the recognizer is not over-broad on the shapes that matter. - The mutant witness is sensitive on exactly the eight leak-fixing arms and insensitive on all six controls, so the change in behaviour is attributable to
isResponsesReasoningSignatureand not to incidental churn in the harness. - Both claims in the new code comment were measured rather than believed.
fillMissingThinkingSignaturesfillingsignature: ''under DeepSeek normalization is true (W8/W14). ThestripAssistantThinkinghalf is incomplete:stripThinkingFromAssistantMessagesguards withif (filtered.length === 0) continue;, so when the unsigned thinking block is the assistant message's only block it survives the strip (W9). Non-blocking and not a regression — base behaves identically on that shape and worse, since the block it leaves behind carries the foreign payload. The guard exists to avoid emitting a zero-content assistant message. Worth tightening the wording, nothing more. - No arm threw at any configuration, including the tool-chain shapes.
Boundaries of this report
- The Gemini-side edit (
llm-content-generator.ts,delete result.thoughtSignatureon a copied part) was not exercised by this harness — it sits in a method on a fully-constructed generator. It is covered by reading only, plus the PR's own tests in the greenTestlane. - "No arm threw" is bounded by the W7 control, which shows these shapes never reach
dropUnsignedThinkingFromAssistantMessages's throw at either arm. The evidence is that the demote path does not create an unsigned block for that pass to trip on, not that the pass's throw is unreachable in general. - The recognizer's known false negative on a BOM-prefixed payload (
trimStart()strips U+FEFF, thenJSON.parseruns on the untrimmed original) is unchanged by this PR and strictly narrower than the pre-PR behaviour, which had no whitespace tolerance at all. Its sole producer is this repo's ownJSON.stringify, which never emits a BOM.
CI at this head
150/150 check-runs fetched (items == total_count), reduced to the latest attempt per lane name (33 lanes). Product lanes: 6 success, 3 structurally skipped (Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x)), 0 failure, 0 unfinished. Automation: 5 success, 4 skipped, 0 failure.
review-pr = failure and route = cancelled in the list, while their latest attempts are skipped and success. Deduplicating by (name, conclusion) instead of taking the latest attempt per name reports a red lane that is not red.
Verdict
No Critical found. The change removes the measured leak on every arm that had one, preserves every native signature, preserves the visible reasoning summary as text on the demote path, and does not regress the strip/normalize passes. Approving on that basis, as of the state read immediately before posting this comment.
中文说明
验证工具说明：这不是 tmux 报告，而且在本 PR 上 tmux 是无效工具。 要走到新增的判定分支，必须同时满足两个条件：使用 Anthropic 线路的 content generator，并且历史 part 的 thoughtSignature 里带着 Responses 形状的回放载荷（只有 openaiResponsesContentGenerator 会产出）。单 provider 的 CLI 会话最多只能走到 false 分支，因此一份绿色的 tmux 记录对改动的行为毫无证明力。改用出站请求体作为判定依据：在 tsx 下直接调用真实的 AnthropicContentConverter.convertLlmRequestToAnthropic()，对返回的 messages（即真正上线的字段）断言植入 encrypted_content 的标记。全程不发起真实模型请求，也不需要 API key。
A/B 纯净性已实测：base 三个生产文件 blob 与 main 完全一致（c4d383781494 / b811296da5f8 / 523d455694c1），head 三个 blob 与 GitHub 在该 head 上报告的 sha 一致（a45afabb82e7 / d5d65e657e53 / 52ef4d65e3ae），所以两个分支之间的唯一差异就是本 PR 自己的生产代码改动；第三组配置把新判定函数强制 return false 作为敏感性见证。
结果（14 组 × 3 配置）：base 在 9 组外来签名场景中有 8 组把载荷泄漏成 thinking.signature，head 全部消除；5 组原生 Anthropic 签名对照组在 base / head / mutant 三种配置下输出完全一致，说明识别函数没有误伤合法签名；变异见证恰好只在 8 组修复场景上敏感、在全部对照组上不敏感，因此行为变化可归因于 isResponsesReasoningSignature 本身。
新注释里的两个断言都做了实测：DeepSeek 归一化下 fillMissingThinkingSignatures 补 signature: '' 为真（W8/W14）；stripAssistantThinking 那半句不完整 —— stripThinkingFromAssistantMessages 有 if (filtered.length === 0) continue; 保护，当无签名 thinking 块是该 assistant 消息的唯一块时它会留存（W9）。这属于非阻断问题，而且不是回归：base 在同一形状下行为相同且更糟（留存的块还带着外来载荷）。该保护本身是为了避免发出零内容的 assistant 消息。
报告边界：Gemini 侧改动（llm-content-generator.ts 中对副本 part 执行 delete result.thoughtSignature）未被本 harness 执行，仅有阅读覆盖，外加绿色 Test lane 中 PR 自带的测试；"没有任何 arm 抛错"受 W7 对照组限定，只能证明降级路径不会为该 pass 造出无签名块，不能证明该 pass 的抛错路径整体不可达；BOM 前缀导致的漏判为既有行为，本 PR 未改变且严格窄于改动前。
CI：150/150 check-run 全部抓取（items == total_count），按 lane 名取最新一次尝试归约（33 条）。产品 lane：6 绿、3 结构性 skip、0 失败、0 未完成；自动化 lane：5 绿、4 skip、0 失败。提醒：不做最新尝试归约的读法会把早期尝试的 review-pr = failure、route = cancelled 当成当前红灯。
结论：未发现 Critical。改动在每一个原本泄漏的场景上都消除了泄漏，保留了全部原生签名，在降级路径上把可见推理摘要保留为文本，且未使 strip/normalize 两个 pass 回归。据此给出 approve（以本条评论发布前即时读到的状态为准）。
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving on the strength of the executed verification in the comment above (issue comment 5636169545), as of the state read immediately before submitting this review.
Basis, all measured at head 11b347100ce92d653931da3382992ca6e335a268:
- The leak is real at base and gone at head. An A/B over the outbound request body — the real
AnthropicContentConverter.convertLlmRequestToAnthropic()undertsx, no live provider and no API key — reproduces the Responses replay payload reaching the wire asthinking.signatureon 8 of 9 foreign-signature arms at the PR base blob, and shows all 9 clean at the head blob. The three base blobs are byte-identical tomainand the three head blobs match this head's reported shas, so the only difference between the arms is this PR's own production delta. - No over-broad recognition. Five native-Anthropic-signature controls are identical across base, head and a mutant that forces the new predicate to
return false; that mutant is sensitive on exactly the eight leak-fixing arms and insensitive on all six controls. - CI at this head: 150/150 check-runs, latest attempt per lane, product lanes 6 success / 3 structurally skipped / 0 failure / 0 unfinished.
Not a tmux report, and the reason is in the comment above: a single-provider CLI session can only reach the false branch of the new guard, so a green tmux transcript would carry no information about the changed behaviour. Boundaries are stated there too — the Gemini-side deletion is covered by reading rather than execution, and the "no arm threw" result is bounded by a control showing these shapes never reach the unsigned-thinking pass at either arm.
One non-blocking note, not a request: the new comment in converter.ts says the unsigned block is removed by stripThinkingFromAssistantMessages under stripAssistantThinking, but that pass guards with if (filtered.length === 0) continue;, so the block survives when it is the assistant message's only block. Pre-existing behaviour and strictly better than base, which leaves the same block behind carrying the foreign payload.
6 participants
What this PR does
Part.thoughtSignatureis the shared cross-provider store for reasoning replay data, but it carries no wire, model, or origin marker, so each content generator interprets the same opaque string its own way. This PR adds one recognizer for the OpenAI Responses replay payload shape —isResponsesReasoningSignatureinpackages/core/src/utils/thoughtUtils.ts— and applies it at the two request-build points that would otherwise forward a foreign payload. The Anthropic converter now leaves the thinking block unsigned instead of emitting the payload as a nativethinking.signature, and the Gemini wire deletes it from the shallow copystripPartFieldsalready builds. Both keep the visible reasoning text, and the Gemini strip is wire-only, so the caller's history still holds the payload for a later switch back to the Responses API.Why it's needed
Since #8169 merged,
responses-converter.tswritesJSON.stringify({ id, encrypted_content })intothoughtSignaturefor every Responses reasoning item. After a provider switch that payload is eligible for forwarding verbatim on the Anthropic wire — the only guard there istypeof part.thoughtSignature === 'string'— and it travels untouched on the Gemini wire, so one provider's opaque replay data goes out as another provider's native signature. This was reproduced onmain@35a702c33with synthetic parts and no credentials; see the repro comment on #9453.This completes a pattern already established in the codebase rather than adding a new heuristic.
responses-converter.tsapplies the symmetric fallback in the other direction for an unreplayable signature — drop the payload, preserve the human-readable summary, log the drop — andllm-chat.ts(isCompleteResponsesReasoningSignature) plusresponses-converter.ts(decodeReasoningSignature) already carry private copies of the same shape check. The new helper is the shared, exported version of that check, placed in the leafthoughtUtils.tsmodule both call sites already import from.Reviewer Test Plan
How to verify
Both new leak tests were confirmed failing before the guard and passing after it, by reverting only the three source files (
git checkout HEAD -- packages/core/src/utils/thoughtUtils.ts packages/core/src/core/anthropicContentGenerator/converter.ts packages/core/src/core/llm-content-generator/llm-content-generator.ts) while keeping the two test files, then restoring from a patch.Before the guard (fix reverted, tests kept):
After the guard:
Adjacent wire regression (the other three generators that touch
thoughtSignature):Gates:
npm run typecheckinpackages/core(tsc --noEmit) passes with zero errors;prettier --checkandeslintpass on all five changed files.Each wire gets both sides asserted, not just "does not throw": a legitimate native signature is forwarded unchanged (Anthropic emits it as
thinking.signature, Gemini passes it through), and the Responses payload is absent whilethought: trueand the visible reasoning text survive. A third test asserts the Gemini strip does not mutate the caller-owned history part, holding it by object identity.Wire coverage is complete with these two guards: the OpenAI Chat converter never reads
thoughtSignature(signature dropped, visible text kept — no leak), andresponses-converter.tsalready handles its own direction.Evidence (Before & After)
N/A — not a user-visible/TUI change. The wire-level before/after evidence is the failing-then-passing assertions above.
Tested on
✅ tested ·⚠️ not tested — unit tests and gates were run on a headless Linux host only; no macOS/Windows run and no live multi-provider session.
Environment (optional)
Unit tests only (vitest,
packages/core). No live provider credentials were used; the repro uses synthetic parts, matching the issue's own statement that no credentials are required to reproduce the conversion behavior.Risk & Scope
{"id": <string>, "encrypted_content": <string>}. Native Anthropicthinking.signatureand GeminithoughtSignaturevalues are opaque tokens that never take that JSON shape, and the recognizer requires both keys to be strings, so this is theoretical rather than observed — and both wires carry an explicit "native signature preserved unchanged" test to catch a regression here. Dropping a foreign payload degrades gracefully: the reasoning text is preserved and the turn is simply sent unsigned.llm-chat.ts's privateisCompleteResponsesReasoningSignaturewas deliberately left alone rather than refactored onto the new shared helper, to keep this a narrow bugfix and avoid churn in a file fix(core): preserve every reasoning episode's signature during history consolidation #8260 just touched; that dedup is a reasonable follow-up.docs/design/entry: AGENTS.md's design-doc rule ("write one indocs/design/if the change touches multiple files or involves design decisions. Skip for small bugfixes.") is exempted here — this is a 67-line source bugfix that completes an existing pattern and deliberately makes no provenance design decision, which stays in Foundational problem: Content[]/Part[] cannot safely encode per-provider reasoning-replay contracts #8533.Linked Issues
Fixes #9453
Related, referenced without a closing keyword: #8533 (versioned provenance contract — where the "unknown by default" half belongs), #9452 (bounded recovery for already-persisted Responses sessions), #8169 (the Responses generator that produces the payload), #8260 (preserves more signatures across history consolidation; its design doc states it "does not repair previously corrupted saved signatures or address context compression and cross-provider provenance", so it does not cover this).
中文说明
这个 PR 做了什么
Part.thoughtSignature是跨提供商共享的推理重放数据存储位，但它没有任何线路、模型或来源标记，因此每个内容生成器都会用自己的方式解释同一个不透明字符串。本 PR 新增一个识别 OpenAI Responses 重放负载形状的判定函数 ——packages/core/src/utils/thoughtUtils.ts中的isResponsesReasoningSignature—— 并在两个会把外来负载转发出去的请求构建点应用它：Anthropic 转换器不再把该负载当作原生thinking.signature发出，而是让 thinking 块保持无签名；Gemini 线路则从stripPartFields已经构建好的浅拷贝中删除它。两者都保留可见推理文本，且 Gemini 的剥离只作用于发往线路的拷贝，调用方持有的历史仍然保留该负载，之后切回 Responses API 还能继续重放。为什么需要
自 #8169 合并后，
responses-converter.ts会为每个 Responses 推理项把JSON.stringify({ id, encrypted_content })写入thoughtSignature。切换提供商后，这个负载在 Anthropic 线路上会被原样转发（那里唯一的判断是typeof part.thoughtSignature === 'string'），在 Gemini 线路上也会原样带出，于是一个提供商的不透明重放数据被当成另一个提供商的原生签名发送。该行为已在main@35a702c33上用合成 Part、无需任何凭据复现，详见 #9453 的复现评论。本 PR 是补全代码库中已有的模式，而不是引入新启发式：
responses-converter.ts在另一个方向上对不可重放的签名已经采用了对称的兜底（丢弃负载、保留可读摘要、记录日志），而llm-chat.ts（isCompleteResponsesReasoningSignature）与responses-converter.ts（decodeReasoningSignature）本就各有一份私有的同形判定。新增的这个 helper 是该判定的共享导出版本，放在两个调用点都已经引用的叶子模块thoughtUtils.ts里。审阅测试计划
如何验证
两个新的泄漏测试都已确认「加守卫前失败、加守卫后通过」：只回退三个源码文件（
git checkout HEAD -- packages/core/src/utils/thoughtUtils.ts packages/core/src/core/anthropicContentGenerator/converter.ts packages/core/src/core/llm-content-generator/llm-content-generator.ts），保留两个测试文件，再用 patch 恢复。守卫前（源码回退、测试保留）： 两个用例失败，断言为
expected '{"id":"rs_68c6c0c9ff5c8191a29b2e78c1a…' to be undefined，实际收到"{\"id\":\"rs_68c6c0c9ff5c8191a29b2e78c1a40c83\",\"encrypted_content\":\"gAAAAABvcmVhc29uaW5nLXJlcGxheS1wYXlsb2Fk\"}"；汇总Test Files 2 failed (2)、Tests 2 failed | 141 passed (143)。守卫后：
converter.test.ts+llm-content-generator.test.ts+thoughtUtils.test.ts汇总Test Files 3 passed (3)、Tests 158 passed (158)。相邻线路回归：
responses-converter.test.ts(90) +anthropicContentGenerator.test.ts(175) +openaiContentGenerator/converter.test.ts(234) 汇总Test Files 3 passed (3)、Tests 499 passed (499)。门禁：
packages/core的npm run typecheck（tsc --noEmit）零错误通过；五个改动文件的prettier --check与eslint均通过。每条线路都断言了两侧，而不是只断言「不抛错」：合法的原生签名被原样转发（Anthropic 作为
thinking.signature发出、Gemini 原样透传），Responses 负载则不存在，同时thought: true与可见推理文本保留。第三个测试通过持有对象引用来断言 Gemini 的剥离没有改动调用方的历史 Part。这两个守卫已覆盖全部线路：OpenAI Chat 转换器从不读取
thoughtSignature（丢签名、留可见文本，不存在泄漏），responses-converter.ts自己那个方向已经处理过了。证据（前后对比）
不适用 —— 非用户可见/TUI 改动。线路层面的前后证据即上面「先失败后通过」的断言。
测试平台
✅ 已测试 ·⚠️ 未测试 —— 单元测试与门禁仅在无图形界面的 Linux 主机上运行；未做 macOS/Windows 验证，也没有真实的多提供商会话。
环境（可选）
仅单元测试（vitest，
packages/core）。未使用任何真实提供商凭据；复现使用合成 Part，与 issue 自身「无需凭据即可复现转换行为」的说明一致。风险与范围
{"id": <string>, "encrypted_content": <string>}的合法原生签名。Anthropic 原生thinking.signature与 GeminithoughtSignature都是不透明 token，永远不会是这个 JSON 形状，且识别函数要求两个键都是字符串，因此这是理论风险而非已观测风险 —— 两条线路都各有一个「原生签名原样保留」的测试来兜住这类回归。丢掉外来负载是优雅降级：推理文本保留，该轮只是以无签名形式发出。llm-chat.ts里私有的isCompleteResponsesReasoningSignature重构到新的共享 helper 上，以保持这是一个范围收窄的 bugfix，并避免在 fix(core): preserve every reasoning episode's signature during history consolidation #8260 刚改过的文件里制造变动；这个去重适合作为后续工作。docs/design/文档：AGENTS.md 的设计文档条款（「如果改动涉及多个文件或包含设计决策，就在docs/design/写一份。小 bugfix 可跳过。」）在此适用豁免 —— 本 PR 是 67 行源码的 bugfix，补全既有模式，并且刻意不做任何来源契约的设计决策（那部分留在 Foundational problem: Content[]/Part[] cannot safely encode per-provider reasoning-replay contracts #8533）。关联 Issue
Fixes #9453
相关但不带关闭关键字的引用：#8533（版本化来源契约 —— 「默认视为不兼容」那一半应归属此处）、#9452（已持久化 Responses 会话的有界恢复）、#8169（产生该负载的 Responses 生成器）、#8260（在历史合并时保留更多签名；其设计文档声明它「不修复此前已损坏的保存签名，也不处理上下文压缩与跨提供商来源问题」，因此不覆盖本问题）。