Support token exchange for bedrock#3587
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryThis PR adds Bedrock-compatible proxy support to the Nexus AI gateway layer and the agent harnesses. The changes span three concerns: (1) OAuth2 token-exchange for Bedrock (mirroring the existing OpenAI mechanism), (2) a custom base-URL override for non-AWS Bedrock-compatible endpoints, and (3) a new
Confidence Score: 4/5The change is additive and backward-compatible; new fields default to safe values (streaming on, no base URL override, no token exchange). The multi-layer streaming-disable design (Bifrost AllowedRequests gate + header-driven pre-callback) is well-tested and logically consistent. The previously missing accessToken field in the Bedrock gRPC message is now correctly sent. The only notable concern is ANTHROPIC_CUSTOM_HEADERS being overwritten unconditionally in the harness — no current code is affected, but it is a latent clobbering risk if additional headers are configured for the same env key in the future. go/deployment-operator/pkg/agentrun-harness/tool/claude/claude_streaming.go — the unconditional overwrite of ANTHROPIC_CUSTOM_HEADERS in settings.local.json could silently drop other headers if this key is ever written elsewhere in the settings builder.
|
| Filename | Overview |
|---|---|
| go/nexus/internal/router/streaming_control.go | New file: header-driven pre-callback that sets stream=false on Anthropic/OpenAI request structs when X-Plural-Enable-Stream: false is present |
| go/nexus/internal/router/bedrock_allowed_requests.go | New file: configures Bifrost AllowedRequests gate to block streaming endpoints when enableStream is explicitly false |
| go/nexus/internal/router/oauth_token.go | New file: extracts shared oauthAccessToken helper used by both OpenAI and Bedrock key handlers; clean refactor |
| go/nexus/internal/router/account_keys.go | Adds OAuth2 token exchange and static access-token support for Bedrock; openAIAPIKey refactored to use shared helper |
| go/nexus/internal/router/account.go | Populates Bedrock NetworkConfig.BaseURL and CustomProviderConfig.AllowedRequests when configured |
| lib/console/ai/provider/bedrock.ex | Adds base_url, token_exchange, and enable_stream support; provider_options now returns {:ok, opts} tuples; add_stream handles disabling streaming |
| lib/console/grpc/server.ex | Previously missing accessToken now included in Bedrock proto; adds baseUrl, tokenExchange, enableStream fields |
| go/deployment-operator/pkg/agentrun-harness/tool/claude/claude_streaming.go | New file: sets CLAUDE_CODE_EXTRA_BODY and ANTHROPIC_CUSTOM_HEADERS env vars when DisableStream is true; ANTHROPIC_CUSTOM_HEADERS is set unconditionally and would overwrite any other custom headers already in settings |
| go/deployment-operator/pkg/agentrun-harness/tool/codex/codex_types.go | Adds HttpHeaders field to ModelProviderInput and ModelProviderConfig for injecting X-Plural-Enable-Stream into TOML config |
| go/deployment-operator/api/v1alpha1/agentruntime_types.go | Adds DisableStream *bool to both ClaudeConfig/ClaudeConfigRaw and CodexConfig/CodexConfigRaw; deepcopy and conversion are consistent |
| lib/console/schema/deployment_settings.ex | Adds base_url, enable_stream (default true), and token_exchange embed to Bedrock settings schema |
| proto/console.proto | Adds baseUrl, tokenExchange, and enableStream optional fields to BedrockConfig message |
Reviews (1): Last reviewed commit: "Support token exchange for bedrock" | Re-trigger Greptile
|
|
||
| settings.WithEnv("CLAUDE_CODE_EXTRA_BODY", claudeExtraBodyDisableStream) | ||
| if in.Config.Run.IsProxyEnabled() { | ||
| settings.WithEnv("ANTHROPIC_CUSTOM_HEADERS", PluralDisableStreamHeader) |
There was a problem hiding this comment.
ANTHROPIC_CUSTOM_HEADERS overwrites any pre-existing value
settings.WithEnv calls do a simple map assignment (b.settings.Env[key] = value), so setting ANTHROPIC_CUSTOM_HEADERS here will silently drop any other custom headers already written into the settings map earlier in the same configuration pass. There are no other callers of this key today, but if one is ever added (e.g., an operator-supplied tracing/auth header), this line will win and the other will be lost without any warning.
| func TestBedrockNetworkBaseURL(t *testing.T) { | ||
| require.Equal(t, "https://bedrock-proxy.example.com", bedrockNetworkBaseURL("https://bedrock-proxy.example.com/")) | ||
| require.Equal(t, "https://bedrock-proxy.example.com", bedrockNetworkBaseURL(" https://bedrock-proxy.example.com ")) | ||
| } |
There was a problem hiding this comment.
TestBedrockNetworkBaseURL test placed in wrong file
TestBedrockNetworkBaseURL lives in bedrock_allowed_requests_test.go but tests the function bedrockNetworkBaseURL defined in provider_base_url.go. Moving it to provider_base_url_test.go (or a new companion test file for provider_base_url.go) would make it easier to find when modifying that function.
This has the following needs: 1. Baseurl support - handled in reqllm but not bifrost 2. Custom tuning of stream enablement 3. Potentially more
60c8120 to
8a49348
Compare
This has the following needs:
Test Plan
e2e
Checklist
Plural Flow: console