diff --git a/docs/public/changelog/2026-08-31.mdx b/docs/public/changelog/2026-08-31.mdx index 609f0d64e..5af128e8b 100644 --- a/docs/public/changelog/2026-08-31.mdx +++ b/docs/public/changelog/2026-08-31.mdx @@ -3,10 +3,13 @@ title: "Immutable workflow versions for CLI runs" date: "2026-08-31" --- -`fabro run` and `fabro create` now validate local workflows, register their -immutable workflow versions and dependencies, and create runs by intent. -`fabro create` leaves the run submitted; `fabro run` starts it with a separate -request. +`fabro run` and `fabro create` now resolve and package local workflows, +register their immutable workflow versions and dependencies, and create runs +by intent. Source parsing or packaging failures remain local and stop before +registration, while full effective-intent validation is authoritative at +server admission. Use `fabro preflight` for explicit local validation without +creating a run. `fabro create` leaves the run submitted; `fabro run` starts it +with a separate request. Workflows can still be selected by project name, from user workflow storage, from another local checkout, or as loose local files. The workflow source is diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 26052f1b1..7db919dba 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -12,11 +12,14 @@ workflow-owned settings. Instead of passing a dozen CLI flags, check fabro run run.toml ``` -`fabro run` and `fabro create` resolve the workflow locally, validate it, +`fabro run` and `fabro create` resolve and package the workflow locally, register its immutable workflow version and dependencies, and then ask the -server to create a run from that version. `fabro create` stops with the run in -the submitted state; `fabro run` performs the same create operation and then -starts the run separately. +server to admit the intent and create a run from that version. Source parsing +or packaging failures stop locally before registration; full effective-intent +validation is authoritative at server admission. Use `fabro preflight` for +explicit local validation without creating a run. `fabro create` stops with +the run in the submitted state; `fabro run` performs the same create operation +and then starts the run separately. The workflow can be selected by name from the current project or user workflow storage, by a path in another local checkout, or as a loose local file. Its diff --git a/lib/apps/fabro-cli/src/commands/run/command.rs b/lib/apps/fabro-cli/src/commands/run/command.rs index 4c7f7a93b..2ec25f636 100644 --- a/lib/apps/fabro-cli/src/commands/run/command.rs +++ b/lib/apps/fabro-cli/src/commands/run/command.rs @@ -15,7 +15,7 @@ pub(crate) async fn execute(mut args: RunArgs, base_ctx: &CommandContext) -> Res let quiet = args.detach; let prevent_idle_sleep = ctx.user_settings().cli.exec.prevent_idle_sleep; - let created_run = Box::pin(super::create::create_run(&ctx, &args, styles, quiet)).await?; + let created_run = Box::pin(super::create::create_run(&ctx, &args, styles)).await?; if !quiet { fabro_util::printerr!( diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 4688064ca..08cca8a84 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -3,12 +3,10 @@ use std::path::Path; use anyhow::{Context as _, anyhow, bail}; use fabro_config::project; use fabro_environment::DEFAULT_ENVIRONMENT_ID; -use fabro_server::manifest_validation; use fabro_types::settings::run::EnvironmentProvider; use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget}; use fabro_util::terminal::Styles; -use super::output::print_workflow_summary; use super::overrides::prepare_intent_overrides; use crate::args::RunArgs; use crate::command_context::CommandContext; @@ -27,7 +25,6 @@ pub(crate) async fn create_run( ctx: &CommandContext, args: &RunArgs, styles: &Styles, - quiet: bool, ) -> anyhow::Result { let workflow_path = args .workflow @@ -46,22 +43,6 @@ pub(crate) async fn create_run( Some(&user_workflows_root), )?; let prepared = prepare_intent_overrides(args, &canonical_cwd)?; - let validation = manifest_validation::validate_collected_workflow( - package.closure(), - Some(&prepared.run_layer), - &prepared.input_overrides, - )?; - if !quiet { - print_workflow_summary( - &validation.workflow, - Some(&package.workflow_location().graph), - styles, - ctx.printer(), - ); - } - if !validation.ok { - bail!("Validation failed"); - } warn_untransmitted_settings( ctx, @@ -184,5 +165,10 @@ fn run_target_for_environment( let target = observation.run_target.ok_or_else(|| { anyhow!("the caller Git checkout cannot be represented as a canonical GitHub run target") })?; + if target.sha.is_none() { + bail!( + "the exact local Git commit could not be made available from the canonical GitHub origin; push the commit and try again" + ); + } Ok((RunTarget::Git(target), dirty)) } diff --git a/lib/apps/fabro-cli/src/commands/run/mod.rs b/lib/apps/fabro-cli/src/commands/run/mod.rs index 859659c35..7d0c2e3ed 100644 --- a/lib/apps/fabro-cli/src/commands/run/mod.rs +++ b/lib/apps/fabro-cli/src/commands/run/mod.rs @@ -42,7 +42,7 @@ pub(crate) async fn dispatch( RunCommands::Create(args) => { let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); let ctx = base_ctx.with_target(&args.target)?; - let created_run = Box::pin(create::create_run(&ctx, &args, styles, true)).await?; + let created_run = Box::pin(create::create_run(&ctx, &args, styles)).await?; if ctx.json_output() { print_json_pretty(&serde_json::json!({ "run_id": created_run.run_id }))?; } else { diff --git a/lib/apps/fabro-cli/src/commands/run/overrides.rs b/lib/apps/fabro-cli/src/commands/run/overrides.rs index d655c8698..ec98736d7 100644 --- a/lib/apps/fabro-cli/src/commands/run/overrides.rs +++ b/lib/apps/fabro-cli/src/commands/run/overrides.rs @@ -21,10 +21,8 @@ pub(crate) struct ManifestSettingsOverrides { #[derive(Debug)] pub(super) struct PreparedIntentOverrides { - pub(super) run_layer: RunLayer, - pub(super) input_overrides: HashMap, - pub(super) intent_args: RunIntentArgs, - pub(super) goal: Option, + pub(super) intent_args: RunIntentArgs, + pub(super) goal: Option, } fn sparse_flag(value: bool) -> Option { @@ -100,7 +98,8 @@ pub(super) fn prepare_intent_overrides( .iter() .map(|(key, value)| { let value = fabro_types::toml_scalar_to_json_value(value) - .map_err(|error| anyhow!("input override `{key}` {error}"))?; + .map_err(anyhow::Error::new) + .with_context(|| format!("failed to convert input override `{key}`"))?; Ok((key.clone(), value)) }) .collect::>>()?; @@ -108,20 +107,7 @@ pub(super) fn prepare_intent_overrides( let dry_run = sparse_flag(args.dry_run); let auto_approve = sparse_flag(args.auto_approve); let preserve_sandbox = sparse_flag(args.preserve_sandbox); - let run_layer = build_run_overrides(RunOverrideInput { - goal: goal.as_deref(), - model: args.model.as_deref(), - provider: args.provider.as_deref(), - environment: args.environment.as_deref(), - preserve_sandbox, - dry_run, - auto_approve, - labels: labels.clone(), - }); - Ok(PreparedIntentOverrides { - run_layer, - input_overrides, intent_args: RunIntentArgs { model: args.model.clone(), provider: args.provider.clone(), @@ -205,11 +191,12 @@ mod tests { args.preserve_sandbox = true; args.verbose = true; - let prepared = prepare_intent_overrides(&args, Path::new("/caller")).unwrap(); + let PreparedIntentOverrides { intent_args, goal } = + prepare_intent_overrides(&args, Path::new("/caller")).unwrap(); - assert_eq!(prepared.goal.as_deref(), Some("Ship it")); + assert_eq!(goal.as_deref(), Some("Ship it")); assert_eq!( - prepared.intent_args.inputs, + intent_args.inputs, HashMap::from([ ("string".to_string(), serde_json::json!("hello")), ("boolean".to_string(), serde_json::json!(true)), @@ -217,38 +204,19 @@ mod tests { ("float".to_string(), serde_json::json!(1.25)), ]) ); - assert_eq!(prepared.intent_args.model.as_deref(), Some("gpt-5")); - assert_eq!(prepared.intent_args.provider.as_deref(), Some("openai")); - assert_eq!( - prepared.intent_args.labels.get("team"), - Some(&"cli".to_string()) - ); - assert_eq!(prepared.intent_args.dry_run, Some(true)); - assert_eq!(prepared.intent_args.auto_approve, Some(true)); - assert_eq!(prepared.intent_args.preserve_sandbox, Some(true)); + assert_eq!(intent_args.model.as_deref(), Some("gpt-5")); + assert_eq!(intent_args.provider.as_deref(), Some("openai")); + assert_eq!(intent_args.labels.get("team"), Some(&"cli".to_string())); + assert_eq!(intent_args.dry_run, Some(true)); + assert_eq!(intent_args.auto_approve, Some(true)); + assert_eq!(intent_args.preserve_sandbox, Some(true)); assert!( - !serde_json::to_value(&prepared.intent_args) + !serde_json::to_value(&intent_args) .unwrap() .as_object() .unwrap() .contains_key("verbose") ); - assert_eq!( - prepared.input_overrides, - HashMap::from([ - ( - "string".to_string(), - toml::Value::String("hello".to_string()) - ), - ("boolean".to_string(), toml::Value::Boolean(true)), - ("integer".to_string(), toml::Value::Integer(42)), - ("float".to_string(), toml::Value::Float(1.25)), - ]) - ); - let RunGoalLayer::Inline(goal) = prepared.run_layer.goal.unwrap() else { - panic!("prepared run layer should use an inline goal"); - }; - assert_eq!(goal.as_source(), "Ship it"); } #[test] @@ -270,13 +238,12 @@ mod tests { for goal_file in [relative, dir.path().join("goals/task.md")] { let mut args = run_args(); args.goal_file = Some(goal_file); - let prepared = prepare_intent_overrides(&args, dir.path()).unwrap(); + let PreparedIntentOverrides { + intent_args: _, + goal, + } = prepare_intent_overrides(&args, dir.path()).unwrap(); - assert_eq!(prepared.goal.as_deref(), Some("Goal from file")); - let RunGoalLayer::Inline(goal) = prepared.run_layer.goal.unwrap() else { - panic!("goal-file content should become an inline validation override"); - }; - assert_eq!(goal.as_source(), "Goal from file"); + assert_eq!(goal.as_deref(), Some("Goal from file")); } } @@ -307,7 +274,12 @@ mod tests { let error = prepare_intent_overrides(&args, Path::new("/caller")).unwrap_err(); assert!(error.to_string().contains("temperature")); - assert!(error.to_string().contains("finite")); + assert!(format!("{error:#}").contains("finite")); + assert!(error.chain().any(|cause| { + cause + .downcast_ref::() + .is_some() + })); } #[test] diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index 778f1814b..d21f79fa7 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -631,7 +631,7 @@ fn create_preserves_named_user_other_checkout_and_loose_file_selection() { } #[test] -fn create_maps_clone_providers_to_shared_git_observations() { +fn create_clone_targets_require_exact_git_observations() { let context = test_context!(); let server = MockServer::start(); let run_id = unique_run_id(); @@ -762,11 +762,16 @@ fn create_maps_clone_providers_to_shared_git_observations() { ]) .output() .unwrap(); + assert!(!branch_output.status.success()); + let branch_stderr = output_stderr(&branch_output); assert!( - branch_output.status.success(), - "{}", - output_stderr(&branch_output) + branch_stderr.contains( + "the exact local Git commit could not be made available from the canonical GitHub origin" + ), + "{branch_stderr}" ); + assert!(!branch_stderr.contains("file://")); + assert!(!branch_stderr.contains("No such file or directory")); let no_repository = tempfile::tempdir().unwrap(); let none_output = context @@ -786,8 +791,8 @@ fn create_maps_clone_providers_to_shared_git_observations() { ); environment_mock.assert_calls(3); - version_mock.assert_calls(3); - create_mock.assert_calls(3); + version_mock.assert_calls(2); + create_mock.assert_calls(2); let requests = requests.lock().unwrap(); assert_eq!(requests[0]["args"]["dry_run"], true); assert_eq!( @@ -799,15 +804,7 @@ fn create_maps_clone_providers_to_shared_git_observations() { "sha": run_git(exact.path(), &["rev-parse", "HEAD"]), }) ); - assert_eq!( - requests[1]["target"], - json!({ - "kind": "git", - "repo": "acme/missing", - "branch": "topic", - }) - ); - assert_eq!(requests[2]["target"], json!({ "kind": "none" })); + assert_eq!(requests[1]["target"], json!({ "kind": "none" })); } #[test] @@ -1266,17 +1263,20 @@ fn create_json_does_not_imply_auto_approve() { #[test] fn create_invalid_workflow_fails_without_creating_run() { let context = test_context!(); + let caller = tempfile::tempdir().unwrap(); let workflow = fixture("invalid.fabro"); let initial_run_count = run_count_for_test_case(&context); let mut cmd = context.create_cmd(); - cmd.arg(workflow.to_str().unwrap()); + cmd.current_dir(caller.path()) + .args(["--quiet", workflow.to_str().unwrap()]); fabro_snapshot!(context.filters(), cmd, @" success: false exit_code: 1 ----- stdout ----- ----- stderr ----- - × Validation failed + × could not create run + ╰─▶ run intent could not be compiled: Validation failed "); let run_count = run_count_for_test_case(&context); @@ -1289,17 +1289,20 @@ fn create_invalid_workflow_fails_without_creating_run() { #[test] fn create_rejects_unbound_template_inputs_without_creating_run() { let context = test_context!(); + let caller = tempfile::tempdir().unwrap(); let workflow = fixture("templated_unbound.fabro"); let initial_run_count = run_count_for_test_case(&context); let mut cmd = context.create_cmd(); - cmd.arg(workflow.to_str().unwrap()); + cmd.current_dir(caller.path()) + .args(["--quiet", workflow.to_str().unwrap()]); fabro_snapshot!(context.filters(), cmd, @" success: false exit_code: 1 ----- stdout ----- ----- stderr ----- - × Validation failed + × could not create run + ╰─▶ run intent could not be compiled: Validation failed "); let run_count = run_count_for_test_case(&context); @@ -1310,56 +1313,51 @@ fn create_rejects_unbound_template_inputs_without_creating_run() { } #[test] -fn create_validates_before_any_remote_request_and_accepts_matching_typed_input() { +fn create_registers_package_before_surfacing_server_admission_rejection() { let context = test_context!(); let server = MockServer::start(); - let mut any_request = server.mock(|when, then| { - when.any_request(); - then.status(500).body("server must remain untouched"); - }); - let invalid = context - .create_cmd() - .args([ - "--server", - &format!("{}/api/v1", server.base_url()), - "--parent", - "must-not-resolve", - fixture("templated_unbound.fabro").to_str().unwrap(), - ]) - .output() - .unwrap(); - assert!(!invalid.status.success()); - any_request.assert_calls(0); - - any_request.delete(); let environment_mock = mock_environment(&server, "local", "local"); - let version_mock = mock_workflow_version_registrations(&server); - let run_id = unique_run_id(); - let requests = Arc::new(Mutex::new(Vec::new())); - let create_mock = mock_intent_create(&server, &run_id, Arc::clone(&requests)); - let valid = context + let registered_versions = Arc::new(Mutex::new(Vec::new())); + let version_mock = + mock_workflow_version_registrations_recording(&server, Arc::clone(®istered_versions)); + let registered_versions_for_create = Arc::clone(®istered_versions); + let create_mock = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + then.respond_with(move |_| { + assert_eq!( + registered_versions_for_create.lock().unwrap().len(), + 1, + "the workflow version must be registered before server admission" + ); + HttpMockResponse::builder() + .status(422) + .header("content-type", "text/plain") + .body("server-authoritative workflow rejection") + .build() + }); + }); + let output = context .create_cmd() .args([ "--server", &format!("{}/api/v1", server.base_url()), "--environment", "local", - "--input", - "app_dir=42", fixture("templated_unbound.fabro").to_str().unwrap(), ]) .output() .unwrap(); - assert!( - valid.status.success(), - "matching input should validate:\n{}", - output_stderr(&valid) - ); - assert!(!output_stderr(&valid).contains("inputs.app_dir")); + assert!(!output.status.success()); environment_mock.assert(); version_mock.assert(); create_mock.assert(); - assert_eq!(requests.lock().unwrap()[0]["args"]["inputs"]["app_dir"], 42); + let stderr = output_stderr(&output); + assert!(stderr.contains("could not create run"), "{stderr}"); + assert!( + stderr.contains("server-authoritative workflow rejection"), + "{stderr}" + ); + assert!(!stderr.contains("Validation failed"), "{stderr}"); } #[test] diff --git a/lib/apps/fabro-cli/tests/it/cmd/run.rs b/lib/apps/fabro-cli/tests/it/cmd/run.rs index fa615a42e..5f1752b38 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/run.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/run.rs @@ -669,14 +669,16 @@ fn remote_foreground_run_consumes_paginated_events_and_prints_server_backed_summ } #[test] -fn run_rejects_unbound_template_inputs_before_creating_remote_run() { +fn run_surfaces_server_rejection_for_unbound_template_inputs() { let context = test_context!(); let server = MockServer::start(); + let environment_mock = mock_environment(&server, "default", "docker"); + let version_mock = mock_workflow_version_registrations(&server); let create = server.mock(|when, then| { when.method("POST").path("/api/v1/runs"); - then.status(500) - .header("Content-Type", "application/json") - .body(serde_json::json!({ "error": "run should not be created" }).to_string()); + then.status(422) + .header("Content-Type", "text/plain") + .body("server-authoritative unbound-input rejection"); }); let workflow = context.install_fixture("templated_unbound.fabro"); @@ -696,32 +698,29 @@ fn run_rejects_unbound_template_inputs_before_creating_remote_run() { String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); - create.assert_calls(0); + environment_mock.assert(); + version_mock.assert(); + create.assert(); let stderr = output_stderr(&output); + assert!(stderr.contains("could not create run"), "{stderr}"); assert!( - stderr.contains("inputs.app_dir"), - "stderr should name the unbound variable: {stderr}" - ); - assert!( - stderr.contains("templated_unbound.fabro"), - "stderr should name the workflow source: {stderr}" - ); - assert!( - !stderr.contains(""), - "stderr should not expose MiniJinja's generic source name: {stderr}" + stderr.contains("server-authoritative unbound-input rejection"), + "{stderr}" ); } #[test] -fn foreground_run_rejects_invalid_workflow_before_creating_remote_run() { +fn foreground_run_surfaces_server_rejection_for_invalid_workflow() { let context = test_context!(); let server = MockServer::start(); + let environment_mock = mock_environment(&server, "default", "docker"); + let version_mock = mock_workflow_version_registrations(&server); let create = server.mock(|when, then| { when.method("POST").path("/api/v1/runs"); - then.status(500) - .header("Content-Type", "application/json") - .body(serde_json::json!({ "error": "run should not be created" }).to_string()); + then.status(422) + .header("Content-Type", "text/plain") + .body("server-authoritative invalid-workflow rejection"); }); let workflow = context.install_fixture("invalid.fabro"); @@ -741,15 +740,16 @@ fn foreground_run_rejects_invalid_workflow_before_creating_remote_run() { String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); - create.assert_calls(0); + environment_mock.assert(); + version_mock.assert(); + create.assert(); let stderr = output_stderr(&output); - assert!(stderr.contains("Workflow: Invalid"), "{stderr}"); + assert!(stderr.contains("could not create run"), "{stderr}"); assert!( - stderr.contains("Pipeline must have exactly one start node"), + stderr.contains("server-authoritative invalid-workflow rejection"), "{stderr}" ); - assert!(stderr.contains("Validation failed"), "{stderr}"); } #[test] @@ -827,10 +827,6 @@ fn dry_run_simple() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: Simple (4 nodes, 3 edges) - Graph: [GRAPH_PATH] - Goal: Run tests and report results - Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME]) @@ -852,8 +848,8 @@ fn dry_run_simple() { #[test] fn dry_run_with_goal_file_reads_contents_into_goal() { // Regression test for the `--goal-file` flag that was previously - // being silently ignored in the v2 path. The file content must end - // up in the effective goal displayed in the workflow summary. + // being silently ignored in the v2 path. The file content must reach + // the server-authoritative run specification. let context = test_context!(); let goal_dir = tempfile::tempdir().unwrap(); @@ -872,10 +868,10 @@ fn dry_run_with_goal_file_reads_contents_into_goal() { "run should succeed:\nstderr:\n{}", String::from_utf8_lossy(&output.stderr) ); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!( - stderr.contains("Ship the rate-limiting feature end to end."), - "goal file content should appear in workflow summary, got:\n{stderr}" + assert_eq!( + run_state(&context.single_run_dir()).spec.graph.goal(), + "Ship the rate-limiting feature end to end.\n", + "goal file content should reach the admitted run" ); } diff --git a/lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs b/lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs index 496a7de04..a6facf8d0 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs @@ -14,12 +14,6 @@ fn dry_run_branching() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: Branch (6 nodes, 6 edges) - Graph: [GRAPH_PATH] - Goal: Implement and validate a feature - - warning [node: implement]: Node 'implement' has goal_gate=true but no retry_target or fallback_retry_target (goal_gate_has_retry) - fix: Add retry_target or fallback_retry_target attribute Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME]) @@ -52,10 +46,6 @@ fn dry_run_conditions() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: Conditions (5 nodes, 5 edges) - Graph: [GRAPH_PATH] - Goal: Test condition evaluation with OR and parentheses - Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME]) @@ -88,10 +78,6 @@ fn dry_run_parallel() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: Parallel (7 nodes, 7 edges) - Graph: [GRAPH_PATH] - Goal: Test parallel and fan-in execution - Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME]) @@ -125,10 +111,6 @@ fn dry_run_styled() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: Styled (5 nodes, 4 edges) - Graph: [GRAPH_PATH] - Goal: Build a styled pipeline - Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME]) @@ -160,10 +142,6 @@ fn dry_run_inferred_command() { exit_code: 0 ----- stdout ----- ----- stderr ----- - Workflow: InferredCommand (3 nodes, 2 edges) - Graph: [GRAPH_PATH] - Goal: Verify a shapeless script node runs as a command - Run: [ULID] Web UI: http://localhost:3000/runs/[ULID] Sandbox: local (ready in [TIME])