scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-05 08:10:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

Harden CLI run intent creation

Browse source
This commit is contained in:
Scott Werner 2026-09-01 16:32:24 -04:00
parent 204dd29e73
commit ccfd23104d
9 changed files with 127 additions and 191 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
docs/public/changelog/2026-08-31.mdx
View file

@ -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

11
docs/public/execution/run-configuration.mdx
View file

@ -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

2
lib/apps/fabro-cli/src/commands/run/command.rs
View file

@ -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!(

24
lib/apps/fabro-cli/src/commands/run/create.rs
View file

@ -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<CreatedRun> {
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))
}

2
lib/apps/fabro-cli/src/commands/run/mod.rs
View file

@ -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 {

80
lib/apps/fabro-cli/src/commands/run/overrides.rs
View file

@ -21,10 +21,8 @@ pub(crate) struct ManifestSettingsOverrides {
#[derive(Debug)]
pub(super) struct PreparedIntentOverrides {
pub(super) run_layer: RunLayer,
pub(super) input_overrides: HashMap<String, toml::Value>,
pub(super) intent_args: RunIntentArgs,
pub(super) goal: Option<String>,
pub(super) intent_args: RunIntentArgs,
pub(super) goal: Option<String>,
}
fn sparse_flag(value: bool) -> Option<bool> {
@ -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::<Result<HashMap<_, _>>>()?;
@ -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::<fabro_types::TomlScalarToJsonError>()
.is_some()
}));
}
#[test]

104
lib/apps/fabro-cli/tests/it/cmd/create.rs
View file

@ -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(&registered_versions));
let registered_versions_for_create = Arc::clone(&registered_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]

62
lib/apps/fabro-cli/tests/it/cmd/run.rs
View file

@ -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("<string>"),
"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"
);
}

22
lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs
View file

@ -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])