P2 [CRF-4] Unknown protocol values pass API validation and silently degrade to unsigned requests at runtime.

Validate() checks that mantle requires a region but never rejects an unknown Protocol string. An operator who sets protocol: "mnatle" (typo) passes validation. At runtime, isBedrockInvokeModel() matches only "" and "invoke-model", isBedrockMantle() matches only "mantle". The unknown value matches neither, so neither signing branch in newMessagesService fires. The request proceeds to the upstream unsigned.

config.AWSBedrock.Validate() has a default reject clause, but it lives inside withBedrockInvokeModelOptions and withBedrockMantleOptions, which are never entered because the dispatch predicates already excluded the unknown value. The validation is unreachable for this failure mode.

"An operator who sets protocol: "tyop" passes validation. The value is stored. At request time, the request falls through to the generic Anthropic path with no SigV4 signing. AWS returns 403, and the operator gets a confusing error with no mention of the invalid protocol." (Pariston P3)

The same gap applies to UpdateAIProviderRequest.Validate() (line 335), which does not inspect the protocol field at all.

Fix in both Create and Update validators:

switch req.Settings.Bedrock.Protocol {
case "", AIProviderBedrockProtocolInvokeModel, AIProviderBedrockProtocolMantle:
default:
    validations = append(validations, ValidationError{
        Field:  "settings.protocol",
        Detail: fmt.Sprintf("unsupported bedrock protocol %q", req.Settings.Bedrock.Protocol),
    })
}

(Hisoka P2, Mafuuu P2, Knov P2, Meruem P2, Ryosuke P2, Pariston P3, Kurapika P3, Melody P3, Chopper P3)

🤖

P3 [CRF-2] UpdateAIProviderRequest.Validate() (line 335) does not check the protocol field at all. An update that sets protocol=mantle without providing a region, or sets an unknown protocol value, passes validation. The same protocol-enum check from CRF-4 and the mantle-region check from line 287 should be replicated in the Update validator.

(Netero, Hisoka, Knov, Meruem)

🤖

P3 [CRF-1] Missing nil guard on i.bedrock. The sibling function withBedrockInvokeModelOptions (line 323) guards with if i.bedrock == nil { return nil, xerrors.New("nil bedrock runtime") }. This function dereferences i.bedrock.Cfg directly. The caller guards via isBedrockMantle() which requires i.bedrock != nil, so this won't panic in production. The risk is a nil-dereference panic if called directly without the caller guard.

(Netero)

🤖

P3 [CRF-5] Test claims passthrough but only asserts Model().

The payload includes thinking, metadata, and context_management fields that augmentRequestForBedrockInvokeModel would strip or rewrite for InvokeModel. The point of mantle is that these fields survive untouched, but no assertion verifies it. If someone accidentally added augmentation to the mantle path, Model() would still return the client's model (it reads from reqPayload for mantle, from config for InvokeModel), and this test would still pass.

Add an assertion that i.reqPayload is unchanged after the mantle code path runs, or remove the fields that aren't being tested.

(Bisky)

🤖

P3 [CRF-6] This test is titled "InvokeModelDoesNotRequireRegionField" but Settings: codersdk.AIProviderSettings{} leaves Bedrock nil, so Validate() never enters the if req.Settings.Bedrock != nil block. The protocol check is never reached.

Set Bedrock: &codersdk.AIProviderBedrockSettings{} (zero value resolves to InvokeModel via ResolvedProtocol()) so the test enters the protocol-gated branch and proves the region check is scoped to mantle.

(Bisky)

🤖

Nit [CRF-19] hasFieldError is a manual loop that slices.ContainsFunc (available since Go 1.21) replaces. The project already uses it in tests.

hasFieldError := func(vs []codersdk.ValidationError, field string) bool {
    return slices.ContainsFunc(vs, func(v codersdk.ValidationError) bool {
        return v.Field == field
    })
}

(Ging-Go)

🤖

Nit [CRF-20] context.Background() used where t.Context() is available since Go 1.24. The same PR's integration test already uses t.Context() for the mantle path.

(Ging-Go)

🤖

Nit [CRF-3] Grammar: "Empty fallbacks to invoke-model" should be "Empty falls back to invoke-model." ("fallbacks" is a noun; the verb form is "falls back.")

(Netero)

🤖

Nit [CRF-23] time.Second*30 is a bare literal in both the mantle block (here) and the InvokeModel block (line 287). A named constant like bedrockCredentialTimeout would make the intent clear and reduce the duplication.

(Gon)

🤖

Nit [CRF-21] BedrockPRMUserAgent doc comment doesn't expand PRM. PRM is the ISV Partner Registration Marker (or Partner Registration Manager, depending on the AWS doc vintage). The value itself is a percent-encoded APN token, adding another unexplained acronym. A one-line expansion in the doc comment saves someone a search:

// BedrockPRMUserAgent is Coder's AWS Partner Registration Marker (PRM),
// appended to Bedrock User-Agent headers for AWS ISV attribution.

(Leorio)

🤖