refactor(auth): simplify credential source reuse

Reuse credential-source APIs across provider discovery and tests, target known-provider LLM client resolution, and remove duplicated hook/source plumbing from the review cleanup.
This commit is contained in:
Bryan Helmkamp 2026-04-24 11:09:32 -04:00
parent 8057739c85
commit 10419c7264
No known key found for this signature in database
18 changed files with 266 additions and 323 deletions

1
Cargo.lock generated
View file

@ -2309,7 +2309,6 @@ dependencies = [
"futures",
"git2",
"hex",
"httpmock",
"md5",
"mime_guess",
"object_store",

View file

@ -36,12 +36,12 @@ use std::sync::Arc;
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let source = EnvCredentialSource::new();
let client = Client::from_source(&source).await?.as_ref().clone();
let client = Client::from_source(&source).await?;
let sandbox = Arc::new(LocalSandbox::new(PathBuf::from(".")));
let profile = Arc::new(AnthropicProfile::new("claude-sonnet-4-5"));
let config = SessionOptions::default();
let mut session = Session::new(client, profile, sandbox, config);
let mut session = Session::new(client, profile, sandbox, config, None);
session.initialize().await;
// Subscribe to events before sending input
@ -72,6 +72,7 @@ pub fn new(
provider_profile: Arc<dyn AgentProfile>,
sandbox: Arc<dyn Sandbox>,
config: SessionOptions,
subagent_manager: Option<Arc<tokio::sync::Mutex<fabro_agent::subagent::SubAgentManager>>>,
) -> Self
```
@ -447,7 +448,7 @@ You cannot use both `.prompt()` and `.messages()` on the same request — this r
| Method | Type | Description |
|---|---|---|
| `new(model, client)` | `(impl Into<String>, Arc<Client>)` | Required. Model ID or alias plus the client to use |
| `new(model, client)` | `(impl Into<String>, impl Into<Arc<Client>>)` | Required. Model ID or alias plus the client to use |
| `.prompt(text)` | `impl Into<String>` | Convenience: sends a single user message |
| `.messages(msgs)` | `Vec<Message>` | Full conversation history |
| `.system(text)` | `impl Into<String>` | System prompt |

View file

@ -448,11 +448,11 @@ pub async fn run_with_args_and_source(
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
let provider = parse_provider(&args)?;
let client = Client::from_source(llm_source.as_ref())
let client = Client::from_source_for(llm_source.as_ref(), &[provider])
.await
.map_err(|e| anyhow::anyhow!("Failed to create LLM client: {e}"))?;
ensure_provider_registered(&client, provider)?;
run_with_args_and_client(args, client, mcp_servers).await
run_with_resolved_provider(args, provider, client, mcp_servers).await
}
#[allow(
@ -462,6 +462,22 @@ pub async fn run_with_args_and_source(
)]
pub async fn run_with_args_and_client(
args: AgentArgs,
client: Client,
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
let provider = parse_provider(&args)?;
ensure_provider_registered(&client, provider)?;
run_with_resolved_provider(args, provider, client, mcp_servers).await
}
#[allow(
clippy::print_stdout,
clippy::print_stderr,
reason = "Assistant output stays on stdout while prompts and diagnostics use stderr."
)]
async fn run_with_resolved_provider(
args: AgentArgs,
provider: Provider,
mut client: Client,
mcp_servers: Vec<McpServerSettings>,
) -> anyhow::Result<()> {
@ -469,9 +485,6 @@ pub async fn run_with_args_and_client(
// threads
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
let provider = parse_provider(&args)?;
ensure_provider_registered(&client, provider)?;
if args.verbose {
client.add_middleware(Arc::new(VerboseMiddleware { styles }));
} else if args.debug {

View file

@ -13,5 +13,19 @@ pub struct ResolvedCredentials {
pub trait CredentialSource: Send + Sync {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials>;
async fn resolve_providers(
&self,
providers: &[Provider],
) -> anyhow::Result<ResolvedCredentials> {
let mut resolved = self.resolve().await?;
resolved
.credentials
.retain(|credential| providers.contains(&credential.provider));
resolved
.auth_issues
.retain(|(provider, _)| providers.contains(provider));
Ok(resolved)
}
async fn configured_providers(&self) -> Vec<Provider>;
}

View file

@ -89,6 +89,22 @@ impl CredentialSource for EnvCredentialSource {
})
}
async fn resolve_providers(
&self,
providers: &[Provider],
) -> anyhow::Result<ResolvedCredentials> {
let credentials = providers
.iter()
.copied()
.filter_map(|provider| self.credential_for(provider))
.collect();
Ok(ResolvedCredentials {
credentials,
auth_issues: Vec::new(),
})
}
async fn configured_providers(&self) -> Vec<Provider> {
Provider::ALL
.iter()

View file

@ -8,6 +8,8 @@ use tokio::sync::RwLock as AsyncRwLock;
use tokio::task::spawn_blocking;
use crate::credential::{ApiKeyHeader, AuthCredential, AuthDetails, credential_id_for};
use crate::credential_source::CredentialSource;
use crate::env_source::EnvCredentialSource;
use crate::refresh::refresh_oauth_credential;
use crate::vault_ext::{vault_get_credential, vault_set_credential};
@ -180,8 +182,12 @@ impl CredentialResolver {
}
}
#[must_use]
pub fn configured_providers(&self, vault: &Vault) -> Vec<Provider> {
pub async fn configured_providers(&self) -> Vec<Provider> {
let vault = self.vault.read().await;
self.configured_providers_for_vault(&vault)
}
fn configured_providers_for_vault(&self, vault: &Vault) -> Vec<Provider> {
Provider::ALL
.iter()
.copied()
@ -312,20 +318,11 @@ pub async fn configured_providers_from_process_env(
) -> Vec<Provider> {
match vault {
Some(vault_arc) => {
let resolver = CredentialResolver::new(Arc::clone(vault_arc));
let guard = vault_arc.read().await;
resolver.configured_providers(&guard)
CredentialResolver::new(Arc::clone(vault_arc))
.configured_providers()
.await
}
None => Provider::ALL
.iter()
.copied()
.filter(|provider| {
provider
.api_key_env_vars()
.iter()
.any(|env_var| std::env::var(env_var).is_ok())
})
.collect(),
None => EnvCredentialSource::new().configured_providers().await,
}
}
@ -726,9 +723,7 @@ mod tests {
)
.unwrap();
let resolver = test_resolver(vault, Arc::new(|_| None));
let vault = resolver.vault.read().await;
assert_eq!(resolver.configured_providers(&vault), vec![
assert_eq!(resolver.configured_providers().await, vec![
Provider::OpenAi
]);
}
@ -741,9 +736,7 @@ mod tests {
vault,
Arc::new(|name| (name == "OPENAI_API_KEY").then(|| "env-key".to_string())),
);
let vault = resolver.vault.read().await;
assert_eq!(resolver.configured_providers(&vault), vec![
assert_eq!(resolver.configured_providers().await, vec![
Provider::OpenAi
]);
}

View file

@ -10,15 +10,14 @@ use crate::{CredentialResolver, CredentialUsage, EnvLookup, ResolveError, Resolv
#[derive(Clone)]
pub struct VaultCredentialSource {
vault: Arc<AsyncRwLock<Vault>>,
resolver: CredentialResolver,
}
impl VaultCredentialSource {
#[must_use]
pub fn new(vault: Arc<AsyncRwLock<Vault>>) -> Self {
let resolver = CredentialResolver::new(Arc::clone(&vault));
Self { vault, resolver }
let resolver = CredentialResolver::new(vault);
Self { resolver }
}
#[must_use]
@ -27,8 +26,8 @@ impl VaultCredentialSource {
F: Fn(&str) -> Option<String> + Send + Sync + 'static,
{
let env_lookup: EnvLookup = Arc::new(env_lookup);
let resolver = CredentialResolver::with_env_lookup(Arc::clone(&vault), env_lookup);
Self { vault, resolver }
let resolver = CredentialResolver::with_env_lookup(vault, env_lookup);
Self { resolver }
}
}
@ -42,10 +41,17 @@ impl std::fmt::Debug for VaultCredentialSource {
#[async_trait]
impl CredentialSource for VaultCredentialSource {
async fn resolve(&self) -> anyhow::Result<ResolvedCredentials> {
self.resolve_providers(Provider::ALL).await
}
async fn resolve_providers(
&self,
providers: &[Provider],
) -> anyhow::Result<ResolvedCredentials> {
let mut credentials = Vec::new();
let mut auth_issues = Vec::new();
for provider in Provider::ALL {
for provider in providers {
match self
.resolver
.resolve(*provider, CredentialUsage::ApiRequest)
@ -64,8 +70,7 @@ impl CredentialSource for VaultCredentialSource {
}
async fn configured_providers(&self) -> Vec<Provider> {
let vault = self.vault.read().await;
self.resolver.configured_providers(&vault)
self.resolver.configured_providers().await
}
}
@ -75,12 +80,12 @@ mod tests {
use chrono::{Duration, Utc};
use fabro_model::Provider;
use fabro_vault::{SecretType, Vault};
use fabro_vault::Vault;
use tokio::sync::RwLock as AsyncRwLock;
use super::VaultCredentialSource;
use crate::credential::{AuthCredential, AuthDetails, OAuthConfig, OAuthTokens};
use crate::{CredentialSource, ResolveError};
use crate::{CredentialSource, ResolveError, vault_set_credential};
fn api_key_credential(provider: Provider, key: &str) -> AuthCredential {
AuthCredential {
@ -117,23 +122,13 @@ mod tests {
async fn resolve_returns_credentials_and_auth_issues() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&expired_openai_credential()).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&api_key_credential(Provider::Anthropic, "anthropic-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault_set_credential(&mut vault, "openai_codex", &expired_openai_credential()).unwrap();
vault_set_credential(
&mut vault,
"anthropic",
&api_key_credential(Provider::Anthropic, "anthropic-key"),
)
.unwrap();
let source =
VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None);
@ -156,24 +151,18 @@ mod tests {
async fn configured_providers_reads_from_vault_without_refreshing() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai",
&serde_json::to_string(&api_key_credential(Provider::OpenAi, "openai-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&api_key_credential(Provider::Anthropic, "anthropic-key"))
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
vault_set_credential(
&mut vault,
"openai",
&api_key_credential(Provider::OpenAi, "openai-key"),
)
.unwrap();
vault_set_credential(
&mut vault,
"anthropic",
&api_key_credential(Provider::Anthropic, "anthropic-key"),
)
.unwrap();
let source =
VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None);

View file

@ -95,7 +95,6 @@ mod tests {
context: &HookContext,
_sandbox: Arc<dyn Sandbox>,
_work_dir: Option<&Path>,
_llm_source: &dyn fabro_auth::CredentialSource,
) -> HookResult {
self.captured_contexts.lock().unwrap().push(context.clone());
HookResult {

View file

@ -49,7 +49,6 @@ pub trait HookExecutor: Send + Sync {
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
work_dir: Option<&Path>,
llm_source: &dyn CredentialSource,
) -> HookResult;
}
@ -76,9 +75,16 @@ where
}
/// Executes hooks via shell commands or HTTP POST.
pub struct HookExecutorImpl;
pub struct HookExecutorImpl {
llm_source: Arc<dyn CredentialSource>,
}
impl HookExecutorImpl {
#[must_use]
pub fn new(llm_source: Arc<dyn CredentialSource>) -> Self {
Self { llm_source }
}
/// Parse a hook decision from JSON stdout and exit code.
fn parse_decision(exit_code: i32, stdout: &str) -> HookDecision {
if exit_code == 0 {
@ -616,7 +622,6 @@ impl HookExecutor for HookExecutorImpl {
context: &HookContext,
sandbox: Arc<dyn Sandbox>,
work_dir: Option<&Path>,
llm_source: &dyn CredentialSource,
) -> HookResult {
use std::sync::OnceLock;
static HTTP_CLIENTS: OnceLock<HttpClientCache> = OnceLock::new();
@ -674,7 +679,7 @@ impl HookExecutor for HookExecutorImpl {
model.as_deref(),
context,
&env,
llm_source,
self.llm_source.as_ref(),
)
.await
}
@ -698,7 +703,7 @@ impl HookExecutor for HookExecutorImpl {
context,
sandbox,
&env,
llm_source,
self.llm_source.as_ref(),
)
.await
}
@ -816,54 +821,42 @@ mod tests {
#[tokio::test]
async fn command_executor_host_success() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = make_definition("exit 0");
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert_eq!(result.decision, HookDecision::Proceed);
assert_eq!(result.hook_name.as_deref(), Some("test-hook"));
}
#[tokio::test]
async fn command_executor_host_failure() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = make_definition("exit 1");
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
#[tokio::test]
async fn command_executor_host_skip_via_exit_2() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = make_definition("exit 2");
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
#[tokio::test]
async fn command_executor_host_json_decision() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = make_definition(r#"echo '{"decision": "skip", "reason": "test skip"}'"#);
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert_eq!(result.decision, HookDecision::Skip {
reason: Some("test skip".into()),
});
@ -871,22 +864,19 @@ mod tests {
#[tokio::test]
async fn command_executor_env_vars_set() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
// Print env vars to stdout for verification
let def = make_definition("echo $ARC_EVENT:$ARC_RUN_ID:$ARC_WORKFLOW");
let mut ctx = make_context();
ctx.node_id = Some("plan".into());
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert_eq!(result.decision, HookDecision::Proceed);
}
#[tokio::test]
async fn no_hook_type_blocks() {
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = HookDefinition {
name: None,
event: HookEvent::StageStart,
@ -899,10 +889,7 @@ mod tests {
};
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
assert!(matches!(result.decision, HookDecision::Block { .. }));
}
@ -1276,7 +1263,7 @@ mod tests {
})
.await;
let executor = HookExecutorImpl;
let executor = HookExecutorImpl::new(test_llm_source());
let def = HookDefinition {
name: Some("http-test".into()),
event: HookEvent::StageStart,
@ -1294,10 +1281,7 @@ mod tests {
};
let ctx = make_context();
let sandbox = make_sandbox();
let source = test_llm_source();
let result = executor
.execute(&def, &ctx, sandbox, None, source.as_ref())
.await;
let result = executor.execute(&def, &ctx, sandbox, None).await;
mock.assert_async().await;
assert_eq!(result.decision, HookDecision::Proceed);

View file

@ -4,8 +4,6 @@ use std::sync::Arc;
use fabro_agent::Sandbox;
use fabro_auth::CredentialSource;
#[cfg(test)]
use fabro_auth::EnvCredentialSource;
use crate::config::{HookDefinition, HookSettings};
use crate::executor::{HookExecutor, HookExecutorImpl};
@ -16,7 +14,6 @@ use crate::types::{HookContext, HookDecision};
pub struct HookRunner {
config: HookSettings,
executor: Arc<dyn HookExecutor>,
llm_source: Arc<dyn CredentialSource>,
/// Pre-compiled regexes keyed by matcher pattern string.
compiled_matchers: HashMap<String, regex::Regex>,
}
@ -27,8 +24,7 @@ impl HookRunner {
let compiled_matchers = Self::compile_matchers(&config);
Self {
config,
executor: Arc::new(HookExecutorImpl),
llm_source,
executor: Arc::new(HookExecutorImpl::new(llm_source)),
compiled_matchers,
}
}
@ -40,7 +36,6 @@ impl HookRunner {
Self {
config,
executor,
llm_source: Arc::new(EnvCredentialSource::new()),
compiled_matchers,
}
}
@ -146,13 +141,7 @@ impl HookRunner {
);
let result = self
.executor
.execute(
hook,
context,
sandbox.clone(),
work_dir,
self.llm_source.as_ref(),
)
.execute(hook, context, sandbox.clone(), work_dir)
.await;
tracing::debug!(
hook = %hook.effective_name(),
@ -200,13 +189,7 @@ impl HookRunner {
);
let result = self
.executor
.execute(
hook,
context,
sandbox.clone(),
work_dir,
self.llm_source.as_ref(),
)
.execute(hook, context, sandbox.clone(), work_dir)
.await;
tracing::debug!(
hook = %hook.effective_name(),
@ -248,7 +231,6 @@ mod tests {
_context: &HookContext,
_sandbox: Arc<dyn Sandbox>,
_work_dir: Option<&Path>,
_llm_source: &dyn CredentialSource,
) -> HookResult {
HookResult {
hook_name: definition.name.clone(),

View file

@ -52,6 +52,28 @@ impl Client {
Self::from_credentials(resolved.credentials).await
}
/// Create a Client from a credential source, resolving only the requested
/// providers.
///
/// # Errors
///
/// Returns `Error` if the source cannot resolve credentials or any provider
/// adapter fails to initialize.
pub async fn from_source_for(
source: &dyn CredentialSource,
providers: &[fabro_model::Provider],
) -> Result<Self, Error> {
let resolved =
source
.resolve_providers(providers)
.await
.map_err(|err| Error::Configuration {
message: format!("Failed to resolve LLM credentials: {err}"),
source: None,
})?;
Self::from_credentials(resolved.credentials).await
}
/// Create a Client from typed provider credentials.
///
/// # Errors

View file

@ -296,30 +296,30 @@ pub struct GenerateParams {
}
impl GenerateParams {
pub fn new(model: impl Into<String>, client: Arc<Client>) -> Self {
pub fn new(model: impl Into<String>, client: impl Into<Arc<Client>>) -> Self {
Self {
model: model.into(),
prompt: None,
messages: None,
system: None,
tools: None,
tool_choice: None,
max_tool_rounds: 1,
response_format: None,
temperature: None,
top_p: None,
max_tokens: None,
stop_sequences: None,
model: model.into(),
prompt: None,
messages: None,
system: None,
tools: None,
tool_choice: None,
max_tool_rounds: 1,
response_format: None,
temperature: None,
top_p: None,
max_tokens: None,
stop_sequences: None,
reasoning_effort: None,
speed: None,
provider: None,
speed: None,
provider: None,
provider_options: None,
metadata: None,
max_retries: 2,
timeout: None,
client,
abort_signal: None,
stop_when: None,
metadata: None,
max_retries: 2,
timeout: None,
client: client.into(),
abort_signal: None,
stop_when: None,
repair_tool_call: None,
}
}

View file

@ -8014,17 +8014,15 @@ provider = "invalid-provider"
5,
|_| None,
);
state
.vault
.write()
.await
.set(
{
let mut vault = state.vault.write().await;
fabro_auth::vault_set_credential(
&mut vault,
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
&openai_api_key_credential("vault-openai-key"),
)
.unwrap();
}
let llm_result = state.resolve_llm_client().await.unwrap();
@ -8040,17 +8038,15 @@ provider = "invalid-provider"
5,
|_| None,
);
state
.vault
.write()
.await
.set(
{
let mut vault = state.vault.write().await;
fabro_auth::vault_set_credential(
&mut vault,
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
&openai_api_key_credential("vault-openai-key"),
)
.unwrap();
}
assert_eq!(state.llm_source.configured_providers().await, vec![
Provider::OpenAi
@ -8082,17 +8078,15 @@ provider = "invalid-provider"
_ => None,
},
);
state
.vault
.write()
.await
.set(
{
let mut vault = state.vault.write().await;
fabro_auth::vault_set_credential(
&mut vault,
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
&openai_api_key_credential("vault-openai-key"),
)
.unwrap();
}
let llm_result = state.resolve_llm_client().await.unwrap();
let response = llm_result

View file

@ -73,7 +73,6 @@ tokio = { workspace = true, features = ["test-util", "macros"] }
object_store.workspace = true
assert_cmd = "2"
predicates = "3"
httpmock = "0.8"
fabro-macros = { path = "../fabro-macros" }
fabro-test = { workspace = true }
fabro-types = { path = "../fabro-types", features = ["test-support"] }

View file

@ -35,6 +35,18 @@ fn build_profile(model: &str, provider: Provider) -> Box<dyn AgentProfile> {
}
}
fn credential_providers(primary: Provider, fallback_chain: &[FallbackTarget]) -> Vec<Provider> {
let mut providers = vec![primary];
for target in fallback_chain {
if let Ok(provider) = target.provider.parse::<Provider>() {
if !providers.contains(&provider) {
providers.push(provider);
}
}
}
providers
}
/// Shared state for tracking file modifications from agent tool calls.
struct FileTracking {
/// Maps tool_call_id → file_path for in-flight write/edit calls.
@ -203,7 +215,7 @@ impl AgentApiBackend {
tool_hooks: Option<Arc<dyn fabro_agent::ToolHookCallback>>,
mcp_servers: Vec<McpServerSettings>,
) -> Result<Session, Error> {
let client = Client::from_source(source)
let client = Client::from_source_for(source, &[provider])
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
@ -286,16 +298,28 @@ impl CodergenBackend for AgentApiBackend {
prompt: &str,
system_prompt: Option<&str>,
) -> Result<CodergenResult, Error> {
let client = Client::from_source(self.source.as_ref())
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let model = node.model().unwrap_or(&self.model);
let provider = node
.provider()
.map(String::from)
.or_else(|| Some(self.provider.as_str().to_string()));
// Build per-request fallback chain: if the node overrides the provider,
// no failover is available; otherwise use the backend's.
let fallback_chain: &[FallbackTarget] = if node.provider().is_some() {
&[]
} else {
&self.fallback_chain
};
let primary_provider = provider
.as_deref()
.and_then(|provider| provider.parse::<Provider>().ok())
.unwrap_or(self.provider);
let providers = credential_providers(primary_provider, fallback_chain);
let client = Client::from_source_for(self.source.as_ref(), &providers)
.await
.map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?;
let max_tokens = node.max_tokens().or_else(|| {
fabro_model::Catalog::builtin()
.get(model)
@ -325,14 +349,6 @@ impl CodergenBackend for AgentApiBackend {
provider_options: None,
};
// Build per-request fallback chain: if the node overrides the provider,
// no failover is available; otherwise use the backend's.
let fallback_chain: &[FallbackTarget] = if node.provider().is_some() {
&[]
} else {
&self.fallback_chain
};
let result = client.complete(&request).await;
let default_provider = self.provider.as_str().to_string();
@ -640,7 +656,7 @@ impl CodergenBackend for AgentApiBackend {
mod tests {
use fabro_agent::subagent::SessionFactory;
use fabro_auth::{AuthCredential, AuthDetails, VaultCredentialSource};
use fabro_vault::{SecretType, Vault};
use fabro_vault::Vault;
use tokio::sync::RwLock as AsyncRwLock;
use super::*;
@ -792,20 +808,13 @@ mod tests {
async fn api_backend_uses_source_credentials() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&AuthCredential {
provider: Provider::Anthropic,
details: AuthDetails::ApiKey {
key: "anthropic-key".to_string(),
},
})
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
fabro_auth::vault_set_credential(&mut vault, "anthropic", &AuthCredential {
provider: Provider::Anthropic,
details: AuthDetails::ApiKey {
key: "anthropic-key".to_string(),
},
})
.unwrap();
let backend = AgentApiBackend::new(
"claude-opus-4-6".to_string(),
Provider::Anthropic,

View file

@ -13,11 +13,10 @@ use fabro_graphviz::graph;
use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner};
use fabro_sandbox::{
ReadBeforeWriteSandbox, SandboxEventCallback, SandboxSpec, WorkdirStrategy, WorktreeOptions,
WorktreeSandbox,
WorktreeSandbox, shell_quote,
};
use fabro_vault::Vault;
use futures::future::try_join_all;
use shlex::try_quote;
use tokio::process::Command as TokioCommand;
use tokio::runtime::Handle;
use tokio::sync::RwLock as AsyncRwLock;
@ -384,6 +383,7 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> {
.arg("-c")
.arg(&shell_command)
.current_dir(&cwd)
.kill_on_drop(true)
.output(),
)
.await
@ -418,7 +418,7 @@ async fn resolve_devcontainer(options: &mut InitOptions) -> Result<(), Error> {
fabro_devcontainer::Command::Args(args) => {
let shell_command = args
.iter()
.map(|arg| try_quote(arg).unwrap_or_else(|_| arg.into()).to_string())
.map(|arg| shell_quote(arg))
.collect::<Vec<_>>()
.join(" ");
run_shell(shell_command).await?;
@ -749,7 +749,7 @@ mod tests {
use fabro_sandbox::SandboxSpec;
use fabro_store::Database;
use fabro_types::{RunId, WorkflowSettings, fixtures};
use fabro_vault::{SecretType, Vault};
use fabro_vault::Vault;
use object_store::memory::InMemory;
use tokio::sync::RwLock as AsyncRwLock;
@ -954,20 +954,13 @@ mod tests {
async fn build_registry_accepts_vault_only_llm_provider() {
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"anthropic",
&serde_json::to_string(&AuthCredential {
provider: fabro_llm::Provider::Anthropic,
details: AuthDetails::ApiKey {
key: "anthropic-key".to_string(),
},
})
.unwrap(),
SecretType::Credential,
None,
)
.unwrap();
fabro_auth::vault_set_credential(&mut vault, "anthropic", &AuthCredential {
provider: fabro_llm::Provider::Anthropic,
details: AuthDetails::ApiKey {
key: "anthropic-key".to_string(),
},
})
.unwrap();
let (graph, _) = llm_graph();
let vault = Arc::new(AsyncRwLock::new(vault));

View file

@ -605,10 +605,8 @@ mod tests {
};
use fabro_store::Database;
use fabro_types::{BilledTokenCounts, RunSpec, SuccessReason, fixtures};
use fabro_vault::{SecretType, Vault};
use fabro_vault::Vault;
use futures::stream;
use httpmock::Method::POST;
use httpmock::MockServer;
use object_store::memory::InMemory;
use tokio::sync::RwLock as AsyncRwLock;
@ -721,30 +719,6 @@ mod tests {
}
}
fn openai_responses_payload(text: &str) -> serde_json::Value {
serde_json::json!({
"id": "resp_1",
"model": "gpt-5.4",
"output": [
{
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": text
}
]
}
],
"status": "completed",
"usage": {
"input_tokens": 10,
"output_tokens": 20
}
})
}
fn make_test_conclusion() -> Conclusion {
Conclusion {
timestamp: Utc::now(),
@ -1289,29 +1263,24 @@ mod tests {
#[tokio::test]
async fn build_pr_body_uses_vault_only_openai_codex_source() {
let server = MockServer::start_async().await;
let response_mock = server
.mock_async(|when, then| {
when.method(POST)
.path("/v1/responses")
.header("authorization", "Bearer vault-openai-key");
then.status(200)
.header("content-type", "application/json")
.json_body(openai_responses_payload("Narrative from vault source."));
})
let twin = fabro_test::twin_openai().await;
let namespace = format!("{}::{}", module_path!(), line!());
fabro_test::TwinScenarios::new(namespace.clone())
.scenario(
fabro_test::TwinScenario::responses("gpt-5.4").text("Narrative from vault source."),
)
.load(twin)
.await;
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let base_url = server.url("/v1");
fabro_auth::vault_set_credential(
&mut vault,
"openai_codex",
&openai_api_key_credential(&namespace),
)
.unwrap();
let base_url = twin.base_url.clone();
let llm_source: Arc<dyn CredentialSource> =
Arc::new(VaultCredentialSource::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
@ -1337,7 +1306,6 @@ mod tests {
.unwrap();
assert!(body.contains("Narrative from vault source."));
response_mock.assert_async().await;
}
// ── parse_dot_summary tests ─────────────────────────────────────────

View file

@ -6610,30 +6610,6 @@ fn openai_api_key_credential(key: &str) -> fabro_auth::AuthCredential {
}
}
fn openai_responses_payload(text: &str) -> serde_json::Value {
serde_json::json!({
"id": "resp_1",
"model": "gpt-5.4",
"output": [
{
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": text
}
]
}
],
"status": "completed",
"usage": {
"input_tokens": 10,
"output_tokens": 20
}
})
}
// ---------------------------------------------------------------------------
// Wait.human freeform edge integration tests (Section 4.6)
// ---------------------------------------------------------------------------
@ -6643,21 +6619,16 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
use chrono::Utc;
use fabro_auth::{CredentialSource, VaultCredentialSource};
use fabro_types::Conclusion;
use fabro_vault::{SecretType, Vault};
use httpmock::Method::POST;
use httpmock::MockServer;
use fabro_vault::Vault;
use tokio::sync::RwLock as AsyncRwLock;
let server = MockServer::start_async().await;
let response_mock = server
.mock_async(|when, then| {
when.method(POST)
.path("/v1/responses")
.header("authorization", "Bearer vault-openai-key");
then.status(200)
.header("content-type", "application/json")
.json_body(openai_responses_payload("Narrative from vault source."));
})
let twin = fabro_test::twin_openai().await;
let namespace = format!("{}::{}", module_path!(), line!());
fabro_test::TwinScenarios::new(namespace.clone())
.scenario(
fabro_test::TwinScenario::responses("gpt-5.4").text("Narrative from vault source."),
)
.load(twin)
.await;
let mut graph = Graph::new("VaultOpenAiCodexPrBody");
@ -6684,15 +6655,13 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
let vault_dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(vault_dir.path().join("secrets.json")).unwrap();
vault
.set(
"openai_codex",
&serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(),
SecretType::Credential,
None,
)
.unwrap();
let base_url = server.url("/v1");
fabro_auth::vault_set_credential(
&mut vault,
"openai_codex",
&openai_api_key_credential(&namespace),
)
.unwrap();
let base_url = twin.base_url.clone();
let llm_source: Arc<dyn CredentialSource> = Arc::new(VaultCredentialSource::with_env_lookup(
Arc::new(AsyncRwLock::new(vault)),
move |name| match name {
@ -6757,7 +6726,6 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() {
.expect("PR body should build from vault-only credentials");
assert!(body.contains("Narrative from vault source."));
response_mock.assert_async().await;
}
/// Freeform-only human gate: free-text input routes through the freeform edge