From 5396889ff12d39cb83af5e71d90cb8aad97dddb8 Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Tue, 12 May 2026 15:48:54 +0300 Subject: [PATCH] bedrock redacted --- crates/openfang-runtime/src/agent_loop.rs | 56 ++++++++++--- .../openfang-runtime/src/drivers/bedrock.rs | 78 ++++++++++++++++++- 2 files changed, 123 insertions(+), 11 deletions(-) diff --git a/crates/openfang-runtime/src/agent_loop.rs b/crates/openfang-runtime/src/agent_loop.rs index 95a91fe5..84f7d0c0 100644 --- a/crates/openfang-runtime/src/agent_loop.rs +++ b/crates/openfang-runtime/src/agent_loop.rs @@ -158,22 +158,30 @@ fn build_assistant_message_preserving_thinking( response_blocks: &[ContentBlock], final_text: &str, ) -> Message { - let has_thinking = response_blocks - .iter() - .any(|b| matches!(b, ContentBlock::Thinking { .. })); - if !has_thinking { + // Key on either Thinking or RedactedThinking — Anthropic/Bedrock both + // reject extended-thinking history that drops the redacted variant, so a + // turn that contains only RedactedThinking must still be preserved. + let has_reasoning = response_blocks.iter().any(|b| { + matches!( + b, + ContentBlock::Thinking { .. } | ContentBlock::RedactedThinking { .. } + ) + }); + if !has_reasoning { return Message::assistant(final_text.to_string()); } - // Preserve order: Thinking blocks first (in original order), then a - // single Text block carrying `final_text`. Tool blocks aren't expected - // here (StopReason::EndTurn path), but copy them through if present so - // we don't drop information. + // Preserve order: Thinking / RedactedThinking blocks first (in original + // order), then a single Text block carrying `final_text`. Tool blocks + // aren't expected here (StopReason::EndTurn path), but copy them through + // if present so we don't drop information. let mut blocks: Vec = Vec::with_capacity(response_blocks.len() + 1); let mut emitted_text = false; for b in response_blocks { match b { - ContentBlock::Thinking { .. } => blocks.push(b.clone()), + ContentBlock::Thinking { .. } | ContentBlock::RedactedThinking { .. } => { + blocks.push(b.clone()) + } ContentBlock::Text { .. } if !emitted_text => { blocks.push(ContentBlock::Text { text: final_text.to_string(), @@ -3380,6 +3388,36 @@ mod tests { assert_eq!(saved_text, Some(final_text)); } + /// Issue #1187 — a turn that contains only `RedactedThinking` (no + /// `Thinking` block) must still trigger the block-preserving path. The + /// previous gate keyed solely on `Thinking`, so redacted-only turns were + /// downgraded to plain text and the encrypted blob was lost on the next + /// request, which Anthropic/Bedrock reject. + #[test] + fn test_build_assistant_message_preserves_redacted_only() { + let response_blocks = vec![ + ContentBlock::RedactedThinking { + data: "encrypted_only".to_string(), + }, + ContentBlock::Text { + text: "Answer".to_string(), + provider_metadata: None, + }, + ]; + let msg = build_assistant_message_preserving_thinking(&response_blocks, "Answer"); + let blocks = match &msg.content { + MessageContent::Blocks(b) => b, + other => panic!("expected Blocks content for redacted-only turn, got {other:?}"), + }; + let has_redacted = blocks + .iter() + .any(|b| matches!(b, ContentBlock::RedactedThinking { data } if data == "encrypted_only")); + assert!( + has_redacted, + "RedactedThinking-only turn must be preserved as Blocks" + ); + } + #[test] fn test_retry_constants() { assert_eq!(MAX_RETRIES, 3); diff --git a/crates/openfang-runtime/src/drivers/bedrock.rs b/crates/openfang-runtime/src/drivers/bedrock.rs index d0333f7d..6c77e898 100644 --- a/crates/openfang-runtime/src/drivers/bedrock.rs +++ b/crates/openfang-runtime/src/drivers/bedrock.rs @@ -92,6 +92,20 @@ enum BedrockContentBlock { #[serde(rename = "toolResult")] tool_result: BedrockToolResult, }, + // Bedrock Converse representation of Anthropic's `redacted_thinking`. + // The encrypted blob is echoed back verbatim under + // reasoningContent.redactedContent so Claude extended-thinking history + // is not rejected on resubmission. + ReasoningContent { + #[serde(rename = "reasoningContent")] + reasoning_content: BedrockReasoningContent, + }, +} + +#[derive(Debug, Serialize)] +struct BedrockReasoningContent { + #[serde(rename = "redactedContent")] + redacted_content: String, } #[derive(Debug, Serialize)] @@ -299,10 +313,23 @@ fn convert_content_block(block: &ContentBlock) -> Option { }, }, }), - // Image, Thinking, RedactedThinking, and Unknown are not supported — silently drop + // Echo redacted_thinking verbatim. Bedrock Converse rejects history + // that drops these blocks on Claude extended-thinking models, mirroring + // the anthropic.rs path. Drop empty blobs (e.g. interrupted stream). + ContentBlock::RedactedThinking { data } => { + if data.is_empty() { + None + } else { + Some(BedrockContentBlock::ReasoningContent { + reasoning_content: BedrockReasoningContent { + redacted_content: data.clone(), + }, + }) + } + } + // Image, Thinking, and Unknown are not supported — silently drop ContentBlock::Image { .. } | ContentBlock::Thinking { .. } - | ContentBlock::RedactedThinking { .. } | ContentBlock::Unknown => None, } } @@ -1126,6 +1153,53 @@ mod tests { assert!(text_at_3 >= 1); } + /// Issue #1187 — Bedrock Converse history must preserve + /// `redacted_thinking` blocks on Claude extended-thinking models. + /// A message containing only RedactedThinking must round-trip through + /// `convert_content_block` without being silently dropped, and the wire + /// format must use `reasoningContent.redactedContent`. + #[test] + fn test_bedrock_redacted_thinking_round_trip() { + let msg = Message::assistant_with_blocks(vec![ContentBlock::RedactedThinking { + data: "encrypted-blob-abc123".to_string(), + }]); + + let bedrock_blocks = convert_message_content(&msg.content); + assert_eq!( + bedrock_blocks.len(), + 1, + "RedactedThinking must survive convert_content_block" + ); + match &bedrock_blocks[0] { + BedrockContentBlock::ReasoningContent { reasoning_content } => { + assert_eq!(reasoning_content.redacted_content, "encrypted-blob-abc123"); + } + other => panic!("expected ReasoningContent block, got {other:?}"), + } + + // Wire format check: serialized JSON must carry + // reasoningContent.redactedContent so Bedrock accepts the history. + let json = serde_json::to_value(&bedrock_blocks[0]).unwrap(); + assert_eq!( + json["reasoningContent"]["redactedContent"], + "encrypted-blob-abc123" + ); + } + + /// Empty RedactedThinking blobs (interrupted stream) must be dropped on + /// outbound, matching the anthropic.rs behavior. + #[test] + fn test_bedrock_redacted_thinking_empty_dropped() { + let msg = Message::assistant_with_blocks(vec![ContentBlock::RedactedThinking { + data: String::new(), + }]); + let bedrock_blocks = convert_message_content(&msg.content); + assert!( + bedrock_blocks.is_empty(), + "empty redacted_thinking must be dropped" + ); + } + #[test] fn test_validate_tool_pairing_noop_on_correct() { // already correct 2-for-2 → no change