fix: stop cross-provider thinking blocks from bricking a session - #103
Merged
Merged
Conversation
ThinkPart carried a single untagged `encrypted` field that the Anthropic, OpenAI Responses and Google GenAI adapters all wrote mutually incompatible values into. After a mid-session model switch the Anthropic adapter replayed a foreign reasoning blob as its own signature, and the API rejected it: messages.1.content.0: Invalid `signature` in `thinking` block Because the block sat in the first assistant message, every later turn resent it, so the session failed identically forever and could not be recovered by retrying, forking or switching Claude models. Tag the blob with the protocol that produced it and emit it only to that protocol, falling through to the existing unsigned branch otherwise. An untagged blob is still treated as compatible, so stored sessions keep their current behaviour rather than relying on format guesswork. Sessions poisoned before this change recover at runtime instead. Classify the rejection with isThinkingSignatureError, add a `thinking: 'strip'` projection axis that removes every thinking and redacted_thinking block from the history while leaving text and tool calls in place, and resend once. The recovery is recorded as a durable event and folded into replayable agent state, so a session pays the error at most once across later turns and reloads. A total strip is the documented remedy; partial stripping is what triggers the separate "blocks in the latest assistant message cannot be modified" error. Verified against the live API on both an adaptive and an extended-thinking model that a total strip is accepted even when the latest assistant turn carried thinking alongside a tool call. The legacy engine gets the same rung through a message builder, but keeps its recovery flags in runTurn locals and has no durable state, so a v1 session re-pays one rejection per turn.
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
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.
Related Issue
No linked issue. This is a fork-internal bug fix against
fork/main, so the upstreamexternal-PR policy (issue with a maintainer
/approve) does not apply. The problem isdescribed in full below, and the fork carries a standalone diagnosis of it in
docs/CLAUDE-FIX.md.Problem
Switching from a Codex or Gemini model to a Claude model inside one session made every
subsequent turn fail, permanently:
ThinkPartcarried a single untaggedencryptedfield, and three adapters wrote mutuallyincompatible values into it — Anthropic its
signature, OpenAI Responses areasoning.encrypted_contentFernet token, Google athoughtSignature. Nothing recordedwhich one produced a given blob, and nothing sanitized history on a model switch, so the
Anthropic adapter replayed a foreign blob verbatim as its own signature. Anthropic decrypts
signatures to verify them, so the request failed validation.
Two things made this unrecoverable rather than merely annoying:
Retrying, forking the session, or picking a different Claude model all replayed it.
isRecoverableRequestStructureErrordoes notmatch this message, and even if it did, the
structure: 'strict're-projection it selectsonly dedupes tool calls, merges assistants and drops leading non-user messages — it never
touches thinking blocks. The projector's
wireSendableContentactively keeps signedthinking, so it protected the poison.
Reproduced against the live Anthropic API using the real blob from a bricked session: the
request returns 400 with the exact message above, and the identical request with that one
block removed returns 200.
What changed
Two independent layers, because prevention alone leaves every existing session dead and
recovery alone leaves the bug live.
Prevention.
ThinkPartgainsencryptedProtocol, typed as the existingProtocolunion so the tag cannot drift from the protocol ids the rest of the code already uses. Every
producer sets it; every consumer reads the blob through one shared helper that returns it
only when the tag matches. A mismatch falls through to the existing unsigned branch. An
untagged blob is still treated as compatible, so stored sessions keep their current
behaviour instead of depending on format guesswork — no sniffing of blob shapes.
Recovery, for sessions poisoned before this lands, whose think parts carry no tag. A new
isThinkingSignatureErrorclassifies the rejection. A newthinking: 'strip'projectionaxis rebuilds the request with every
thinkingandredacted_thinkingblock removed fromthe whole history, leaving text and tool calls in place, and the request is resent once. The
recovery is recorded as a durable
llm.thinking_strippedevent folded into replayable agentstate, so a session pays the error at most once — across later turns and across reloads.
The classifier is deliberately not added to
STRUCTURAL_REQUEST_MESSAGE_PATTERNS. Thatarray selects the strict re-projection, which cannot remove thinking, so routing this error
there would burn a retry and fail identically. It also deliberately excludes Anthropic
configuration errors (
"thinking.type.enabled" is not supported for this model,adaptive thinking is not supported,block_binding: Extra inputs are not permitted), which need adifferent remedy and must not be silently swallowed.
A total strip is what Anthropic documents as the remedy; partial stripping is what
triggers the separate
blocks in the latest assistant message cannot be modifiederror. Thiswas verified live rather than assumed: a total strip is accepted (200) even when the latest
assistant turn carried thinking alongside a
tool_use, on both an adaptive model(
claude-opus-5) and an extended-thinking budget-mode model (claude-haiku-4-5).All three engines are covered. The legacy v1 engine gets the same rung through a
buildMessagesThinkingStrippedbuilder, but v1 keeps its recovery flags inrunTurnlocalsand has no durable state, so a v1 session re-pays one rejection per turn. That asymmetry is
intentional and documented rather than papered over.
Verification
tsc --noEmitclean for each;
check-no-commentsclean (agent-core-v2 is a comment-free zone).returns 400 before the strip and 200 after it.
Note for reviewers on CI: this branch does not touch
kap-serverorminidb, but theirsearch/searchService,search/searchRouteandworker-buildsuites fail on Node v26 withworker-spawn errors (
SearchWorkerHost.doSpawn,RawPostingsWriter.openENOENT). Both failidentically in isolation on an unmodified tree, so they are pre-existing and environmental.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update — the bilingual VitePress docsdescribe no behaviour this changes.