diff --git a/.config/nextest.toml b/.config/nextest.toml index 450bc2254..64e384f63 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -34,6 +34,22 @@ filter = """ """ test-group = 'medium' +[[profile.default.overrides]] +filter = "package(fabro-hooks)" +slow-timeout = { period = "5s", terminate-after = 3 } + +[[profile.default.overrides]] +filter = "package(fabro-agent)" +slow-timeout = { period = "5s", terminate-after = 3 } + +[[profile.default.overrides]] +filter = "package(fabro-llm)" +slow-timeout = { period = "5s", terminate-after = 3 } + +[[profile.default.overrides]] +filter = "package(fabro-server)" +slow-timeout = { period = "5s", terminate-after = 3 } + [profile.e2e] # E2E (ignored) tests: flag SLOW after 10s, hard-kill after 30s slow-timeout = { period = "10s", terminate-after = 3 } diff --git a/Cargo.lock b/Cargo.lock index abc0643d0..99f5f48d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1948,6 +1948,7 @@ dependencies = [ "fabro-macros", "serde", "serde_json", + "ulid", ] [[package]] diff --git a/lib/crates/fabro-cli/src/commands/pr/list.rs b/lib/crates/fabro-cli/src/commands/pr/list.rs index bfb9a56bb..69354682e 100644 --- a/lib/crates/fabro-cli/src/commands/pr/list.rs +++ b/lib/crates/fabro-cli/src/commands/pr/list.rs @@ -49,7 +49,7 @@ async fn list_from( let pr_path = run.path.join("pull_request.json"); if let Ok(content) = std::fs::read_to_string(&pr_path) { if let Ok(record) = serde_json::from_str::(&content) { - entries.push((run.run_id.clone(), record)); + entries.push((run.run_id.to_string(), record)); } } } diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 8187bab17..391c54cd5 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -7,6 +7,7 @@ use std::time::{Duration, Instant}; use anyhow::{Result, bail}; use fabro_config::FabroSettingsExt; +use fabro_types::RunId; use futures::StreamExt; use fabro_interview::{AnswerValue, ConsoleInterviewer}; @@ -33,7 +34,7 @@ const INTERVIEW_UNANSWERED_MESSAGE: &str = /// Returns exit code 0 for success/partial_success, 1 otherwise. pub(crate) async fn attach_run( run_dir: &Path, - run_id: Option<&str>, + run_id: Option<&RunId>, kill_on_detach: bool, styles: &'static Styles, engine_child: Option, @@ -43,7 +44,7 @@ pub(crate) async fn attach_run( run_record .as_ref() .map(|record| record.settings.storage_dir()), - run_id.or_else(|| run_record.as_ref().map(|record| record.run_id.as_str())), + run_id.or_else(|| run_record.as_ref().map(|record| &record.run_id)), ) { match store::open_run_reader(&storage_dir, run_id).await { Ok(Some(run_store)) => match run_store.list_events().await { @@ -65,7 +66,7 @@ pub(crate) async fn attach_run( } Err(err) => { tracing::warn!( - run_id, + run_id = %run_id, error = %err, "Failed to list events from store; falling back to filesystem attach" ); @@ -74,7 +75,7 @@ pub(crate) async fn attach_run( Ok(None) => {} Err(err) => { tracing::warn!( - run_id, + run_id = %run_id, error = %err, "Failed to open store reader; falling back to filesystem attach" ); diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index b36d1835c..a3ceda0b6 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use crate::args::RunArgs; use fabro_config::{ConfigLayer, FabroSettings}; +use fabro_types::RunId; use fabro_util::terminal::Styles; use fabro_workflows::error::FabroError; use fabro_workflows::operations::{CreateRunInput, WorkflowInput, create}; @@ -16,7 +17,7 @@ pub(crate) fn create_run( cli_defaults: ConfigLayer, styles: &Styles, quiet: bool, -) -> anyhow::Result<(String, PathBuf)> { +) -> anyhow::Result<(RunId, PathBuf)> { let workflow_path = args .workflow .as_ref() @@ -28,13 +29,20 @@ pub(crate) fn create_run( .combine(cli_defaults) .resolve()?; + let run_id = args + .run_id + .as_deref() + .map(str::parse::) + .transpose() + .map_err(|err| anyhow::anyhow!("invalid run ID: {err}"))?; + let created = match create(CreateRunInput { workflow: WorkflowInput::Path(workflow_path.clone()), settings, cwd, workflow_slug: None, run_dir: None, - run_id: args.run_id.clone(), + run_id, base_branch: None, host_repo_path: None, }) { diff --git a/lib/crates/fabro-cli/src/commands/run/detached.rs b/lib/crates/fabro-cli/src/commands/run/detached.rs index 902e52ace..66cc4d924 100644 --- a/lib/crates/fabro-cli/src/commands/run/detached.rs +++ b/lib/crates/fabro-cli/src/commands/run/detached.rs @@ -22,7 +22,8 @@ pub(crate) async fn execute(run_dir: PathBuf, launcher_path: PathBuf, resume: bo let run_record = RunRecord::load(&run_dir)?; let on_node: fabro_workflows::OnNodeCallback = Some({ - let short_id = super::short_run_id(&run_record.run_id).to_string(); + let run_id = run_record.run_id.to_string(); + let short_id = super::short_run_id(&run_id).to_string(); fabro_proctitle::set(&format!("fabro: {short_id}")); Arc::new(move |node_id: &str| { fabro_proctitle::set(&format!("fabro: {short_id} {node_id}")); diff --git a/lib/crates/fabro-cli/src/commands/run/fork.rs b/lib/crates/fabro-cli/src/commands/run/fork.rs index 12259c686..fdec1153e 100644 --- a/lib/crates/fabro-cli/src/commands/run/fork.rs +++ b/lib/crates/fabro-cli/src/commands/run/fork.rs @@ -36,20 +36,23 @@ pub(crate) async fn run(args: &ForkArgs, styles: &Styles, globals: &GlobalArgs) let new_run_id = fork( &store, &ForkRunInput { - source_run_id: run_id.clone(), + source_run_id: run_id, target, push: !args.no_push, }, )?; + let run_id_string = run_id.to_string(); + let new_run_id_string = new_run_id.to_string(); + eprintln!( "\nForked run {} -> {}", - &run_id[..8.min(run_id.len())], - &new_run_id[..8.min(new_run_id.len())] + &run_id_string[..8.min(run_id_string.len())], + &new_run_id_string[..8.min(new_run_id_string.len())] ); eprintln!( "To resume: fabro resume {}", - &new_run_id[..8.min(new_run_id.len())] + &new_run_id_string[..8.min(new_run_id_string.len())] ); Ok(()) diff --git a/lib/crates/fabro-cli/src/commands/run/launcher.rs b/lib/crates/fabro-cli/src/commands/run/launcher.rs index f2c4a5567..80831292a 100644 --- a/lib/crates/fabro-cli/src/commands/run/launcher.rs +++ b/lib/crates/fabro-cli/src/commands/run/launcher.rs @@ -3,12 +3,13 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use chrono::{DateTime, Utc}; use fabro_config::FabroSettingsExt; +use fabro_types::RunId; use fabro_workflows::records::{RunRecord, RunRecordExt}; use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct LauncherRecord { - pub run_id: String, + pub run_id: RunId, pub run_dir: PathBuf, pub pid: u32, pub resume: bool, @@ -20,11 +21,11 @@ pub(crate) fn launcher_dir(storage_dir: &Path) -> PathBuf { storage_dir.join("launchers") } -pub(crate) fn launcher_record_path(storage_dir: &Path, run_id: &str) -> PathBuf { +pub(crate) fn launcher_record_path(storage_dir: &Path, run_id: &RunId) -> PathBuf { launcher_dir(storage_dir).join(format!("{run_id}.json")) } -pub(crate) fn launcher_log_path(storage_dir: &Path, run_id: &str) -> PathBuf { +pub(crate) fn launcher_log_path(storage_dir: &Path, run_id: &RunId) -> PathBuf { launcher_dir(storage_dir).join(format!("{run_id}.log")) } @@ -93,7 +94,8 @@ fn launcher_process_matches(record: &LauncherRecord) -> bool { fn command_matches_launcher(record: &LauncherRecord, command: &str) -> bool { let run_dir = record.run_dir.to_string_lossy(); let old_match = command.contains("__detached") && command.contains(run_dir.as_ref()); - let new_match = command.contains(&format!("fabro: {}", super::short_run_id(&record.run_id))); + let run_id = record.run_id.to_string(); + let new_match = command.contains(&format!("fabro: {}", super::short_run_id(&run_id))); old_match || new_match } @@ -108,6 +110,7 @@ mod tests { use chrono::Utc; use fabro_config::FabroSettings; use fabro_graphviz::graph::Graph; + use fabro_types::fixtures; use fabro_workflows::records::RunRecord; #[test] @@ -118,7 +121,7 @@ mod tests { std::fs::create_dir_all(&run_dir).unwrap(); RunRecord { - run_id: "run-test".to_string(), + run_id: fixtures::RUN_1, created_at: Utc::now(), settings: FabroSettings { storage_dir: Some(storage_dir.clone()), @@ -134,11 +137,11 @@ mod tests { .save(&run_dir) .unwrap(); - let launcher_path = launcher_record_path(&storage_dir, "run-test"); + let launcher_path = launcher_record_path(&storage_dir, &fixtures::RUN_1); write_launcher_record( &launcher_path, &LauncherRecord { - run_id: "run-test".to_string(), + run_id: fixtures::RUN_1, run_dir: run_dir.clone(), pid: u32::MAX, resume: false, @@ -157,7 +160,7 @@ mod tests { fn command_matches_launcher_accepts_old_detached_format() { let dir = tempfile::tempdir().unwrap(); let record = LauncherRecord { - run_id: "01JGABCDEF12ZYXW".to_string(), + run_id: fixtures::RUN_2, run_dir: dir.path().join("run"), pid: 42, resume: false, @@ -178,7 +181,7 @@ mod tests { fn command_matches_launcher_accepts_new_title_format() { let dir = tempfile::tempdir().unwrap(); let record = LauncherRecord { - run_id: "01JGABCDEF12ZYXW".to_string(), + run_id: fixtures::RUN_3, run_dir: dir.path().join("run"), pid: 42, resume: false, @@ -188,7 +191,10 @@ mod tests { assert!(command_matches_launcher( &record, - "fabro: 01JGABCDEF12 plan" + &format!( + "fabro: {} plan", + crate::commands::run::short_run_id(&record.run_id.to_string()) + ) )); } } diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index f6ffe554e..fe977a49f 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -69,7 +69,8 @@ pub(crate) fn print_diagnostics_from_error( print_diagnostics(diagnostics, styles); } -pub(crate) fn print_run_summary(run_dir: &Path, run_id: &str, styles: &Styles) { +pub(crate) fn print_run_summary(run_dir: &Path, run_id: impl std::fmt::Display, styles: &Styles) { + let run_id = run_id.to_string(); let conclusion_path = run_dir.join("conclusion.json"); let Ok(conclusion) = Conclusion::load(&conclusion_path) else { return; @@ -85,7 +86,7 @@ pub(crate) fn print_run_summary(run_dir: &Path, run_id: &str, styles: &Styles) { print_run_conclusion( &conclusion, - run_id, + &run_id, run_dir, None, pr_url.as_deref(), @@ -97,12 +98,13 @@ pub(crate) fn print_run_summary(run_dir: &Path, run_id: &str, styles: &Styles) { pub(crate) fn print_run_conclusion( conclusion: &Conclusion, - run_id: &str, + run_id: impl std::fmt::Display, run_dir: &Path, pushed_branch: Option<&str>, pr_url: Option<&str>, styles: &Styles, ) { + let run_id = run_id.to_string(); eprintln!("\n{}", styles.bold.apply_to("=== Run Result ===")); eprintln!("{}", styles.dim.apply_to(format!("Run: {run_id}"))); diff --git a/lib/crates/fabro-cli/src/commands/run/rewind.rs b/lib/crates/fabro-cli/src/commands/run/rewind.rs index 19fbe5af5..981b7a843 100644 --- a/lib/crates/fabro-cli/src/commands/run/rewind.rs +++ b/lib/crates/fabro-cli/src/commands/run/rewind.rs @@ -37,15 +37,17 @@ pub(crate) async fn run(args: &RewindArgs, styles: &Styles, globals: &GlobalArgs rewind( &store, &RewindInput { - run_id: run_id.clone(), + run_id, target, push: !args.no_push, }, )?; + let run_id_string = run_id.to_string(); + eprintln!( "\nTo resume: fabro resume {}", - &run_id[..8.min(run_id.len())] + &run_id_string[..8.min(run_id_string.len())] ); Ok(()) diff --git a/lib/crates/fabro-cli/src/commands/run/wait.rs b/lib/crates/fabro-cli/src/commands/run/wait.rs index fc33f591e..d29d95cb3 100644 --- a/lib/crates/fabro-cli/src/commands/run/wait.rs +++ b/lib/crates/fabro-cli/src/commands/run/wait.rs @@ -2,6 +2,7 @@ use std::io::Write; use anyhow::{Result, bail}; use fabro_config::FabroSettingsExt; +use fabro_types::RunId; use fabro_util::terminal::Styles; use fabro_workflows::records::{Conclusion, ConclusionExt}; use fabro_workflows::run_lookup::{resolve_run_combined, runs_base}; @@ -92,7 +93,7 @@ pub(crate) async fn run(args: &WaitArgs, styles: &Styles, globals: &GlobalArgs) fn build_json_output( status: RunStatus, - run_id: &str, + run_id: &RunId, conclusion: Option<&Conclusion>, ) -> serde_json::Value { let mut value = serde_json::json!({ @@ -110,7 +111,7 @@ fn build_json_output( fn print_human_output( status: RunStatus, - run_id: &str, + run_id: &RunId, conclusion: Option<&Conclusion>, styles: &Styles, ) { @@ -145,6 +146,7 @@ fn print_human_output( #[cfg(test)] mod tests { use super::*; + use fabro_types::fixtures; use fabro_workflows::outcome::StageStatus; use fabro_workflows::records::Conclusion; @@ -154,6 +156,7 @@ mod tests { #[test] fn json_output_succeeded_with_conclusion() { + let run_id = fixtures::RUN_1; let conclusion = Conclusion { timestamp: chrono::Utc::now(), status: StageStatus::Success, @@ -170,8 +173,8 @@ mod tests { total_reasoning_tokens: 0, has_pricing: false, }; - let json = build_json_output(RunStatus::Succeeded, "ABC123", Some(&conclusion)); - assert_eq!(json["run_id"], "ABC123"); + let json = build_json_output(RunStatus::Succeeded, &run_id, Some(&conclusion)); + assert_eq!(json["run_id"], run_id.to_string()); assert_eq!(json["status"], "succeeded"); assert_eq!(json["duration_ms"], 12345); assert!((json["total_cost"].as_f64().unwrap() - 0.42).abs() < f64::EPSILON); @@ -179,8 +182,9 @@ mod tests { #[test] fn json_output_failed_without_conclusion() { - let json = build_json_output(RunStatus::Failed, "DEF456", None); - assert_eq!(json["run_id"], "DEF456"); + let run_id = fixtures::RUN_2; + let json = build_json_output(RunStatus::Failed, &run_id, None); + assert_eq!(json["run_id"], run_id.to_string()); assert_eq!(json["status"], "failed"); assert!(json.get("duration_ms").is_none()); assert!(json.get("total_cost").is_none()); @@ -188,12 +192,13 @@ mod tests { #[test] fn json_output_dead_status() { - let json = build_json_output(RunStatus::Dead, "GHI789", None); + let json = build_json_output(RunStatus::Dead, &fixtures::RUN_3, None); assert_eq!(json["status"], "dead"); } #[test] fn json_output_no_cost_when_none() { + let run_id = fixtures::RUN_4; let conclusion = Conclusion { timestamp: chrono::Utc::now(), status: StageStatus::Fail, @@ -210,7 +215,7 @@ mod tests { total_reasoning_tokens: 0, has_pricing: false, }; - let json = build_json_output(RunStatus::Failed, "JKL012", Some(&conclusion)); + let json = build_json_output(RunStatus::Failed, &run_id, Some(&conclusion)); assert!(json.get("total_cost").is_none()); assert_eq!(json["duration_ms"], 500); } @@ -218,6 +223,7 @@ mod tests { #[test] fn human_output_succeeded() { let styles = no_color_styles(); + let run_id = fixtures::RUN_5; let conclusion = Conclusion { timestamp: chrono::Utc::now(), status: StageStatus::Success, @@ -235,13 +241,13 @@ mod tests { has_pricing: false, }; // Just verify no panic; actual stderr output is hard to capture - print_human_output(RunStatus::Succeeded, "ABC123", Some(&conclusion), &styles); + print_human_output(RunStatus::Succeeded, &run_id, Some(&conclusion), &styles); } #[test] fn human_output_failed_no_conclusion() { let styles = no_color_styles(); - print_human_output(RunStatus::Failed, "DEF456", None, &styles); + print_human_output(RunStatus::Failed, &fixtures::RUN_6, None, &styles); } #[test] diff --git a/lib/crates/fabro-cli/src/commands/runs/inspect.rs b/lib/crates/fabro-cli/src/commands/runs/inspect.rs index 5619ed170..17b508c5b 100644 --- a/lib/crates/fabro-cli/src/commands/runs/inspect.rs +++ b/lib/crates/fabro-cli/src/commands/runs/inspect.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use fabro_config::FabroSettingsExt; use fabro_sandbox::SandboxRecordExt; +use fabro_types::RunId; use fabro_workflows::records::{CheckpointExt, ConclusionExt, RunRecordExt, StartRecordExt}; use serde::Serialize; @@ -43,7 +44,7 @@ pub(crate) async fn run(args: &InspectArgs, globals: &GlobalArgs) -> Result<()> } async fn inspect_run_store( - run_id: &str, + run_id: &RunId, run_dir: &Path, status: RunStatus, run_store: &dyn fabro_store::RunStore, @@ -74,7 +75,7 @@ async fn inspect_run_store( } } -fn inspect_run_dir(run_id: &str, run_dir: &Path, status: RunStatus) -> InspectOutput { +fn inspect_run_dir(run_id: &RunId, run_dir: &Path, status: RunStatus) -> InspectOutput { let run_record = RunRecord::load(run_dir) .ok() .and_then(|v| serde_json::to_value(v).ok()); diff --git a/lib/crates/fabro-cli/src/commands/runs/list.rs b/lib/crates/fabro-cli/src/commands/runs/list.rs index 7893dff51..0fefc7564 100644 --- a/lib/crates/fabro-cli/src/commands/runs/list.rs +++ b/lib/crates/fabro-cli/src/commands/runs/list.rs @@ -95,9 +95,10 @@ pub(crate) async fn list_command( .host_repo_path .as_deref() .map_or_else(|| "-".to_string(), |p| tilde_path(Path::new(p))); + let run_id = run.run_id.to_string(); vec![ - short_run_id(&run.run_id) + short_run_id(&run_id) .cell() .foreground_color(color_if(use_color, Color::Ansi256(8))), run.workflow_name.clone().cell(), diff --git a/lib/crates/fabro-cli/src/commands/runs/rm.rs b/lib/crates/fabro-cli/src/commands/runs/rm.rs index 2fc6065f4..875b27650 100644 --- a/lib/crates/fabro-cli/src/commands/runs/rm.rs +++ b/lib/crates/fabro-cli/src/commands/runs/rm.rs @@ -37,9 +37,10 @@ async fn remove_from(args: &RunsRemoveArgs, store: &dyn Store, base: &Path) -> R }; if run.status.is_active() && !args.force { + let run_id = run.run_id.to_string(); eprintln!( "cannot remove active run {} (status: {}, use -f to force)", - short_run_id(&run.run_id), + short_run_id(&run_id), run.status ); had_errors = true; @@ -82,7 +83,8 @@ async fn remove_from(args: &RunsRemoveArgs, store: &dyn Store, base: &Path) -> R .delete_run(&run.run_id) .await .with_context(|| format!("failed to delete store state for {}", run.run_id))?; - eprintln!("{}", short_run_id(&run.run_id)); + let run_id = run.run_id.to_string(); + eprintln!("{}", short_run_id(&run_id)); } if had_errors { diff --git a/lib/crates/fabro-cli/src/commands/store/dump.rs b/lib/crates/fabro-cli/src/commands/store/dump.rs index 5f4aa6170..c84b7b7ac 100644 --- a/lib/crates/fabro-cli/src/commands/store/dump.rs +++ b/lib/crates/fabro-cli/src/commands/store/dump.rs @@ -350,8 +350,8 @@ mod tests { use fabro_store::{EventEnvelope, EventPayload, InMemoryStore, Store as _}; use fabro_types::{ AggregateStats, AttrValue, Checkpoint, Conclusion, FabroSettings, Graph, NodeStatusRecord, - Retro, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, StageStatus, StartRecord, - StatusReason, + Retro, RunId, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, StageStatus, + StartRecord, StatusReason, fixtures, }; fn dt(rfc3339: &str) -> DateTime { @@ -360,14 +360,18 @@ mod tests { .with_timezone(&Utc) } - fn sample_run_record(run_id: &str, created_at: DateTime) -> RunRecord { + fn test_run_id() -> RunId { + fixtures::RUN_1 + } + + fn sample_run_record(run_id: RunId, created_at: DateTime) -> RunRecord { let mut graph = Graph::new("night-sky"); graph.attrs.insert( "goal".to_string(), AttrValue::String("map the constellations".to_string()), ); RunRecord { - run_id: run_id.to_string(), + run_id, created_at, settings: FabroSettings::default(), graph, @@ -379,11 +383,11 @@ mod tests { } } - fn sample_start_record(run_id: &str, created_at: DateTime) -> StartRecord { + fn sample_start_record(run_id: RunId, created_at: DateTime) -> StartRecord { StartRecord { - run_id: run_id.to_string(), + run_id, start_time: created_at + chrono::Duration::seconds(5), - run_branch: Some("fabro/run/demo".to_string()), + run_branch: Some(format!("fabro/run/{run_id}")), base_sha: Some("abc123".to_string()), } } @@ -434,9 +438,9 @@ mod tests { } } - fn sample_retro(run_id: &str) -> Retro { + fn sample_retro(run_id: RunId) -> Retro { Retro { - run_id: run_id.to_string(), + run_id, workflow_name: "night-sky".to_string(), goal: "map the constellations".to_string(), timestamp: dt("2026-03-27T12:20:00Z"), @@ -478,14 +482,14 @@ mod tests { } } - fn event_payload(run_id: &str, ts: &str, event: &str) -> EventPayload { + fn event_payload(run_id: RunId, ts: &str, event: &str) -> EventPayload { EventPayload::new( serde_json::json!({ "ts": ts, - "run_id": run_id, + "run_id": run_id.to_string(), "event": event }), - run_id, + &run_id, ) .unwrap() } @@ -498,12 +502,13 @@ mod tests { async fn export_run_writes_expected_directory_tree() { let store = InMemoryStore::default(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run_id = test_run_id(); + let run = store.create_run(&run_id, created_at, None).await.unwrap(); - run.put_run(&sample_run_record("run-1", created_at)) + run.put_run(&sample_run_record(run_id, created_at)) .await .unwrap(); - run.put_start(&sample_start_record("run-1", created_at)) + run.put_start(&sample_start_record(run_id, created_at)) .await .unwrap(); run.put_status(&sample_status()).await.unwrap(); @@ -514,7 +519,7 @@ mod tests { .await .unwrap(); run.put_conclusion(&sample_conclusion()).await.unwrap(); - run.put_retro(&sample_retro("run-1")).await.unwrap(); + run.put_retro(&sample_retro(run_id)).await.unwrap(); run.put_graph("digraph night_sky {}").await.unwrap(); run.put_sandbox(&sample_sandbox()).await.unwrap(); @@ -532,14 +537,14 @@ mod tests { run.put_retro_prompt("How did it go?").await.unwrap(); run.put_retro_response("Smooth enough").await.unwrap(); run.append_event(&event_payload( - "run-1", + run_id, "2026-03-27T12:00:00.000Z", "WorkflowRunStarted", )) .await .unwrap(); run.append_event(&event_payload( - "run-1", + run_id, "2026-03-27T12:00:01.000Z", "StageCompleted", )) @@ -568,10 +573,10 @@ mod tests { assert_eq!(file_count, 22); let exported_run: RunRecord = read_json(&output.path().join("run.json")); - assert_eq!(exported_run.run_id, "run-1"); + assert_eq!(exported_run.run_id, run_id); let exported_start: StartRecord = read_json(&output.path().join("start.json")); - assert_eq!(exported_start.run_id, "run-1"); + assert_eq!(exported_start.run_id, run_id); let exported_status: RunStatusRecord = read_json(&output.path().join("status.json")); assert_eq!(exported_status.status, RunStatus::Running); @@ -658,9 +663,10 @@ mod tests { async fn export_run_rejects_path_traversal_and_leaves_no_partial_output() { let store = InMemoryStore::default(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run_id = test_run_id(); + let run = store.create_run(&run_id, created_at, None).await.unwrap(); - run.put_run(&sample_run_record("run-1", created_at)) + run.put_run(&sample_run_record(run_id, created_at)) .await .unwrap(); run.put_asset( diff --git a/lib/crates/fabro-cli/src/commands/system/df.rs b/lib/crates/fabro-cli/src/commands/system/df.rs index 007f6d726..f1f6f0a54 100644 --- a/lib/crates/fabro-cli/src/commands/system/df.rs +++ b/lib/crates/fabro-cli/src/commands/system/df.rs @@ -61,7 +61,7 @@ async fn df_from( } if args.verbose { run_details.push(RunSizeInfo { - run_id: run.run_id.clone(), + run_id: run.run_id.to_string(), workflow_name: run.workflow_name.clone(), status: run.status, start_time_dt: run.start_time_dt, diff --git a/lib/crates/fabro-cli/src/store.rs b/lib/crates/fabro-cli/src/store.rs index 6b3e31ff8..8242edb8f 100644 --- a/lib/crates/fabro-cli/src/store.rs +++ b/lib/crates/fabro-cli/src/store.rs @@ -4,6 +4,7 @@ use std::time::Duration; use anyhow::Result; use fabro_store::{RunStore, SlateStore, Store}; +use fabro_types::RunId; use object_store::local::LocalFileSystem; pub(crate) fn build_store(storage_dir: &Path) -> Result> { @@ -19,7 +20,7 @@ pub(crate) fn build_store(storage_dir: &Path) -> Result> { pub(crate) async fn open_run_reader( storage_dir: &Path, - run_id: &str, + run_id: &RunId, ) -> Result>> { build_store(storage_dir)? .open_run_reader(run_id) diff --git a/lib/crates/fabro-cli/tests/it/cli.rs b/lib/crates/fabro-cli/tests/it/cli.rs index 6208e18d4..313f5414c 100644 --- a/lib/crates/fabro-cli/tests/it/cli.rs +++ b/lib/crates/fabro-cli/tests/it/cli.rs @@ -12,7 +12,7 @@ use fabro_config::user::ExecutionMode; use fabro_git_storage::branchstore::BranchStore; use fabro_git_storage::gitobj::Store as GitStore; use fabro_store::{NodeVisitRef, RuntimeState, SlateStore, Store as _}; -use fabro_types::{Checkpoint, Graph, RunRecord, StartRecord}; +use fabro_types::{Checkpoint, Graph, RunId, RunRecord, StartRecord, fixtures}; use git2::{Repository, Signature}; use object_store::local::LocalFileSystem; use predicates::prelude::*; @@ -302,7 +302,7 @@ fn checkpoint_record( } } -async fn seed_durable_run(storage_dir: &Path, repo_dir: &Path, run_id: &str) { +async fn seed_durable_run(storage_dir: &Path, repo_dir: &Path, run_id: RunId) { let store_path = storage_dir.join("store"); std::fs::create_dir_all(&store_path).unwrap(); let object_store = Arc::new(LocalFileSystem::new_with_prefix(&store_path).unwrap()); @@ -311,10 +311,10 @@ async fn seed_durable_run(storage_dir: &Path, repo_dir: &Path, run_id: &str) { .with_ymd_and_hms(2026, 1, 1, 0, 0, 0) .single() .unwrap(); - let run_store = store.create_run(run_id, created_at, None).await.unwrap(); + let run_store = store.create_run(&run_id, created_at, None).await.unwrap(); let run_record = RunRecord { - run_id: run_id.to_string(), + run_id, created_at, settings: FabroSettings::default(), graph: Graph::default(), @@ -327,7 +327,7 @@ async fn seed_durable_run(storage_dir: &Path, repo_dir: &Path, run_id: &str) { run_store.put_run(&run_record).await.unwrap(); run_store .put_start(&StartRecord { - run_id: run_id.to_string(), + run_id, start_time: created_at, run_branch: Some(format!("fabro/run/{run_id}")), base_sha: None, @@ -835,7 +835,7 @@ fn dry_run_writes_jsonl_and_live_json() { fn run_id_passthrough_uses_provided_ulid() { let tmp = tempfile::tempdir().unwrap(); let storage_dir = tmp.path().join("fabro-data"); - let my_ulid = "01JTEST1234567890ABCDE"; + let my_ulid = fixtures::RUN_10.to_string(); arc() .args([ @@ -843,14 +843,14 @@ fn run_id_passthrough_uses_provided_ulid() { "--dry-run", "--auto-approve", "--run-id", - my_ulid, + my_ulid.as_str(), "--storage-dir", storage_dir.to_str().unwrap(), "../../../test/simple.fabro", ]) .assert() .success() - .stderr(predicate::str::contains(my_ulid)); + .stderr(predicate::str::contains(&my_ulid)); } // == --detach flag ============================================================= @@ -971,8 +971,10 @@ fn rewind_and_fork_recover_missing_metadata_from_store() { let repo_dir = tempfile::tempdir().unwrap(); Repository::init(repo_dir.path()).unwrap(); - let source_run_id = "run-recovery-source"; - let expected_shas = seed_run_branch(repo_dir.path(), source_run_id, &["start", "build"]); + let source_run_id = fixtures::RUN_1; + let source_run_id_string = source_run_id.to_string(); + let expected_shas = + seed_run_branch(repo_dir.path(), &source_run_id_string, &["start", "build"]); Runtime::new().unwrap().block_on(seed_durable_run( &storage_dir, repo_dir.path(), @@ -988,7 +990,7 @@ fn rewind_and_fork_recover_missing_metadata_from_store() { .env("HOME", configured_home.path()) .env("NO_COLOR", "1") .current_dir(repo_dir.path()) - .args(["rewind", source_run_id, "--list"]) + .args(["rewind", &source_run_id_string, "--list"]) .timeout(Duration::from_secs(15)) .assert() .success() @@ -1009,7 +1011,7 @@ fn rewind_and_fork_recover_missing_metadata_from_store() { "rebuilt timeline should persist backfilled SHAs: {rewind_list}" ); - let rebuilt_checkpoints = metadata_checkpoints(repo_dir.path(), source_run_id); + let rebuilt_checkpoints = metadata_checkpoints(repo_dir.path(), &source_run_id_string); assert_eq!(rebuilt_checkpoints.len(), 2); assert_eq!( rebuilt_checkpoints[0].git_commit_sha.as_deref(), @@ -1025,7 +1027,7 @@ fn rewind_and_fork_recover_missing_metadata_from_store() { .env("HOME", configured_home.path()) .env("NO_COLOR", "1") .current_dir(repo_dir.path()) - .args(["fork", source_run_id, "--no-push"]) + .args(["fork", &source_run_id_string, "--no-push"]) .timeout(Duration::from_secs(15)) .assert() .success(); @@ -1173,6 +1175,7 @@ fn completed_run_preserves_workflow_slug_for_lookup() { let home = tempfile::tempdir().unwrap(); let project = tempfile::tempdir().unwrap(); let workflow_dir = project.path().join("workflows").join("sluggy"); + let run_id = fixtures::RUN_11.to_string(); std::fs::create_dir_all(&workflow_dir).unwrap(); let workflow_path = workflow_dir.join("workflow.fabro"); std::fs::write( @@ -1195,7 +1198,7 @@ digraph BarBaz { "--dry-run", "--auto-approve", "--run-id", - "opaque-run-999", + run_id.as_str(), workflow_path.to_str().unwrap(), ]) .assert() @@ -1211,7 +1214,7 @@ digraph BarBaz { arc() .env("HOME", home.path()) .current_dir(project.path()) - .args(["attach", "opaque-run-999"]) + .args(["attach", run_id.as_str()]) .timeout(std::time::Duration::from_secs(10)) .assert() .success(); @@ -1224,7 +1227,7 @@ digraph BarBaz { .assert() .success(); - let run_dir = find_run_dir(home.path(), "opaque-run-999"); + let run_dir = find_run_dir(home.path(), &run_id); let run_record: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(run_dir.join("run.json")).unwrap()).unwrap(); assert_eq!(run_record["graph"]["name"].as_str(), Some("BarBaz")); @@ -1236,6 +1239,7 @@ fn standalone_file_run_uses_file_stem_slug_for_lookup() { let home = tempfile::tempdir().unwrap(); let workflow_dir = tempfile::tempdir().unwrap(); let workflow_path = workflow_dir.path().join("alpha.fabro"); + let run_id = fixtures::RUN_12.to_string(); std::fs::write( &workflow_path, "\ @@ -1255,7 +1259,7 @@ digraph FooWorkflow { "--dry-run", "--auto-approve", "--run-id", - "opaque-run-alpha", + run_id.as_str(), workflow_path.to_str().unwrap(), ]) .assert() @@ -1274,7 +1278,7 @@ digraph FooWorkflow { .assert() .success(); - let run_dir = find_run_dir(home.path(), "opaque-run-alpha"); + let run_dir = find_run_dir(home.path(), &run_id); let run_record: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(run_dir.join("run.json")).unwrap()).unwrap(); assert_eq!(run_record["graph"]["name"].as_str(), Some("FooWorkflow")); @@ -1284,7 +1288,7 @@ digraph FooWorkflow { #[test] fn dry_run_create_start_attach_works_with_default_run_lookup() { let home = tempfile::tempdir().unwrap(); - let run_id = "drysplit-test-123"; + let run_id = fixtures::RUN_13.to_string(); arc() .env("HOME", home.path()) @@ -1293,14 +1297,14 @@ fn dry_run_create_start_attach_works_with_default_run_lookup() { "--dry-run", "--auto-approve", "--run-id", - run_id, + run_id.as_str(), "../../../test/simple.fabro", ]) .assert() .success() - .stdout(predicate::str::contains(run_id)); + .stdout(predicate::str::contains(&run_id)); - let run_dir = find_run_dir(home.path(), run_id); + let run_dir = find_run_dir(home.path(), &run_id); assert!( run_dir.join("run.json").exists(), "create should persist run.json so the run is discoverable" @@ -1308,13 +1312,13 @@ fn dry_run_create_start_attach_works_with_default_run_lookup() { arc() .env("HOME", home.path()) - .args(["start", run_id]) + .args(["start", run_id.as_str()]) .assert() .success(); arc() .env("HOME", home.path()) - .args(["attach", run_id]) + .args(["attach", run_id.as_str()]) .timeout(std::time::Duration::from_secs(10)) .assert() .success(); @@ -1325,7 +1329,7 @@ fn dry_run_create_start_attach_works_with_default_run_lookup() { #[test] fn dry_run_detach_attach_works_with_default_run_lookup() { let home = tempfile::tempdir().unwrap(); - let run_id = "drydetach-test-123"; + let run_id = fixtures::RUN_14.to_string(); arc() .env("HOME", home.path()) @@ -1335,16 +1339,16 @@ fn dry_run_detach_attach_works_with_default_run_lookup() { "--dry-run", "--auto-approve", "--run-id", - run_id, + run_id.as_str(), "../../../test/simple.fabro", ]) .assert() .success() - .stdout(predicate::str::contains(run_id)); + .stdout(predicate::str::contains(&run_id)); arc() .env("HOME", home.path()) - .args(["attach", run_id]) + .args(["attach", run_id.as_str()]) .timeout(std::time::Duration::from_secs(10)) .assert() .success(); @@ -1354,11 +1358,13 @@ fn dry_run_detach_attach_works_with_default_run_lookup() { fn start_by_workflow_name_prefers_newly_created_submitted_run() { let home = tempfile::tempdir().unwrap(); let old_run_dir = home.path().join(".fabro").join("runs").join("old-smoke"); + let old_run_id = fixtures::RUN_15.to_string(); + let run_id = fixtures::RUN_16.to_string(); std::fs::create_dir_all(&old_run_dir).unwrap(); std::fs::write( old_run_dir.join("run.json"), serde_json::json!({ - "run_id": "old-smoke", + "run_id": old_run_id, "created_at": "2026-01-01T00:00:00Z", "settings": {}, "graph": { @@ -1379,7 +1385,6 @@ fn start_by_workflow_name_prefers_newly_created_submitted_run() { ) .unwrap(); - let run_id = "new-smoke-run-123"; arc() .env("HOME", home.path()) .args([ @@ -1387,12 +1392,12 @@ fn start_by_workflow_name_prefers_newly_created_submitted_run() { "--dry-run", "--auto-approve", "--run-id", - run_id, + run_id.as_str(), "smoke", ]) .assert() .success() - .stdout(predicate::str::contains(run_id)); + .stdout(predicate::str::contains(&run_id)); arc() .env("HOME", home.path()) @@ -1402,12 +1407,12 @@ fn start_by_workflow_name_prefers_newly_created_submitted_run() { arc() .env("HOME", home.path()) - .args(["attach", run_id]) + .args(["attach", run_id.as_str()]) .timeout(std::time::Duration::from_secs(10)) .assert() .success(); - let new_run_dir = find_run_dir(home.path(), run_id); + let new_run_dir = find_run_dir(home.path(), &run_id); let status = std::fs::read_to_string(new_run_dir.join("status.json")).unwrap(); assert!( status.contains("\"status\": \"succeeded\""), @@ -1423,6 +1428,7 @@ fn bug2_detached_uses_cached_graph_not_original_path() { let dir = tempfile::tempdir().unwrap(); let storage_dir = dir.path().join("storage"); let run_dir = storage_dir.join("runs").join("20260101-test-bug2"); + let run_id = fixtures::RUN_17.to_string(); std::fs::create_dir_all(&run_dir).unwrap(); let dot = "\ @@ -1434,7 +1440,7 @@ digraph G { // run.json: working_directory is valid but original workflow path no longer exists let run_record = serde_json::json!({ - "run_id": "test-bug2", + "run_id": run_id, "created_at": "2026-01-01T00:00:00Z", "settings": { "dry_run": true, @@ -1592,6 +1598,7 @@ fn bug5_detached_uses_snapshotted_app_id_for_github_credentials() { let storage_dir = tempfile::tempdir().unwrap(); let home = init_cli_home(storage_dir.path()); let run_dir = storage_dir.path().join("runs").join("20260101-test-bug5"); + let run_id = fixtures::RUN_18.to_string(); std::fs::create_dir_all(&run_dir).unwrap(); let dot = "\ @@ -1602,7 +1609,7 @@ digraph G { }"; let run_record = serde_json::json!({ - "run_id": "test-bug5", + "run_id": run_id, "created_at": "2026-01-01T00:00:00Z", "settings": { "dry_run": true, @@ -1661,14 +1668,16 @@ digraph G { #[test] fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { let home = tempfile::tempdir().unwrap(); + let run_id = fixtures::RUN_7.to_string(); + let stage_started = format!( + r#"{{"ts":"2026-01-01T00:00:01Z","run_id":"{run_id}","event":"StageStarted","node_id":"gate","name":"Gate","index":0,"attempt":1,"max_attempts":1}}"# + ); let run_dir = setup_run_dir( home.path(), - "bug3-test", + &run_id, serde_json::json!({}), - &[ - r#"{"ts":"2026-01-01T00:00:01Z","run_id":"bug3","event":"StageStarted","node_id":"gate","name":"Gate","index":0,"attempt":1,"max_attempts":1}"#, - ], + &[&stage_started], ); // Terminal status still allows attach to answer the interview once before exiting. @@ -1702,7 +1711,7 @@ fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { let _ = arc() .env("HOME", home.path()) .env("NO_COLOR", "1") - .args(["attach", "bug3-test"]) + .args(["attach", &run_id]) .write_stdin("y\n") .timeout(std::time::Duration::from_secs(5)) .output(); @@ -1724,14 +1733,16 @@ fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { #[test] fn attach_closed_stdin_keeps_interview_pending() { let home = tempfile::tempdir().unwrap(); + let run_id = fixtures::RUN_8.to_string(); + let stage_started = format!( + r#"{{"ts":"2026-01-01T00:00:01Z","run_id":"{run_id}","event":"StageStarted","node_id":"gate","name":"Gate","index":0,"attempt":1,"max_attempts":1}}"# + ); let run_dir = setup_run_dir( home.path(), - "attach-closed-stdin", + &run_id, serde_json::json!({}), - &[ - r#"{"ts":"2026-01-01T00:00:01Z","run_id":"attach-closed-stdin","event":"StageStarted","node_id":"gate","name":"Gate","index":0,"attempt":1,"max_attempts":1}"#, - ], + &[&stage_started], ); std::fs::write( @@ -1761,7 +1772,7 @@ fn attach_closed_stdin_keeps_interview_pending() { let assert = arc() .env("HOME", home.path()) .env("NO_COLOR", "1") - .args(["attach", "attach-closed-stdin"]) + .args(["attach", &run_id]) .timeout(std::time::Duration::from_secs(5)) .assert() .failure(); @@ -1790,21 +1801,34 @@ fn attach_closed_stdin_keeps_interview_pending() { #[test] fn bug4_attach_respects_verbose_from_spec() { let home = tempfile::tempdir().unwrap(); + let run_id = fixtures::RUN_9.to_string(); // Use pre-rename field names so handle_json_line can parse them // (isolates this test from bug 1). With 2 turns and 1 tool call, // verbose mode should display "(2 turns, 1 tools, …)" in the output. let run_dir = setup_run_dir( home.path(), - "bug4-test", + &run_id, serde_json::json!({"verbose": true}), &[ - r#"{"ts":"2026-01-01T12:00:00Z","run_id":"bug4","event":"StageStarted","node_id":"code","name":"Code","index":0,"attempt":1,"max_attempts":1}"#, - r#"{"ts":"2026-01-01T12:00:01Z","run_id":"bug4","event":"Agent.AssistantMessage","stage":"code","model":"claude-sonnet"}"#, - r#"{"ts":"2026-01-01T12:00:02Z","run_id":"bug4","event":"Agent.AssistantMessage","stage":"code","model":"claude-sonnet"}"#, - r#"{"ts":"2026-01-01T12:00:03Z","run_id":"bug4","event":"Agent.ToolCallStarted","stage":"code","tool_name":"read_file","tool_call_id":"tc1","arguments":{}}"#, - r#"{"ts":"2026-01-01T12:00:04Z","run_id":"bug4","event":"Agent.ToolCallCompleted","stage":"code","tool_name":"read_file","tool_call_id":"tc1","is_error":false}"#, - r#"{"ts":"2026-01-01T12:00:10Z","run_id":"bug4","event":"StageCompleted","node_id":"code","name":"Code","index":0,"duration_ms":10000,"status":"success","usage":{"input_tokens":1000,"output_tokens":500}}"#, + &format!( + r#"{{"ts":"2026-01-01T12:00:00Z","run_id":"{run_id}","event":"StageStarted","node_id":"code","name":"Code","index":0,"attempt":1,"max_attempts":1}}"# + ), + &format!( + r#"{{"ts":"2026-01-01T12:00:01Z","run_id":"{run_id}","event":"Agent.AssistantMessage","stage":"code","model":"claude-sonnet"}}"# + ), + &format!( + r#"{{"ts":"2026-01-01T12:00:02Z","run_id":"{run_id}","event":"Agent.AssistantMessage","stage":"code","model":"claude-sonnet"}}"# + ), + &format!( + r#"{{"ts":"2026-01-01T12:00:03Z","run_id":"{run_id}","event":"Agent.ToolCallStarted","stage":"code","tool_name":"read_file","tool_call_id":"tc1","arguments":{{}}}}"# + ), + &format!( + r#"{{"ts":"2026-01-01T12:00:04Z","run_id":"{run_id}","event":"Agent.ToolCallCompleted","stage":"code","tool_name":"read_file","tool_call_id":"tc1","is_error":false}}"# + ), + &format!( + r#"{{"ts":"2026-01-01T12:00:10Z","run_id":"{run_id}","event":"StageCompleted","node_id":"code","name":"Code","index":0,"duration_ms":10000,"status":"success","usage":{{"input_tokens":1000,"output_tokens":500}}}}"# + ), ], ); @@ -1831,7 +1855,7 @@ fn bug4_attach_respects_verbose_from_spec() { let output = arc() .env("HOME", home.path()) .env("NO_COLOR", "1") - .args(["attach", "bug4-test"]) + .args(["attach", &run_id]) .timeout(std::time::Duration::from_secs(10)) .output() .expect("process should start"); @@ -1994,7 +2018,7 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() { let (home, project, storage_dir) = setup_external_workflow_fixture(); let cwd = tempfile::tempdir().unwrap(); let workflow = project.path().join("workflow.toml"); - let run_id = "external-config-run"; + let run_id = fixtures::RUN_19.to_string(); arc() .env("HOME", home.path()) @@ -2005,7 +2029,7 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() { "--model", "gpt-5.2", "--run-id", - run_id, + run_id.as_str(), workflow.to_str().unwrap(), ]) .assert() @@ -2020,7 +2044,7 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() { path.is_dir() && path .file_name() - .is_some_and(|name| name.to_string_lossy().ends_with(run_id)) + .is_some_and(|name| name.to_string_lossy().ends_with(&run_id)) }) .unwrap_or_else(|| { panic!( diff --git a/lib/crates/fabro-hooks/src/bridge.rs b/lib/crates/fabro-hooks/src/bridge.rs index a0f0526a6..a50d221fb 100644 --- a/lib/crates/fabro-hooks/src/bridge.rs +++ b/lib/crates/fabro-hooks/src/bridge.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use std::sync::Arc; use fabro_agent::{Sandbox, ToolHookCallback, ToolHookDecision}; +use fabro_types::RunId; use crate::runner::HookRunner; use crate::types::{HookContext, HookDecision, HookEvent}; @@ -13,7 +14,7 @@ use crate::types::{HookContext, HookDecision, HookEvent}; pub struct WorkflowToolHookCallback { pub hook_runner: Arc, pub sandbox: Arc, - pub run_id: String, + pub run_id: RunId, pub workflow_name: String, pub work_dir: Option, pub node_id: String, @@ -75,6 +76,7 @@ mod tests { use crate::config::{HookConfig, HookDefinition}; use crate::executor::HookExecutor; use crate::types::{HookContext, HookResult}; + use fabro_types::fixtures; use std::path::Path; use std::sync::Mutex; @@ -127,7 +129,7 @@ mod tests { WorkflowToolHookCallback { hook_runner, sandbox, - run_id: "run-1".into(), + run_id: fixtures::RUN_1, workflow_name: "test-wf".into(), work_dir: None, node_id: "plan".into(), @@ -160,7 +162,7 @@ mod tests { contexts[0].tool_input, Some(serde_json::json!({"command": "ls"})) ); - assert_eq!(contexts[0].run_id, "run-1"); + assert_eq!(contexts[0].run_id, fixtures::RUN_1); assert_eq!(contexts[0].node_id.as_deref(), Some("plan")); } diff --git a/lib/crates/fabro-hooks/src/executor.rs b/lib/crates/fabro-hooks/src/executor.rs index 1aa43f186..0add15b1a 100644 --- a/lib/crates/fabro-hooks/src/executor.rs +++ b/lib/crates/fabro-hooks/src/executor.rs @@ -125,7 +125,7 @@ impl HookExecutorImpl { let mut env_vars = HashMap::new(); env_vars.insert("FABRO_EVENT".to_string(), context.event.to_string()); - env_vars.insert("FABRO_RUN_ID".to_string(), context.run_id.clone()); + env_vars.insert("FABRO_RUN_ID".to_string(), context.run_id.to_string()); env_vars.insert("FABRO_WORKFLOW".to_string(), context.workflow_name.clone()); if let Some(ref node_id) = context.node_id { env_vars.insert("FABRO_NODE_ID".to_string(), node_id.clone()); @@ -603,9 +603,10 @@ mod tests { use super::*; use crate::config::HookType; use crate::types::HookEvent; + use fabro_types::fixtures; fn make_context() -> HookContext { - HookContext::new(HookEvent::StageStart, "run-1".into(), "test-wf".into()) + HookContext::new(HookEvent::StageStart, fixtures::RUN_1, "test-wf".into()) } fn make_sandbox() -> Arc { diff --git a/lib/crates/fabro-hooks/src/runner.rs b/lib/crates/fabro-hooks/src/runner.rs index b52b42b1b..e56aee971 100644 --- a/lib/crates/fabro-hooks/src/runner.rs +++ b/lib/crates/fabro-hooks/src/runner.rs @@ -212,6 +212,7 @@ mod tests { use super::*; use crate::config::HookConfig; use crate::types::{HookContext, HookEvent, HookResult}; + use fabro_types::fixtures; struct MockExecutor { decision: HookDecision, @@ -241,7 +242,7 @@ mod tests { } fn make_context(event: HookEvent) -> HookContext { - HookContext::new(event, "run-1".into(), "test-wf".into()) + HookContext::new(event, fixtures::RUN_1, "test-wf".into()) } fn make_hook(event: HookEvent, name: &str) -> HookDefinition { diff --git a/lib/crates/fabro-hooks/src/types.rs b/lib/crates/fabro-hooks/src/types.rs index 6a0780c38..a976bbe84 100644 --- a/lib/crates/fabro-hooks/src/types.rs +++ b/lib/crates/fabro-hooks/src/types.rs @@ -1,12 +1,13 @@ pub use fabro_config::hook::HookEvent; +use fabro_types::RunId; use serde::{Deserialize, Serialize}; /// Rich JSON payload sent to hooks. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct HookContext { pub event: HookEvent, - pub run_id: String, + pub run_id: RunId, pub workflow_name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub cwd: Option, @@ -44,7 +45,7 @@ pub struct HookContext { impl HookContext { #[must_use] - pub fn new(event: HookEvent, run_id: String, workflow_name: String) -> Self { + pub fn new(event: HookEvent, run_id: RunId, workflow_name: String) -> Self { Self { event, run_id, @@ -126,12 +127,13 @@ pub struct HookResult { #[cfg(test)] mod tests { use super::*; + use fabro_types::fixtures; #[test] fn hook_context_serde_round_trip() { let ctx = HookContext { event: HookEvent::StageStart, - run_id: "run-123".into(), + run_id: fixtures::RUN_1, workflow_name: "test-wf".into(), cwd: Some("/tmp".into()), node_id: Some("plan".into()), @@ -153,13 +155,13 @@ mod tests { let json = serde_json::to_string(&ctx).unwrap(); let back: HookContext = serde_json::from_str(&json).unwrap(); assert_eq!(back.event, HookEvent::StageStart); - assert_eq!(back.run_id, "run-123"); + assert_eq!(back.run_id, fixtures::RUN_1); assert_eq!(back.node_id.as_deref(), Some("plan")); } #[test] fn hook_context_omits_none_fields() { - let ctx = HookContext::new(HookEvent::RunStart, "run-1".into(), "wf".into()); + let ctx = HookContext::new(HookEvent::RunStart, fixtures::RUN_1, "wf".into()); let json = serde_json::to_string(&ctx).unwrap(); assert!(!json.contains("node_id")); assert!(!json.contains("failure_reason")); @@ -258,7 +260,7 @@ mod tests { #[test] fn hook_context_with_tool_fields() { - let mut ctx = HookContext::new(HookEvent::PreToolUse, "run-1".into(), "wf".into()); + let mut ctx = HookContext::new(HookEvent::PreToolUse, fixtures::RUN_1, "wf".into()); ctx.tool_name = Some("shell".into()); ctx.tool_input = Some(serde_json::json!({"command": "ls"})); ctx.tool_call_id = Some("call_123".into()); @@ -270,7 +272,7 @@ mod tests { #[test] fn hook_context_tool_output_serializes() { - let mut ctx = HookContext::new(HookEvent::PostToolUse, "run-1".into(), "wf".into()); + let mut ctx = HookContext::new(HookEvent::PostToolUse, fixtures::RUN_1, "wf".into()); ctx.tool_name = Some("shell".into()); ctx.tool_output = Some("file1.txt\nfile2.txt".into()); let json = serde_json::to_string(&ctx).unwrap(); diff --git a/lib/crates/fabro-retro/src/retro.rs b/lib/crates/fabro-retro/src/retro.rs index 7bf228cce..6c07964b9 100644 --- a/lib/crates/fabro-retro/src/retro.rs +++ b/lib/crates/fabro-retro/src/retro.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::path::Path; +use fabro_types::RunId; pub use fabro_types::retro::{ AggregateStats, FrictionKind, FrictionPoint, Learning, LearningCategory, OpenItem, OpenItemKind, Retro, RetroNarrative, SmoothnessRating, StageRetro, @@ -68,7 +69,7 @@ pub fn extract_stage_durations(run_dir: &Path) -> HashMap { } pub fn derive_retro( - run_id: &str, + run_id: RunId, workflow_name: &str, goal: &str, completed_stages: Vec, @@ -133,7 +134,7 @@ pub fn derive_retro( }; Retro { - run_id: run_id.to_string(), + run_id, workflow_name: workflow_name.to_string(), goal: goal.to_string(), timestamp: chrono::Utc::now(), diff --git a/lib/crates/fabro-sandbox/src/daytona/mod.rs b/lib/crates/fabro-sandbox/src/daytona/mod.rs index ad79f6657..2e831c490 100644 --- a/lib/crates/fabro-sandbox/src/daytona/mod.rs +++ b/lib/crates/fabro-sandbox/src/daytona/mod.rs @@ -12,6 +12,7 @@ use crate::{ use async_trait::async_trait; use daytona_sdk::api_types::SignedPortPreviewUrl; use fabro_github::GitHubAppCredentials; +use fabro_types::RunId; use rand::Rng; use tokio::fs; use tokio::sync::OnceCell; @@ -36,7 +37,7 @@ pub struct DaytonaSandbox { event_callback: Option, /// HTTPS origin URL stored after clone so we can refresh push credentials later. origin_url: OnceCell, - run_id: Option, + run_id: Option, /// Explicit branch to clone. When set, overrides the branch detected by /// `detect_repo_info` — avoids cloning a local-only worktree branch /// (e.g. `fabro/run/...`) that was never pushed to origin. @@ -48,7 +49,7 @@ impl DaytonaSandbox { pub async fn new( config: DaytonaConfig, github_app: Option, - run_id: Option, + run_id: Option, clone_branch: Option, ) -> Result { let client = daytona_sdk::Client::new() diff --git a/lib/crates/fabro-sandbox/src/exe/mod.rs b/lib/crates/fabro-sandbox/src/exe/mod.rs index 51dd5631b..5eb82355a 100644 --- a/lib/crates/fabro-sandbox/src/exe/mod.rs +++ b/lib/crates/fabro-sandbox/src/exe/mod.rs @@ -11,6 +11,7 @@ use crate::{ format_lines_numbered, }; use async_trait::async_trait; +use fabro_types::RunId; use tokio_util::sync::CancellationToken; pub use crate::ssh_common::{GitCloneParams, SshOutput, SshRunner}; @@ -73,7 +74,7 @@ pub struct ExeSandbox { data_ssh_factory: DataSshFactory, config: ExeConfig, clone_params: Option, - run_id: Option, + run_id: Option, origin_url: tokio::sync::OnceCell, github_app: Option, } @@ -84,7 +85,7 @@ impl ExeSandbox { mgmt_ssh: Box, config: ExeConfig, clone_params: Option, - run_id: Option, + run_id: Option, github_app: Option, ) -> Self { Self { diff --git a/lib/crates/fabro-sandbox/src/sandbox_spec.rs b/lib/crates/fabro-sandbox/src/sandbox_spec.rs index 0d5ecef3b..dc5a9c9f9 100644 --- a/lib/crates/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/crates/fabro-sandbox/src/sandbox_spec.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::sync::Arc; -use fabro_types::settings::WorktreeMode; +use fabro_types::{RunId, settings::WorktreeMode}; #[cfg(any(feature = "docker", feature = "daytona", feature = "exe"))] use anyhow::anyhow; @@ -35,14 +35,14 @@ pub enum SandboxSpec { Daytona { config: DaytonaConfig, github_app: Option, - run_id: Option, + run_id: Option, clone_branch: Option, }, #[cfg(feature = "exe")] Exe { config: ExeConfig, clone_params: Option, - run_id: Option, + run_id: Option, github_app: Option, mgmt_destination: String, }, @@ -50,7 +50,7 @@ pub enum SandboxSpec { Ssh { config: SshConfig, clone_params: Option, - run_id: Option, + run_id: Option, github_app: Option, }, } diff --git a/lib/crates/fabro-sandbox/src/ssh/mod.rs b/lib/crates/fabro-sandbox/src/ssh/mod.rs index 4d8dd74ca..97a1975ed 100644 --- a/lib/crates/fabro-sandbox/src/ssh/mod.rs +++ b/lib/crates/fabro-sandbox/src/ssh/mod.rs @@ -13,6 +13,7 @@ use crate::{ format_lines_numbered, }; use async_trait::async_trait; +use fabro_types::RunId; use tokio::fs; use tokio::sync::OnceCell; use tokio_util::sync::CancellationToken; @@ -32,7 +33,7 @@ pub struct SshSandbox { ssh: OnceCell>, config: SshConfig, clone_params: Option, - run_id: Option, + run_id: Option, github_app: Option, rg_available: OnceCell, event_callback: Option, @@ -44,7 +45,7 @@ impl SshSandbox { pub fn new( config: SshConfig, clone_params: Option, - run_id: Option, + run_id: Option, github_app: Option, ) -> Self { Self { diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 076520c29..0cab82a94 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -18,6 +18,7 @@ use fabro_llm::types::{ }; use fabro_retro::retro::Retro; use fabro_store::{InMemoryStore, Store}; +use fabro_types::RunId; use fabro_util::redact::redact_jsonl_line; use fabro_workflows::error::FabroError; use fabro_workflows::handler::HandlerRegistry; @@ -30,6 +31,7 @@ use tokio::time::{sleep, timeout}; use tokio_stream::StreamExt; use tokio_stream::wrappers::BroadcastStream; use tower::{ServiceExt, service_fn}; +use ulid::Ulid; use tracing::{error, info}; @@ -117,7 +119,7 @@ type RegistryFactoryOverride = dyn Fn(Arc) -> HandlerRegistry + /// Shared application state for the server. pub struct AppState { - runs: Mutex>, + runs: Mutex>, aggregate_usage: Mutex, store: Arc, pub db: sqlx::SqlitePool, @@ -452,7 +454,7 @@ async fn list_runs( let all_items: Vec = runs .iter() .map(|(id, managed_run)| RunStatusResponse { - id: id.clone(), + id: id.to_string(), status: managed_run.status, error: managed_run .error @@ -477,8 +479,8 @@ async fn list_runs( .into_response() } -fn compute_queue_positions(runs: &HashMap) -> HashMap { - let mut queued: Vec<(&String, &ManagedRun)> = runs +fn compute_queue_positions(runs: &HashMap) -> HashMap { + let mut queued: Vec<(&RunId, &ManagedRun)> = runs .iter() .filter(|(_, r)| r.status == RunStatus::Queued) .collect(); @@ -486,10 +488,15 @@ fn compute_queue_positions(runs: &HashMap) -> HashMap Result { + id.parse::() + .map_err(|_| ApiError::bad_request("Invalid run ID.").into_response()) +} + fn clear_live_run_state(run: &mut ManagedRun) { run.interviewer = None; run.event_tx = None; @@ -502,7 +509,7 @@ async fn start_run( State(state): State>, Json(req): Json, ) -> Response { - let run_id = ulid::Ulid::new().to_string(); + let run_id = RunId::new(); info!(run_id = %run_id, "Run queued"); let run_dir = std::env::temp_dir().join(format!("fabro-{}", uuid::Uuid::new_v4())); let settings = state.settings.read().unwrap().clone(); @@ -515,7 +522,7 @@ async fn start_run( cwd: std::env::current_dir().unwrap_or_else(|_| std::env::temp_dir()), workflow_slug: None, run_dir: Some(run_dir.clone()), - run_id: Some(run_id.clone()), + run_id: Some(run_id), host_repo_path: None, base_branch: None, }) { @@ -549,7 +556,7 @@ async fn start_run( { let mut runs = state.runs.lock().expect("runs lock poisoned"); runs.insert( - run_id.clone(), + run_id, ManagedRun { dot_source: req.dot_source, status: RunStatus::Queued, @@ -571,7 +578,7 @@ async fn start_run( ( StatusCode::CREATED, Json(RunStatusResponse { - id: run_id, + id: run_id.to_string(), status: RunStatus::Queued, error: None, queue_position: None, @@ -582,7 +589,7 @@ async fn start_run( } /// Execute a single run: transitions queued → starting → running → completed/failed/cancelled. -async fn execute_run(state: Arc, run_id: String) { +async fn execute_run(state: Arc, run_id: RunId) { // Transition to Starting and set up cancel infrastructure let (cancel_rx, run_dir, event_tx, cancel_token) = { let mut runs = state.runs.lock().expect("runs lock poisoned"); @@ -828,6 +835,10 @@ async fn get_run_status( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { Some(managed_run) => { @@ -840,7 +851,7 @@ async fn get_run_status( ( StatusCode::OK, Json(RunStatusResponse { - id: id.clone(), + id: id.to_string(), status: managed_run.status, error: managed_run .error @@ -863,6 +874,10 @@ async fn get_questions( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { Some(managed_run) => { @@ -910,6 +925,10 @@ async fn submit_answer( Path((id, qid)): Path<(String, String)>, Json(req): Json, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { Some(managed_run) => { @@ -970,6 +989,10 @@ async fn get_events( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let rx = { let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { @@ -1002,6 +1025,10 @@ async fn get_checkpoint( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { Some(managed_run) => match &managed_run.checkpoint { @@ -1017,6 +1044,10 @@ async fn get_context( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { Some(managed_run) => match &managed_run.context { @@ -1032,6 +1063,10 @@ async fn cancel_run( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let mut runs = state.runs.lock().expect("runs lock poisoned"); match runs.get_mut(&id) { Some(managed_run) => match managed_run.status { @@ -1047,7 +1082,7 @@ async fn cancel_run( ( StatusCode::OK, Json(RunStatusResponse { - id: id.clone(), + id: id.to_string(), status: RunStatus::Cancelled, error: None, queue_position: None, @@ -1067,6 +1102,10 @@ async fn pause_run( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let mut runs = state.runs.lock().expect("runs lock poisoned"); match runs.get_mut(&id) { Some(managed_run) => match managed_run.status { @@ -1076,7 +1115,7 @@ async fn pause_run( ( StatusCode::OK, Json(RunStatusResponse { - id: id.clone(), + id: id.to_string(), status: RunStatus::Paused, error: None, queue_position: None, @@ -1096,6 +1135,10 @@ async fn unpause_run( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let mut runs = state.runs.lock().expect("runs lock poisoned"); match runs.get_mut(&id) { Some(managed_run) => match managed_run.status { @@ -1105,7 +1148,7 @@ async fn unpause_run( ( StatusCode::OK, Json(RunStatusResponse { - id: id.clone(), + id: id.to_string(), status: RunStatus::Running, error: None, queue_position: None, @@ -1308,7 +1351,7 @@ async fn create_completion( // Dry-run mode returns a stub response if state.dry_run() { - let msg_id = ulid::Ulid::new().to_string(); + let msg_id = Ulid::new().to_string(); if use_stream { let finish_event = StreamEvent::finish( FinishReason::Stop, @@ -1408,7 +1451,7 @@ async fn create_completion( .into_response() } else { // Non-streaming path - let msg_id = ulid::Ulid::new().to_string(); + let msg_id = Ulid::new().to_string(); if let Some(schema) = req.schema { // Structured output uses generate_object for JSON parsing logic @@ -1469,6 +1512,10 @@ async fn get_retro( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let run_dir = { let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { @@ -1529,6 +1576,10 @@ async fn get_graph( State(state): State>, Path(id): Path, ) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; let dot_source = { let runs = state.runs.lock().expect("runs lock poisoned"); match runs.get(&id) { @@ -1545,6 +1596,7 @@ mod tests { use super::*; use axum::body::Body; use axum::http::Request; + use fabro_types::fixtures; use fabro_workflows::records::{RunRecord, RunRecordExt}; use tower::ServiceExt; @@ -1720,7 +1772,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Give run a moment to start tokio::time::sleep(std::time::Duration::from_millis(50)).await; @@ -1736,7 +1788,7 @@ mod tests { assert_eq!(response.status(), StatusCode::OK); let body = body_json(response.into_body()).await; - assert_eq!(body["id"].as_str().unwrap(), run_id); + assert_eq!(body["id"].as_str().unwrap(), run_id.to_string()); let status = body["status"].as_str().unwrap(); assert!( status == "queued" @@ -1750,10 +1802,11 @@ mod tests { #[tokio::test] async fn get_run_status_not_found() { let app = test_app_with(test_db().await); + let missing_run_id = fixtures::RUN_64; let req = Request::builder() .method("GET") - .uri("/runs/nonexistent") + .uri(format!("/runs/{missing_run_id}")) .body(Body::empty()) .unwrap(); @@ -1778,7 +1831,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Get questions (should be empty for a run without wait.human nodes) let req = Request::builder() @@ -1798,10 +1851,11 @@ mod tests { #[tokio::test] async fn submit_answer_not_found_run() { let app = test_app_with(test_db().await); + let missing_run_id = fixtures::RUN_64; let req = Request::builder() .method("POST") - .uri("/runs/nonexistent/questions/q1/answer") + .uri(format!("/runs/{missing_run_id}/questions/q1/answer")) .header("content-type", "application/json") .body(Body::from( serde_json::to_string(&serde_json::json!({"value": "yes"})).unwrap(), @@ -1815,10 +1869,11 @@ mod tests { #[tokio::test] async fn get_events_not_found() { let app = test_app_with(test_db().await); + let missing_run_id = fixtures::RUN_64; let req = Request::builder() .method("GET") - .uri("/runs/nonexistent/events") + .uri(format!("/runs/{missing_run_id}/events")) .body(Body::empty()) .unwrap(); @@ -1843,7 +1898,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Get checkpoint immediately (before run completes, may be null) let req = Request::builder() @@ -1873,7 +1928,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Get context let req = Request::builder() @@ -1906,7 +1961,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Cancel it let req = Request::builder() @@ -1927,10 +1982,11 @@ mod tests { #[tokio::test] async fn cancel_nonexistent_run_returns_not_found() { let app = test_app_with(test_db().await); + let missing_run_id = fixtures::RUN_64; let req = Request::builder() .method("POST") - .uri("/runs/nonexistent/cancel") + .uri(format!("/runs/{missing_run_id}/cancel")) .body(Body::empty()) .unwrap(); @@ -1955,7 +2011,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Wait for scheduler to promote run (creates event_tx) tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -2006,7 +2062,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Poll until run completes let mut status = String::new(); @@ -2045,7 +2101,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Request graph SVG let req = Request::builder() @@ -2085,10 +2141,11 @@ mod tests { #[tokio::test] async fn get_graph_not_found() { let app = test_app_with(test_db().await); + let missing_run_id = fixtures::RUN_64; let req = Request::builder() .method("GET") - .uri("/runs/nonexistent/graph") + .uri(format!("/runs/{missing_run_id}/graph")) .body(Body::empty()) .unwrap(); @@ -2126,7 +2183,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // List should now contain one run let req = Request::builder() @@ -2140,7 +2197,7 @@ mod tests { let body = body_json(response.into_body()).await; let items = body["data"].as_array().unwrap(); assert_eq!(items.len(), 1); - assert_eq!(items[0]["id"].as_str().unwrap(), run_id); + assert_eq!(items[0]["id"].as_str().unwrap(), run_id.to_string()); assert!(items[0]["status"].as_str().is_some()); assert!(!body["meta"]["has_more"].as_bool().unwrap()); } @@ -2185,7 +2242,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Poll until run completes let mut status = String::new(); @@ -2236,7 +2293,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::CREATED); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Check status is queued (no scheduler running) let req = Request::builder() @@ -2309,7 +2366,7 @@ mod tests { let response = app.oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::CREATED); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); let run_dir = { let runs = state.runs.lock().expect("runs lock poisoned"); @@ -2344,11 +2401,11 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::CREATED); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); *state.settings.write().unwrap() = FabroSettings::default(); - execute_run(Arc::clone(&state), run_id.clone()).await; + execute_run(Arc::clone(&state), run_id).await; let runs = state.runs.lock().expect("runs lock poisoned"); let managed_run = runs.get(&run_id).expect("run should still exist"); @@ -2378,7 +2435,7 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); // Cancel it let req = Request::builder() @@ -2425,9 +2482,9 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::CREATED); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - let runner = tokio::spawn(execute_run(Arc::clone(&state), run_id.clone())); + let runner = tokio::spawn(execute_run(Arc::clone(&state), run_id)); tokio::time::sleep(std::time::Duration::from_millis(100)).await; { @@ -2498,9 +2555,9 @@ mod tests { let response = app.clone().oneshot(req).await.unwrap(); assert_eq!(response.status(), StatusCode::CREATED); let body = body_json(response.into_body()).await; - let run_id = body["id"].as_str().unwrap().to_string(); + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - let runner = tokio::spawn(execute_run(Arc::clone(&state), run_id.clone())); + let runner = tokio::spawn(execute_run(Arc::clone(&state), run_id)); tokio::time::sleep(std::time::Duration::from_millis(50)).await; let req = Request::builder() diff --git a/lib/crates/fabro-store/src/disk_projecting.rs b/lib/crates/fabro-store/src/disk_projecting.rs index 8f2c5212e..73d9e793b 100644 --- a/lib/crates/fabro-store/src/disk_projecting.rs +++ b/lib/crates/fabro-store/src/disk_projecting.rs @@ -375,7 +375,8 @@ mod tests { use super::*; use crate::{InMemoryStore, Store}; use fabro_types::{ - AggregateStats, AttrValue, FabroSettings, Graph, RunStatus, StageStatus, StatusReason, + AggregateStats, AttrValue, FabroSettings, Graph, RunId, RunStatus, StageStatus, + StatusReason, fixtures, }; fn dt(rfc3339: &str) -> DateTime { @@ -384,6 +385,13 @@ mod tests { .with_timezone(&Utc) } + fn test_run_id(label: &str) -> RunId { + match label { + "run-1" => fixtures::RUN_1, + _ => panic!("unknown test run id: {label}"), + } + } + fn sample_run_record(run_id: &str, created_at: DateTime) -> RunRecord { let mut graph = Graph::new("night-sky"); graph.attrs.insert( @@ -391,7 +399,7 @@ mod tests { AttrValue::String("map the constellations".to_string()), ); RunRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), created_at, settings: FabroSettings::default(), graph, @@ -405,7 +413,7 @@ mod tests { fn sample_start_record(run_id: &str, created_at: DateTime) -> StartRecord { StartRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), start_time: created_at + ChronoDuration::seconds(5), run_branch: Some("fabro/run/demo".to_string()), base_sha: Some("abc123".to_string()), @@ -460,7 +468,7 @@ mod tests { fn sample_retro(run_id: &str) -> Retro { Retro { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), workflow_name: "night-sky".to_string(), goal: "map the constellations".to_string(), timestamp: dt("2026-03-27T12:20:00Z"), @@ -506,10 +514,10 @@ mod tests { EventPayload::new( serde_json::json!({ "ts": ts, - "run_id": run_id, + "run_id": test_run_id(run_id).to_string(), "event": event, }), - run_id, + &test_run_id(run_id), ) .unwrap() } @@ -520,7 +528,7 @@ mod tests { ) -> (Arc, DiskProjectingRunStore) { let inner = InMemoryStore::default() .create_run( - "run-1", + &test_run_id("run-1"), created_at, Some(run_dir.to_string_lossy().as_ref()), ) @@ -796,7 +804,7 @@ mod tests { let created_at = dt("2026-03-27T12:00:00Z"); let inner = InMemoryStore::default() .create_run( - "run-1", + &test_run_id("run-1"), created_at, Some(temp.path().to_string_lossy().as_ref()), ) diff --git a/lib/crates/fabro-store/src/lib.rs b/lib/crates/fabro-store/src/lib.rs index af13adc4b..825a774ea 100644 --- a/lib/crates/fabro-store/src/lib.rs +++ b/lib/crates/fabro-store/src/lib.rs @@ -24,8 +24,8 @@ pub use types::{ }; use fabro_types::{ - Checkpoint, Conclusion, NodeStatusRecord, Retro, RunRecord, RunStatusRecord, SandboxRecord, - StartRecord, + Checkpoint, Conclusion, NodeStatusRecord, Retro, RunId, RunRecord, RunStatusRecord, + SandboxRecord, StartRecord, }; #[derive(Debug, Default, Clone, PartialEq, Eq)] @@ -38,14 +38,14 @@ pub struct ListRunsQuery { pub trait Store: Send + Sync { async fn create_run( &self, - run_id: &str, + run_id: &RunId, created_at: DateTime, run_dir: Option<&str>, ) -> Result>; - async fn open_run(&self, run_id: &str) -> Result>>; - async fn open_run_reader(&self, run_id: &str) -> Result>>; + async fn open_run(&self, run_id: &RunId) -> Result>>; + async fn open_run_reader(&self, run_id: &RunId) -> Result>>; async fn list_runs(&self, query: &ListRunsQuery) -> Result>; - async fn delete_run(&self, run_id: &str) -> Result<()>; + async fn delete_run(&self, run_id: &RunId) -> Result<()>; } #[async_trait] diff --git a/lib/crates/fabro-store/src/memory.rs b/lib/crates/fabro-store/src/memory.rs index d4af91e3a..3953e2aa2 100644 --- a/lib/crates/fabro-store/src/memory.rs +++ b/lib/crates/fabro-store/src/memory.rs @@ -18,13 +18,13 @@ use crate::{ RunSnapshot, RunStore, RunSummary, Store, StoreError, }; use fabro_types::{ - Checkpoint, Conclusion, NodeStatusRecord, Retro, RunRecord, RunStatusRecord, SandboxRecord, - StartRecord, + Checkpoint, Conclusion, NodeStatusRecord, Retro, RunId, RunRecord, RunStatusRecord, + SandboxRecord, StartRecord, }; #[derive(Debug, Default)] pub struct InMemoryStore { - runs: Mutex>, + runs: Mutex>, } #[derive(Debug, Clone)] @@ -35,7 +35,7 @@ struct InMemoryCatalog { #[derive(Debug)] struct InMemoryRunStore { - run_id: String, + run_id: RunId, created_at: DateTime, data: Mutex>>, event_seq: AtomicU32, @@ -45,13 +45,13 @@ struct InMemoryRunStore { impl InMemoryRunStore { fn new( - run_id: &str, + run_id: &RunId, created_at: DateTime, db_prefix: String, run_dir: Option, ) -> Result { let record = CatalogRecord { - run_id: run_id.to_string(), + run_id: *run_id, created_at, db_prefix, run_dir, @@ -59,7 +59,7 @@ impl InMemoryRunStore { let mut data = BTreeMap::new(); data.insert(keys::init().to_string(), serde_json::to_vec(&record)?); Ok(Self { - run_id: run_id.to_string(), + run_id: *run_id, created_at, data: Mutex::new(data), event_seq: AtomicU32::new(1), @@ -247,7 +247,7 @@ impl InMemoryRunStore { impl Store for InMemoryStore { async fn create_run( &self, - run_id: &str, + run_id: &RunId, created_at: DateTime, run_dir: Option<&str>, ) -> Result> { @@ -268,25 +268,25 @@ impl Store for InMemoryStore { )?); let catalog = InMemoryCatalog { record: CatalogRecord { - run_id: run_id.to_string(), + run_id: *run_id, created_at, db_prefix, run_dir: run_dir.map(ToOwned::to_owned), }, run_store: Arc::clone(&run_store), }; - runs.insert(run_id.to_string(), catalog); + runs.insert(*run_id, catalog); Ok(run_store as Arc) } - async fn open_run(&self, run_id: &str) -> Result>> { + async fn open_run(&self, run_id: &RunId) -> Result>> { let runs = self.runs.lock().await; Ok(runs .get(run_id) .map(|catalog| Arc::clone(&catalog.run_store) as Arc)) } - async fn open_run_reader(&self, run_id: &str) -> Result>> { + async fn open_run_reader(&self, run_id: &RunId) -> Result>> { self.open_run(run_id).await } @@ -308,7 +308,7 @@ impl Store for InMemoryStore { Ok(summaries) } - async fn delete_run(&self, run_id: &str) -> Result<()> { + async fn delete_run(&self, run_id: &RunId) -> Result<()> { self.runs.lock().await.remove(run_id); Ok(()) } @@ -621,7 +621,7 @@ fn build_run_summary( }); Ok(RunSummary { - run_id: record.run_id.clone(), + run_id: record.run_id, created_at: record.created_at, db_prefix: record.db_prefix.clone(), run_dir: record.run_dir.clone(), @@ -649,13 +649,25 @@ mod tests { use std::time::Duration; use chrono::Duration as ChronoDuration; - use fabro_types::{AttrValue, FabroSettings, Graph, RunStatus, StageStatus, StatusReason}; + use fabro_types::{ + AttrValue, FabroSettings, Graph, RunId, RunStatus, StageStatus, StatusReason, fixtures, + }; fn dt(rfc3339: &str) -> DateTime { DateTime::parse_from_rfc3339(rfc3339) .unwrap() .with_timezone(&Utc) } + fn test_run_id(label: &str) -> RunId { + match label { + "run-1" => fixtures::RUN_1, + "run-early" => fixtures::RUN_2, + "run-late" => fixtures::RUN_3, + "other-run" => fixtures::RUN_4, + _ => panic!("unknown test run id: {label}"), + } + } + fn sample_run_record(run_id: &str, created_at: DateTime) -> RunRecord { let mut graph = Graph::new("night-sky"); graph.attrs.insert( @@ -663,7 +675,7 @@ mod tests { AttrValue::String("map the constellations".to_string()), ); RunRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), created_at, settings: FabroSettings::default(), graph, @@ -677,7 +689,7 @@ mod tests { fn sample_start_record(run_id: &str, created_at: DateTime) -> StartRecord { StartRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), start_time: created_at + ChronoDuration::seconds(5), run_branch: Some("fabro/run/demo".to_string()), base_sha: Some("abc123".to_string()), @@ -732,7 +744,7 @@ mod tests { fn sample_retro(run_id: &str) -> Retro { Retro { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), workflow_name: "night-sky".to_string(), goal: "map the constellations".to_string(), timestamp: dt("2026-03-27T12:20:00Z"), @@ -778,7 +790,10 @@ mod tests { async fn create_run_put_get_and_snapshot_round_trip() { let store = InMemoryStore::default(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); let run_record = sample_run_record("run-1", created_at); let start_record = sample_start_record("run-1", created_at); @@ -899,7 +914,10 @@ mod tests { async fn list_artifact_values_and_all_assets_include_asset_only_visits() { let store = InMemoryStore::default(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); @@ -955,7 +973,7 @@ mod tests { async fn append_event_validates_payload_shape_and_run_id() { let store = InMemoryStore::default(); let run = store - .create_run("run-1", dt("2026-03-27T12:00:00Z"), None) + .create_run(&test_run_id("run-1"), dt("2026-03-27T12:00:00Z"), None) .await .unwrap(); @@ -980,7 +998,10 @@ mod tests { async fn put_run_rejects_created_at_mismatch() { let store = InMemoryStore::default(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); let err = run .put_run(&sample_run_record( "run-1", @@ -995,25 +1016,25 @@ mod tests { async fn watch_events_from_receives_existing_and_live_events() { let store = InMemoryStore::default(); let run = store - .create_run("run-1", dt("2026-03-27T12:00:00Z"), None) + .create_run(&test_run_id("run-1"), dt("2026-03-27T12:00:00Z"), None) .await .unwrap(); let first = EventPayload::new( serde_json::json!({ "ts": "2026-03-27T12:00:00.000Z", - "run_id": "run-1", + "run_id": test_run_id("run-1").to_string(), "event": "WorkflowRunStarted" }), - "run-1", + &test_run_id("run-1"), ) .unwrap(); let second = EventPayload::new( serde_json::json!({ "ts": "2026-03-27T12:00:01.000Z", - "run_id": "run-1", + "run_id": test_run_id("run-1").to_string(), "event": "StageCompleted" }), - "run-1", + &test_run_id("run-1"), ) .unwrap(); @@ -1046,7 +1067,7 @@ mod tests { async fn checkpoint_history_round_trips() { let store = InMemoryStore::default(); let run = store - .create_run("run-1", dt("2026-03-27T12:00:00Z"), None) + .create_run(&test_run_id("run-1"), dt("2026-03-27T12:00:00Z"), None) .await .unwrap(); let checkpoint = sample_checkpoint(); @@ -1065,7 +1086,7 @@ mod tests { async fn node_visit_storage_round_trips() { let store = InMemoryStore::default(); let run = store - .create_run("run-1", dt("2026-03-27T12:00:00Z"), None) + .create_run(&test_run_id("run-1"), dt("2026-03-27T12:00:00Z"), None) .await .unwrap(); @@ -1094,13 +1115,19 @@ mod tests { let early = dt("2026-03-27T10:00:00Z"); let late = dt("2026-03-27T12:00:00Z"); - let early_run = store.create_run("run-early", early, None).await.unwrap(); + let early_run = store + .create_run(&test_run_id("run-early"), early, None) + .await + .unwrap(); early_run .put_run(&sample_run_record("run-early", early)) .await .unwrap(); - let late_run = store.create_run("run-late", late, None).await.unwrap(); + let late_run = store + .create_run(&test_run_id("run-late"), late, None) + .await + .unwrap(); late_run .put_run(&sample_run_record("run-late", late)) .await @@ -1120,7 +1147,7 @@ mod tests { let all = store.list_runs(&ListRunsQuery::default()).await.unwrap(); assert_eq!(all.len(), 2); - assert_eq!(all[0].run_id, "run-late"); + assert_eq!(all[0].run_id, test_run_id("run-late")); assert_eq!(all[0].workflow_name, Some("night-sky".to_string())); assert_eq!(all[0].goal, Some("map the constellations".to_string())); assert_eq!( @@ -1140,19 +1167,25 @@ mod tests { .await .unwrap(); assert_eq!(filtered.len(), 1); - assert_eq!(filtered[0].run_id, "run-late"); + assert_eq!(filtered[0].run_id, test_run_id("run-late")); } #[tokio::test] async fn delete_run_is_idempotent() { let store = InMemoryStore::default(); store - .create_run("run-1", dt("2026-03-27T12:00:00Z"), None) + .create_run(&test_run_id("run-1"), dt("2026-03-27T12:00:00Z"), None) .await .unwrap(); - store.delete_run("run-1").await.unwrap(); - store.delete_run("run-1").await.unwrap(); - assert!(store.open_run("run-1").await.unwrap().is_none()); + store.delete_run(&test_run_id("run-1")).await.unwrap(); + store.delete_run(&test_run_id("run-1")).await.unwrap(); + assert!( + store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .is_none() + ); } #[tokio::test] @@ -1161,14 +1194,23 @@ mod tests { let ts = dt("2026-03-27T12:00:00Z"); // First create succeeds. - store.create_run("run-1", ts, None).await.unwrap(); + store + .create_run(&test_run_id("run-1"), ts, None) + .await + .unwrap(); // Retry with exact same created_at succeeds (idempotent). - store.create_run("run-1", ts, None).await.unwrap(); + store + .create_run(&test_run_id("run-1"), ts, None) + .await + .unwrap(); // Different created_at for the same run_id is rejected. let different_ts = dt("2026-03-27T12:00:01Z"); - match store.create_run("run-1", different_ts, None).await { + match store + .create_run(&test_run_id("run-1"), different_ts, None) + .await + { Err(StoreError::RunAlreadyExists(_)) => {} // expected Err(other) => panic!("expected RunAlreadyExists, got: {other:?}"), Ok(_) => panic!("expected RunAlreadyExists, but create_run succeeded"), diff --git a/lib/crates/fabro-store/src/slate/catalog.rs b/lib/crates/fabro-store/src/slate/catalog.rs index 51332bfd4..7925b59ff 100644 --- a/lib/crates/fabro-store/src/slate/catalog.rs +++ b/lib/crates/fabro-store/src/slate/catalog.rs @@ -7,17 +7,18 @@ use object_store::ObjectStore; use object_store::path::Path; use crate::{CatalogRecord, ListRunsQuery, Result}; +use fabro_types::RunId; pub(crate) async fn write_catalog( store: Arc, base_prefix: &str, - run_id: &str, + run_id: &RunId, created_at: DateTime, db_prefix: &str, run_dir: Option<&str>, ) -> Result { let record = CatalogRecord { - run_id: run_id.to_string(), + run_id: *run_id, created_at, db_prefix: db_prefix.to_string(), run_dir: run_dir.map(ToOwned::to_owned), @@ -38,7 +39,7 @@ pub(crate) async fn write_catalog( pub(crate) async fn read_locator( store: Arc, base_prefix: &str, - run_id: &str, + run_id: &RunId, ) -> Result> { read_catalog_path(store, by_id_path(base_prefix, run_id)).await } @@ -129,18 +130,18 @@ pub(super) async fn repair_catalog(store: Arc, base_prefix: &st Ok(()) } -pub(crate) fn db_prefix(base_prefix: &str, created_at: DateTime, run_id: &str) -> String { +pub(crate) fn db_prefix(base_prefix: &str, created_at: DateTime, run_id: &RunId) -> String { format!( "{base_prefix}db/{}/{run_id}/", created_at.format("%Y-%m-%d-%H-%M-%S-%3f") ) } -pub(crate) fn by_id_path(base_prefix: &str, run_id: &str) -> Path { +pub(crate) fn by_id_path(base_prefix: &str, run_id: &RunId) -> Path { Path::from(format!("{base_prefix}by-id/{run_id}.json")) } -pub(crate) fn by_start_path(base_prefix: &str, created_at: DateTime, run_id: &str) -> Path { +pub(crate) fn by_start_path(base_prefix: &str, created_at: DateTime, run_id: &RunId) -> Path { Path::from(format!( "{base_prefix}by-start/{}/{run_id}.json", created_at.format("%Y-%m-%d-%H-%M") diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index 9e9545d57..3a19dafa2 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -16,6 +16,7 @@ use tokio::sync::Mutex; use crate::keys; use crate::{CatalogRecord, ListRunsQuery, Result, RunStore, RunSummary, Store, StoreError}; +use fabro_types::RunId; use run_store::{SlateRunStore, SlateRunStoreInner}; #[derive(Clone)] @@ -23,7 +24,7 @@ pub struct SlateStore { object_store: Arc, base_prefix: String, flush_interval: Duration, - active_runs: Arc>>>, + active_runs: Arc>>>, } impl std::fmt::Debug for SlateStore { @@ -81,7 +82,7 @@ impl SlateStore { Ok(items.try_next().await?.is_some()) } - async fn get_active_run(&self, run_id: &str) -> Option { + async fn get_active_run(&self, run_id: &RunId) -> Option { let mut active_runs = self.active_runs.lock().await; let weak = active_runs.get(run_id).cloned()?; if let Some(inner) = weak.upgrade() { @@ -96,10 +97,10 @@ impl SlateStore { self.active_runs .lock() .await - .insert(run_store.record().run_id.clone(), run_store.downgrade()); + .insert(run_store.record().run_id, run_store.downgrade()); } - async fn remove_active_run(&self, run_id: &str) -> Option { + async fn remove_active_run(&self, run_id: &RunId) -> Option { let weak = self.active_runs.lock().await.remove(run_id)?; weak.upgrade().map(SlateRunStore::from_inner) } @@ -173,7 +174,7 @@ impl SlateStore { impl Store for SlateStore { async fn create_run( &self, - run_id: &str, + run_id: &RunId, created_at: DateTime, run_dir: Option<&str>, ) -> Result> { @@ -209,7 +210,7 @@ impl Store for SlateStore { }; let record = CatalogRecord { - run_id: run_id.to_string(), + run_id: *run_id, created_at, db_prefix: db_prefix.clone(), run_dir: run_dir.map(ToOwned::to_owned), @@ -232,7 +233,7 @@ impl Store for SlateStore { Ok(Arc::new(run_store) as Arc) } - async fn open_run(&self, run_id: &str) -> Result>> { + async fn open_run(&self, run_id: &RunId) -> Result>> { let Some(locator) = catalog::read_locator(self.object_store.clone(), &self.base_prefix, run_id).await? else { @@ -245,7 +246,7 @@ impl Store for SlateStore { Ok(Some(Arc::new(run_store) as Arc)) } - async fn open_run_reader(&self, run_id: &str) -> Result>> { + async fn open_run_reader(&self, run_id: &RunId) -> Result>> { let Some(locator) = catalog::read_locator(self.object_store.clone(), &self.base_prefix, run_id).await? else { @@ -291,7 +292,7 @@ impl Store for SlateStore { Ok(summaries) } - async fn delete_run(&self, run_id: &str) -> Result<()> { + async fn delete_run(&self, run_id: &RunId) -> Result<()> { let active = self.remove_active_run(run_id).await; let active_record = active.as_ref().map(SlateRunStore::record); if let Some(active) = &active { @@ -382,8 +383,8 @@ mod tests { use bytes::Bytes; use fabro_types::{ - AttrValue, Checkpoint, Conclusion, FabroSettings, Graph, NodeStatusRecord, RunRecord, - RunStatus, RunStatusRecord, StageStatus, StartRecord, StatusReason, + AttrValue, Checkpoint, Conclusion, FabroSettings, Graph, NodeStatusRecord, RunId, + RunRecord, RunStatus, RunStatusRecord, StageStatus, StartRecord, StatusReason, fixtures, }; use object_store::memory::InMemory; use slatedb::{CloseReason, ErrorKind}; @@ -402,6 +403,14 @@ mod tests { (object_store, store) } + fn test_run_id(label: &str) -> RunId { + match label { + "run-1" => fixtures::RUN_1, + "other-run" => fixtures::RUN_2, + _ => panic!("unknown test run id: {label}"), + } + } + fn sample_run_record(run_id: &str, created_at: DateTime) -> RunRecord { let mut graph = Graph::new("night-sky"); graph.attrs.insert( @@ -409,7 +418,7 @@ mod tests { AttrValue::String("map the constellations".to_string()), ); RunRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), created_at, settings: FabroSettings::default(), graph, @@ -423,7 +432,7 @@ mod tests { fn sample_start_record(run_id: &str, created_at: DateTime) -> StartRecord { StartRecord { - run_id: run_id.to_string(), + run_id: test_run_id(run_id), start_time: created_at + chrono::Duration::seconds(5), run_branch: Some("fabro/run/demo".to_string()), base_sha: Some("abc123".to_string()), @@ -486,10 +495,10 @@ mod tests { EventPayload::new( serde_json::json!({ "ts": ts, - "run_id": run_id, + "run_id": test_run_id(run_id).to_string(), "event": event }), - run_id, + &test_run_id(run_id), ) .unwrap() } @@ -534,7 +543,10 @@ mod tests { async fn create_open_list_and_delete_full_lifecycle() { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await @@ -550,25 +562,35 @@ mod tests { .unwrap(); run.put_conclusion(&sample_conclusion()).await.unwrap(); - let by_id = catalog::by_id_path("runs/", "run-1"); - let by_start = catalog::by_start_path("runs/", created_at, "run-1"); + let by_id = catalog::by_id_path("runs/", &test_run_id("run-1")); + let by_start = catalog::by_start_path("runs/", created_at, &test_run_id("run-1")); assert!(object_exists(object_store.clone(), &by_id).await); assert!(object_exists(object_store.clone(), &by_start).await); let summary = store.list_runs(&ListRunsQuery::default()).await.unwrap(); assert_eq!(summary.len(), 1); - assert_eq!(summary[0].run_id, "run-1"); + assert_eq!(summary[0].run_id, test_run_id("run-1")); assert_eq!(summary[0].workflow_name, Some("night-sky".to_string())); assert_eq!(summary[0].goal, Some("map the constellations".to_string())); assert_eq!(summary[0].status, Some(RunStatus::Succeeded)); assert_eq!(summary[0].status_reason, Some(StatusReason::Completed)); - let reopened = store.open_run("run-1").await.unwrap().unwrap(); + let reopened = store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .unwrap(); let stored = reopened.get_run().await.unwrap().unwrap(); - assert_eq!(stored.run_id, "run-1"); + assert_eq!(stored.run_id, test_run_id("run-1")); - store.delete_run("run-1").await.unwrap(); - assert!(store.open_run("run-1").await.unwrap().is_none()); + store.delete_run(&test_run_id("run-1")).await.unwrap(); + assert!( + store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .is_none() + ); assert!(!object_exists(object_store.clone(), &by_id).await); assert!(!object_exists(object_store.clone(), &by_start).await); assert!(list_paths(object_store, "runs/db").await.is_empty()); @@ -579,9 +601,9 @@ mod tests { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); let record = CatalogRecord { - run_id: "run-1".to_string(), + run_id: test_run_id("run-1"), created_at, - db_prefix: catalog::db_prefix("runs/", created_at, "run-1"), + db_prefix: catalog::db_prefix("runs/", created_at, &test_run_id("run-1")), run_dir: None, }; @@ -596,13 +618,19 @@ mod tests { object_store .put( - &catalog::by_id_path("runs/", "run-1"), + &catalog::by_id_path("runs/", &test_run_id("run-1")), serde_json::to_vec(&record).unwrap().into(), ) .await .unwrap(); - assert!(store.open_run("run-1").await.unwrap().is_some()); + assert!( + store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .is_some() + ); assert!( store .list_runs(&ListRunsQuery::default()) @@ -617,7 +645,7 @@ mod tests { assert!( object_exists( object_store, - &catalog::by_start_path("runs/", created_at, "run-1") + &catalog::by_start_path("runs/", created_at, &test_run_id("run-1")) ) .await ); @@ -627,7 +655,10 @@ mod tests { async fn reopen_recovers_event_and_checkpoint_sequences() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); @@ -640,7 +671,11 @@ mod tests { run.append_checkpoint(&sample_checkpoint()).await.unwrap(); drop(run); - let reopened = store.open_run("run-1").await.unwrap().unwrap(); + let reopened = store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .unwrap(); let next_event = reopened .append_event(&event_payload( "run-1", @@ -662,9 +697,9 @@ mod tests { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); let record = CatalogRecord { - run_id: "run-1".to_string(), + run_id: test_run_id("run-1"), created_at, - db_prefix: catalog::db_prefix("runs/", created_at, "run-1"), + db_prefix: catalog::db_prefix("runs/", created_at, &test_run_id("run-1")), run_dir: None, }; @@ -672,20 +707,26 @@ mod tests { db.close().await.unwrap(); object_store .put( - &catalog::by_id_path("runs/", "run-1"), + &catalog::by_id_path("runs/", &test_run_id("run-1")), serde_json::to_vec(&record).unwrap().into(), ) .await .unwrap(); object_store .put( - &catalog::by_start_path("runs/", created_at, "run-1"), + &catalog::by_start_path("runs/", created_at, &test_run_id("run-1")), serde_json::to_vec(&record).unwrap().into(), ) .await .unwrap(); - assert!(store.open_run("run-1").await.unwrap().is_none()); + assert!( + store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .is_none() + ); assert!( store .list_runs(&ListRunsQuery::default()) @@ -699,11 +740,21 @@ mod tests { async fn create_run_allows_idempotent_retry_and_rejects_conflict() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - store.create_run("run-1", created_at, None).await.unwrap(); - store.create_run("run-1", created_at, None).await.unwrap(); + store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); + store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); let conflict = store - .create_run("run-1", created_at + chrono::Duration::seconds(1), None) + .create_run( + &test_run_id("run-1"), + created_at + chrono::Duration::seconds(1), + None, + ) .await; assert!(matches!(conflict, Err(StoreError::RunAlreadyExists(_)))); } @@ -712,7 +763,10 @@ mod tests { async fn list_runs_and_open_run_reuse_active_handle_without_fencing() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); @@ -720,7 +774,11 @@ mod tests { let listed = store.list_runs(&ListRunsQuery::default()).await.unwrap(); assert_eq!(listed.len(), 1); - let reopened = store.open_run("run-1").await.unwrap().unwrap(); + let reopened = store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .unwrap(); let first_event = run .append_event(&event_payload("run-1", "2026-03-27T12:00:00Z", "Started")) .await @@ -745,7 +803,10 @@ mod tests { async fn watch_events_from_polls_new_events() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); let mut stream = run.watch_events_from(1).await.unwrap(); run.append_event(&event_payload("run-1", "2026-03-27T12:00:00Z", "Started")) @@ -768,9 +829,9 @@ mod tests { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); let record = CatalogRecord { - run_id: "run-1".to_string(), + run_id: test_run_id("run-1"), created_at, - db_prefix: catalog::db_prefix("runs/", created_at, "run-1"), + db_prefix: catalog::db_prefix("runs/", created_at, &test_run_id("run-1")), run_dir: None, }; let db = seed_db(object_store.clone(), &record, true).await; @@ -783,14 +844,14 @@ mod tests { db.close().await.unwrap(); object_store .put( - &catalog::by_start_path("runs/", created_at, "run-1"), + &catalog::by_start_path("runs/", created_at, &test_run_id("run-1")), serde_json::to_vec(&record).unwrap().into(), ) .await .unwrap(); - store.delete_run("run-1").await.unwrap(); - store.delete_run("run-1").await.unwrap(); + store.delete_run(&test_run_id("run-1")).await.unwrap(); + store.delete_run(&test_run_id("run-1")).await.unwrap(); assert!(list_paths(object_store, "runs").await.is_empty()); } @@ -798,19 +859,28 @@ mod tests { async fn delete_run_closes_active_handles() { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); - store.delete_run("run-1").await.unwrap(); + store.delete_run(&test_run_id("run-1")).await.unwrap(); let err = run.put_graph("digraph night_sky {}").await.unwrap_err(); assert!(matches!( err, StoreError::Slate(err) if matches!(err.kind(), ErrorKind::Closed(CloseReason::Clean)) )); - assert!(store.open_run("run-1").await.unwrap().is_none()); + assert!( + store + .open_run(&test_run_id("run-1")) + .await + .unwrap() + .is_none() + ); assert!(list_paths(object_store, "runs").await.is_empty()); } @@ -819,18 +889,21 @@ mod tests { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); let wrong_time = dt("2026-03-27T11:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); - let locator = catalog::read_locator(object_store.clone(), "runs/", "run-1") + let locator = catalog::read_locator(object_store.clone(), "runs/", &test_run_id("run-1")) .await .unwrap() .unwrap(); object_store .put( - &catalog::by_start_path("runs/", wrong_time, "run-1"), + &catalog::by_start_path("runs/", wrong_time, &test_run_id("run-1")), serde_json::to_vec(&locator).unwrap().into(), ) .await @@ -839,7 +912,7 @@ mod tests { store.repair_catalog().await.unwrap(); let paths = list_paths(object_store, "runs/by-start").await; assert_eq!(paths.len(), 1); - assert!(paths[0].contains("2026-03-27-12-00/run-1.json")); + assert!(paths[0].contains(&format!("2026-03-27-12-00/{}.json", test_run_id("run-1")))); } #[tokio::test] @@ -848,12 +921,12 @@ mod tests { let old_created_at = dt("2026-03-27T12:00:00Z"); let new_created_at = dt("2026-03-27T12:00:30Z"); let orphan = CatalogRecord { - run_id: "run-1".to_string(), + run_id: test_run_id("run-1"), created_at: old_created_at, - db_prefix: catalog::db_prefix("runs/", old_created_at, "run-1"), + db_prefix: catalog::db_prefix("runs/", old_created_at, &test_run_id("run-1")), run_dir: None, }; - let new_prefix = catalog::db_prefix("runs/", new_created_at, "run-1"); + let new_prefix = catalog::db_prefix("runs/", new_created_at, &test_run_id("run-1")); assert_ne!(orphan.db_prefix, new_prefix); let db = seed_db(object_store.clone(), &orphan, true).await; @@ -861,12 +934,12 @@ mod tests { db.close().await.unwrap(); let run = store - .create_run("run-1", new_created_at, None) + .create_run(&test_run_id("run-1"), new_created_at, None) .await .unwrap(); assert_eq!(run.get_graph().await.unwrap(), None); - let locator = catalog::read_locator(object_store, "runs/", "run-1") + let locator = catalog::read_locator(object_store, "runs/", &test_run_id("run-1")) .await .unwrap() .unwrap(); @@ -878,7 +951,7 @@ mod tests { async fn create_run_rejects_mismatched_init_for_existing_prefix() { let (object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let db_prefix = catalog::db_prefix("runs/", created_at, "run-1"); + let db_prefix = catalog::db_prefix("runs/", created_at, &test_run_id("run-1")); let db = slatedb::Db::builder(db_prefix.clone(), object_store) .with_settings(slatedb::config::Settings { flush_interval: Some(Duration::from_millis(5)), @@ -888,7 +961,7 @@ mod tests { .await .unwrap(); let mismatched = CatalogRecord { - run_id: "other-run".to_string(), + run_id: test_run_id("other-run"), created_at, db_prefix, run_dir: None, @@ -898,7 +971,10 @@ mod tests { .unwrap(); db.close().await.unwrap(); - let err = match store.create_run("run-1", created_at, None).await { + let err = match store + .create_run(&test_run_id("run-1"), created_at, None) + .await + { Ok(_) => panic!("expected create_run to reject mismatched _init.json"), Err(err) => err, }; @@ -912,7 +988,10 @@ mod tests { async fn slate_run_store_round_trips_node_data_and_assets() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); @@ -944,7 +1023,10 @@ mod tests { async fn slate_run_store_lists_artifact_values_and_asset_only_visits() { let (_object_store, store) = make_store(); let created_at = dt("2026-03-27T12:00:00Z"); - let run = store.create_run("run-1", created_at, None).await.unwrap(); + let run = store + .create_run(&test_run_id("run-1"), created_at, None) + .await + .unwrap(); run.put_run(&sample_run_record("run-1", created_at)) .await .unwrap(); diff --git a/lib/crates/fabro-store/src/slate/run_store.rs b/lib/crates/fabro-store/src/slate/run_store.rs index 1b431dcdd..b7bb07406 100644 --- a/lib/crates/fabro-store/src/slate/run_store.rs +++ b/lib/crates/fabro-store/src/slate/run_store.rs @@ -20,8 +20,8 @@ use crate::{ RunStore, RunSummary, StoreError, }; use fabro_types::{ - Checkpoint, Conclusion, NodeStatusRecord, Retro, RunRecord, RunStatusRecord, SandboxRecord, - StartRecord, + Checkpoint, Conclusion, NodeStatusRecord, Retro, RunId, RunRecord, RunStatusRecord, + SandboxRecord, StartRecord, }; #[derive(Clone)] @@ -30,7 +30,7 @@ pub(crate) struct SlateRunStore { } pub(crate) struct SlateRunStoreInner { - run_id: String, + run_id: RunId, created_at: DateTime, db_prefix: String, run_dir: Option, @@ -92,7 +92,7 @@ impl SlateRunStore { pub(crate) fn record(&self) -> CatalogRecord { CatalogRecord { - run_id: self.inner.run_id.clone(), + run_id: self.inner.run_id, created_at: self.inner.created_at, db_prefix: self.inner.db_prefix.clone(), run_dir: self.inner.run_dir.clone(), @@ -161,7 +161,7 @@ impl SlateRunStore { }); Ok(RunSummary { - run_id: catalog.run_id.clone(), + run_id: catalog.run_id, created_at: catalog.created_at, db_prefix: catalog.db_prefix.clone(), run_dir: catalog.run_dir.clone(), diff --git a/lib/crates/fabro-store/src/types.rs b/lib/crates/fabro-store/src/types.rs index f642e0c2e..9aa89df99 100644 --- a/lib/crates/fabro-store/src/types.rs +++ b/lib/crates/fabro-store/src/types.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use crate::{Result, StoreError}; use fabro_types::{ - Checkpoint, Conclusion, NodeStatusRecord, Retro, RunRecord, RunStatus, RunStatusRecord, + Checkpoint, Conclusion, NodeStatusRecord, Retro, RunId, RunRecord, RunStatus, RunStatusRecord, SandboxRecord, StartRecord, StatusReason, }; @@ -17,7 +17,7 @@ pub struct NodeVisitRef<'a> { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CatalogRecord { - pub run_id: String, + pub run_id: RunId, pub created_at: DateTime, pub db_prefix: String, pub run_dir: Option, @@ -25,7 +25,7 @@ pub struct CatalogRecord { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunSummary { - pub run_id: String, + pub run_id: RunId, pub created_at: DateTime, pub db_prefix: String, pub run_dir: Option, @@ -70,13 +70,13 @@ pub struct NodeSnapshot { pub struct EventPayload(serde_json::Value); impl EventPayload { - pub fn new(value: serde_json::Value, expected_run_id: &str) -> Result { + pub fn new(value: serde_json::Value, expected_run_id: &RunId) -> Result { let payload = Self(value); payload.validate(expected_run_id)?; Ok(payload) } - pub fn validate(&self, expected_run_id: &str) -> Result<()> { + pub fn validate(&self, expected_run_id: &RunId) -> Result<()> { let obj = self.0.as_object().ok_or_else(|| { StoreError::InvalidEvent("event payload must be a JSON object".into()) })?; @@ -93,7 +93,9 @@ impl EventPayload { } match obj.get("run_id") { - Some(serde_json::Value::String(run_id)) if run_id == expected_run_id => Ok(()), + Some(serde_json::Value::String(run_id)) if run_id == &expected_run_id.to_string() => { + Ok(()) + } Some(serde_json::Value::String(run_id)) => Err(StoreError::InvalidEvent(format!( "payload run_id {run_id:?} does not match store run_id {expected_run_id:?}" ))), diff --git a/lib/crates/fabro-types/Cargo.toml b/lib/crates/fabro-types/Cargo.toml index 5bb5d0227..5b134afef 100644 --- a/lib/crates/fabro-types/Cargo.toml +++ b/lib/crates/fabro-types/Cargo.toml @@ -23,3 +23,4 @@ clap = { workspace = true, optional = true } fabro-macros = { path = "../fabro-macros" } serde.workspace = true serde_json.workspace = true +ulid.workspace = true diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 3257b8bec..ebde3eeaf 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -9,6 +9,7 @@ pub mod node_status; pub mod outcome; pub mod retro; pub mod run; +pub mod run_id; pub mod sandbox_record; pub mod settings; pub mod start; @@ -26,6 +27,8 @@ pub use retro::{ OpenItemKind, Retro, RetroNarrative, SmoothnessRating, StageRetro, }; pub use run::RunRecord; +pub use run_id::RunId; +pub use run_id::fixtures; pub use sandbox_record::SandboxRecord; pub use settings::FabroSettings; pub use start::StartRecord; diff --git a/lib/crates/fabro-types/src/retro.rs b/lib/crates/fabro-types/src/retro.rs index be7d4d766..92c93d1d2 100644 --- a/lib/crates/fabro-types/src/retro.rs +++ b/lib/crates/fabro-types/src/retro.rs @@ -3,6 +3,8 @@ use std::fmt; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use crate::run_id::RunId; + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum SmoothnessRating { @@ -118,7 +120,7 @@ pub struct RetroNarrative { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Retro { - pub run_id: String, + pub run_id: RunId, pub workflow_name: String, pub goal: String, pub timestamp: DateTime, diff --git a/lib/crates/fabro-types/src/run.rs b/lib/crates/fabro-types/src/run.rs index 71efb97d4..c465c1bd3 100644 --- a/lib/crates/fabro-types/src/run.rs +++ b/lib/crates/fabro-types/src/run.rs @@ -5,11 +5,12 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use crate::graph::Graph; +use crate::run_id::RunId; use crate::settings::FabroSettings; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct RunRecord { - pub run_id: String, + pub run_id: RunId, pub created_at: DateTime, pub settings: FabroSettings, pub graph: Graph, diff --git a/lib/crates/fabro-types/src/run_id.rs b/lib/crates/fabro-types/src/run_id.rs new file mode 100644 index 000000000..13f54a5aa --- /dev/null +++ b/lib/crates/fabro-types/src/run_id.rs @@ -0,0 +1,175 @@ +use std::fmt; +use std::str::FromStr; + +use serde::de::Error as _; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use ulid::Ulid; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct RunId(Ulid); + +impl RunId { + pub fn new() -> Self { + Self(Ulid::new()) + } +} + +impl Default for RunId { + fn default() -> Self { + Self::new() + } +} + +impl fmt::Display for RunId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for RunId { + type Err = ulid::DecodeError; + + fn from_str(s: &str) -> Result { + Ok(Self(Ulid::from_str(s)?)) + } +} + +impl From for RunId { + fn from(value: Ulid) -> Self { + Self(value) + } +} + +impl From for Ulid { + fn from(value: RunId) -> Self { + value.0 + } +} + +impl From for String { + fn from(value: RunId) -> Self { + value.to_string() + } +} + +impl From<&RunId> for String { + fn from(value: &RunId) -> Self { + value.to_string() + } +} + +impl Serialize for RunId { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} + +impl<'de> Deserialize<'de> for RunId { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = String::deserialize(deserializer)?; + value.parse().map_err(D::Error::custom) + } +} + +pub mod fixtures { + use super::RunId; + use ulid::Ulid; + + macro_rules! define_run_ids { + ($($name:ident = $value:expr),+ $(,)?) => { + $(pub const $name: RunId = RunId(Ulid($value));)+ + }; + } + + define_run_ids!( + RUN_1 = 1, + RUN_2 = 2, + RUN_3 = 3, + RUN_4 = 4, + RUN_5 = 5, + RUN_6 = 6, + RUN_7 = 7, + RUN_8 = 8, + RUN_9 = 9, + RUN_10 = 10, + RUN_11 = 11, + RUN_12 = 12, + RUN_13 = 13, + RUN_14 = 14, + RUN_15 = 15, + RUN_16 = 16, + RUN_17 = 17, + RUN_18 = 18, + RUN_19 = 19, + RUN_20 = 20, + RUN_21 = 21, + RUN_22 = 22, + RUN_23 = 23, + RUN_24 = 24, + RUN_25 = 25, + RUN_26 = 26, + RUN_27 = 27, + RUN_28 = 28, + RUN_29 = 29, + RUN_30 = 30, + RUN_31 = 31, + RUN_32 = 32, + RUN_33 = 33, + RUN_34 = 34, + RUN_35 = 35, + RUN_36 = 36, + RUN_37 = 37, + RUN_38 = 38, + RUN_39 = 39, + RUN_40 = 40, + RUN_41 = 41, + RUN_42 = 42, + RUN_43 = 43, + RUN_44 = 44, + RUN_45 = 45, + RUN_46 = 46, + RUN_47 = 47, + RUN_48 = 48, + RUN_49 = 49, + RUN_50 = 50, + RUN_51 = 51, + RUN_52 = 52, + RUN_53 = 53, + RUN_54 = 54, + RUN_55 = 55, + RUN_56 = 56, + RUN_57 = 57, + RUN_58 = 58, + RUN_59 = 59, + RUN_60 = 60, + RUN_61 = 61, + RUN_62 = 62, + RUN_63 = 63, + RUN_64 = 64 + ); +} + +#[cfg(test)] +mod tests { + use super::{RunId, fixtures}; + + #[test] + fn serializes_as_a_ulid_string() { + let value = serde_json::to_value(fixtures::RUN_1).unwrap(); + assert_eq!(value, serde_json::json!("00000000000000000000000001")); + } + + #[test] + fn deserializes_from_a_ulid_string() { + let value: RunId = + serde_json::from_value(serde_json::json!("0000000000000000000000001A")).unwrap(); + + assert_eq!(value, fixtures::RUN_42); + } +} diff --git a/lib/crates/fabro-types/src/start.rs b/lib/crates/fabro-types/src/start.rs index f2b221f4c..6a78addd6 100644 --- a/lib/crates/fabro-types/src/start.rs +++ b/lib/crates/fabro-types/src/start.rs @@ -1,9 +1,11 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use crate::run_id::RunId; + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct StartRecord { - pub run_id: String, + pub run_id: RunId, pub start_time: DateTime, #[serde(default, skip_serializing_if = "Option::is_none")] pub run_branch: Option, diff --git a/lib/crates/fabro-workflows/src/event.rs b/lib/crates/fabro-workflows/src/event.rs index bc8430ef7..7f1c5495c 100644 --- a/lib/crates/fabro-workflows/src/event.rs +++ b/lib/crates/fabro-workflows/src/event.rs @@ -6,6 +6,7 @@ use std::sync::atomic::{AtomicI64, Ordering}; use anyhow::{Context, Result}; use chrono::{SecondsFormat, Utc}; use fabro_store::{EventPayload, RunStore}; +use fabro_types::RunId; use serde::{Deserialize, Serialize}; use tokio::sync::{mpsc, oneshot}; @@ -28,7 +29,7 @@ pub enum RunNoticeLevel { pub enum WorkflowRunEvent { WorkflowRunStarted { name: String, - run_id: String, + run_id: RunId, #[serde(default, skip_serializing_if = "Option::is_none")] base_branch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -344,7 +345,7 @@ impl WorkflowRunEvent { use tracing::{debug, error, info, warn}; match self { Self::WorkflowRunStarted { name, run_id, .. } => { - info!(workflow = name.as_str(), run_id, "Workflow run started"); + info!(workflow = name.as_str(), run_id = %run_id, "Workflow run started"); } Self::WorkflowRunCompleted { duration_ms, @@ -804,7 +805,7 @@ pub fn flatten_event( (event_name, fields) } -pub fn build_event_envelope(event: &WorkflowRunEvent, run_id: &str) -> serde_json::Value { +pub fn build_event_envelope(event: &WorkflowRunEvent, run_id: &RunId) -> serde_json::Value { let (event_name, event_fields) = flatten_event(event); let mut envelope = serde_json::Map::new(); envelope.insert( @@ -826,7 +827,7 @@ pub fn build_event_envelope(event: &WorkflowRunEvent, run_id: &str) -> serde_jso pub fn build_redacted_event_payload( event: &WorkflowRunEvent, - run_id: &str, + run_id: &RunId, ) -> Result { let envelope = build_event_envelope(event, run_id); let line = serde_json::to_string(&envelope)?; @@ -835,7 +836,11 @@ pub fn build_redacted_event_payload( EventPayload::new(value, run_id).map_err(anyhow::Error::from) } -pub fn append_progress_event(run_dir: &Path, run_id: &str, event: &WorkflowRunEvent) -> Result<()> { +pub fn append_progress_event( + run_dir: &Path, + run_id: &RunId, + event: &WorkflowRunEvent, +) -> Result<()> { let envelope = build_event_envelope(event, run_id); let line = serde_json::to_string(&envelope)?; let line = redact_jsonl_line(&line); @@ -861,15 +866,15 @@ pub fn append_progress_event(run_dir: &Path, run_id: &str, event: &WorkflowRunEv pub struct ProgressLogger { run_dir: PathBuf, - run_id: String, + run_id: RunId, } impl ProgressLogger { #[must_use] - pub fn new(run_dir: impl Into, run_id: impl Into) -> Self { + pub fn new(run_dir: impl Into, run_id: RunId) -> Self { Self { run_dir: run_dir.into(), - run_id: run_id.into(), + run_id, } } @@ -897,12 +902,12 @@ enum StoreProgressCommand { #[derive(Clone)] pub struct StoreProgressLogger { tx: mpsc::UnboundedSender, - run_id: Arc>, + run_id: Arc>, } impl StoreProgressLogger { #[must_use] - pub fn new(run_store: Arc, run_id: impl Into) -> Self { + pub fn new(run_store: Arc, run_id: RunId) -> Self { let (tx, mut rx) = mpsc::unbounded_channel(); tokio::spawn(async move { @@ -922,7 +927,7 @@ impl StoreProgressLogger { Self { tx, - run_id: Arc::new(std::sync::Mutex::new(run_id.into())), + run_id: Arc::new(std::sync::Mutex::new(run_id)), } } @@ -1291,6 +1296,7 @@ impl EventEmitter { mod tests { use super::*; use fabro_llm::types::Usage; + use fabro_types::fixtures; use std::sync::{Arc, Mutex}; #[test] @@ -1313,7 +1319,7 @@ mod tests { }); emitter.emit(&WorkflowRunEvent::WorkflowRunStarted { name: "test".to_string(), - run_id: "1".to_string(), + run_id: fixtures::RUN_1, base_branch: None, base_sha: None, run_branch: None, @@ -1814,7 +1820,7 @@ mod tests { assert_eq!(emitter.last_event_at(), 0); emitter.emit(&WorkflowRunEvent::WorkflowRunStarted { name: "test".to_string(), - run_id: "1".to_string(), + run_id: fixtures::RUN_1, base_branch: None, base_sha: None, run_branch: None, @@ -2023,7 +2029,7 @@ mod tests { fn rename_fields_workflow_run_started() { let event = WorkflowRunEvent::WorkflowRunStarted { name: "my_pipeline".to_string(), - run_id: "r1".to_string(), + run_id: fixtures::RUN_1, base_branch: None, base_sha: None, run_branch: None, @@ -2669,7 +2675,7 @@ mod tests { fn workflow_run_started_with_goal_round_trip() { let event = WorkflowRunEvent::WorkflowRunStarted { name: "my_workflow".to_string(), - run_id: "r42".to_string(), + run_id: fixtures::RUN_42, base_branch: None, base_sha: None, run_branch: None, @@ -2687,7 +2693,8 @@ mod tests { #[test] fn workflow_run_started_without_goal_backward_compat() { // Old JSONL without `goal` field should deserialize to `goal: None` - let json = r#"{"WorkflowRunStarted":{"name":"old_wf","run_id":"r1"}}"#; + let json = + r#"{"WorkflowRunStarted":{"name":"old_wf","run_id":"00000000000000000000000001"}}"#; let deserialized: WorkflowRunEvent = serde_json::from_str(json).unwrap(); assert!(matches!( deserialized, @@ -2699,7 +2706,7 @@ mod tests { fn workflow_run_started_goal_none_omitted_from_json() { let event = WorkflowRunEvent::WorkflowRunStarted { name: "wf".to_string(), - run_id: "r1".to_string(), + run_id: fixtures::RUN_1, base_branch: None, base_sha: None, run_branch: None, diff --git a/lib/crates/fabro-workflows/src/git.rs b/lib/crates/fabro-workflows/src/git.rs index cbf6fc5b1..6d0134746 100644 --- a/lib/crates/fabro-workflows/src/git.rs +++ b/lib/crates/fabro-workflows/src/git.rs @@ -558,6 +558,7 @@ impl MetadataStore { #[cfg(test)] mod tests { use super::*; + use fabro_types::fixtures; use std::fs; use crate::records::{CheckpointExt, RunRecordExt}; @@ -654,13 +655,18 @@ mod tests { init_repo(dir.path()); let store = MetadataStore::new(dir.path(), &GitAuthor::default()); - let run_record = br#"{"run_id":"RUN1","created_at":"2025-01-01T00:00:00Z","settings":{},"graph":{"name":"test","nodes":{},"edges":[],"attrs":{}},"working_directory":"/tmp"}"#; - store.init_run("RUN1", &[("run.json", run_record)]).unwrap(); + let run_id = fixtures::RUN_1.to_string(); + let run_record = format!( + r#"{{"run_id":"{run_id}","created_at":"2025-01-01T00:00:00Z","settings":{{}},"graph":{{"name":"test","nodes":{{}},"edges":[],"attrs":{{}}}},"working_directory":"/tmp"}}"# + ); + store + .init_run(&run_id, &[("run.json", run_record.as_bytes())]) + .unwrap(); - let read_record = MetadataStore::read_run_record(dir.path(), "RUN1") + let read_record = MetadataStore::read_run_record(dir.path(), &run_id) .unwrap() .unwrap(); - assert_eq!(read_record.run_id, "RUN1"); + assert_eq!(read_record.run_id, fixtures::RUN_1); assert_eq!(read_record.workflow_name(), "test"); } @@ -834,27 +840,32 @@ mod tests { init_repo(dir.path()); let store = MetadataStore::new(dir.path(), &GitAuthor::default()); - let run_record = br#"{"run_id":"RUN5","created_at":"2025-01-01T00:00:00Z","settings":{},"graph":{"name":"test","nodes":{},"edges":[],"attrs":{}},"working_directory":"/tmp"}"#; - store.init_run("RUN5", &[("run.json", run_record)]).unwrap(); + let run_id = fixtures::RUN_5.to_string(); + let run_record = format!( + r#"{{"run_id":"{run_id}","created_at":"2025-01-01T00:00:00Z","settings":{{}},"graph":{{"name":"test","nodes":{{}},"edges":[],"attrs":{{}}}},"working_directory":"/tmp"}}"# + ); + store + .init_run(&run_id, &[("run.json", run_record.as_bytes())]) + .unwrap(); store .write_files( - "RUN5", + &run_id, &[("retro.json", b"{\"status\":\"ok\"}")], "finalize", ) .unwrap(); - let data = MetadataStore::read_file(dir.path(), "RUN5", "retro.json") + let data = MetadataStore::read_file(dir.path(), &run_id, "retro.json") .unwrap() .unwrap(); assert_eq!(data, b"{\"status\":\"ok\"}"); // Original files still present - let record = MetadataStore::read_run_record(dir.path(), "RUN5") + let record = MetadataStore::read_run_record(dir.path(), &run_id) .unwrap() .unwrap(); - assert_eq!(record.run_id, "RUN5"); + assert_eq!(record.run_id, fixtures::RUN_5); } #[test] diff --git a/lib/crates/fabro-workflows/src/handler/agent.rs b/lib/crates/fabro-workflows/src/handler/agent.rs index 646e9af37..9bc6f9c94 100644 --- a/lib/crates/fabro-workflows/src/handler/agent.rs +++ b/lib/crates/fabro-workflows/src/handler/agent.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use async_trait::async_trait; use fabro_agent::Sandbox; use fabro_store::NodeVisitRef; +use fabro_types::RunId; use crate::context::keys; use crate::context::{Context, WorkflowContext}; @@ -271,12 +272,16 @@ impl Handler for AgentHandler { // 3. Call LLM backend (agent loop) let thread_id = context.thread_id(); + let run_id = context + .run_id() + .parse::() + .map_err(|err| FabroError::handler(format!("invalid internal run_id: {err}")))?; let tool_hooks: Option> = services.hook_runner.as_ref().map(|hr| { Arc::new(fabro_hooks::WorkflowToolHookCallback { hook_runner: Arc::clone(hr), sandbox: Arc::clone(&services.sandbox), - run_id: context.run_id(), + run_id, workflow_name: graph.name.clone(), work_dir: None, node_id: node.id.clone(), @@ -397,17 +402,27 @@ mod tests { use super::*; use crate::event::EventEmitter; use fabro_graphviz::graph::AttrValue; + use fabro_types::fixtures; use tempfile::TempDir; fn make_services() -> EngineServices { EngineServices::test_default() } + fn test_context() -> Context { + let context = Context::new(); + context.set( + crate::context::keys::INTERNAL_RUN_ID, + serde_json::json!(fixtures::RUN_1.to_string()), + ); + context + } + #[tokio::test] async fn codergen_handler_simulate() { let handler = AgentHandler::new(None); let node = Node::new("plan"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -436,7 +451,7 @@ mod tests { "prompt".to_string(), AttrValue::String("Achieve: $goal".to_string()), ); - let context = Context::new(); + let context = test_context(); let mut graph = Graph::new("test"); graph.attrs.insert( "goal".to_string(), @@ -463,7 +478,7 @@ mod tests { "label".to_string(), AttrValue::String("Do work".to_string()), ); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -482,7 +497,7 @@ mod tests { async fn codergen_handler_context_updates() { let handler = AgentHandler::new(None); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -515,7 +530,7 @@ mod tests { let handler = AgentHandler::new(None); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -573,7 +588,7 @@ mod tests { let handler = AgentHandler::new(Some(Box::new(DirectiveBackend))); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -632,7 +647,7 @@ mod tests { let handler = AgentHandler::new(Some(Box::new(LastFileBackend))); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -737,7 +752,7 @@ mod tests { let handler = AgentHandler::new(Some(Box::new(backend))); let node = Node::new("work"); - let context = Context::new(); + let context = test_context(); // Simulate what the engine stores in internal.thread_id context.set(keys::INTERNAL_THREAD_ID, serde_json::json!("main")); let graph = Graph::new("test"); @@ -790,7 +805,7 @@ mod tests { let handler = AgentHandler::new(Some(Box::new(backend))); let node = Node::new("work"); - let context = Context::new(); + let context = test_context(); // No thread context set let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -827,7 +842,7 @@ mod tests { let handler = AgentHandler::new(Some(Box::new(FailingBackend))); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -971,7 +986,7 @@ Some text in between. let handler = AgentHandler::new(Some(Box::new(ValidationFailBackend))); let node = Node::new("step"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -1025,7 +1040,7 @@ Some text in between. "prompt".to_string(), AttrValue::String("Summarize the results".to_string()), ); - let context = Context::new(); + let context = test_context(); context.set( keys::CURRENT_PREAMBLE, serde_json::json!("## Test Output\n10 passed, 0 failed"), @@ -1095,7 +1110,7 @@ Some text in between. "prompt".to_string(), AttrValue::String("Summarize the results".to_string()), ); - let context = Context::new(); + let context = test_context(); // No preamble set -- context.get_string returns "" let graph = Graph::new("test"); let tmp = TempDir::new().unwrap(); @@ -1117,7 +1132,7 @@ Some text in between. "prompt".to_string(), AttrValue::String("Summarize".to_string()), ); - let context = Context::new(); + let context = test_context(); context.set( keys::CURRENT_PREAMBLE, serde_json::json!("## Script Output\nAll tests passed"), diff --git a/lib/crates/fabro-workflows/src/handler/manager_loop.rs b/lib/crates/fabro-workflows/src/handler/manager_loop.rs index 4b3edf589..29aba758f 100644 --- a/lib/crates/fabro-workflows/src/handler/manager_loop.rs +++ b/lib/crates/fabro-workflows/src/handler/manager_loop.rs @@ -8,6 +8,7 @@ use async_trait::async_trait; use chrono::Utc; use fabro_config::FabroSettings; use fabro_store::{InMemoryStore, Store}; +use fabro_types::RunId; use crate::condition::evaluate_condition; use crate::context::keys; @@ -158,7 +159,6 @@ impl Handler for SubWorkflowHandler { let child_logs = run_dir.join(format!("nodes/{}_{visit}/child", node.id)); let _ = std::fs::create_dir_all(&child_logs); - let parent_run_id = context.run_id(); let cancel_token = Arc::new(AtomicBool::new(false)); let child_cancel = Arc::clone(&cancel_token); @@ -166,7 +166,7 @@ impl Handler for SubWorkflowHandler { settings: fabro_config::FabroSettings::default(), run_dir: child_logs, cancel_token: Some(cancel_token), - run_id: format!("{parent_run_id}_child_{}", node.id), + run_id: RunId::new(), labels: HashMap::new(), workflow_slug: None, github_app: None, diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs index 07b824385..8627564ba 100644 --- a/lib/crates/fabro-workflows/src/handler/parallel.rs +++ b/lib/crates/fabro-workflows/src/handler/parallel.rs @@ -4,6 +4,7 @@ use std::time::Instant; use async_trait::async_trait; use fabro_agent::{Sandbox, WorktreeConfig, WorktreeSandbox}; +use fabro_types::RunId; use tokio::sync::Semaphore; use crate::context::keys; @@ -155,11 +156,12 @@ impl Handler for ParallelHandler { join_policy: join_policy.to_string(), }); { - let mut hook_ctx = HookContext::new( - HookEvent::ParallelStart, - context.run_id(), - graph.name.clone(), - ); + let run_id = context + .run_id() + .parse::() + .map_err(|err| FabroError::handler(format!("invalid internal run_id: {err}")))?; + let mut hook_ctx = + HookContext::new(HookEvent::ParallelStart, run_id, graph.name.clone()); set_hook_node(&mut hook_ctx, node); let _ = services.run_hooks(&hook_ctx).await; } @@ -177,7 +179,7 @@ impl Handler for ParallelHandler { let base_sha: Option = if let Some(ref gs) = git_state { let result = git_checkpoint( &*services.sandbox, - &gs.run_id, + &gs.run_id.to_string(), &node.id, "parallel_base", 0, @@ -219,9 +221,12 @@ impl Handler for ParallelHandler { ); // Compute worktree path (each sandbox type knows its own path scheme) - let wt_path_str = services - .sandbox - .parallel_worktree_path(run_dir, &gs.run_id, &node.id, branch_key); + let wt_path_str = services.sandbox.parallel_worktree_path( + run_dir, + &gs.run_id.to_string(), + &node.id, + branch_key, + ); tracing::debug!(branch = %branch_name, path = %wt_path_str, "Creating worktree for parallel branch"); // Set up worktree via WorktreeSandbox @@ -329,7 +334,9 @@ impl Handler for ParallelHandler { // Checkpoint commit after branch execution (capture head_sha) let head_sha = if has_git { - let rid = run_id.as_deref().unwrap_or("unknown"); + let rid = run_id + .map(|run_id| run_id.to_string()) + .unwrap_or_else(|| "unknown".to_string()); let nid = &setup.target_id; let status_str = outcome.status.to_string(); // Use exec_command to commit and capture HEAD in the branch worktree @@ -482,11 +489,12 @@ impl Handler for ParallelHandler { failure_count: fail_count, }); { - let mut hook_ctx = HookContext::new( - HookEvent::ParallelComplete, - context.run_id(), - graph.name.clone(), - ); + let run_id = context + .run_id() + .parse::() + .map_err(|err| FabroError::handler(format!("invalid internal run_id: {err}")))?; + let mut hook_ctx = + HookContext::new(HookEvent::ParallelComplete, run_id, graph.name.clone()); set_hook_node(&mut hook_ctx, node); let _ = services.run_hooks(&hook_ctx).await; } @@ -574,16 +582,26 @@ fn find_join_node(results: &[BranchResult], graph: &Graph) -> Option { mod tests { use super::*; use fabro_graphviz::graph::{AttrValue, Edge}; + use fabro_types::fixtures; fn make_services() -> EngineServices { EngineServices::test_default() } + fn test_context() -> Context { + let context = Context::new(); + context.set( + crate::context::keys::INTERNAL_RUN_ID, + serde_json::json!(fixtures::RUN_1.to_string()), + ); + context + } + #[tokio::test] async fn parallel_handler_no_branches() { let services = make_services(); let node = Node::new("par"); - let context = Context::new(); + let context = test_context(); let graph = Graph::new("test"); let run_dir = Path::new("/tmp/test"); @@ -602,7 +620,7 @@ mod tests { "shape".to_string(), AttrValue::String("component".to_string()), ); - let context = Context::new(); + let context = test_context(); let mut graph = Graph::new("test"); graph.nodes.insert("par".to_string(), node.clone()); graph @@ -654,7 +672,7 @@ mod tests { "join_policy".to_string(), AttrValue::String("first_success".to_string()), ); - let context = Context::new(); + let context = test_context(); let mut graph = Graph::new("test"); graph.nodes.insert("par".to_string(), node.clone()); graph @@ -696,7 +714,7 @@ mod tests { "shape".to_string(), AttrValue::String("component".to_string()), ); - let context = Context::new(); + let context = test_context(); let mut graph = Graph::new("test"); graph.nodes.insert("par".to_string(), node.clone()); graph diff --git a/lib/crates/fabro-workflows/src/hook_context.rs b/lib/crates/fabro-workflows/src/hook_context.rs index 2fdc1f6ae..a0c7baa08 100644 --- a/lib/crates/fabro-workflows/src/hook_context.rs +++ b/lib/crates/fabro-workflows/src/hook_context.rs @@ -12,6 +12,7 @@ pub(crate) fn set_hook_node(ctx: &mut HookContext, node: &GvNode) { mod tests { use fabro_graphviz::graph::{AttrValue, Node}; use fabro_hooks::HookEvent; + use fabro_types::fixtures; use super::*; @@ -25,7 +26,7 @@ mod tests { node.attrs .insert("type".to_string(), AttrValue::String("human".to_string())); - let mut ctx = HookContext::new(HookEvent::StageStart, "run-1".into(), "graph".into()); + let mut ctx = HookContext::new(HookEvent::StageStart, fixtures::RUN_1, "graph".into()); set_hook_node(&mut ctx, &node); assert_eq!(ctx.node_id.as_deref(), Some("approve")); diff --git a/lib/crates/fabro-workflows/src/lifecycle/disk.rs b/lib/crates/fabro-workflows/src/lifecycle/disk.rs index 0b8f50d99..6b6f7a947 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/disk.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/disk.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use async_trait::async_trait; use fabro_store::{NodeVisitRef, RunStore}; -use fabro_types::NodeStatusRecord; +use fabro_types::{NodeStatusRecord, RunId}; use fabro_core::error::Result as CoreResult; use fabro_core::graph::NodeSpec; @@ -27,7 +27,7 @@ type WfNodeResult = NodeResult>; /// Sub-lifecycle responsible for writing run state to disk (node status, checkpoints). pub(crate) struct DiskLifecycle { pub run_dir: PathBuf, - pub run_id: String, + pub run_id: RunId, pub run_store: Arc, pub graph: Arc, pub run_options: Arc, @@ -41,7 +41,7 @@ impl RunLifecycle for DiskLifecycle { async fn on_run_start(&self, _graph: &WorkflowGraph, _state: &WfRunState) -> CoreResult<()> { let git_state = self.run_options.git.as_ref(); let start_record = StartRecord { - run_id: self.run_id.clone(), + run_id: self.run_id, start_time: chrono::Utc::now(), run_branch: git_state.and_then(|g| g.run_branch.clone()), base_sha: git_state.and_then(|g| g.base_sha.clone()), diff --git a/lib/crates/fabro-workflows/src/lifecycle/event.rs b/lib/crates/fabro-workflows/src/lifecycle/event.rs index 363527d51..2eaa003e6 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/event.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/event.rs @@ -21,6 +21,7 @@ use crate::outcome::{ FailureCategory, FailureDetail, Outcome, StageStatus, StageUsage, stage_usage_to_llm, }; use fabro_graphviz::graph::types::Node as GvNode; +use fabro_types::RunId; type WfRunState = RunState>; type WfNodeResult = NodeResult>; @@ -37,7 +38,7 @@ fn node_script(node: &GvNode) -> Option { pub(crate) struct EventLifecycle { pub emitter: Arc, pub graph_name: String, - pub run_id: String, + pub run_id: RunId, pub run_start: Mutex, /// Set in on_edge_selected when loop_restart approved; emitted+cleared in on_run_start. pub restarted_from: Arc>>, diff --git a/lib/crates/fabro-workflows/src/lifecycle/git.rs b/lib/crates/fabro-workflows/src/lifecycle/git.rs index 5fb87a146..b0285bd8b 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/git.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use fabro_store::RunStore; +use fabro_types::RunId; use fabro_core::error::{CoreError, Result as CoreResult}; use fabro_core::graph::NodeSpec; @@ -37,7 +38,7 @@ pub(crate) struct GitLifecycle { pub artifact_store: Arc>, pub emitter: Arc, pub run_dir: PathBuf, - pub run_id: String, + pub run_id: RunId, pub run_store: Arc, pub run_options: Arc, pub start_node_id: Option, @@ -97,7 +98,7 @@ impl RunLifecycle for GitLifecycle { if let Some(ref data) = sandbox_json { files.push(("sandbox.json", data)); } - if let Err(e) = store.init_run(&self.run_id, &files) { + if let Err(e) = store.init_run(&self.run_id.to_string(), &files) { tracing::warn!( run_id = %self.run_id, error = %e, @@ -160,7 +161,7 @@ impl RunLifecycle for GitLifecycle { .iter() .map(|(k, v)| (k.as_str(), v.as_slice())) .collect(); - match store.write_checkpoint(&self.run_id, &cp_json, &extra_refs) { + match store.write_checkpoint(&self.run_id.to_string(), &cp_json, &extra_refs) { Ok(sha) => Some(sha), Err(e) => { self.emitter.emit(&WorkflowRunEvent::RunNotice { @@ -183,7 +184,7 @@ impl RunLifecycle for GitLifecycle { let git_author = self.run_options.git_author(); let commit_result = git_checkpoint( &*self.sandbox, - &self.run_id, + &self.run_id.to_string(), node_id, &result.outcome.status.to_string(), completed_count, diff --git a/lib/crates/fabro-workflows/src/lifecycle/hook.rs b/lib/crates/fabro-workflows/src/lifecycle/hook.rs index 09f809ac3..e8372c89e 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/hook.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/hook.rs @@ -16,6 +16,7 @@ use crate::hook_context::set_hook_node; use crate::outcome::{Outcome, OutcomeExt, StageStatus, StageUsage}; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_sandbox::Sandbox; +use fabro_types::RunId; type WfRunState = RunState>; type WfNodeResult = NodeResult>; @@ -26,7 +27,7 @@ pub(crate) struct HookLifecycle { pub hook_runner: Option>, pub sandbox: Arc, pub hook_work_dir: Option, - pub run_id: String, + pub run_id: RunId, pub graph_name: String, } diff --git a/lib/crates/fabro-workflows/src/lifecycle/mod.rs b/lib/crates/fabro-workflows/src/lifecycle/mod.rs index 61a57365a..0ecfcbf30 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/mod.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/mod.rs @@ -16,6 +16,7 @@ use std::time::Instant; use async_trait::async_trait; use fabro_store::RunStore; use fabro_store::RuntimeState; +use fabro_types::RunId; use fabro_core::error::Result as CoreResult; use fabro_core::graph::NodeSpec; @@ -72,7 +73,7 @@ pub(crate) struct WorkflowLifecycle { is_initial_resume: AtomicBool, // Config needed for context seeding graph: Arc, - run_id: String, + run_id: RunId, working_directory: Option, } diff --git a/lib/crates/fabro-workflows/src/operations/create.rs b/lib/crates/fabro-workflows/src/operations/create.rs index dafb3ef55..0ffd23653 100644 --- a/lib/crates/fabro-workflows/src/operations/create.rs +++ b/lib/crates/fabro-workflows/src/operations/create.rs @@ -6,6 +6,7 @@ use fabro_config::{FabroSettings, FabroSettingsExt}; use fabro_graphviz::graph::{AttrValue, Graph}; use fabro_model::{Catalog, Provider}; use fabro_sandbox::SandboxProvider; +use fabro_types::RunId; use crate::error::FabroError; use crate::pipeline::types::PersistOptions; @@ -27,7 +28,7 @@ pub struct CreateRunInput { pub cwd: PathBuf, pub workflow_slug: Option, pub run_dir: Option, - pub run_id: Option, + pub run_id: Option, pub host_repo_path: Option, pub base_branch: Option, } @@ -35,7 +36,7 @@ pub struct CreateRunInput { #[derive(Debug)] pub struct CreatedRun { pub persisted: Persisted, - pub run_id: String, + pub run_id: RunId, pub run_dir: PathBuf, pub dot_path: Option, } @@ -43,7 +44,7 @@ pub struct CreatedRun { struct PersistCreateOptions { settings: FabroSettings, run_dir: Option, - run_id: Option, + run_id: Option, workflow_slug: Option, labels: HashMap, base_branch: Option, @@ -76,12 +77,12 @@ pub fn create(request: CreateRunInput) -> Result { } = request; let settings = resolved.settings.clone(); - let run_id = run_id.unwrap_or_else(|| ulid::Ulid::new().to_string()); + let run_id = run_id.unwrap_or_else(RunId::new); let storage_dir = settings.storage_dir(); let run_dir = run_dir.unwrap_or_else(|| { make_run_dir( &storage_dir.join("runs"), - &run_id, + &run_id.to_string(), settings.dry_run_enabled(), ) }); @@ -102,7 +103,7 @@ pub fn create(request: CreateRunInput) -> Result { PersistCreateOptions { settings, run_dir: Some(run_dir.clone()), - run_id: Some(run_id.clone()), + run_id: Some(run_id), workflow_slug: workflow_slug.or(resolved.workflow_slug.clone()), labels: resolved.settings.labels.clone(), base_branch, @@ -230,8 +231,9 @@ fn persist_validated( let settings = resolve_run_settings(settings, validated.graph()); - let run_id = run_id.unwrap_or_else(|| ulid::Ulid::new().to_string()); - let run_dir = run_dir.unwrap_or_else(|| default_run_dir(&run_id, settings.dry_run_enabled())); + let run_id = run_id.unwrap_or_else(RunId::new); + let run_dir = + run_dir.unwrap_or_else(|| default_run_dir(&run_id.to_string(), settings.dry_run_enabled())); let run_record = RunRecord { run_id, @@ -319,6 +321,7 @@ pub(crate) fn make_run_dir(runs_base: &Path, run_id: &str, dry_run: bool) -> Pat mod tests { use super::*; use fabro_graphviz::graph::AttrValue; + use fabro_types::fixtures; use crate::operations::{ValidateInput, validate}; use crate::run_status::RunStatusRecordExt; @@ -564,13 +567,13 @@ mod tests { cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), run_dir: Some(dir.path().join("run")), - run_id: Some("run-123".to_string()), + run_id: Some(fixtures::RUN_1), host_repo_path: Some(dir.path().display().to_string()), base_branch: Some("main".to_string()), }) .unwrap(); - assert_eq!(created.run_id, "run-123"); + assert_eq!(created.run_id, fixtures::RUN_1); assert_eq!(created.persisted.run_record().graph.goal(), "override goal"); assert_eq!( created @@ -670,7 +673,7 @@ mod tests { cwd: dir.path().to_path_buf(), workflow_slug: None, run_dir: Some(dir.path().join("run")), - run_id: Some("run-cwd".to_string()), + run_id: Some(fixtures::RUN_2), host_repo_path: None, base_branch: None, }) diff --git a/lib/crates/fabro-workflows/src/operations/fork.rs b/lib/crates/fabro-workflows/src/operations/fork.rs index a7acd72f5..d75f89724 100644 --- a/lib/crates/fabro-workflows/src/operations/fork.rs +++ b/lib/crates/fabro-workflows/src/operations/fork.rs @@ -1,6 +1,7 @@ use anyhow::{Context, Result}; use fabro_git_storage::branchstore::BranchStore; use fabro_git_storage::gitobj::Store; +use fabro_types::RunId; use git2::{Oid, Signature}; use crate::git::{MetadataStore, RUN_BRANCH_PREFIX, push_run_branches}; @@ -12,7 +13,7 @@ use super::rewind::{RewindTarget, TimelineEntry, build_timeline}; #[derive(Debug, Clone)] pub struct ForkRunInput { - pub source_run_id: String, + pub source_run_id: RunId, pub target: Option, pub push: bool, } @@ -20,8 +21,8 @@ pub struct ForkRunInput { /// Create a new run that branches from an existing run at a specific checkpoint. /// /// Returns the new run ID. -pub fn fork(store: &Store, input: &ForkRunInput) -> Result { - let timeline = build_timeline(store, &input.source_run_id)?; +pub fn fork(store: &Store, input: &ForkRunInput) -> Result { + let timeline = build_timeline(store, &input.source_run_id.to_string())?; let entry = match input.target.as_ref() { Some(target) => timeline.resolve(target)?, None => timeline.entries.last().ok_or_else(|| { @@ -33,11 +34,11 @@ pub fn fork(store: &Store, input: &ForkRunInput) -> Result { fn fork_from_entry( store: &Store, - source_run_id: &str, + source_run_id: &RunId, entry: &TimelineEntry, push: bool, -) -> Result { - let new_run_id = ulid::Ulid::new().to_string(); +) -> Result { + let new_run_id = RunId::new(); let sig = Signature::now("Fabro", "noreply@fabro.sh")?; let new_run_branch = format!("{RUN_BRANCH_PREFIX}{new_run_id}"); @@ -57,8 +58,8 @@ fn fork_from_entry( } } - let source_meta_branch = MetadataStore::branch_name(source_run_id); - let new_meta_branch = MetadataStore::branch_name(&new_run_id); + let source_meta_branch = MetadataStore::branch_name(&source_run_id.to_string()); + let new_meta_branch = MetadataStore::branch_name(&new_run_id.to_string()); let source_bs = BranchStore::new(store, &source_meta_branch, &sig); let new_bs = BranchStore::new(store, &new_meta_branch, &sig); @@ -88,14 +89,14 @@ fn fork_from_entry( let mut run_record: RunRecord = serde_json::from_slice(&run_record_bytes).context("failed to parse source run.json")?; - run_record.run_id.clone_from(&new_run_id); + run_record.run_id = new_run_id; run_record.created_at = now; let new_run_record_bytes = serde_json::to_vec_pretty(&run_record).context("failed to serialize new run.json")?; let new_start_record_bytes = if start_record_bytes.is_some() { let start_record = StartRecord { - run_id: new_run_id.clone(), + run_id: new_run_id, start_time: now, run_branch: Some(new_run_branch.clone()), base_sha: None, @@ -160,13 +161,18 @@ mod tests { use super::super::test_support::*; use super::*; + use fabro_types::RunId; use git2::Oid; use crate::operations::find_run_id_by_prefix; - fn make_run_record_json(run_id: &str) -> Vec { + fn parse_run_id(value: &str) -> RunId { + value.parse().unwrap() + } + + fn make_run_record_json(run_id: &RunId) -> Vec { let record = serde_json::json!({ - "run_id": run_id, + "run_id": run_id.to_string(), "created_at": "2025-01-01T00:00:00Z", "settings": {}, "graph": { @@ -187,16 +193,16 @@ mod tests { serde_json::to_vec_pretty(&record).unwrap() } - fn make_start_record_json(run_id: &str) -> Vec { + fn make_start_record_json(run_id: &RunId) -> Vec { let record = serde_json::json!({ - "run_id": run_id, + "run_id": run_id.to_string(), "start_time": "2025-01-01T00:00:00Z", "run_branch": format!("{}{}", RUN_BRANCH_PREFIX, run_id), }); serde_json::to_vec_pretty(&record).unwrap() } - fn setup_source_run(store: &Store, run_id: &str, nodes: &[&str]) -> Vec { + fn setup_source_run(store: &Store, run_id: &RunId, nodes: &[&str]) -> Vec { let sig = test_sig(); let run_branch = format!("{}{run_id}", RUN_BRANCH_PREFIX); @@ -222,7 +228,7 @@ mod tests { parent = Some(oid); } - let meta_branch = MetadataStore::branch_name(run_id); + let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let bs = BranchStore::new(store, &meta_branch, &sig); bs.ensure_branch().unwrap(); @@ -246,13 +252,13 @@ mod tests { #[test] fn fork_creates_new_run_and_metadata_branches() { let (_dir, store) = temp_repo(); - let source_run_id = "run-source"; - let run_oids = setup_source_run(&store, source_run_id, &["start", "build", "test"]); + let source_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); + let run_oids = setup_source_run(&store, &source_run_id, &["start", "build", "test"]); let new_run_id = fork( &store, &ForkRunInput { - source_run_id: source_run_id.to_string(), + source_run_id, target: Some(RewindTarget::from_str("@2").unwrap()), push: false, }, @@ -260,7 +266,7 @@ mod tests { .unwrap(); let new_run_branch = format!("{}{new_run_id}", RUN_BRANCH_PREFIX); - let new_meta_branch = MetadataStore::branch_name(&new_run_id); + let new_meta_branch = MetadataStore::branch_name(&new_run_id.to_string()); assert!(store.resolve_ref(&new_run_branch).unwrap().is_some()); assert!(store.resolve_ref(&new_meta_branch).unwrap().is_some()); @@ -271,7 +277,7 @@ mod tests { let run_record: RunRecord = serde_json::from_slice(&run_json).unwrap(); assert_eq!(run_record.run_id, new_run_id); - let timeline = build_timeline(&store, &new_run_id).unwrap(); + let timeline = build_timeline(&store, &new_run_id.to_string()).unwrap(); assert_eq!(timeline.entries.len(), 1); assert_eq!(timeline.entries[0].node_name, "build"); assert_eq!( @@ -284,11 +290,11 @@ mod tests { fn fork_rejects_checkpoint_without_run_sha() { let (_dir, store) = temp_repo(); let sig = test_sig(); - let run_id = "run-no-sha"; - let meta_branch = MetadataStore::branch_name(run_id); + let run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAW"); + let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let bs = BranchStore::new(&store, &meta_branch, &sig); bs.ensure_branch().unwrap(); - bs.write_entry("run.json", &make_run_record_json(run_id), "init") + bs.write_entry("run.json", &make_run_record_json(&run_id), "init") .unwrap(); let cp = make_checkpoint_json("start", 1, None); @@ -303,7 +309,7 @@ mod tests { run_commit_sha: None, }; - let err = fork_from_entry(&store, run_id, &entry, false) + let err = fork_from_entry(&store, &run_id, &entry, false) .unwrap_err() .to_string(); assert!(err.contains("cannot fork")); @@ -312,10 +318,10 @@ mod tests { #[test] fn fork_supports_prefix_resolved_source_run_ids() { let (_dir, store) = temp_repo(); - let source_run_id = "abc-123-long"; - setup_source_run(&store, source_run_id, &["start", "build"]); + let source_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAX"); + setup_source_run(&store, &source_run_id, &["start", "build"]); - let resolved = find_run_id_by_prefix(store.repo(), "abc-123").unwrap(); + let resolved = find_run_id_by_prefix(store.repo(), "01ARZ3").unwrap(); assert_eq!(resolved, source_run_id); } } diff --git a/lib/crates/fabro-workflows/src/operations/hydrate.rs b/lib/crates/fabro-workflows/src/operations/hydrate.rs index e6f268582..c37aba4de 100644 --- a/lib/crates/fabro-workflows/src/operations/hydrate.rs +++ b/lib/crates/fabro-workflows/src/operations/hydrate.rs @@ -95,7 +95,7 @@ pub async fn open_or_hydrate_run( async fn hydrate_events( run_dir: &Path, - run_id: &str, + run_id: &fabro_types::RunId, run_store: &dyn RunStore, ) -> Result<(), FabroError> { let progress_path = run_dir.join("progress.jsonl"); @@ -224,16 +224,20 @@ mod tests { use fabro_config::FabroSettings; use fabro_graphviz::graph::Graph; use fabro_store::{InMemoryStore, Store}; - use fabro_types::{Conclusion, RunStatus, RunStatusRecord, StageStatus}; + use fabro_types::{Conclusion, RunStatus, RunStatusRecord, StageStatus, fixtures}; use super::open_or_hydrate_run; use crate::event::{WorkflowRunEvent, append_progress_event}; use crate::records::{Checkpoint, CheckpointExt, ConclusionExt, RunRecord, RunRecordExt}; use crate::run_status::RunStatusRecordExt; + fn test_run_id() -> fabro_types::RunId { + fixtures::RUN_1 + } + fn write_run(run_dir: &Path) { let record = RunRecord { - run_id: "run-123".to_string(), + run_id: test_run_id(), created_at: Utc::now(), settings: FabroSettings::default(), graph: Graph::new("test"), @@ -284,7 +288,7 @@ mod tests { conclusion.save(&run_dir.join("conclusion.json")).unwrap(); append_progress_event( run_dir, - "run-123", + &test_run_id(), &WorkflowRunEvent::RunNotice { level: crate::event::RunNoticeLevel::Info, code: "hydrated".to_string(), @@ -305,7 +309,7 @@ mod tests { assert_eq!( run_store.get_run().await.unwrap().unwrap().run_id, - "run-123" + test_run_id() ); assert!(run_store.get_checkpoint().await.unwrap().is_some()); assert!(run_store.get_conclusion().await.unwrap().is_some()); diff --git a/lib/crates/fabro-workflows/src/operations/rebuild_meta.rs b/lib/crates/fabro-workflows/src/operations/rebuild_meta.rs index 0727d198c..a92029ce5 100644 --- a/lib/crates/fabro-workflows/src/operations/rebuild_meta.rs +++ b/lib/crates/fabro-workflows/src/operations/rebuild_meta.rs @@ -8,6 +8,7 @@ use fabro_git_storage::gitobj::Store as GitStore; use fabro_store::{ ListRunsQuery, NodeVisitRef, RunStore as DurableRunStore, Store as DurableStore, }; +use fabro_types::RunId; use git2::{Repository, Signature}; use ulid::Ulid; @@ -18,9 +19,9 @@ use crate::records::Checkpoint; pub async fn rebuild_metadata_branch( git_store: &GitStore, run_store: &dyn DurableRunStore, - run_id: &str, + run_id: &RunId, ) -> Result<()> { - let branch = MetadataStore::branch_name(run_id); + let branch = MetadataStore::branch_name(&run_id.to_string()); if git_store.resolve_ref(&branch)?.is_some() { bail!("metadata branch already exists for run {run_id}"); } @@ -121,16 +122,16 @@ pub async fn rebuild_metadata_branch( pub async fn build_timeline_or_rebuild( git_store: &GitStore, run_store: Option<&dyn DurableRunStore>, - run_id: &str, + run_id: &RunId, ) -> Result { - let branch = MetadataStore::branch_name(run_id); + let branch = MetadataStore::branch_name(&run_id.to_string()); if git_store.resolve_ref(&branch)?.is_some() { - return build_timeline(git_store, run_id); + return build_timeline(git_store, &run_id.to_string()); } if let Some(run_store) = run_store { rebuild_metadata_branch(git_store, run_store, run_id).await?; - return build_timeline(git_store, run_id); + return build_timeline(git_store, &run_id.to_string()); } Ok(RunTimeline { @@ -143,7 +144,7 @@ pub async fn find_run_id_by_prefix_or_store( repo: &Repository, fabro_store: &dyn DurableStore, prefix: &str, -) -> Result { +) -> Result { if let Some(run_id) = find_run_id_by_prefix_in_refs(repo, prefix)? { return Ok(run_id); } @@ -151,7 +152,7 @@ pub async fn find_run_id_by_prefix_or_store( let current_repo_root = canonical_repo_root(repo)?; let mut matches = Vec::new(); for summary in fabro_store.list_runs(&ListRunsQuery::default()).await? { - if summary.run_id == prefix { + if summary.run_id.to_string() == prefix { if summary.host_repo_path.is_none() { return Ok(summary.run_id); } @@ -180,7 +181,7 @@ pub async fn find_run_id_by_prefix_or_store( let Ok(host_repo_root) = canonical_repo_root(&host_repo) else { continue; }; - if host_repo_root == current_repo_root && summary.run_id.starts_with(prefix) { + if host_repo_root == current_repo_root && summary.run_id.to_string().starts_with(prefix) { matches.push(summary.run_id); } } @@ -203,7 +204,7 @@ fn write_entries( fn backfill_missing_checkpoint_shas( git_store: &GitStore, - run_id: &str, + run_id: &RunId, checkpoints: &mut [(u32, Checkpoint)], ) { if !checkpoints @@ -213,7 +214,7 @@ fn backfill_missing_checkpoint_shas( return; } - let node_commits = rewind::run_commit_shas_by_node(git_store, run_id); + let node_commits = rewind::run_commit_shas_by_node(git_store, &run_id.to_string()); let mut node_indices: HashMap = HashMap::new(); for (_seq, checkpoint) in checkpoints.iter_mut() { @@ -241,7 +242,7 @@ fn node_file_path(node_id: &str, visit: u32, filename: &str) -> String { } } -fn find_run_id_by_prefix_in_refs(repo: &Repository, prefix: &str) -> Result> { +fn find_run_id_by_prefix_in_refs(repo: &Repository, prefix: &str) -> Result> { let refs = repo.references()?; let pattern = "refs/heads/fabro/meta/"; let mut matches = Vec::new(); @@ -253,12 +254,15 @@ fn find_run_id_by_prefix_in_refs(repo: &Repository, prefix: &str) -> Result() else { + continue; + }; - if run_id == prefix { - return Ok(Some(run_id.to_string())); + if run_id.to_string() == prefix { + return Ok(Some(run_id)); } - if run_id.starts_with(prefix) { - matches.push(run_id.to_string()); + if run_id.to_string().starts_with(prefix) { + matches.push(run_id); } } @@ -278,7 +282,7 @@ fn canonical_repo_root(repo: &Repository) -> Result { .with_context(|| format!("failed to canonicalize repo root {}", root.display())) } -fn resolve_prefix_matches(prefix: &str, matches: Vec) -> Result { +fn resolve_prefix_matches(prefix: &str, matches: Vec) -> Result { match matches.len() { 0 => bail!("no run found matching '{prefix}'"), 1 => Ok(matches.into_iter().next().unwrap()), @@ -302,7 +306,9 @@ mod tests { use fabro_config::FabroSettings; use fabro_graphviz::graph::Graph; use fabro_store::{InMemoryStore, Store as _}; - use fabro_types::{NodeStatusRecord, RunRecord, SandboxRecord, StageStatus, StartRecord}; + use fabro_types::{ + NodeStatusRecord, RunId, RunRecord, SandboxRecord, StageStatus, StartRecord, fixtures, + }; use super::*; use crate::operations::test_support::{make_checkpoint_json, temp_repo, test_sig}; @@ -312,9 +318,17 @@ mod tests { Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap() } - fn sample_run_record(run_id: &str, host_repo_path: Option<&str>) -> RunRecord { + fn parse_run_id(value: &str) -> RunId { + value.parse().unwrap() + } + + fn test_run_id() -> RunId { + fixtures::RUN_1 + } + + fn sample_run_record(run_id: RunId, host_repo_path: Option<&str>) -> RunRecord { RunRecord { - run_id: run_id.to_string(), + run_id, created_at: created_at(), settings: FabroSettings::default(), graph: Graph::new("test"), @@ -326,9 +340,9 @@ mod tests { } } - fn sample_start_record(run_id: &str) -> StartRecord { + fn sample_start_record(run_id: RunId) -> StartRecord { StartRecord { - run_id: run_id.to_string(), + run_id, start_time: created_at(), run_branch: Some(format!("fabro/run/{run_id}")), base_sha: Some("base-sha".to_string()), @@ -384,10 +398,10 @@ mod tests { async fn create_run_store( store: &InMemoryStore, - run_id: &str, + run_id: RunId, host_repo_path: Option<&str>, ) -> Arc { - let run_store = store.create_run(run_id, created_at(), None).await.unwrap(); + let run_store = store.create_run(&run_id, created_at(), None).await.unwrap(); run_store .put_run(&sample_run_record(run_id, host_repo_path)) .await @@ -395,7 +409,7 @@ mod tests { run_store } - fn seed_run_branch(git_store: &GitStore, run_id: &str, nodes: &[&str]) -> Vec { + fn seed_run_branch(git_store: &GitStore, run_id: RunId, nodes: &[&str]) -> Vec { let sig = test_sig(); let run_branch = format!("fabro/run/{run_id}"); let empty_tree = git_store.write_empty_tree().unwrap(); @@ -424,9 +438,9 @@ mod tests { async fn rebuild_metadata_branch_round_trips_timeline() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; run_store - .put_start(&sample_start_record("run-1")) + .put_start(&sample_start_record(test_run_id())) .await .unwrap(); run_store @@ -462,11 +476,11 @@ mod tests { .await .unwrap(); - rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap(); - let timeline = build_timeline(&git_store, "run-1").unwrap(); + let timeline = build_timeline(&git_store, &test_run_id().to_string()).unwrap(); assert_eq!(timeline.entries.len(), 3); assert_eq!(timeline.entries[0].node_name, "start"); assert_eq!(timeline.entries[0].visit, 1); @@ -483,7 +497,7 @@ mod tests { async fn rebuild_metadata_branch_preserves_historical_node_visits() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; let build_v1 = NodeVisitRef { node_id: "build", @@ -526,12 +540,12 @@ mod tests { .await .unwrap(); - rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap(); let sig = test_sig(); - let branch = MetadataStore::branch_name("run-1"); + let branch = MetadataStore::branch_name(&test_run_id().to_string()); let bs = BranchStore::new(&git_store, &branch, &sig); let checkpoint_commits: Vec<_> = bs .log(100) @@ -576,15 +590,15 @@ mod tests { async fn rebuild_metadata_branch_refuses_to_overwrite_existing_branch() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; let sig = test_sig(); - let branch = MetadataStore::branch_name("run-1"); + let branch = MetadataStore::branch_name(&test_run_id().to_string()); let bs = BranchStore::new(&git_store, &branch, &sig); bs.ensure_branch().unwrap(); bs.write_entry("run.json", b"{}", "init run").unwrap(); - let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap_err(); assert!(err.to_string().contains("metadata branch already exists")); @@ -594,7 +608,7 @@ mod tests { async fn build_timeline_or_rebuild_rebuilds_missing_branch() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; run_store .append_checkpoint(&sample_checkpoint( "start", @@ -605,9 +619,10 @@ mod tests { .await .unwrap(); - let timeline = build_timeline_or_rebuild(&git_store, Some(run_store.as_ref()), "run-1") - .await - .unwrap(); + let timeline = + build_timeline_or_rebuild(&git_store, Some(run_store.as_ref()), &test_run_id()) + .await + .unwrap(); assert_eq!(timeline.entries.len(), 1); assert_eq!(timeline.entries[0].node_name, "start"); @@ -617,7 +632,7 @@ mod tests { async fn build_timeline_or_rebuild_preserves_existing_branch() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; run_store .append_checkpoint(&sample_checkpoint( "start", @@ -647,7 +662,7 @@ mod tests { .unwrap(); let sig = test_sig(); - let branch = MetadataStore::branch_name("run-1"); + let branch = MetadataStore::branch_name(&test_run_id().to_string()); let bs = BranchStore::new(&git_store, &branch, &sig); bs.ensure_branch().unwrap(); bs.write_entry("run.json", b"{}", "init run").unwrap(); @@ -664,9 +679,10 @@ mod tests { ) .unwrap(); - let timeline = build_timeline_or_rebuild(&git_store, Some(run_store.as_ref()), "run-1") - .await - .unwrap(); + let timeline = + build_timeline_or_rebuild(&git_store, Some(run_store.as_ref()), &test_run_id()) + .await + .unwrap(); assert_eq!(timeline.entries.len(), 2); assert_eq!(timeline.entries[0].node_name, "start"); @@ -676,7 +692,7 @@ mod tests { #[tokio::test] async fn build_timeline_or_rebuild_returns_empty_without_store() { let (_dir, git_store) = temp_repo(); - let timeline = build_timeline_or_rebuild(&git_store, None, "run-1") + let timeline = build_timeline_or_rebuild(&git_store, None, &test_run_id()) .await .unwrap(); assert!(timeline.entries.is_empty()); @@ -688,11 +704,11 @@ mod tests { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); let run_store = durable_store - .create_run("run-1", created_at(), None) + .create_run(&test_run_id(), created_at(), None) .await .unwrap(); - let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap_err(); assert!(err.to_string().contains("run record not found")); @@ -703,13 +719,15 @@ mod tests { let (dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); let repo_path = dir.path().to_string_lossy().to_string(); - let _run_store = create_run_store(&durable_store, "abc-123-long", Some(&repo_path)).await; + let repo_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); + let _run_store = create_run_store(&durable_store, repo_run_id, Some(&repo_path)).await; + let prefix = &repo_run_id.to_string()[..6]; - let run_id = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123") + let run_id = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, prefix) .await .unwrap(); - assert_eq!(run_id, "abc-123-long"); + assert_eq!(run_id, repo_run_id); } #[tokio::test] @@ -718,10 +736,12 @@ mod tests { let (other_dir, _other_git_store) = temp_repo(); let durable_store = InMemoryStore::default(); let other_repo_path = other_dir.path().to_string_lossy().to_string(); + let other_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); let _run_store = - create_run_store(&durable_store, "abc-123-long", Some(&other_repo_path)).await; + create_run_store(&durable_store, other_run_id, Some(&other_repo_path)).await; + let prefix = &other_run_id.to_string()[..6]; - let err = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123") + let err = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, prefix) .await .unwrap_err(); @@ -732,19 +752,23 @@ mod tests { async fn find_run_id_by_prefix_or_store_requires_exact_match_without_repo_path() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let _run_store = create_run_store(&durable_store, "abc-123-long", None).await; + let repo_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); + let _run_store = create_run_store(&durable_store, repo_run_id, None).await; + let prefix = &repo_run_id.to_string()[..6]; - let prefix_err = - find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123") - .await - .unwrap_err(); + let prefix_err = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, prefix) + .await + .unwrap_err(); assert!(prefix_err.to_string().contains("no run found matching")); - let exact = - find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123-long") - .await - .unwrap(); - assert_eq!(exact, "abc-123-long"); + let exact = find_run_id_by_prefix_or_store( + git_store.repo(), + &durable_store, + &repo_run_id.to_string(), + ) + .await + .unwrap(); + assert_eq!(exact, repo_run_id); } #[tokio::test] @@ -752,35 +776,50 @@ mod tests { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); let repo_path = git_store.repo_dir().to_string_lossy().to_string(); - let _short = create_run_store(&durable_store, "abc-123", Some(&repo_path)).await; - let _long = create_run_store(&durable_store, "abc-123-long", Some(&repo_path)).await; + let exact_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); + let other_run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAW"); + let _exact = create_run_store(&durable_store, exact_run_id, Some(&repo_path)).await; + let _other = create_run_store(&durable_store, other_run_id, Some(&repo_path)).await; - let from_store = - find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123") - .await - .unwrap(); - assert_eq!(from_store, "abc-123"); + let from_store = find_run_id_by_prefix_or_store( + git_store.repo(), + &durable_store, + &exact_run_id.to_string(), + ) + .await + .unwrap(); + assert_eq!(from_store, exact_run_id); let sig = test_sig(); - let short_branch = - BranchStore::new(&git_store, MetadataStore::branch_name("abc-123"), &sig); - short_branch.ensure_branch().unwrap(); + let exact_branch = BranchStore::new( + &git_store, + &MetadataStore::branch_name(&exact_run_id.to_string()), + &sig, + ); + exact_branch.ensure_branch().unwrap(); - let long_branch = - BranchStore::new(&git_store, MetadataStore::branch_name("abc-123-long"), &sig); - long_branch.ensure_branch().unwrap(); + let other_branch = BranchStore::new( + &git_store, + &MetadataStore::branch_name(&other_run_id.to_string()), + &sig, + ); + other_branch.ensure_branch().unwrap(); - let from_refs = find_run_id_by_prefix_or_store(git_store.repo(), &durable_store, "abc-123") - .await - .unwrap(); - assert_eq!(from_refs, "abc-123"); + let from_refs = find_run_id_by_prefix_or_store( + git_store.repo(), + &durable_store, + &exact_run_id.to_string(), + ) + .await + .unwrap(); + assert_eq!(from_refs, exact_run_id); } #[tokio::test] async fn rebuild_metadata_branch_persists_backfilled_run_shas_in_checkpoint_blobs() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; run_store .append_checkpoint(&sample_checkpoint( @@ -801,14 +840,14 @@ mod tests { .await .unwrap(); - let expected_shas = seed_run_branch(&git_store, "run-1", &["start", "build"]); + let expected_shas = seed_run_branch(&git_store, test_run_id(), &["start", "build"]); - rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap(); let sig = test_sig(); - let branch = MetadataStore::branch_name("run-1"); + let branch = MetadataStore::branch_name(&test_run_id().to_string()); let bs = BranchStore::new(&git_store, &branch, &sig); let checkpoint_commits: Vec<_> = bs .log(100) @@ -848,7 +887,7 @@ mod tests { async fn rebuild_metadata_branch_is_atomic_on_failure() { let (_dir, git_store) = temp_repo(); let durable_store = InMemoryStore::default(); - let run_store = create_run_store(&durable_store, "run-1", None).await; + let run_store = create_run_store(&durable_store, test_run_id(), None).await; let bad_node = "bad\0node"; let bad_visit = NodeVisitRef { @@ -869,13 +908,13 @@ mod tests { .await .unwrap(); - let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), "run-1") + let err = rebuild_metadata_branch(&git_store, run_store.as_ref(), &test_run_id()) .await .unwrap_err(); assert!(err.to_string().contains("nul") || err.to_string().contains("NUL")); assert!( git_store - .resolve_ref(&MetadataStore::branch_name("run-1")) + .resolve_ref(&MetadataStore::branch_name(&test_run_id().to_string())) .unwrap() .is_none() ); @@ -886,7 +925,9 @@ mod tests { .unwrap() .flatten() .filter_map(|reference| reference.name().map(ToOwned::to_owned)) - .filter(|name| name.starts_with("refs/heads/fabro/meta-rebuild/run-1/")) + .filter(|name| { + name.starts_with(&format!("refs/heads/fabro/meta-rebuild/{}/", test_run_id())) + }) .collect(); assert!( scratch_refs.is_empty(), diff --git a/lib/crates/fabro-workflows/src/operations/rewind.rs b/lib/crates/fabro-workflows/src/operations/rewind.rs index 4ce314bd9..b12978cc5 100644 --- a/lib/crates/fabro-workflows/src/operations/rewind.rs +++ b/lib/crates/fabro-workflows/src/operations/rewind.rs @@ -5,6 +5,7 @@ use std::str::FromStr; use anyhow::{Context, Result, bail}; use fabro_git_storage::branchstore::{BranchStore, CommitInfo}; use fabro_git_storage::gitobj::Store; +use fabro_types::RunId; use git2::{Oid, Repository, Signature}; use crate::git::{MetadataStore, RUN_BRANCH_PREFIX, push_run_branches}; @@ -113,7 +114,7 @@ impl RunTimeline { #[derive(Debug, Clone)] pub struct RewindInput { - pub run_id: String, + pub run_id: RunId, pub target: RewindTarget, pub push: bool, } @@ -248,14 +249,14 @@ fn detect_parallel_interior(graph: &Graph) -> HashMap { } pub fn rewind(store: &Store, input: &RewindInput) -> Result<()> { - let timeline = build_timeline(store, &input.run_id)?; + let timeline = build_timeline(store, &input.run_id.to_string())?; let entry = timeline.resolve(&input.target)?; rewind_to_entry(store, &input.run_id, entry, input.push) } #[allow(clippy::print_stderr)] -fn rewind_to_entry(store: &Store, run_id: &str, entry: &TimelineEntry, push: bool) -> Result<()> { - let meta_branch = MetadataStore::branch_name(run_id); +fn rewind_to_entry(store: &Store, run_id: &RunId, entry: &TimelineEntry, push: bool) -> Result<()> { + let meta_branch = MetadataStore::branch_name(&run_id.to_string()); store .update_ref(&meta_branch, entry.metadata_commit_oid) .map_err(|e| anyhow::anyhow!("failed to update metadata ref: {e}"))?; @@ -304,7 +305,7 @@ fn rewind_to_entry(store: &Store, run_id: &str, entry: &TimelineEntry, push: boo Ok(()) } -pub fn find_run_id_by_prefix(repo: &Repository, prefix: &str) -> Result { +pub fn find_run_id_by_prefix(repo: &Repository, prefix: &str) -> Result { let refs = repo.references()?; let pattern = "refs/heads/fabro/meta/"; let mut matches = Vec::new(); @@ -314,11 +315,14 @@ pub fn find_run_id_by_prefix(repo: &Repository, prefix: &str) -> Result continue; }; if let Some(run_id) = name.strip_prefix(pattern) { - if run_id == prefix { - return Ok(run_id.to_string()); + let Ok(run_id) = run_id.parse::() else { + continue; + }; + if run_id.to_string() == prefix { + return Ok(run_id); } - if run_id.starts_with(prefix) { - matches.push(run_id.to_string()); + if run_id.to_string().starts_with(prefix) { + matches.push(run_id); } } } @@ -367,6 +371,11 @@ fn load_parallel_map(store: &Store, run_id: &str) -> HashMap { mod tests { use super::super::test_support::*; use super::*; + use fabro_types::{RunId, fixtures}; + + fn parse_run_id(value: &str) -> RunId { + value.parse().unwrap() + } #[test] fn parse_target_ordinal() { @@ -485,7 +494,7 @@ mod tests { fn rewind_moves_metadata_ref() { let (_dir, store) = temp_repo(); let sig = test_sig(); - let branch = MetadataStore::branch_name("run-1"); + let branch = MetadataStore::branch_name(&fixtures::RUN_1.to_string()); let bs = BranchStore::new(&store, &branch, &sig); bs.ensure_branch().unwrap(); @@ -501,7 +510,7 @@ mod tests { rewind( &store, &RewindInput { - run_id: "run-1".to_string(), + run_id: fixtures::RUN_1, target: RewindTarget::Ordinal(1), push: false, }, @@ -516,11 +525,12 @@ mod tests { fn find_run_id_prefix_match() { let (_dir, store) = temp_repo(); let sig = test_sig(); - let branch = MetadataStore::branch_name("abc-123-long-id"); + let run_id = parse_run_id("01ARZ3NDEKTSV4RRFFQ69G5FAV"); + let branch = MetadataStore::branch_name(&run_id.to_string()); let bs = BranchStore::new(&store, &branch, &sig); bs.ensure_branch().unwrap(); - let result = find_run_id_by_prefix(store.repo(), "abc-123").unwrap(); - assert_eq!(result, "abc-123-long-id"); + let result = find_run_id_by_prefix(store.repo(), "01ARZ3").unwrap(); + assert_eq!(result, run_id); } } diff --git a/lib/crates/fabro-workflows/src/operations/start.rs b/lib/crates/fabro-workflows/src/operations/start.rs index faa73f03b..298a197ee 100644 --- a/lib/crates/fabro-workflows/src/operations/start.rs +++ b/lib/crates/fabro-workflows/src/operations/start.rs @@ -12,6 +12,7 @@ use fabro_interview::{AutoApproveInterviewer, Interviewer}; use fabro_model::{Catalog, FallbackTarget, Provider}; use fabro_sandbox::{SandboxProvider, SandboxSpec, detect_clone_params}; use fabro_store::{DiskProjectingRunStore, ProjectionError, RunStore}; +use fabro_types::RunId; use serde::Serialize; use crate::context::Context; @@ -672,7 +673,7 @@ const POSTRUN_CANCELLED_MESSAGE: &str = "Run cancelled before post-run finalizat struct DetachedRunCompletionGuard { run_dir: PathBuf, run_store: Arc, - run_id: Option, + run_id: Option, cancel_token: Option>, active: bool, } @@ -780,16 +781,16 @@ impl Drop for DetachedRunCompletionGuard { } } -fn load_run_id(run_dir: &Path) -> Option { +fn load_run_id(run_dir: &Path) -> Option { RunRecord::load(run_dir) .ok() .map(|record| record.run_id) - .filter(|run_id| !run_id.trim().is_empty()) .or_else(|| { std::fs::read_to_string(run_dir.join("id.txt")) .ok() .map(|run_id| run_id.trim().to_string()) .filter(|run_id| !run_id.is_empty()) + .and_then(|run_id| run_id.parse().ok()) }) } @@ -909,6 +910,7 @@ mod tests { use chrono::Utc; use fabro_config::FabroSettings; use fabro_store::InMemoryStore; + use fabro_types::fixtures; use super::*; use crate::context::Context; @@ -942,7 +944,7 @@ mod tests { .to_path_buf(), workflow_slug: Some("test".to_string()), run_dir: Some(run_dir.to_path_buf()), - run_id: Some("run-test".to_string()), + run_id: Some(fixtures::RUN_1), host_repo_path: None, base_branch: None, }) diff --git a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs index 069ade696..02f68ce32 100644 --- a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs @@ -13,6 +13,7 @@ use fabro_hooks::HookConfig; use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; use fabro_store::InMemoryStore; +use fabro_types::{RunId, fixtures}; use super::*; use crate::context::{self, Context}; @@ -67,11 +68,18 @@ fn make_registry() -> HandlerRegistry { registry } +fn test_run_id(label: &str) -> RunId { + match label { + "git-cp-test" => fixtures::RUN_2, + _ => fixtures::RUN_1, + } +} + fn test_run_options(run_dir: &Path, run_id: &str) -> RunOptions { RunOptions { run_dir: run_dir.to_path_buf(), cancel_token: None, - run_id: run_id.into(), + run_id: test_run_id(run_id), settings: FabroSettings::default(), git: None, host_repo_path: None, @@ -106,14 +114,14 @@ fn simple_validated_graph() -> (Graph, String) { (graph, source) } -fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: &str) -> Persisted { +fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunId) -> Persisted { Persisted::new( graph.clone(), source, vec![], run_dir.to_path_buf(), RunRecord { - run_id: run_id.to_string(), + run_id, created_at: Utc::now(), settings: FabroSettings::default(), graph, @@ -139,10 +147,10 @@ fn test_lifecycle(setup_commands: Vec) -> LifecycleOptions { } } -async fn test_run_store(_run_dir: &Path) -> Arc { +async fn test_run_store(_run_dir: &Path, run_id: &RunId) -> Arc { let store: &dyn fabro_store::Store = &InMemoryStore::default(); store - .create_run("test-run", chrono::Utc::now(), None) + .create_run(run_id, chrono::Utc::now(), None) .await .unwrap() } @@ -154,10 +162,10 @@ async fn execute_runs_start_to_exit_and_returns_final_context() { std::fs::create_dir_all(&run_dir).unwrap(); let (graph, source) = simple_validated_graph(); let initialized = initialize( - persisted_workflow(graph, source, &run_dir, "run-test"), + persisted_workflow(graph, source, &run_dir, test_run_id("run-test")), InitOptions { - run_id: "run-test".to_string(), - run_store: test_run_store(&run_dir).await, + run_id: test_run_id("run-test"), + run_store: test_run_store(&run_dir, &test_run_id("run-test")).await, dry_run: false, emitter: Arc::new(crate::event::EventEmitter::new()), sandbox: SandboxSpec::Local { @@ -205,7 +213,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() { executed .final_context .get(crate::context::keys::INTERNAL_RUN_ID), - Some(serde_json::json!("run-test")) + Some(serde_json::json!(test_run_id("run-test").to_string())) ); } @@ -221,10 +229,10 @@ async fn run_with_lifecycle( let run_dir = run_options.run_dir.clone(); let run_id = run_options.run_id.clone(); let initialized = initialize( - persisted_workflow(graph.clone(), String::new(), &run_dir, &run_id), + persisted_workflow(graph.clone(), String::new(), &run_dir, run_id), InitOptions { run_id, - run_store: test_run_store(&run_dir).await, + run_store: test_run_store(&run_dir, &run_id).await, dry_run: false, emitter, sandbox: SandboxSpec::Local { @@ -551,7 +559,7 @@ async fn execute_writes_start_json_and_node_status() { let mut run_options = test_run_options(dir.path(), "test-run"); run_options.git = Some(GitCheckpointOptions { base_sha: Some("abc123".into()), - run_branch: Some("fabro/run/test-run".into()), + run_branch: Some(format!("fabro/run/{}", test_run_id("test-run"))), meta_branch: None, }); @@ -566,8 +574,11 @@ async fn execute_writes_start_json_and_node_status() { .unwrap(); let start = crate::records::StartRecord::load(dir.path()).unwrap(); - assert_eq!(start.run_id, "test-run"); - assert_eq!(start.run_branch.as_deref(), Some("fabro/run/test-run")); + assert_eq!(start.run_id, test_run_id("test-run")); + assert_eq!( + start.run_branch.as_deref(), + Some(format!("fabro/run/{}", test_run_id("test-run")).as_str()) + ); assert_eq!(start.base_sha.as_deref(), Some("abc123")); let status_path = dir.path().join("nodes").join("start").join("status.json"); @@ -964,7 +975,9 @@ async fn git_checkpoint_skips_start_node() { run_options.git = Some(GitCheckpointOptions { base_sha: Some(base_sha), run_branch: None, - meta_branch: Some(crate::git::MetadataStore::branch_name("git-cp-test")), + meta_branch: Some(crate::git::MetadataStore::branch_name( + &test_run_id("git-cp-test").to_string(), + )), }); run_options.host_repo_path = Some(repo.to_path_buf()); diff --git a/lib/crates/fabro-workflows/src/pipeline/finalize.rs b/lib/crates/fabro-workflows/src/pipeline/finalize.rs index a38fa620e..40be8ce20 100644 --- a/lib/crates/fabro-workflows/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/finalize.rs @@ -291,7 +291,7 @@ pub async fn write_finalize_commit( .iter() .map(|(k, v)| (k.as_str(), v.as_slice())) .collect(); - if let Err(e) = store.write_files(&run_options.run_id, &refs, "finalize run") { + if let Err(e) = store.write_files(&run_options.run_id.to_string(), &refs, "finalize run") { tracing::warn!(error = %e, "Failed to write finalize commit to metadata branch"); return; } @@ -320,13 +320,13 @@ async fn run_hooks( async fn cleanup_sandbox( hook_runner: Option>, sandbox: Arc, - run_id: &str, + run_id: &fabro_types::RunId, workflow_name: &str, preserve: bool, ) -> std::result::Result<(), String> { let hook_ctx = HookContext::new( HookEvent::SandboxCleanup, - run_id.to_string(), + *run_id, workflow_name.to_string(), ); run_hooks(hook_runner.as_deref(), &hook_ctx, Arc::clone(&sandbox)).await; @@ -441,17 +441,22 @@ mod tests { use fabro_config::FabroSettings; use fabro_graphviz::graph::Graph; use fabro_store::{InMemoryStore, Store}; + use fabro_types::{RunId, fixtures}; use super::*; use crate::pipeline::types::Retroed; use crate::run_options::RunOptions; + fn test_run_id() -> RunId { + fixtures::RUN_1 + } + fn test_run_options(run_dir: &std::path::Path) -> RunOptions { RunOptions { settings: FabroSettings::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, - run_id: "run-test".to_string(), + run_id: test_run_id(), labels: HashMap::new(), workflow_slug: None, github_app: None, @@ -469,7 +474,7 @@ mod tests { std::fs::create_dir_all(&run_dir).unwrap(); let inner_store = InMemoryStore::default() .create_run( - "run-test", + &test_run_id(), Utc::now(), Some(run_dir.to_string_lossy().as_ref()), ) @@ -496,7 +501,7 @@ mod tests { retroed, &FinalizeOptions { run_dir: run_dir.clone(), - run_id: "run-test".to_string(), + run_id: test_run_id(), run_store: Arc::clone(&run_store), workflow_name: "test".to_string(), hook_runner: None, diff --git a/lib/crates/fabro-workflows/src/pipeline/initialize.rs b/lib/crates/fabro-workflows/src/pipeline/initialize.rs index 1b8df4317..41cba104d 100644 --- a/lib/crates/fabro-workflows/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/initialize.rs @@ -401,7 +401,7 @@ pub async fn initialize( options.run_options.git = Some(GitCheckpointOptions { base_sha: Some(plan.base_sha.clone()), run_branch: Some(plan.branch_name.clone()), - meta_branch: Some(MetadataStore::branch_name(&options.run_id)), + meta_branch: Some(MetadataStore::branch_name(&options.run_id.to_string())), }); } @@ -530,7 +530,10 @@ pub async fn initialize( .and_then(|g| g.run_branch.as_ref()) .is_some(); if !has_run_branch { - match sandbox.setup_git_for_run(&options.run_options.run_id).await { + match sandbox + .setup_git_for_run(&options.run_options.run_id.to_string()) + .await + { Ok(Some(info)) => { let base_sha = options .run_options @@ -542,7 +545,9 @@ pub async fn initialize( options.run_options.git = Some(GitCheckpointOptions { base_sha, run_branch: Some(info.run_branch.clone()), - meta_branch: Some(MetadataStore::branch_name(&options.run_options.run_id)), + meta_branch: Some(MetadataStore::branch_name( + &options.run_options.run_id.to_string(), + )), }); if options.run_options.base_branch.is_none() { options.run_options.base_branch = info.base_branch; @@ -653,12 +658,17 @@ mod tests { use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; use fabro_store::InMemoryStore; + use fabro_types::{RunId, fixtures}; use super::*; use crate::pipeline::types::InitOptions; use crate::records::RunRecord; use crate::run_options::RunOptions; + fn test_run_id() -> RunId { + fixtures::RUN_1 + } + fn simple_graph() -> (Graph, String) { let source = r#"digraph test { start [shape=Mdiamond]; @@ -688,7 +698,7 @@ mod tests { settings: FabroSettings::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, - run_id: "run-test".to_string(), + run_id: test_run_id(), labels: HashMap::new(), workflow_slug: None, github_app: None, @@ -706,7 +716,7 @@ mod tests { vec![], run_dir.to_path_buf(), RunRecord { - run_id: "run-test".to_string(), + run_id: test_run_id(), created_at: Utc::now(), settings: FabroSettings::default(), graph, @@ -731,12 +741,12 @@ mod tests { let initialized = initialize( persisted, InitOptions { - run_id: "run-test".to_string(), + run_id: test_run_id(), run_store: { let store: &dyn fabro_store::Store = &InMemoryStore::default(); let inner = store .create_run( - "test-run", + &test_run_id(), chrono::Utc::now(), Some(run_dir.to_string_lossy().as_ref()), ) @@ -809,12 +819,12 @@ mod tests { let initialized = initialize( persisted, InitOptions { - run_id: "run-test".to_string(), + run_id: test_run_id(), run_store: { let store: &dyn fabro_store::Store = &InMemoryStore::default(); let inner = store .create_run( - "test-run", + &test_run_id(), chrono::Utc::now(), Some(run_dir.to_string_lossy().as_ref()), ) diff --git a/lib/crates/fabro-workflows/src/pipeline/persist.rs b/lib/crates/fabro-workflows/src/pipeline/persist.rs index 3120da770..51c2fa1a7 100644 --- a/lib/crates/fabro-workflows/src/pipeline/persist.rs +++ b/lib/crates/fabro-workflows/src/pipeline/persist.rs @@ -96,6 +96,7 @@ mod tests { use chrono::Utc; use fabro_config::FabroSettings; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; + use fabro_types::fixtures; use super::*; use crate::records::RunRecord; @@ -146,7 +147,7 @@ mod tests { fn sample_record(graph: Graph) -> RunRecord { RunRecord { - run_id: "run-123".to_string(), + run_id: fixtures::RUN_1, created_at: Utc::now(), settings: FabroSettings { dry_run: Some(true), diff --git a/lib/crates/fabro-workflows/src/pipeline/pull_request.rs b/lib/crates/fabro-workflows/src/pipeline/pull_request.rs index 6ad4a5f4a..9a3e2c739 100644 --- a/lib/crates/fabro-workflows/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflows/src/pipeline/pull_request.rs @@ -634,6 +634,7 @@ mod tests { use fabro_retro::retro::{ AggregateStats, FrictionKind, FrictionPoint, OpenItem, OpenItemKind, StageRetro, }; + use fabro_types::fixtures; use futures::stream; struct MockProvider { @@ -763,7 +764,7 @@ mod tests { fn make_test_retro() -> Retro { Retro { - run_id: "test-run".to_string(), + run_id: fixtures::RUN_1, workflow_name: "implement".to_string(), goal: "Fix the bug".to_string(), timestamp: Utc::now(), @@ -866,7 +867,7 @@ mod tests { #[test] fn format_retro_section_empty_stats() { let retro = Retro { - run_id: "test".to_string(), + run_id: fixtures::RUN_2, workflow_name: "test".to_string(), goal: "test".to_string(), timestamp: Utc::now(), diff --git a/lib/crates/fabro-workflows/src/pipeline/retro.rs b/lib/crates/fabro-workflows/src/pipeline/retro.rs index 7b3745e32..f4e13d365 100644 --- a/lib/crates/fabro-workflows/src/pipeline/retro.rs +++ b/lib/crates/fabro-workflows/src/pipeline/retro.rs @@ -41,7 +41,7 @@ pub async fn run_retro(options: &RetroOptions, dry_run: bool) -> Option { } }; let mut retro = derive_retro( - &options.run_id, + options.run_id, &options.workflow_name, &options.goal, completed_stages, @@ -174,6 +174,7 @@ mod tests { use fabro_config::FabroSettings; use fabro_graphviz::graph::Graph; use fabro_store::{InMemoryStore, Store}; + use fabro_types::{RunId, fixtures}; use super::*; use crate::context::Context; @@ -182,6 +183,10 @@ mod tests { use crate::records::{Checkpoint, CheckpointExt}; use crate::run_options::RunOptions; + fn test_run_id() -> RunId { + fixtures::RUN_1 + } + fn write_checkpoint(run_dir: &std::path::Path) -> Checkpoint { let context = Context::new(); context.set("response.work", serde_json::json!("done")); @@ -208,7 +213,7 @@ mod tests { ) -> Arc { let inner = InMemoryStore::default() .create_run( - "run-test", + &test_run_id(), Utc::now(), Some(run_dir.to_string_lossy().as_ref()), ) @@ -226,7 +231,7 @@ mod tests { settings: FabroSettings::default(), run_dir: run_dir.to_path_buf(), cancel_token: None, - run_id: "run-test".to_string(), + run_id: test_run_id(), labels: HashMap::new(), workflow_slug: None, github_app: None, @@ -266,7 +271,7 @@ mod tests { let retroed = retro( executed, &RetroOptions { - run_id: "run-test".to_string(), + run_id: test_run_id(), run_store: test_run_store(&run_dir, &checkpoint).await, workflow_name: "test".to_string(), goal: "Ship it".to_string(), @@ -303,7 +308,7 @@ mod tests { let retro = run_retro( &RetroOptions { - run_id: "run-test".to_string(), + run_id: test_run_id(), run_store: test_run_store(&run_dir, &checkpoint).await, workflow_name: "test".to_string(), goal: "Ship it".to_string(), diff --git a/lib/crates/fabro-workflows/src/pipeline/types.rs b/lib/crates/fabro-workflows/src/pipeline/types.rs index 03e0cb315..121060907 100644 --- a/lib/crates/fabro-workflows/src/pipeline/types.rs +++ b/lib/crates/fabro-workflows/src/pipeline/types.rs @@ -12,6 +12,7 @@ use fabro_mcp::config::McpServerConfig; use fabro_model::FallbackTarget; use fabro_sandbox::SandboxSpec; use fabro_store::RunStore; +use fabro_types::RunId; use fabro_validate::Diagnostic; use crate::context::Context; @@ -230,7 +231,7 @@ pub struct DevcontainerSpec { } pub struct InitOptions { - pub run_id: String, + pub run_id: RunId, pub run_store: Arc, pub dry_run: bool, pub emitter: Arc, @@ -304,7 +305,7 @@ pub struct Retroed { /// Output of the FINALIZE phase. #[non_exhaustive] pub struct Concluded { - pub run_id: String, + pub run_id: RunId, pub outcome: Result, pub conclusion: Conclusion, pub pushed_branch: Option, @@ -316,7 +317,7 @@ pub struct Concluded { /// Output of the PULL_REQUEST phase. #[non_exhaustive] pub struct Finalized { - pub run_id: String, + pub run_id: RunId, pub outcome: Result, pub conclusion: Conclusion, pub pushed_branch: Option, @@ -331,7 +332,7 @@ pub struct TransformOptions { /// Options for the RETRO phase. pub struct RetroOptions { - pub run_id: String, + pub run_id: RunId, pub run_store: Arc, pub workflow_name: String, pub goal: String, @@ -349,7 +350,7 @@ pub struct RetroOptions { /// Options for the FINALIZE phase. pub struct FinalizeOptions { pub run_dir: PathBuf, - pub run_id: String, + pub run_id: RunId, pub run_store: Arc, pub workflow_name: String, pub hook_runner: Option>, diff --git a/lib/crates/fabro-workflows/src/run_lookup.rs b/lib/crates/fabro-workflows/src/run_lookup.rs index b7b2c78d1..a6cd9c983 100644 --- a/lib/crates/fabro-workflows/src/run_lookup.rs +++ b/lib/crates/fabro-workflows/src/run_lookup.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; use fabro_store::{ListRunsQuery, Store}; +use fabro_types::RunId; use serde::Serialize; use crate::records::{ @@ -13,7 +14,7 @@ use crate::run_status::{RunStatus, RunStatusRecord, RunStatusRecordExt, StatusRe #[derive(Debug, Clone, Serialize)] pub struct RunInfo { - pub run_id: String, + pub run_id: RunId, pub dir_name: String, pub workflow_name: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -116,7 +117,12 @@ pub fn scan_runs(base: &Path) -> Result> { let mtime = mtime_dt.map(|dt| dt.to_rfc3339()).unwrap_or_default(); let run_id = std::fs::read_to_string(path.join("id.txt")) - .map_or_else(|_| dir_name.clone(), |s| s.trim().to_string()); + .ok() + .and_then(|s| parse_run_id(&s)) + .or_else(|| parse_run_id(&dir_name)); + let Some(run_id) = run_id else { + continue; + }; let status_info = read_status(&path); let is_orphan = matches!(status_info.status, RunStatus::Dead); @@ -151,9 +157,9 @@ pub fn scan_runs(base: &Path) -> Result> { } pub async fn scan_runs_combined(store: &dyn Store, base: &Path) -> Result> { - let mut runs_by_id: HashMap = scan_runs(base)? + let mut runs_by_id: HashMap = scan_runs(base)? .into_iter() - .map(|run| (run.run_id.clone(), run)) + .map(|run| (run.run_id, run)) .collect(); if let Ok(store_runs) = store.list_runs(&ListRunsQuery::default()).await { @@ -184,7 +190,7 @@ pub async fn scan_runs_combined(store: &dyn Store, base: &Path) -> Result Result { let runs = scan_runs(base).context("Failed to scan runs")?; let matches: Vec<_> = runs .iter() - .filter(|run| run.run_id.starts_with(prefix)) + .filter(|run| run_id_matches(run.run_id, prefix)) .collect(); match matches.len() { 0 => bail!("No run found matching prefix '{prefix}'"), 1 => Ok(matches[0].path.clone()), count => { - let ids: Vec<&str> = matches.iter().map(|run| run.run_id.as_str()).collect(); + let ids: Vec = matches.iter().map(|run| run.run_id.to_string()).collect(); bail!( "Ambiguous prefix '{prefix}': {count} runs match: {}", ids.join(", ") @@ -328,13 +334,16 @@ pub fn resolve_run(base: &Path, identifier: &str) -> Result { let id_matches: Vec<_> = runs .iter() - .filter(|run| run.run_id.starts_with(identifier)) + .filter(|run| run_id_matches(run.run_id, identifier)) .collect(); match id_matches.len() { 1 => return Ok(id_matches[0].clone()), count if count > 1 => { - let ids: Vec<&str> = id_matches.iter().map(|run| run.run_id.as_str()).collect(); + let ids: Vec = id_matches + .iter() + .map(|run| run.run_id.to_string()) + .collect(); bail!( "Ambiguous prefix '{identifier}': {count} runs match: {}", ids.join(", ") @@ -374,13 +383,16 @@ pub async fn resolve_run_combined( let id_matches: Vec<_> = runs .iter() - .filter(|run| run.run_id.starts_with(identifier)) + .filter(|run| run_id_matches(run.run_id, identifier)) .collect(); match id_matches.len() { 1 => return Ok(id_matches[0].clone()), count if count > 1 => { - let ids: Vec<&str> = id_matches.iter().map(|run| run.run_id.as_str()).collect(); + let ids: Vec = id_matches + .iter() + .map(|run| run.run_id.to_string()) + .collect(); bail!( "Ambiguous prefix '{identifier}': {count} runs match: {}", ids.join(", ") @@ -412,3 +424,12 @@ pub async fn resolve_run_combined( fn collapse_separators(s: &str) -> String { s.chars().filter(|c| *c != '-' && *c != '_').collect() } + +fn parse_run_id(value: &str) -> Option { + let value = value.trim(); + (!value.is_empty()).then_some(value)?.parse().ok() +} + +fn run_id_matches(run_id: RunId, prefix: &str) -> bool { + run_id.to_string().starts_with(prefix) +} diff --git a/lib/crates/fabro-workflows/src/run_options.rs b/lib/crates/fabro-workflows/src/run_options.rs index eb541526c..4c8f1cb93 100644 --- a/lib/crates/fabro-workflows/src/run_options.rs +++ b/lib/crates/fabro-workflows/src/run_options.rs @@ -5,6 +5,7 @@ use std::sync::atomic::AtomicBool; use fabro_config::FabroSettings; use fabro_config::run::PullRequestSettings; +use fabro_types::RunId; /// Git checkpoint options for a workflow run. #[derive(Clone)] @@ -21,7 +22,7 @@ pub struct RunOptions { pub run_dir: PathBuf, pub cancel_token: Option>, /// Unique identifier for this workflow run. - pub run_id: String, + pub run_id: RunId, /// User-defined key-value labels for this run. pub labels: HashMap, /// Workflow directory slug (e.g. "smoke" from `fabro/workflows/smoke/`). diff --git a/lib/crates/fabro-workflows/src/sandbox_git.rs b/lib/crates/fabro-workflows/src/sandbox_git.rs index 859daa64c..2c39b205e 100644 --- a/lib/crates/fabro-workflows/src/sandbox_git.rs +++ b/lib/crates/fabro-workflows/src/sandbox_git.rs @@ -2,6 +2,7 @@ use std::path::Path; use fabro_agent::Sandbox; use fabro_git_storage::trailerlink::{self, Trailer}; +use fabro_types::RunId; use crate::asset_snapshot; use crate::git::{GitAuthor, blocking_push_with_timeout, push_ref}; @@ -10,7 +11,7 @@ use fabro_sandbox::daytona::detect_repo_info; /// Captured git state for a workflow run, shared with handlers. #[derive(Debug, Clone)] pub struct GitState { - pub run_id: String, + pub run_id: RunId, pub base_sha: String, pub run_branch: Option, pub meta_branch: Option, diff --git a/lib/crates/fabro-workflows/tests/it/daytona_integration.rs b/lib/crates/fabro-workflows/tests/it/daytona_integration.rs index 8be1f2341..5c7b45930 100644 --- a/lib/crates/fabro-workflows/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflows/tests/it/daytona_integration.rs @@ -4,6 +4,8 @@ //! Run with: `cargo test --package arc-workflows -- --ignored daytona` use std::collections::HashMap; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; use std::path::Path; use std::sync::Arc; @@ -14,6 +16,7 @@ use fabro_llm::provider::Provider; use fabro_sandbox::SandboxRecordExt; use fabro_sandbox::daytona::{DaytonaConfig, DaytonaSandbox, DaytonaSnapshotConfig}; use fabro_store::RuntimeState; +use fabro_types::RunId; use fabro_workflows::artifact::sync_artifacts_to_env; use fabro_workflows::context::Context; use fabro_workflows::error::FabroError; @@ -25,6 +28,13 @@ use fabro_workflows::outcome::{Outcome, OutcomeExt, StageStatus}; use fabro_workflows::records::{Checkpoint, CheckpointExt}; use fabro_workflows::run_options::{GitCheckpointOptions, RunOptions}; use fabro_workflows::test_support::WorkflowRunner; +use ulid::Ulid; + +fn test_run_id(label: &str) -> RunId { + let mut hasher = DefaultHasher::new(); + label.hash(&mut hasher); + RunId::from(Ulid(u128::from(hasher.finish()))) +} async fn create_env() -> DaytonaSandbox { let creds = load_github_app_credentials(); @@ -394,7 +404,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -473,7 +483,7 @@ impl Handler for FileWriterHandler { /// Set up git inside a Daytona sandbox for checkpoint commits. /// Returns (run_id, base_sha, branch_name) on success. -async fn setup_daytona_git(sandbox: &dyn Sandbox) -> (String, String, String) { +async fn setup_daytona_git(sandbox: &dyn Sandbox) -> (RunId, String, String) { // Get current HEAD as base SHA let sha_result = sandbox .exec_command("git rev-parse HEAD", 10_000, None, None, None) @@ -486,7 +496,7 @@ async fn setup_daytona_git(sandbox: &dyn Sandbox) -> (String, String, String) { ); let base_sha = sha_result.stdout.trim().to_string(); - let run_id = ulid::Ulid::new().to_string(); + let run_id = RunId::from(Ulid::new()); let branch_name = format!("fabro/run/{run_id}"); let checkout_cmd = format!("git checkout -b {branch_name}"); @@ -584,7 +594,7 @@ async fn daytona_git_checkpoint_remote_emits_events() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "git-cp-test".into(), + run_id: test_run_id("git-cp-test"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -769,7 +779,7 @@ async fn daytona_parallel_git_branching_e2e() { settings: FabroSettings::default(), run_dir: run_tmp.path().to_path_buf(), cancel_token: None, - run_id: run_id.clone(), + run_id, labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1138,13 +1148,13 @@ async fn daytona_git_checkpoint_with_shadow_branch() { registry.register("start", Box::new(StartHandler)); registry.register("exit", Box::new(ExitHandler)); - let meta_branch = MetadataStore::branch_name(&run_id); + let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let engine = WorkflowRunner::new(registry, Arc::new(EventEmitter::new()), env.clone()); let run_options = RunOptions { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: run_id.clone(), + run_id, labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1164,7 +1174,7 @@ async fn daytona_git_checkpoint_with_shadow_branch() { assert_eq!(outcome.status, StageStatus::Success); // Assert shadow branch on host has checkpoint data - let checkpoint = MetadataStore::read_checkpoint(host_repo.path(), &run_id) + let checkpoint = MetadataStore::read_checkpoint(host_repo.path(), &run_id.to_string()) .expect("read_checkpoint should not error") .expect("shadow branch should contain checkpoint data"); assert!( @@ -1288,7 +1298,7 @@ async fn daytona_asset_collection() { }, run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "asset-test-daytona".into(), + run_id: test_run_id("asset-test-daytona"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1533,7 +1543,7 @@ async fn daytona_git_push_run_branch_to_origin() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: run_id.clone(), + run_id, labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, diff --git a/lib/crates/fabro-workflows/tests/it/integration.rs b/lib/crates/fabro-workflows/tests/it/integration.rs index 6696f6667..45708fffc 100644 --- a/lib/crates/fabro-workflows/tests/it/integration.rs +++ b/lib/crates/fabro-workflows/tests/it/integration.rs @@ -1,4 +1,6 @@ use std::collections::VecDeque; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; use std::path::Path; use std::sync::Arc; use std::time::Duration; @@ -12,6 +14,7 @@ use fabro_interview::{ }; use fabro_llm::provider::Provider; use fabro_store::RuntimeState; +use fabro_types::RunId; use fabro_validate::{Severity, validate, validate_or_raise}; use fabro_workflows::context::Context; use fabro_workflows::error::{FabroError, FailureSignatureExt}; @@ -36,6 +39,7 @@ use fabro_workflows::test_support::{WorkflowRunner, run_graph_with_hooks}; use fabro_workflows::transform::{ StylesheetApplicationTransform, Transform, VariableExpansionTransform, }; +use ulid::Ulid; fn local_env() -> Arc { Arc::new(fabro_agent::LocalSandbox::new( @@ -43,6 +47,12 @@ fn local_env() -> Arc { )) } +fn test_run_id(label: &str) -> RunId { + let mut hasher = DefaultHasher::new(); + label.hash(&mut hasher); + RunId::from(Ulid(u128::from(hasher.finish()))) +} + // --------------------------------------------------------------------------- // 1. Parse and validate all 3 spec examples (Section 2.13) // --------------------------------------------------------------------------- @@ -198,7 +208,7 @@ async fn end_to_end_linear_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -338,7 +348,7 @@ async fn end_to_end_branching_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -457,7 +467,7 @@ async fn end_to_end_human_gate_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -552,7 +562,7 @@ async fn human_gate_aborted_input_fails_closed_without_fail_route() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -662,7 +672,7 @@ async fn human_gate_aborted_input_routes_via_outcome_fail_condition() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -774,7 +784,7 @@ async fn goal_gate_routes_to_retry_target_on_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -894,7 +904,7 @@ async fn goal_gate_routes_to_retry_target_when_present() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1205,7 +1215,7 @@ async fn retry_on_failure_then_succeed() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1279,7 +1289,7 @@ async fn pipeline_with_many_nodes() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1600,7 +1610,7 @@ async fn smoke_test_with_mock_codergen_backend() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1700,7 +1710,7 @@ async fn end_to_end_parallel_fan_out_fan_in() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1812,7 +1822,7 @@ async fn resume_from_checkpoint_completes_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1910,7 +1920,7 @@ async fn resume_from_checkpoint_preserves_goal_gate_outcomes() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1952,7 +1962,7 @@ async fn graph_goal_in_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -1987,7 +1997,7 @@ async fn event_streaming_lifecycle() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2080,7 +2090,7 @@ async fn context_flow_between_stages() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2132,7 +2142,7 @@ async fn tool_handler_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2203,7 +2213,7 @@ async fn auto_approve_interviewer_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2239,7 +2249,7 @@ async fn codergen_without_backend_simulated() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2343,7 +2353,7 @@ async fn branching_loop_back_on_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2425,7 +2435,7 @@ async fn human_gate_loops_back() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2482,7 +2492,7 @@ async fn scenario_ship_a_feature() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2571,7 +2581,7 @@ async fn scenario_parallel_expert_review() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2654,7 +2664,7 @@ async fn scenario_node_retries_on_retry_status() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2715,7 +2725,7 @@ async fn scenario_loop_restart_resets_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2782,7 +2792,7 @@ async fn scenario_bug_triage_router() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2840,7 +2850,7 @@ async fn scenario_crash_recovery() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -2948,7 +2958,7 @@ async fn manager_loop_stop_condition_satisfied_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3026,7 +3036,7 @@ async fn manager_loop_max_cycles_exceeded_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3163,7 +3173,7 @@ async fn conditional_branching_success_fail_paths() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3215,7 +3225,7 @@ async fn edge_selection_condition_match_wins_over_weight() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3261,7 +3271,7 @@ async fn edge_selection_weight_breaks_ties() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3299,7 +3309,7 @@ async fn edge_selection_lexical_tiebreak() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3356,7 +3366,7 @@ async fn context_updates_visible_across_nodes() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3399,7 +3409,7 @@ async fn stylesheet_applies_model_override() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3454,7 +3464,7 @@ async fn custom_handler_registration_and_execution() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3524,7 +3534,7 @@ async fn integration_smoke_plan_implement_review_done() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3633,7 +3643,7 @@ async fn manager_loop_runs_child_engine_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3766,7 +3776,7 @@ async fn manager_loop_context_flows_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3838,7 +3848,7 @@ async fn manager_loop_child_dotfile_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -3939,7 +3949,7 @@ async fn import_e2e_through_engine() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4092,7 +4102,7 @@ async fn fidelity_default_is_compact() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4148,7 +4158,7 @@ async fn fidelity_graph_default_applied() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4200,7 +4210,7 @@ async fn fidelity_node_overrides_graph_default() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4258,7 +4268,7 @@ async fn fidelity_edge_overrides_node_and_graph() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4306,7 +4316,7 @@ async fn fidelity_full_produces_empty_preamble() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4364,7 +4374,7 @@ async fn fidelity_truncate_preamble_minimal() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4435,7 +4445,7 @@ async fn fidelity_summary_low_mode() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4501,7 +4511,7 @@ async fn fidelity_summary_medium_mode() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4567,7 +4577,7 @@ async fn fidelity_summary_high_mode() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4626,7 +4636,7 @@ async fn fidelity_full_sets_thread_id_in_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4696,7 +4706,7 @@ async fn fidelity_full_nodes_share_thread_id() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4776,7 +4786,7 @@ async fn fidelity_resume_degrades_full_to_summary_high() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4872,7 +4882,7 @@ async fn fidelity_resume_degrade_only_affects_first_hop() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4955,7 +4965,7 @@ async fn fidelity_resume_no_degrade_when_not_full() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -4996,7 +5006,7 @@ async fn fidelity_stored_in_checkpoint_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5081,7 +5091,7 @@ async fn fidelity_precedence_multi_node_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5148,7 +5158,7 @@ async fn fidelity_compact_preamble_includes_completed_stages_and_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5222,7 +5232,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { settings: FabroSettings::default(), run_dir: dir_low.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5288,7 +5298,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { settings: FabroSettings::default(), run_dir: dir_med.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5358,7 +5368,7 @@ async fn fidelity_thread_id_fallback_to_previous_node_in_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5411,7 +5421,7 @@ async fn fidelity_thread_id_from_node_class_in_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5467,7 +5477,7 @@ async fn fidelity_edge_thread_id_override_in_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5524,7 +5534,7 @@ async fn fidelity_full_without_explicit_thread_id_uses_previous_node() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5591,7 +5601,7 @@ async fn fidelity_from_parsed_dot_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5638,7 +5648,7 @@ async fn fidelity_checkpoint_roundtrip_preserves_fidelity() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5707,7 +5717,7 @@ async fn fidelity_node_thread_id_overrides_edge_thread_id_in_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5793,7 +5803,7 @@ async fn fidelity_resume_preserves_context_values_across_checkpoint() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -5922,7 +5932,7 @@ mod real_llm { }) } - use super::local_env; + use super::{local_env, test_run_id}; use fabro_graphviz::graph::{AttrValue, Edge, Graph}; use fabro_interview::AutoApproveInterviewer; use fabro_workflows::event::EventEmitter; @@ -6007,7 +6017,7 @@ mod real_llm { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6120,7 +6130,7 @@ mod real_llm { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6258,7 +6268,7 @@ mod real_llm { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6364,7 +6374,7 @@ mod real_llm { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6459,7 +6469,7 @@ async fn human_gate_freeform_only_routes_text() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6588,7 +6598,7 @@ async fn human_gate_freeform_with_fixed_choice_match() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6702,7 +6712,7 @@ async fn human_gate_freeform_fallback_on_unmatched_text() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6829,7 +6839,7 @@ async fn human_gate_freeform_sets_allow_freeform_on_question() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -6936,7 +6946,7 @@ async fn human_gate_without_freeform_sets_allow_freeform_false() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -7220,7 +7230,7 @@ fn make_run_options(dir: &std::path::Path) -> RunOptions { settings: FabroSettings::default(), run_dir: dir.to_path_buf(), cancel_token: None, - run_id: "hook-test-run".into(), + run_id: test_run_id("hook-test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -7639,7 +7649,7 @@ async fn hook_receives_env_vars() { assert!(env_file.exists(), "Env file should be written by hook"); let content = std::fs::read_to_string(&env_file).unwrap(); - // Should contain lines like: event=stage_complete run=hook-test-run wf=HookTest node=work + // Should contain lines like: event=stage_complete run= wf=HookTest node=work let lines: Vec<&str> = content.lines().collect(); let work_line = lines.iter().find(|l| l.contains("node=work")); assert!( @@ -7652,7 +7662,7 @@ async fn hook_receives_env_vars() { "FABRO_EVENT should be set: {line}" ); assert!( - line.contains("run=hook-test-run"), + line.contains(&format!("run={}", test_run_id("hook-test-run"))), "FABRO_RUN_ID should be set: {line}" ); assert!( @@ -8325,7 +8335,7 @@ async fn arc_e2e_with_real_llm() { settings: FabroSettings::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -8452,7 +8462,7 @@ async fn run_fidelity_prompt_pipeline(fidelity: &str) -> String { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -8650,7 +8660,7 @@ async fn large_context_values_are_offloaded_to_artifact_store() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -8864,7 +8874,7 @@ async fn artifact_pointers_rewritten_for_remote_sandbox() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -8993,7 +9003,7 @@ async fn node_dir_uses_visit_count_on_revisit() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -9966,7 +9976,7 @@ async fn full_pipeline_with_cli_backend_node() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -10096,7 +10106,7 @@ async fn stylesheet_backend_property_routes_to_cli() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -10316,8 +10326,9 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { .unwrap(); String::from_utf8_lossy(&out.stdout).trim().to_string() }; + let run_branch = format!("fabro/run/{}", test_run_id("test-docker")); std::process::Command::new("git") - .args(["branch", "fabro/run/test-docker", "HEAD"]) + .args(["branch", &run_branch, "HEAD"]) .current_dir(repo.path()) .output() .unwrap(); @@ -10325,7 +10336,7 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { std::process::Command::new("git") .args(["worktree", "add"]) .arg(&worktree_path) - .arg("fabro/run/test-docker") + .arg(&run_branch) .current_dir(repo.path()) .output() .unwrap(); @@ -10374,7 +10385,7 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { settings: FabroSettings::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "test-docker".into(), + run_id: test_run_id("test-docker"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -10383,7 +10394,7 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { host_repo_path: Some(worktree_path.clone()), git: Some(GitCheckpointOptions { base_sha: Some(base_sha.clone()), - run_branch: Some("fabro/run/test-docker".to_string()), + run_branch: Some(run_branch), meta_branch: None, }), }; @@ -10494,7 +10505,7 @@ async fn git_checkpoint_host_writes_shadow_branch() { .unwrap(); // 2. Create a branch and worktree - let run_id = "test-shadow"; + let run_id = test_run_id("test-shadow"); let base_sha = { let out = std::process::Command::new("git") .args(["rev-parse", "HEAD"]) @@ -10571,12 +10582,12 @@ async fn git_checkpoint_host_writes_shadow_branch() { registry.register("exit", Box::new(ExitHandler)); let engine = WorkflowRunner::new(registry, Arc::new(emitter), env); - let meta_branch = MetadataStore::branch_name(run_id); + let meta_branch = MetadataStore::branch_name(&run_id.to_string()); let run_options = RunOptions { settings: FabroSettings::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: run_id.into(), + run_id, labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -10597,7 +10608,7 @@ async fn git_checkpoint_host_writes_shadow_branch() { assert_eq!(outcome.status, StageStatus::Success); // 6. Assert shadow branch has checkpoint data on the host repo - let checkpoint = MetadataStore::read_checkpoint(repo.path(), run_id) + let checkpoint = MetadataStore::read_checkpoint(repo.path(), &run_id.to_string()) .expect("read_checkpoint should not error") .expect("shadow branch should contain checkpoint data"); assert!( @@ -10630,7 +10641,7 @@ async fn git_checkpoint_host_writes_shadow_branch() { ); // 8. Verify round-trip: shadow checkpoint's completed_nodes matches expected - let run_record = MetadataStore::read_run_record(repo.path(), run_id) + let run_record = MetadataStore::read_run_record(repo.path(), &run_id.to_string()) .expect("read_run_record should not error") .expect("shadow branch should contain run record"); assert_eq!(run_record.run_id, run_id); @@ -10686,7 +10697,7 @@ async fn parallel_git_branching_host_e2e() { .unwrap(); String::from_utf8_lossy(&out.stdout).trim().to_string() }; - let run_id = "par-git-test"; + let run_id = test_run_id("par-git-test"); let run_branch = format!("fabro/run/{run_id}"); std::process::Command::new("git") .args(["branch", &run_branch, "HEAD"]) @@ -10773,7 +10784,7 @@ async fn parallel_git_branching_host_e2e() { settings: FabroSettings::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: run_id.into(), + run_id, labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -10980,8 +10991,9 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { .unwrap(); String::from_utf8_lossy(&out.stdout).trim().to_string() }; + let run_branch = format!("fabro/run/{}", test_run_id("empty-diff")); std::process::Command::new("git") - .args(["branch", "fabro/run/empty-diff", "HEAD"]) + .args(["branch", &run_branch, "HEAD"]) .current_dir(repo.path()) .output() .unwrap(); @@ -10989,7 +11001,7 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { std::process::Command::new("git") .args(["worktree", "add"]) .arg(&worktree_path) - .arg("fabro/run/empty-diff") + .arg(&run_branch) .current_dir(repo.path()) .output() .unwrap(); @@ -11035,7 +11047,7 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { settings: FabroSettings::default(), run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "empty-diff".into(), + run_id: test_run_id("empty-diff"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11044,7 +11056,7 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { host_repo_path: Some(worktree_path.clone()), git: Some(GitCheckpointOptions { base_sha: Some(base_sha.clone()), - run_branch: Some("fabro/run/empty-diff".to_string()), + run_branch: Some(run_branch), meta_branch: None, }), }; @@ -11416,7 +11428,7 @@ async fn e2e_circuit_breaker_deterministic_self_loop() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-circuit-breaker".into(), + run_id: test_run_id("e2e-circuit-breaker"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11462,7 +11474,7 @@ async fn e2e_circuit_breaker_custom_limit() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-custom-limit".into(), + run_id: test_run_id("e2e-custom-limit"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11501,7 +11513,7 @@ async fn e2e_circuit_breaker_ignores_transient_failures() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-transient-no-breaker".into(), + run_id: test_run_id("e2e-transient-no-breaker"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11547,7 +11559,7 @@ async fn e2e_circuit_breaker_different_reasons_separate_counters() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-varying-reasons".into(), + run_id: test_run_id("e2e-varying-reasons"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11586,7 +11598,7 @@ async fn e2e_circuit_breaker_loop_restart() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-breaker".into(), + run_id: test_run_id("e2e-restart-breaker"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11647,7 +11659,7 @@ async fn e2e_failure_signature_persisted_in_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-sig-context".into(), + run_id: test_run_id("e2e-sig-context"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11710,7 +11722,7 @@ async fn e2e_failure_signature_hint_overrides_reason_in_context() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-sig-hint".into(), + run_id: test_run_id("e2e-sig-hint"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11765,7 +11777,7 @@ async fn e2e_signature_maps_persist_in_checkpoint() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-sig-persist".into(), + run_id: test_run_id("e2e-sig-persist"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11891,7 +11903,7 @@ async fn e2e_circuit_breaker_emits_events_before_abort() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-events".into(), + run_id: test_run_id("e2e-events"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -11957,7 +11969,7 @@ async fn e2e_circuit_breaker_does_not_fire_below_limit() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-below-limit".into(), + run_id: test_run_id("e2e-below-limit"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12052,7 +12064,7 @@ async fn e2e_circuit_breaker_multi_stage_impl_verify_cycle() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-impl-verify-cycle".into(), + run_id: test_run_id("e2e-impl-verify-cycle"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12148,7 +12160,7 @@ async fn e2e_loop_restart_blocked_for_deterministic_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-blocked-det".into(), + run_id: test_run_id("e2e-restart-blocked-det"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12187,7 +12199,7 @@ async fn e2e_loop_restart_blocked_for_structural_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-blocked-struct".into(), + run_id: test_run_id("e2e-restart-blocked-struct"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12226,7 +12238,7 @@ async fn e2e_loop_restart_blocked_for_budget_exhausted_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-blocked-budget".into(), + run_id: test_run_id("e2e-restart-blocked-budget"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12265,7 +12277,7 @@ async fn e2e_loop_restart_blocked_for_canceled_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-blocked-canceled".into(), + run_id: test_run_id("e2e-restart-blocked-canceled"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12301,7 +12313,7 @@ async fn e2e_loop_restart_blocked_for_compilation_loop_failure() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-blocked-comploop".into(), + run_id: test_run_id("e2e-restart-blocked-comploop"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12341,7 +12353,7 @@ async fn e2e_loop_restart_allowed_for_transient_infra() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "e2e-restart-allowed-transient".into(), + run_id: test_run_id("e2e-restart-allowed-transient"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12444,7 +12456,7 @@ async fn e2e_stall_watchdog_triggers_from_dot_parsed_pipeline() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "stall-e2e".into(), + run_id: test_run_id("stall-e2e"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12499,7 +12511,7 @@ async fn e2e_stall_watchdog_kept_alive_by_handler_events() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "stall-alive-e2e".into(), + run_id: test_run_id("stall-alive-e2e"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12544,7 +12556,7 @@ async fn e2e_stall_watchdog_disabled_with_zero_timeout() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "stall-disabled-e2e".into(), + run_id: test_run_id("stall-disabled-e2e"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12608,7 +12620,7 @@ async fn e2e_stall_watchdog_with_explicit_timeout_override() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "stall-override-e2e".into(), + run_id: test_run_id("stall-override-e2e"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12743,7 +12755,7 @@ async fn asset_collection_local_sandbox_success() { }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "asset-test-local".into(), + run_id: test_run_id("asset-test-local"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12868,7 +12880,7 @@ async fn asset_collection_local_sandbox_on_failure() { }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "asset-test-fail".into(), + run_id: test_run_id("asset-test-fail"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -12959,7 +12971,7 @@ async fn asset_collection_docker_sandbox() { }, run_dir: run_dir.path().to_path_buf(), cancel_token: None, - run_id: "asset-test-docker".into(), + run_id: test_run_id("asset-test-docker"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None, @@ -13022,7 +13034,7 @@ async fn wait_timer_e2e() { settings: FabroSettings::default(), run_dir: dir.path().to_path_buf(), cancel_token: None, - run_id: "test-run".into(), + run_id: test_run_id("test-run"), labels: std::collections::HashMap::new(), workflow_slug: None, github_app: None,