mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
Unify workflow resolution for validate and parse commands
Extract resolve_workflow() in project_config to handle bare name, .toml, and .dot resolution in one place. Previously validate and parse treated bare words as literal file paths while run resolved them via arc.toml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
943779c1e0
commit
17e384de24
4 changed files with 151 additions and 21 deletions
|
|
@ -13,7 +13,8 @@ pub fn parse_command(args: &ParseArgs) -> anyhow::Result<()> {
|
|||
}
|
||||
|
||||
fn parse_command_to(args: &ParseArgs, mut out: impl Write) -> anyhow::Result<()> {
|
||||
let source = read_dot_file(&args.workflow)?;
|
||||
let (dot_path, _cfg) = super::project_config::resolve_workflow(&args.workflow)?;
|
||||
let source = read_dot_file(&dot_path)?;
|
||||
let ast = crate::parser::parse_ast(&source)?;
|
||||
serde_json::to_writer_pretty(&mut out, &ast)?;
|
||||
writeln!(out)?;
|
||||
|
|
@ -29,7 +30,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn parse_command_outputs_json_ast() {
|
||||
let mut tmp = tempfile::NamedTempFile::new().unwrap();
|
||||
let mut tmp = tempfile::Builder::new().suffix(".dot").tempfile().unwrap();
|
||||
write!(
|
||||
tmp,
|
||||
r#"digraph Hello {{
|
||||
|
|
@ -53,7 +54,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn parse_command_rejects_invalid_dot() {
|
||||
let mut tmp = tempfile::NamedTempFile::new().unwrap();
|
||||
let mut tmp = tempfile::Builder::new().suffix(".dot").tempfile().unwrap();
|
||||
write!(tmp, "not a valid dot file").unwrap();
|
||||
|
||||
let args = ParseArgs {
|
||||
|
|
@ -63,6 +64,36 @@ mod tests {
|
|||
assert!(result.is_err(), "expected Err for invalid syntax");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_toml_path() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let wf_dir = tmp.path().join("workflows").join("hello");
|
||||
std::fs::create_dir_all(&wf_dir).unwrap();
|
||||
std::fs::write(
|
||||
wf_dir.join("workflow.toml"),
|
||||
"version = 1\ngraph = \"workflow.dot\"\n",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(
|
||||
wf_dir.join("workflow.dot"),
|
||||
r#"digraph Hello {
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
start -> exit
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let args = ParseArgs {
|
||||
workflow: wf_dir.join("workflow.toml"),
|
||||
};
|
||||
let mut buf = Vec::new();
|
||||
parse_command_to(&args, &mut buf).unwrap();
|
||||
|
||||
let deserialized: DotGraph = serde_json::from_slice(&buf).unwrap();
|
||||
assert_eq!(deserialized.name, "Hello");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_command_rejects_missing_file() {
|
||||
let args = ParseArgs {
|
||||
|
|
|
|||
|
|
@ -166,6 +166,31 @@ fn find_closest_match(input: &str, candidates: &[String]) -> Option<String> {
|
|||
.map(|(name, _)| name.clone())
|
||||
}
|
||||
|
||||
/// Resolve a workflow argument to a DOT path and optional run config.
|
||||
///
|
||||
/// Calls `resolve_workflow_arg` first, then if the result is a `.toml` file,
|
||||
/// loads the run config and resolves the graph path within it.
|
||||
pub fn resolve_workflow(
|
||||
arg: &Path,
|
||||
) -> anyhow::Result<(PathBuf, Option<super::run_config::WorkflowRunConfig>)> {
|
||||
let start = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
resolve_workflow_from(arg, &start)
|
||||
}
|
||||
|
||||
fn resolve_workflow_from(
|
||||
arg: &Path,
|
||||
start_dir: &Path,
|
||||
) -> anyhow::Result<(PathBuf, Option<super::run_config::WorkflowRunConfig>)> {
|
||||
let path = resolve_workflow_arg_from(arg, start_dir)?;
|
||||
if path.extension().is_some_and(|ext| ext == "toml") {
|
||||
let cfg = super::run_config::load_run_config(&path)?;
|
||||
let dot = super::run_config::resolve_graph_path(&path, &cfg.graph);
|
||||
Ok((dot, Some(cfg)))
|
||||
} else {
|
||||
Ok((path, None))
|
||||
}
|
||||
}
|
||||
|
||||
/// Check whether retros are enabled in the project config.
|
||||
/// Returns `true` (the default) if no config is found or on error.
|
||||
pub fn is_retro_enabled() -> bool {
|
||||
|
|
@ -412,4 +437,53 @@ mod tests {
|
|||
let result = resolve_workflow_arg_from(Path::new("factory"), tmp.path()).unwrap();
|
||||
assert_eq!(result, wf_dir.join("workflow.toml"));
|
||||
}
|
||||
|
||||
/// Helper: create a temp dir with arc.toml + workflows/{name}/{workflow.toml, workflow.dot}
|
||||
/// and chdir into it so `resolve_workflow` (which uses cwd) can find the config.
|
||||
fn setup_workflow_project(name: &str) -> (TempDir, PathBuf) {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
fs::write(tmp.path().join("arc.toml"), "version = 1\n").unwrap();
|
||||
let wf_dir = tmp.path().join("workflows").join(name);
|
||||
fs::create_dir_all(&wf_dir).unwrap();
|
||||
fs::write(
|
||||
wf_dir.join("workflow.toml"),
|
||||
"version = 1\ngraph = \"workflow.dot\"\n",
|
||||
)
|
||||
.unwrap();
|
||||
fs::write(
|
||||
wf_dir.join("workflow.dot"),
|
||||
"digraph G { start [shape=Mdiamond]; exit [shape=Msquare]; start -> exit }",
|
||||
)
|
||||
.unwrap();
|
||||
let dot_path = wf_dir.join("workflow.dot");
|
||||
(tmp, dot_path)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_workflow_bare_name() {
|
||||
let (tmp, expected_dot) = setup_workflow_project("hello");
|
||||
let (dot_path, cfg) = resolve_workflow_from(Path::new("hello"), tmp.path()).unwrap();
|
||||
assert_eq!(
|
||||
dot_path.canonicalize().unwrap(),
|
||||
expected_dot.canonicalize().unwrap()
|
||||
);
|
||||
assert!(cfg.is_some(), "expected Some(RunConfig) for bare name");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_workflow_toml_path() {
|
||||
let (tmp, expected_dot) = setup_workflow_project("hello");
|
||||
let toml_path = tmp.path().join("workflows/hello/workflow.toml");
|
||||
let (dot_path, cfg) = resolve_workflow_from(&toml_path, tmp.path()).unwrap();
|
||||
assert_eq!(dot_path, expected_dot);
|
||||
assert!(cfg.is_some(), "expected Some(RunConfig) for .toml path");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_workflow_dot_path() {
|
||||
let (tmp, expected_dot) = setup_workflow_project("hello");
|
||||
let (dot_path, cfg) = resolve_workflow_from(&expected_dot, tmp.path()).unwrap();
|
||||
assert_eq!(dot_path, expected_dot);
|
||||
assert!(cfg.is_none(), "expected None for .dot path");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -276,7 +276,6 @@ pub async fn run_command(
|
|||
.workflow
|
||||
.as_ref()
|
||||
.ok_or_else(|| anyhow::anyhow!("--workflow is required unless --run-branch is provided"))?;
|
||||
let workflow_path = &super::project_config::resolve_workflow_arg(workflow_path)?;
|
||||
|
||||
// Apply project-level config overrides (arc.toml) on top of CLI defaults.
|
||||
// Workflow-level config (workflow.toml) still wins via apply_defaults below.
|
||||
|
|
@ -292,14 +291,16 @@ pub async fn run_command(
|
|||
}
|
||||
}
|
||||
|
||||
// 0. Load run config if TOML, resolve DOT path, apply defaults
|
||||
let (dot_path, run_cfg) = if workflow_path.extension().is_some_and(|ext| ext == "toml") {
|
||||
let mut cfg = run_config::load_run_config(workflow_path)?;
|
||||
cfg.apply_defaults(&run_defaults);
|
||||
let dot = run_config::resolve_graph_path(workflow_path, &cfg.graph);
|
||||
(dot, Some(cfg))
|
||||
} else {
|
||||
(workflow_path.clone(), None)
|
||||
// 0. Resolve workflow arg, load run config if TOML, resolve DOT path, apply defaults
|
||||
let (dot_path, run_cfg) = {
|
||||
let (dot, cfg) = super::project_config::resolve_workflow(workflow_path)?;
|
||||
match cfg {
|
||||
Some(mut cfg) => {
|
||||
cfg.apply_defaults(&run_defaults);
|
||||
(dot, Some(cfg))
|
||||
}
|
||||
None => (dot, None),
|
||||
}
|
||||
};
|
||||
|
||||
let directory = run_cfg
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
use anyhow::bail;
|
||||
use arc_util::terminal::Styles;
|
||||
|
||||
use crate::cli::run_config;
|
||||
use crate::validation::Severity;
|
||||
use crate::workflow::prepare_from_file;
|
||||
|
||||
|
|
@ -13,12 +12,7 @@ use super::{print_diagnostics, ValidateArgs};
|
|||
///
|
||||
/// Returns an error if the file cannot be read, parsed, or has validation errors.
|
||||
pub fn validate_command(args: &ValidateArgs, styles: &Styles) -> anyhow::Result<()> {
|
||||
let dot_path = if args.workflow.extension().is_some_and(|ext| ext == "toml") {
|
||||
let cfg = run_config::load_run_config(&args.workflow)?;
|
||||
run_config::resolve_graph_path(&args.workflow, &cfg.graph)
|
||||
} else {
|
||||
args.workflow.clone()
|
||||
};
|
||||
let (dot_path, _cfg) = super::project_config::resolve_workflow(&args.workflow)?;
|
||||
|
||||
let (graph, diagnostics) = prepare_from_file(&dot_path)?;
|
||||
|
||||
|
|
@ -47,7 +41,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn validate_valid_workflow() {
|
||||
let mut tmp = tempfile::NamedTempFile::new().unwrap();
|
||||
let mut tmp = tempfile::Builder::new().suffix(".dot").tempfile().unwrap();
|
||||
write!(
|
||||
tmp,
|
||||
r#"digraph Simple {{
|
||||
|
|
@ -75,7 +69,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn validate_invalid_syntax() {
|
||||
let mut tmp = tempfile::NamedTempFile::new().unwrap();
|
||||
let mut tmp = tempfile::Builder::new().suffix(".dot").tempfile().unwrap();
|
||||
write!(tmp, "not a valid dot file").unwrap();
|
||||
|
||||
let args = ValidateArgs {
|
||||
|
|
@ -122,6 +116,36 @@ mod tests {
|
|||
assert!(result.is_ok(), "expected Ok but got: {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_toml_path() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let wf_dir = tmp.path().join("workflows").join("hello");
|
||||
std::fs::create_dir_all(&wf_dir).unwrap();
|
||||
std::fs::write(
|
||||
wf_dir.join("workflow.toml"),
|
||||
"version = 1\ngraph = \"workflow.dot\"\n",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(
|
||||
wf_dir.join("workflow.dot"),
|
||||
r#"digraph Hello {
|
||||
graph [goal="Test"]
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
run [label="Run", prompt="Do it"]
|
||||
start -> run -> exit
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let args = ValidateArgs {
|
||||
workflow: wf_dir.join("workflow.toml"),
|
||||
};
|
||||
let styles = Styles::new(false);
|
||||
let result = validate_command(&args, &styles);
|
||||
assert!(result.is_ok(), "expected Ok but got: {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_missing_file() {
|
||||
let args = ValidateArgs {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue