fix: address cache_control/beta-header passthrough review issues#124
Open
Milo0821 wants to merge 3 commits into
Open
fix: address cache_control/beta-header passthrough review issues#124Milo0821 wants to merge 3 commits into
Milo0821 wants to merge 3 commits into
Conversation
Clients using the Anthropic SDK can annotate messages, system blocks,
and tool definitions with cache_control: { type: "ephemeral" } to enable
prompt caching. These markers were previously silently dropped.
This change maps them to Bedrock cachePoint: { type: "default" } blocks
at all three locations: system blocks, user message content, and tool
definitions. The existing config-based promptCache approach is preserved
as a fallback for backward compatibility.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ude Code compatibility Add BEDROCK_BETA_BLOCKLIST constant to strip Anthropic-specific beta headers that Bedrock does not support (files-api, computer-use, prompt-caching-scope, redact-thinking, advisor-tool). Pass the client's anthropic-beta header through both complete() and chat() entry points into toPayload(), merging with auto-injected model-specific features while filtering blocklisted values. Deduplication is handled by switching from array to Set<string>. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Four correctness and quality issues identified in code review:
1. Double-system-content bug (provider.ts): when body.system is an
array, the old code pushed a flattened role:'system' message into
messages[] AND set system_blocks, causing fragile dual-path routing.
Now only the string case pushes to messages[]; array form is handled
exclusively via system_blocks.
2. Mutation of shared messages array (bedrock_converse.ts): the user-
message loop mutated message.content in place, corrupting the original
chatRequest on retries. Fixed by computing into newContent and pushing
a shallow clone { ...message, content: newContent }.
3. cache_control leaked to Bedrock (bedrock_converse.ts): the fallthrough
return in convertConverseSingleType forwarded cache_control on unknown
content types, risking ValidationException. Now strips it via
destructuring before returning.
4. Import order violation (bedrock_converse.ts): BEDROCK_BETA_BLOCKLIST
const was declared between two import statements, violating ESLint
import/first. Moved to after all imports.
Also adds a typed system_blocks field to the ChatRequest interface
(chat_request.ts) so TypeScript catches typos at compile time.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
anthropic-betaheader passthrough from client → Bedrock, filtered against a compile-timeBEDROCK_BETA_BLOCKLISTof unsupported headerscache_control→cachePointtranslation for user message blocks, tool definitions, and system blocksSet(deduplication)🤖 Generated with Claude Code