From 3510d5081d911bfc3a0e18d272b1727323e2b33b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 10 Sep 2026 09:16:39 -0600 Subject: [PATCH] Implement the lithos CredentialProvider trait directly fabro-auth defined its own `CredentialSource` trait beside the lithos `CredentialProvider`, with a parallel `ResolveError` and an adapter between them, because lithos had no way to ask which providers a store can serve right now. It does now: `credentials::readiness`, `ClientBuilder::build_ready`, and `CredentialError::Unusable`. - The vault, SQL vault, API-key, and extra-headers stores implement `CredentialProvider` directly. Material that is present but unusable (an expired token with no refresh, a wrong-typed vault entry, a header secret that did not resolve, a store read failure) is `CredentialError::Unusable` with the operator-facing reason; its `Display` replaces `auth_issue_message`. `is_configured` is the cheap presence check. - `fabro_llm::build_client` calls `build_ready`; `FabroClient::auth_issues` carries `CredentialError`. `fabro_llm::configured_providers` replaces the per-store `configured_providers` method. - `CredentialSource`, `ResolvedCredentials`, `lithos_credentials`, `ResolveError`, and `auth_issue_message` are deleted. Twenty files that held `Arc` hold `Arc`. Co-Authored-By: Claude Fable 5.1 --- docs/internal/llm-client-resolution.md | 6 +- docs/public/reference/sdk.mdx | 2 +- lib/apps/fabro-cli/src/command_context.rs | 11 +- lib/apps/fabro-server/src/diagnostics.rs | 5 +- lib/apps/fabro-server/src/run_manifest.rs | 3 +- lib/apps/fabro-server/src/server.rs | 11 +- .../fabro-server/src/server/handler/models.rs | 5 +- lib/apps/fabro-server/src/server/tests.rs | 29 +-- .../tests/it/scenario/run_completion.rs | 2 +- lib/components/fabro-agent/src/cli.rs | 7 +- lib/components/fabro-hooks/src/bridge.rs | 3 +- lib/components/fabro-hooks/src/executor.rs | 17 +- lib/components/fabro-hooks/src/runner.rs | 10 +- .../fabro-hooks/tests/host_command_hooks.rs | 5 +- lib/components/fabro-llm/src/client.rs | 47 +++-- lib/components/fabro-llm/src/lib.rs | 2 +- lib/components/fabro-llm/src/probe.rs | 5 +- .../fabro-workflow/src/handler/llm/api.rs | 12 +- .../fabro-workflow/src/operations/start.rs | 10 +- .../fabro-workflow/src/pipeline/initialize.rs | 36 ++-- .../src/pipeline/pull_request.rs | 23 ++- lib/components/fabro-workflow/src/services.rs | 37 ++-- .../fabro-workflow/src/test_support.rs | 9 +- .../fabro-workflow/tests/it/integration.rs | 7 +- .../fabro-auth/src/api_key_source.rs | 37 ++-- .../fabro-auth/src/credential_source.rs | 98 --------- lib/foundation/fabro-auth/src/error.rs | 83 -------- .../fabro-auth/src/extra_headers_source.rs | 56 ++--- lib/foundation/fabro-auth/src/lib.rs | 14 +- .../fabro-auth/src/sql_vault_source.rs | 43 ++-- lib/foundation/fabro-auth/src/test_support.rs | 6 +- lib/foundation/fabro-auth/src/vault_source.rs | 192 +++++++++--------- 32 files changed, 342 insertions(+), 491 deletions(-) delete mode 100644 lib/foundation/fabro-auth/src/credential_source.rs delete mode 100644 lib/foundation/fabro-auth/src/error.rs diff --git a/docs/internal/llm-client-resolution.md b/docs/internal/llm-client-resolution.md index 3187b22de..891528c85 100644 --- a/docs/internal/llm-client-resolution.md +++ b/docs/internal/llm-client-resolution.md @@ -4,12 +4,12 @@ This document defines how Fabro resolves LLM credentials and constructs `fabro-l ## Core Rules -- `fabro_auth::CredentialSource` is the credential authority. -- Long-lived runtime contexts store `Arc` and `Arc`, not `Client`. +- The lithos `CredentialProvider` trait is the credential authority; Fabro's vault, SQL secret store, and API-key stores implement it directly. +- Long-lived runtime contexts store `Arc` and `Arc`, not `Client`. - Call `fabro_llm::client::Client::from_source(&source, catalog).await?` at the point of use. - Standalone setup and tests that use default settings build a default `Arc` locally, then pass it explicitly. - `GenerateParams::new(model, client)` always receives an explicit `Arc`. -- When a caller needs diagnostics in runtime request-serving paths, call `source.resolve(catalog)` directly and consume both `credentials` and `auth_issues`. +- When a caller needs diagnostics in runtime request-serving paths, read `FabroClient::ready` and `auth_issues` (from `ClientBuilder::build_ready`), or call `lithos_llm::credentials::readiness` directly. - `VaultCredentialSource` is the normal source for vault-backed runtime contexts; `VaultCredentialSource::environment_only()` serves env-only or no-vault contexts. ## Why diff --git a/docs/public/reference/sdk.mdx b/docs/public/reference/sdk.mdx index 05668b258..1b34463dc 100644 --- a/docs/public/reference/sdk.mdx +++ b/docs/public/reference/sdk.mdx @@ -374,7 +374,7 @@ The `fabro_llm::catalog` module reads Fabro policy from the catalog: `enabled_pr ### Client -`fabro_llm::build_client(catalog, source, options)` returns a `FabroClient`: the lithos `Client`, the providers that are ready, the providers whose credentials could not be used, and the providers lithos could not build an adapter for. Credentials are read from the `CredentialSource` on every provider attempt, so a refreshed OAuth token is picked up without rebuilding the client. +`fabro_llm::build_client(catalog, credentials, options)` takes any lithos `CredentialProvider` and returns a `FabroClient`: the lithos `Client`, the providers that are ready, the providers whose credentials could not be used, and the providers lithos could not build an adapter for. Credentials are read from the provider on every attempt, so a refreshed OAuth token is picked up without rebuilding the client. `ClientOptions::standard()` turns on the lithos retry middleware (three attempts with short exponential backoff) and local attachment inlining. Add middleware with `with_middleware`, replace a provider's adapter with `with_adapter`, or set `http` to inject a configured HTTP client. `fabro_llm::build_offline_client(catalog, options)` builds a client whose only providers are custom adapters, which is how `fabro exec --server` routes every call through a Fabro server. diff --git a/lib/apps/fabro-cli/src/command_context.rs b/lib/apps/fabro-cli/src/command_context.rs index fea71ff73..0d78fa996 100644 --- a/lib/apps/fabro-cli/src/command_context.rs +++ b/lib/apps/fabro-cli/src/command_context.rs @@ -2,8 +2,9 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, OnceLock}; use anyhow::{Context as _, Result, bail}; -use fabro_auth::{CredentialSource, SqlVaultCredentialSource}; +use fabro_auth::SqlVaultCredentialSource; use fabro_config::{CliLayer, Storage, load_llm_overlay}; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_types::UserSettings; use fabro_types::settings::RunNamespace; @@ -44,7 +45,7 @@ pub(crate) struct CommandContext { run_settings_key_presence: RunSettingsKeyPresence, server_mode: ServerMode, server: OnceCell>, - llm_source: OnceCell>, + llm_source: OnceCell>, catalog: OnceLock>, } @@ -163,7 +164,7 @@ impl CommandContext { Ok(Arc::clone(client)) } - pub(crate) async fn llm_source(&self) -> Result> { + pub(crate) async fn llm_source(&self) -> Result> { let storage_dir = self.storage_dir.clone(); let source = self @@ -173,9 +174,9 @@ impl CommandContext { let store = SecretStore::open(storage.sqlite_path(), storage.secrets_path()) .await .context("opening the Fabro secret store")?; - let source: Arc = + let source: Arc = Arc::new(SqlVaultCredentialSource::new(Arc::new(store))); - Ok::, anyhow::Error>(source) + Ok::, anyhow::Error>(source) }) .await?; diff --git a/lib/apps/fabro-server/src/diagnostics.rs b/lib/apps/fabro-server/src/diagnostics.rs index c5d325175..0397c6c11 100644 --- a/lib/apps/fabro-server/src/diagnostics.rs +++ b/lib/apps/fabro-server/src/diagnostics.rs @@ -4,7 +4,6 @@ use std::time::Duration; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; -use fabro_auth::auth_issue_message; use fabro_http::Response; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::probe::{self, ModelTestStatus}; @@ -219,7 +218,7 @@ pub(crate) async fn test_llm_providers(state: &AppState) -> anyhow::Result, ) -> ProviderProbeResult { if let Some(message) = auth_issue { - // `auth_issue_message` already embeds the provider's display name, so the + // The credential error already names the provider, so the // diagnostics detail uses the message as-is rather than re-prefixing. return provider_probe_error(provider, None, message.clone(), Some(message)); } diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 6b94bfc89..dcef70c8f 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -6,7 +6,6 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_api::types; -use fabro_auth::auth_issue_message; use fabro_config::parse::SettingsSource; use fabro_config::{ CliLayer, CliOutputLayer, EnvironmentLayer, MergeMap, RunLayer, SettingsLayer, @@ -1112,7 +1111,7 @@ async fn run_llm_check( status: CheckStatus::Warning, summary: model_id.clone(), details: vec![CheckDetail::new(format!("Provider: {provider_name}"))], - remediation: Some(auth_issue_message(&provider_id, issue)), + remediation: Some(issue.to_string()), })); } else if let Some(issue) = registration_issues .iter() diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 70e22b006..6911f9c88 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -48,7 +48,7 @@ pub use fabro_api::types::{ SystemRepairRunsResponse, SystemResourcesResponse, SystemRunCounts, TimelineEntryResponse, UpdateVariableRequest, VariableListResponse, VncPreviewResponse, WriteBlobResponse, }; -use fabro_auth::{CredentialSource, SqlVaultCredentialSource, auth_issue_message}; +use fabro_auth::SqlVaultCredentialSource; use fabro_automation::{self, AutomationStore}; use fabro_config::daemon::ServerDaemon; use fabro_config::{LlmLayer, RunLayer, Storage, WorkflowSettingsBuilder}; @@ -57,6 +57,7 @@ use fabro_environment::EnvironmentStore; use fabro_interview::{ Answer, AnswerSubmission, ControlInterviewer, Interviewer, Question, WorkerControlEnvelope, }; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::{ClientOptions, FabroClient, catalog}; use fabro_mcp_store::McpServerStore; @@ -1126,7 +1127,7 @@ pub struct AppState { parent_link_lock: AsyncMutex<()>, pub(super) server_secrets: ServerSecrets, - pub(crate) llm_source: Arc, + pub(crate) llm_source: Arc, manifest_run_defaults: RwLock>, manifest_run_settings: RwLock>, pub(crate) server_settings: RwLock>, @@ -1401,7 +1402,7 @@ impl AppState { pub(crate) async fn configured_llm_provider_ids(&self) -> Vec { let catalog = self.catalog(); - self.llm_source.configured_providers(catalog.as_ref()).await + fabro_llm::configured_providers(catalog.as_ref(), self.llm_source.as_ref()).await } /// Resolve the LLM client once and derive the ready provider IDs from it, @@ -1679,7 +1680,7 @@ impl AppState { /// Builds the server's LLM client: retries and attachment inlining on, the /// server's HTTP client for provider requests when one is configured. async fn resolve_llm_client_from_source( - source: Arc, + source: Arc, catalog: Arc, http_client: Option, ) -> anyhow::Result { @@ -2455,7 +2456,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result = Arc::new(SqlVaultCredentialSource::vault_only( + let llm_source: Arc = Arc::new(SqlVaultCredentialSource::vault_only( Arc::clone(&secret_store), )); let (global_event_tx, _) = broadcast::channel(4096); diff --git a/lib/apps/fabro-server/src/server/handler/models.rs b/lib/apps/fabro-server/src/server/handler/models.rs index cb47cea3f..544721f6d 100644 --- a/lib/apps/fabro-server/src/server/handler/models.rs +++ b/lib/apps/fabro-server/src/server/handler/models.rs @@ -11,8 +11,7 @@ use fabro_types::ReasoningEffort; use super::super::{ ApiError, AppState, FromStr, IntoResponse, Json, MAX_PAGE_OFFSET, ModelTestMode, Path, ProviderCredentialTestRequest, ProviderCredentialTestResponse, ProviderId, ProviderList, Query, - RequiredUser, Response, Router, State, StatusCode, auth_issue_message, default_page_limit, - error, get, post, + RequiredUser, Response, Router, State, StatusCode, default_page_limit, error, get, post, }; use crate::diagnostics; @@ -251,7 +250,7 @@ async fn test_model( .iter() .find(|(provider, _)| provider == &provider_id) { - return ApiError::bad_request(auth_issue_message(&provider_id, issue)).into_response(); + return ApiError::bad_request(issue.to_string()).into_response(); } if !llm_result.has_provider(&provider_id) { return Json(serde_json::json!({ diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 68b2503a7..e753cfe28 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -1813,22 +1813,18 @@ async fn resolve_llm_client_ignores_env_lookup_provider_tokens() { struct FailingCredentialSource; #[async_trait::async_trait] -impl CredentialSource for FailingCredentialSource { +impl CredentialProvider for FailingCredentialSource { async fn credentials( &self, provider: &fabro_llm::lithos_catalog::CatalogProvider, - ) -> Result { - Err(fabro_auth::ResolveError::NotConfigured( - provider.id().clone(), - )) + ) -> Result { + Err(fabro_llm::credentials::CredentialError::NotConfigured { + provider: provider.id().clone(), + }) } - async fn configured_providers( - &self, - catalog: &fabro_llm::lithos_catalog::Catalog, - ) -> Vec { - let _ = catalog; - Vec::new() + async fn is_configured(&self, _provider: &fabro_llm::lithos_catalog::CatalogProvider) -> bool { + false } } @@ -1864,14 +1860,9 @@ async fn llm_source_configured_providers_reads_openai_token_from_vault() { .await .unwrap(); - let catalog = state.catalog(); - assert_eq!( - state - .llm_source - .configured_providers(catalog.as_ref()) - .await, - vec![fabro_types::provider_ids::openai()] - ); + assert_eq!(state.configured_llm_provider_ids().await, vec![ + fabro_types::provider_ids::openai() + ]); } #[tokio::test] diff --git a/lib/apps/fabro-server/tests/it/scenario/run_completion.rs b/lib/apps/fabro-server/tests/it/scenario/run_completion.rs index 3f2073886..f32702a30 100644 --- a/lib/apps/fabro-server/tests/it/scenario/run_completion.rs +++ b/lib/apps/fabro-server/tests/it/scenario/run_completion.rs @@ -38,7 +38,7 @@ fn test_app_with_openai_agent_backend(openai_base_url: String, api_key: String) )); let source_api_key = api_key.clone(); let env_api_key = api_key.clone(); - let llm_source: Arc = + let llm_source: Arc = test_support::env_credential_source(move |name| match name { "OPENAI_API_KEY" => Some(source_api_key.clone()), _ => None, diff --git a/lib/components/fabro-agent/src/cli.rs b/lib/components/fabro-agent/src/cli.rs index 84f257831..a677ceacf 100644 --- a/lib/components/fabro-agent/src/cli.rs +++ b/lib/components/fabro-agent/src/cli.rs @@ -9,9 +9,10 @@ use std::sync::{Arc, Mutex}; use anyhow::Context as _; use clap::{Args, Parser}; -use fabro_auth::{CredentialSource, SqlVaultCredentialSource}; +use fabro_auth::SqlVaultCredentialSource; use fabro_config::Storage; use fabro_config::user::default_storage_dir; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::middleware::{Call, Middleware, Next, Output}; use fabro_llm::{Client, ClientOptions, Error as LlmError, catalog}; @@ -244,7 +245,7 @@ fn resolve_provider_id( catalog::canonical_provider_id(catalog, requested.as_str()).unwrap_or(requested) } -async fn standalone_llm_source() -> anyhow::Result> { +async fn standalone_llm_source() -> anyhow::Result> { let storage = Storage::new(default_storage_dir()); let store = SecretStore::open(storage.sqlite_path(), storage.secrets_path()) .await @@ -455,7 +456,7 @@ pub async fn run_with_args( )] pub async fn run_with_args_and_source_and_catalog( args: AgentArgs, - llm_source: Arc, + llm_source: Arc, mcp_servers: Vec, catalog: Arc, ) -> anyhow::Result<()> { diff --git a/lib/components/fabro-hooks/src/bridge.rs b/lib/components/fabro-hooks/src/bridge.rs index 6f49a6a00..b064e6171 100644 --- a/lib/components/fabro-hooks/src/bridge.rs +++ b/lib/components/fabro-hooks/src/bridge.rs @@ -78,6 +78,7 @@ mod tests { use std::path::PathBuf; use std::sync::Mutex; + use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_types::fixtures; @@ -100,7 +101,7 @@ mod tests { context: &HookContext, _sandbox: Arc, execution_context: &HookExecutionContext, - _llm_source: Arc, + _llm_source: Arc, _catalog: Arc, ) -> HookResult { self.captured_contexts.lock().unwrap().push(context.clone()); diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs index 3d208ed7e..418a77270 100644 --- a/lib/components/fabro-hooks/src/executor.rs +++ b/lib/components/fabro-hooks/src/executor.rs @@ -6,7 +6,7 @@ use std::time::Instant; use async_trait::async_trait; use fabro_agent::Sandbox; use fabro_agent::tool_registry::ToolContext; -use fabro_auth::CredentialSource; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::{Client, ClientOptions, Request, structured}; use fabro_redact::redacted_url_for_log; @@ -48,7 +48,7 @@ pub trait HookExecutor: Send + Sync { context: &HookContext, sandbox: Arc, execution_context: &HookExecutionContext, - llm_source: Arc, + llm_source: Arc, catalog: Arc, ) -> HookResult; } @@ -281,7 +281,7 @@ impl HookExecutorImpl { prompt: &InterpString, model: Option<&InterpString>, context: &HookContext, - llm_source: Arc, + llm_source: Arc, catalog: Arc, ) -> HookDecision { let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) { @@ -361,7 +361,7 @@ impl HookExecutorImpl { max_tool_rounds: Option, context: &HookContext, sandbox: Arc, - llm_source: Arc, + llm_source: Arc, catalog: Arc, ) -> HookDecision { let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) { @@ -476,7 +476,7 @@ impl HookExecutorImpl { /// serve, with standard retries. async fn build_client( catalog: Arc, - llm_source: Arc, + llm_source: Arc, ) -> Result { fabro_llm::build_client( Catalog::clone(&catalog), @@ -662,7 +662,7 @@ impl HookExecutor for HookExecutorImpl { context: &HookContext, sandbox: Arc, execution_context: &HookExecutionContext, - llm_source: Arc, + llm_source: Arc, catalog: Arc, ) -> HookResult { use std::sync::OnceLock; @@ -761,7 +761,8 @@ impl HookExecutor for HookExecutorImpl { #[cfg(test)] mod tests { - use fabro_auth::{CredentialSource, test_support}; + use fabro_auth::test_support; + use fabro_llm::credentials::CredentialProvider; use fabro_types::fixtures; use fabro_types::settings::ResolveErrorKind; @@ -779,7 +780,7 @@ mod tests { )) } - fn test_llm_source() -> Arc { + fn test_llm_source() -> Arc { test_support::vault_only_credential_source() } diff --git a/lib/components/fabro-hooks/src/runner.rs b/lib/components/fabro-hooks/src/runner.rs index 489edd0dd..7a9f8fd57 100644 --- a/lib/components/fabro-hooks/src/runner.rs +++ b/lib/components/fabro-hooks/src/runner.rs @@ -2,9 +2,9 @@ use std::collections::HashMap; use std::sync::Arc; use fabro_agent::Sandbox; -use fabro_auth::CredentialSource; #[cfg(test)] use fabro_auth::test_support; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use crate::config::{HookDefinition, HookSettings}; @@ -16,7 +16,7 @@ use crate::types::{HookContext, HookDecision, HookExecutionContext}; pub struct HookRunner { config: HookSettings, executor: Arc, - llm_source: Arc, + llm_source: Arc, catalog: Arc, /// Pre-compiled regexes keyed by matcher pattern string. compiled_matchers: HashMap, @@ -26,7 +26,7 @@ impl HookRunner { #[must_use] pub fn new( config: HookSettings, - llm_source: Arc, + llm_source: Arc, catalog: Arc, ) -> Self { let compiled_matchers = Self::compile_matchers(&config); @@ -256,7 +256,7 @@ mod tests { _context: &HookContext, _sandbox: Arc, _execution_context: &HookExecutionContext, - _llm_source: Arc, + _llm_source: Arc, _catalog: Arc, ) -> HookResult { HookResult { @@ -277,7 +277,7 @@ mod tests { HookContext::new(event, fixtures::RUN_1, "test-wf".into()) } - fn test_llm_source() -> Arc { + fn test_llm_source() -> Arc { test_support::vault_only_credential_source() } diff --git a/lib/components/fabro-hooks/tests/host_command_hooks.rs b/lib/components/fabro-hooks/tests/host_command_hooks.rs index 74cacd33c..8cac52b80 100644 --- a/lib/components/fabro-hooks/tests/host_command_hooks.rs +++ b/lib/components/fabro-hooks/tests/host_command_hooks.rs @@ -2,16 +2,17 @@ use std::path::Path; use std::sync::Arc; use fabro_agent::{LocalSandbox, Sandbox}; -use fabro_auth::{CredentialSource, test_support}; +use fabro_auth::test_support; use fabro_hooks::{ HookContext, HookDecision, HookDefinition, HookEvent, HookExecutionContext, HookRunner, HookSettings, InterpString, }; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_types::RunId; use tokio::fs; -fn test_llm_source() -> Arc { +fn test_llm_source() -> Arc { test_support::vault_only_credential_source() } diff --git a/lib/components/fabro-llm/src/client.rs b/lib/components/fabro-llm/src/client.rs index 412456ffb..708819e7f 100644 --- a/lib/components/fabro-llm/src/client.rs +++ b/lib/components/fabro-llm/src/client.rs @@ -3,11 +3,11 @@ use std::sync::Arc; use std::time::Duration; -use fabro_auth::{CredentialSource, ResolveError, lithos_credentials}; use fabro_types::ProviderId; use lithos_llm::adapter::ProviderAdapter; use lithos_llm::catalog::Catalog; use lithos_llm::client::{Client, ClientBuildError, ClientBuilder, ProviderBuildIssue}; +use lithos_llm::credentials::{CredentialError, CredentialProvider}; use lithos_llm::middleware::{ Call, Middleware, Observer, RetryMiddleware, RetryPolicy, RetryStage, }; @@ -167,8 +167,9 @@ pub struct FabroClient { pub client: Client, /// Enabled providers with working credentials, in catalog order. pub ready: Vec, - /// Enabled providers whose credential material could not be used. - pub auth_issues: Vec<(ProviderId, ResolveError)>, + /// Enabled providers whose credential material could not be used. The + /// error's `Display` is the operator-facing line. + pub auth_issues: Vec<(ProviderId, CredentialError)>, /// Ready providers lithos could not build an adapter for. pub build_issues: Vec, } @@ -193,34 +194,42 @@ pub enum LlmSetupError { Build(#[from] ClientBuildError), } -/// Builds a client whose ready providers are those the credential source can -/// serve. Credentials are re-read from `source` on every provider attempt. +/// Builds a client whose ready providers are those `credentials` can serve. +/// Credentials are re-read on every provider attempt, so a refreshed OAuth +/// token is picked up by the next retry. pub async fn build_client( catalog: Catalog, - source: Arc, + credentials: Arc, options: ClientOptions, ) -> Result { - let resolved = source.resolve_all(&catalog).await; - let mut ready = resolved.ready; - for provider in options.adapter_providers() { - if !ready.contains(provider) { - ready.push(provider.clone()); - } - } let builder = Client::builder() .catalog(catalog) .application(APPLICATION_NAME) - .credentials_arc(lithos_credentials(source)) - .enabled_providers(ready.iter().cloned()); - let build = options.apply(builder).build()?; + .credentials_arc(credentials); + let build = options.apply(builder).build_ready().await?; Ok(FabroClient { - client: build.client, - ready, - auth_issues: resolved.auth_issues, + client: build.client, + ready: build.ready, + auth_issues: build.credential_issues, build_issues: build.issues, }) } +/// The enabled providers `credentials` holds material for, in catalog order, +/// without refreshing anything. Cheap enough for listings. +pub async fn configured_providers( + catalog: &Catalog, + credentials: &dyn CredentialProvider, +) -> Vec { + let mut configured = Vec::new(); + for provider in catalog.providers().filter(|provider| provider.is_enabled()) { + if credentials.is_configured(provider).await { + configured.push(provider.id().clone()); + } + } + configured +} + /// Builds a client that needs no credentials: every available provider is /// served by a custom adapter from `options.adapters`, such as the /// `fabro exec` gateway or a test double. diff --git a/lib/components/fabro-llm/src/lib.rs b/lib/components/fabro-llm/src/lib.rs index 31cd9d74b..00e9b8c7d 100644 --- a/lib/components/fabro-llm/src/lib.rs +++ b/lib/components/fabro-llm/src/lib.rs @@ -37,7 +37,7 @@ pub mod test_support; pub use catalog::{build_catalog, default_catalog}; pub use client::{ ClientOptions, FabroClient, LlmSetupError, RetryListener, RetryNotice, build_client, - build_offline_client, + build_offline_client, configured_providers, }; pub use error::{ErrorFacts, LlmError}; pub use lithos_llm::client::{Client, ClientBuild}; diff --git a/lib/components/fabro-llm/src/probe.rs b/lib/components/fabro-llm/src/probe.rs index 1e2cbcb25..58c5a1e51 100644 --- a/lib/components/fabro-llm/src/probe.rs +++ b/lib/components/fabro-llm/src/probe.rs @@ -120,10 +120,7 @@ pub async fn probe_provider_with_api_key( .iter() .find(|(candidate, _)| candidate == &provider_id) { - return Ok(ModelTestOutcome::error(fabro_auth::auth_issue_message( - &provider_id, - issue, - ))); + return Ok(ModelTestOutcome::error(issue.to_string())); } Ok(run_basic_probe(&built.client, &selector, timeout).await) } diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index ab140a10c..87cffd0fa 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -10,8 +10,8 @@ use fabro_agent::{ Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider, ToolSecrets, WebFetchSummarizer, canonical_tool_name, register_question_tools, }; -use fabro_auth::CredentialSource; use fabro_graphviz::graph::{AttrValue, Node}; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::error::failover_eligible; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::types::ResponseFormat; @@ -638,7 +638,7 @@ pub struct AgentApiBackend { mcp_servers: Vec, tool_secrets: ToolSecrets, run_model_controls: RunModelControls, - source: Arc, + source: Arc, steering_hub: Arc, catalog: Arc, fabro_run_tools: Option, @@ -784,7 +784,7 @@ impl AgentApiBackend { model: String, provider_id: impl Into, fallbacks: ModelFallbackPolicy, - source: Arc, + source: Arc, steering_hub: Arc, ) -> Self { let catalog = Arc::new(fabro_llm::default_catalog()); @@ -803,7 +803,7 @@ impl AgentApiBackend { model: String, provider_id: ProviderId, fallbacks: ModelFallbackPolicy, - source: Arc, + source: Arc, steering_hub: Arc, catalog: Arc, ) -> Self { @@ -1051,7 +1051,7 @@ impl AgentApiBackend { controls: EffectiveRequestControls, node: &Node, sandbox: &Arc, - source: Arc, + source: Arc, catalog: Arc, tool_env: Option<&Arc>, tool_hooks: Option>, @@ -1453,7 +1453,7 @@ impl AgentApiBackend { /// Build the LLM client a stage session dispatches through. async fn build_llm_client( catalog: &Arc, - source: Arc, + source: Arc, ) -> Result { fabro_llm::build_client(Catalog::clone(catalog), source, ClientOptions::standard()) .await diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index f5a5c8ad0..83e56fbcf 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -4,8 +4,9 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use fabro_auth::{CredentialSource, VaultCredentialSource}; +use fabro_auth::VaultCredentialSource; use fabro_interview::{AutoApproveInterviewer, Interviewer}; +use fabro_llm::credentials::readiness; use fabro_llm::lithos_catalog::Catalog; use fabro_mcp::config::McpServerSettings; use fabro_sandbox::daytona::DaytonaConfig; @@ -746,11 +747,8 @@ async fn configured_providers_for_start( vault: &Arc>, catalog: Arc, ) -> Vec { - let source: Arc = Arc::new(VaultCredentialSource::with_env_lookup( - Arc::clone(vault), - process_env_var, - )); - source.resolve_all(catalog.as_ref()).await.ready + let source = VaultCredentialSource::with_env_lookup(Arc::clone(vault), process_env_var); + readiness(catalog.enabled_providers(), &source).await.ready } fn git_checkpoint_options_from_start( diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 38fc31e51..d4effcf2e 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -4,10 +4,11 @@ use std::sync::Arc; use std::time::Instant; use fabro_agent::{Sandbox, ToolSecrets}; -use fabro_auth::{CredentialSource, ExtraHeadersCredentialSource, VaultCredentialSource}; +use fabro_auth::{ExtraHeadersCredentialSource, VaultCredentialSource}; use fabro_github::token_source::InstallationTokenSource; use fabro_graphviz::graph; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookExecutionContext, HookRunner}; +use fabro_llm::credentials::{CredentialProvider, readiness}; use fabro_llm::lithos_catalog::Catalog; use fabro_sandbox::{ GitSetupIntent, SandboxEventCallback, SandboxSpec, reconnect_for_run_with_callback, shell_quote, @@ -210,7 +211,7 @@ async fn build_registry( tool_env_provider: Arc, github_token_refresh_managed: bool, graph: &graph::Graph, - llm_source: Arc, + llm_source: Arc, catalog: Arc, tool_secrets: ToolSecrets, fabro_run_tools: Option, @@ -276,11 +277,17 @@ async fn build_registry( return Ok((build_llm_registry(), false)); } - let result = llm_source.resolve_all(catalog.as_ref()).await; + let result = readiness(catalog.enabled_providers(), llm_source.as_ref()).await; if result.ready.is_empty() { if graph_needs_llm { - let detail = - (!result.auth_issues.is_empty()).then(|| result.issue_messages().join("; ")); + let detail = (!result.issues.is_empty()).then(|| { + result + .issues + .iter() + .map(|(_, issue)| issue.to_string()) + .collect::>() + .join("; ") + }); let prefix = detail.map_or_else( || "No LLM providers configured".to_string(), |detail| format!("No usable LLM providers configured: {detail}"), @@ -314,7 +321,7 @@ const SESSION_ID_HEADER: &str = "x-session-id"; fn build_llm_source( vault: Arc>, run_id: fabro_types::RunId, -) -> Arc { +) -> Arc { Arc::new(ExtraHeadersCredentialSource::new( Arc::new(VaultCredentialSource::new(vault)), HashMap::from([(SESSION_ID_HEADER.to_string(), run_id.to_string())]), @@ -1087,14 +1094,13 @@ mod tests { fabro_types::provider_ids::anthropic() ); assert!( - initialized - .engine - .run - .llm_source - .resolve_all(&initialized.engine.run.catalog) - .await - .ready - .is_empty() + readiness( + initialized.engine.run.catalog.enabled_providers(), + initialized.engine.run.llm_source.as_ref(), + ) + .await + .ready + .is_empty() ); } @@ -1244,7 +1250,7 @@ mod tests { let source = build_llm_source(vault, run_id); let catalog = test_catalog(); - let resolved = source.resolve_all(catalog.as_ref()).await; + let resolved = readiness(catalog.enabled_providers(), source.as_ref()).await; assert!(!resolved.ready.is_empty()); for provider in &resolved.ready { diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index f6fb74de5..d7990d5fe 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -2,9 +2,9 @@ use std::collections::HashSet; use std::sync::{Arc, LazyLock}; use std::time::Duration; -use fabro_auth::CredentialSource; use fabro_github::{self as github_app, ssh_url_to_https}; use fabro_graphviz::parser; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::{Client, ClientOptions, Request, selection, structured}; use fabro_store::RunProjection; @@ -333,7 +333,7 @@ pub async fn build_pr_content( goal: &str, model: &str, run_store: &RunStoreHandle, - llm_source: Arc, + llm_source: Arc, catalog: Arc, conclusion: Option<&Conclusion>, run_state: Option<&RunProjection>, @@ -462,7 +462,7 @@ pub struct OpenPullRequestRequest<'a> { pub draft: bool, pub auto_merge: Option, pub run_store: &'a RunStoreHandle, - pub llm_source: Arc, + pub llm_source: Arc, pub catalog: Arc, pub conclusion: Option<&'a Conclusion>, pub run_state: Option<&'a RunProjection>, @@ -686,9 +686,10 @@ mod tests { use std::time::Duration; use chrono::Utc; - use fabro_auth::{CredentialSource, VaultCredentialSource}; + use fabro_auth::VaultCredentialSource; use fabro_graphviz::graph::Graph; use fabro_llm::adapter::{ProviderAdapter, ResolvedCall}; + use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::AdapterId; use fabro_llm::{Response, ResponseStream}; use fabro_store::Database; @@ -1308,9 +1309,9 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr None, ) .unwrap(); - let llm_source: Arc = Arc::new(VaultCredentialSource::new(Arc::new( - AsyncRwLock::new(vault), - ))); + let llm_source: Arc = Arc::new(VaultCredentialSource::new( + Arc::new(AsyncRwLock::new(vault)), + )); // Use catalog settings to override base_url instead of env var let catalog = test_catalog_with_provider_base_url("openai", &server.url("/v1")); @@ -1696,7 +1697,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr branch_mock_id: usize, reconcile_mock_id: usize, github_mock_id: usize, - llm_source: Arc, + llm_source: Arc, catalog: Arc, creds: fabro_github::GitHubCredentials, run_store: RunStoreHandle, @@ -1804,9 +1805,9 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr None, ) .unwrap(); - let llm_source: Arc = Arc::new(VaultCredentialSource::new(Arc::new( - AsyncRwLock::new(vault), - ))); + let llm_source: Arc = Arc::new(VaultCredentialSource::new( + Arc::new(AsyncRwLock::new(vault)), + )); // Use catalog settings to override base_url instead of env var let catalog = test_catalog_with_provider_base_url("openai", &openai_server.url("/v1")); diff --git a/lib/components/fabro-workflow/src/services.rs b/lib/components/fabro-workflow/src/services.rs index b1662de01..2456b56fb 100644 --- a/lib/components/fabro-workflow/src/services.rs +++ b/lib/components/fabro-workflow/src/services.rs @@ -5,10 +5,10 @@ use std::sync::Arc; use std::time::Duration; use fabro_agent::{Sandbox, ToolEnvProvider}; -use fabro_auth::CredentialSource; use fabro_github::token_source::InstallationTokenSource; use fabro_hooks::{HookContext, HookDecision, HookExecutionContext, HookRunner}; use fabro_interview::Interviewer; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_types::{ManifestPath, ProviderId, RunId}; use tokio_util::sync::CancellationToken; @@ -99,7 +99,7 @@ pub struct RunServices { pub(crate) cancel_token: CancellationToken, pub provider_id: ProviderId, pub model: String, - pub llm_source: Arc, + pub llm_source: Arc, pub catalog: Arc, pub(crate) sandbox_git: Arc, pub(crate) metadata_runtime: Arc, @@ -121,7 +121,7 @@ impl RunServices { cancel_token: CancellationToken, provider_id: ProviderId, model: String, - llm_source: Arc, + llm_source: Arc, catalog: Arc, sandbox_git: Arc, metadata_runtime: Arc, @@ -268,19 +268,22 @@ impl EngineServices { struct StubCredentialSource; #[async_trait::async_trait] - impl CredentialSource for StubCredentialSource { + impl CredentialProvider for StubCredentialSource { async fn credentials( &self, provider: &fabro_llm::lithos_catalog::CatalogProvider, - ) -> Result { - Err(fabro_auth::ResolveError::NotConfigured( - provider.id().clone(), - )) + ) -> Result + { + Err(fabro_llm::credentials::CredentialError::NotConfigured { + provider: provider.id().clone(), + }) } - async fn configured_providers(&self, catalog: &Catalog) -> Vec { - let _ = catalog; - Vec::new() + async fn is_configured( + &self, + _provider: &fabro_llm::lithos_catalog::CatalogProvider, + ) -> bool { + false } } @@ -383,12 +386,12 @@ mod tests { let services = EngineServices::test_default(); assert!( - services - .run - .llm_source - .configured_providers(&services.run.catalog) - .await - .is_empty() + fabro_llm::configured_providers( + &services.run.catalog, + services.run.llm_source.as_ref() + ) + .await + .is_empty() ); } diff --git a/lib/components/fabro-workflow/src/test_support.rs b/lib/components/fabro-workflow/src/test_support.rs index 9efbeae84..57a39592f 100644 --- a/lib/components/fabro-workflow/src/test_support.rs +++ b/lib/components/fabro-workflow/src/test_support.rs @@ -5,10 +5,11 @@ use std::sync::Arc; use std::time::Duration; use fabro_agent::Sandbox; -use fabro_auth::{CredentialSource, test_support as auth_test_support}; +use fabro_auth::test_support as auth_test_support; use fabro_graphviz::graph::Graph as GvGraph; use fabro_interview::AutoApproveInterviewer; use fabro_llm::catalog; +use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_llm::test_support::test_catalog; use fabro_store::{ArtifactStore, RunProjection, test_support as store_test_support}; @@ -145,7 +146,7 @@ struct InitializedOptions { hook_runner: Option>, env: HashMap, checkpoint: Option, - llm_source: Option>, + llm_source: Option>, } struct InitializedState { @@ -481,7 +482,7 @@ pub async fn run_graph_with_state_and_llm_source( sandbox: Arc, graph: &GvGraph, run_options: &RunOptions, - llm_source: Arc, + llm_source: Arc, ) -> Result<(Outcome, RunProjection)> { let initialized = initialized( registry, @@ -576,7 +577,7 @@ impl WorkflowRunner { &self, graph: &GvGraph, run_options: &RunOptions, - llm_source: Arc, + llm_source: Arc, ) -> Result<(Outcome, RunProjection)> { let registry = self .registry diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index ee407786e..1be24c4cb 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -7395,7 +7395,7 @@ mod real_llm { } fabro_test::require_env("ANTHROPIC_API_KEY")?; - let source: Arc = + let source: Arc = Arc::new(VaultCredentialSource::environment_only()); Some(Arc::new( fabro_llm::build_client( @@ -8081,7 +8081,8 @@ fn openai_responses_payload(text: &str) -> serde_json::Value { #[tokio::test] async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() { use chrono::Utc; - use fabro_auth::{CredentialSource, VaultCredentialSource}; + use fabro_auth::VaultCredentialSource; + use fabro_llm::credentials::CredentialProvider; use fabro_types::Conclusion; use fabro_vault::{SecretType, Vault}; use httpmock::Method::POST; @@ -8138,7 +8139,7 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() { None, ) .unwrap(); - let llm_source: Arc = Arc::new(VaultCredentialSource::new(Arc::new( + let llm_source: Arc = Arc::new(VaultCredentialSource::new(Arc::new( AsyncRwLock::new(vault), ))); // Use catalog settings to override base_url instead of env var diff --git a/lib/foundation/fabro-auth/src/api_key_source.rs b/lib/foundation/fabro-auth/src/api_key_source.rs index 17b706498..621b696f6 100644 --- a/lib/foundation/fabro-auth/src/api_key_source.rs +++ b/lib/foundation/fabro-auth/src/api_key_source.rs @@ -9,14 +9,14 @@ use std::sync::Arc; use async_trait::async_trait; use fabro_vault::Vault; -use lithos_llm::catalog::{Catalog, CatalogProvider, ProviderId}; -use lithos_llm::credentials::{ConventionalCredentials, CredentialProvider, Credentials}; +use lithos_llm::catalog::{CatalogProvider, ProviderId}; +use lithos_llm::credentials::{ + ConventionalCredentials, CredentialError, CredentialProvider, Credentials, +}; use tokio::sync::RwLock as AsyncRwLock; -use crate::credential_source::CredentialSource; -use crate::error::ResolveError; use crate::secrets::expected_secret_name; -use crate::vault_source::{auth_scheme_name, interpolated_headers, resolve_error}; +use crate::vault_source::interpolated_headers; pub struct ApiKeyCredentialSource { provider: ProviderId, @@ -61,19 +61,17 @@ pub(crate) async fn credentials_for_api_key( provider: &CatalogProvider, key: String, vault: &Vault, -) -> Result { +) -> Result { let Some(name) = expected_secret_name(provider) else { - return Err(ResolveError::SchemeMismatch { + return Err(CredentialError::SchemeMismatch { provider: provider.id().clone(), - scheme: auth_scheme_name(provider.auth()).to_string(), }); }; let interpolated = interpolated_headers(vault, provider)?; let mut credentials = ConventionalCredentials::new() .with_lookup(move |candidate| (candidate == name).then(|| key.clone())) .credentials(provider) - .await - .map_err(|err| resolve_error(provider, &err))?; + .await?; if let Credentials::Http(http) = &mut credentials { http.extra_headers.extend(interpolated); } @@ -81,21 +79,22 @@ pub(crate) async fn credentials_for_api_key( } #[async_trait] -impl CredentialSource for ApiKeyCredentialSource { - async fn credentials(&self, provider: &CatalogProvider) -> Result { +impl CredentialProvider for ApiKeyCredentialSource { + async fn credentials( + &self, + provider: &CatalogProvider, + ) -> Result { if provider.id() != &self.provider { - return Err(ResolveError::NotConfigured(provider.id().clone())); + return Err(CredentialError::NotConfigured { + provider: provider.id().clone(), + }); } let vault = self.vault.read().await.clone(); credentials_for_api_key(provider, self.key.clone(), &vault).await } - async fn configured_providers(&self, catalog: &Catalog) -> Vec { - catalog - .provider(self.provider.as_str()) - .ok() - .map(|provider| vec![provider.id().clone()]) - .unwrap_or_default() + async fn is_configured(&self, provider: &CatalogProvider) -> bool { + provider.id() == &self.provider } } diff --git a/lib/foundation/fabro-auth/src/credential_source.rs b/lib/foundation/fabro-auth/src/credential_source.rs deleted file mode 100644 index 69b677385..000000000 --- a/lib/foundation/fabro-auth/src/credential_source.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! Per-attempt credential lookup for LLM providers. -//! -//! [`CredentialSource`] is Fabro's storage-aware credential seam: the vault, -//! the SQL secret store, and the process environment each implement it. -//! [`lithos_credentials`] adapts a source into the lithos -//! [`CredentialProvider`] the client calls before every provider attempt, so a -//! refreshed OAuth token is picked up by the next retry. - -use std::sync::Arc; - -use async_trait::async_trait; -use lithos_llm::catalog::{Catalog, CatalogProvider, ProviderId}; -use lithos_llm::credentials::{CredentialError, CredentialProvider, Credentials}; - -use crate::{ResolveError, auth_issue_message}; - -/// Which providers a source can serve right now, and why the rest cannot. -#[derive(Debug, Default)] -pub struct ResolvedCredentials { - /// Enabled providers whose credentials resolved. - pub ready: Vec, - /// Enabled providers with credential material that failed to resolve, - /// such as an expired OAuth token that could not be refreshed. Providers - /// with no material at all are not issues; they are simply absent. - pub auth_issues: Vec<(ProviderId, ResolveError)>, -} - -impl ResolvedCredentials { - /// A human-readable line per auth issue. - #[must_use] - pub fn issue_messages(&self) -> Vec { - self.auth_issues - .iter() - .map(|(provider, issue)| auth_issue_message(provider, issue)) - .collect() - } -} - -#[async_trait] -pub trait CredentialSource: Send + Sync { - /// Resolves `provider`'s credentials for one request attempt. - async fn credentials(&self, provider: &CatalogProvider) -> Result; - - /// Providers with credential material present. Does not refresh or - /// validate anything, so it is cheap enough for listings. - async fn configured_providers(&self, catalog: &Catalog) -> Vec; - - /// Resolves every enabled provider once, separating the ready set from - /// the providers that have material but cannot use it. - async fn resolve_all(&self, catalog: &Catalog) -> ResolvedCredentials { - let mut resolved = ResolvedCredentials::default(); - for provider in catalog.providers().filter(|provider| provider.is_enabled()) { - match self.credentials(provider).await { - Ok(_) => resolved.ready.push(provider.id().clone()), - Err(ResolveError::NotConfigured(_)) => {} - Err(err) => resolved.auth_issues.push((provider.id().clone(), err)), - } - } - resolved - } -} - -/// Adapts a [`CredentialSource`] into the lithos credential provider. -#[must_use] -pub fn lithos_credentials(source: Arc) -> Arc { - Arc::new(SourceCredentialProvider { source }) -} - -struct SourceCredentialProvider { - source: Arc, -} - -#[async_trait] -impl CredentialProvider for SourceCredentialProvider { - async fn credentials( - &self, - provider: &CatalogProvider, - ) -> Result { - self.source.credentials(provider).await.map_err(|err| { - tracing::warn!( - provider = %provider.id(), - error = %err, - "LLM credentials could not be resolved for this attempt" - ); - match err { - ResolveError::NotConfigured(provider) => { - CredentialError::NotConfigured { provider } - } - ResolveError::SchemeMismatch { provider, .. } => { - CredentialError::SchemeMismatch { provider } - } - other => CredentialError::NotConfigured { - provider: other.provider().clone(), - }, - } - }) - } -} diff --git a/lib/foundation/fabro-auth/src/error.rs b/lib/foundation/fabro-auth/src/error.rs deleted file mode 100644 index 7986637da..000000000 --- a/lib/foundation/fabro-auth/src/error.rs +++ /dev/null @@ -1,83 +0,0 @@ -//! Why a provider's credentials could not be resolved. - -use fabro_types::settings::ResolveError as InterpResolveError; -use fabro_vault::SecretType; -use lithos_llm::catalog::ProviderId; - -#[derive(Debug, thiserror::Error)] -pub enum ResolveError { - #[error("{0} is not configured")] - NotConfigured(ProviderId), - #[error("{provider} header interpolation failed: {source}")] - Interpolation { - provider: ProviderId, - #[source] - source: InterpResolveError, - }, - #[error("{provider} vault credential '{name}' is not valid Oauth JSON: {source}")] - VaultDecodeFailed { - provider: ProviderId, - name: String, - #[source] - source: serde_json::Error, - }, - #[error("{provider} vault credential '{name}' has schema {actual:?}, expected Token or Oauth")] - VaultSchemaMismatch { - provider: ProviderId, - name: String, - actual: SecretType, - }, - #[error("{provider} requires re-authentication: {source}")] - RefreshFailed { - provider: ProviderId, - #[source] - source: anyhow::Error, - }, - #[error("{0} requires re-authentication: missing refresh token")] - RefreshTokenMissing(ProviderId), - #[error("{provider} resolved a secret its `{scheme}` auth scheme cannot use")] - SchemeMismatch { - provider: ProviderId, - scheme: String, - }, -} - -impl ResolveError { - #[must_use] - pub fn provider(&self) -> &ProviderId { - match self { - Self::NotConfigured(provider) - | Self::RefreshTokenMissing(provider) - | Self::Interpolation { provider, .. } - | Self::VaultDecodeFailed { provider, .. } - | Self::VaultSchemaMismatch { provider, .. } - | Self::RefreshFailed { provider, .. } - | Self::SchemeMismatch { provider, .. } => provider, - } - } -} - -#[must_use] -pub fn auth_issue_message(provider: &ProviderId, err: &ResolveError) -> String { - match err { - ResolveError::NotConfigured(_) => format!("{provider} is not configured"), - ResolveError::Interpolation { source, .. } => { - format!("{provider} header interpolation failed: {source}") - } - ResolveError::VaultDecodeFailed { name, source, .. } => { - format!("{provider} vault credential '{name}' is not valid OAuth JSON: {source}") - } - ResolveError::VaultSchemaMismatch { name, actual, .. } => format!( - "{provider} vault credential '{name}' has schema {actual:?}, expected Token or Oauth" - ), - ResolveError::RefreshFailed { source, .. } => { - format!("{provider} requires re-authentication: {source}") - } - ResolveError::RefreshTokenMissing(_) => { - format!("{provider} requires re-authentication: refresh token missing") - } - ResolveError::SchemeMismatch { scheme, .. } => { - format!("{provider} resolved a secret its `{scheme}` auth scheme cannot use") - } - } -} diff --git a/lib/foundation/fabro-auth/src/extra_headers_source.rs b/lib/foundation/fabro-auth/src/extra_headers_source.rs index 0226e1fee..8b358bdde 100644 --- a/lib/foundation/fabro-auth/src/extra_headers_source.rs +++ b/lib/foundation/fabro-auth/src/extra_headers_source.rs @@ -2,33 +2,35 @@ use std::collections::HashMap; use std::sync::Arc; use async_trait::async_trait; -use lithos_llm::catalog::{Catalog, CatalogProvider, ProviderId}; -use lithos_llm::credentials::{CredentialHeader, Credentials, SecretValue}; +use lithos_llm::catalog::CatalogProvider; +use lithos_llm::credentials::{ + CredentialError, CredentialHeader, CredentialProvider, Credentials, SecretValue, +}; -use crate::ResolveError; -use crate::credential_source::CredentialSource; - -/// Decorates another [`CredentialSource`] by appending fixed extra headers to -/// every HTTP credential it resolves. +/// Decorates another [`CredentialProvider`] by appending fixed extra headers +/// to every HTTP credential it resolves. /// /// Headers already present on a credential (for example from explicit /// provider configuration) are left untouched. AWS-signed credentials carry /// no header list and pass through unchanged. pub struct ExtraHeadersCredentialSource { - inner: Arc, + inner: Arc, headers: HashMap, } impl ExtraHeadersCredentialSource { #[must_use] - pub fn new(inner: Arc, headers: HashMap) -> Self { + pub fn new(inner: Arc, headers: HashMap) -> Self { Self { inner, headers } } } #[async_trait] -impl CredentialSource for ExtraHeadersCredentialSource { - async fn credentials(&self, provider: &CatalogProvider) -> Result { +impl CredentialProvider for ExtraHeadersCredentialSource { + async fn credentials( + &self, + provider: &CatalogProvider, + ) -> Result { let mut credentials = self.inner.credentials(provider).await?; if let Credentials::Http(http) = &mut credentials { for (name, value) in &self.headers { @@ -48,8 +50,8 @@ impl CredentialSource for ExtraHeadersCredentialSource { Ok(credentials) } - async fn configured_providers(&self, catalog: &Catalog) -> Vec { - self.inner.configured_providers(catalog).await + async fn is_configured(&self, provider: &CatalogProvider) -> bool { + self.inner.is_configured(provider).await } } @@ -61,16 +63,16 @@ mod tests { use crate::test_support::test_catalog; struct StubSource { - configured_providers: Vec, - existing_header: Option<(String, String)>, + configured: bool, + existing_header: Option<(String, String)>, } #[async_trait] - impl CredentialSource for StubSource { + impl CredentialProvider for StubSource { async fn credentials( &self, provider: &CatalogProvider, - ) -> Result { + ) -> Result { if provider.id().as_str() == "bedrock" { return Ok(Credentials::AwsDefaultChain { region: None }); } @@ -86,8 +88,8 @@ mod tests { Ok(credentials) } - async fn configured_providers(&self, _catalog: &Catalog) -> Vec { - self.configured_providers.clone() + async fn is_configured(&self, _provider: &CatalogProvider) -> bool { + self.configured } } @@ -107,8 +109,8 @@ mod tests { let catalog = test_catalog(); let source = ExtraHeadersCredentialSource::new( Arc::new(StubSource { - configured_providers: Vec::new(), - existing_header: None, + configured: false, + existing_header: None, }), HashMap::from([("x-session-id".to_string(), "run-123".to_string())]), ); @@ -133,8 +135,8 @@ mod tests { let catalog = test_catalog(); let source = ExtraHeadersCredentialSource::new( Arc::new(StubSource { - configured_providers: vec![ProviderId::new("openai")], - existing_header: Some(("X-Session-Id".to_string(), "configured".to_string())), + configured: true, + existing_header: Some(("X-Session-Id".to_string(), "configured".to_string())), }), HashMap::from([("x-session-id".to_string(), "run-123".to_string())]), ); @@ -143,8 +145,10 @@ mod tests { .await .unwrap(); assert_eq!(header(&credentials, "x-session-id"), Some("configured")); - assert_eq!(source.configured_providers(&catalog).await, vec![ - ProviderId::new("openai") - ]); + assert!( + source + .is_configured(catalog.provider("openai").unwrap()) + .await + ); } } diff --git a/lib/foundation/fabro-auth/src/lib.rs b/lib/foundation/fabro-auth/src/lib.rs index 34cd2590e..878863027 100644 --- a/lib/foundation/fabro-auth/src/lib.rs +++ b/lib/foundation/fabro-auth/src/lib.rs @@ -1,8 +1,16 @@ +//! Fabro's credential stores and login flows over lithos-llm. +//! +//! Every store implements the lithos [`CredentialProvider`] trait, so the +//! client, `credentials::readiness`, and `ClientBuilder::build_ready` read +//! them directly. What is Fabro's own: the vault and the SQL secret store as +//! the place secrets live, the Codex OAuth login and refresh, and the +//! `{{ secrets.NAME }}` interpolation of a provider's `default_headers`. +//! +//! [`CredentialProvider`]: lithos_llm::credentials::CredentialProvider + mod api_key_source; mod context; mod credential; -mod credential_source; -mod error; mod extra_headers_source; mod refresh; mod secrets; @@ -18,8 +26,6 @@ pub mod strategies; pub use api_key_source::ApiKeyCredentialSource; pub use context::{AuthContextRequest, AuthContextResponse}; pub use credential::{OAuthConfig, OAuthCredential, OAuthTokens}; -pub use credential_source::{CredentialSource, ResolvedCredentials, lithos_credentials}; -pub use error::{ResolveError, auth_issue_message}; pub use extra_headers_source::ExtraHeadersCredentialSource; pub use refresh::refresh_oauth_credential; pub use secrets::{accepts_api_key, expected_secret_name, secret_names}; diff --git a/lib/foundation/fabro-auth/src/sql_vault_source.rs b/lib/foundation/fabro-auth/src/sql_vault_source.rs index 6f0c0803b..330eede74 100644 --- a/lib/foundation/fabro-auth/src/sql_vault_source.rs +++ b/lib/foundation/fabro-auth/src/sql_vault_source.rs @@ -3,13 +3,13 @@ use std::sync::Arc; use async_trait::async_trait; use fabro_types::SecretType; use fabro_vault::{SecretSnapshot, SecretStore, SecretStoreError, Vault}; -use lithos_llm::catalog::{Catalog, CatalogProvider, ProviderId}; -use lithos_llm::credentials::Credentials; +use lithos_llm::catalog::{CatalogProvider, ProviderId}; +use lithos_llm::credentials::{CredentialError, CredentialProvider, Credentials}; use tokio::sync::RwLock; use tracing::error; -use crate::credential_source::CredentialSource; -use crate::{EnvLookup, ResolveError, VaultCredentialSource}; +use crate::vault_source::unusable; +use crate::{EnvLookup, VaultCredentialSource}; /// Credentials backed by the SQL secret store. /// @@ -90,11 +90,12 @@ impl SqlVaultCredentialSource { Ok(true) } - fn store_error(provider: &ProviderId, err: SecretStoreError) -> ResolveError { - ResolveError::RefreshFailed { - provider: provider.clone(), - source: anyhow::Error::new(err), - } + fn store_error(provider: &ProviderId, err: SecretStoreError) -> CredentialError { + unusable( + provider, + format!("the secret store could not be read: {err}"), + Some(Box::new(err)), + ) } } @@ -106,8 +107,11 @@ impl std::fmt::Debug for SqlVaultCredentialSource { } #[async_trait] -impl CredentialSource for SqlVaultCredentialSource { - async fn credentials(&self, provider: &CatalogProvider) -> Result { +impl CredentialProvider for SqlVaultCredentialSource { + async fn credentials( + &self, + provider: &CatalogProvider, + ) -> Result { for _ in 0..2 { let before = self .store @@ -134,22 +138,23 @@ impl CredentialSource for SqlVaultCredentialSource { return Ok(credentials); } } - Err(ResolveError::RefreshFailed { - provider: provider.id().clone(), - source: anyhow::anyhow!("OAuth credential changed concurrently during refresh"), - }) + Err(unusable( + provider.id(), + "the OAuth credential changed concurrently during refresh", + None, + )) } - async fn configured_providers(&self, catalog: &Catalog) -> Vec { + async fn is_configured(&self, provider: &CatalogProvider) -> bool { let snapshot = match self.store.snapshot().await { Ok(snapshot) => snapshot, Err(err) => { - error!(error = ?err, "Failed to load configured providers from secret store"); - return Vec::new(); + error!(error = ?err, "Failed to read the secret store while checking a provider"); + return false; } }; self.source_for_snapshot(snapshot) - .configured_providers(catalog) + .is_configured(provider) .await } } diff --git a/lib/foundation/fabro-auth/src/test_support.rs b/lib/foundation/fabro-auth/src/test_support.rs index 0b12e04d7..4030d87c0 100644 --- a/lib/foundation/fabro-auth/src/test_support.rs +++ b/lib/foundation/fabro-auth/src/test_support.rs @@ -9,9 +9,9 @@ use std::sync::Arc; use fabro_vault::Vault; use lithos_llm::catalog::Catalog; +use lithos_llm::credentials::CredentialProvider; use tokio::sync::RwLock as AsyncRwLock; -use crate::credential_source::CredentialSource; use crate::vault_source::VaultCredentialSource; /// The lithos built-in catalog. @@ -45,7 +45,7 @@ pub fn empty_vault() -> Arc> { /// Tests that inject fake provider keys use this instead of reading the real /// process environment, which would make them order-dependent. #[must_use] -pub fn env_credential_source(env_lookup: F) -> Arc +pub fn env_credential_source(env_lookup: F) -> Arc where F: Fn(&str) -> Option + Send + Sync + 'static, { @@ -57,6 +57,6 @@ where /// A vault-backed source over an empty vault with no process-env fallback. #[must_use] -pub fn vault_only_credential_source() -> Arc { +pub fn vault_only_credential_source() -> Arc { Arc::new(VaultCredentialSource::vault_only(empty_vault())) } diff --git a/lib/foundation/fabro-auth/src/vault_source.rs b/lib/foundation/fabro-auth/src/vault_source.rs index f99654d91..c1bef4db0 100644 --- a/lib/foundation/fabro-auth/src/vault_source.rs +++ b/lib/foundation/fabro-auth/src/vault_source.rs @@ -14,6 +14,7 @@ //! never reaches the wire; //! - OpenAI organization and project headers from the environment. +use std::error::Error as StdError; use std::sync::Arc; use async_trait::async_trait; @@ -21,7 +22,7 @@ use fabro_static::EnvVars; use fabro_types::provider_ids; use fabro_types::settings::{InterpString, ResolveCtx}; use fabro_vault::{SecretType, Vault}; -use lithos_llm::catalog::{AuthScheme, Catalog, CatalogProvider, ProviderId}; +use lithos_llm::catalog::{CatalogProvider, ProviderId}; use lithos_llm::credentials::{ ConventionalCredentials, CredentialError, CredentialHeader, CredentialProvider, Credentials, HttpAuthentication, HttpCredentials, SecretValue, @@ -30,8 +31,6 @@ use tokio::sync::RwLock as AsyncRwLock; use tokio::task::spawn_blocking; use crate::credential::OAuthCredential; -use crate::credential_source::CredentialSource; -use crate::error::ResolveError; use crate::refresh::refresh_oauth_credential; use crate::secrets::oauth_secret_name; use crate::vault_ext::{VaultLookupError, vault_get_oauth, vault_set_oauth, vault_token_lookup}; @@ -106,7 +105,7 @@ impl VaultCredentialSource { &self, provider: &CatalogProvider, vault: &Vault, - ) -> Result, ResolveError> { + ) -> Result, CredentialError> { let Some(name) = oauth_secret_name(provider.id()) else { return Ok(None); }; @@ -124,13 +123,20 @@ impl VaultCredentialSource { .expect("entry is present"); let credential = if credential.needs_refresh() { if credential.tokens.refresh_token.is_none() { - return Err(ResolveError::RefreshTokenMissing(provider.id().clone())); + return Err(unusable( + provider.id(), + "requires re-authentication: refresh token missing", + None, + )); } let refreshed = refresh_oauth_credential(&credential) .await - .map_err(|source| ResolveError::RefreshFailed { - provider: provider.id().clone(), - source, + .map_err(|source| { + unusable( + provider.id(), + format!("requires re-authentication: {source}"), + Some(source.into()), + ) })?; self.persist_oauth(provider.id(), name, &refreshed).await?; refreshed @@ -145,13 +151,16 @@ impl VaultCredentialSource { provider: &ProviderId, name: &str, refreshed: &OAuthCredential, - ) -> Result<(), ResolveError> { + ) -> Result<(), CredentialError> { let refreshed = refreshed.clone(); let name = name.to_string(); let vault = Arc::clone(&self.vault); - let failed = |source| ResolveError::RefreshFailed { - provider: provider.clone(), - source, + let failed = |source: anyhow::Error| { + unusable( + provider, + format!("the refreshed token could not be stored: {source}"), + Some(source.into()), + ) }; spawn_blocking(move || { let mut vault = vault.blocking_write(); @@ -204,30 +213,25 @@ impl std::fmt::Debug for VaultCredentialSource { } #[async_trait] -impl CredentialSource for VaultCredentialSource { - async fn credentials(&self, provider: &CatalogProvider) -> Result { +impl CredentialProvider for VaultCredentialSource { + async fn credentials( + &self, + provider: &CatalogProvider, + ) -> Result { let vault = self.snapshot().await; let interpolated = interpolated_headers(&vault, provider)?; if let Some(oauth) = self.oauth_credentials(provider, &vault).await? { return Ok(self.decorate(provider, oauth, interpolated)); } - let credentials = self - .conventional(&vault) - .credentials(provider) - .await - .map_err(|err| resolve_error(provider, &err))?; + let credentials = self.conventional(&vault).credentials(provider).await?; Ok(self.decorate(provider, credentials, interpolated)) } - async fn configured_providers(&self, catalog: &Catalog) -> Vec { + /// Presence without refresh: an OAuth entry under the provider's name, or + /// a conventional secret that resolves. + async fn is_configured(&self, provider: &CatalogProvider) -> bool { let vault = self.snapshot().await; - let mut configured = Vec::new(); - for provider in catalog.providers().filter(|provider| provider.is_enabled()) { - if self.has_credential_material(&vault, provider).await { - configured.push(provider.id().clone()); - } - } - configured + self.has_credential_material(&vault, provider).await } } @@ -254,7 +258,7 @@ fn oauth_bearer(credential: &OAuthCredential) -> Credentials { pub(crate) fn interpolated_headers( vault: &Vault, provider: &CatalogProvider, -) -> Result, ResolveError> { +) -> Result, CredentialError> { let mut ctx = ResolveCtx::new().with_secrets(|secret_name| vault_token_lookup(vault, secret_name)); provider @@ -263,55 +267,42 @@ pub(crate) fn interpolated_headers( .map(|(name, source)| (name, InterpString::parse(source))) .filter(|(_, template)| !template.is_literal()) .map(|(name, template)| { - let value = - template - .resolve_with(&mut ctx) - .map_err(|source| ResolveError::Interpolation { - provider: provider.id().clone(), - source, - })?; + let value = template.resolve_with(&mut ctx).map_err(|source| { + unusable( + provider.id(), + format!("header interpolation failed: {source}"), + Some(Box::new(source)), + ) + })?; Ok(CredentialHeader::new(name.clone(), SecretValue::new(value))) }) .collect() } -pub(crate) fn auth_scheme_name(scheme: &AuthScheme) -> &'static str { - match scheme { - AuthScheme::None => "none", - AuthScheme::Bearer { .. } => "bearer", - AuthScheme::Header { .. } => "header", - AuthScheme::Headers => "headers", - AuthScheme::Aws { .. } => "aws", - AuthScheme::BedrockBearer => "bedrock_bearer", - _ => "unknown", +/// Material is present but cannot be used. `reason` is the operator-facing +/// line and must not carry secret content. +pub(crate) fn unusable( + provider: &ProviderId, + reason: impl Into, + source: Option>, +) -> CredentialError { + CredentialError::Unusable { + provider: provider.clone(), + reason: reason.into(), + source, } } -/// Maps a lithos lookup failure onto Fabro's vocabulary. A missing secret is -/// not an issue to report; the provider is simply not configured. -pub(crate) fn resolve_error(provider: &CatalogProvider, err: &CredentialError) -> ResolveError { - match err { - CredentialError::SchemeMismatch { .. } => ResolveError::SchemeMismatch { - provider: provider.id().clone(), - scheme: auth_scheme_name(provider.auth()).to_string(), - }, - _ => ResolveError::NotConfigured(provider.id().clone()), - } -} - -fn vault_lookup_error(provider: &ProviderId, name: &str, err: VaultLookupError) -> ResolveError { - match err { - VaultLookupError::SchemaMismatch { actual, .. } => ResolveError::VaultSchemaMismatch { - provider: provider.clone(), - name: name.to_string(), - actual, - }, - VaultLookupError::DecodeFailed { source, .. } => ResolveError::VaultDecodeFailed { - provider: provider.clone(), - name: name.to_string(), - source, - }, - } +fn vault_lookup_error(provider: &ProviderId, name: &str, err: VaultLookupError) -> CredentialError { + let reason = match &err { + VaultLookupError::SchemaMismatch { actual, .. } => { + format!("vault credential '{name}' has schema {actual:?}, expected Token or Oauth") + } + VaultLookupError::DecodeFailed { .. } => { + format!("vault credential '{name}' is not valid OAuth JSON") + } + }; + unusable(provider, reason, Some(Box::new(err))) } #[cfg(test)] @@ -322,12 +313,13 @@ mod tests { use httpmock::Method::POST; use httpmock::MockServer; use lithos_llm::catalog::Catalog; + use lithos_llm::credentials::readiness; use super::*; + use crate::OPENAI_CODEX_VAULT_SECRET_NAME; use crate::credential::{OAuthConfig, OAuthTokens}; use crate::test_support::{test_catalog, test_catalog_with_overlay}; use crate::vault_ext::vault_set_token; - use crate::{OPENAI_CODEX_VAULT_SECRET_NAME, auth_issue_message}; fn oauth_credential(token_url: String, expires_at: chrono::DateTime) -> OAuthCredential { OAuthCredential { @@ -488,7 +480,10 @@ api_model = "large" .credentials(catalog.provider("gateway").unwrap()) .await .unwrap_err(); - assert!(matches!(err, ResolveError::Interpolation { .. }), "{err}"); + assert!( + matches!(&err, CredentialError::Unusable { reason, .. } if reason.contains("header interpolation failed")), + "{err}" + ); assert!(!err.to_string().contains("gw-key")); } @@ -565,10 +560,8 @@ api_model = "large" .credentials(catalog.provider("anthropic").unwrap()) .await .unwrap_err(); - assert!(matches!( - err, - ResolveError::NotConfigured(provider) if provider.as_str() == "anthropic" - )); + assert!(err.is_not_configured(), "{err}"); + assert_eq!(err.provider().as_str(), "anthropic"); } #[tokio::test] @@ -578,10 +571,13 @@ api_model = "large" let source = source_with(vault, |_| None); let catalog = test_catalog(); let modal = catalog.provider("modal").unwrap(); - assert!(matches!( - source.credentials(modal).await.unwrap_err(), - ResolveError::NotConfigured(_) - )); + assert!( + source + .credentials(modal) + .await + .unwrap_err() + .is_not_configured() + ); let mut vault = empty_vault(); vault_set_token(&mut vault, "MODAL_TOKEN_ID", "wk-test").unwrap(); @@ -592,6 +588,16 @@ api_model = "large" assert_eq!(header_value(&credentials, "Modal-Secret"), Some("ws-test")); } + async fn configured(source: &VaultCredentialSource, catalog: &Catalog) -> Vec { + let mut ids = Vec::new(); + for provider in catalog.enabled_providers() { + if source.is_configured(provider).await { + ids.push(provider.id().clone()); + } + } + ids + } + #[tokio::test] async fn configured_providers_reads_vault_and_env_without_refreshing() { let mut vault = empty_vault(); @@ -609,7 +615,7 @@ api_model = "large" (name == "ANTHROPIC_API_KEY").then(|| "env".to_string()) }); let catalog = test_catalog(); - let configured = source.configured_providers(&catalog).await; + let configured = configured(&source, &catalog).await; assert!(configured.contains(&ProviderId::new("openai"))); assert!(configured.contains(&ProviderId::new("anthropic"))); assert!(configured.contains(&ProviderId::new("openai-codex"))); @@ -632,13 +638,16 @@ api_model = "large" .unwrap(); vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "anthropic-key").unwrap(); let source = source_with(vault, |_| None); - let resolved = source.resolve_all(&test_catalog()).await; + let catalog = test_catalog(); + let resolved = readiness(catalog.enabled_providers(), &source).await; assert_eq!(resolved.ready, vec![ProviderId::new("anthropic")]); - assert_eq!(resolved.auth_issues.len(), 1); - assert!(matches!( - &resolved.auth_issues[0].1, - ResolveError::RefreshFailed { provider, .. } if provider.as_str() == "openai-codex" - )); + assert_eq!(resolved.issues.len(), 1); + let (provider, issue) = &resolved.issues[0]; + assert_eq!(provider.as_str(), "openai-codex"); + assert!( + issue.to_string().contains("requires re-authentication"), + "{issue}" + ); } #[tokio::test] @@ -646,10 +655,10 @@ api_model = "large" let catalog = test_catalog(); let vault_only = VaultCredentialSource::vault_only(Arc::new(AsyncRwLock::new(empty_vault()))); - assert!(vault_only.configured_providers(&catalog).await.is_empty()); - let resolved = vault_only.resolve_all(&catalog).await; + assert!(configured(&vault_only, &catalog).await.is_empty()); + let resolved = readiness(catalog.enabled_providers(), &vault_only).await; assert!(resolved.ready.is_empty()); - assert!(resolved.auth_issues.is_empty()); + assert!(resolved.issues.is_empty()); } #[tokio::test] @@ -722,10 +731,9 @@ api_model = "large" .credentials(catalog.provider("openai-codex").unwrap()) .await .unwrap_err(); - assert!(matches!(err, ResolveError::RefreshTokenMissing(_))); assert_eq!( - auth_issue_message(&ProviderId::new("openai-codex"), &err), - "openai-codex requires re-authentication: refresh token missing" + err.to_string(), + "credentials for provider openai-codex cannot be used: requires re-authentication: refresh token missing" ); } }