mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
Fix workflow slug lookup for split and resumed runs
This commit is contained in:
parent
b5c014443e
commit
f5d363237a
4 changed files with 210 additions and 23 deletions
|
|
@ -6,8 +6,7 @@ use fabro_workflows::manifest::Manifest;
|
|||
use fabro_workflows::run_spec::RunSpec;
|
||||
|
||||
use super::run::{
|
||||
cached_graph_path, default_run_dir, prepare_workflow, workflow_slug_from_path,
|
||||
write_run_config_snapshot, RunArgs,
|
||||
cached_graph_path, default_run_dir, prepare_workflow, write_run_config_snapshot, RunArgs,
|
||||
};
|
||||
use fabro_util::terminal::Styles;
|
||||
|
||||
|
|
@ -102,7 +101,7 @@ pub async fn create_run(
|
|||
base_sha: None,
|
||||
labels,
|
||||
base_branch,
|
||||
workflow_slug: workflow_slug_from_path(workflow_path),
|
||||
workflow_slug: prep.workflow_slug.clone(),
|
||||
host_repo_path: Some(working_directory.to_string_lossy().to_string()),
|
||||
};
|
||||
manifest.save(&run_dir.join("manifest.json"))?;
|
||||
|
|
|
|||
|
|
@ -206,6 +206,7 @@ async fn prepare_from_checkpoint(
|
|||
let graph = prepared.graph;
|
||||
let run_cfg = prepared.run_cfg;
|
||||
let sandbox_provider = prepared.sandbox_provider;
|
||||
let workflow_slug = prepared.workflow_slug;
|
||||
|
||||
eprintln!(
|
||||
"{} {} from checkpoint {}",
|
||||
|
|
@ -437,7 +438,7 @@ async fn prepare_from_checkpoint(
|
|||
.or(run_defaults.assets.as_ref())
|
||||
.map(|a| a.include.clone())
|
||||
.unwrap_or_default(),
|
||||
workflow_slug: None,
|
||||
workflow_slug,
|
||||
};
|
||||
|
||||
let devcontainer_env = devcontainer_config
|
||||
|
|
@ -521,7 +522,7 @@ async fn prepare_from_branch(
|
|||
.or_else(|| repo_info.as_ref().and_then(|(_, branch)| branch.clone()));
|
||||
let base_sha = manifest.as_ref().and_then(|m| m.base_sha.clone());
|
||||
|
||||
let (graph, graph_source, run_cfg, mut sandbox_provider) =
|
||||
let (graph, graph_source, run_cfg, mut sandbox_provider, workflow_slug) =
|
||||
if let Some(ref workflow_path) = args.workflow {
|
||||
let prepared = prepare_workflow_with_project_config(
|
||||
&resume_as_run_args(args, workflow_path.clone()),
|
||||
|
|
@ -535,6 +536,7 @@ async fn prepare_from_branch(
|
|||
prepared.source,
|
||||
prepared.run_cfg,
|
||||
prepared.sandbox_provider,
|
||||
prepared.workflow_slug,
|
||||
)
|
||||
} else {
|
||||
let (graph, diagnostics) =
|
||||
|
|
@ -549,7 +551,13 @@ async fn prepare_from_branch(
|
|||
} else {
|
||||
resolve_sandbox_provider(args.sandbox.map(Into::into), None, run_defaults)?
|
||||
};
|
||||
(graph, source.clone(), None, sandbox_provider)
|
||||
(
|
||||
graph,
|
||||
source.clone(),
|
||||
None,
|
||||
sandbox_provider,
|
||||
manifest.as_ref().and_then(|m| m.workflow_slug.clone()),
|
||||
)
|
||||
};
|
||||
|
||||
eprintln!(
|
||||
|
|
@ -826,7 +834,7 @@ async fn prepare_from_branch(
|
|||
.or(run_defaults.assets.as_ref())
|
||||
.map(|a| a.include.clone())
|
||||
.unwrap_or_default(),
|
||||
workflow_slug: None,
|
||||
workflow_slug,
|
||||
};
|
||||
|
||||
let devcontainer_env = devcontainer_config
|
||||
|
|
|
|||
|
|
@ -193,14 +193,21 @@ pub(crate) fn default_run_dir(run_id: &str, dry_run: bool) -> PathBuf {
|
|||
}
|
||||
|
||||
pub(crate) fn workflow_slug_from_path(workflow_path: &Path) -> Option<String> {
|
||||
let file_name = workflow_path.file_name()?.to_string_lossy();
|
||||
if workflow_path.extension().is_none() {
|
||||
Some(workflow_path.to_string_lossy().into_owned())
|
||||
} else {
|
||||
workflow_path
|
||||
return Some(file_name.into_owned());
|
||||
}
|
||||
|
||||
let file_stem = workflow_path.file_stem()?.to_string_lossy();
|
||||
if file_stem == "workflow" {
|
||||
return workflow_path
|
||||
.parent()
|
||||
.and_then(|p| p.file_name())
|
||||
.map(|n| n.to_string_lossy().into_owned())
|
||||
.or_else(|| Some(file_stem.into_owned()));
|
||||
}
|
||||
|
||||
Some(file_stem.into_owned())
|
||||
}
|
||||
|
||||
fn is_cached_run_restart(workflow_path: &Path, run_dir: &Path) -> bool {
|
||||
|
|
@ -504,13 +511,13 @@ pub(crate) async fn write_run_config_snapshot(
|
|||
|
||||
pub(crate) fn resolve_workflow_source(
|
||||
workflow_path: &Path,
|
||||
) -> anyhow::Result<(PathBuf, Option<WorkflowRunConfig>)> {
|
||||
) -> anyhow::Result<(PathBuf, PathBuf, Option<WorkflowRunConfig>)> {
|
||||
let path = project_config::resolve_workflow_arg(workflow_path)?;
|
||||
if path.extension().is_some_and(|ext| ext == "toml") {
|
||||
match run_config::load_run_config(&path) {
|
||||
Ok(cfg) => {
|
||||
let dot = run_config::resolve_graph_path(&path, &cfg.graph);
|
||||
Ok((dot, Some(cfg)))
|
||||
Ok((path, dot, Some(cfg)))
|
||||
}
|
||||
// Backward compatibility for detached runs created before run.toml existed.
|
||||
// Use path.exists() to distinguish a genuinely missing run.toml from one
|
||||
|
|
@ -519,12 +526,12 @@ pub(crate) fn resolve_workflow_source(
|
|||
if !path.exists()
|
||||
&& path.starts_with(fabro_workflows::run_lookup::default_runs_base()) =>
|
||||
{
|
||||
Ok((path.with_file_name(RUN_GRAPH_FILE), None))
|
||||
Ok((path.clone(), path.with_file_name(RUN_GRAPH_FILE), None))
|
||||
}
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
} else {
|
||||
Ok((path, None))
|
||||
Ok((path.clone(), path, None))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -536,6 +543,7 @@ pub(crate) struct PreparedWorkflow {
|
|||
pub sandbox_provider: SandboxProvider,
|
||||
pub model: String,
|
||||
pub provider: Option<String>,
|
||||
pub workflow_slug: Option<String>,
|
||||
pub run_defaults: RunDefaults,
|
||||
}
|
||||
|
||||
|
|
@ -575,16 +583,17 @@ pub(crate) fn prepare_workflow_with_project_config(
|
|||
}
|
||||
|
||||
// Resolve workflow arg, load run config if TOML, apply defaults
|
||||
let (dot_path, run_cfg) = {
|
||||
let (dot, cfg) = resolve_workflow_source(workflow_path)?;
|
||||
let (resolved_workflow_path, dot_path, run_cfg) = {
|
||||
let (resolved, dot, cfg) = resolve_workflow_source(workflow_path)?;
|
||||
match cfg {
|
||||
Some(mut cfg) => {
|
||||
cfg.apply_defaults(&run_defaults);
|
||||
(dot, Some(cfg))
|
||||
(resolved, dot, Some(cfg))
|
||||
}
|
||||
None => (dot, None),
|
||||
None => (resolved, dot, None),
|
||||
}
|
||||
};
|
||||
let workflow_slug = workflow_slug_from_path(&resolved_workflow_path);
|
||||
|
||||
let directory = run_cfg
|
||||
.as_ref()
|
||||
|
|
@ -682,6 +691,7 @@ pub(crate) fn prepare_workflow_with_project_config(
|
|||
sandbox_provider,
|
||||
model,
|
||||
provider,
|
||||
workflow_slug,
|
||||
run_defaults,
|
||||
})
|
||||
}
|
||||
|
|
@ -705,6 +715,7 @@ pub async fn run_command(
|
|||
sandbox_provider,
|
||||
model,
|
||||
provider,
|
||||
workflow_slug: prepared_workflow_slug,
|
||||
run_defaults,
|
||||
} = prepare_workflow(&args, run_defaults, styles, false)?;
|
||||
|
||||
|
|
@ -756,10 +767,13 @@ pub async fn run_command(
|
|||
} else {
|
||||
None
|
||||
};
|
||||
let workflow_slug = existing_manifest
|
||||
.as_ref()
|
||||
.and_then(|manifest| manifest.workflow_slug.clone())
|
||||
.or_else(|| workflow_slug_from_path(workflow_path));
|
||||
let workflow_slug = if cached_run_restart {
|
||||
existing_manifest
|
||||
.as_ref()
|
||||
.and_then(|manifest| manifest.workflow_slug.clone())
|
||||
} else {
|
||||
prepared_workflow_slug
|
||||
};
|
||||
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?;
|
||||
|
|
@ -2730,7 +2744,7 @@ mod tests {
|
|||
let dir = tempfile::tempdir_in(&runs_base).unwrap();
|
||||
std::fs::write(dir.path().join(RUN_GRAPH_FILE), "digraph test {}").unwrap();
|
||||
|
||||
let (dot_path, run_cfg) =
|
||||
let (_resolved_path, dot_path, run_cfg) =
|
||||
resolve_workflow_source(&dir.path().join(RUN_CONFIG_FILE)).unwrap();
|
||||
|
||||
assert_eq!(dot_path, dir.path().join(RUN_GRAPH_FILE));
|
||||
|
|
@ -2746,6 +2760,42 @@ mod tests {
|
|||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_slug_from_path_uses_file_stem_for_standalone_files() {
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("/tmp/alpha.fabro")).as_deref(),
|
||||
Some("alpha")
|
||||
);
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("/tmp/beta.toml")).as_deref(),
|
||||
Some("beta")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_slug_from_path_uses_parent_for_workflow_files() {
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("/tmp/sluggy/workflow.fabro")).as_deref(),
|
||||
Some("sluggy")
|
||||
);
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("/tmp/sluggy/workflow.toml")).as_deref(),
|
||||
Some("sluggy")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_slug_from_path_uses_final_component_for_extensionless_inputs() {
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("implement-issue")).as_deref(),
|
||||
Some("implement-issue")
|
||||
);
|
||||
assert_eq!(
|
||||
workflow_slug_from_path(Path::new("nested/repl")).as_deref(),
|
||||
Some("repl")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prepare_workflow_with_project_config_resolves_workflow_toml_settings() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -687,6 +687,136 @@ digraph BarBaz {
|
|||
assert_eq!(manifest["workflow_slug"].as_str(), Some("sluggy"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
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");
|
||||
std::fs::write(
|
||||
&workflow_path,
|
||||
"\
|
||||
digraph FooWorkflow {
|
||||
start [shape=Mdiamond, label=\"Start\"]
|
||||
exit [shape=Msquare, label=\"Exit\"]
|
||||
start -> exit
|
||||
}
|
||||
",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.args([
|
||||
"create",
|
||||
"--dry-run",
|
||||
"--auto-approve",
|
||||
"--run-id",
|
||||
"opaque-run-alpha",
|
||||
workflow_path.to_str().unwrap(),
|
||||
])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.args(["start", "alpha"])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.args(["attach", "alpha"])
|
||||
.timeout(std::time::Duration::from_secs(10))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let run_dir = find_run_dir(home.path(), "opaque-run-alpha");
|
||||
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("FooWorkflow"));
|
||||
assert_eq!(manifest["workflow_slug"].as_str(), Some("alpha"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resumed_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();
|
||||
let original_run_dir = project.path().join("original-run");
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.current_dir(project.path())
|
||||
.args([
|
||||
"run",
|
||||
"--dry-run",
|
||||
"--auto-approve",
|
||||
"--no-retro",
|
||||
"--run-dir",
|
||||
original_run_dir.to_str().unwrap(),
|
||||
workflow_path.to_str().unwrap(),
|
||||
])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.current_dir(project.path())
|
||||
.args([
|
||||
"resume",
|
||||
"--checkpoint",
|
||||
original_run_dir.join("checkpoint.json").to_str().unwrap(),
|
||||
"--workflow",
|
||||
workflow_path.to_str().unwrap(),
|
||||
"--dry-run",
|
||||
"--auto-approve",
|
||||
"--no-retro",
|
||||
])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
arc()
|
||||
.env("HOME", home.path())
|
||||
.current_dir(project.path())
|
||||
.args(["attach", "sluggy"])
|
||||
.timeout(std::time::Duration::from_secs(10))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let resumed_runs_dir = home.path().join(".fabro").join("runs");
|
||||
let resumed_run_dir = std::fs::read_dir(&resumed_runs_dir)
|
||||
.unwrap()
|
||||
.flatten()
|
||||
.map(|entry| entry.path())
|
||||
.find(|path| path.is_dir())
|
||||
.unwrap_or_else(|| {
|
||||
panic!(
|
||||
"expected a resumed run under {}",
|
||||
resumed_runs_dir.display()
|
||||
)
|
||||
});
|
||||
let manifest: serde_json::Value = serde_json::from_str(
|
||||
&std::fs::read_to_string(resumed_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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue