Skip to content

fix(agent): clear ordered thinking replay state on signature recovery - #70107

Open
Eklps wants to merge 1 commit into
NousResearch:mainfrom
Eklps:fix/sig-recovery-ordered-blocks
Open

fix(agent): clear ordered thinking replay state on signature recovery#70107
Eklps wants to merge 1 commit into
NousResearch:mainfrom
Eklps:fix/sig-recovery-ordered-blocks

Conversation

@Eklps

@Eklps Eklps commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Follow-up to #66694.

On Invalid signature in thinking block recovery, Hermes previously stripped only reasoning_details from API-time message copies but retained anthropic_content_blocks — a separate ordered-block channel that can replay the same invalid signed thinking block, causing the retry to 400 again.

This clears both fields from api_messages while leaving the canonical messages store untouched (session history is preserved).

Root cause

When a provider returns HTTP 400 Invalid signature in thinking block, the recovery path in conversation_loop.py strips reasoning_details from shallow-copied api_messages and retries. However, build_assistant_message also lifts anthropic_content_blocks (the verbatim, order-preserving Anthropic content block list) onto the stored message. The replay fast-path in convert_messages_to_anthropic sources from this channel when present — so the retry still replayed the same invalid signed thinking block and 400'd again.

Fix

Strip anthropic_content_blocks from API-time copies alongside reasoning_details during signature recovery. The canonical messages list (and therefore state.db) is unchanged.

Verification

  • python -m py_compile — passed
  • git diff --check — passed
  • tests/run_agent/test_thinking_sig_recovery_persistence.py4 passed (updated to cover both reasoning_details and anthropic_content_blocks)

Scope

Two files:

  • agent/conversation_loop.py — add anthropic_content_blocks to the recovery strip
  • tests/run_agent/test_thinking_sig_recovery_persistence.py — update tests to cover both fields

No changes to agent/anthropic_adapter.py, agent/transports/, or any provider plugin.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/anthropic Anthropic native Messages API labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the ordered-block replay channel. The premise holds on current main: agent/conversation_loop.py:3942-3945 strips only reasoning_details, while agent/anthropic_adapter.py:2044-2102 preferentially replays non-empty anthropic_content_blocks; agent/chat_completion_helpers.py:1541-1551 stores that channel on assistant messages.

Problems

  • tests/run_agent/test_thinking_sig_recovery_persistence.py mirrors the stripping loop rather than invoking agent/conversation_loop.py:3936-3957 and the Anthropic conversion path. It therefore cannot catch a wiring regression where the production retry still replays anthropic_content_blocks.

Suggested changes

  • Add a real-path recovery test that verifies the retry wire payload drops both fields, canonical messages retain both fields, and convert_messages_to_anthropic() cannot select the ordered replay path for that retry.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two open PRs address the same invalid signed-thinking replay failure during signature recovery. #24407 strips thinking blocks from canonical messages, while #70107 clears both replay channels from API-time copies and preserves canonical history.

Related pull requests

  • fix(agent): strip thinking blocks from content arrays on signature recovery #24407 duplicate — (+145/-7) — verify verdict: n/a: The diff removes thinking and redacted_thinking blocks from assistant content arrays and clears reasoning_details, directly targeting the content-array failure, but mutates canonical messages and tests a reproduced loop rather than the production retry/conversion path. The automated keep-open verdict records medium salvageability and identifies moving the filtering to api_messages plus a real-path test as necessary.
  • fix(agent): clear ordered thinking replay state on signature recovery #70107 related — (+48/-10) — verify verdict: n/a: The diff removes reasoning_details and anthropic_content_blocks from shallow-copied api_messages, addressing the ordered replay channel while leaving canonical messages unchanged; its tests still mirror the stripping loop instead of exercising production retry and Anthropic conversion. The automated keep-open verdict records high salvageability and requests a real-path test covering the clean wire payload, preserved canonical state, and disabled ordered replay.

Duplicates

#24407 and #70107 substantially overlap: both target repeated HTTP 400 failures caused by replayed signed thinking state during recovery. #24407 is the broader content-array variant; #70107 targets the current API-time ordered-replay architecture.

Suggested consolidation

Keep open #70107 with a salvage path: retain its API-time clearing of both replay channels and add the requested real-path retry/conversion test. Close #24407 as duplicate of #70107 after preserving its content-array diagnosis; this differs from #24407's automated keep-open verdict, but the recorded diff shows canonical-history mutation and the same fix must be relocated to the API-time path.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup24407 ["PRs duplicating each other"]
        P24407["PR #24407 (open)"]
        P70107["PR #70107 (open)"]
    end
    class P24407 open
    class P70107 open
    class P70107 target
    click P24407 "https://github.com/NousResearch/hermes-agent/pull/24407"
    click P70107 "https://github.com/NousResearch/hermes-agent/pull/70107"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 5 kB of issue/PR text, 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

On Invalid signature in thinking block recovery, Hermes previously stripped
only reasoning_details from API-time message copies but retained
anthropic_content_blocks — a separate ordered-block channel that can replay
the same invalid signed thinking block, causing the retry to 400 again.

This clears both fields from api_messages while leaving the canonical messages
store untouched (session history is preserved).

Adds a real-path test that exercises convert_messages_to_anthropic() to verify
the ordered-block replay fast path does not fire after recovery stripping,
while canonical messages retain both fields.

Addresses review feedback on NousResearch#70107 from @teknium1 and triage bot.
@Eklps
Eklps force-pushed the fix/sig-recovery-ordered-blocks branch from fca586e to e78c9f6 Compare August 4, 2026 04:52
@Eklps

Eklps commented Aug 4, 2026

Copy link
Copy Markdown
Author

Updated with real-path test coverage as requested by @teknium1 and triage review.

The new test_recovery_strips_ordered_blocks_from_retry_wire_payload exercises the production path:

  1. Before recovery: builds assistant message with both reasoning_details and anthropic_content_blocks (signed thinking + tool_use), then calls convert_messages_to_anthropic() — confirms the ordered-block fast path fires and replays signed thinking.
  2. After recovery: shallow-copies to api_messages, strips both fields, calls convert_messages_to_anthropic() again — confirms the ordered-block fast path does NOT fire (no thinking block in output), while tool_use survives via reconstruction from tool_calls.
  3. Canonical messages: verifies both fields are still intact on the original messages list.

Rebased onto current upstream/main (70db671). All 5 tests pass.

@alt-glitch alt-glitch removed the sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/anthropic Anthropic native Messages API sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants