fix(agent): strip api_messages in thinking-signature recovery so the retry actually omits thinking blocks - #35265
Closed
0xyg3n wants to merge 1 commit into
Closed
Conversation
…retry actually omits thinking blocks
The thinking-signature recovery in agent/conversation_loop.py popped
reasoning_details from messages, then continued to retry. That had two
defects.
First, the strip never reached the wire payload. api_messages is built
once at the start of the turn by shallow-copying every entry in messages
(line 919 area). Each api_messages entry has its own reference to the
same reasoning_details list. When build_api_kwargs runs on every retry
iteration of the inner while-loop, it consumes api_messages, not
messages. Popping reasoning_details from messages left api_messages
untouched, so the retry's request still carried the same thinking
blocks Anthropic had just rejected. The classifier latched
thinking_sig_retry_attempted = True after the first attempt, and the
loop terminated with max_retries_exhausted on the same 400.
Second, the pop mutated the canonical message list. messages is the
same list _persist_session writes to state.db and the session
transcript, so a single recovery permanently wiped every signed
thinking block from the stored conversation. Subsequent turns reloaded
the stripped state, hit the same 400 ('invalid signature' or 'cannot
be modified', see NousResearch#24107), and the agent stopped responding entirely.
Cascading compaction-ended sessions then chained off the corrupted
parent and the affected chat could not produce a response on any
future turn.
Move the strip onto api_messages, which is the API-call-time list
rebuilt into kwargs on every retry. messages is no longer touched, so
disk I/O stays clean and the recovery actually reaches the wire.
Observed against the native Anthropic Messages API on claude-opus-4-7
and claude-opus-4-8 with the interleaved-thinking-2025-05-14 beta on
hermes-agent 0.12.0 and 0.14.0. PR NousResearch#24107 narrows the trigger; this
change makes the recovery do what it always claimed to do, and
prevents the destructive aftermath.
Tests cover the api_messages strip in isolation: pop on a shallow copy
does not affect the source, the canonical messages list survives the
strip, idempotency on a duplicate firing path, and a no-op when no
reasoning_details exist on the messages.
Related: NousResearch#24107, NousResearch#26959, NousResearch#17861.
0xyg3n
force-pushed
the
fix/thinking-sig-recovery-transient-strip
branch
from
May 30, 2026 10:28
c93cdc8 to
ed6a744
Compare
9 tasks
Contributor
|
Merged via PR #43667 — your commit was cherry-picked onto current main with your authorship preserved in git log (9f95f72). One adjustment during salvage: main had moved the retry flag into TurnRetryState, so the resolved version uses _retry.thinking_sig_retry_attempted with your api_messages strip logic intact. Thanks! |
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.
Summary
The thinking-signature recovery in
agent/conversation_loop.pypoppedreasoning_detailsfrommessagesand continued to retry. That had two defects, only one of which was the visible one.The visible defect: the pop mutated the canonical conversation list.
messagesis the same list_persist_sessionwrites tostate.dband the session transcript, so a single recovery permanently wiped every signed thinking block from the stored conversation. Subsequent turns reloaded the stripped state, hit the same HTTP 400 ("invalid signature" or "cannot be modified", see #24107), and the agent stopped responding on that chat. Cascading compaction-ended sessions chained off the corrupted parent.The hidden defect: the strip never reached the wire payload either.
api_messagesis built once at the start of the turn by shallow-copying every entry inmessages(line 919 area). Each entry inapi_messageshas its own reference to the samereasoning_detailslist. Whenbuild_api_kwargsruns on every retry inside the inner while-loop, it consumesapi_messages, notmessages. Poppingreasoning_detailsfrommessagesleftapi_messagesuntouched, so the retry's request still carried the same thinking blocks Anthropic had just rejected. The classifier had already latchedthinking_sig_retry_attempted = Trueafter the first attempt, so the recovery branch did not fire a second time, and the loop terminated withmax_retries_exhaustedon the same 400.This PR moves the strip onto
api_messages.messagesis no longer touched, so disk I/O stays clean, and the strip actually reaches the wire becauseapi_messagesis whatbuild_api_kwargsconsumes on every retry.Root cause
agent/conversation_loop.py, current behavior:agent/conversation_loop.py, message-list construction earlier inrun_conversation:.copy()is a shallow dict copy. Top-level keys are independent across the two dicts, so apopon one does not affect the other. The values, including thereasoning_detailslist, are shared by reference. After the loop,messages[i]andapi_messages[i]are two dicts each carrying their own key"reasoning_details"pointing at the same list object.agent/conversation_loop.py, retry loop:api_kwargsis rebuilt fromapi_messageson every iteration. The strip needs to reachapi_messagesto affect the next call.Fix
agent/conversation_loop.py:messagesis not touched._persist_sessionwrites intactreasoning_detailson every persist boundary. The strip propagates intoapi_kwargson the next iteration through_build_api_kwargs(api_messages), so the retry's request goes out without thinking blocks.Versions observed
Reproduced against the native Anthropic Messages API on
claude-opus-4-7andclaude-opus-4-8with theinterleaved-thinking-2025-05-14beta enabled (Hermes sends this by default for Claude 4.x).hermes-agent 0.12.0(editable install)hermes-agent 0.14.0(editable install, including a clean vanilla checkout with no other patches)The trigger surface is independent of the local hermes version: any deployment that sends signed thinking blocks against the interleaved-thinking beta and replays them on multi-turn tool-use conversations is exposed once
FailoverReason.thinking_signaturefires for any reason.Reproduction signature in
state.dbAffected sessions show a clean before/after split in the
messagestable: assistant rows up to the recovery point carry intactreasoning_detailswith signatures; every assistant row from that point on hasreasoning_details = NULLeven though the model returned a thinking block. Countingsignedvstool_calls without signed reasoning_detailsper session is a reliable detector for the corruption signature this PR prevents.Tests
tests/run_agent/test_thinking_sig_recovery_persistence.pycovers the mutation surface in isolation:test_pop_on_shallow_copy_does_not_affect_source- the invariant the recovery relies on.test_strip_api_messages_leaves_canonical_messages_intact- the recovery loop mirrored against shallow-copied messages.test_strip_is_idempotent_when_run_twice- duplicate firing is safe.test_strip_skips_messages_without_reasoning_details- non-applicable messages are untouched.Regression sweep:
Compatibility
_build_api_kwargssignature is unchanged._persist_sessionsignature and behaviour are unchanged.messagestable was already wiped by the previous behaviour are not retroactively repaired. Such sessions continue to fail on replay until they are trimmed or the user starts a fresh chat. PR fix(anthropic): preserve thinking blocks on prior tool_use turns (interleaved-thinking contract) #24107 reduces the rate at which fresh sessions reach this state.Related