Sorry for the wait time here @5herlocked, we'll get this reviewed ASAP.

Dw about it -- I know y'all are focused on git stuff!

Hey, sorry for taking a long time to review. This mostly looks good to me, just two things about the UI that confused me:

1. We should drop (Optional) from the Bedrock API key input since it is not actually optional if you want to auth via API key

2. When I am using an API key it says "You are using static credentials". Should be "API key configured" like we do for other providers

No issues! I totally know you're busy.

1/ Good point -- in my mind it was open to interpretation -- but I suppose the OR is pretty descriptive

2/ Absolutely. I'll make the changes rn

Tech Debt cleanup -- writeup

Problem Statement

The Bedrock provider had two parallel authentication paths that created confusion and complexity:

1. Settings-based authentication (BedrockAuthMethod): NamedProfile, SingleSignOn, Automatic
2. UX-provided credentials (BedrockCredentials): Access keys or bearer tokens stored in keychain

This led to several issues:

• Unclear precedence: If a user had both settings-based auth AND UX credentials, the behavior was ambiguous
• Overloaded None case: When auth_method was None, the code would "use UX creds if present, else default chain"
• ApiKey enum variant existed but wasn't really used: The code path for UX credentials happened when auth_method was None
• Confusing is_authenticated() check: Returned true for two different reasons with different behaviors
• Region was coupled to credentials: Even though region is orthogonal to auth method

Design Goals

1. Settings override UX credentials — Enterprises can control authentication via managed settings
2. Single source of truth — One resolved auth type, not multiple fields to check
3. Explicit auth types — IAM credentials and API Keys behave differently, model them separately
4. Region independence — Region is orthogonal to auth method, handle it separately
5. Clear UI feedback — Users should see exactly which auth method is active

Solution: Unified BedrockAuth Enum

We introduced a single enum that represents the fully-resolved authentication configuration:

pub enum BedrockAuth {
    /// Use default AWS credential provider chain (IMDSv2, PodIdentity, env vars, etc.)
    Automatic,
    
    /// Use AWS named profile from ~/.aws/credentials or ~/.aws/config
    NamedProfile { profile_name: String },
    
    /// Use AWS SSO profile
    SingleSignOn { profile_name: String },
    
    /// Use IAM credentials (access key + secret + optional session token)
    IamCredentials {
        access_key_id: String,
        secret_access_key: String,
        session_token: Option<String>,
    },
    
    /// Use Bedrock API Key (bearer token authentication)
    ApiKey { api_key: String },
}

Why split IamCredentials and ApiKey?

These authenticate completely differently in the AWS SDK:

• IAM Credentials → Credentials::new() with access_key_id, secret_access_key, optional session_token
• API Key → token_provider() with httpBearerAuth scheme

Modeling them as separate variants makes the client initialization clean and explicit.

Why separate region?

Region can be used with any auth method. Previously it was stored in BedrockCredentials, but that only made sense for UX-provided credentials. Now region resolution is independent:

1. Environment variable (ZED_AWS_REGION)
2. Settings (bedrock.region)
3. Default (us-east-1)

Authentication Resolution Flow

The new authenticate() method follows a clear priority:

1. Check settings.authentication_method (enterprise control)
   ├─ Automatic      → BedrockAuth::Automatic
   ├─ NamedProfile   → BedrockAuth::NamedProfile { profile_name }
   ├─ SingleSignOn   → BedrockAuth::SingleSignOn { profile_name }
   └─ ApiKey         → Load from keychain/env → BedrockAuth::ApiKey or IamCredentials

2. No settings auth method? Try static credentials
   ├─ Env vars (ZED_BEDROCK_BEARER_TOKEN, ZED_ACCESS_KEY_ID, etc.)
   └─ Keychain

This gives enterprises control via settings while allowing individual users to configure via the UI.

Client Initialization

The get_or_init_client() method is now a clean single match:

match auth {
    Some(BedrockAuth::Automatic) | None => {
        // Default AWS credential provider chain
    }
    Some(BedrockAuth::NamedProfile { profile_name })
    | Some(BedrockAuth::SingleSignOn { profile_name }) => {
        config_builder = config_builder.profile_name(profile_name);
    }
    Some(BedrockAuth::IamCredentials { access_key_id, secret_access_key, session_token }) => {
        let aws_creds = Credentials::new(...);
        config_builder = config_builder.credentials_provider(aws_creds);
    }
    Some(BedrockAuth::ApiKey { api_key }) => {
        config_builder = config_builder
            .auth_scheme_preference(["httpBearerAuth".into()])
            .token_provider(Token::new(api_key, None));
    }
}

UI Improvements

The configuration view now shows exactly which auth method is active:

Users can only reset credentials when using static credentials (not settings-derived auth).

Backward Compatibility

• BedrockAuthMethod enum retained for settings mapping
• BedrockCredentials struct retained for keychain serialization (minus region field)
• All existing settings continue to work
• Environment variables continue to work

Bedrock API key should be reflected in reset popup too.

And shouldn't we adhere to use AWS_BEARER_TOKEN_BEDROCK instead of ZED_BEDROCK_BEARER_TOKEN and other ZED_XXX too. So, there will be seamless integration with zed, claude code, opencode, crush and others using the same keys.

Thanks for the suggestion. I'll update the tooltip as you recommended.

Regarding the credential isolation approach: we've deliberately chosen to use separate ZED_ prefixed credentials based on real-world customer patterns. Many organizations use isolated accounts specifically for Bedrock and AI consumption, which is exactly what this approach enables. Beyond that organizational pattern, it provides a fundamental level of isolation for the Zed agent itself - making it trivially easy to distinguish actions taken by an LLM from those taken by the actual user through clear credential separation.

I understand the concern about friction, but I think the benefits outweigh it here. That said, if we see significant pushback from the community, I'm open to revisiting the decision. For now, I'd prefer to keep it as-is given the security and auditability benefits.

Worth noting too that we're actively trying to move people away from static credentials entirely, so this specific implementation detail becomes less relevant as we push toward more secure authentication patterns.

I guessed it was deliberate and tried to find the discussion about it, but couldn't. Thanks for the info.

Why is this being accepted when Azure Foundry PR was closed? #43742 I'm thinking whether that PR should be reconsidered in the light of this one. Or then implement this via the extension API if one for that is coming in the near future.

Why is this being accepted when Azure Foundry PR was closed? #43742 I'm thinking whether that PR should be reconsidered in the light of this one. Or then implement this via the extension API if one for that is coming in the near future.

It's likely because Bedrock is already a existing provider and this PR adds new functionality, while Azure Foundry is a completely new provider.

Go ahead merge 🚀

@bennetbo