From 05c7fedd317755f07153d1bd48740f85f6b3db3d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 14 Apr 2026 12:28:55 -0400 Subject: [PATCH] refactor(server): remove implicit dry-run fallback Remove the server startup path that inferred dry-run from provider availability and let run.execution.mode inherit normally from settings. Model tests now return skip for unconfigured providers at request time, completions use the real error path, and the CLI/docs/tests are updated for the removed server --dry-run flag. --- Cargo.lock | 1 + docs/administration/server-configuration.mdx | 1 - docs/api-reference/fabro-api.yaml | 3 +- docs/reference/cli.mdx | 1 - lib/crates/fabro-cli/src/commands/doctor.rs | 23 +- lib/crates/fabro-cli/src/commands/model.rs | 74 ++++- .../fabro-cli/src/commands/server/start.rs | 4 - .../fabro-cli/tests/it/cmd/model_test.rs | 95 ++++++ lib/crates/fabro-cli/tests/it/cmd/ps.rs | 2 +- .../fabro-cli/tests/it/cmd/server_start.rs | 20 +- .../tests/it/scenario/server_lifecycle.rs | 2 +- .../fabro-config/src/effective_settings.rs | 15 +- lib/crates/fabro-server/Cargo.toml | 1 + lib/crates/fabro-server/src/run_manifest.rs | 23 -- lib/crates/fabro-server/src/serve.rs | 85 ++---- lib/crates/fabro-server/src/server.rs | 276 +++++------------- lib/crates/fabro-server/src/server_secrets.rs | 4 - .../fabro-server/tests/it/api/system.rs | 4 - lib/crates/fabro-server/tests/it/helpers.rs | 39 +-- .../fabro-server/tests/it/scenario/dry_run.rs | 179 +++++++++++- .../tests/it/scenario/run_completion.rs | 22 +- .../fabro-server/tests/it/scenario/sse.rs | 6 +- .../fabro-server/tests/it/scenario/usage.rs | 11 +- .../src/models/model-test-result.ts | 5 +- 24 files changed, 491 insertions(+), 405 deletions(-) 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];