Repository navigation
Conversation
…#5887 option A) Store providerExecuted and the encrypted results for successful Anthropic web_search calls, and replay server_tool_use + web_search_tool_result in place, so later requests keep the signed thinking after a search and the prompt cache holds. Other server tools and other providers keep the client pair; the #6004 receipt stays the fallback for them.
…he client pair (#5887) Perf-owner condition: a web search whose summed encryptedContent exceeds ANTHROPIC_NATIVE_SERVER_TOOL_MAX_CIPHERTEXT_CHARS (256 KiB) is stored as the pre-#5887 client pair without ciphertext, and the receipt keeps the thinking after it out. A search called in parallel with a client tool gets its result at the start of the next step. One stored part cannot replay the call and the result at their two positions, so it is stored as the client pair too. tool-call-end now carries the stored output, so dropped ciphertext does not cross IPC. validateAnthropicCompliance skips provider-executed calls, whose result rides in the same message. The projection skips rows with no server tool without allocating.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c546df6ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9410c83879
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800c93b634
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…uire pageAge (#5887 review)
|
Paused at the review cap. The 6 Codex reviews (3 code-plus-security pairs) are used up. The last pair found 2 blockers. Their fix ( Generated with |
|
Final review pair. One extra code-plus-security review pair was granted for this PR (reviews 7 and 8), on the pushed head. It is an extension, not a budget reset. If this pair finds any valid blocker, the PR is parked with no further repair cycle, and the #6004 containment stays the behavior. This head carries the round-3 fixes and the reorder fix found by the live in-message check (#5887 issuecomment-6095755816). Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ca04a885c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (part.providerExecuted !== true || part.toolName !== "web_search") return false; | ||
| if (part.state !== "output-available") return false; | ||
| // The SDK validates every native result against a schema that requires encryptedContent and | ||
| // a string-or-null title: one missing field throws, and no request is sent at all. | ||
| return Array.isArray(part.output) && part.output.every(isReplayableWebSearchResult); |
There was a problem hiding this comment.
When a persisted providerExecuted search has a corrupted non-object input, sanitizeToolInputs() rewrites it to {}, but this predicate validates only the output and still replays the part natively. Any later signed thinking is then sent against an altered server_tool_use prefix, causing Anthropic signature validation to reject subsequent turns; validate the input before replay or demote the search so the existing reasoning-stripping recovery runs.
AGENTS.md reference: AGENTS.md:L128-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid. This is the blocker that parks this PR, so it is not fixed here.
isNativeAnthropicReplayablechecks only the output. A storedproviderExecutedsearch with a non-object input passes it, andsanitizeToolInputs(src/browser/utils/messages/sanitizeToolInput.ts) then rewrites that input to{}. So the request sends an alteredserver_tool_use, and the signed thinking after it no longer matches.- Trigger: only corrupted stored input. The stream stores the object the API returned, and in the live re-check the replayed
server_tool_usewas identical to the raw response block. - Impact: on
block_binding: drop_blockroutes, the API drops that thinking (degraded, no 400). On other routes, the request gets one 400, then the existing signature repair writes the replay receipt and the workspace continues. - Fix for a future attempt: require a plain-object input in the predicate, so such a part is demoted and its thinking stripped (about 1 line).
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $133.33
|
Parked. The final granted review pair (reviews 7 and 8) on The branch stays at Generated with |
This branch has not been deployed
1 participant
Summary
Xum now replays a successful Anthropic
web_searchfrom history in its original position:server_tool_useandweb_search_tool_result(withencrypted_content), next to the signed thinking blocks around them. Before this change, history replayed the search as a clienttool_use/tool_resultpair. The #6004 receipt then had to drop all thinking for the rest of the context segment, and the cached prefix changed. With native replay, later requests keep the thinking and resend the same prefix.Part of #5887. The issue stays open for the text-first reorder audit.
Background
Preserved thinking binds each thinking block to everything before it. In one step the API returns
thinking, server_tool_use, web_search_tool_result, thinking, .... The SDK replays exactly that within the turn. Xum's stored part lost two things: theproviderExecutedflag, and the result'sencryptedContent(removed bystripEncryptedContent). So the next turn could only send the client pair. #6004 contained this by writing the thinking-replay receipt. This PR keeps the native identity, so a successful search no longer needs the receipt.Implementation
StreamManager): the tool-call part of an Anthropic Messages server tool storesproviderExecuted: true. The result keeps its ciphertext only when the stored call part carries the flag.toStoredServerToolPart(in the newsrc/common/utils/messages/anthropicNativeServerTools.ts) decides the stored form when the result arrives. The part keeps the flag only when history can replay it natively (isNativeAnthropicReplayable). Null titles are stored asnull, because the SDK stream omits them and its replay schema requires them.messagePipeline.ts):projectAnthropicServerToolskeeps replayable parts native only whenproviderForMessages === "anthropic". Every other provider-executed part is demoted to the client pair without ciphertext. If a demoted part has reasoning after it in the same row, the request sends no Anthropic thinking. That flag is a pure function of the rows, so every request in the segment decides the same way. Rows with no server tool are returned as they are, with no new allocation.modelMessageTransform.ts):splitMixedContentMessageskeeps provider-executed calls inside their message.ensureAnthropicThinkingBeforeToolCallskeeps the API's order for a message that opens with a provider-executed call: the API itself started that response withserver_tool_use, and moving the thinking in front of the search breaks the thinking's binding.validateAnthropicComplianceskips provider-executed calls, because their result rides in the same message.WebSearchToolCallhidesencryptedContentin its expanded JSON.tool-call-endcarries the stored output, so dropped ciphertext never crosses IPC.stripEncryptedContentmoved fromsrc/nodetosrc/common(git mv) so the common projection can use it.What still replays as the client pair (today's behavior, plus the receipt)
web_fetch,code_execution, ...).encryptedContentaboveANTHROPIC_NATIVE_SERVER_TOOL_MAX_ROW_CIPHERTEXT_CHARS(256 KiB summed over every native search in the row,src/constants/anthropicServerTools.ts), or any single result whose fields the 12,000-character sanitizer would rewrite. The partial holding it is rewritten on every throttled write for the rest of the turn, the row crosses IPC, andchat.jsonlreaders skip rows over 1 MiB.Known limits
encryptedContent, or any field the sanitizer would rewrite, is stored as the client pair. The receipt then drops the thinking after it, and the prompt cache does not hold past that search. Option A preserves thinking and the cache only for searches that fit. In L1, the 9 real results were 752 to 2,864 characters each, so 0 of 9 were demoted by the 12,000-character limit.count_tokensprobe for that, and it needs real ciphertext from L1. The summary is headless and one-shot, so this costs no turn cache.block_binding: drop_block):Validation
All tests drive scripted SSE through the real Anthropic (and OpenAI) SDKs, with a real
HistoryService, and read the request body the SDK sends.systemandtoolsare equal across requests. Each request's messages equal the previous request's messages, plus the previous reply exactly as the API returned it, plus the new user turn.cache_controlis compared separately: system and tools keep the same breakpoints, and the message breakpoint sits only on the newest block.assemblePromptPayload.tool-call-endevent carries no ciphertext. The existing 🤖 fix: turn Anthropic thinking replay off after a server tool between thinking blocks #6004 and 🤖 fix: continuous compaction summary misses a replay receipt in the retained tail #5996 tests now use a non-native search, so they still guard the receipt path.main's production code, the native, null-title, T2 and lost-ciphertext tests fail.messagePipeline.tsandmodelMessageTransform.tstomainand fed them a new-format native row. The request builds without throwing. The old splitter keeps native blocks when no client tool follows them, and splits or drops them when one does: degraded, not bricked. Rows that fail the predicate are stored without the flag, so an older build's SDK never sees a flagged result it cannot validate.Risks
Medium, Anthropic only. The change touches the stored shape of server-tool parts and the provider request for every Anthropic request that contains one. If the API rejects a native replay that the scripted fixtures accept, the next request in that segment fails. I have not verified that the existing thinking-signature repair recovers from that error. L1 is the check for this. Other providers and client tools are unchanged: their parts carry no flag, and the projection returns their rows as they are.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$115.89