diff --git a/lib/crates/fabro-cli/src/commands/upgrade.rs b/lib/crates/fabro-cli/src/commands/upgrade.rs index 694ac7838..0ef5d495e 100644 --- a/lib/crates/fabro-cli/src/commands/upgrade.rs +++ b/lib/crates/fabro-cli/src/commands/upgrade.rs @@ -255,8 +255,15 @@ impl Backend { } async fn select_backend() -> Backend { + select_backend_for_gh_command("gh").await +} + +async fn select_backend_for_gh_command(gh_command: &str) -> Backend { // Check if gh is available - let gh_version = TokioCommand::new("gh").arg("--version").output().await; + let gh_version = TokioCommand::new(gh_command) + .arg("--version") + .output() + .await; let Ok(output) = gh_version else { debug!("gh CLI not found, using HTTP backend"); return Backend::Http(http_client().expect("failed to build HTTP client")); @@ -267,7 +274,7 @@ async fn select_backend() -> Backend { } // Check if gh is authenticated - let auth_status = TokioCommand::new("gh") + let auth_status = TokioCommand::new(gh_command) .args(["auth", "status"]) .output() .await; @@ -888,9 +895,9 @@ mod tests { // -- Backend selection -- #[tokio::test] - async fn select_backend_returns_a_variant() { - // Just ensure it doesn't panic; actual variant depends on environment - let _backend = select_backend().await; + async fn select_backend_falls_back_to_http_when_gh_is_missing() { + let backend = select_backend_for_gh_command("fabro-test-gh-that-should-not-exist").await; + assert!(matches!(backend, Backend::Http(_))); } // -- Release selection -- diff --git a/lib/crates/fabro-cli/src/gh.rs b/lib/crates/fabro-cli/src/gh.rs index cfd7fc275..8f9d149ec 100644 --- a/lib/crates/fabro-cli/src/gh.rs +++ b/lib/crates/fabro-cli/src/gh.rs @@ -15,7 +15,11 @@ impl GhCli { /// Returns `None` if `gh` is not installed or not authenticated /// against github.com. pub(crate) async fn detect() -> Option { - let version = Command::new("gh").arg("--version").output().await; + Self::detect_with_command("gh").await + } + + async fn detect_with_command(command: &str) -> Option { + let version = Command::new(command).arg("--version").output().await; let Ok(output) = version else { debug!("gh CLI not found on PATH"); return None; @@ -25,7 +29,7 @@ impl GhCli { return None; } - let auth = Command::new("gh") + let auth = Command::new(command) .args(["auth", "status", "--hostname", "github.com"]) .output() .await; @@ -96,9 +100,8 @@ mod tests { use super::*; #[tokio::test] - async fn detect_does_not_panic() { - // Validates graceful degradation — in CI where gh may not be installed - // this returns None without panicking. - let _result = GhCli::detect().await; + async fn detect_returns_none_when_gh_is_missing() { + let result = GhCli::detect_with_command("fabro-test-gh-that-should-not-exist").await; + assert!(result.is_none()); } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/archive.rs b/lib/crates/fabro-cli/tests/it/cmd/archive.rs index 2825c043e..0185bcd2d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/archive.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/archive.rs @@ -2,7 +2,7 @@ use fabro_test::{fabro_snapshot, test_context}; use httpmock::MockServer; use serde_json::Value; -use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; +use super::support::{setup_seeded_completed_dry_run, setup_seeded_created_dry_run}; use crate::support::unique_run_id; fn ulid_filter() -> (String, String) { @@ -62,7 +62,7 @@ fn archive_requires_at_least_one_id() { #[test] fn archive_succeeded_run_hides_it_from_default_ps() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(ulid_filter()); @@ -106,7 +106,7 @@ fn archive_succeeded_run_hides_it_from_default_ps() { fn archive_running_run_rejects_with_must_be_terminal_message() { // A `create`d run is in `submitted` — not yet terminal. let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let output = context .command() .args(["archive", &run.run_id]) @@ -123,7 +123,7 @@ fn archive_running_run_rejects_with_must_be_terminal_message() { #[test] fn archive_already_archived_is_idempotent() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let first = context .command() @@ -164,7 +164,7 @@ fn archive_unknown_id_renders_clean_error() { #[test] fn archive_json_output_shape() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() @@ -183,7 +183,7 @@ fn archive_json_output_shape() { #[test] fn archive_mixed_batch_aggregates_errors() { let context = test_context!(); - let good = setup_completed_fast_dry_run(&context); + let good = setup_seeded_completed_dry_run(&context); let bad = unique_run_id(); let output = context diff --git a/lib/crates/fabro-cli/tests/it/cmd/artifact_cp.rs b/lib/crates/fabro-cli/tests/it/cmd/artifact_cp.rs index 9622a74e3..79e0a0a59 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/artifact_cp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/artifact_cp.rs @@ -1,11 +1,11 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::setup_completed_fast_dry_run; +use super::support::setup_seeded_completed_dry_run; #[test] fn artifact_cp_empty_run_reports_no_artifacts() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let dest = context.temp_dir.join("artifact-dest"); let mut cmd = context.command(); cmd.args(["artifact", "cp", &run.run_id, dest.to_str().unwrap()]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/artifact_list.rs b/lib/crates/fabro-cli/tests/it/cmd/artifact_list.rs index f2415fc16..bffb1c560 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/artifact_list.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/artifact_list.rs @@ -1,11 +1,11 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::setup_completed_fast_dry_run; +use super::support::setup_seeded_completed_dry_run; #[test] fn artifact_list_empty_run_reports_no_artifacts() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut cmd = context.command(); cmd.args(["artifact", "list", &run.run_id]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/diff.rs b/lib/crates/fabro-cli/tests/it/cmd/diff.rs index d0fefb277..c7579817c 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/diff.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/diff.rs @@ -1,6 +1,6 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::{git_filters, setup_git_backed_changed_run, setup_git_backed_noop_run}; +use super::support::{git_filters, setup_git_backed_noop_run, setup_seeded_git_backed_changed_run}; #[test] fn help() { @@ -50,7 +50,7 @@ fn diff_completed_run_without_changes_reports_no_patch() { #[test] fn diff_missing_node_diff_reports_helpful_error() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); cmd.args(["diff", &setup.run.run_id, "--node", "missing"]); @@ -66,7 +66,7 @@ fn diff_missing_node_diff_reports_helpful_error() { #[test] fn diff_completed_run_with_changes_prints_patch() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); cmd.args(["diff", &setup.run.run_id]); @@ -89,7 +89,7 @@ fn diff_completed_run_with_changes_prints_patch() { #[test] fn diff_completed_run_reads_store_final_patch_without_disk_file() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let _ = std::fs::remove_file(setup.run.run_dir.join("final.patch")); let mut cmd = context.command(); @@ -114,7 +114,7 @@ fn diff_completed_run_reads_store_final_patch_without_disk_file() { #[test] fn diff_node_outputs_specific_patch() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); cmd.args(["diff", &setup.run.run_id, "--node", "step_one"]); @@ -136,7 +136,7 @@ fn diff_node_outputs_specific_patch() { #[test] fn diff_node_reads_store_patch_without_disk_file() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); cmd.args(["diff", &setup.run.run_id, "--node", "step_one"]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/dump.rs b/lib/crates/fabro-cli/tests/it/cmd/dump.rs index 9224ab842..c78246ea5 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/dump.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/dump.rs @@ -10,7 +10,8 @@ use fabro_test::{fabro_snapshot, test_context}; use insta::assert_snapshot; use super::support::{ - local_dev_token, server_target, setup_completed_dry_run, setup_created_dry_run, + local_dev_token, server_target, setup_completed_dry_run, setup_seeded_completed_dry_run, + setup_seeded_created_dry_run, }; use crate::support::{LightweightCli, unique_run_id}; @@ -46,7 +47,7 @@ fn help() { #[test] fn dump_accepts_server_target_from_separate_home() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let cli = LightweightCli::new(); let output_dir = context.temp_dir.join("remote-export"); let server = server_target(&context.storage_dir); @@ -289,7 +290,7 @@ fn dump_exports_completed_run_snapshot() { #[test] fn dump_succeeds_when_run_log_is_missing() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let output_dir = context.temp_dir.join("export-missing-log"); let mut cmd = context.command(); @@ -315,7 +316,7 @@ fn dump_succeeds_when_run_log_is_missing() { #[test] fn dump_rejects_non_empty_output_dir() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output_dir = context.temp_dir.join("nonempty"); std::fs::create_dir_all(&output_dir).unwrap(); std::fs::write(output_dir.join("file.txt"), "x").unwrap(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/fork.rs b/lib/crates/fabro-cli/tests/it/cmd/fork.rs index f0311d34c..3afba41c7 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/fork.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/fork.rs @@ -2,8 +2,7 @@ use fabro_test::{fabro_snapshot, run_and_format, test_context}; use insta::assert_snapshot; use super::support::{ - git_filters, output_stdout, run_branch_commits_since_base, run_state_by_id, - setup_git_backed_changed_run, + git_filters, output_stdout, run_state_by_id, setup_seeded_git_backed_changed_run, }; #[test] @@ -54,10 +53,9 @@ fn fork_outside_git_repo_errors() { #[test] fn fork_latest_prints_new_run_and_resume_hint() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); - cmd.current_dir(&setup.repo_dir); cmd.args(["fork", &setup.run.run_id]); let (snapshot, output) = run_and_format(&mut cmd, &git_filters(&context)); @@ -76,16 +74,10 @@ fn fork_latest_prints_new_run_and_resume_hint() { #[test] fn fork_from_earlier_checkpoint_uses_expected_sha() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); - let expected_head = - run_branch_commits_since_base(&setup.repo_dir, &setup.run.run_id, &setup.base_sha) - .into_iter() - .next() - .expect("source run should have a first run commit"); + let setup = setup_seeded_git_backed_changed_run(&context); let output = context .command() - .current_dir(&setup.repo_dir) .args(["fork", &setup.run.run_id, "@2", "--json"]) .output() .expect("fork should execute"); @@ -114,7 +106,7 @@ fn fork_from_earlier_checkpoint_uses_expected_sha() { .checkpoint .as_ref() .and_then(|checkpoint| checkpoint.git_commit_sha.as_deref()), - Some(expected_head.as_str()) + Some(setup.step_one_sha.as_str()) ); assert_eq!( @@ -123,7 +115,7 @@ fn fork_from_earlier_checkpoint_uses_expected_sha() { .as_ref() .and_then(|spec| spec.fork_source_ref.as_ref()) .map(|source| source.checkpoint_sha.as_str()), - Some(expected_head.as_str()) + Some(setup.step_one_sha.as_str()) ); assert_eq!( run_snapshot diff --git a/lib/crates/fabro-cli/tests/it/cmd/inspect.rs b/lib/crates/fabro-cli/tests/it/cmd/inspect.rs index 2a8f239cb..071e78f6a 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/inspect.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/inspect.rs @@ -4,8 +4,8 @@ use insta::assert_snapshot; use serde_json::json; use super::support::{ - compact_git_inspect, compact_inspect, run_success, setup_completed_fast_dry_run, - setup_created_fast_dry_run, setup_git_backed_changed_run, + compact_git_inspect, compact_inspect, run_success, setup_seeded_completed_dry_run, + setup_seeded_created_dry_run, setup_seeded_git_backed_changed_run, }; use crate::support::unique_run_id; @@ -122,7 +122,7 @@ fn inspect_resolves_selector_via_server_endpoint() { #[test] fn inspect_created_run_shows_run_spec_without_start_or_conclusion() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let output = run_success(&context, &["inspect", &run.run_id]); assert_snapshot!(serde_json::to_string_pretty(&compact_inspect(&output)).unwrap(), @r#" @@ -160,7 +160,7 @@ fn inspect_created_run_shows_run_spec_without_start_or_conclusion() { #[test] fn inspect_completed_run_shows_run_start_conclusion_checkpoint() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = run_success(&context, &["inspect", &run.run_id]); assert_snapshot!(serde_json::to_string_pretty(&compact_inspect(&output)).unwrap(), @r#" @@ -215,7 +215,7 @@ fn inspect_completed_run_shows_run_start_conclusion_checkpoint() { #[test] fn inspect_json_omits_run_dir() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = run_success(&context, &["inspect", &run.run_id]); let items: serde_json::Value = serde_json::from_slice(&output.stdout).expect("inspect output should parse"); @@ -232,7 +232,7 @@ fn inspect_json_omits_run_dir() { #[test] fn inspect_completed_run_reads_store_without_disk_metadata_files() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = run_success(&context, &["inspect", &run.run_id]); assert_snapshot!(serde_json::to_string_pretty(&compact_inspect(&output)).unwrap(), @r#" @@ -287,7 +287,7 @@ fn inspect_completed_run_reads_store_without_disk_metadata_files() { #[test] fn inspect_git_backed_run_exposes_checkpoint_and_sandbox_state() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let output = run_success(&context, &["inspect", &setup.run.run_id]); assert_snapshot!( diff --git a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs index 31eed3c6e..245a3d151 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs @@ -1,7 +1,7 @@ use fabro_test::test_context; use serde_json::Value; -use super::support::{fixture, output_stderr, output_stdout, setup_completed_fast_dry_run}; +use super::support::{fixture, output_stderr, output_stdout, setup_seeded_completed_dry_run}; #[test] fn completion_rejects_json() { @@ -115,7 +115,7 @@ fn completion_succeeds_with_json_output_format_from_home_config() { #[test] fn ps_supports_global_flag_and_env_var() { let context = test_context!(); - setup_completed_fast_dry_run(&context); + setup_seeded_completed_dry_run(&context); let test_case_label = context.test_case_label(); let global_output = context @@ -174,7 +174,7 @@ fn ps_supports_global_flag_and_env_var() { #[test] fn logs_json_wins_over_pretty() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/logs.rs b/lib/crates/fabro-cli/tests/it/cmd/logs.rs index 2ded011d2..6b08a28a4 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/logs.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/logs.rs @@ -1,7 +1,7 @@ use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; -use super::support::{setup_completed_dry_run, setup_detached_dry_run}; +use super::support::{setup_detached_dry_run, setup_seeded_completed_dry_run}; fn parse_ndjson(stdout: &[u8]) -> Vec { String::from_utf8(stdout.to_vec()) @@ -78,7 +78,7 @@ fn help() { #[test] fn logs_completed_run_outputs_raw_ndjson() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut cmd = context.command(); cmd.args(["logs", &run.run_id]); let output = cmd.output().expect("command should execute"); @@ -104,7 +104,7 @@ fn logs_completed_run_outputs_raw_ndjson() { #[test] fn logs_completed_run_reads_store_without_progress_jsonl() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( @@ -140,7 +140,7 @@ fn logs_completed_run_reads_store_without_progress_jsonl() { #[test] fn logs_tail_limits_output() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z".to_string(), @@ -174,7 +174,7 @@ fn logs_tail_limits_output() { #[test] fn logs_pretty_formats_small_run() { let context = test_context!(); - let run = setup_completed_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push((r"\b\d{2}:\d{2}:\d{2}\b".to_string(), "[CLOCK]".to_string())); filters.push(( diff --git a/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs b/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs index 3bc6511dd..55984b851 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/pr_create.rs @@ -6,7 +6,7 @@ use fabro_test::{fabro_snapshot, test_context}; use httpmock::MockServer; -use super::support::{mock_resolved_run, setup_completed_fast_dry_run}; +use super::support::{mock_resolved_run, setup_seeded_completed_dry_run}; use crate::support::unique_run_id; #[test] @@ -42,7 +42,7 @@ fn help() { #[test] fn pr_create_nongit_run_reports_missing_repo_origin() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut cmd = context.command(); cmd.args(["pr", "create", &run.run_id]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs index 4ae50d7b8..f1e475fc8 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs @@ -8,7 +8,7 @@ use fabro_types::run_event::PullRequestCreatedProps; use fabro_types::{EventBody, RunEvent, RunId}; use httpmock::MockServer; -use super::support::{mock_resolved_run, server_endpoint, setup_completed_fast_dry_run}; +use super::support::{mock_resolved_run, server_endpoint, setup_seeded_completed_dry_run}; use crate::support::unique_run_id; #[test] @@ -42,7 +42,7 @@ fn help() { #[test] fn pr_view_missing_pull_request_json_errors() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut cmd = context.command(); cmd.args(["pr", "view", &run.run_id]); @@ -58,7 +58,7 @@ fn pr_view_missing_pull_request_json_errors() { #[test] fn pr_view_reads_pull_request_from_store_without_pull_request_json() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let run_id: RunId = run.run_id.parse().unwrap(); let runtime = tokio::runtime::Runtime::new().unwrap(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index e7bd850db..d9c4b291a 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -4,7 +4,9 @@ use fabro_util::dev_token; use httpmock::MockServer; use serde_json::Value; -use super::support::{local_dev_token, setup_completed_fast_dry_run, setup_created_fast_dry_run}; +use super::support::{ + local_dev_token, setup_seeded_completed_dry_run, setup_seeded_created_dry_run, +}; use crate::support::{fatal_error_line, unique_run_id}; const TEST_DEV_TOKEN: &str = @@ -165,7 +167,7 @@ fn ps_explicit_local_tcp_server_target_accepts_explicit_dev_token() { #[test] fn ps_default_excludes_non_running_runs() { let context = test_context!(); - setup_completed_fast_dry_run(&context); + setup_seeded_completed_dry_run(&context); let mut cmd = context.ps(); cmd.args(["--label", &context.test_case_label()]); @@ -181,8 +183,8 @@ fn ps_default_excludes_non_running_runs() { #[test] fn ps_all_json_lists_created_and_completed_runs() { let context = test_context!(); - setup_completed_fast_dry_run(&context); - setup_created_fast_dry_run(&context); + setup_seeded_completed_dry_run(&context); + setup_seeded_created_dry_run(&context); let output = context .ps() .args(["-a", "--json", "--label", &context.test_case_label()]) @@ -219,11 +221,11 @@ fn ps_all_json_lists_created_and_completed_runs() { } #[test] -fn setup_completed_fast_dry_run_preserves_handle_when_another_run_exists() { +fn setup_seeded_run_helpers_preserve_handle_when_another_run_exists() { let context = test_context!(); - let created = setup_created_fast_dry_run(&context); - let completed = setup_completed_fast_dry_run(&context); + let created = setup_seeded_created_dry_run(&context); + let completed = setup_seeded_completed_dry_run(&context); assert_ne!(created.run_id, completed.run_id); assert_ne!(created.run_dir, completed.run_dir); @@ -234,8 +236,8 @@ fn setup_completed_fast_dry_run_preserves_handle_when_another_run_exists() { #[test] fn ps_quiet_outputs_run_ids_only() { let context = test_context!(); - setup_completed_fast_dry_run(&context); - setup_created_fast_dry_run(&context); + setup_seeded_completed_dry_run(&context); + setup_seeded_created_dry_run(&context); let mut cmd = context.ps(); cmd.args(["-a", "--quiet", "--label", &context.test_case_label()]); diff --git a/lib/crates/fabro-cli/tests/it/cmd/resume.rs b/lib/crates/fabro-cli/tests/it/cmd/resume.rs index 06c10efb3..b8c9bd789 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/resume.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/resume.rs @@ -7,8 +7,6 @@ use fabro_test::{fabro_snapshot, test_context}; use super::support::{git_stdout, output_stderr, run_state_by_id, setup_git_backed_changed_run}; -const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); - #[test] fn help() { let context = test_context!(); @@ -98,42 +96,6 @@ fn resume_rewound_run_succeeds() { assert_ne!(resumed_head.trim(), rewound_head); } -#[test] -fn resume_detached_does_not_create_launcher_record() { - let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); - - let new_run_id = rewind_replacement_run_id(&context, &setup); - - let mut resume_cmd = context.command(); - resume_cmd.current_dir(&setup.repo_dir); - resume_cmd.env("OPENAI_API_KEY", "test"); - resume_cmd.args(["resume", "--detach", &new_run_id]); - let resume_output = resume_cmd.output().expect("resume should execute"); - assert!( - resume_output.status.success(), - "resume should succeed\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&resume_output.stdout), - output_stderr(&resume_output) - ); - - assert!( - !context - .storage_dir - .join("launchers") - .join(format!("{new_run_id}.json")) - .exists(), - "server-owned resume should not create a launcher record" - ); - - context - .command() - .args(["wait", &new_run_id]) - .timeout(SHARED_DAEMON_TIMEOUT) - .assert() - .success(); -} - fn rewind_replacement_run_id( context: &fabro_test::TestContext, setup: &super::support::GitRunSetup, diff --git a/lib/crates/fabro-cli/tests/it/cmd/rewind.rs b/lib/crates/fabro-cli/tests/it/cmd/rewind.rs index 6de76ec2f..a9454832f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/rewind.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/rewind.rs @@ -2,8 +2,8 @@ use fabro_test::{fabro_snapshot, run_and_format, test_context}; use insta::assert_snapshot; use super::support::{ - git_filters, output_stderr as support_stderr, run_branch_commits_since_base, run_events, - run_state, run_state_by_id, setup_git_backed_changed_run, + git_filters, output_stderr as support_stderr, run_events, run_state, run_state_by_id, + setup_seeded_git_backed_changed_run, }; #[test] @@ -54,9 +54,8 @@ fn rewind_outside_git_repo_errors() { #[test] fn rewind_list_prints_timeline_for_completed_git_run() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); - cmd.current_dir(&setup.repo_dir); cmd.args(["rewind", &setup.run.run_id, "--list"]); fabro_snapshot!(git_filters(&context), cmd, @" @@ -74,15 +73,9 @@ fn rewind_list_prints_timeline_for_completed_git_run() { #[test] fn rewind_target_updates_metadata_and_resume_hint() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); - let expected_run_head = - run_branch_commits_since_base(&setup.repo_dir, &setup.run.run_id, &setup.base_sha) - .into_iter() - .next() - .expect("source run should have a first run commit"); + let setup = setup_seeded_git_backed_changed_run(&context); let mut cmd = context.command(); - cmd.current_dir(&setup.repo_dir); cmd.args(["rewind", &setup.run.run_id, "@2"]); let (snapshot, output) = run_and_format(&mut cmd, &git_filters(&context)); @@ -110,14 +103,14 @@ fn rewind_target_updates_metadata_and_resume_hint() { replacement .checkpoint .and_then(|checkpoint| checkpoint.git_commit_sha), - Some(expected_run_head) + Some(setup.step_one_sha) ); } #[test] fn rewind_archives_source_and_records_superseded_by() { let context = test_context!(); - let setup = setup_git_backed_changed_run(&context); + let setup = setup_seeded_git_backed_changed_run(&context); let before_events = run_events(&setup.run.run_dir); assert!( before_events @@ -127,7 +120,6 @@ fn rewind_archives_source_and_records_superseded_by() { ); let mut cmd = context.command(); - cmd.current_dir(&setup.repo_dir); cmd.args(["rewind", &setup.run.run_id, "@2"]); let output = cmd.output().expect("rewind should execute"); assert!( diff --git a/lib/crates/fabro-cli/tests/it/cmd/rm.rs b/lib/crates/fabro-cli/tests/it/cmd/rm.rs index a1a86577f..bbf231911 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/rm.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/rm.rs @@ -3,7 +3,7 @@ use httpmock::MockServer; use serde_json::Value; use super::support::{ - setup_completed_fast_dry_run, setup_created_fast_dry_run, setup_local_sandbox_run, + setup_local_sandbox_run, setup_seeded_completed_dry_run, setup_seeded_created_dry_run, }; use crate::support::unique_run_id; @@ -39,7 +39,7 @@ fn help() { #[test] fn rm_deletes_completed_run() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -71,7 +71,7 @@ fn rm_deletes_completed_run() { #[test] fn rm_rejects_submitted_run_without_force() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -92,7 +92,7 @@ fn rm_rejects_submitted_run_without_force() { #[test] fn rm_force_deletes_submitted_run() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -295,7 +295,7 @@ fn rm_without_force_uses_resolve_then_surfaces_server_conflict() { #[test] fn rm_partial_failure_reports_which_identifiers_failed() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b[0-9A-HJKMNP-TV-Z]{12}\b".to_string(), @@ -321,7 +321,7 @@ fn rm_partial_failure_reports_which_identifiers_failed() { #[test] fn rm_partial_failure_json_includes_removed_and_errors() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/sandbox_cp.rs b/lib/crates/fabro-cli/tests/it/cmd/sandbox_cp.rs index 122f8da98..e3d45056d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/sandbox_cp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/sandbox_cp.rs @@ -5,7 +5,7 @@ use fabro_test::{fabro_snapshot, test_context}; -use super::support::{read_text, setup_created_dry_run, setup_local_sandbox_run, text_tree}; +use super::support::{read_text, setup_local_sandbox_run, setup_seeded_created_dry_run, text_tree}; #[test] fn help() { @@ -40,7 +40,7 @@ fn help() { #[test] fn sandbox_cp_run_without_sandbox_json_errors_cleanly() { let context = test_context!(); - let run = setup_created_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let dest = context.temp_dir.join("missing.txt"); let mut cmd = context.cp(); cmd.args([&format!("{}:foo.txt", run.run_id), dest.to_str().unwrap()]); 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 d9ecaf8fa..ff8f76813 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -16,7 +16,7 @@ use fabro_config::{Storage, envfile}; use fabro_static::EnvVars; use fabro_test::{ TestContext, apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files, - test_context, wait_for_log_line, wait_for_path, + test_context, wait_for_log_line, }; use fabro_util::dev_token; @@ -596,113 +596,6 @@ fn daemon_start_writes_tracing_to_storage_server_log() { ); } -#[test] -#[expect( - clippy::disallowed_methods, - reason = "This sync integration test starts two real foreground server processes to verify lock ownership protects log truncation." -)] -fn concurrent_foreground_start_does_not_retruncate_storage_server_log() { - let home_dir = tempfile::tempdir_in("/tmp").unwrap(); - let storage_root = isolated_storage_dir(); - let storage_dir = storage_root.path().join("storage"); - let first_socket_path = storage_root.path().join("foreground-first.sock"); - let second_socket_path = storage_root.path().join("foreground-second.sock"); - let config_dir = tempfile::tempdir_in("/tmp").unwrap(); - let config_path = config_dir.path().join("settings.toml"); - write_dev_token_server_settings(&config_path, ""); - provision_dev_token_auth(home_dir.path(), &storage_dir); - - let storage_log_path = storage_dir.join("logs").join("server.log"); - std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap(); - std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap(); - - let mut first = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - apply_test_isolation(&mut first, home_dir.path()); - first - .args(["server", "start", "--foreground"]) - .arg("--storage-dir") - .arg(&storage_dir) - .arg("--bind") - .arg(&first_socket_path) - .arg("--config") - .arg(&config_path) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()); - - let first_child = first.spawn().expect("first foreground start should spawn"); - wait_for_path(&storage_dir.join("server.json")); - wait_for_log_line(&storage_log_path, "API server started"); - - let marker = "marker-after-first-start\n"; - { - use std::io::Write as _; - - let mut file = std::fs::OpenOptions::new() - .append(true) - .open(&storage_log_path) - .unwrap(); - file.write_all(marker.as_bytes()).unwrap(); - } - - let second_output = { - let mut second = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - apply_test_isolation(&mut second, home_dir.path()); - second - .args(["server", "start", "--foreground"]) - .arg("--storage-dir") - .arg(&storage_dir) - .arg("--bind") - .arg(&second_socket_path) - .arg("--config") - .arg(&config_path) - .output() - .expect("second foreground start should run") - }; - - assert!( - !second_output.status.success(), - "second foreground start should fail:\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&second_output.stdout), - String::from_utf8_lossy(&second_output.stderr) - ); - let second_stderr = String::from_utf8_lossy(&second_output.stderr); - assert!( - second_stderr.contains("timed out waiting for server lock"), - "expected lock timeout failure, got:\n{second_stderr}" - ); - - let storage_log = std::fs::read_to_string(&storage_log_path).unwrap_or_default(); - assert!( - storage_log.contains(marker.trim_end()), - "expected second start to avoid retruncating the log, got:\n{storage_log}", - ); - assert!( - !storage_log.contains("stale pre-start log entry"), - "expected the first start to truncate stale log contents, got:\n{storage_log}", - ); - - let stop_output = { - let mut stop = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - apply_test_isolation(&mut stop, home_dir.path()); - stop.args(["server", "stop", "--timeout", "0"]) - .arg("--storage-dir") - .arg(&storage_dir) - .output() - .expect("server stop should run") - }; - assert!( - stop_output.status.success(), - "server stop should succeed:\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&stop_output.stdout), - String::from_utf8_lossy(&stop_output.stderr) - ); - - let _ = first_child - .wait_with_output() - .expect("first child should exit"); -} - #[test] #[expect( clippy::disallowed_methods, diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index b7d499d12..66a2cbf7d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -11,6 +11,7 @@ use std::path::{Path, PathBuf}; use std::process::Output; +use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{Duration, Instant}; use fabro_config::bind::Bind; @@ -27,6 +28,7 @@ use crate::support::unique_run_id; const LOCAL_COMMAND_TIMEOUT: Duration = Duration::from_secs(30); const CI_COMMAND_TIMEOUT: Duration = Duration::from_secs(90); +static NEXT_SEEDED_EVENT_ID: AtomicU64 = AtomicU64::new(1); pub(crate) use fabro_store::RunProjection; @@ -45,7 +47,11 @@ pub(crate) struct RunSetup { pub(crate) struct GitRunSetup { pub(crate) run: RunSetup, pub(crate) repo_dir: PathBuf, - pub(crate) base_sha: String, +} + +pub(crate) struct SeededGitRunSetup { + pub(crate) run: RunSetup, + pub(crate) step_one_sha: String, } pub(crate) struct ProjectFixture { @@ -68,6 +74,12 @@ enum GitWorkflowKind { Noop, } +#[derive(Clone, Copy)] +enum SeededRunState { + Submitted, + Completed, +} + fn command_timeout() -> Duration { if std::env::var_os("CI").is_some() { CI_COMMAND_TIMEOUT @@ -174,6 +186,14 @@ pub(crate) fn setup_completed_fast_dry_run(context: &TestContext) -> RunSetup { run_completed_dry_run(context, &workflow) } +pub(crate) fn setup_seeded_completed_dry_run(context: &TestContext) -> RunSetup { + block_on(seed_dry_run(context, SeededRunState::Completed)) +} + +pub(crate) fn setup_seeded_created_dry_run(context: &TestContext) -> RunSetup { + block_on(seed_dry_run(context, SeededRunState::Submitted)) +} + fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { let run_id = unique_run_id(); let mut cmd = context.run_cmd(); @@ -209,47 +229,6 @@ fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { run_setup } -pub(crate) fn setup_created_dry_run(context: &TestContext) -> RunSetup { - let workflow = context.install_fixture("simple.fabro"); - run_created_dry_run(context, &workflow) -} - -pub(crate) fn setup_created_fast_dry_run(context: &TestContext) -> RunSetup { - let workflow = fast_simple_workflow(context); - run_created_dry_run(context, &workflow) -} - -fn run_created_dry_run(context: &TestContext, workflow: &Path) -> RunSetup { - let run_id = unique_run_id(); - let mut cmd = context.create_cmd(); - cmd.current_dir(&context.temp_dir); - cmd.timeout(command_timeout()); - cmd.args([ - "--run-id", - run_id.as_str(), - "--dry-run", - "--auto-approve", - "--no-retro", - "--sandbox", - "local", - ]); - cmd.arg(workflow); - let output = cmd.output().expect("command should execute"); - if !output.status.success() { - panic!( - "command failed: fabro create --dry-run --auto-approve --no-retro --sandbox local {}\nstdout:\n{}\nstderr:\n{}", - workflow.display(), - stdout(&output), - stderr(&output) - ); - } - assert_eq!(stdout(&output).trim(), run_id); - RunSetup { - run_dir: context.find_run_dir(&run_id), - run_id, - } -} - fn fast_simple_workflow(context: &TestContext) -> PathBuf { let workflow = context.temp_dir.join("simple.fabro"); if !workflow.exists() { @@ -324,6 +303,10 @@ pub(crate) fn setup_git_backed_noop_run(context: &TestContext) -> GitRunSetup { setup_git_backed_run(context, GitWorkflowKind::Noop) } +pub(crate) fn setup_seeded_git_backed_changed_run(context: &TestContext) -> SeededGitRunSetup { + block_on(seed_git_backed_changed_run(context)) +} + pub(crate) fn setup_project_fixture(context: &TestContext) -> ProjectFixture { let project_dir = context.temp_dir.join("project"); let fabro_root = project_dir.join(".fabro"); @@ -798,25 +781,641 @@ pub(crate) fn wait_for_event_names(run_dir: &Path, expected: &[&str]) { } } -pub(crate) fn git_stdout(repo_dir: &Path, args: &[&str]) -> String { - stdout(&git_success(repo_dir, args)) +async fn seed_dry_run(context: &TestContext, state: SeededRunState) -> RunSetup { + let run = create_seeded_run( + context, + "simple.fabro", + fast_simple_workflow_source(), + serde_json::json!({ + "dry_run": true, + "auto_approve": true, + "no_retro": true, + "sandbox": "local", + "label": test_labels(context), + }), + None, + ) + .await; + + if matches!(state, SeededRunState::Completed) { + let (client, base_url) = server_endpoint(&context.storage_dir) + .expect("test server endpoint should be available for seeded run events"); + append_seeded_simple_completion_events(&client, &base_url, &run, context).await; + } + + run } -pub(crate) fn run_branch_commits_since_base( - repo_dir: &Path, - run_id: &str, +async fn seed_git_backed_changed_run(context: &TestContext) -> SeededGitRunSetup { + let base_sha = "1111111111111111111111111111111111111111"; + let step_one_sha = "2222222222222222222222222222222222222222"; + let step_two_sha = "3333333333333333333333333333333333333333"; + let run = create_seeded_run( + context, + "flow.fabro", + changed_git_workflow_source(), + serde_json::json!({ + "provider": "openai", + "sandbox": "local", + "no_retro": true, + "label": test_labels(context), + }), + Some(serde_json::json!({ + "origin_url": "https://github.com/fabro-sh/seeded-fixture.git", + "branch": "main", + "sha": base_sha, + "dirty": "clean", + "push_outcome": { + "type": "succeeded", + "remote": "origin", + "branch": "main", + }, + })), + ) + .await; + + let (client, base_url) = server_endpoint(&context.storage_dir) + .expect("test server endpoint should be available for seeded run events"); + append_seeded_git_completion_events( + &client, + &base_url, + &run, + context, + base_sha, + step_one_sha, + step_two_sha, + ) + .await; + + SeededGitRunSetup { + run, + step_one_sha: step_one_sha.to_string(), + } +} + +async fn create_seeded_run( + context: &TestContext, + target_path: &str, + source: &str, + args: serde_json::Value, + git: Option, +) -> RunSetup { + let run_id = unique_run_id(); + let mut manifest = serde_json::json!({ + "version": 1, + "run_id": run_id.as_str(), + "cwd": context.temp_dir.display().to_string(), + "target": { + "identifier": target_path, + "path": target_path, + }, + "args": args, + "workflows": { + (target_path): { + "source": source, + "files": {}, + }, + }, + }); + if let Some(git) = git { + manifest["git"] = git; + } + + let (client, base_url) = server_endpoint(&context.storage_dir) + .expect("test server endpoint should be available for seeded run creation"); + let response = client + .post(format!("{base_url}/api/v1/runs")) + .header("user-agent", "fabro-cli/test") + .json(&manifest) + .send() + .await + .expect("seeded run create request should execute"); + let response = expect_reqwest_status( + response, + fabro_http::StatusCode::CREATED, + "POST /api/v1/runs for seeded fixture", + ) + .await; + let body: serde_json::Value = response + .json() + .await + .expect("seeded run create response should parse"); + assert_eq!( + body["id"].as_str(), + Some(run_id.as_str()), + "seeded run should use requested run id" + ); + + RunSetup { + run_dir: context.find_run_dir(&run_id), + run_id, + } +} + +async fn append_seeded_simple_completion_events( + client: &fabro_http::HttpClient, + base_url: &str, + run: &RunSetup, + context: &TestContext, +) { + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.ready", + serde_json::json!({ + "provider": "local", + "duration_ms": 1, + "name": null, + "cpu": null, + "memory": null, + "url": null, + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.initialized", + serde_json::json!({ + "working_directory": context.temp_dir.display().to_string(), + "provider": "local", + "identifier": null, + "repo_cloned": false, + "clone_origin_url": null, + "clone_branch": null, + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.started", + serde_json::json!({ + "name": "Simple", + "base_branch": null, + "base_sha": null, + "run_branch": null, + "worktree_dir": null, + "goal": "Run tests and report results", + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.starting", + serde_json::json!({}), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.running", + serde_json::json!({}), + ) + .await; + + append_seeded_stage(client, base_url, &run.run_id, "start", "Start", 0, None).await; + append_seeded_edge(client, base_url, &run.run_id, "start", "run_tests").await; + append_seeded_stage( + client, + base_url, + &run.run_id, + "run_tests", + "Run Tests", + 1, + Some("Dry run: would execute `true`."), + ) + .await; + append_seeded_edge(client, base_url, &run.run_id, "run_tests", "report").await; + append_seeded_stage( + client, + base_url, + &run.run_id, + "report", + "Report", + 2, + Some("Dry run: would execute `true`."), + ) + .await; + append_seeded_edge(client, base_url, &run.run_id, "report", "exit").await; + append_seeded_stage(client, base_url, &run.run_id, "exit", "Exit", 3, None).await; + append_run_event( + client, + base_url, + &run.run_id, + Some("report"), + "checkpoint.completed", + checkpoint_properties( + "success", + "report", + &["start", "run_tests", "report"], + Some("exit"), + None, + None, + ), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.completed", + serde_json::json!({ + "duration_ms": 123, + "artifact_count": 0, + "status": "success", + "reason": "completed", + "total_usd_micros": null, + "final_git_commit_sha": null, + "final_patch": null, + "billing": null, + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.cleanup.started", + serde_json::json!({ + "provider": "local", + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.cleanup.completed", + serde_json::json!({ + "provider": "local", + "duration_ms": 1, + }), + ) + .await; +} + +async fn append_seeded_git_completion_events( + client: &fabro_http::HttpClient, + base_url: &str, + run: &RunSetup, + context: &TestContext, base_sha: &str, -) -> Vec { - git_stdout(repo_dir, &[ - "rev-list", - "--reverse", - &format!("{base_sha}..fabro/run/{run_id}"), - ]) - .lines() - .map(str::trim) - .filter(|line| !line.is_empty()) - .map(ToOwned::to_owned) - .collect() + step_one_sha: &str, + step_two_sha: &str, +) { + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.ready", + serde_json::json!({ + "provider": "local", + "duration_ms": 1, + "name": null, + "cpu": null, + "memory": null, + "url": null, + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "sandbox.initialized", + serde_json::json!({ + "working_directory": context.temp_dir.display().to_string(), + "provider": "local", + "identifier": null, + "repo_cloned": false, + "clone_origin_url": null, + "clone_branch": null, + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.started", + serde_json::json!({ + "name": "Flow", + "base_branch": "main", + "base_sha": base_sha, + "run_branch": format!("fabro/run/{}", run.run_id), + "worktree_dir": context.temp_dir.display().to_string(), + "goal": "Edit a tracked file", + }), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.starting", + serde_json::json!({}), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.running", + serde_json::json!({}), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + Some("start"), + "checkpoint.completed", + checkpoint_properties("success", "start", &["start"], Some("step_one"), None, None), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + Some("step_one"), + "checkpoint.completed", + checkpoint_properties( + "success", + "step_one", + &["start", "step_one"], + Some("step_two"), + Some(step_one_sha), + Some(step_one_patch()), + ), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + Some("step_two"), + "checkpoint.completed", + checkpoint_properties( + "success", + "step_two", + &["start", "step_one", "step_two"], + Some("exit"), + Some(step_two_sha), + Some(step_two_patch()), + ), + ) + .await; + append_run_event( + client, + base_url, + &run.run_id, + None, + "run.completed", + serde_json::json!({ + "duration_ms": 456, + "artifact_count": 0, + "status": "success", + "reason": "completed", + "total_usd_micros": null, + "final_git_commit_sha": step_two_sha, + "final_patch": final_story_patch(), + "billing": null, + }), + ) + .await; +} + +async fn append_seeded_stage( + client: &fabro_http::HttpClient, + base_url: &str, + run_id: &str, + node_id: &str, + name: &str, + index: usize, + response: Option<&str>, +) { + append_run_event( + client, + base_url, + run_id, + Some(node_id), + "stage.started", + serde_json::json!({ + "index": index, + "handler_type": "noop", + "attempt": 1, + "max_attempts": 1, + }), + ) + .await; + append_run_event( + client, + base_url, + run_id, + Some(node_id), + "stage.completed", + stage_completed_properties(index, response), + ) + .await; + + let _ = name; +} + +async fn append_seeded_edge( + client: &fabro_http::HttpClient, + base_url: &str, + run_id: &str, + from_node: &str, + to_node: &str, +) { + append_run_event( + client, + base_url, + run_id, + Some(from_node), + "edge.selected", + serde_json::json!({ + "from_node": from_node, + "to_node": to_node, + "label": null, + "condition": null, + "reason": "unconditional", + "preferred_label": null, + "suggested_next_ids": [], + "stage_status": "success", + "is_jump": false, + }), + ) + .await; +} + +async fn append_run_event( + client: &fabro_http::HttpClient, + base_url: &str, + run_id: &str, + node_id: Option<&str>, + event_name: &str, + properties: serde_json::Value, +) { + let event_id = NEXT_SEEDED_EVENT_ID.fetch_add(1, Ordering::Relaxed); + let mut event = serde_json::json!({ + "id": format!("00000000-0000-0000-0000-{event_id:012x}"), + "ts": chrono::Utc::now().to_rfc3339(), + "run_id": run_id, + "event": event_name, + "properties": properties, + "actor": { + "kind": "system", + "id": "worker", + "display": "system:worker", + }, + }); + if let Some(node_id) = node_id { + event["node_id"] = serde_json::Value::String(node_id.to_string()); + event["node_label"] = serde_json::Value::String(node_label(node_id).to_string()); + } + + let response = client + .post(format!("{base_url}/api/v1/runs/{run_id}/events")) + .json(&event) + .send() + .await + .unwrap_or_else(|err| panic!("append seeded event {event_name} should execute: {err}")); + expect_reqwest_status( + response, + fabro_http::StatusCode::OK, + format!("POST /api/v1/runs/{run_id}/events ({event_name})"), + ) + .await; +} + +fn test_labels(context: &TestContext) -> Vec { + vec![context.test_run_label(), context.test_case_label()] +} + +fn stage_completed_properties(index: usize, response: Option<&str>) -> serde_json::Value { + serde_json::json!({ + "index": index, + "duration_ms": 1, + "status": "success", + "preferred_label": null, + "suggested_next_ids": [], + "billing": null, + "failure": null, + "notes": null, + "files_touched": [], + "context_updates": null, + "jump_to_node": null, + "context_values": null, + "node_visits": null, + "loop_failure_signatures": null, + "restart_failure_signatures": null, + "response": response, + "attempt": 1, + "max_attempts": 1, + }) +} + +fn checkpoint_properties( + status: &str, + current_node: &str, + completed_nodes: &[&str], + next_node_id: Option<&str>, + git_commit_sha: Option<&str>, + diff: Option<&str>, +) -> serde_json::Value { + serde_json::json!({ + "status": status, + "current_node": current_node, + "completed_nodes": completed_nodes, + "node_retries": {}, + "context_values": {}, + "node_outcomes": {}, + "next_node_id": next_node_id, + "git_commit_sha": git_commit_sha, + "loop_failure_signatures": {}, + "restart_failure_signatures": {}, + "node_visits": { + (current_node): 1, + }, + "diff": diff, + }) +} + +fn node_label(node_id: &str) -> &str { + match node_id { + "start" => "Start", + "run_tests" => "Run Tests", + "report" => "Report", + "exit" => "Exit", + "step_one" => "step_one", + "step_two" => "step_two", + other => other, + } +} + +fn fast_simple_workflow_source() -> &'static str { + r#"digraph Simple { + graph [goal="Run tests and report results"] + rankdir=LR + + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + + run_tests [shape=parallelogram, label="Run Tests", script="true"] + report [shape=parallelogram, label="Report", script="true"] + + start -> run_tests -> report -> exit +} +"# +} + +fn changed_git_workflow_source() -> &'static str { + r#"digraph Flow { + graph [goal="Edit a tracked file"]; + start [shape=Mdiamond]; + exit [shape=Msquare]; + step_one [shape=parallelogram, script="printf 'line 1\nline 2\n' > story.txt"]; + step_two [shape=parallelogram, script="printf 'line 1\nline 2\nline 3\n' > story.txt"]; + start -> step_one -> step_two -> exit; +} +"# +} + +fn step_one_patch() -> &'static str { + "diff --git a/story.txt b/story.txt\nindex 1111111..2222222 100644\n--- a/story.txt\n+++ b/story.txt\n@@ -1 +1,2 @@\n line 1\n+line 2\n" +} + +fn step_two_patch() -> &'static str { + "diff --git a/story.txt b/story.txt\nindex 2222222..3333333 100644\n--- a/story.txt\n+++ b/story.txt\n@@ -1,2 +1,3 @@\n line 1\n line 2\n+line 3\n" +} + +fn final_story_patch() -> &'static str { + "diff --git a/story.txt b/story.txt\nindex 1111111..3333333 100644\n--- a/story.txt\n+++ b/story.txt\n@@ -1 +1,3 @@\n line 1\n+line 2\n+line 3\n" +} + +pub(crate) fn git_stdout(repo_dir: &Path, args: &[&str]) -> String { + stdout(&git_success(repo_dir, args)) } pub(crate) fn text_tree(root: &Path) -> Vec { @@ -1100,11 +1699,7 @@ fn setup_git_backed_run(context: &TestContext, workflow: GitWorkflowKind) -> Git } } - GitRunSetup { - run, - repo_dir, - base_sha, - } + GitRunSetup { run, repo_dir } } fn git_success(repo_dir: &Path, args: &[&str]) -> Output { diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_df.rs b/lib/crates/fabro-cli/tests/it/cmd/system_df.rs index f73032e3c..b6d0fc560 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_df.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_df.rs @@ -6,7 +6,7 @@ use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; -use super::support::setup_completed_fast_dry_run; +use super::support::setup_seeded_completed_dry_run; #[test] fn help() { @@ -37,7 +37,7 @@ fn help() { #[test] fn system_df_summarizes_runs_and_logs() { let context = test_context!(); - setup_completed_fast_dry_run(&context); + setup_seeded_completed_dry_run(&context); std::fs::create_dir_all(context.storage_dir.join("logs")).unwrap(); std::fs::write(context.storage_dir.join("logs/cli.log"), b"log line\n").unwrap(); @@ -66,7 +66,7 @@ fn system_df_summarizes_runs_and_logs() { #[test] fn system_df_verbose_lists_runs_with_reclaimable_marker() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() @@ -101,7 +101,7 @@ fn system_df_verbose_lists_runs_with_reclaimable_marker() { #[test] fn system_df_json_verbose_includes_runs() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs b/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs index 8dc1d3e0a..956559100 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_prune.rs @@ -1,7 +1,7 @@ use fabro_test::{fabro_snapshot, test_context}; use httpmock::MockServer; -use super::support::setup_created_fast_dry_run; +use super::support::setup_seeded_created_dry_run; #[test] fn help() { @@ -158,7 +158,7 @@ fn system_prune_yes_uses_server_target_and_reports_deleted_runs() { #[test] fn system_prune_does_not_delete_active_or_submitted_runs() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let mut cmd = context.command(); cmd.args([ "system", diff --git a/lib/crates/fabro-cli/tests/it/cmd/unarchive.rs b/lib/crates/fabro-cli/tests/it/cmd/unarchive.rs index 25fc4b011..8b5f18c53 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/unarchive.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/unarchive.rs @@ -2,7 +2,7 @@ use fabro_test::{fabro_snapshot, test_context}; use httpmock::MockServer; use serde_json::Value; -use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; +use super::support::{setup_seeded_completed_dry_run, setup_seeded_created_dry_run}; use crate::support::unique_run_id; fn ulid_filter() -> (String, String) { @@ -62,7 +62,7 @@ fn unarchive_requires_at_least_one_id() { #[test] fn unarchive_archived_run_restores_prior_terminal_status() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); // Archive first. let archive = context @@ -102,7 +102,7 @@ fn unarchive_archived_run_restores_prior_terminal_status() { fn unarchive_on_non_archived_terminal_is_idempotent() { // Unarchiving a succeeded (not-archived) run returns success with no event. let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let output = context .command() @@ -119,7 +119,7 @@ fn unarchive_on_non_archived_terminal_is_idempotent() { #[test] fn unarchive_on_active_run_rejects_with_not_archived_message() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let output = context .command() @@ -154,7 +154,7 @@ fn unarchive_unknown_id_renders_clean_error() { #[test] fn unarchive_json_output_shape() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); context .command() .args(["archive", &run.run_id]) @@ -178,13 +178,13 @@ fn unarchive_json_output_shape() { #[test] fn unarchive_mixed_batch_aggregates_errors() { let context = test_context!(); - let archived_run = setup_completed_fast_dry_run(&context); + let archived_run = setup_seeded_completed_dry_run(&context); context .command() .args(["archive", &archived_run.run_id]) .output() .expect("archive should execute"); - let active_run = setup_created_fast_dry_run(&context); + let active_run = setup_seeded_created_dry_run(&context); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/wait.rs b/lib/crates/fabro-cli/tests/it/cmd/wait.rs index b4a96f78d..bd0cccb41 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/wait.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/wait.rs @@ -2,7 +2,7 @@ use fabro_test::{fabro_snapshot, test_context}; use httpmock::MockServer; use serde_json::json; -use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run}; +use super::support::{setup_seeded_completed_dry_run, setup_seeded_created_dry_run}; use crate::support::unique_run_id; fn remote_run_summary(run_id: &str, status: &serde_json::Value) -> serde_json::Value { @@ -56,7 +56,7 @@ fn help() { #[test] fn wait_completed_run_prints_success_summary() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b\d+(\.\d+)?(ms|s)\b".to_string(), @@ -77,7 +77,7 @@ fn wait_completed_run_prints_success_summary() { #[test] fn wait_completed_run_reads_store_without_status_or_conclusion_files() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r"\b\d+(\.\d+)?(ms|s)\b".to_string(), @@ -98,7 +98,7 @@ fn wait_completed_run_reads_store_without_status_or_conclusion_files() { #[test] fn wait_completed_run_json_outputs_status_and_duration() { let context = test_context!(); - let run = setup_completed_fast_dry_run(&context); + let run = setup_seeded_completed_dry_run(&context); let mut filters = context.filters(); filters.push(( r#""duration_ms":\s*\d+"#.to_string(), @@ -123,16 +123,16 @@ fn wait_completed_run_json_outputs_status_and_duration() { #[test] fn wait_submitted_run_times_out() { let context = test_context!(); - let run = setup_created_fast_dry_run(&context); + let run = setup_seeded_created_dry_run(&context); let mut cmd = context.command(); - cmd.args(["wait", "--timeout", "1", "--interval", "10", &run.run_id]); + cmd.args(["wait", "--timeout", "0", "--interval", "10", &run.run_id]); fabro_snapshot!(context.filters(), cmd, @" success: false exit_code: 1 ----- stdout ----- ----- stderr ----- - × Timed out after 1s waiting for run '[ULID]' + × Timed out after 0s waiting for run '[ULID]' "); } @@ -178,7 +178,7 @@ fn wait_blocked_run_times_out_without_treating_it_as_terminal() { "--server", &format!("{}/api/v1", server.base_url()), "--timeout", - "1", + "0", "--interval", "10", run_id.as_str(), @@ -189,7 +189,7 @@ fn wait_blocked_run_times_out_without_treating_it_as_terminal() { exit_code: 1 ----- stdout ----- ----- stderr ----- - × Timed out after 1s waiting for run '[ULID]' + × Timed out after 0s waiting for run '[ULID]' "); resolve_run.assert(); assert!( diff --git a/lib/crates/fabro-cli/tests/it/scenario/recovery.rs b/lib/crates/fabro-cli/tests/it/scenario/recovery.rs index 9e1fcbd84..fb6a4a085 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/recovery.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/recovery.rs @@ -6,13 +6,9 @@ use std::collections::BTreeSet; use std::path::Path; -use fabro_checkpoint::git::Store as GitStore; -use fabro_store::RunProjection; -use fabro_test::{TestContext, fabro_snapshot, test_context}; -use fabro_workflow::operations::{RunTimeline, build_timeline}; +use fabro_test::{fabro_snapshot, test_context}; use git2::Repository; -use crate::cmd::support::output_stdout; use crate::support::unique_run_id; fn list_metadata_run_ids(repo_dir: &Path) -> BTreeSet { @@ -28,85 +24,6 @@ fn list_metadata_run_ids(repo_dir: &Path) -> BTreeSet { .collect() } -fn load_metadata_projection(repo_dir: &Path, run_id: &str) -> Result { - let repo = Repository::discover(repo_dir) - .map_err(|err| format!("recovery fixture should be a git repo: {err}"))?; - let store = GitStore::new(repo); - let tip = store - .resolve_ref(&format!("fabro/meta/{run_id}")) - .map_err(|err| format!("metadata branch should resolve: {err}"))? - .ok_or_else(|| "metadata branch tip should exist".to_string())?; - let projection_blob = store - .read_blob_at(tip, "run.json") - .map_err(|err| format!("latest projection blob should load: {err}"))? - .ok_or_else(|| "latest projection blob should exist".to_string())?; - serde_json::from_slice(&projection_blob) - .map_err(|err| format!("latest projection blob should deserialize: {err}")) -} - -fn timeline_run_shas(repo_dir: &Path, run_id: &str) -> Vec> { - build_timeline_when_ready(repo_dir, run_id) - .entries - .into_iter() - .map(|entry| entry.run_commit_sha) - .collect() -} - -#[expect( - clippy::disallowed_methods, - reason = "This sync git integration helper polls until metadata commits become readable without requiring Tokio." -)] -fn build_timeline_when_ready(repo_dir: &Path, run_id: &str) -> RunTimeline { - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); - loop { - let timeline = load_metadata_projection(repo_dir, run_id) - .map_err(anyhow::Error::msg) - .and_then(|projection| build_timeline(&projection)); - match timeline { - Ok(timeline) => return timeline, - Err(err) => { - assert!( - std::time::Instant::now() < deadline, - "timeline for {run_id} never became readable: {err}" - ); - std::thread::sleep(std::time::Duration::from_millis(50)); - } - } - } -} - -fn fork_run_json(context: &TestContext, repo_dir: &Path, source_run_id: &str) -> String { - let output = context - .command() - .current_dir(repo_dir) - .args(["fork", source_run_id, "--json"]) - .timeout(std::time::Duration::from_secs(15)) - .output() - .expect("fork command should execute"); - assert!( - output.status.success(), - "fork should succeed\nstdout:\n{}\nstderr:\n{}", - output_stdout(&output), - String::from_utf8_lossy(&output.stderr) - ); - serde_json::from_str::(&output_stdout(&output)) - .expect("fork json should parse") - .get("new_run_id") - .and_then(|value| value.as_str()) - .expect("fork json should contain new_run_id") - .to_string() -} - -fn latest_store_checkpoint_sha(context: &TestContext, run_id: &str) -> Option { - let state: RunProjection = super::block_on(super::get_server_json_for_storage( - &context.storage_dir, - &format!("/api/v1/runs/{run_id}/state"), - )); - state - .checkpoint - .and_then(|checkpoint| checkpoint.git_commit_sha) -} - #[expect( clippy::disallowed_methods, reason = "This sync git integration helper retries metadata branch deletion until libgit2 releases its lock." @@ -262,45 +179,3 @@ fn rewind_list_reports_empty_timeline_when_metadata_branch_is_missing() { "server timeline should not rebuild missing metadata" ); } - -#[test] -fn fork_chain_preserves_checkpoint_metadata() { - let context = test_context!(); - context.ensure_home_server_auth_methods(); - let repo_dir = tempfile::tempdir().unwrap(); - let source_run_id = unique_run_id(); - - init_repo_with_workflow(repo_dir.path()); - - context - .command() - .current_dir(repo_dir.path()) - .args([ - "run", - "--dry-run", - "--no-retro", - "--sandbox", - "local", - "--run-id", - source_run_id.as_str(), - "workflow.fabro", - ]) - .assert() - .success(); - - let timeline_shas = timeline_run_shas(repo_dir.path(), &source_run_id); - let build_sha = timeline_shas.last().cloned().flatten(); - assert!(build_sha.is_some()); - - let child_run_id = fork_run_json(&context, repo_dir.path(), &source_run_id); - assert_eq!( - latest_store_checkpoint_sha(&context, &child_run_id), - build_sha - ); - - let grandchild_run_id = fork_run_json(&context, repo_dir.path(), &child_run_id); - assert_eq!( - latest_store_checkpoint_sha(&context, &grandchild_run_id), - build_sha - ); -}