diff --git a/amplifier_module_provider_anthropic/__init__.py b/amplifier_module_provider_anthropic/__init__.py index ce49402..2aea861 100644 --- a/amplifier_module_provider_anthropic/__init__.py +++ b/amplifier_module_provider_anthropic/__init__.py @@ -3462,7 +3462,7 @@ def parse_tool_calls(self, response: ChatResponse) -> list[ToolCall]: return valid_calls - def _clean_content_block(self, block: dict[str, Any]) -> dict[str, Any]: + def _clean_content_block(self, block: dict[str, Any]) -> dict[str, Any] | None: """Clean a content block for API by removing fields not accepted by Anthropic API. Anthropic API may include extra fields (like 'visibility') in responses, @@ -3472,17 +3472,33 @@ def _clean_content_block(self, block: dict[str, Any]) -> dict[str, Any]: block: Raw content block dict (may include visibility, etc.) Returns: - Cleaned content block dict with only API-accepted fields + Cleaned content block dict with only API-accepted fields, or None + if the block must be dropped entirely (thinking block without a + valid signature -- Anthropic rejects those with HTTP 400) """ block_type = block.get("type") if block_type == "text": return {"type": "text", "text": block.get("text", "")} if block_type == "thinking": - cleaned = {"type": "thinking", "thinking": block.get("thinking", "")} - if "signature" in block: - cleaned["signature"] = block["signature"] - return cleaned + # Anthropic requires a valid non-empty signature string on every + # thinking block sent as input. Thinking blocks persisted by other + # providers (e.g. OpenAI reasoning turns in a mixed-provider + # session) carry no signature; sending signature=None bricks the + # session with invalid_request_error on every resume, so drop the + # block entirely. See microsoft/amplifier#330. + signature = block.get("signature") + if not isinstance(signature, str) or not signature: + logger.debug( + "Dropping thinking block without valid signature " + "(likely from a non-Anthropic provider in session history)" + ) + return None + return { + "type": "thinking", + "thinking": block.get("thinking", ""), + "signature": signature, + } if block_type == "tool_use": return { "type": "tool_use", @@ -3614,10 +3630,12 @@ def _convert_messages(self, messages: list[dict[str, Any]]) -> list[dict[str, An has_thinking = "thinking_block" in msg and msg["thinking_block"] if has_thinking: # Clean thinking block (remove visibility field not accepted by API) + # May return None for unsigned thinking blocks - skip those cleaned_thinking = self._clean_content_block( msg["thinking_block"] ) - content_blocks.append(cleaned_thinking) + if cleaned_thinking is not None: + content_blocks.append(cleaned_thinking) # Add text content if present, BUT skip when we have thinking + tool_calls # When all three are present (thinking + text + tool_use), the text was generated @@ -3666,8 +3684,11 @@ def _convert_messages(self, messages: list[dict[str, Any]]) -> list[dict[str, An elif "thinking_block" in msg and msg["thinking_block"]: # Assistant message with thinking block # Clean thinking block (remove visibility field not accepted by API) + # May return None for unsigned thinking blocks - skip those cleaned_thinking = self._clean_content_block(msg["thinking_block"]) - content_blocks = [cleaned_thinking] + content_blocks = ( + [cleaned_thinking] if cleaned_thinking is not None else [] + ) if content: if isinstance(content, list): # Content is a list of blocks - extract text blocks only @@ -3693,6 +3714,13 @@ def _convert_messages(self, messages: list[dict[str, Any]]) -> list[dict[str, An else: # Content is a simple string content_blocks.append({"type": "text", "text": content}) + if not content_blocks: + # All content was dropped (e.g. only unsigned thinking + # blocks) - Anthropic rejects empty content arrays, so + # emit a minimal placeholder instead + content_blocks = [ + {"type": "text", "text": "(internal reasoning omitted)"} + ] anthropic_messages.append( {"role": "assistant", "content": content_blocks} ) @@ -3700,9 +3728,18 @@ def _convert_messages(self, messages: list[dict[str, Any]]) -> list[dict[str, An # Regular assistant message - may have structured content blocks if isinstance(content, list): # Content is a list of blocks - clean each block - cleaned_blocks = [ - self._clean_content_block(block) for block in content - ] + # (None means the block was dropped, e.g. unsigned thinking) + cleaned_blocks = [] + for block in content: + cleaned_block = self._clean_content_block(block) + if cleaned_block is not None: + cleaned_blocks.append(cleaned_block) + if not cleaned_blocks: + # All content was dropped - Anthropic rejects empty + # content arrays, so emit a minimal placeholder + cleaned_blocks = [ + {"type": "text", "text": "(internal reasoning omitted)"} + ] anthropic_messages.append( {"role": "assistant", "content": cleaned_blocks} ) @@ -4033,13 +4070,22 @@ def _convert_to_chat_response(self, response: Any) -> ChatResponse: text_accumulator.append(block.text) event_blocks.append(TextContent(text=block.text)) elif block.type == "thinking": - content_blocks.append( - ThinkingBlock( - thinking=block.thinking, - signature=getattr(block, "signature", None), - visibility="internal", + # Only persist thinking blocks that carry a valid signature - + # an unsigned block persisted to history would be rejected by + # the API on every resume (microsoft/amplifier#330) + signature = getattr(block, "signature", None) + if isinstance(signature, str) and signature: + content_blocks.append( + ThinkingBlock( + thinking=block.thinking, + signature=signature, + visibility="internal", + ) + ) + else: + logger.debug( + "Skipping thinking block without valid signature in response" ) - ) event_blocks.append(ThinkingContent(text=block.thinking)) # NOTE: Do NOT add thinking to text_accumulator - it's internal process, not response content elif block.type == "tool_use": diff --git a/tests/test_thinking_signature_guard.py b/tests/test_thinking_signature_guard.py new file mode 100644 index 0000000..5f253cf --- /dev/null +++ b/tests/test_thinking_signature_guard.py @@ -0,0 +1,223 @@ +"""Tests for the unsigned-thinking-block guard (microsoft/amplifier#330). + +Sessions that ran partly on non-Anthropic providers (e.g. OpenAI) persist +thinking blocks with signature=None or no signature field at all. Anthropic +requires a valid non-empty signature string on every thinking block sent as +input; forwarding an unsigned block yields HTTP 400 +(messages.N.content.0.thinking.signature.str: Input should be a valid string) +and permanently bricks the session on resume. + +Covers: + (a) Thinking block with a valid string signature passes through unchanged + (b) Thinking block with signature=None is dropped; sibling text/tool_use + blocks are preserved + (c) Thinking block with a missing signature field is dropped + (d) Thinking block with an empty-string signature is dropped + (e) Assistant message whose ONLY content is unsigned thinking blocks still + produces a valid message (placeholder text, never an empty content array) + (f) thinking_block message field: unsigned block is skipped, and a + thinking-only message falls back to a placeholder + (g) _convert_to_chat_response never persists an unsigned thinking block + from an API response +""" + +from types import SimpleNamespace +from typing import cast + +from amplifier_core import ModuleCoordinator + +from amplifier_module_provider_anthropic import AnthropicProvider +from tests._helpers import DummyResponse, FakeCoordinator + + +def _make_provider() -> AnthropicProvider: + provider = AnthropicProvider( + api_key="test-key", + config={"max_retries": 0}, + ) + provider.coordinator = cast(ModuleCoordinator, FakeCoordinator()) + return provider + + +def _thinking(signature=..., thinking: str = "some reasoning") -> dict: + """Build a thinking block dict; pass signature=... to omit the field.""" + block = {"type": "thinking", "thinking": thinking} + if signature is not ...: + block["signature"] = signature + return block + + +# --------------------------------------------------------------------------- +# (a) Valid signature passes through unchanged +# --------------------------------------------------------------------------- +def test_thinking_block_with_valid_signature_passes_through(): + provider = _make_provider() + cleaned = provider._clean_content_block( + _thinking(signature="sig-abc123", thinking="deep thought") + ) + assert cleaned == { + "type": "thinking", + "thinking": "deep thought", + "signature": "sig-abc123", + } + + +# --------------------------------------------------------------------------- +# (b) signature=None is dropped; sibling blocks preserved +# --------------------------------------------------------------------------- +def test_thinking_block_with_none_signature_dropped_siblings_preserved(): + provider = _make_provider() + assert provider._clean_content_block(_thinking(signature=None)) is None + + messages = [ + {"role": "user", "content": "Hello"}, + { + "role": "assistant", + "content": [ + _thinking(signature=None), + {"type": "text", "text": "visible reply"}, + {"type": "tool_use", "id": "tc-1", "name": "grep", "input": {}}, + ], + }, + ] + converted = provider._convert_messages(messages) + assistant = converted[-1] + assert assistant["role"] == "assistant" + block_types = [b["type"] for b in assistant["content"]] + assert "thinking" not in block_types + assert block_types == ["text", "tool_use"] + assert assistant["content"][0]["text"] == "visible reply" + assert assistant["content"][1]["id"] == "tc-1" + + +# --------------------------------------------------------------------------- +# (c) Missing signature field is dropped +# --------------------------------------------------------------------------- +def test_thinking_block_with_missing_signature_dropped(): + provider = _make_provider() + assert provider._clean_content_block(_thinking()) is None + + +# --------------------------------------------------------------------------- +# (d) Empty-string signature is dropped +# --------------------------------------------------------------------------- +def test_thinking_block_with_empty_signature_dropped(): + provider = _make_provider() + assert provider._clean_content_block(_thinking(signature="")) is None + + +# --------------------------------------------------------------------------- +# (e) All-unsigned-thinking assistant message never yields empty content +# --------------------------------------------------------------------------- +def test_all_unsigned_thinking_message_gets_placeholder_not_empty_content(): + provider = _make_provider() + messages = [ + {"role": "user", "content": "Hello"}, + { + "role": "assistant", + "content": [ + _thinking(signature=None, thinking="reasoning one"), + _thinking(thinking="reasoning two"), # missing signature + ], + }, + {"role": "user", "content": "Continue"}, + ] + converted = provider._convert_messages(messages) + assistant = next(m for m in converted if m["role"] == "assistant") + # Never an empty content array - Anthropic rejects those + assert assistant["content"] + assert all(b["type"] == "text" for b in assistant["content"]) + assert assistant["content"][0]["text"] + + +# --------------------------------------------------------------------------- +# (f) thinking_block message field paths +# --------------------------------------------------------------------------- +def test_unsigned_thinking_block_field_skipped_with_tool_calls(): + provider = _make_provider() + messages = [ + {"role": "user", "content": "Hello"}, + { + "role": "assistant", + "content": "", + "thinking_block": _thinking(signature=None), + "tool_calls": [{"id": "tc-1", "tool": "grep", "arguments": {}}], + }, + ] + converted = provider._convert_messages(messages) + assistant = converted[-1] + block_types = [b["type"] for b in assistant["content"]] + assert "thinking" not in block_types + assert "tool_use" in block_types + + +def test_unsigned_thinking_block_field_only_content_gets_placeholder(): + provider = _make_provider() + messages = [ + {"role": "user", "content": "Hello"}, + { + "role": "assistant", + "content": "", + "thinking_block": _thinking(signature=None), + }, + ] + converted = provider._convert_messages(messages) + assistant = converted[-1] + assert assistant["content"] + assert all(b["type"] == "text" for b in assistant["content"]) + + +def test_signed_thinking_block_field_preserved(): + provider = _make_provider() + messages = [ + {"role": "user", "content": "Hello"}, + { + "role": "assistant", + "content": "reply text", + "thinking_block": _thinking(signature="sig-xyz"), + }, + ] + converted = provider._convert_messages(messages) + assistant = converted[-1] + assert assistant["content"][0] == { + "type": "thinking", + "thinking": "some reasoning", + "signature": "sig-xyz", + } + + +# --------------------------------------------------------------------------- +# (g) _convert_to_chat_response never persists unsigned thinking blocks +# --------------------------------------------------------------------------- +def test_response_conversion_skips_unsigned_thinking_block(): + provider = _make_provider() + response = DummyResponse( + content=[ + SimpleNamespace(type="thinking", thinking="unsigned", signature=None), + SimpleNamespace(type="text", text="hello"), + ] + ) + result = provider._convert_to_chat_response(response) + thinking_blocks = [ + b for b in result.content if getattr(b, "type", "") == "thinking" + ] + assert thinking_blocks == [] + text_blocks = [b for b in result.content if getattr(b, "type", "") == "text"] + assert len(text_blocks) == 1 + assert text_blocks[0].text == "hello" + + +def test_response_conversion_keeps_signed_thinking_block(): + provider = _make_provider() + response = DummyResponse( + content=[ + SimpleNamespace(type="thinking", thinking="signed", signature="sig-1"), + SimpleNamespace(type="text", text="hello"), + ] + ) + result = provider._convert_to_chat_response(response) + thinking_blocks = [ + b for b in result.content if getattr(b, "type", "") == "thinking" + ] + assert len(thinking_blocks) == 1 + assert thinking_blocks[0].signature == "sig-1"