diff --git a/Cargo.lock b/Cargo.lock index af15ac832..14c6292d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1954,6 +1954,7 @@ dependencies = [ "hex", "hmac", "http-body-util", + "httpmock", "hyper", "hyper-util", "jsonwebtoken", diff --git a/docs/administration/server-configuration.mdx b/docs/administration/server-configuration.mdx index 13d578741..a88bacfe9 100644 --- a/docs/administration/server-configuration.mdx +++ b/docs/administration/server-configuration.mdx @@ -111,7 +111,6 @@ Several `settings.toml` settings can be overridden via `fabro server start` flag | `--sandbox` | — | Override default sandbox provider | | `--max-concurrent-runs` | `5` | Maximum concurrent run executions | | `--config` | `~/.fabro/settings.toml` | Path to server config file | -| `--dry-run` | — | Execute with simulated LLM backend | CLI flags take precedence over `settings.toml` values. See [Run Configuration — Precedence](/execution/run-configuration#precedence) for the full resolution order. diff --git a/docs/api-reference/fabro-api.yaml b/docs/api-reference/fabro-api.yaml index 26a70e49f..7dcc17b18 100644 --- a/docs/api-reference/fabro-api.yaml +++ b/docs/api-reference/fabro-api.yaml @@ -1854,7 +1854,8 @@ components: enum: - ok - error - description: Whether the model responded successfully. + - skip + description: Whether the model responded successfully, failed, or was skipped because its provider is not configured. error_message: type: string nullable: true diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 839b828fa..244a376fe 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -348,7 +348,6 @@ fabro server start --sandbox daytona --max-concurrent-runs 4 | `--foreground` | Run in the foreground instead of daemonizing | — | | `--model ` | Override default LLM model | — | | `--provider ` | Override default LLM provider | — | -| `--dry-run` | Execute with simulated LLM backend | — | | `--sandbox ` | Sandbox for agent tools: `local`, `docker`, or `daytona` | — | | `--max-concurrent-runs ` | Maximum number of concurrent run executions | — | | `--config ` | Path to server config file | `~/.fabro/settings.toml` | diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 8850335d7..bfe271407 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -74,19 +74,16 @@ pub(crate) fn check_config( } fn check_legacy_env(path: Option) -> Option { - match path { - Some(path) => Some(CheckResult { - name: "Legacy .env".to_string(), - status: CheckStatus::Warning, - summary: "legacy secrets file detected".to_string(), - details: vec![CheckDetail::new(format!( - "{} is no longer read by fabro", - path.display() - ))], - remediation: Some("Re-enter credentials with `fabro provider login`.".to_string()), - }), - None => None, - } + path.map(|path| CheckResult { + name: "Legacy .env".to_string(), + status: CheckStatus::Warning, + summary: "legacy secrets file detected".to_string(), + details: vec![CheckDetail::new(format!( + "{} is no longer read by fabro", + path.display() + ))], + remediation: Some("Re-enter credentials with `fabro provider login`.".to_string()), + }) } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/lib/crates/fabro-cli/src/commands/model.rs b/lib/crates/fabro-cli/src/commands/model.rs index f4b25506c..e66e1a9ae 100644 --- a/lib/crates/fabro-cli/src/commands/model.rs +++ b/lib/crates/fabro-cli/src/commands/model.rs @@ -36,6 +36,7 @@ struct ModelTestOutput { results: Vec, total: usize, failures: u32, + skipped: u32, } pub(crate) async fn execute( @@ -247,6 +248,8 @@ async fn test_models_via_server( let mut rows: Vec> = Vec::new(); let mut json_rows = Vec::new(); let mut failures = 0u32; + let mut skipped = 0u32; + let mut skipped_providers: Vec = Vec::new(); if let Some(model_id) = model { if !json_output { eprint!("Testing {model_id}..."); @@ -266,6 +269,10 @@ async fn test_models_via_server( })?; if resp.status == api_types::ModelTestResultStatus::Ok { (info, Color::Green, "ok".to_string()) + } else if resp.status == api_types::ModelTestResultStatus::Skip { + failures += 1; + skipped += 1; + (info, Color::Yellow, "not configured".to_string()) } else { failures += 1; let message = resp @@ -315,6 +322,21 @@ async fn test_models_via_server( Ok(resp) if resp.status == api_types::ModelTestResultStatus::Ok => { (Color::Green, "ok".to_string()) } + Ok(resp) if resp.status == api_types::ModelTestResultStatus::Skip => { + skipped += 1; + let provider_name = info.provider.display_name().to_string(); + if !skipped_providers.contains(&provider_name) { + skipped_providers.push(provider_name); + } + if json_output { + json_rows.push(model_test_row_from_status( + info, + "not configured", + Color::Yellow, + )); + } + continue; + } Ok(resp) => { failures += 1; let message = resp @@ -346,6 +368,7 @@ async fn test_models_via_server( serde_json::to_string_pretty(&ModelTestOutput { total: json_rows.len(), failures, + skipped, results: json_rows, })? ); @@ -355,13 +378,23 @@ async fn test_models_via_server( return Ok(()); } - let table = rows - .table() - .title(title) - .color_choice(color_choice(use_color)) - .border(Border::builder().build()) - .separator(Separator::builder().build()); - println!("{}", table.display()?); + if !rows.is_empty() { + let table = rows + .table() + .title(title) + .color_choice(color_choice(use_color)) + .border(Border::builder().build()) + .separator(Separator::builder().build()); + println!("{}", table.display()?); + } + + if skipped > 0 && model.is_none() { + eprintln!( + "Skipped {} model(s) (no credentials: {})", + skipped, + skipped_providers.join(", ") + ); + } if failures > 0 { bail!("{failures} model(s) failed"); @@ -571,6 +604,33 @@ mod tests { assert_eq!(response.error_message.as_deref(), Some("timeout")); } + #[tokio::test] + async fn test_model_via_server_parses_skip() { + let server = httpmock::MockServer::start_async().await; + server + .mock_async(|when, then| { + when.method("POST").path("/api/v1/models/kimi-k2.5/test"); + then.status(200) + .header("Content-Type", "application/json") + .body( + serde_json::json!({ + "model_id": "kimi-k2.5", + "status": "skip" + }) + .to_string(), + ); + }) + .await; + + let client = test_api_client(&server.url("")); + let response = test_model_via_server(&client, "kimi-k2.5", None) + .await + .unwrap(); + + assert_eq!(response.status, api_types::ModelTestResultStatus::Skip); + assert!(response.error_message.is_none()); + } + #[tokio::test] async fn test_model_via_server_404() { let server = httpmock::MockServer::start_async().await; diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 0611d0e26..b3c51e199 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -86,7 +86,6 @@ async fn ensure_server_running_with_bind( no_web: false, model: None, provider: None, - dry_run: false, sandbox: None, max_concurrent_runs: server_max_concurrent_runs_override(), config: Some(config_path.to_path_buf()), @@ -334,9 +333,6 @@ async fn execute_daemon( if serve_args.no_web { cmd.arg("--no-web"); } - if serve_args.dry_run { - cmd.arg("--dry-run"); - } if let Some(ref sandbox) = serve_args.sandbox { cmd.args(["--sandbox", &sandbox.to_string()]); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/model_test.rs b/lib/crates/fabro-cli/tests/it/cmd/model_test.rs index af8efe243..21d42f552 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/model_test.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/model_test.rs @@ -1,5 +1,17 @@ +use assert_cmd::Command; use fabro_test::{fabro_snapshot, test_context}; +fn remove_provider_env(cmd: &mut Command) -> &mut Command { + cmd.env_remove("ANTHROPIC_API_KEY") + .env_remove("OPENAI_API_KEY") + .env_remove("GEMINI_API_KEY") + .env_remove("GOOGLE_API_KEY") + .env_remove("KIMI_API_KEY") + .env_remove("ZAI_API_KEY") + .env_remove("MINIMAX_API_KEY") + .env_remove("INCEPTION_API_KEY") +} + #[test] fn help() { let context = test_context!(); @@ -43,3 +55,86 @@ fn model_test_unknown_model_errors() { error: Unknown model: nonexistent-model-xyz "); } + +#[test] +fn single_model_skip_exits_nonzero() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["model", "test", "--model", "gemini-3.1-pro-preview"]); + remove_provider_env(&mut cmd); + + fabro_snapshot!(context.filters(), cmd, @" + success: false + exit_code: 1 + ----- stdout ----- + MODEL PROVIDER ALIASES CONTEXT COST SPEED RESULT + gemini-3.1-pro-preview gemini gemini-pro 1m $2.0 / $12.0 85 tok/s not configured + ----- stderr ----- + Testing gemini-3.1-pro-preview... done + error: 1 model(s) failed + "); +} + +#[test] +fn bulk_skip_exits_zero_and_prints_summary() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["model", "test"]); + remove_provider_env(&mut cmd); + + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + ----- stderr ----- + Testing claude-opus-4-6... done + Testing claude-sonnet-4-5... done + Testing claude-sonnet-4-6... done + Testing claude-haiku-4-5... done + Testing gpt-5.2... done + Testing gpt-5-mini... done + Testing gpt-5.2-codex... done + Testing gpt-5.3-codex... done + Testing gpt-5.3-codex-spark... done + Testing gpt-5.4... done + Testing gpt-5.4-pro... done + Testing gpt-5.4-mini... done + Testing gemini-3.1-pro-preview... done + Testing gemini-3.1-pro-preview-customtools... done + Testing gemini-3-flash-preview... done + Testing gemini-3.1-flash-lite-preview... done + Testing kimi-k2.5... done + Testing glm-4.7... done + Testing minimax-m2.5... done + Testing mercury-2... done + Skipped 20 model(s) (no credentials: Anthropic, OpenAI, Gemini, Kimi, Zai, Minimax, Inception) + "); +} + +#[test] +fn json_output_includes_skipped_models() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args([ + "model", + "test", + "--model", + "gemini-3.1-pro-preview", + "--json", + ]); + remove_provider_env(&mut cmd); + + let output = cmd.output().expect("failed to execute model test"); + assert!( + !output.status.success(), + "expected single-model skip to exit non-zero:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout).expect("invalid JSON output"); + + assert_eq!(json["failures"], 1); + assert_eq!(json["skipped"], 1); + assert_eq!(json["results"][0]["result"], "skip"); +} diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index bde099b90..c10bb077d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -45,7 +45,7 @@ fn ps_accepts_local_tcp_server_target() { context .command() .env("FABRO_STORAGE_DIR", &storage_dir) - .args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]) + .args(["server", "start", "--bind", "127.0.0.1"]) .assert() .success(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs index 83c9b4352..3150d96a4 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -48,8 +48,6 @@ fn help() { Override default LLM model --provider Override default LLM provider - --dry-run - Execute with simulated LLM backend --sandbox Sandbox for agent tools --max-concurrent-runs @@ -75,7 +73,7 @@ fn start_already_running_exits_with_error() { context .command() .env("FABRO_STORAGE_DIR", &storage_dir) - .args(["server", "start", "--dry-run", "--bind", &bind_str]) + .args(["server", "start", "--bind", &bind_str]) .assert() .success(); @@ -84,7 +82,7 @@ fn start_already_running_exits_with_error() { filters.push((regex::escape(&bind_str), "[SOCKET_PATH]".to_string())); let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.args(["server", "start", "--dry-run", "--bind", &bind_str]); + cmd.args(["server", "start", "--bind", &bind_str]); fabro_snapshot!(filters, cmd, @" success: false exit_code: 1 @@ -112,7 +110,7 @@ fn start_without_bind_uses_home_socket_instead_of_storage_socket() { context .command() .env("FABRO_STORAGE_DIR", &storage_dir) - .args(["server", "start", "--dry-run"]) + .args(["server", "start"]) .assert() .success(); @@ -159,13 +157,7 @@ address = "127.0.0.1:0" let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.args([ - "server", - "start", - "--dry-run", - "--config", - config_path.to_str().unwrap(), - ]); + cmd.args(["server", "start", "--config", config_path.to_str().unwrap()]); let output = cmd.output().expect("server start command should run"); assert!( output.status.success(), @@ -211,7 +203,7 @@ fn start_with_tcp_host_only_bind_resolves_to_host_and_port() { let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); + cmd.args(["server", "start", "--bind", "127.0.0.1"]); let output = cmd.output().expect("server start command should run"); assert!( output.status.success(), @@ -276,7 +268,7 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); + cmd.args(["server", "start", "--bind", "127.0.0.1"]); fabro_snapshot!(filters, cmd, @" success: true exit_code: 0 diff --git a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs index e6d74b3fe..425633129 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs @@ -25,7 +25,7 @@ fn start_status_stop_lifecycle() { let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.args(["server", "start", "--dry-run", "--bind", &bind_str]); + cmd.args(["server", "start", "--bind", &bind_str]); fabro_snapshot!(filters.clone(), cmd, @" success: true exit_code: 0 diff --git a/lib/crates/fabro-config/src/effective_settings.rs b/lib/crates/fabro-config/src/effective_settings.rs index 915344700..b16f2065b 100644 --- a/lib/crates/fabro-config/src/effective_settings.rs +++ b/lib/crates/fabro-config/src/effective_settings.rs @@ -74,7 +74,7 @@ pub fn materialize_settings_layer( strip_owner_domains(&mut workflow); strip_owner_domains(&mut project); - let server_defaults = server_defaults_file(server_settings); + let server_defaults = server_settings.clone(); let combined = combine_files(combine_files(combine_files(user, project), workflow), args); @@ -111,19 +111,6 @@ fn strip_owner_domains(file: &mut SettingsLayer) { file.server = None; } -/// Copy of the server settings with startup-time dry-run fallback cleared. -/// Run manifests carry their own dry-run intent; a daemon's startup-time -/// fallback mode must not silently force every submitted run into simulation. -fn server_defaults_file(settings: &SettingsLayer) -> SettingsLayer { - let mut out = settings.clone(); - if let Some(run) = out.run.as_mut() { - if let Some(execution) = run.execution.as_mut() { - execution.mode = None; - } - } - out -} - /// Apply server-side defaults to a client-layered [`SettingsLayer`]. /// /// Server-owned domains (`server`, `features`, and parts of `run`) flow from diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index db59de395..3519ec272 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -83,6 +83,7 @@ thiserror.workspace = true tokio = { workspace = true, features = ["test-util", "macros"] } tower = "0.5" http-body-util = "0.1" +httpmock = "0.8" openapiv3 = "2" serde_yaml = "0.9" fabro-sandbox = { path = "../fabro-sandbox" } diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index c223f5b0a..39166f843 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -915,29 +915,6 @@ mod tests { fabro_config::parse_settings_layer(source).expect("v2 fixture should parse") } - #[test] - fn prepare_manifest_does_not_inherit_server_dry_run_fallback() { - let server_settings = server_settings_fixture( - r#" -_version = 1 - -[run.execution] -mode = "dry_run" - -[server.storage] -root = "/srv/fabro" -"#, - ); - - let prepared = - prepare_manifest_with_mode(&server_settings, &minimal_manifest(), false).unwrap(); - - let resolved_run = fabro_config::resolve_run_from_file(&prepared.settings).unwrap(); - let resolved_server = fabro_config::resolve_server_from_file(&prepared.settings).unwrap(); - assert!(resolved_run.execution.mode != fabro_types::settings::run::RunMode::DryRun); - assert_eq!(resolved_server.storage.root.as_source(), "/srv/fabro"); - } - #[test] fn prepare_manifest_preserves_explicit_manifest_dry_run() { let server_settings = server_settings_fixture( diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 2fcc5b349..a358ecf3f 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -14,13 +14,12 @@ use fabro_types::settings::{ ServerSettings as ResolvedServerSettings, SettingsLayer, }; use fabro_util::terminal::Styles; -use fabro_vault::Vault; use object_store::ObjectStore; use object_store::aws::AmazonS3Builder; use object_store::local::LocalFileSystem; use object_store::memory::InMemory; use tokio::net::{TcpListener, UnixListener}; -use tokio::sync::{RwLock as AsyncRwLock, watch}; +use tokio::sync::watch; use tokio::time::interval; use tracing::{error, info, warn}; @@ -31,11 +30,12 @@ use crate::server::{ RouterOptions, build_app_state_with_path, build_router_with_options, reconcile_incomplete_runs_on_startup, shutdown_active_workers, spawn_scheduler, }; -use crate::server_secrets::{ProviderCredentials, ServerSecrets}; +use crate::server_secrets::ServerSecrets; use crate::tls::{build_rustls_config, serve_tls_with_shutdown}; const TEST_IN_MEMORY_STORE_ENV: &str = "FABRO_TEST_IN_MEMORY_STORE"; pub const DEFAULT_TCP_PORT: u16 = 32276; +type EnvLookup = Arc Option + Send + Sync>; #[derive(Clone, Copy)] enum ServerTitlePhase { @@ -68,10 +68,6 @@ pub struct ServeArgs { #[arg(long)] pub provider: Option, - /// Execute with simulated LLM backend - #[arg(long)] - pub dry_run: bool, - /// Sandbox for agent tools #[arg(long, value_enum)] pub sandbox: Option, @@ -89,23 +85,12 @@ fn load_settings(path: Option<&Path>) -> anyhow::Result { Ok(load_settings_config(path)?) } -fn apply_serve_overrides( - base: &SettingsLayer, - args: &ServeArgs, - dry_run_mode: bool, -) -> SettingsLayer { +fn apply_serve_overrides(base: &SettingsLayer, args: &ServeArgs) -> SettingsLayer { use fabro_types::settings::cli::CliLayer; use fabro_types::settings::interp::InterpString; - use fabro_types::settings::run::{ - RunExecutionLayer, RunLayer, RunMode, RunModelLayer, RunSandboxLayer, - }; + use fabro_types::settings::run::{RunLayer, RunModelLayer, RunSandboxLayer}; use fabro_types::settings::server::{ServerLayer, ServerWebLayer}; let mut settings = base.clone(); - if dry_run_mode { - let run = settings.run.get_or_insert_with(RunLayer::default); - let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); - execution.mode = Some(RunMode::DryRun); - } if args.web || args.no_web { let server = settings.server.get_or_insert_with(ServerLayer::default); let web = server.web.get_or_insert_with(ServerWebLayer::default); @@ -134,12 +119,11 @@ fn apply_serve_overrides( fn apply_runtime_settings( base: &SettingsLayer, args: &ServeArgs, - dry_run_mode: bool, data_dir: &Path, ) -> SettingsLayer { use fabro_types::settings::interp::InterpString; use fabro_types::settings::server::{ServerLayer, ServerStorageLayer}; - let mut settings = apply_serve_overrides(base, args, dry_run_mode); + let mut settings = apply_serve_overrides(base, args); let server = settings.server.get_or_insert_with(ServerLayer::default); let storage = server .storage @@ -306,39 +290,10 @@ where }; let storage = Storage::new(&data_dir); let vault_path = storage.secrets_path(); - let vault = Arc::new(AsyncRwLock::new(Vault::load(vault_path.clone())?)); let server_secrets = ServerSecrets::load(storage.server_state().env_path())?; - // Resolve dry-run mode (same pattern as run.rs) - let dry_run_mode = if args.dry_run { - true - } else { - match ProviderCredentials::new(Arc::clone(&vault)) - .build_llm_client() - .await - { - Ok(result) - if result.client.provider_names().is_empty() && result.auth_issues.is_empty() => - { - eprintln!( - "{} No LLM providers configured. Running in dry-run mode.", - styles.yellow.apply_to("Warning:"), - ); - true - } - Ok(_) => false, - Err(e) => { - eprintln!( - "{} Failed to initialize LLM client: {e}. Running in dry-run mode.", - styles.yellow.apply_to("Warning:"), - ); - true - } - } - }; - // Shared config for live reloading - let effective_settings = apply_runtime_settings(&disk_settings, &args, dry_run_mode, &data_dir); + let effective_settings = apply_runtime_settings(&disk_settings, &args, &data_dir); let resolved_server_settings = resolve_server_settings(&effective_settings)?; let bind_request = resolve_bind_request_from_settings(&effective_settings, args.bind.as_deref())?; @@ -365,6 +320,7 @@ where let (artifact_object_store, artifact_prefix) = build_artifact_object_store(&resolved_server_settings)?; let artifact_store = fabro_store::ArtifactStore::new(artifact_object_store, artifact_prefix); + let env_lookup: EnvLookup = Arc::new(|name| std::env::var(name).ok()); let state = build_app_state_with_path( Arc::clone(&shared_settings), None, @@ -373,6 +329,7 @@ where artifact_store, &vault_path, true, + &env_lookup, )?; let reconciled = reconcile_incomplete_runs_on_startup(&state).await?; if reconciled > 0 { @@ -463,7 +420,6 @@ where let effective = apply_runtime_settings( &new_disk_settings, &args_for_poll, - dry_run_mode, &data_dir_for_poll, ); let changed = { @@ -521,7 +477,7 @@ where if tls_settings.is_some() { warn!("TLS is configured but not supported on Unix sockets; ignoring TLS settings"); } - announce_server_ready(&bind_addr, styles, dry_run_mode); + announce_server_ready(&bind_addr, styles); axum::serve(listener, router) .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) .await?; @@ -532,7 +488,7 @@ where let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); info!("TLS enabled"); - announce_server_ready(&bind_addr, styles, dry_run_mode); + announce_server_ready(&bind_addr, styles); serve_tls_with_shutdown( listener, @@ -542,7 +498,7 @@ where ) .await?; } else { - announce_server_ready(&bind_addr, styles, dry_run_mode); + announce_server_ready(&bind_addr, styles); axum::serve(listener, router) .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) .await?; @@ -659,9 +615,9 @@ async fn wait_for_shutdown(mut shutdown_rx: watch::Receiver) { } #[allow(clippy::print_stderr)] // Startup status belongs on stderr for operator-facing CLI output. -fn announce_server_ready(bind_addr: &Bind, styles: &'static Styles, dry_run_mode: bool) { +fn announce_server_ready(bind_addr: &Bind, styles: &'static Styles) { set_server_title(ServerTitlePhase::Listening, Some(bind_addr)); - info!(bind = %bind_addr, dry_run = dry_run_mode, "API server started"); + info!(bind = %bind_addr, "API server started"); eprintln!( "{}", @@ -670,9 +626,6 @@ fn announce_server_ready(bind_addr: &Bind, styles: &'static Styles, dry_run_mode styles.cyan.apply_to(bind_addr) )), ); - if dry_run_mode { - eprintln!("{}", styles.dim.apply_to("(dry-run mode)")); - } } fn set_server_title(phase: ServerTitlePhase, bind: Option<&Bind>) { @@ -723,7 +676,6 @@ mod tests { bind: None, model: None, provider: None, - dry_run: false, sandbox: None, web: false, no_web: false, @@ -731,8 +683,7 @@ mod tests { config: None, }; - let resolved = - apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro-storage")); + let resolved = apply_runtime_settings(&base, &args, &PathBuf::from("/srv/fabro-storage")); let storage_root = resolved .server @@ -757,7 +708,6 @@ enabled = false bind: None, model: None, provider: None, - dry_run: false, sandbox: None, web: true, no_web: false, @@ -765,7 +715,7 @@ enabled = false config: None, }; - let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); + let resolved = apply_runtime_settings(&base, &args, &PathBuf::from("/srv/fabro")); assert_eq!( resolved @@ -784,7 +734,6 @@ enabled = false bind: None, model: None, provider: None, - dry_run: false, sandbox: None, web: false, no_web: true, @@ -792,7 +741,7 @@ enabled = false config: None, }; - let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); + let resolved = apply_runtime_settings(&base, &args, &PathBuf::from("/srv/fabro")); assert_eq!( resolved diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 1b742a5fc..ad7f451a8 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -45,8 +45,8 @@ use fabro_interview::{ use fabro_llm::generate::{GenerateParams, generate_object}; use fabro_llm::model_test::{ModelTestMode, run_model_test_with_client}; use fabro_llm::types::{ - ContentPart, FinishReason, Message as LlmMessage, Request as LlmRequest, - Response as LlmResponse, Role, StreamEvent, TokenCounts, ToolChoice, ToolDefinition, + ContentPart, FinishReason, Message as LlmMessage, Request as LlmRequest, Role, ToolChoice, + ToolDefinition, }; use fabro_model::{BilledModelUsage, BilledTokenCounts}; use fabro_sandbox::daytona::DaytonaSandbox; @@ -86,7 +86,6 @@ use fabro_workflow::run_lookup::{ use fabro_workflow::run_status::{ RunStatus as WorkflowRunStatus, StatusReason as WorkflowStatusReason, }; -use futures_util::stream; use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, Validation}; use object_store::memory::InMemory as MemoryObjectStore; use rand::RngCore; @@ -117,6 +116,8 @@ use crate::server_secrets::{ }; use crate::{demo, diagnostics, run_manifest, settings_view, static_files, web_auth}; +type EnvLookup = Arc Option + Send + Sync>; + pub fn default_page_limit() -> u32 { 20 } @@ -590,12 +591,6 @@ impl AppState { ) } - pub(crate) fn dry_run(&self) -> bool { - fabro_config::resolve_run_from_file(&self.settings.read().unwrap()) - .map(|settings| settings.execution.mode == RunMode::DryRun) - .unwrap_or(false) - } - pub(crate) async fn build_llm_client(&self) -> Result { self.provider_credentials.build_llm_client().await } @@ -2148,6 +2143,7 @@ pub fn create_app_state_with_settings_and_registry_factory( registry_factory_override: impl Fn(Arc) -> HandlerRegistry + Send + Sync + 'static, ) -> Arc { let (store, artifact_store) = test_store_bundle(); + let env_lookup = default_env_lookup(); build_app_state_with_path( Arc::new(RwLock::new(settings)), Some(Box::new(registry_factory_override)), @@ -2156,6 +2152,7 @@ pub fn create_app_state_with_settings_and_registry_factory( artifact_store, &test_secret_store_path(), false, + &env_lookup, ) .expect("test app state should build") } @@ -2166,11 +2163,30 @@ pub fn create_app_state_with_options( max_concurrent_runs: usize, ) -> Arc { let (store, artifact_store) = test_store_bundle(); - create_app_state_with_store( + let env_lookup = default_env_lookup(); + create_app_state_with_store_and_env_lookup( Arc::new(RwLock::new(settings)), max_concurrent_runs, store, artifact_store, + &env_lookup, + ) +} + +#[doc(hidden)] +pub fn create_app_state_with_env_lookup( + settings: SettingsLayer, + max_concurrent_runs: usize, + env_lookup: impl Fn(&str) -> Option + Send + Sync + 'static, +) -> Arc { + let (store, artifact_store) = test_store_bundle(); + let env_lookup: EnvLookup = Arc::new(env_lookup); + create_app_state_with_store_and_env_lookup( + Arc::new(RwLock::new(settings)), + max_concurrent_runs, + store, + artifact_store, + &env_lookup, ) } @@ -2193,6 +2209,7 @@ pub(crate) fn create_test_app_state_with_session_key( .expect("test server env should be writable"); } let (store, artifact_store) = test_store_bundle(); + let env_lookup = default_env_lookup(); build_app_state_with_path( Arc::new(RwLock::new(settings)), None, @@ -2201,6 +2218,7 @@ pub(crate) fn create_test_app_state_with_session_key( artifact_store, &secrets_path, local_daemon_mode, + &env_lookup, ) .expect("test app state should build") } @@ -2221,6 +2239,23 @@ pub fn create_app_state_with_store( max_concurrent_runs: usize, store: Arc, artifact_store: ArtifactStore, +) -> Arc { + let env_lookup = default_env_lookup(); + create_app_state_with_store_and_env_lookup( + settings, + max_concurrent_runs, + store, + artifact_store, + &env_lookup, + ) +} + +fn create_app_state_with_store_and_env_lookup( + settings: Arc>, + max_concurrent_runs: usize, + store: Arc, + artifact_store: ArtifactStore, + env_lookup: &EnvLookup, ) -> Arc { build_app_state_with_path( settings, @@ -2230,10 +2265,15 @@ pub fn create_app_state_with_store( artifact_store, &test_secret_store_path(), false, + env_lookup, ) .expect("test app state should build") } +fn default_env_lookup() -> EnvLookup { + Arc::new(|name| std::env::var(name).ok()) +} + pub(crate) fn build_app_state_with_path( settings: Arc>, registry_factory_override: Option>, @@ -2242,14 +2282,21 @@ pub(crate) fn build_app_state_with_path( artifact_store: ArtifactStore, vault_path: &std::path::Path, local_daemon_mode: bool, + env_lookup: &EnvLookup, ) -> anyhow::Result> { let vault = Arc::new(AsyncRwLock::new(Vault::load(vault_path.to_path_buf())?)); let server_env_path = vault_path.parent().map_or_else( || PathBuf::from("server.env"), |parent| parent.join("server.env"), ); - let server_secrets = ServerSecrets::load(server_env_path)?; - let provider_credentials = ProviderCredentials::new(Arc::clone(&vault)); + let server_secrets = ServerSecrets::with_env_lookup(server_env_path, { + let env_lookup = Arc::clone(env_lookup); + move |name| env_lookup(name) + })?; + let provider_credentials = ProviderCredentials::with_env_lookup(Arc::clone(&vault), { + let env_lookup = Arc::clone(env_lookup); + move |name| env_lookup(name) + }); let (global_event_tx, _) = broadcast::channel(4096); let resolved_server_settings = { let settings = settings.read().expect("settings lock poisoned"); @@ -5839,14 +5886,6 @@ async fn test_model( return ApiError::not_found(format!("Model not found: {id}")).into_response(); }; - if state.dry_run() { - return Json(serde_json::json!({ - "model_id": info.id, - "status": "ok", - })) - .into_response(); - } - let llm_result = match state.build_llm_client().await { Ok(result) => result, Err(err) => { @@ -5864,6 +5903,17 @@ async fn test_model( { return ApiError::bad_request(auth_issue_message(info.provider, issue)).into_response(); } + if !llm_result + .client + .provider_names() + .contains(&info.provider.as_str()) + { + return Json(serde_json::json!({ + "model_id": info.id, + "status": "skip", + })) + .into_response(); + } let client = Arc::new(llm_result.client); let outcome = run_model_test_with_client(info, mode, client).await; @@ -6014,48 +6064,6 @@ async fn create_completion( // Force non-streaming for structured output let use_stream = req.stream && req.schema.is_none(); - // Dry-run mode returns a stub response - if state.dry_run() { - let msg_id = Ulid::new().to_string(); - if use_stream { - let finish_event = - StreamEvent::finish(FinishReason::Stop, TokenCounts::default(), LlmResponse { - id: msg_id.clone(), - model: model_id.clone(), - provider: String::new(), - message: LlmMessage::assistant(""), - finish_reason: FinishReason::Stop, - usage: TokenCounts::default(), - raw: None, - warnings: vec![], - rate_limit: None, - }); - let json = serde_json::to_string(&finish_event).unwrap_or_default(); - let sse_stream = stream::iter(vec![Ok::<_, std::convert::Infallible>( - Event::default().event("stream_event").data(json), - )]); - return Sse::new(sse_stream).into_response(); - } - let empty_msg = CompletionMessage { - role: CompletionMessageRole::Assistant, - content: vec![], - name: None, - tool_call_id: None, - }; - return Json(CompletionResponse { - id: msg_id, - model: model_id, - message: empty_msg, - stop_reason: "end_turn".to_string(), - usage: CompletionUsage { - input_tokens: 0, - output_tokens: 0, - }, - output: None, - }) - .into_response(); - } - // Get or create LLM client (cached in AppState) let llm_result = match state.build_llm_client().await { Ok(result) => result, @@ -6244,20 +6252,6 @@ mod tests { start -> exit }"#; - fn dry_run_settings() -> SettingsLayer { - use fabro_types::settings::run::{RunExecutionLayer, RunLayer, RunMode}; - SettingsLayer { - run: Some(RunLayer { - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - }), - ..SettingsLayer::default() - } - } - fn test_app_with() -> Router { let state = create_app_state(); build_router(state, AuthMode::Disabled) @@ -6711,28 +6705,9 @@ type = "http" assert_eq!(response.status(), StatusCode::NOT_FOUND); } - #[tokio::test] - async fn test_model_known_returns_200_with_status() { - let app = test_app_with(); - - let req = Request::builder() - .method("POST") - .uri(api("/models/claude-opus-4-6/test")) - .header("content-type", "application/json") - .body(Body::empty()) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - - let body = body_json(response.into_body()).await; - assert_eq!(body["model_id"], "claude-opus-4-6"); - assert!(body["status"] == "ok" || body["status"] == "error"); - } - #[tokio::test] async fn test_model_alias_returns_canonical_model_id() { - let state = create_app_state_with_options(dry_run_settings(), 5); + let state = create_app_state_with_env_lookup(SettingsLayer::default(), 5, |_| None); let app = build_router(state, AuthMode::Disabled); let req = Request::builder() @@ -6747,12 +6722,12 @@ type = "http" let body = body_json(response.into_body()).await; assert_eq!(body["model_id"], "claude-sonnet-4-6"); - assert_eq!(body["status"], "ok"); + assert_eq!(body["status"], "skip"); } #[tokio::test] async fn test_model_invalid_mode_returns_400() { - let state = create_app_state_with_options(dry_run_settings(), 5); + let state = create_app_state_with_env_lookup(SettingsLayer::default(), 5, |_| None); let app = build_router(state, AuthMode::Disabled); let req = Request::builder() @@ -6927,42 +6902,6 @@ slug = "fabro" ); } - #[tokio::test] - async fn test_model_dry_run_returns_ok() { - let state = create_app_state_with_options(dry_run_settings(), 5); - let app = build_router(state, AuthMode::Disabled); - - let req = Request::builder() - .method("POST") - .uri(api("/models/claude-opus-4-6/test")) - .header("content-type", "application/json") - .body(Body::empty()) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - - let body = body_json(response.into_body()).await; - assert_eq!(body["model_id"], "claude-opus-4-6"); - assert_eq!(body["status"], "ok"); - } - - #[tokio::test] - async fn test_model_dry_run_unknown_returns_404() { - let state = create_app_state_with_options(dry_run_settings(), 5); - let app = build_router(state, AuthMode::Disabled); - - let req = Request::builder() - .method("POST") - .uri(api("/models/nonexistent-model-xyz/test")) - .header("content-type", "application/json") - .body(Body::empty()) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - assert_eq!(response.status(), StatusCode::NOT_FOUND); - } - #[tokio::test] async fn post_runs_starts_run_and_returns_id() { let app = test_app_with(); @@ -8116,8 +8055,8 @@ level = "debug" let resolved_run = fabro_config::resolve_run_from_file(&run_record.settings).unwrap(); let resolved_server = fabro_config::resolve_server_from_file(&run_record.settings).unwrap(); - // Server-side `dry_run` default must not override the manifest's intent. - // Verify a sampling of the persisted v2 settings. + // Verify a sampling of the persisted v2 settings, including inherited + // run execution mode from server settings. assert_eq!( match resolved_run.goal { Some(fabro_types::settings::run::RunGoal::Inline(value)) => Some(value.as_source()), @@ -8128,8 +8067,8 @@ level = "debug" "goal should be persisted from the manifest" ); assert!( - resolved_run.execution.mode != fabro_types::settings::run::RunMode::DryRun, - "server-local dry_run fallback must not override manifest intent" + resolved_run.execution.mode == fabro_types::settings::run::RunMode::DryRun, + "run execution mode should inherit from server settings" ); assert_eq!( resolved_run @@ -8678,67 +8617,6 @@ timeout = "30s" assert_eq!(response.status(), StatusCode::CONFLICT); } - #[tokio::test] - async fn create_completion_non_streaming_returns_json() { - let state = create_app_state_with_options(dry_run_settings(), 5); - let app = build_router(state, AuthMode::Disabled); - - let req = Request::builder() - .method("POST") - .uri(api("/completions")) - .header("content-type", "application/json") - .body(Body::from( - serde_json::to_string(&serde_json::json!({ - "messages": [{"role": "user", "content": [{"kind": "text", "data": "Hello"}]}], - "stream": false - })) - .unwrap(), - )) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - - let body = body_json(response.into_body()).await; - assert!(body["id"].is_string()); - assert!(body["model"].is_string()); - assert_eq!(body["stop_reason"], "end_turn"); - assert!(body["message"].is_object()); - assert!(body["usage"]["input_tokens"].is_number()); - assert!(body["usage"]["output_tokens"].is_number()); - } - - #[tokio::test] - async fn create_completion_streaming_returns_sse() { - let state = create_app_state_with_options(dry_run_settings(), 5); - let app = build_router(state, AuthMode::Disabled); - - let req = Request::builder() - .method("POST") - .uri(api("/completions")) - .header("content-type", "application/json") - .body(Body::from( - serde_json::to_string(&serde_json::json!({ - "messages": [{"role": "user", "content": [{"kind": "text", "data": "Hello"}]}], - "stream": true - })) - .unwrap(), - )) - .unwrap(); - - let response = app.oneshot(req).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - assert_eq!( - response - .headers() - .get("content-type") - .unwrap() - .to_str() - .unwrap(), - "text/event-stream" - ); - } - #[tokio::test] async fn create_completion_missing_messages_returns_422() { let app = test_app_with(); diff --git a/lib/crates/fabro-server/src/server_secrets.rs b/lib/crates/fabro-server/src/server_secrets.rs index 16b630766..90bb24479 100644 --- a/lib/crates/fabro-server/src/server_secrets.rs +++ b/lib/crates/fabro-server/src/server_secrets.rs @@ -63,10 +63,6 @@ pub(crate) struct ProviderCredentials { } impl ProviderCredentials { - pub(crate) fn new(vault: Arc>) -> Self { - Self::with_env_lookup(vault, |name| std::env::var(name).ok()) - } - pub(crate) fn with_env_lookup(vault: Arc>, env_lookup: F) -> Self where F: Fn(&str) -> Option + Send + Sync + 'static, diff --git a/lib/crates/fabro-server/tests/it/api/system.rs b/lib/crates/fabro-server/tests/it/api/system.rs index df1e0e84f..72e1ebf0e 100644 --- a/lib/crates/fabro-server/tests/it/api/system.rs +++ b/lib/crates/fabro-server/tests/it/api/system.rs @@ -7,7 +7,6 @@ use fabro_config::Storage; use fabro_types::RunId; use fabro_types::settings::SettingsLayer; use fabro_types::settings::interp::InterpString; -use fabro_types::settings::run::{RunExecutionLayer, RunLayer, RunMode}; use fabro_types::settings::server::{ServerLayer, ServerStorageLayer}; use http_body_util::BodyExt; use tempfile::tempdir; @@ -23,9 +22,6 @@ fn temp_storage_settings() -> (tempfile::TempDir, SettingsLayer, PathBuf) { let temp = tempdir().expect("tempdir should create"); let mut settings = test_settings(); let storage_dir = temp.path().join("storage"); - let run = settings.run.get_or_insert_with(RunLayer::default); - let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); - execution.mode = Some(RunMode::DryRun); let server = settings.server.get_or_insert_with(ServerLayer::default); server.storage = Some(ServerStorageLayer { root: Some(InterpString::parse(&storage_dir.to_string_lossy())), diff --git a/lib/crates/fabro-server/tests/it/helpers.rs b/lib/crates/fabro-server/tests/it/helpers.rs index 61cf0fb51..6ca6c0653 100644 --- a/lib/crates/fabro-server/tests/it/helpers.rs +++ b/lib/crates/fabro-server/tests/it/helpers.rs @@ -5,13 +5,11 @@ use axum::body::{Body, to_bytes}; use axum::http::{Request, StatusCode}; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{ - AppState, build_router, create_app_state, create_app_state_with_settings_and_registry_factory, - spawn_scheduler, + AppState, build_router, create_app_state, create_app_state_with_env_lookup, + create_app_state_with_settings_and_registry_factory, spawn_scheduler, }; use fabro_types::settings::SettingsLayer; -use fabro_types::settings::run::{ - LocalSandboxLayer, RunExecutionLayer, RunLayer, RunMode, RunSandboxLayer, WorktreeMode, -}; +use fabro_types::settings::run::{LocalSandboxLayer, RunLayer, RunSandboxLayer, WorktreeMode}; use tokio::time::sleep; use tower::ServiceExt; @@ -54,22 +52,23 @@ pub(crate) fn test_settings() -> SettingsLayer { } } -pub(crate) fn dry_run_settings() -> SettingsLayer { - let mut settings = test_settings(); - let run = settings.run.get_or_insert_with(RunLayer::default); - let execution = run.execution.get_or_insert_with(RunExecutionLayer::default); - execution.mode = Some(RunMode::DryRun); - settings -} - -pub(crate) fn dry_run_app() -> axum::Router { - let state = test_app_state_with_options(dry_run_settings(), 5); +pub(crate) fn test_app_with_scheduler(state: Arc) -> axum::Router { spawn_scheduler(Arc::clone(&state)); build_router(state, AuthMode::Disabled) } -pub(crate) fn test_app_with_scheduler(state: Arc) -> axum::Router { - spawn_scheduler(Arc::clone(&state)); +pub(crate) fn test_app_with_no_providers() -> axum::Router { + let state = create_app_state_with_env_lookup(test_settings(), 5, |_| None); + build_router(state, AuthMode::Disabled) +} + +pub(crate) fn test_app_with_mock_anthropic(mock_base_url: &str) -> axum::Router { + let base_url = mock_base_url.to_string(); + let state = create_app_state_with_env_lookup(test_settings(), 5, move |name| match name { + "ANTHROPIC_API_KEY" => Some("test-key".to_string()), + "ANTHROPIC_BASE_URL" => Some(base_url.clone()), + _ => None, + }); build_router(state, AuthMode::Disabled) } @@ -82,12 +81,6 @@ pub(crate) async fn body_json(body: Body) -> serde_json::Value { serde_json::from_slice(&bytes).unwrap() } -/// Create a run via POST /runs, then start it via POST /runs/{id}/start. -/// Returns the run_id string. -pub(crate) async fn create_and_start_run(app: &axum::Router, dot_source: &str) -> String { - create_and_start_run_from_manifest(app, minimal_manifest_json(dot_source)).await -} - pub(crate) async fn create_and_start_run_from_manifest( app: &axum::Router, manifest: serde_json::Value, diff --git a/lib/crates/fabro-server/tests/it/scenario/dry_run.rs b/lib/crates/fabro-server/tests/it/scenario/dry_run.rs index 897f98aeb..b08ced4a6 100644 --- a/lib/crates/fabro-server/tests/it/scenario/dry_run.rs +++ b/lib/crates/fabro-server/tests/it/scenario/dry_run.rs @@ -1,25 +1,92 @@ +use std::ffi::OsString; +use std::sync::{LazyLock, Mutex, MutexGuard}; + use axum::body::Body; use axum::http::{Request, StatusCode}; +use httpmock::MockServer; use tower::ServiceExt; use crate::helpers::{ - MINIMAL_DOT, api, body_json, create_and_start_run, dry_run_app, minimal_manifest_json, - wait_for_run_status, + MINIMAL_DOT, api, body_json, create_and_start_run_from_manifest, minimal_manifest_json, + minimal_manifest_json_with_dry_run, test_app_state_with_options, test_app_with_mock_anthropic, + test_app_with_no_providers, test_app_with_scheduler, test_settings, wait_for_run_status, }; +static ENV_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); + +struct ProxyPolicyGuard { + _lock: MutexGuard<'static, ()>, + previous: Option, +} + +impl ProxyPolicyGuard { + fn disabled() -> Self { + let lock = ENV_LOCK.lock().unwrap(); + let previous = std::env::var_os(fabro_http::HTTP_PROXY_POLICY_ENV); + std::env::set_var(fabro_http::HTTP_PROXY_POLICY_ENV, "disabled"); + Self { + _lock: lock, + previous, + } + } +} + +impl Drop for ProxyPolicyGuard { + fn drop(&mut self) { + match self.previous.as_ref() { + Some(value) => std::env::set_var(fabro_http::HTTP_PROXY_POLICY_ENV, value), + None => std::env::remove_var(fabro_http::HTTP_PROXY_POLICY_ENV), + } + } +} + +fn completion_request(stream: bool) -> Request { + Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_string(&serde_json::json!({ + "messages": [{"role": "user", "content": [{"kind": "text", "data": "Hello"}]}], + "stream": stream + })) + .unwrap(), + )) + .unwrap() +} + +fn completion_request_with_model(stream: bool, model: &str) -> Request { + Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_string(&serde_json::json!({ + "model": model, + "messages": [{"role": "user", "content": [{"kind": "text", "data": "Hi"}]}], + "stream": stream + })) + .unwrap(), + )) + .unwrap() +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn dry_run_serve_starts_and_runs_workflow() { - let app = dry_run_app(); + let state = test_app_state_with_options(test_settings(), 5); + let app = test_app_with_scheduler(state); - let run_id = create_and_start_run(&app, MINIMAL_DOT).await; + let run_id = + create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) + .await; let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; assert_eq!(status, "succeeded"); } #[tokio::test] -async fn test_model_known_via_full_router() { - let app = dry_run_app(); +async fn test_model_skip_when_no_providers() { + let app = test_app_with_no_providers(); let req = Request::builder() .method("POST") @@ -33,13 +100,12 @@ async fn test_model_known_via_full_router() { let body = body_json(response.into_body()).await; assert_eq!(body["model_id"], "claude-opus-4-6"); - // No API keys in test env, so status will be "error" - assert!(body["status"] == "ok" || body["status"] == "error"); + assert_eq!(body["status"], "skip"); } #[tokio::test] async fn test_model_unknown_via_full_router() { - let app = dry_run_app(); + let app = test_app_with_no_providers(); let req = Request::builder() .method("POST") @@ -54,7 +120,7 @@ async fn test_model_unknown_via_full_router() { #[tokio::test] async fn dry_run_serve_rejects_invalid_dot() { - let app = dry_run_app(); + let app = test_app_with_no_providers(); let req = Request::builder() .method("POST") @@ -68,3 +134,96 @@ async fn dry_run_serve_rejects_invalid_dot() { let response = app.oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::BAD_REQUEST); } + +#[tokio::test] +async fn completion_no_provider_non_streaming_returns_502() { + let app = test_app_with_no_providers(); + + let response = app.oneshot(completion_request(false)).await.unwrap(); + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); +} + +#[tokio::test] +async fn completion_no_provider_streaming_returns_502() { + let app = test_app_with_no_providers(); + + let response = app.oneshot(completion_request(true)).await.unwrap(); + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); +} + +#[tokio::test] +async fn completion_non_streaming_returns_valid_json() { + let _proxy_guard = ProxyPolicyGuard::disabled(); + let mock_server = MockServer::start_async().await; + mock_server + .mock_async(|when, then| { + when.method("POST").path("/v1/messages"); + then.status(200) + .header("content-type", "application/json") + .body( + serde_json::to_string(&serde_json::json!({ + "id": "msg_test_123", + "type": "message", + "role": "assistant", + "model": "claude-sonnet-4-5", + "content": [{"type": "text", "text": "Hello!"}], + "stop_reason": "end_turn", + "usage": {"input_tokens": 10, "output_tokens": 5} + })) + .unwrap(), + ); + }) + .await; + + let app = test_app_with_mock_anthropic(&format!("{}/v1", mock_server.url(""))); + + let response = app + .oneshot(completion_request_with_model(false, "claude-sonnet-4-5")) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + + let body = body_json(response.into_body()).await; + assert!(body["id"].is_string()); + assert_eq!(body["model"], "claude-sonnet-4-5"); + assert_eq!(body["stop_reason"], "end_turn"); + assert!(body["message"].is_object()); + assert!(body["usage"]["input_tokens"].is_number()); + assert!(body["usage"]["output_tokens"].is_number()); +} + +#[tokio::test] +async fn completion_streaming_returns_sse() { + let _proxy_guard = ProxyPolicyGuard::disabled(); + let mock_server = MockServer::start_async().await; + mock_server + .mock_async(|when, then| { + when.method("POST").path("/v1/messages"); + then.status(200) + .header("content-type", "text/event-stream") + .body( + "event: message_start\ndata: {\"type\":\"message_start\",\"message\":{\"id\":\"msg_test\",\"type\":\"message\",\"role\":\"assistant\",\"content\":[],\"model\":\"claude-sonnet-4-5\",\"stop_reason\":null,\"usage\":{\"input_tokens\":10,\"output_tokens\":0}}}\n\n\ + event: content_block_start\ndata: {\"type\":\"content_block_start\",\"index\":0,\"content_block\":{\"type\":\"text\",\"text\":\"\"}}\n\n\ + event: content_block_delta\ndata: {\"type\":\"content_block_delta\",\"index\":0,\"delta\":{\"type\":\"text_delta\",\"text\":\"Hi\"}}\n\n\ + event: content_block_stop\ndata: {\"type\":\"content_block_stop\",\"index\":0}\n\n\ + event: message_delta\ndata: {\"type\":\"message_delta\",\"delta\":{\"stop_reason\":\"end_turn\"},\"usage\":{\"output_tokens\":1}}\n\n\ + event: message_stop\ndata: {\"type\":\"message_stop\"}\n\n", + ); + }) + .await; + + let app = test_app_with_mock_anthropic(&format!("{}/v1", mock_server.url(""))); + + let response = app + .oneshot(completion_request_with_model(true, "claude-sonnet-4-5")) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let content_type = response + .headers() + .get("content-type") + .unwrap() + .to_str() + .unwrap(); + assert!(content_type.contains("text/event-stream")); +} diff --git a/lib/crates/fabro-server/tests/it/scenario/run_completion.rs b/lib/crates/fabro-server/tests/it/scenario/run_completion.rs index c8952956a..36aa38b98 100644 --- a/lib/crates/fabro-server/tests/it/scenario/run_completion.rs +++ b/lib/crates/fabro-server/tests/it/scenario/run_completion.rs @@ -4,16 +4,18 @@ use tokio::time::sleep; use tower::ServiceExt; use crate::helpers::{ - MINIMAL_DOT, api, create_and_start_run, dry_run_settings, test_app_state_with_options, - test_app_with_scheduler, wait_for_run_status, + MINIMAL_DOT, api, create_and_start_run_from_manifest, minimal_manifest_json_with_dry_run, + test_app_state_with_options, test_app_with_scheduler, test_settings, wait_for_run_status, }; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn run_completes_and_status_is_completed() { - let state = test_app_state_with_options(dry_run_settings(), 5); + let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); - let run_id = create_and_start_run(&app, MINIMAL_DOT).await; + let run_id = + create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) + .await; let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; assert_eq!(status, "succeeded"); @@ -21,10 +23,12 @@ async fn run_completes_and_status_is_completed() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn attach_run_events_returns_sse_stream() { - let state = test_app_state_with_options(dry_run_settings(), 5); + let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); - let run_id = create_and_start_run(&app, MINIMAL_DOT).await; + let run_id = + create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) + .await; // Wait for scheduler to promote run. sleep(std::time::Duration::from_millis(100)).await; @@ -51,10 +55,12 @@ async fn attach_run_events_returns_sse_stream() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn attach_run_events_replays_terminal_event_after_completion() { - let state = test_app_state_with_options(dry_run_settings(), 5); + let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); - let run_id = create_and_start_run(&app, MINIMAL_DOT).await; + let run_id = + create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) + .await; let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; assert_eq!(status, "succeeded"); diff --git a/lib/crates/fabro-server/tests/it/scenario/sse.rs b/lib/crates/fabro-server/tests/it/scenario/sse.rs index cb2cb16a3..c0946752f 100644 --- a/lib/crates/fabro-server/tests/it/scenario/sse.rs +++ b/lib/crates/fabro-server/tests/it/scenario/sse.rs @@ -8,8 +8,8 @@ use tower::ServiceExt; use crate::helpers::{ POLL_ATTEMPTS, POLL_INTERVAL, api, body_json, create_and_start_run_from_manifest, - dry_run_settings, minimal_manifest_json_with_dry_run, test_app_state_with_options, - test_app_with_scheduler, wait_for_run_status_not_in, + minimal_manifest_json_with_dry_run, test_app_state_with_options, test_app_with_scheduler, + test_settings, wait_for_run_status_not_in, }; const SIMPLE_DOT: &str = r#"digraph SSETest { @@ -38,7 +38,7 @@ async fn wait_for_checkpoint(app: &axum::Router, run_id: &str) -> serde_json::Va #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn sse_stream_contains_expected_event_types() { - let state = test_app_state_with_options(dry_run_settings(), 5); + let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); let run_id = diff --git a/lib/crates/fabro-server/tests/it/scenario/usage.rs b/lib/crates/fabro-server/tests/it/scenario/usage.rs index 158723511..93a26beaa 100644 --- a/lib/crates/fabro-server/tests/it/scenario/usage.rs +++ b/lib/crates/fabro-server/tests/it/scenario/usage.rs @@ -4,16 +4,19 @@ use tokio::time::sleep; use tower::ServiceExt; use crate::helpers::{ - MINIMAL_DOT, POLL_ATTEMPTS, POLL_INTERVAL, api, body_json, create_and_start_run, - dry_run_settings, test_app_state_with_options, test_app_with_scheduler, wait_for_run_status, + MINIMAL_DOT, POLL_ATTEMPTS, POLL_INTERVAL, api, body_json, create_and_start_run_from_manifest, + minimal_manifest_json_with_dry_run, test_app_state_with_options, test_app_with_scheduler, + test_settings, wait_for_run_status, }; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn aggregate_billing_increments_after_run_completes() { - let state = test_app_state_with_options(dry_run_settings(), 5); + let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); - let run_id = create_and_start_run(&app, MINIMAL_DOT).await; + let run_id = + create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) + .await; // Poll until run completes let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; diff --git a/lib/packages/fabro-api-client/src/models/model-test-result.ts b/lib/packages/fabro-api-client/src/models/model-test-result.ts index 8a1acfdbb..f21c5fbda 100644 --- a/lib/packages/fabro-api-client/src/models/model-test-result.ts +++ b/lib/packages/fabro-api-client/src/models/model-test-result.ts @@ -23,7 +23,7 @@ export interface ModelTestResult { */ 'model_id': string; /** - * Whether the model responded successfully. + * Whether the model responded successfully, failed, or was skipped because its provider is not configured. */ 'status': ModelTestResultStatusEnum; /** @@ -34,7 +34,8 @@ export interface ModelTestResult { export const ModelTestResultStatusEnum = { OK: 'ok', - ERROR: 'error' + ERROR: 'error', + SKIP: 'skip' } as const; export type ModelTestResultStatusEnum = typeof ModelTestResultStatusEnum[keyof typeof ModelTestResultStatusEnum];