Preserve split run metadata across restarts

This commit is contained in:
Bryan Helmkamp 2026-03-23 10:04:49 -04:00
parent 6f0023698b
commit b5c014443e
No known key found for this signature in database
3 changed files with 148 additions and 12 deletions

View file

@ -568,7 +568,7 @@ async fn prepare_from_branch(
} else if args.dry_run {
default_run_dir(&run_id, true)
} else {
find_existing_run_dir(&run_id).unwrap_or_else(|| default_run_dir(&run_id, false))
find_existing_run_dir(&run_id, false).unwrap_or_else(|| default_run_dir(&run_id, false))
};
tokio::fs::create_dir_all(&run_dir).await?;
let run_dir = tokio::fs::canonicalize(&run_dir).await.unwrap_or(run_dir);
@ -1503,20 +1503,35 @@ async fn run_resumed(
}
}
/// Scan `~/.fabro/runs/` for an existing directory whose name ends with `-{run_id}`.
fn find_existing_run_dir(run_id: &str) -> Option<PathBuf> {
let base = dirs::home_dir()?.join(".fabro").join("runs");
/// Scan a runs directory for an existing directory matching the requested dry-run mode.
fn find_existing_run_dir_in(
base: &std::path::Path,
run_id: &str,
dry_run: bool,
) -> Option<PathBuf> {
let suffix = format!("-{run_id}");
let entries = std::fs::read_dir(&base).ok()?;
let entries = std::fs::read_dir(base).ok()?;
for entry in entries.flatten() {
let name = entry.file_name();
if name.to_string_lossy().ends_with(&suffix) && entry.path().is_dir() {
let name = name.to_string_lossy();
if entry.path().is_dir() && run_dir_name_matches_mode(&name, &suffix, dry_run) {
return Some(entry.path());
}
}
None
}
/// Scan `~/.fabro/runs/` for an existing directory matching the requested dry-run mode.
fn find_existing_run_dir(run_id: &str, dry_run: bool) -> Option<PathBuf> {
let base = dirs::home_dir()?.join(".fabro").join("runs");
find_existing_run_dir_in(&base, run_id, dry_run)
}
fn run_dir_name_matches_mode(name: &str, run_id_suffix: &str, dry_run: bool) -> bool {
name.strip_suffix(run_id_suffix)
.is_some_and(|prefix| prefix.ends_with("-dry-run") == dry_run)
}
#[cfg(test)]
mod tests {
use super::*;
@ -1561,6 +1576,43 @@ mod tests {
assert_eq!(selected, cwd.path());
}
#[test]
fn find_existing_run_dir_in_respects_dry_run_mode() {
let runs = tempfile::tempdir().unwrap();
let non_dry = runs.path().join("20260323-run-1");
let dry = runs.path().join("20260323-dry-run-run-1");
std::fs::create_dir_all(&non_dry).unwrap();
std::fs::create_dir_all(&dry).unwrap();
assert_eq!(
find_existing_run_dir_in(runs.path(), "run-1", false),
Some(non_dry)
);
assert_eq!(
find_existing_run_dir_in(runs.path(), "run-1", true),
Some(dry)
);
}
#[test]
fn find_existing_run_dir_in_does_not_misclassify_run_ids_containing_dry_run() {
let runs = tempfile::tempdir().unwrap();
let run_id = "feature-dry-run-fix";
let non_dry = runs.path().join(format!("20260323-{run_id}"));
let dry = runs.path().join(format!("20260323-dry-run-{run_id}"));
std::fs::create_dir_all(&non_dry).unwrap();
std::fs::create_dir_all(&dry).unwrap();
assert_eq!(
find_existing_run_dir_in(runs.path(), run_id, false),
Some(non_dry)
);
assert_eq!(
find_existing_run_dir_in(runs.path(), run_id, true),
Some(dry)
);
}
#[test]
fn resume_bootstrap_guard_marks_failed_on_drop() {
let dir = tempfile::tempdir().unwrap();

View file

@ -25,6 +25,7 @@ use fabro_workflows::engine::{RunConfig, WorkflowRunEngine};
use fabro_workflows::event::{EventEmitter, RunNoticeLevel, WorkflowRunEvent};
use fabro_workflows::git::GitSyncStatus;
use fabro_workflows::handler::default_registry;
use fabro_workflows::manifest::Manifest;
use fabro_workflows::outcome::{Outcome, StageStatus};
use fabro_workflows::run_status::{RunStatus, StatusReason};
use fabro_workflows::sandbox_provider::SandboxProvider;
@ -202,6 +203,13 @@ pub(crate) fn workflow_slug_from_path(workflow_path: &Path) -> Option<String> {
}
}
fn is_cached_run_restart(workflow_path: &Path, run_dir: &Path) -> bool {
workflow_path.starts_with(run_dir)
&& workflow_path.file_name().is_some_and(|f| {
f == std::ffi::OsStr::new(RUN_CONFIG_FILE) || f == std::ffi::OsStr::new(RUN_GRAPH_FILE)
})
}
/// Resolve model and provider through the full precedence chain:
/// CLI flag > TOML config > run defaults > DOT graph attrs > provider-specific defaults.
/// Then resolve through the catalog for alias expansion.
@ -700,11 +708,7 @@ pub async fn run_command(
run_defaults,
} = prepare_workflow(&args, run_defaults, styles, false)?;
// Extract workflow slug from the workflow path argument.
// If bare name (no extension, e.g. "smoke"), use it directly.
// Otherwise derive from the parent directory of the resolved .toml path.
let workflow_path = args.workflow.as_ref().unwrap(); // safe: prepare_workflow validated
let workflow_slug = workflow_slug_from_path(workflow_path);
// Collect setup commands — they'll be run inside the sandbox
let setup_commands: Vec<String> = run_cfg
@ -746,6 +750,16 @@ pub async fn run_command(
.run_dir
.unwrap_or_else(|| default_run_dir(&run_id, args.dry_run));
tokio::fs::create_dir_all(&run_dir).await?;
let cached_run_restart = is_cached_run_restart(workflow_path, &run_dir);
let existing_manifest = if cached_run_restart {
Manifest::load(&run_dir.join("manifest.json")).ok()
} else {
None
};
let workflow_slug = existing_manifest
.as_ref()
.and_then(|manifest| manifest.workflow_slug.clone())
.or_else(|| workflow_slug_from_path(workflow_path));
fabro_util::run_log::activate(&run_dir.join("cli.log"))
.context("Failed to activate per-run log")?;
tokio::fs::write(cached_graph_path(&run_dir), &source).await?;
@ -1467,7 +1481,10 @@ pub async fn run_command(
dry_run: dry_run_mode,
run_id: run_id.clone(),
git_checkpoint_enabled: worktree_path.is_some(),
host_repo_path: Some(original_cwd.clone()),
host_repo_path: existing_manifest
.as_ref()
.and_then(|manifest| manifest.host_repo_path.as_deref().map(PathBuf::from))
.or_else(|| Some(original_cwd.clone())),
base_sha: worktree_base_sha,
run_branch: worktree_branch,
meta_branch,
@ -1480,7 +1497,10 @@ pub async fn run_command(
checkpoint_exclude_globs,
github_app: github_app.clone(),
git_author,
base_branch: detected_base_branch,
base_branch: existing_manifest
.as_ref()
.and_then(|manifest| manifest.base_branch.clone())
.or(detected_base_branch),
pull_request: run_cfg
.as_ref()
.and_then(|c| c.pull_request.as_ref())

View file

@ -623,6 +623,70 @@ fn find_run_dir(home: &std::path::Path, run_id: &str) -> std::path::PathBuf {
})
}
#[test]
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");
std::fs::create_dir_all(&workflow_dir).unwrap();
let workflow_path = workflow_dir.join("workflow.fabro");
std::fs::write(
&workflow_path,
"\
digraph BarBaz {
start [shape=Mdiamond, label=\"Start\"]
exit [shape=Msquare, label=\"Exit\"]
start -> exit
}
",
)
.unwrap();
arc()
.env("HOME", home.path())
.current_dir(project.path())
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
"opaque-run-999",
workflow_path.to_str().unwrap(),
])
.assert()
.success();
arc()
.env("HOME", home.path())
.current_dir(project.path())
.args(["start", "sluggy"])
.assert()
.success();
arc()
.env("HOME", home.path())
.current_dir(project.path())
.args(["attach", "opaque-run-999"])
.timeout(std::time::Duration::from_secs(10))
.assert()
.success();
arc()
.env("HOME", home.path())
.current_dir(project.path())
.args(["attach", "sluggy"])
.timeout(std::time::Duration::from_secs(10))
.assert()
.success();
let run_dir = find_run_dir(home.path(), "opaque-run-999");
let manifest: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(run_dir.join("manifest.json")).unwrap())
.unwrap();
assert_eq!(manifest["workflow_name"].as_str(), Some("BarBaz"));
assert_eq!(manifest["workflow_slug"].as_str(), Some("sluggy"));
}
#[test]
fn dry_run_create_start_attach_works_with_default_run_lookup() {
let home = tempfile::tempdir().unwrap();