diff --git a/amplifier_module_provider_anthropic/__init__.py b/amplifier_module_provider_anthropic/__init__.py index a12eb55..e32257f 100644 --- a/amplifier_module_provider_anthropic/__init__.py +++ b/amplifier_module_provider_anthropic/__init__.py @@ -230,6 +230,21 @@ async def _get_process_semaphore(max_concurrent: int) -> asyncio.Semaphore | Non BETA_HEADER_FAST_MODE = "fast-mode-2026-02-01" PROVIDER_FALLBACK_OPEN = "provider:fallback_open" PROVIDER_FALLBACK_ACTIVE = "provider:fallback_active" + +# Emitted by _sanitize_thinking_blocks (amplifier-support#207) whenever a +# thinking/redacted_thinking block with an invalid or missing signature is +# stripped from outgoing history. Kept as a module-level constant so tests +# and other observers can reference it instead of a magic string. +PROVIDER_THINKING_SIGNATURE_STRIPPED = "provider:thinking_signature_stripped" + +# Neutral, non-empty placeholder used by _sanitize_thinking_blocks when +# stripping invalid thinking blocks would otherwise leave an assistant +# message with an empty content array -- Anthropic rejects empty content +# arrays just as strictly as it rejects unsigned thinking blocks. +_THINKING_PLACEHOLDER_BLOCK: dict[str, str] = { + "type": "text", + "text": "[thinking omitted: signature unavailable after provider switch]", +} FALLBACK_STATE_VERSION = 1 # --------------------------------------------------------------------------- @@ -2205,6 +2220,15 @@ async def _complete_chat_request( # Combine: context THEN conversation all_messages = context_user_msgs + conversation_msgs + # DEFENSIVE: strip thinking blocks with invalid/missing signatures before + # anything else touches the final message list. See _sanitize_thinking_blocks + # for the full rationale (fixes amplifier-support#207). Run this BEFORE + # cache control so the cache breakpoint lands on the truly-final content + # array (including any placeholder inserted here), not on a block that + # gets stripped a moment later. + all_messages = await self._sanitize_thinking_blocks( + all_messages, model=kwargs.get("model", self.default_model) + ) # Apply cache control to last message for incremental context caching all_messages = self._apply_message_cache_control(all_messages) logger.info(f"[PROVIDER] Final message count for API: {len(all_messages)}") @@ -3551,6 +3575,148 @@ def _convert_messages(self, messages: list[dict[str, Any]]) -> list[dict[str, An return anthropic_messages + async def _sanitize_thinking_blocks( + self, messages: list[dict[str, Any]], model: str | None = None + ) -> list[dict[str, Any]]: + """Strip thinking/redacted_thinking blocks with invalid signatures. + + Fixes amplifier-support#207. + + Anthropic strict-validates ``thinking.signature`` as a non-empty + string on EVERY replay request. Sessions that switch providers + mid-conversation (e.g. some turns handled by provider-chat-completions + or provider-openai, interleaved with anthropic turns) can persist + thinking blocks whose signature is ``null``, absent entirely, or + otherwise not something Anthropic minted -- the signing/verification + scheme is provider-specific and cannot be retrofitted onto a block + another provider produced. A single such block anywhere in history is + enough for Anthropic to reject the ENTIRE request, e.g.:: + + messages.51.content[0].thinking.signature.str: Input should be a valid string + + ...which bricks the session on every subsequent resume attempt, even + though only one of many messages is at fault. + + This is a defensive, chokepoint pass over the fully-assembled message + list. It runs ONCE in ``_complete_chat_request`` on ``all_messages``, + which both the streaming transport (``client.messages.stream``) and + the non-streaming transport (``client.messages.with_raw_response.create``) + consume via the same ``params`` dict -- as does the refusal-fallback + retry path. One sanitize pass here covers all three. + + Two malformed shapes are handled by the SAME check (both reduce to + "signature is not a valid non-empty string" once read via + ``dict.get``, which returns None whether a key is missing or present + with a None value): + (a) ``{"type": "thinking", "thinking": "...", "signature": None}`` + -- produced when provider-chat-completions round-trips a + thinking block through its own (non-Anthropic) history format. + (b) ``{"type": "thinking", "content": ["", "rs_..."]}`` + with no ``signature`` key at all -- provider-openai's Responses + API persists encrypted reasoning content plus a reasoning-item + id instead of an Anthropic-style signature. + + ``redacted_thinking`` blocks are left alone unless some upstream + producer attached an (invalid) ``signature`` key to one -- Anthropic + doesn't require a signature on redacted_thinking at all; its ``data`` + field is the whole payload. + + Non-dict entries in a content array (e.g. a raw string surviving a + corrupted/partial transcript) are tolerated, not dropped: this method + only removes blocks it can positively identify as invalid thinking + blocks, and must never raise on malformed input it merely can't + interpret. + + Args: + messages: Anthropic-formatted messages -- the output of + ``_convert_messages``, i.e. plain ``role``/``content`` dicts + that have already been run through ``_clean_content_block``. + model: The model this request targets, used only for the warning + log / observability event. Falls back to ``self.default_model`` + when the caller didn't override the model via kwargs. + + Returns: + The same list, mutated in place. Assistant messages have any + invalid thinking/redacted_thinking blocks removed. If stripping + would leave a message's content array empty, a minimal + placeholder text block is inserted instead -- Anthropic rejects + empty content arrays just as strictly as it rejects unsigned + thinking blocks. + """ + total_stripped = 0 + + for msg in messages: + if msg.get("role") != "assistant": + continue + content = msg.get("content") + if not isinstance(content, list): + continue + + kept: list[Any] = [] + msg_stripped = 0 + for block in content: + if isinstance(block, dict) and self._is_invalid_thinking_block(block): + msg_stripped += 1 + continue + kept.append(block) + + if not msg_stripped: + continue + + if not kept: + # Anthropic rejects empty content arrays just as strictly as it + # rejects unsigned thinking blocks -- insert a neutral, + # non-empty placeholder so the turn stays a well-formed (if + # content-free) assistant message rather than an invalid one. + kept.append(dict(_THINKING_PLACEHOLDER_BLOCK)) + + msg["content"] = kept + total_stripped += msg_stripped + + if total_stripped: + # One aggregate warning + event per request (matching the + # tool_sequence_repaired precedent), not one per affected message -- + # a session with many mid-conversation provider switches could + # otherwise spam the log/event bus with one line per turn. + effective_model = model or self.default_model + logger.warning( + "[PROVIDER] Stripped %d thinking block(s) with invalid/missing " + "signature from assistant message history before sending to " + "Anthropic (model=%s). This is expected after a session " + "switched providers mid-conversation; see amplifier-support#207.", + total_stripped, + effective_model, + ) + await self._emit_provider_event( + PROVIDER_THINKING_SIGNATURE_STRIPPED, + { + "provider": self.name, + "model": effective_model, + "stripped_count": total_stripped, + }, + ) + + return messages + + @staticmethod + def _is_invalid_thinking_block(block: dict[str, Any]) -> bool: + """True if `block` is a thinking-family block Anthropic will reject. + + Anthropic requires `signature` to be a non-empty string on `thinking` + blocks. `redacted_thinking` blocks don't normally carry a signature at + all (their `data` field is the whole payload) -- only treat one as + invalid if some upstream producer attached a `signature` key to it and + that value fails the same non-empty-string check. + """ + block_type = block.get("type") + if block_type == "thinking": + signature = block.get("signature") + return not isinstance(signature, str) or not signature.strip() + if block_type == "redacted_thinking" and "signature" in block: + signature = block.get("signature") + return not isinstance(signature, str) or not signature.strip() + return False + def _convert_tools_from_request(self, tools: list) -> list[dict[str, Any]]: """Convert ToolSpec objects from ChatRequest to Anthropic format. diff --git a/tests/test_thinking_sanitization.py b/tests/test_thinking_sanitization.py new file mode 100644 index 0000000..4f56fd3 --- /dev/null +++ b/tests/test_thinking_sanitization.py @@ -0,0 +1,518 @@ +"""Tests for defensive sanitization of invalid thinking-block signatures. + +Fixes microsoft-amplifier/amplifier-support#207. + +Background +---------- +Anthropic strict-validates the ``signature`` field of every ``thinking`` +content block it receives as history: it must be a non-empty string. A +session that switches providers mid-conversation (e.g. some turns handled by +provider-chat-completions or provider-openai, interleaved with anthropic +turns) can persist thinking blocks whose signature is ``null``, absent +entirely, or otherwise not something Anthropic minted -- the signing scheme +is provider-specific and cannot be retrofitted. A single such block anywhere +in history causes Anthropic to 400 the *entire* request with e.g.:: + + messages.51.content[0].thinking.signature.str: Input should be a valid string + +which bricks the session on every future resume attempt. + +``AnthropicProvider._sanitize_thinking_blocks`` is a defensive pass over the +fully-assembled ``params["messages"]`` list (run once in +``_complete_chat_request``, so it covers the streaming transport, the +non-streaming transport, and the refusal-fallback retry -- they all consume +the same params dict) that strips any thinking block it cannot positively +verify as Anthropic-signed, replacing an emptied message's content with a +neutral placeholder rather than sending an empty array (which Anthropic also +rejects). + +Two malformed shapes are covered (see issue #207 for the real-world repro): + (a) ``{"type": "thinking", "thinking": "...", "signature": None}`` -- + round-tripped through provider-chat-completions' own history format. + (b) ``{"type": "thinking", "content": ["", "rs_..."]}`` with no + ``signature`` key at all -- provider-openai's Responses API persists + encrypted reasoning + a reasoning-item id instead of a signature. + +Both shapes reduce to the same check once evaluated with ``dict.get`` (a +missing key and an explicit ``None`` value are indistinguishable to +``.get()``), which is why one filter handles both. Note that when shape (a) +or (b) is constructed via the real ``ThinkingBlock``/``Message`` pydantic +models (as most tests below do, to stay on the same end-to-end path real +history takes), ``model_dump()`` always serializes the ``signature`` field +(defaulting to ``None``) -- so the "signature key entirely absent from the +dict" nuance can only be observed by calling ``_sanitize_thinking_blocks`` +directly with a hand-built plain dict, which a couple of tests do below to +prove the implementation uses ``.get()`` (never bare indexing, which would +KeyError on truly-missing keys). +""" + +import asyncio +import logging +from typing import Any, cast +from unittest.mock import AsyncMock, MagicMock + +from amplifier_core import ModuleCoordinator +from amplifier_core.message_models import ( + ChatRequest, + Message, + RedactedThinkingBlock, + TextBlock, + ThinkingBlock, + ToolCallBlock, +) +from amplifier_module_provider_anthropic import AnthropicProvider + +from tests._helpers import DummyResponse, FakeCoordinator + + +# --------------------------------------------------------------------------- +# Helpers (mirrors tests/test_reasoning_effort.py style) +# --------------------------------------------------------------------------- + + +def _make_provider( + default_model: str = "claude-sonnet-4-5-20250929", +) -> AnthropicProvider: + provider = AnthropicProvider( + api_key="test-key", + config={ + "use_streaming": False, + "max_retries": 0, + "default_model": default_model, + }, + ) + provider.coordinator = cast(ModuleCoordinator, FakeCoordinator()) + return provider + + +def _make_raw_mock() -> MagicMock: + raw = MagicMock() + raw.parse.return_value = DummyResponse() + raw.headers = {} + return raw + + +def _wire_mock(provider: AnthropicProvider) -> AsyncMock: + mock = AsyncMock(return_value=_make_raw_mock()) + provider.client.messages.with_raw_response.create = mock + return mock + + +def _get_api_params(mock_create: AsyncMock) -> dict[str, Any]: + """Extract the kwargs actually sent to the (mocked) Anthropic SDK call. + + This is the payload-level assertion surface: it's the same dict that + would have been serialized onto the wire, so asserting on it here is + the e2e check for a provider module (no live API call needed). + """ + assert mock_create.await_count == 1 + _, kwargs = mock_create.call_args + return kwargs + + +def _assistant_messages(params: dict[str, Any]) -> list[dict[str, Any]]: + return [m for m in params["messages"] if m.get("role") == "assistant"] + + +# --------------------------------------------------------------------------- +# (a) shape (a): signature explicitly null -> stripped +# --------------------------------------------------------------------------- + + +def test_null_signature_thinking_block_is_stripped_from_outgoing_payload(): + """A thinking block with signature=None must never reach the API.""" + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock(thinking="internal reasoning", signature=None), + TextBlock(text="here is my answer"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + assert len(assistant_msgs) == 1 + + block_types = [b["type"] for b in assistant_msgs[0]["content"]] + assert "thinking" not in block_types + assert block_types == ["text"] # bad thinking gone, text preserved + + +# --------------------------------------------------------------------------- +# (b) shape (b): OpenAI-Responses-API-shaped block (content list, no +# meaningful signature) -> stripped +# --------------------------------------------------------------------------- + + +def test_reasoning_shaped_thinking_block_with_no_signature_is_stripped(): + """A thinking block carrying opaque cross-provider `content` (encrypted + payload + reasoning-item id) instead of a real signature must be + stripped, exactly like shape (a). + """ + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock( + thinking="", + content=["gAAAAAB123encryptedblob", "rs_abc123"], + ), + TextBlock(text="answer text"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + block_types = [b["type"] for b in assistant_msgs[0]["content"]] + assert "thinking" not in block_types + assert block_types == ["text"] + + +def test_sanitize_directly_handles_truly_absent_signature_key(): + """Direct unit test of ``_sanitize_thinking_blocks`` with a plain dict + where the ``signature`` key is genuinely absent (not just None) -- the + one nuance that can't be produced via the pydantic Message/ThinkingBlock + models, since model_dump() always serializes declared fields. Proves + the implementation reads the field defensively (``.get``), not via + direct indexing (which would KeyError here). + """ + provider = _make_provider() + + messages = [ + { + "role": "assistant", + "content": [ + { + "type": "thinking", + "content": ["gAAAAAB123encryptedblob", "rs_abc123"], + # NOTE: no "signature" key at all. + }, + {"type": "text", "text": "answer text"}, + ], + } + ] + + sanitized = asyncio.run(provider._sanitize_thinking_blocks(messages)) + + block_types = [b["type"] for b in sanitized[0]["content"]] + assert "thinking" not in block_types + assert block_types == ["text"] + + +# --------------------------------------------------------------------------- +# Valid signed thinking blocks are left untouched +# --------------------------------------------------------------------------- + + +def test_valid_signed_thinking_block_is_preserved_verbatim(): + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock( + thinking="valid internal reasoning", + signature="a-real-anthropic-signature", + ), + TextBlock(text="answer text"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + thinking_blocks = [ + b for b in assistant_msgs[0]["content"] if b["type"] == "thinking" + ] + assert len(thinking_blocks) == 1 + assert thinking_blocks[0]["thinking"] == "valid internal reasoning" + assert thinking_blocks[0]["signature"] == "a-real-anthropic-signature" + + # Order preserved: thinking first, then text. + assert [b["type"] for b in assistant_msgs[0]["content"]] == [ + "thinking", + "text", + ] + + +# --------------------------------------------------------------------------- +# Emptied message gets a placeholder, never an empty content array +# --------------------------------------------------------------------------- + + +def test_message_with_only_invalid_thinking_block_gets_placeholder(): + """Anthropic rejects empty content arrays just as strictly as it rejects + unsigned thinking blocks. If stripping leaves nothing behind, a minimal + non-empty placeholder block must be inserted instead. + """ + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ThinkingBlock(thinking="orphaned reasoning", signature=None)], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + content = assistant_msgs[0]["content"] + + assert content != [] + assert len(content) >= 1 + assert content[0]["type"] == "text" + assert isinstance(content[0]["text"], str) and content[0]["text"].strip() + + +# --------------------------------------------------------------------------- +# Mixed message: only the bad thinking block is removed; other blocks +# (tool calls, text) are untouched and stay in order. +# --------------------------------------------------------------------------- + + +def test_only_invalid_thinking_block_removed_tool_calls_and_text_intact(): + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Do the thing"), + Message( + role="assistant", + content=[ + ThinkingBlock(thinking="planning...", signature=None), + TextBlock(text="I'll call a tool"), + ToolCallBlock(id="call_1", name="do_something", input={"x": 1}), + ], + ), + Message( + role="tool", + tool_call_id="call_1", + content="tool result", + ), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + assert len(assistant_msgs) == 1 + + content = assistant_msgs[0]["content"] + types = [b["type"] for b in content] + + assert "thinking" not in types + assert types == ["text", "tool_call"] + assert content[0]["text"] == "I'll call a tool" + assert content[1]["id"] == "call_1" + assert content[1]["name"] == "do_something" + + +def test_redacted_thinking_without_signature_key_is_untouched(): + """redacted_thinking blocks don't normally carry a signature at all -- + their `data` field is the whole payload, and Anthropic doesn't require + signature validation on them. Only strip one if some producer attached + an invalid `signature` key to it; leave the normal (no-signature) shape + alone. + """ + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + RedactedThinkingBlock(data="opaque-encrypted-payload"), + TextBlock(text="answer text"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + types = [b["type"] for b in assistant_msgs[0]["content"]] + assert types == ["redacted_thinking", "text"] + + +# --------------------------------------------------------------------------- +# Non-dict content block present -> no crash +# --------------------------------------------------------------------------- + + +def test_sanitize_does_not_crash_on_non_dict_content_block(): + """A corrupted/partial transcript could in principle surface a raw + string inside a content array. The sanitizer must tolerate this rather + than raising (block.get(...) would AttributeError on a str). + """ + provider = _make_provider() + + messages: list[dict[str, Any]] = [ + { + "role": "assistant", + "content": [ + "gAAAAAB123encryptedblob", # malformed: raw str, not a dict + {"type": "text", "text": "answer text"}, + ], + } + ] + + sanitized = asyncio.run(provider._sanitize_thinking_blocks(messages)) + + # No crash, and both entries survive untouched (neither is identifiable + # as an invalid thinking block). + assert sanitized[0]["content"][0] == "gAAAAAB123encryptedblob" + assert sanitized[0]["content"][1] == {"type": "text", "text": "answer text"} + + +# --------------------------------------------------------------------------- +# Clean histories are completely unaffected (no behavior change) +# --------------------------------------------------------------------------- + + +def test_clean_history_with_no_thinking_blocks_is_unaffected(): + provider = _make_provider() + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message(role="assistant", content="Hi there"), + Message(role="user", content="How are you?"), + ] + ) + asyncio.run(provider.complete(request)) + + params = _get_api_params(mock) + assistant_msgs = _assistant_messages(params) + assert len(assistant_msgs) == 1 + # Plain string content should remain a plain string (untouched). + assert assistant_msgs[0]["content"] == "Hi there" + + +# --------------------------------------------------------------------------- +# Observability: warning logged + event emitted when stripping occurs +# --------------------------------------------------------------------------- + + +def test_warning_logged_with_count_and_model_when_blocks_stripped(caplog): + provider = _make_provider(default_model="claude-sonnet-4-5-20250929") + mock = _wire_mock(provider) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock(thinking="bad 1", signature=None), + TextBlock(text="ok"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + with caplog.at_level(logging.WARNING): + asyncio.run(provider.complete(request)) + + _get_api_params(mock) # sanity: request still succeeded + + warnings = [r for r in caplog.records if r.levelno >= logging.WARNING] + stripped_warnings = [ + r for r in warnings if "thinking" in r.message.lower() and "strip" in r.message.lower() + ] + assert stripped_warnings, f"expected a strip warning, got: {[r.message for r in warnings]}" + assert "1" in stripped_warnings[0].message # stripped-count present + + +def test_event_emitted_when_blocks_stripped(): + provider = _make_provider() + mock = _wire_mock(provider) + fake_coordinator = cast(FakeCoordinator, provider.coordinator) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock(thinking="bad", signature=None), + TextBlock(text="ok"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + _get_api_params(mock) + + event_names = fake_coordinator.hooks.emitted_names() + matches = [n for n in event_names if ("thinking" in n and "strip" in n) or "sanitiz" in n] + assert matches, f"expected an observability event for stripped thinking blocks, got: {event_names}" + + +def test_no_event_emitted_when_nothing_stripped(): + """No behavior change for clean histories: no spurious strip event.""" + provider = _make_provider() + mock = _wire_mock(provider) + fake_coordinator = cast(FakeCoordinator, provider.coordinator) + + request = ChatRequest( + messages=[ + Message(role="user", content="Hello"), + Message( + role="assistant", + content=[ + ThinkingBlock(thinking="fine", signature="valid-sig"), + TextBlock(text="ok"), + ], + ), + Message(role="user", content="Continue"), + ] + ) + asyncio.run(provider.complete(request)) + + _get_api_params(mock) + + event_names = fake_coordinator.hooks.emitted_names() + matches = [ + n for n in event_names if ("thinking" in n and "strip" in n) or "sanitiz" in n + ] + assert not matches, f"unexpected strip event on clean history: {matches}"