feat(site): add Bedrock mantle protocol selector to provider form - #27156
Conversation
e7db488 to
96b3d0f
Compare
Implements https://linear.app/codercom/issue/AIGOV-213/add-bedrock-provider # AWS Bedrock mantle support in AI Gateway ## Summary Add support for the AWS Bedrock **mantle** endpoint (`bedrock-mantle.{region}.api.aws/anthropic/v1/messages`) to AI Gateway. Mantle serves Claude through the native Anthropic Messages API. We model it as a `protocol` field on the existing Bedrock provider settings (`invoke-model` default, or `mantle`) rather than as a new provider type, and we treat mantle as a pure passthrough: SigV4-sign and forward, no body translation. ## Background Claude on AWS Bedrock is reachable through two endpoints, each speaking exactly one wire protocol: 1. **InvokeModel** (existing): `bedrock-runtime.{region}.amazonaws.com`. Model id in the URL path, request translated into Bedrock's InvokeModel format, responses returned as a binary AWS eventstream. This is what AI Gateway already supported for Bedrock. 2. **Mantle** (this doc): `bedrock-mantle.{region}.api.aws/anthropic/v1/messages`. Native Anthropic Messages API: model in the body, plain SSE streaming. ## Why a `protocol` field, not a new provider type The alternative is to model mantle as its own `ai_provider_type` (`bedrock-mantle`) alongside `bedrock`. I chose the `protocol` field instead for two reasons: 1. Mantle reads more like a protocol of Bedrock than a separate provider. It is the same AWS account, credentials, region, and IAM, reached over a different wire protocol and host. One Bedrock provider with two protocols (`invoke-model` default and `mantle`) models that more organically than two provider types. 2. It avoids a database migration. The `protocol` field lives in the settings JSON blob (empty resolves to `invoke-model`, so existing providers are unaffected), whereas a new type means an enum value and the `ALTER TYPE ... ADD VALUE` migration that goes with it. ## Why passthrough, not translation The client already emits Bedrock-legal requests in mantle mode: ```sh export CLAUDE_CODE_USE_MANTLE=1 export CLAUDE_CODE_SKIP_MANTLE_AUTH=1 export ANTHROPIC_BEDROCK_MANTLE_BASE_URL=https://<coder>/api/v2/aibridge/<provider-name> ``` So the gateway just forwards the body and SigV4-signs it (service `bedrock-mantle`), and skips all the InvokeModel body-translation (model remap, thinking conversion, beta-flag allowlist, field stripping). This keeps the mantle path thin and avoids a second copy of translation logic to maintain. ## Consequences - Protocol-dependent fields: `model` / `small_fast_model` are used by InvokeModel but ignored by mantle (the client sends the model), and `base_url` is required for mantle but optional for InvokeModel. Validation is protocol-aware. - No central model control on mantle: because it is a passthrough, the operator cannot pin the model. - `region` and the `base_url` host must name the same region (the SigV4 scope must match the endpoint); a mismatch surfaces as `Credential should be scoped to a valid region`. ## Draft UI <img width="1100" height="579" alt="image" src="https://github.com/user-attachments/assets/37bab46d-8958-4a96-9f47-1fef3493e1b6" /> ## Follow-up PRs: - #27156
5448d68 to
ed55d25
Compare
ddb458f to
a23bc9e
Compare
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.9.0 | Round 3 | Last posted: Round 3, 17 findings (1 P1, 3 P3, 5 Nit, 8 Note), COMMENT. Review Finding inventoryFinding inventory: PR #27156Findings
Cross-check notes
Contested and acknowledgedCRF-3 (Note, ProviderForm.tsx:128) - Default region us-east-2 -> us-east-1
CRF-6 (Note, ProviderForm.stories.tsx:143) - AddBedrockMantle unreachable seed state
CRF-8 (Note, ProviderForm.tsx:586) - "recommended by AWS" unsourced claim
CRF-9 (Nit, ProviderForm.tsx:594) - Duplicated description wrapper
|
Documentation CheckThis PR adds a user-facing Protocol selector (InvokeModel vs Mantle) to the Bedrock provider form in AI Settings, with a distinct mantle endpoint format ( Updates Needed
Automated review via Coder Agents |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a clean, well-tested feature PR. The protocol enumeration is consistent across the Go type, generated TS, the Yup oneOf, and the two SelectItems; the mantle passthrough correctly omits the model fields on the wire and reads legacy/empty protocol back as InvokeModel; the region invariant the backend requires for mantle is guaranteed by the schema-and-parse coupling; and the endpoint regexes are fully anchored against userinfo bypass, suffix widening, and path traversal. The unit suite (96 tests) and both new interaction stories pass.
One blocking issue. handleBedrockProtocolChange updates protocol and baseUrl with two sequential setFieldValue calls, and Formik 2.4.9 validates each against a stale render-closure snapshot of the other field. The result: after a user switches protocol, form.isValid goes false against a fully valid form and the Save button stays disabled until they touch another field. That breaks the feature's own happy path, and the existing switch story never asserts submit-button state, so CI is green. An atomic setValues fixes the class.
Severity count: 1 P1, 3 P3, 4 Nit, 7 Note. The P3s share the theme the P1 exposed: the protocol switch and mantle-submit paths have thin coverage (the rewrite value, the conditional schema branches, and no story that actually submits a mantle provider). Two Notes need a human decision, not a code change: the "recommended by AWS" hint copy is an unsourced third-party claim, and the shipped default (InvokeModel) is the option the copy steers users away from.
On the endpoint error messages, Leorio put it well: the mantle message is a diagnosis while the InvokeModel one is "a doctor saying "you're sick."" Giving both the format string is a one-line fix.
Requesting changes for the P1; everything else is polish or coverage.
site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.test.ts:621
Note [CRF-15] The mantle branch of buildBedrockSettings is verified only through providerFormValuesToCreate; the providerFormValuesToUpdate Bedrock suite has no mantle row. (Bisky)
Low risk: both paths call the same buildBedrockSettings, and create already asserts the model-field omission and protocol: "mantle". Worth one row for symmetry with the existing update Bedrock cases.
🤖
🤖 This review was automatically generated with Coder Agents.
| // Switching protocols rewrites the base URL to the matching host, keeping | ||
| // the region the user already entered so they do not retype it. | ||
| const handleBedrockProtocolChange = (protocol: AIProviderBedrockProtocol) => { | ||
| void form.setFieldValue("protocol", protocol); |
There was a problem hiding this comment.
P1 [CRF-4] handleBedrockProtocolChange fires two sequential setFieldValue calls, so validation runs against a stale protocol and Save stays disabled after switching protocol even though every value is valid. (Meruem P1)
Formik 2.4.9's setFieldValue(field, value) validates setIn(state.values, field, value), where state.values is the render-closure snapshot. Both calls share that snapshot:
form.setFieldValue("protocol", protocol); // validates {mantle, oldInvokeUrl}
form.setFieldValue("baseUrl", bedrockMantleBaseUrl(region)); // validates {invoke-model (stale), mantleUrl}
The last-resolving validation checks the new mantle URL against the InvokeModel regex, sets errors.baseUrl, and form.isValid goes false. Submit is disabled={... || !form.isValid ...} (line 709) and nothing re-validates until another field changes, so the primary happy path (pick Mantle, then Save) is broken; the bug is symmetric on switching back. Verified against the Formik source and Meruem's useFormik harness. Fix atomically so validation runs once against the complete next state:
form.setValues({ ...form.values, protocol, baseUrl });
🤖
There was a problem hiding this comment.
Verified fixed in 87bdcaf. handleBedrockProtocolChange now uses a single atomic form.setValues, so validation runs once against the complete next state; the phantom-disabled-Save class is eliminated in both switch directions, and AddBedrockProtocolSwitchKeepsSaveEnabled asserts Save stays enabled after the switch.
🤖
|
|
||
| // Switching protocols rewrites the base URL to the matching host, keeping | ||
| // the region the user already entered so they do not retype it. | ||
| const handleBedrockProtocolChange = (protocol: AIProviderBedrockProtocol) => { |
There was a problem hiding this comment.
P3 [CRF-1] No test asserts the base-URL rewrite performed by handleBedrockProtocolChange. (Netero)
AddBedrockSwitchToMantleonly asserts the model fields hide and the hint text flips; it starts from an emptybaseUrl, so both the region-preservation branch and the resulting endpoint value go unverified.
A regression that dropped the region or wrote the wrong host would pass CI. Add a story that types an InvokeModel URL with a non-default region, switches to Mantle, and asserts the endpoint input equals https://bedrock-mantle.{that-region}.api.aws/anthropic. The same missing assertion (isValid / submit state) is why CRF-4 was not caught.
🤖
There was a problem hiding this comment.
Verified fixed in 9e9a986 (with 87bdcaf). AddBedrockProtocolSwitchKeepsSaveEnabled starts from a non-default eu-west-1 InvokeModel URL and asserts the endpoint is rewritten to https://bedrock-mantle.eu-west-1.api.aws/anthropic, covering region preservation on the field value.
🤖
| @@ -167,16 +197,38 @@ const makeBedrockSchema = (editing: boolean) => | |||
| name: makeNameSchema(editing), | |||
There was a problem hiding this comment.
P3 [CRF-5] The protocol-conditional Bedrock schema (mantle URL enforcement, model/smallFastModel becoming optional under mantle) has no test, and no story submits a mantle provider. (Bisky)
makeBedrockSchemais module-local and never imported by a test... if a regression leftmodelrequired under mantle, the form becomes permanently unsubmittable and no test would catch it.
The unit tests exercise the mappers, which run regardless of the Yup schema, so the .when("protocol", ...) wiring is unguarded. Add a story that submits a mantle provider: reject the InvokeModel URL shape, then accept bedrock-mantle.{region}.api.aws/anthropic and confirm onSubmit fires with no model fields required.
🤖
There was a problem hiding this comment.
Verified fixed in 4455552. AddBedrockMantleRejectsInvokeUrl seeds protocol=mantle with an InvokeModel-shaped URL and asserts submit stays disabled and onSubmit is never called, genuinely exercising the mantle .when schema branch.
🤖
| otherwise: (schema) => | ||
| schema.matches( | ||
| BEDROCK_CANONICAL_URL_REGEX, | ||
| "Endpoint must be a standard AWS Bedrock URL.", |
There was a problem hiding this comment.
P3 [CRF-7] The InvokeModel endpoint validation error names the problem but not the fix, while its new mantle sibling one line up hands the user the exact format. (Leorio)
mantle:
Endpoint must be a Bedrock mantle URL (https://bedrock-mantle.{region}.api.aws/anthropic).
invoke-model:Endpoint must be a standard AWS Bedrock URL.
A user who pasted the wrong shape has no idea what "standard" means. Match the sibling:
Endpoint must be a standard AWS Bedrock URL (https://bedrock-runtime.{region}.amazonaws.com).
🤖
There was a problem hiding this comment.
Verified fixed in 5fb2c5d. The InvokeModel endpoint error now reads "Endpoint must be a Bedrock InvokeModel URL (https://bedrock-runtime.{region}.amazonaws.com).", matching the format hint its mantle sibling provides.
🤖
| void form.setFieldValue("protocol", protocol); | ||
| const region = | ||
| parseBedrockRegionFromBaseUrl(form.values.baseUrl) ?? | ||
| BEDROCK_MANTLE_DEFAULT_REGION; |
There was a problem hiding this comment.
Nit [CRF-2] BEDROCK_MANTLE_DEFAULT_REGION is used as the fallback region when switching to InvokeModel too, so the MANTLE in its name is inaccurate. (Netero, Gon, Robin)
The fallback applies regardless of target protocol; switching an unparseable URL to InvokeModel yields bedrock-runtime.us-east-1.amazonaws.com from a constant named for mantle. Rename to BEDROCK_DEFAULT_REGION (or similar) to reflect that it is the shared region default.
🤖
There was a problem hiding this comment.
Verified fixed in 68b0594. BEDROCK_MANTLE_DEFAULT_REGION is renamed to BEDROCK_DEFAULT_REGION at all use sites, so the shared fallback is no longer named for mantle.
🤖
| // does not configure model fields (the client sends the model), and the | ||
| // endpoint hint points at the mantle host. | ||
| export const AddBedrockMantle: Story = { | ||
| args: { |
There was a problem hiding this comment.
Note [CRF-6] AddBedrockMantle seeds a state no user can reach: protocol: "mantle" with no baseUrl, so it inherits the InvokeModel host from typeDefaults.bedrock. (Hisoka, Melody)
the rendered form shows a mantle protocol pointing at an InvokeModel endpoint that fails the mantle URL schema.
The play asserts only that the hint text /bedrock-mantle/i renders (the description, not the field value), so the mismatched endpoint goes unchecked. Seed a coherent mantle baseUrl and assert the endpoint field shows it.
🤖
There was a problem hiding this comment.
won't fix, minor
| </Select> | ||
| <p className="text-xs text-content-secondary m-0"> | ||
| {isMantle | ||
| ? "Newer Anthropic-compatible Bedrock endpoint, recommended by AWS for new deployments." |
There was a problem hiding this comment.
Note [CRF-8] The protocol hint asserts "recommended by AWS for new deployments" with no cited source, and the shipped default contradicts the recommendation. (Leorio, Mafu-san)
Nothing in the diff or the linked backend PR grounds the AWS recommendation; if AWS's positioning changes, the copy goes stale silently. Separately, the selector labels InvokeModel "(default)" while the hint calls it "Legacy... Mantle is recommended", so the shipped default is the option the copy steers users away from. This needs a human decision: source or soften the claim, and decide whether the default and the recommendation should agree.
🤖
There was a problem hiding this comment.
won't fix, minor
| // An empty or missing protocol resolves to InvokeModel (legacy rows), | ||
| // mirroring the backend. Any other stored value passes through unchanged | ||
| // rather than being collapsed to InvokeModel. | ||
| const protocol: AIProviderBedrockProtocol = s.protocol || "invoke-model"; |
There was a problem hiding this comment.
Note [CRF-12] An unrecognized stored protocol renders a Protocol selector with no visible selection. (Nami)
s.protocol || "invoke-model" passes any truthy stored value through unchanged; <SelectValue /> has no placeholder and no matching <SelectItem>, so a future server-side protocol the form does not know about would render an empty dropdown, then oneOf blocks save. Unreachable today (backend emits only the two known values); worth knowing before a third protocol lands.
🤖
There was a problem hiding this comment.
won't fix, minor
| }; | ||
|
|
||
| const HTTP_SCHEME_REGEX = /^https?:\/\//i; | ||
| // AWS Bedrock InvokeModel URL, e.g. https://bedrock-runtime.{region}.amazonaws.com |
There was a problem hiding this comment.
Note [CRF-13] The mantle endpoint contract (host shape + required /anthropic suffix) is re-encoded client-side in BEDROCK_MANTLE_URL_REGEX and spelled out in four places: the regex, bedrockMantleBaseUrl, the description hint, and the schema error message. (Pariston, Melody, Zoro)
If AWS or the SDK revises the mantle URL contract, valid endpoints get rejected client-side or the region parse and schema disagree. This follows the pre-existing BEDROCK_CANONICAL_URL_REGEX coupling, so it is an inference about where a future break would originate, not a change required in this PR.
🤖
There was a problem hiding this comment.
Correct, but I followed pre-existing BEDROCK_CANONICAL_URL_REGEX coupling.
| ), | ||
| }) | ||
| .required("Endpoint is required"), | ||
| apiKey: Yup.string(), |
There was a problem hiding this comment.
Note [CRF-14] The model and smallFastModel schema entries are the same .when("protocol", { is: p => p !== "mantle", ... }) block twice, differing only in the message. (Robin)
Two instances is not yet worth an abstraction, but a third protocol-gated field would justify a small requiredUnlessMantle(message) helper. Noting the shape so the next addition reuses rather than copies.
🤖
There was a problem hiding this comment.
won't fix, minor
7ba0a5b to
87bdcaf
Compare
632aa1d to
68b0594
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
Round 2. The author addressed the substance quickly: CRF-4 (P1) is now a single atomic form.setValues({ ...form.values, protocol, baseUrl }) with a new story asserting Save stays enabled after the switch; CRF-1, CRF-5, and CRF-7 (the P3s) have code and story fixes; CRF-2 and CRF-10 (Nits) are renamed and de-duplicated; and CRF-3 was answered (the us-east-1 default is intentional, easier mantle setup). Those fixes are claimed but not yet panel-verified; the panel will verify them next round.
Panel re-review is blocked this round because eight findings received no code change and no response. Under this review's no-follow-up assumption, a silent Nit or Note stays in the code forever, so each needs a disposition: fix it, or reply with either a reason it should not change or a tracking ticket.
- CRF-6 (Note, ProviderForm.stories.tsx:143): AddBedrockMantle still seeds a mantle protocol with an InvokeModel endpoint and asserts only the hint text, not the endpoint field.
- CRF-8 (Note, ProviderForm.tsx:586): the "recommended by AWS" hint copy is an unsourced third-party claim, and the shipped default (InvokeModel) contradicts it. This one needs a human decision.
- CRF-9 (Nit, ProviderForm.tsx:594): the Endpoint description ternary still duplicates the whole
In the format of <code>wrapper (four reviewers flagged it). - CRF-11 (Nit, providerFormApiMap.test.ts:421): comments that restate the code or narrate intent rather than describe behavior.
- CRF-12 (Note, providerFormApiMap.ts:263): an unrecognized stored protocol renders an empty Select.
- CRF-13 (Note, ProviderForm.tsx:49): the mantle URL contract is spelled out in four places.
- CRF-14 (Note, ProviderForm.tsx:219): the two model-field
.whenschema blocks are duplicated. - CRF-15 (Note, providerFormApiMap.test.ts:621): no mantle row in the providerFormValuesToUpdate suite.
None of these is blocking on its own; a one-line reply per thread (fix, ticket, or "won't fix because...") clears the block and lets the panel verify the fixes above. Separately, the doc-check bot flagged that docs/ai-coder/ai-gateway/providers.md still documents only the InvokeModel endpoint and no protocol choice; worth folding into this PR or a follow-up.
The prior REQUEST_CHANGES stands until the panel verifies the CRF-4 fix next round.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
There was a problem hiding this comment.
Round 3. The blocking P1 is fixed at the root cause, so I'm dismissing the earlier changes request. handleBedrockProtocolChange now writes protocol and baseUrl in one atomic form.setValues, so validation runs once against the complete next state and the phantom-disabled-Save class is gone in both switch directions, not just the tested one; Netero and the panel (Meruem, Mafuuu, Pariston, Razor, Nami) verified the mechanism independently. The three P3s are genuinely fixed with tests that have teeth: AddBedrockProtocolSwitchKeepsSaveEnabled starts from a non-default eu-west-1 URL and asserts the rewritten endpoint plus enabled Save (CRF-1, CRF-4), AddBedrockMantleRejectsInvokeUrl blocks submit on the mantle .when branch and asserts onSubmit never fires (CRF-5), and the InvokeModel endpoint error now names its format (CRF-7). The two Nits (CRF-2 rename, CRF-10 helper dedup) are done. Unit suite 96 passing; the panel could not run the Storybook browser project on this runner, so the story verdicts are by reading each play against the rendered tree, not a green run.
The author dispositioned the round-1 Nits/Notes by reply. The panel respects those human decisions and found no new evidence to reopen them, with one residual worth an informed final call: CRF-8's hint says Mantle is "recommended by AWS for new deployments" with no source, while the form ships InvokeModel as the default. Pariston notes the backend itself calls InvokeModel "legacy," so defaulting to it is a sound backward-compatible choice; the only weak part is the unsourced AWS attribution. Source it or soften it, your call. CRF-3 (the us-east-1 default) holds as intentional; consider one line in the PR description so the default change is discoverable.
Two new low-severity items this round, both optional: CRF-16 (rename BEDROCK_CANONICAL_URL_REGEX to match the new protocol-paired naming) and CRF-17 (the fallback-region arm and the mantle->InvokeModel reverse switch have no test). Also still open from round 1: CRF-15 (no mantle row in the providerFormValuesToUpdate suite), low value now that the shared buildBedrockSettings mantle path is covered on the create side.
Hisoka summed the round up: "I came looking for a fight worth having. This one earned the silence."
Dismissing the changes request; the remaining items are polish, coverage, and one editorial call.
🤖 This review was automatically generated with Coder Agents.
|
|
||
| const HTTP_SCHEME_REGEX = /^https?:\/\//i; | ||
| // AWS Bedrock InvokeModel URL, e.g. https://bedrock-runtime.{region}.amazonaws.com | ||
| const BEDROCK_CANONICAL_URL_REGEX = |
There was a problem hiding this comment.
Nit [CRF-16] BEDROCK_CANONICAL_URL_REGEX is the only member of the protocol-paired set not named by its protocol. (Gon)
The PR adds the sibling
BEDROCK_MANTLE_URL_REGEXand the builder pairbedrockInvokeModelBaseUrl/bedrockMantleBaseUrl... only the regex is named "canonical."
The tell is the // AWS Bedrock InvokeModel URL comment added directly above it to map "canonical" onto "InvokeModel." Rename to BEDROCK_INVOKE_MODEL_URL_REGEX (two call sites) and that clarifying comment becomes redundant. Optional, and tied to the naming convention this PR introduces.
🤖
| // the region the user already entered so they do not retype it. | ||
| const handleBedrockProtocolChange = (protocol: AIProviderBedrockProtocol) => { | ||
| const region = | ||
| parseBedrockRegionFromBaseUrl(form.values.baseUrl) ?? |
There was a problem hiding this comment.
Note [CRF-17] handleBedrockProtocolChange's ?? BEDROCK_DEFAULT_REGION fallback and the mantle -> InvokeModel reverse switch are untested. (Bisky, Mafu-san, Netero)
The new switch stories all start from a URL with a parseable region and go InvokeModel -> Mantle. Nothing exercises the fallback arm (blank or non-canonical baseUrl, then switch, expecting us-east-1), and nothing switches the reverse direction. The atomic setValues fix is direction-independent, so the reverse path is low-risk, but the ?? arm is a live branch in new code. A story that seeds baseUrl: "", switches to Mantle, and asserts the endpoint becomes https://bedrock-mantle.us-east-1.api.aws/anthropic proves it.
🤖
…idy bedrock endpoint hint and cover protocol switches - Collapse the duplicated "In the format of" endpoint description ternary into a single wrapper that only swaps the URL. - Rename BEDROCK_CANONICAL_URL_REGEX to BEDROCK_INVOKE_MODEL_URL_REGEX so it is named by protocol like its mantle sibling. - Add stories covering the Mantle to InvokeModel reverse switch and the region fallback when the endpoint has no parseable region.
On the Protocol hint copy ( FYI I also pushed a small commit addressing a few of the review nits: collapsed the duplicated endpoint-hint ternary, renamed |
I like the changes.
That’s a good point. I left I’ll give it some more thought when I add the documentation. |
2 participants
Implements: https://linear.app/codercom/issue/AIGOV-517/add-ui-for-bedrock-mantle
Follow-up to #26745, which added AWS Bedrock mantle support to the backend and modeled it as a
protocolfield on the Bedrock provider settings.What changed
https://bedrock-mantle.{region}.api.aws/anthropicURL; InvokeModel keeps thehttps://bedrock-runtime.{region}.amazonaws.comshape. The/anthropicsuffix is required for mantle because the SDK appends/v1/messagesto the base URL.Manual Testing
Tested the following scenarios:
Screenshots
InvokeModel
Mantle