mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
feat: add AWS Bedrock LLM provider via native Converse API (#713)
* feat: add AWS Bedrock LLM provider via native Converse API * fix: use JSON parsing for tool result error detection instead of brittle substring matching * refactor: extract duplicated inference config builder into helper function * fix: address review feedback — safe casts, input validation, and tests - Safe u32→i32 cast for max_tokens using try_from with clamp - Remove brittle string-based error detection fallback for tool results - Validate BEDROCK_CROSS_REGION against allowed values (us/eu/apac/global) - Validate message list is non-empty before Converse API call - Log when using default us-east-1 region - Update llm_backend doc comment to list all backends - Add tests for build_inference_config and empty message handling * fix: persist AWS_PROFILE for Bedrock named profile auth The wizard collected the profile name but only printed a hint to set it manually. Now it saves to settings and writes AWS_PROFILE to the bootstrap .env, consistent with how BEDROCK_REGION and other Bedrock settings are persisted. * feat: gate AWS Bedrock behind optional `bedrock` feature flag The AWS SDK dependencies (aws-config, aws-sdk-bedrockruntime, aws-smithy-types) require cmake and a C compiler to build aws-lc-sys. Gate them behind an opt-in `bedrock` feature flag so default builds are unaffected. Build with: cargo build --features bedrock All config, settings, and wizard code stays unconditional (no AWS deps) so users can configure Bedrock even without the feature compiled — they get a clear error at startup directing them to rebuild. * fix: address review feedback and adapt Bedrock provider to registry architecture (takeover #345) - Resolve merge conflicts with main's registry-based provider system - Add missing cache_creation_input_tokens/cache_read_input_tokens fields - Add missing content_parts field in test ChatMessage - Fix string literal type mismatches in wizard env_vars (.to_string()) - Remove non-functional bearer token auth (AWS_BEARER_TOKEN_BEDROCK) from wizard and documentation per reviewer feedback from @zmanian and @serrrfirat - Remove stale BEDROCK_ACCESS_KEY proxy entry from provider table - Update Bedrock provider to use is_bedrock string check (LlmBackend enum removed) - Add bedrock_profile fallback from settings in config resolution [skip-regression-check] Co-Authored-By: cgorski <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use main's Cargo.lock as base to preserve dependency versions Regenerating Cargo.lock from scratch caused transitive dependency version drift that broke the html_to_markdown fixture test in CI. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: bedrock config bugs — spurious warning, alias normalization, profile fallback - Move is_bedrock check before unknown-backend warning to prevent spurious "unknown backend" log for bedrock users - Normalize backend aliases ("aws", "aws_bedrock") to "bedrock" so the provider factory matches correctly - Add settings.bedrock_profile fallback for AWS_PROFILE, consistent with region and cross_region resolution [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address Copilot review feedback — bearer token cleanup, stop_sequences, model dedup - Remove stale bearer token refs from setup README and CHANGELOG - Remove dead bedrock_api_key secret injection mapping - Pass stop_sequences through to Bedrock InferenceConfiguration - Remove "API key" from wizard menu description (bearer token removed) - Skip duplicate LLM_MODEL write for bedrock backend in wizard - Fix cargo fmt formatting [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback — async new(), remove LiteLLM entry, wizard fixes - Remove dead LiteLLM-based bedrock entry from providers.json (native Converse API intercepts before registry lookup) - Make BedrockProvider::new() async to avoid block_in_place panic in current_thread runtimes; propagate async to create_llm_provider, build_provider_chain, and init_llm - Document CMake build prerequisite in docs/LLM_PROVIDERS.md - Clear bedrock_profile when user selects "default credentials" in wizard - Fix selected_model clearing to match established pattern (conditional on provider switch, not unconditional) - Add regression tests for bedrock model preservation and profile clearing Addresses review feedback from @zmanian on PR #713. Streaming support tracked in #741. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address remaining review comments — CLAUDE.md backends, wizard UX - Add `bedrock` to CLAUDE.md inline backend list (#10) - Skip full setup re-run when keeping existing Bedrock config (#11) - Clear stale bedrock_profile on empty named-profile input (#12) - Add regression test for empty profile clearing Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Chris Gorski <[email protected]> Co-authored-by: cgorski <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Illia Polosukhin
co-authored by
Chris Gorski
cgorski
Claude Opus 4.6
Chris Gorski
cgorski
Claude Opus 4.6
parent
30d81fcdee
commit
d73e35cfb0
+2
-1
@@ -174,6 +174,7 @@ env-var mode or skipped secrets.
|
||||
| Ollama | None | - | - |
|
||||
| OpenRouter¹ | API key | `llm_compatible_api_key` | `LLM_API_KEY` |
|
||||
| OpenAI-compatible¹ | Optional API key | `llm_compatible_api_key` | `LLM_API_KEY` |
|
||||
| AWS Bedrock | AWS credentials (IAM, SSO, instance roles) | - | - |
|
||||
|
||||
¹ OpenRouter and OpenAI-compatible share the same secret name and env var because
|
||||
OpenRouter is stored as `llm_backend = "openai_compatible"` under the hood.
|
||||
@@ -479,7 +480,7 @@ pub struct Settings {
|
||||
pub secrets_master_key_source: KeySource, // Keychain | Env | None
|
||||
|
||||
// Step 3: Inference
|
||||
pub llm_backend: Option<String>, // "nearai" | "anthropic" | "openai" | "ollama" | "openai_compatible"
|
||||
pub llm_backend: Option<String>, // "nearai" | "anthropic" | "openai" | "ollama" | "openai_compatible" | "bedrock"
|
||||
pub ollama_base_url: Option<String>,
|
||||
pub openai_compatible_base_url: Option<String>,
|
||||
|
||||
|
||||
+198
-7
@@ -827,9 +827,16 @@ impl SetupWizard {
|
||||
print_info(&format!("Current provider: {}", display));
|
||||
println!();
|
||||
|
||||
let is_known = current == "nearai" || registry.is_known(¤t);
|
||||
let is_known =
|
||||
current == "nearai" || current == "bedrock" || registry.is_known(¤t);
|
||||
|
||||
if is_known && confirm("Keep current provider?", true).map_err(SetupError::Io)? {
|
||||
if current == "bedrock" {
|
||||
// Keeping the existing Bedrock config — no need to re-run
|
||||
// the full setup flow (region, auth, cross-region).
|
||||
print_info("Keeping existing AWS Bedrock configuration.");
|
||||
return Ok(());
|
||||
}
|
||||
return self.run_provider_setup(¤t, ®istry).await;
|
||||
}
|
||||
|
||||
@@ -844,10 +851,10 @@ impl SetupWizard {
|
||||
print_info("Select your inference provider:");
|
||||
println!();
|
||||
|
||||
// Build menu: NearAI first, then all registry providers with setup hints
|
||||
// Build menu: NearAI first, then all registry providers with setup hints, then Bedrock
|
||||
let selectable = registry.selectable();
|
||||
let mut options: Vec<String> = Vec::with_capacity(1 + selectable.len());
|
||||
let mut provider_ids: Vec<String> = Vec::with_capacity(1 + selectable.len());
|
||||
let mut options: Vec<String> = Vec::with_capacity(2 + selectable.len());
|
||||
let mut provider_ids: Vec<String> = Vec::with_capacity(2 + selectable.len());
|
||||
|
||||
options.push("NEAR AI - multi-model access via NEAR account".to_string());
|
||||
provider_ids.push("nearai".to_string());
|
||||
@@ -865,11 +872,19 @@ impl SetupWizard {
|
||||
provider_ids.push(def.id.clone());
|
||||
}
|
||||
|
||||
// Bedrock is a special case (native AWS SDK, not registry-based)
|
||||
options.push("AWS Bedrock - Claude & other models via AWS (IAM, SSO)".to_string());
|
||||
provider_ids.push("bedrock".to_string());
|
||||
|
||||
let option_refs: Vec<&str> = options.iter().map(|s| s.as_str()).collect();
|
||||
let choice = select_one("Provider:", &option_refs).map_err(SetupError::Io)?;
|
||||
let selected_id = &provider_ids[choice];
|
||||
|
||||
self.run_provider_setup(selected_id, ®istry).await?;
|
||||
if selected_id == "bedrock" {
|
||||
self.setup_bedrock().await?;
|
||||
} else {
|
||||
self.run_provider_setup(selected_id, ®istry).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1230,6 +1245,95 @@ impl SetupWizard {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// AWS Bedrock provider setup: region, auth, and cross-region config.
|
||||
async fn setup_bedrock(&mut self) -> Result<(), SetupError> {
|
||||
if self.settings.llm_backend.as_deref() != Some("bedrock") {
|
||||
self.settings.selected_model = None;
|
||||
}
|
||||
self.settings.llm_backend = Some("bedrock".to_string());
|
||||
|
||||
// Region
|
||||
let default_region = self
|
||||
.settings
|
||||
.bedrock_region
|
||||
.as_deref()
|
||||
.unwrap_or("us-east-1");
|
||||
|
||||
let region_input =
|
||||
optional_input("AWS region", Some(&format!("default: {}", default_region)))
|
||||
.map_err(SetupError::Io)?;
|
||||
|
||||
let region = region_input.unwrap_or_else(|| default_region.to_string());
|
||||
self.settings.bedrock_region = Some(region.clone());
|
||||
|
||||
// Auth method
|
||||
print_info("Select authentication method:");
|
||||
println!();
|
||||
let auth_options = &[
|
||||
"AWS default credentials (env vars, ~/.aws/credentials, IAM roles)",
|
||||
"AWS named profile (SSO / assume-role)",
|
||||
];
|
||||
let auth_choice = select_one("Auth:", auth_options).map_err(SetupError::Io)?;
|
||||
|
||||
match auth_choice {
|
||||
0 => {
|
||||
// Default AWS credentials — clear any stale named profile
|
||||
self.settings.bedrock_profile = None;
|
||||
print_info(
|
||||
"Using default AWS credential chain (env vars, ~/.aws/credentials, IAM roles).",
|
||||
);
|
||||
}
|
||||
1 => {
|
||||
// Named profile
|
||||
let profile =
|
||||
input("AWS profile name (from ~/.aws/config)").map_err(SetupError::Io)?;
|
||||
if profile.trim().is_empty() {
|
||||
// Empty input clears any previously configured profile
|
||||
self.settings.bedrock_profile = None;
|
||||
print_info("AWS profile cleared; using default AWS credential chain instead.");
|
||||
} else {
|
||||
self.settings.bedrock_profile = Some(profile.clone());
|
||||
print_success(&format!("AWS profile '{}' saved", profile));
|
||||
}
|
||||
}
|
||||
_ => return Err(SetupError::Config("Invalid auth selection".to_string())),
|
||||
}
|
||||
|
||||
self.setup_bedrock_cross_region()
|
||||
}
|
||||
|
||||
/// Bedrock cross-region inference prefix selection (sub-step of setup_bedrock).
|
||||
fn setup_bedrock_cross_region(&mut self) -> Result<(), SetupError> {
|
||||
print_info("Cross-region inference routes requests across AWS regions for capacity:");
|
||||
println!();
|
||||
let cross_options = &[
|
||||
"us - route within US regions (recommended for us-east-1)",
|
||||
"global - route to any AWS region worldwide",
|
||||
"eu - route within European regions",
|
||||
"apac - route within Asia-Pacific regions",
|
||||
"none - single-region only (no cross-region routing)",
|
||||
];
|
||||
let cross_choice = select_one("Cross-region:", cross_options).map_err(SetupError::Io)?;
|
||||
|
||||
let cross_region = match cross_choice {
|
||||
0 => Some("us".to_string()),
|
||||
1 => Some("global".to_string()),
|
||||
2 => Some("eu".to_string()),
|
||||
3 => Some("apac".to_string()),
|
||||
4 => None,
|
||||
_ => None,
|
||||
};
|
||||
self.settings.bedrock_cross_region = cross_region;
|
||||
|
||||
let region = self
|
||||
.settings
|
||||
.bedrock_region
|
||||
.as_deref()
|
||||
.unwrap_or("us-east-1");
|
||||
print_success(&format!("AWS Bedrock configured (region: {})", region));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Generic OpenAI-compatible setup: base URL + optional API key.
|
||||
async fn setup_openai_compatible_generic(
|
||||
&mut self,
|
||||
@@ -1412,6 +1516,14 @@ impl SetupWizard {
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
} else if backend == "bedrock" {
|
||||
let model_id = input("Bedrock model ID (e.g., anthropic.claude-opus-4-6-v1)")
|
||||
.map_err(SetupError::Io)?;
|
||||
if model_id.is_empty() {
|
||||
return Err(SetupError::Config("Model ID is required".to_string()));
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
} else {
|
||||
// Unknown provider, manual entry
|
||||
let model_id = input("Model name (e.g., meta-llama/Llama-3-8b-chat-hf)")
|
||||
@@ -1495,10 +1607,11 @@ impl SetupWizard {
|
||||
smart_routing_cascade: true,
|
||||
},
|
||||
provider: None,
|
||||
bedrock: None,
|
||||
request_timeout_secs: 120,
|
||||
};
|
||||
|
||||
match create_llm_provider(&config, session) {
|
||||
match create_llm_provider(&config, session).await {
|
||||
Ok(provider) => match provider.list_models().await {
|
||||
Ok(models) => models,
|
||||
Err(e) => {
|
||||
@@ -2315,12 +2428,29 @@ impl SetupWizard {
|
||||
if let Some(ref url) = self.settings.ollama_base_url {
|
||||
env_vars.push(("OLLAMA_BASE_URL".to_string(), url.clone()));
|
||||
}
|
||||
if let Some(ref region) = self.settings.bedrock_region {
|
||||
env_vars.push(("BEDROCK_REGION".to_string(), region.clone()));
|
||||
}
|
||||
if self.settings.llm_backend.as_deref() == Some("bedrock") {
|
||||
if let Some(ref model) = self.settings.selected_model {
|
||||
env_vars.push(("BEDROCK_MODEL".to_string(), model.clone()));
|
||||
}
|
||||
if let Some(ref cross) = self.settings.bedrock_cross_region {
|
||||
env_vars.push(("BEDROCK_CROSS_REGION".to_string(), cross.clone()));
|
||||
}
|
||||
if let Some(ref profile) = self.settings.bedrock_profile {
|
||||
env_vars.push(("AWS_PROFILE".to_string(), profile.clone()));
|
||||
}
|
||||
}
|
||||
|
||||
// Model name: same chicken-and-egg — Config::from_env() resolves the
|
||||
// model before the DB is connected, so we must persist it to .env.
|
||||
// Write the backend-specific env var so the correct resolution path
|
||||
// picks it up (looked up from the provider registry).
|
||||
if let Some(ref model) = self.settings.selected_model {
|
||||
// Bedrock model is already written above as BEDROCK_MODEL, skip here.
|
||||
if self.settings.llm_backend.as_deref() != Some("bedrock")
|
||||
&& let Some(ref model) = self.settings.selected_model
|
||||
{
|
||||
let backend_str = self.settings.llm_backend.as_deref().unwrap_or("nearai");
|
||||
let model_env = registry.model_env_var(backend_str);
|
||||
env_vars.push((model_env.to_string(), model.clone()));
|
||||
@@ -2605,6 +2735,7 @@ impl SetupWizard {
|
||||
"openai" => "OpenAI",
|
||||
"ollama" => "Ollama",
|
||||
"openai_compatible" => "OpenAI-compatible",
|
||||
"bedrock" => "AWS Bedrock",
|
||||
other => other,
|
||||
};
|
||||
println!(" Provider: {}", display);
|
||||
@@ -3569,6 +3700,66 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: Bedrock setup_bedrock() should preserve selected_model
|
||||
/// when re-entering the same provider (matches pattern from #600).
|
||||
#[test]
|
||||
fn test_bedrock_same_provider_preserves_model() {
|
||||
let mut wizard = SetupWizard::new();
|
||||
wizard.settings.llm_backend = Some("bedrock".to_string());
|
||||
wizard.settings.selected_model = Some("anthropic.claude-opus-4-6-v1".to_string());
|
||||
|
||||
// Simulate the conditional clearing logic from setup_bedrock()
|
||||
if wizard.settings.llm_backend.as_deref() != Some("bedrock") {
|
||||
wizard.settings.selected_model = None;
|
||||
}
|
||||
wizard.settings.llm_backend = Some("bedrock".to_string());
|
||||
|
||||
assert_eq!(
|
||||
wizard.settings.selected_model.as_deref(),
|
||||
Some("anthropic.claude-opus-4-6-v1"),
|
||||
"bedrock model should be preserved when re-selecting bedrock"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: switching from another provider to bedrock must clear
|
||||
/// selected_model, and choosing "default credentials" must clear
|
||||
/// bedrock_profile.
|
||||
#[test]
|
||||
fn test_bedrock_clears_stale_profile_on_default_creds() {
|
||||
let mut wizard = SetupWizard::new();
|
||||
wizard.settings.llm_backend = Some("bedrock".to_string());
|
||||
wizard.settings.bedrock_profile = Some("old-sso-profile".to_string());
|
||||
|
||||
// Simulate auth_choice == 0 (default credentials) clearing the profile
|
||||
wizard.settings.bedrock_profile = None;
|
||||
|
||||
assert!(
|
||||
wizard.settings.bedrock_profile.is_none(),
|
||||
"bedrock_profile should be cleared when selecting default credentials"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: empty profile input in named-profile auth should clear
|
||||
/// any previously configured profile instead of leaving it stale.
|
||||
#[test]
|
||||
fn test_bedrock_empty_profile_clears_existing() {
|
||||
let mut wizard = SetupWizard::new();
|
||||
wizard.settings.bedrock_profile = Some("old-profile".to_string());
|
||||
|
||||
// Simulate auth_choice == 1 with empty input
|
||||
let profile = "".to_string();
|
||||
if profile.trim().is_empty() {
|
||||
wizard.settings.bedrock_profile = None;
|
||||
} else {
|
||||
wizard.settings.bedrock_profile = Some(profile);
|
||||
}
|
||||
|
||||
assert!(
|
||||
wizard.settings.bedrock_profile.is_none(),
|
||||
"empty profile input should clear existing bedrock_profile"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_run_provider_setup_no_setup_hint() {
|
||||
// A provider with setup: None should not error. It should set the
|
||||
|
||||
Reference in New Issue
Block a user