Skip to content

fix(agent): strip thinking blocks from content arrays on signature recovery - #24407

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/issue-24401-thinking-block-recovery
Open

fix(agent): strip thinking blocks from content arrays on signature recovery#24407
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/issue-24401-thinking-block-recovery

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

The thinking-signature recovery path only stripped the top-level reasoning_details field from messages, but Anthropic thinking blocks live inside assistant message content arrays. After recovery, convert_messages_to_anthropic() on the retry kept the signed thinking blocks from the last assistant message, causing the same HTTP 400 "Invalid signature in thinking block" error on every retry attempt.

Root Cause

When a user switches from a provider that emits thinking blocks with its own signatures (e.g. MiniMax via Anthropic transport) to Anthropic proper, the message history contains thinking blocks with MiniMax-specific signatures. Anthropic rejects these with HTTP 400. The recovery code at run_agent.py (thinking_signature branch) only did:

_m.pop("reasoning_details", None)

But the thinking blocks are in _m["content"] as a list of content blocks, not in reasoning_details. After the continue, the retry goes through convert_messages_to_anthropic() which re-processes messages and keeps signed thinking blocks from the last assistant message — the same invalid signatures that caused the original error.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

…covery

The thinking-signature recovery path (FailoverReason.thinking_signature)
only stripped the top-level reasoning_details field from messages. But
Anthropic thinking blocks live inside assistant message content arrays.
After the recovery, convert_messages_to_anthropic() on the retry kept
the signed thinking blocks from the last assistant message, causing the
same HTTP 400 'Invalid signature in thinking block' error on every retry.

This is especially visible when switching from a provider that emits
thinking blocks with its own signatures (e.g. MiniMax via Anthropic
transport) to Anthropic proper — the MiniMax signatures are invalid for
Anthropic and the recovery never actually removed them.

Fix: also strip thinking/redacted_thinking blocks from assistant message
content arrays during recovery. If all content was thinking, insert a
placeholder. Add 7 regression tests covering the recovery logic.

Fixes [Bug]: cross-provider switch from MiniMax to Anthropic fails with
Invalid signature in thinking block NousResearch#24401
@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 May 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the content-array variant of this recovery gap. The premise still holds on current main: agent/conversation_loop.py:2843-2846 strips only reasoning_details, while agent/anthropic_adapter.py:1982-1985 converts assistant content arrays and :2311-2344 preserves signed thinking on the latest direct-Anthropic assistant turn.

Problems

  • The current retry wire list is api_messages, created as shallow copies at agent/conversation_loop.py:787-830. Current recovery intentionally leaves canonical messages untouched (:2819-2836). The PR's messages mutation needs to be moved to api_messages; otherwise it risks persisted-history corruption and does not match the current retry architecture.
  • tests/agent/test_thinking_signature_recovery.py only reimplements the proposed loop. It does not verify the actual retry or convert_messages_to_anthropic() output.

Suggested changes

  • Filter thinking and redacted_thinking blocks by rebinding api_messages[*]["content"] in the current recovery branch.
  • Add a real-path retry test proving the retry payload is clean and canonical history is unchanged.

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:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Two PRs address the repeated HTTP 400 caused by replaying invalid signed Anthropic thinking state. #24407 filters thinking blocks from assistant content arrays but mutates canonical history, whereas #70107 removes both replay channels from API-time copies while preserving canonical messages.

Related pull requests

  • #24407 related — (+145/-7) — superseded / needs rework: The diff removes thinking and redacted-thinking blocks from canonical messages, directly addressing the reported content-array signature failure but risking persisted-history corruption; its tests only reproduce the loop rather than exercising the actual retry and Anthropic conversion path. Despite the keep_open review on #24407, that contributor review itself identifies these blocking issues and requires moving the fix to api_messages.
  • #70107 duplicate — (+48/-10) — preferred fix: The diff updates the current recovery path to clear both reasoning_details and the ordered anthropic_content_blocks replay channel from shallow-copied api_messages, preventing the invalid signature from being resent while leaving canonical history unchanged.

Duplicates

#24407 and #70107 target the same signature-recovery retry failure and substantially overlap in intent; #70107 implements the fix against the current API-time replay architecture.

Suggested consolidation

Merge #70107 as the narrower current-architecture fix; #24407 can be closed as superseded because its present diff mutates canonical messages and lacks a real-path retry/conversion test, notwithstanding its keep_open review.

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 P24407 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 or no verify verdict yet (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, 1 kB of discussion (1 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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