diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 6fa3f671e..b3797c6a8 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -77,12 +77,15 @@ impl ServerTargetArgs { #[derive(Args, Debug, Clone, Default)] pub(crate) struct ServerConnectionArgs { /// Local storage directory (default: ~/.fabro) - #[arg(long, env = "FABRO_STORAGE_DIR", conflicts_with = "server")] + #[arg(long, env = "FABRO_STORAGE_DIR")] pub(crate) storage_dir: Option, /// Fabro server target: http(s) URL or absolute Unix socket path - #[arg(long = "server", env = "FABRO_SERVER", conflicts_with = "storage_dir")] + #[arg(long = "server", env = "FABRO_SERVER")] pub(crate) server: Option, + + #[arg(skip)] + pub(crate) storage_dir_explicit: bool, } impl ServerConnectionArgs { @@ -93,6 +96,10 @@ impl ServerConnectionArgs { pub(crate) fn server(&self) -> Option<&str> { self.server.as_deref() } + + pub(crate) fn storage_dir_is_explicit(&self) -> bool { + self.storage_dir_explicit + } } #[derive(Debug, Clone, Copy, ValueEnum)] @@ -125,7 +132,7 @@ impl From for CliSandboxProvider { #[derive(Args)] pub(crate) struct RunArgs { #[command(flatten)] - pub(crate) storage_dir: StorageDirArgs, + pub(crate) target: ServerConnectionArgs, /// Path to a .fabro workflow file or .toml task config #[arg(required = true)] diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 20420ec73..0742814ae 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -808,6 +808,7 @@ pub(crate) async fn run_install(args: &InstallArgs, globals: &GlobalArgs) -> Res target: ServerConnectionArgs { storage_dir: Some(storage_dir.clone()), server: None, + storage_dir_explicit: true, }, verbose: true, }; diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 4201e1336..e9ac16d4f 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -47,29 +47,7 @@ pub(crate) async fn attach_run( if let (Some(storage_dir), Some(run_id)) = (storage_dir.as_deref(), run_id.as_ref()) { let client = server_client::connect_server(storage_dir).await?; - let state = client.get_run_state(run_id).await?; - let verbose = state - .run - .as_ref() - .is_some_and(|record| record.settings.verbose_enabled()); - let events = client.list_run_events(run_id, None, None).await?; - let event_lines = events - .iter() - .map(event_payload_line) - .collect::>>()?; - let initial_exit_code = events.iter().rev().find_map(event_exit_code); - return attach_run_server( - &client, - run_id, - verbose, - event_lines, - events.last().map_or(0, |event| event.seq), - initial_exit_code, - kill_on_detach, - styles, - json_output, - ) - .await; + return attach_run_with_client(&client, run_id, kill_on_detach, styles, json_output).await; } Err(anyhow::anyhow!( @@ -77,6 +55,38 @@ pub(crate) async fn attach_run( )) } +pub(crate) async fn attach_run_with_client( + client: &server_client::ServerStoreClient, + run_id: &RunId, + kill_on_detach: bool, + styles: &'static Styles, + json_output: bool, +) -> Result { + let state = client.get_run_state(run_id).await?; + let verbose = state + .run + .as_ref() + .is_some_and(|record| record.settings.verbose_enabled()); + let events = client.list_run_events(run_id, None, None).await?; + let event_lines = events + .iter() + .map(event_payload_line) + .collect::>>()?; + let initial_exit_code = events.iter().rev().find_map(event_exit_code); + attach_run_server( + client, + run_id, + verbose, + event_lines, + events.last().map_or(0, |event| event.seq), + initial_exit_code, + kill_on_detach, + styles, + json_output, + ) + .await +} + async fn attach_run_server( client: &server_client::ServerStoreClient, run_id: &RunId, diff --git a/lib/crates/fabro-cli/src/commands/run/command.rs b/lib/crates/fabro-cli/src/commands/run/command.rs index 8bd81a368..fab1d50da 100644 --- a/lib/crates/fabro-cli/src/commands/run/command.rs +++ b/lib/crates/fabro-cli/src/commands/run/command.rs @@ -2,19 +2,19 @@ use anyhow::Result; use fabro_util::terminal::Styles; use crate::args::{GlobalArgs, RunArgs}; +use crate::server_client; use crate::shared::print_json_pretty; use crate::user_config::{self, user_layer_with_storage_dir}; pub(crate) async fn execute(mut args: RunArgs, globals: &GlobalArgs) -> Result<()> { let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); - let cli_settings = - user_config::load_user_settings_with_storage_dir(args.storage_dir.as_deref())?; - let cli = user_layer_with_storage_dir(args.storage_dir.as_deref())?; + let cli_settings = user_config::load_user_settings_with_storage_dir(args.target.storage_dir())?; + let cli = user_layer_with_storage_dir(args.target.storage_dir())?; args.verbose = args.verbose || cli_settings.verbose_enabled(); let quiet = args.detach; let prevent_idle_sleep = cli_settings.prevent_idle_sleep_enabled(); - let (run_id, run_dir) = Box::pin(super::create::create_run(&args, cli, styles, quiet)).await?; + let created_run = Box::pin(super::create::create_run(&args, cli, styles, quiet)).await?; #[cfg(feature = "sleep_inhibitor")] let _sleep_guard = crate::sleep_inhibitor::guard(prevent_idle_sleep); @@ -22,29 +22,29 @@ pub(crate) async fn execute(mut args: RunArgs, globals: &GlobalArgs) -> Result<( #[cfg(not(feature = "sleep_inhibitor"))] let _ = prevent_idle_sleep; - super::start::start_run(&run_id, &cli_settings.storage_dir(), false).await?; + let client = server_client::connect_server_connection(&created_run.connection).await?; + super::start::start_run_with_client(&client, &created_run.run_id, false).await?; if args.detach { if globals.json { - print_json_pretty(&serde_json::json!({ "run_id": run_id }))?; + print_json_pretty(&serde_json::json!({ "run_id": created_run.run_id }))?; } else { - println!("{run_id}"); + println!("{}", created_run.run_id); } } else { - let exit_code = super::attach::attach_run( - &run_dir, - Some(cli_settings.storage_dir().as_path()), - Some(&run_id), + let exit_code = super::attach::attach_run_with_client( + &client, + &created_run.run_id, true, styles, globals.json, ) .await?; if !globals.json { - super::output::print_run_summary( - cli_settings.storage_dir().as_path(), - &run_dir, - run_id, + super::output::print_run_summary_with_client( + &client, + &created_run.run_id, + created_run.local_run_dir.as_deref(), styles, ) .await?; diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index e8fe8ac5d..0e8c484d0 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -9,6 +9,13 @@ use fabro_workflow::operations::make_run_dir; use super::output::{api_diagnostics_to_local, print_preflight_workflow_summary}; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; use crate::server_client; +use crate::user_config::{self, ServerConnection}; + +pub(crate) struct CreatedRun { + pub(crate) run_id: RunId, + pub(crate) local_run_dir: Option, + pub(crate) connection: ServerConnection, +} /// Create a workflow run: allocate run directory, persist RunRecord, return (run_id, run_dir). /// @@ -18,7 +25,7 @@ pub(crate) async fn create_run( cli_defaults: ConfigLayer, styles: &Styles, quiet: bool, -) -> anyhow::Result<(RunId, PathBuf)> { +) -> anyhow::Result { let workflow_path = args .workflow .as_ref() @@ -45,7 +52,8 @@ pub(crate) async fn create_run( run_id, })?; - let client = server_client::connect_server(settings.storage_dir().as_path()).await?; + let connection = user_config::server_backed_command_connection(&args.target, &settings)?; + let client = server_client::connect_server_connection(&connection).await?; if !quiet { let preflight = client.run_preflight(built.manifest.clone()).await?; let diagnostics = api_diagnostics_to_local(&preflight.workflow.diagnostics); @@ -58,7 +66,16 @@ pub(crate) async fn create_run( } let created_run_id = client.create_run_from_manifest(built.manifest).await?; - let run_dir = make_run_dir(&settings.storage_dir().join("runs"), &created_run_id); + let local_run_dir = match &connection { + ServerConnection::Local { storage_dir } => { + Some(make_run_dir(&storage_dir.join("runs"), &created_run_id)) + } + ServerConnection::Target(_) => None, + }; - Ok((created_run_id, run_dir)) + Ok(CreatedRun { + run_id: created_run_id, + local_run_dir, + connection, + }) } diff --git a/lib/crates/fabro-cli/src/commands/run/mod.rs b/lib/crates/fabro-cli/src/commands/run/mod.rs index 9446c32c2..17dd2272b 100644 --- a/lib/crates/fabro-cli/src/commands/run/mod.rs +++ b/lib/crates/fabro-cli/src/commands/run/mod.rs @@ -39,12 +39,12 @@ pub(crate) async fn dispatch(cmd: RunCommands, globals: &GlobalArgs) -> Result<( RunCommands::Create(mut args) => { apply_json_defaults(&mut args, globals); let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); - let cli = user_layer_with_storage_dir(args.storage_dir.as_deref())?; - let (run_id, _run_dir) = Box::pin(create::create_run(&args, cli, styles, true)).await?; + let cli = user_layer_with_storage_dir(args.target.storage_dir())?; + let created_run = Box::pin(create::create_run(&args, cli, styles, true)).await?; if globals.json { - print_json_pretty(&serde_json::json!({ "run_id": run_id }))?; + print_json_pretty(&serde_json::json!({ "run_id": created_run.run_id }))?; } else { - println!("{run_id}"); + println!("{}", created_run.run_id); } Ok(()) } diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index ab04a0c55..623be7cf3 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -128,42 +128,50 @@ pub(crate) async fn print_run_summary( run_id: impl std::fmt::Display, styles: &Styles, ) -> Result<()> { - let run_id = run_id.to_string(); - let (checkpoint, conclusion, pr_url) = match run_id.parse() { - Ok(parsed_run_id) => { - let client = server_client::connect_server(storage_dir).await?; - let run_state = client.get_run_state(&parsed_run_id).await?; - let checkpoint = run_state.checkpoint.clone(); - let conclusion = run_state.conclusion.clone(); - let pr_url = run_state - .pull_request - .as_ref() - .map(|record: &PullRequestRecord| record.html_url.clone()); - (checkpoint, conclusion, pr_url) - } - Err(_) => (None, None, None), - }; + let run_id = run_id + .to_string() + .parse() + .map_err(|err| anyhow::anyhow!("invalid run ID: {err}"))?; + let client = server_client::connect_server(storage_dir).await?; + print_run_summary_with_client(&client, &run_id, Some(run_dir), styles).await +} + +pub(crate) async fn print_run_summary_with_client( + client: &server_client::ServerStoreClient, + run_id: &fabro_types::RunId, + local_run_dir: Option<&Path>, + styles: &Styles, +) -> Result<()> { + let run_state = client.get_run_state(run_id).await?; + let checkpoint = run_state.checkpoint.clone(); + let conclusion = run_state.conclusion.clone(); + let pr_url = run_state + .pull_request + .as_ref() + .map(|record: &PullRequestRecord| record.html_url.clone()); let Some(conclusion) = conclusion else { return Ok(()); }; print_run_conclusion( &conclusion, - &run_id, - run_dir, + run_id, + local_run_dir, None, pr_url.as_deref(), styles, ); - print_final_output(checkpoint.as_ref(), run_dir, styles); - print_assets(run_dir, styles); + print_final_output(checkpoint.as_ref(), styles); + if let Some(run_dir) = local_run_dir { + print_assets(run_dir, styles); + } Ok(()) } pub(crate) fn print_run_conclusion( conclusion: &Conclusion, run_id: impl std::fmt::Display, - run_dir: &Path, + run_dir: Option<&Path>, pushed_branch: Option<&str>, pr_url: Option<&str>, styles: &Styles, @@ -227,12 +235,14 @@ pub(crate) fn print_run_conclusion( } } - eprintln!( - "{}", - styles - .dim - .apply_to(format!("Run: {}", tilde_path(run_dir))) - ); + if let Some(run_dir) = run_dir { + eprintln!( + "{}", + styles + .dim + .apply_to(format!("Run: {}", tilde_path(run_dir))) + ); + } if let Some(ref failure) = conclusion.failure_reason { eprintln!("Failure: {}", styles.red.apply_to(failure)); @@ -249,11 +259,7 @@ pub(crate) fn print_run_conclusion( } } -pub(crate) fn print_final_output( - checkpoint: Option<&fabro_types::Checkpoint>, - _run_dir: &Path, - styles: &Styles, -) { +pub(crate) fn print_final_output(checkpoint: Option<&fabro_types::Checkpoint>, styles: &Styles) { let Some(checkpoint) = checkpoint else { return; }; diff --git a/lib/crates/fabro-cli/src/commands/run/start.rs b/lib/crates/fabro-cli/src/commands/run/start.rs index 3066e48a7..6931c0d14 100644 --- a/lib/crates/fabro-cli/src/commands/run/start.rs +++ b/lib/crates/fabro-cli/src/commands/run/start.rs @@ -8,5 +8,13 @@ use crate::server_client; /// Queue a run for server-owned execution. pub(crate) async fn start_run(run_id: &RunId, storage_dir: &Path, resume: bool) -> Result<()> { let client = server_client::connect_server(storage_dir).await?; + start_run_with_client(&client, run_id, resume).await +} + +pub(crate) async fn start_run_with_client( + client: &server_client::ServerStoreClient, + run_id: &RunId, + resume: bool, +) -> Result<()> { client.start_run(run_id, resume).await } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 3561eac6c..a6c7e5d65 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -13,12 +13,13 @@ mod user_config; use anyhow::Result; use args::{Commands, GlobalArgs, LONG_VERSION, RunCommands, ServerCommand, ServerNamespace}; -use clap::{CommandFactory, Parser}; +use clap::{CommandFactory, FromArgMatches, Parser, error::ErrorKind, parser::ValueSource}; use fabro_config::server::load_server_settings; use fabro_telemetry::{git, panic as tel_panic, sanitize, sender}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use rustls::crypto::ring::default_provider; +use std::ffi::OsString; use tracing::debug; #[derive(Parser)] @@ -31,6 +32,123 @@ struct Cli { command: Box, } +#[derive(Clone, Copy, Debug, Default)] +struct ServerConnectionValueSources { + storage_dir: Option, + server: Option, +} + +impl ServerConnectionValueSources { + fn command_line_conflict(self) -> bool { + self.storage_dir == Some(ValueSource::CommandLine) + && self.server == Some(ValueSource::CommandLine) + } + + fn storage_dir_is_explicit(self) -> bool { + self.storage_dir == Some(ValueSource::CommandLine) + } +} + +impl Cli { + fn parse() -> Self { + Self::try_parse_from(std::env::args_os()).unwrap_or_else(|err| err.exit()) + } + + fn try_parse_from(args: I) -> Result + where + I: IntoIterator, + T: Into + Clone, + { + let args: Vec = args.into_iter().map(Into::into).collect(); + let mut command = Self::command(); + let mut matches = command.try_get_matches_from_mut(args)?; + let sources = server_connection_value_sources(&matches); + if sources.command_line_conflict() { + return Err(command.error( + ErrorKind::ArgumentConflict, + "the argument '--server ' cannot be used with '--storage-dir '", + )); + } + + let mut cli = ::from_arg_matches_mut(&mut matches)?; + cli.apply_server_connection_value_sources(sources); + Ok(cli) + } + + fn apply_server_connection_value_sources(&mut self, sources: ServerConnectionValueSources) { + self.command.apply_server_connection_value_sources(sources); + } +} + +impl Commands { + fn apply_server_connection_value_sources(&mut self, sources: ServerConnectionValueSources) { + match self { + Self::RunCmd(RunCommands::Run(args) | RunCommands::Create(args)) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Preflight(args) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Validate(args) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Graph(args) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Model { + command: Some(args::ModelsCommand::List(args)), + } => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Model { + command: Some(args::ModelsCommand::Test(args)), + } => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Doctor(args) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Repo(args::RepoNamespace { + command: args::RepoCommand::Init(args), + }) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Provider(args::ProviderNamespace { + command: args::ProviderCommand::Login(args), + }) => { + args.target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + Self::Secret(args::SecretNamespace { target, .. }) => { + target.storage_dir_explicit = sources.storage_dir_is_explicit(); + } + _ => {} + } + } +} + +fn server_connection_value_sources(matches: &clap::ArgMatches) -> ServerConnectionValueSources { + let sources = ServerConnectionValueSources { + storage_dir: matches + .try_contains_id("storage_dir") + .ok() + .and_then(|present| present.then(|| matches.value_source("storage_dir"))) + .flatten(), + server: matches + .try_contains_id("server") + .ok() + .and_then(|present| present.then(|| matches.value_source("server"))) + .flatten(), + }; + if sources.storage_dir.is_some() || sources.server.is_some() { + return sources; + } + + matches + .subcommand() + .map(|(_, subcommand_matches)| server_connection_value_sources(subcommand_matches)) + .unwrap_or_default() +} + #[tokio::main] async fn main() { tel_panic::install_panic_hook(); @@ -273,7 +391,6 @@ mod tests { use args::{ Commands, ModelsCommand, ProviderCommand, ProviderNamespace, StoreCommand, StoreNamespace, }; - use clap::Parser; #[test] fn parse_provider_login_openai() { @@ -344,7 +461,7 @@ mod tests { match *cli.command { Commands::RunCmd(RunCommands::Run(args)) => { assert_eq!( - args.storage_dir.as_deref(), + args.target.storage_dir(), Some(std::path::Path::new("/tmp/fabro")) ); assert_eq!( diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 8ac65b880..bdbf5d34d 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -114,6 +114,14 @@ pub(crate) async fn connect_server_backed( }) } +pub(crate) async fn connect_server_connection( + connection: &user_config::ServerConnection, +) -> Result { + Ok(ServerStoreClient { + client: connect_resolved_api_client(connection).await?, + }) +} + pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result { let bind = start::ensure_server_running(storage_dir) .with_context(|| format!("Failed to start fabro server for {}", storage_dir.display()))?; @@ -155,7 +163,16 @@ pub(crate) fn connect_remote_api_client( tls: Option<&user_config::ClientTlsSettings>, ) -> Result { let http_client = user_config::build_server_client(tls)?; - Ok(fabro_api::Client::new_with_client(api_url, http_client)) + let normalized = normalize_remote_server_target(api_url); + Ok(fabro_api::Client::new_with_client(&normalized, http_client)) +} + +fn normalize_remote_server_target(api_url: &str) -> String { + api_url + .trim_end_matches('/') + .strip_suffix("/api/v1") + .unwrap_or(api_url.trim_end_matches('/')) + .to_string() } pub(crate) async fn connect_unix_socket_api_client(path: &Path) -> Result { diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 2a17aa8ec..0fbe22ebe 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -115,7 +115,10 @@ fn resolve_server_connection( .as_ref() .and_then(|server| server.tls.clone()), )?) - } else if let Some(storage_dir) = args.storage_dir() { + } else if args.storage_dir_is_explicit() { + let storage_dir = args.storage_dir().ok_or_else(|| { + anyhow::anyhow!("--storage-dir flag was present but no value was parsed") + })?; ServerConnection::Local { storage_dir: storage_dir.to_path_buf(), } @@ -207,6 +210,7 @@ mod tests { ServerConnectionArgs { storage_dir: storage_dir.map(PathBuf::from), server: server.map(str::to_string), + storage_dir_explicit: storage_dir.is_some(), } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/create.rs b/lib/crates/fabro-cli/tests/it/cmd/create.rs index ae52a6017..30fe40957 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/create.rs @@ -1,3 +1,4 @@ +use httpmock::MockServer; use insta::assert_snapshot; use serde_json::json; @@ -7,6 +8,14 @@ use crate::support::{fabro_json_snapshot, unique_run_id}; use super::support::{fixture, output_stdout, resolve_run, run_count_for_test_case, run_state}; +fn run_status_response(run_id: &str, status: &str) -> serde_json::Value { + serde_json::json!({ + "id": run_id, + "status": status, + "created_at": "2026-04-05T12:00:00Z" + }) +} + #[test] fn help() { let context = test_context!(); @@ -27,11 +36,12 @@ fn help() { --json Output as JSON [env: FABRO_JSON=] --storage-dir Local storage directory (default: ~/.fabro) [env: FABRO_STORAGE_DIR=[STORAGE_DIR]] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --dry-run Execute with simulated LLM backend - --auto-approve Auto-approve all human gates --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] - --goal Override the workflow goal (exposed as $goal in prompts) + --auto-approve Auto-approve all human gates --quiet Suppress non-essential output [env: FABRO_QUIET=] + --goal Override the workflow goal (exposed as $goal in prompts) --goal-file Read the workflow goal from a file --model Override default LLM model --provider Override default LLM provider @@ -46,6 +56,157 @@ fn help() { "); } +#[test] +fn create_uses_explicit_server_target_and_prints_remote_run_id() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + let mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + + let output = context + .create_cmd() + .args([ + "--server", + &format!("{}/api/v1", server.base_url()), + "--dry-run", + fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + mock.assert(); + assert_eq!(output_stdout(&output).trim(), run_id.as_str()); +} + +#[test] +fn create_uses_configured_server_target_without_server_flag() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + let mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + context.write_home( + ".fabro/user.toml", + format!("[server]\ntarget = \"{}/api/v1\"\n", server.base_url()), + ); + + let output = context + .create_cmd() + .args(["--dry-run", fixture("simple.fabro").to_str().unwrap()]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + mock.assert(); + assert_eq!(output_stdout(&output).trim(), run_id.as_str()); +} + +#[test] +fn create_storage_dir_suppresses_configured_server_target() { + let context = test_context!(); + let server = MockServer::start(); + let mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let local_storage = std::path::PathBuf::from(format!( + "/tmp/fabro-create-{}", + &context.test_case_id()[..8] + )); + context.write_home( + ".fabro/user.toml", + format!("[server]\ntarget = \"{}/api/v1\"\n", server.base_url()), + ); + + let output = context + .create_cmd() + .args([ + "--storage-dir", + local_storage.to_str().unwrap(), + "--dry-run", + fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + mock.assert_calls(0); + assert!(!output_stdout(&output).trim().is_empty()); +} + +#[test] +fn create_cli_server_target_overrides_configured_server_target() { + let context = test_context!(); + let config_server = MockServer::start(); + let config_mock = config_server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let cli_server = MockServer::start(); + let run_id = unique_run_id(); + let cli_mock = cli_server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + context.write_home( + ".fabro/user.toml", + format!( + "[server]\ntarget = \"{}/api/v1\"\n", + config_server.base_url() + ), + ); + + let output = context + .create_cmd() + .args([ + "--server", + &format!("{}/api/v1", cli_server.base_url()), + "--dry-run", + fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + cli_mock.assert(); + config_mock.assert_calls(0); + assert_eq!(output_stdout(&output).trim(), run_id.as_str()); +} + #[test] fn create_persists_directory_workflow_slug_and_cached_graph() { let context = test_context!(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index c19d451b0..f78918964 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -1,5 +1,6 @@ use fabro_test::{fabro_snapshot, test_context}; use fabro_types::StatusReason; +use httpmock::MockServer; use serde_json::Value; use super::support::{ @@ -10,6 +11,95 @@ use crate::support::{example_fixture, fabro_json_snapshot, run_output_filters, u const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +fn run_status_response(run_id: &str, status: &str) -> serde_json::Value { + serde_json::json!({ + "id": run_id, + "status": status, + "created_at": "2026-04-05T12:00:00Z" + }) +} + +fn preflight_response() -> serde_json::Value { + serde_json::json!({ + "ok": true, + "workflow": { + "name": "Simple", + "graph_path": null, + "nodes": 4, + "edges": 3, + "goal": "Run tests and report results", + "diagnostics": [] + }, + "checks": { + "title": "Preflight", + "sections": [] + } + }) +} + +fn remote_run_state_response() -> serde_json::Value { + serde_json::json!({ + "run": null, + "graph_source": null, + "start": null, + "status": null, + "checkpoint": { + "timestamp": "2026-04-05T12:00:01Z", + "current_node": "exit", + "completed_nodes": ["report"], + "node_retries": {}, + "context_values": { + "response.report": "Remote output" + }, + "node_outcomes": {}, + "next_node_id": null, + "git_commit_sha": null, + "loop_failure_signatures": {}, + "restart_failure_signatures": {}, + "node_visits": {} + }, + "checkpoints": [], + "conclusion": { + "timestamp": "2026-04-05T12:00:01Z", + "status": "success", + "duration_ms": 12, + "stages": [], + "total_cost": null, + "total_retries": 0, + "total_input_tokens": 0, + "total_output_tokens": 0, + "total_cache_read_tokens": 0, + "total_cache_write_tokens": 0, + "total_reasoning_tokens": 0, + "has_pricing": false + }, + "retro": null, + "retro_prompt": null, + "retro_response": null, + "sandbox": null, + "final_patch": null, + "pull_request": null, + "nodes": {} + }) +} + +fn run_completed_event(run_id: &str) -> serde_json::Value { + serde_json::json!({ + "seq": 1, + "payload": { + "event": "run.completed", + "id": "evt-run-completed", + "run_id": run_id, + "ts": "2026-04-05T12:00:01Z", + "properties": { + "duration_ms": 12, + "artifact_count": 0, + "status": "success" + } + } + }) +} + #[test] fn help() { let context = test_context!(); @@ -30,11 +120,12 @@ fn help() { --json Output as JSON [env: FABRO_JSON=] --storage-dir Local storage directory (default: ~/.fabro) [env: FABRO_STORAGE_DIR=[STORAGE_DIR]] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --dry-run Execute with simulated LLM backend - --auto-approve Auto-approve all human gates --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] - --goal Override the workflow goal (exposed as $goal in prompts) + --auto-approve Auto-approve all human gates --quiet Suppress non-essential output [env: FABRO_QUIET=] + --goal Override the workflow goal (exposed as $goal in prompts) --goal-file Read the workflow goal from a file --model Override default LLM model --provider Override default LLM provider @@ -49,6 +140,321 @@ fn help() { "); } +#[test] +fn detach_uses_explicit_server_target_and_prints_remote_run_id() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + let create_mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + let start_mock = server.mock(|when, then| { + when.method("POST") + .path(format!("/api/v1/runs/{run_id}/start")); + then.status(200) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "queued").to_string()); + }); + + let output = context + .run_cmd() + .args([ + "--server", + &format!("{}/api/v1", server.base_url()), + "--detach", + "--dry-run", + "--auto-approve", + example_fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + create_mock.assert(); + start_mock.assert(); + assert_eq!(output_stderr(&output), ""); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + run_id.as_str() + ); +} + +#[test] +fn detach_uses_configured_server_target_without_server_flag() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + let create_mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + let start_mock = server.mock(|when, then| { + when.method("POST") + .path(format!("/api/v1/runs/{run_id}/start")); + then.status(200) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "queued").to_string()); + }); + context.write_home( + ".fabro/user.toml", + format!("[server]\ntarget = \"{}/api/v1\"\n", server.base_url()), + ); + + let output = context + .run_cmd() + .args([ + "--detach", + "--dry-run", + "--auto-approve", + example_fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + create_mock.assert(); + start_mock.assert(); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + run_id.as_str() + ); +} + +#[test] +fn detach_storage_dir_suppresses_configured_server_target() { + let context = test_context!(); + let server = MockServer::start(); + let create_mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let start_mock = server.mock(|when, then| { + when.method("POST").path_includes("/api/v1/runs/"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let local_storage = + std::path::PathBuf::from(format!("/tmp/fabro-run-{}", &context.test_case_id()[..8])); + context.write_home( + ".fabro/user.toml", + format!("[server]\ntarget = \"{}/api/v1\"\n", server.base_url()), + ); + + let output = context + .run_cmd() + .args([ + "--storage-dir", + local_storage.to_str().unwrap(), + "--detach", + "--dry-run", + "--auto-approve", + "--no-retro", + example_fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + create_mock.assert_calls(0); + start_mock.assert_calls(0); + assert!(!String::from_utf8_lossy(&output.stdout).trim().is_empty()); +} + +#[test] +fn detach_cli_server_target_overrides_configured_server_target() { + let context = test_context!(); + let config_server = MockServer::start(); + let config_create = config_server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let config_start = config_server.mock(|when, then| { + when.method("POST").path_includes("/api/v1/runs/"); + then.status(500) + .body("configured-server-should-not-be-used"); + }); + let cli_server = MockServer::start(); + let run_id = unique_run_id(); + let cli_create = cli_server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + let cli_start = cli_server.mock(|when, then| { + when.method("POST") + .path(format!("/api/v1/runs/{run_id}/start")); + then.status(200) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "queued").to_string()); + }); + context.write_home( + ".fabro/user.toml", + format!( + "[server]\ntarget = \"{}/api/v1\"\n", + config_server.base_url() + ), + ); + + let output = context + .run_cmd() + .args([ + "--server", + &format!("{}/api/v1", cli_server.base_url()), + "--detach", + "--dry-run", + "--auto-approve", + example_fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + cli_create.assert(); + cli_start.assert(); + config_create.assert_calls(0); + config_start.assert_calls(0); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + run_id.as_str() + ); +} + +#[test] +fn remote_foreground_run_prints_server_backed_summary_without_local_run_dir() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = unique_run_id(); + server.mock(|when, then| { + when.method("POST").path("/api/v1/preflight"); + then.status(200) + .header("Content-Type", "application/json") + .body(preflight_response().to_string()); + }); + server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.status(201) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "submitted").to_string()); + }); + server.mock(|when, then| { + when.method("POST") + .path(format!("/api/v1/runs/{run_id}/start")); + then.status(200) + .header("Content-Type", "application/json") + .body(run_status_response(run_id.as_str(), "queued").to_string()); + }); + server.mock(|when, then| { + when.method("GET") + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param_missing("since_seq"); + then.status(200) + .header("Content-Type", "application/json") + .body( + serde_json::json!({ + "data": [run_completed_event(run_id.as_str())], + "meta": { "has_more": false } + }) + .to_string(), + ); + }); + server.mock(|when, then| { + when.method("GET") + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("since_seq", "2"); + then.status(200) + .header("Content-Type", "application/json") + .body( + serde_json::json!({ + "data": [], + "meta": { "has_more": false } + }) + .to_string(), + ); + }); + server.mock(|when, then| { + when.method("GET") + .path(format!("/api/v1/runs/{run_id}/questions")) + .query_param("page[limit]", "100") + .query_param("page[offset]", "0"); + then.status(200) + .header("Content-Type", "application/json") + .body( + serde_json::json!({ + "data": [], + "meta": { "has_more": false } + }) + .to_string(), + ); + }); + server.mock(|when, then| { + when.method("GET") + .path(format!("/api/v1/runs/{run_id}/state")); + then.status(200) + .header("Content-Type", "application/json") + .body(remote_run_state_response().to_string()); + }); + + let output = context + .run_cmd() + .args([ + "--server", + &format!("{}/api/v1", server.base_url()), + "--dry-run", + "--auto-approve", + example_fixture("simple.fabro").to_str().unwrap(), + ]) + .output() + .expect("command should execute"); + + assert!( + output.status.success(), + "command failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let stderr = output_stderr(&output); + assert!(stderr.contains("=== Run Result ==="), "{stderr}"); + assert!(stderr.contains("Remote output"), "{stderr}"); + assert_eq!( + stderr + .lines() + .filter(|line| line.trim_start().starts_with("Run:")) + .count(), + 1, + "{stderr}" + ); + assert!(!stderr.contains("=== Artifacts ==="), "{stderr}"); +} + #[test] fn dry_run_simple() { let context = test_context!();