fix(agents): strip thinking blocks with invalid signatures before API replay by castaples · Pull Request #70054 · openclaw/openclaw

castaples · 2026-04-22T07:46:54Z

Rebase of #45072 onto current main, scoped down to the prevention piece so it can land independently. Original work by @4ier (credited via Co-authored-by trailer).

Summary

Adds stripInvalidThinkingSignatures() to src/agents/pi-embedded-runner/thinking.ts. Strips assistant thinking blocks whose thinkingSignature is missing, empty, or non-string — the cases where Bedrock streaming has been observed to persist a corrupt block to the session JSONL transcript.
Wires it into sanitizeSessionHistory() in src/agents/pi-embedded-runner/replay-history.ts (this is where fix(agents): strip thinking blocks with invalid signatures before API replay #45072's google.ts change moved to in the file restructure that happened after that PR was opened). Runs whenever policy.preserveSignatures is true (Anthropic / Bedrock path), before the existing dropThinkingBlocks step.
Adds isInvalidThinkingSignatureError() so a future PR can wrap the runner stream with a retry-on-API-rejection safety net using the existing wrapAnthropicStreamWithRecovery pattern.
Tests in thinking.test.ts cover the empty / missing / mixed-validity / non-assistant cases plus the error-matcher.

Why this fixes #45010

The reproducible failure on Bedrock + Claude Opus 4.6 + extended thinking is:

Bedrock's converse-stream provider returns a thinking block with thinkingSignature: \"\".
The runner persists it verbatim to the session JSONL.
Every subsequent turn replays the corrupt block to the Anthropic API, which rejects with Invalid signature in thinking block.
The session is permanently poisoned — no path to recover except /reset.

Stripping at replay-sanitisation time prevents step 3 from ever sending the bad block to the API. Both newly-corrupted sessions and existing damaged ones recover automatically on the next turn.

What this PR deliberately does NOT include from #45072

The original PR added a defense-in-depth retry by threading a new forceDropThinkingBlocks flag through run.ts, run/attempt.ts, and run/types.ts. Those files have changed substantially since #45072 was opened, and wrapAnthropicStreamWithRecovery (introduced after #45072) provides the same retry shape with less plumbing. Splitting prevention from retry keeps this PR small and avoids re-doing diverged plumbing — retry can ride on top of the wrapper in a follow-up that also covers THINKING_BLOCK_ERROR_PATTERN-style errors.

Bot feedback from #45072

Both review comments (greptile + codex) on the original PR were about MIN_VALID_SIGNATURE_LENGTH = 10 being too permissive. @4ier addressed them in c999982 by removing the length heuristic entirely and treating the API as the authoritative validator. Those resolutions are incorporated here — the implementation only checks for empty / missing / non-string signatures, never length thresholds, and the test for a "valid" signature uses \"b\".repeat(356) to match the documented real-signature lower bound.

Local verification

pnpm exec tsgo --noEmit -p tsconfig.json — clean
pnpm exec oxlint <touched files> — 0 warnings, 0 errors
pnpm exec oxfmt --check <touched files> — formatted
Vitest run was attempted locally on Windows but test/setup-openclaw-runtime.ts:372 failed with Vitest failed to find the runner for all 381 tests in the agents project — looks like a local/Windows env issue unrelated to this change. Relying on CI (which was green for these files in fix(agents): strip thinking blocks with invalid signatures before API replay #45072) to confirm.

Test plan

CI green on Linux + Windows + macOS shards
Greptile / Codex review surface no new findings
Maintainer to confirm the scope split (prevention here, retry follow-up) is the preferred shape

Closes #45010
Supersedes #45072

Made with Cursor

AI-assisted PR disclosure

Per CONTRIBUTING.md:

AI-assisted: Authored with assistance from an LLM coding agent (Cursor + Claude). The original PR fix(agents): strip thinking blocks with invalid signatures before API replay #45072 was authored by @4ier (credited via Co-authored-by trailer); this PR is a manual rebase of that work onto current main with the scope reduction described above. All edits, the rebase strategy, and the bot-feedback responses were reviewed and approved by the human submitter.
Degree of testing: Lightly tested. pnpm exec tsgo --noEmit -p tsconfig.json, pnpm exec oxlint, and pnpm exec oxfmt --check are clean for the touched files. Local Vitest run was attempted but test/setup-openclaw-runtime.ts:372 failed with Vitest failed to find the runner for all 381 agents-project tests on Windows — appears to be a local env issue unrelated to this change. Relying on CI (currently 92 SUCCESS / 8 SKIPPED / 0 FAILURE) for full test validation. Production workaround in our downstream deployment confirms the empty-signature symptom and that boundary-side stripping mitigates it; the upstream fix here is the canonical version.
Codex review: GitHub Codex review did not trigger automatically on this PR. Ran codex review --base upstream/main locally (codex-cli 0.122.0) and it caught a P1 regression in my own implementation — stripInvalidThinkingSignatures was missing the snake_case thought_signature field that sanitizeSessionMessagesImages preserves, which would have silently erased valid Claude-signed reasoning turns. Fixed in e073423 with new tests; re-running codex on the fix confirms no remaining actionable findings. Greptile also reviewed (5/5 confidence, all findings addressed).
I understand what the code does: stripInvalidThinkingSignatures walks assistant messages in the replay history and removes thinking / redacted_thinking blocks whose signature field is missing, empty, or non-string. It runs in sanitizeSessionHistory only when policy.preserveSignatures is true (Anthropic / Bedrock providers), and runs before dropThinkingBlocks so the latest-assistant-message preservation in dropThinkingBlocks doesn't reintroduce corrupt blocks. Returns the original array reference when nothing changed (caller can use reference equality to skip work). The retry helper isInvalidThinkingSignatureError is additive and not yet wired — left for a follow-up that uses wrapAnthropicStreamWithRecovery to retry on API rejection.

… replay When Bedrock returns thinking blocks with empty thinkingSignature, they get persisted to the session JSONL transcript. On subsequent turns, the Anthropic API rejects these with 'Invalid signature in thinking block', causing the session to become unrecoverable without /reset. Add stripInvalidThinkingSignatures() that filters out thinking blocks with missing, empty, or non-string signatures during transcript sanitization, before messages are sent to the API. Wire it into sanitizeSessionHistory in replay-history.ts so it runs whenever preserveSignatures is true (Anthropic/Bedrock providers). Also adds isInvalidThinkingSignatureError() so callers can detect the specific Anthropic rejection and apply dropThinkingBlocks + retry as a follow-up safety net (left for a future PR — replay-side prevention should fix openclaw#45010 on its own for the empty-signature case). This is a rebase of openclaw#45072 onto current main, with two changes: - The original PR routed retry logic through run.ts/attempt.ts/types.ts. That plumbing has diverged significantly. Splitting prevention from retry keeps this PR scoped to the replay-history change so it can land independently; retry can ride on top of the new wrapAnthropicStreamWithRecovery wrapper (introduced after openclaw#45072 was opened) in a follow-up. - google.ts has been renamed to replay-history.ts since openclaw#45072 was opened; the wire-in target moved with it. Fixes openclaw#45010 Co-authored-by: Fourier <[email protected]> Made-with: Cursor

greptile-apps · 2026-04-22T07:49:39Z

Greptile Summary

This PR adds stripInvalidThinkingSignatures() to remove assistant thinking blocks with empty, missing, or non-string thinkingSignature from session history before API replay, fixing a Bedrock-specific bug (#45010) where empty signatures persisted to JSONL would permanently poison subsequent turns. The new function is correctly gated behind policy.preserveSignatures in sanitizeSessionHistory and runs before dropThinkingBlocks, ensuring the latest assistant message's corrupt blocks are cleaned up before dropThinkingBlocks preserves them.

Confidence Score: 5/5

Safe to merge — targeted, well-tested fix with correct ordering and reference-equality optimisation.

All findings are P2 (style/edge-case notes). The core logic is correct: empty/missing signatures are stripped, valid signatures and redacted_thinking blocks are left untouched, the policy.preserveSignatures guard scopes the fix to the right providers, and the synthetic fallback preserves turn structure. Tests cover the important paths.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/thinking.ts
Line: 89

Comment:
**`redacted_thinking` blocks with invalid fields not covered**

`stripInvalidThinkingSignatures` only processes `type === "thinking"` blocks and passes `redacted_thinking` blocks through unconditionally. `redacted_thinking` blocks carry a `signature` field (not `thinkingSignature`) and `isSignedThinkingBlock` treats them as always-signed (line 43: `record.type === "redacted_thinking"` is sufficient). If Bedrock ever returns a `redacted_thinking` block with an empty or missing `signature`, this function would silently forward the corrupt block to the API. This is a narrow edge-case today but worth noting if the validation scope expands.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(agents): strip thinking blocks with ..." | Re-trigger Greptile}

…gnatures Address Greptile review feedback on openclaw#70054. The original implementation only checked `type === "thinking"` blocks and would silently forward any `type === "redacted_thinking"` block with an empty/missing `signature` field to the API. Bedrock has not been observed corrupting `redacted_thinking.signature` in practice (the original openclaw#45010 report is specifically about `thinkingSignature` on `thinking` blocks), but the cost of covering both shapes is trivial and matches `isSignedThinkingBlock`'s view that both block types carry a signature worth validating. Adds three tests for the redacted_thinking shape: empty signature, missing signature, and valid signature (preserved). Made-with: Cursor

castaples · 2026-04-22T08:12:36Z

Addressed in 540a031 — stripInvalidThinkingSignatures now also strips redacted_thinking blocks with empty/missing/non-string signature field. Same pattern, same conservative "only the obvious cases" stance (no length heuristics). Tests added for the three shapes (empty, missing, valid).

…alidThinkingSignatures Local `codex review --base upstream/main` caught a P1 regression: the previous implementation only checked `thinkingSignature` and `signature`, but `isSignedThinkingBlock` (and the rest of the runner) also recognises the snake_case `thought_signature` field that `sanitizeSessionMessagesImages` preserves when `preserveSignatures: true`. Anthropic / Bedrock replay histories that carry valid signatures only in `thought_signature` would have been classified as unsigned and stripped, silently erasing the latest signed reasoning turn and breaking immutable thinking replay on the next request — exactly the data-loss class of bug this helper exists to prevent. Fix: - Extract a `hasNonEmptyStringSignature` helper that checks all three recognised signature field names with union semantics. A block is signed if ANY of `signature` / `thinkingSignature` / `thought_signature` is a non-empty string. A block is stripped only when ALL of them are missing, empty, or non-string. - Use the existing `isThinkingBlock` predicate so the type-gating logic stays in one place and stays in sync if the set of thinking block types ever expands. Tests cover: - thinking blocks signed only via `thought_signature` (preserved) - redacted_thinking blocks signed only via `thought_signature` (preserved) - thinking blocks where all signature fields are empty (stripped) - thinking blocks where one signature field is empty but another is valid (preserved — belt-and-braces) Made-with: Cursor

castaples · 2026-04-22T08:45:30Z

Local codex review --base upstream/main finding addressed (e073423)

Per CONTRIBUTING.md ("If you have access to Codex, run codex review --base origin/main locally"), I've now run codex against this PR. It surfaced one P1 regression in my own changes that the GitHub Codex bot didn't trigger to catch:

The new signature-stripping logic drops valid preserved thinking blocks whenever they use the existing thought_signature field, so Anthropic/Bedrock replay can be corrupted by this change.

Codex was right. isSignedThinkingBlock recognises three signature fields (signature, thinkingSignature, thought_signature) and sanitizeSessionMessagesImages({ preserveSignatures: true }) keeps the snake_case form intact. My implementation only checked two of them, which would have classified thought_signature-only blocks as unsigned and stripped them — silently erasing valid Claude-signed reasoning turns. Exactly the data-loss class of bug we're trying to prevent.

Fix:

Extracted hasNonEmptyStringSignature helper that matches isSignedThinkingBlock's union semantics across all three field names.
A block is now stripped only when ALL recognised signature fields are missing, empty, or non-string.
Added four new tests: thought_signature-only thinking, thought_signature-only redacted_thinking, all-fields-empty (stripped), and mixed-fields (preserved when at least one is valid).

Local validation re-run on the new code: tsgo --noEmit clean, oxlint clean, oxfmt --check clean. Re-running codex review on the fix confirms no remaining actionable findings.

Updating my "Codex review" disclosure box in the PR body accordingly.

prtags · 2026-04-23T18:19:12Z

Related work from PRtags group current-gecko-gpfw

Title: Open PR candidate: invalid thinking signature replay fix rebased and scope-reduced

Number	Title
#45072	fix(agents): strip thinking blocks with invalid signatures before API replay
#70054*	fix(agents): strip thinking blocks with invalid signatures before API replay

* This PR

Fixes #45010. Supersedes #70054. Co-authored-by: Chris Staples <[email protected]> Co-authored-by: Fourier <[email protected]>

steipete · 2026-04-25T19:13:17Z

Codex maintainer follow-up: thanks @castaples, this was the right bug class and the PR gave us the useful shape.

I landed the fix directly on main in 791ad08, with co-author credit preserved for you and Fourier.

What changed from this PR before landing:

reused the existing non-empty [assistant reasoning omitted] fallback instead of inserting blank text, because both Anthropic and Bedrock reject blank assistant text blocks
treated whitespace-only signatures as invalid
kept the sanitizer conservative: it strips only missing/empty/blank/non-string signatures and leaves opaque non-empty signatures for the provider to validate
added helper tests plus sanitizeSessionHistory integration coverage for Anthropic and Bedrock replay
updated transcript hygiene docs and changelog

Live verification before landing:

Anthropic direct API rejects empty/blank thinking signatures with Invalid signature in thinking block
Anthropic accepts the non-empty omitted-reasoning fallback
Bedrock Converse on us.anthropic.claude-sonnet-4-6 rejects an empty reasoningText.signature with the same invalid-signature error
Bedrock accepts the non-empty omitted-reasoning fallback

Closing this PR as superseded by the landed commit. Thank you for the report, rebase, and test direction here.

Fixes openclaw#45010. Supersedes openclaw#70054. Co-authored-by: Chris Staples <[email protected]> Co-authored-by: Fourier <[email protected]>

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Apr 22, 2026

greptile-apps Bot reviewed Apr 22, 2026

View reviewed changes

Comment thread src/agents/pi-embedded-runner/thinking.ts Outdated

castaples mentioned this pull request Apr 22, 2026

Invalid signature in thinking block: empty thinkingSignature from Bedrock causes unrecoverable session errors #45010

Closed

prtags Bot mentioned this pull request Apr 23, 2026

fix(agents): strip thinking blocks with invalid signatures before API replay #45072

Closed

steipete added a commit that referenced this pull request Apr 25, 2026

fix: strip invalid thinking replay signatures

791ad08

Fixes #45010. Supersedes #70054. Co-authored-by: Chris Staples <[email protected]> Co-authored-by: Fourier <[email protected]>

steipete closed this Apr 25, 2026

yuxiaoyang2007-prog mentioned this pull request Apr 28, 2026

[Bug] thought_signature 400 regression in 2026.4.25 – fix was supposedly in 2026.4.24 #72879

Closed

clawsweeper Bot mentioned this pull request Apr 30, 2026

feat: Strip thinking blocks from history except last assistant message (token savings) #45016

Closed

ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026

fix: strip invalid thinking replay signatures

dfb5829

Fixes openclaw#45010. Supersedes openclaw#70054. Co-authored-by: Chris Staples <[email protected]> Co-authored-by: Fourier <[email protected]>

denissutherland52 mentioned this pull request May 15, 2026

OpenRouter passthrough: literal field-name string written to thinkingSignature, breaks 2nd turn on reasoning models (Anthropic, xAI) #82335

Closed

tweetodavid mentioned this pull request May 26, 2026

Bug: Empty thinking-block signatures poison Anthropic Messages sessions (fix from #70054 not applied to standard Anthropic transport) #86886

Closed

Kailigithub mentioned this pull request May 27, 2026

fix: strip invalid thinking signatures for all anthropic-messages sessions #87138

Closed

xydigit-sj mentioned this pull request May 29, 2026

fix(agents): downgrade thinking blocks with empty signatures to text before anthropic-messages replay #87963

Closed

sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026

fix: strip invalid thinking replay signatures

af4a2eb

Fixes openclaw#45010. Supersedes openclaw#70054. Co-authored-by: Chris Staples <[email protected]> Co-authored-by: Fourier <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): strip thinking blocks with invalid signatures before API replay#70054

fix(agents): strip thinking blocks with invalid signatures before API replay#70054
castaples wants to merge 3 commits into
openclaw:mainfrom
castaples:fix/45010-strip-invalid-thinking-signatures-rebased

castaples commented Apr 22, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Uh oh!

Uh oh!

castaples commented Apr 22, 2026

Uh oh!

castaples commented Apr 22, 2026

Uh oh!

prtags Bot commented Apr 23, 2026

Uh oh!

steipete commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

castaples commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this fixes #45010

What this PR deliberately does NOT include from #45072

Bot feedback from #45072

Local verification

Test plan

AI-assisted PR disclosure

Uh oh!

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

castaples commented Apr 22, 2026

Uh oh!

castaples commented Apr 22, 2026

Uh oh!

prtags Bot commented Apr 23, 2026

Uh oh!

steipete commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

castaples commented Apr 22, 2026 •

edited

Loading