fabro/lib/crates/fabro-cli/tests/it/cmd/create.rs
Scott Werner accd91a0a6
Some checks are pending
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
Demote non-interpolating config fields to plain String (#492)
# Demote non-category leak fields to plain `String`

Second slice of the interpolation unification — the first **reducing**
PR
stacked on the foundation (#472), per the reduce-first sequencing:
narrowing
changes land before capability additions. (The other reducing slice, the
DOT
de-templating, already landed independently as #474.)

## Why

The target model gives `InterpString` to fields in five categories —
`command` / `script` / `headers` / `env` / `url` — wherever they appear.
A
handful of fields were typed `InterpString` but are *identifiers or
commit
content*, not in any category:

- `run.model.provider` / `run.model.name`
- `cli.exec.model.provider` / `cli.exec.model.name`
- `run.git.author.name` / `run.git.author.email`
- `run.scm.owner` / `run.scm.repository`

Their consumers never resolved them — they leaked raw source text via
`as_source()`. This PR demotes them to plain `String` (layer and
resolved
structs) with **no interpolation**.

## The principle: only `InterpString` fields access variables

These fields are dropped from the variable substitute pass entirely, so
both
`{{ vars.* }}` and `{{ env.* }}` are now literal text. This **removes an
incidental behavior**: run-scoped plain-`String` fields used to get
`{{ vars.* }}` substituted via the String pass (a lucky accident), while
`env` always leaked literally. Variable access becomes deliberate and
typed
rather than accidental; if any of these fields should support variables
later, that's a controlled promotion back to `InterpString`.

## Behavior changes (honest list)

- **The incidental run-scoped `{{ vars.* }}` substitution on these eight
fields stops working.** To keep the removal visible rather than silent,
a
`tracing::warn!` fires at resolve time when a demoted field still
contains
claimed template tokens (`warn_if_demoted_template`). Unclaimed `{{ ...
}}`
  text (jq programs, Go templates) never interpolated and does not warn.
- `{{ env.* }}` / `{{ secrets.* }}` / `{{ inputs.* }}` never resolved on
  these fields, so nothing else changes.

## Added in review: D11 demotions (separate commit, revertable)

The rule got refined during review: a field is `InterpString` iff it is
in one
of the five categories **and resolved at the run boundary** (the only
point
where `vars`/`secrets`/`inputs` exist — they're server state, so
connect-time
and startup-time fields can't reach them even in principle). A separate
commit
applies the clean subset so it can be cherry-picked out if we change
course:

- `cli.target.http.url` / `cli.target.unix.path` — consumed at CLI
connect
time; consumers only ever leaked raw source, so nothing working is
removed.
- `run.working_dir` — **the outlier; see the PR comment.** Its `{{
vars.* }}`
  substitution worked; demoted on the category test alone.

## What's deliberately NOT here

- The **control-plane fields** (`server.storage.root` /
`listen.unix.path` /
  S3 fields / `github.app_id/client_id/slug` / `server.api.url` /
  `server.web.url`) — untouched here, demoted in a follow-up PR.
**Resolved during review** (see the resolution comment): `InterpString`
was
  conflating the user-facing workflow language with the internal control
plane. Control-plane fields never interpolate; the few deployment knobs
that need late binding (e.g. `FABRO_WEB_URL`, whose only real usage is
the
  split-web PoC ferrying a compose env var across a file mount) become
explicit native `EnvVars` reads, and `fabro-server/src/interp.rs`
shrinks
to deletion. `slack.default_channel` stays `InterpString` (consumed with
  run context).

## Implementation notes

- Consumers move from `as_source()` to direct `String` access; the
  foundation's `#[expect(disallowed_methods, ... demotion pending ...)]`
  annotations for these fields are removed (no longer `InterpString`).
- `fabro-checkpoint`'s author plumbing and `fabro-manifest`'s scm fields
  simplify accordingly.

## Verification

- `cargo build --workspace`
- `cargo nextest run --workspace` → 6684 passed, 181 skipped
- `cargo +nightly fmt --check --all`
- `cargo +nightly clippy --workspace --all-targets -- -D warnings` →
clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 13:16:31 -04:00

528 lines
16 KiB
Rust
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context};
use httpmock::MockServer;
use insta::assert_snapshot;
use serde_json::json;
use super::support::{
fixture, output_stdout, remote_run_summary_json, resolve_run, run_count_for_test_case,
run_state,
};
use crate::support::unique_run_id;
fn resolved_run(settings: &fabro_types::WorkflowSettings) -> fabro_types::settings::RunNamespace {
settings.run.clone()
}
fn run_status_response(run_id: &str, status: &str) -> serde_json::Value {
let status = match status {
"submitted" => json!({ "kind": "submitted" }),
other => panic!("unsupported test status {other:?}"),
};
remote_run_summary_json(
run_id,
"Test Workflow",
"test-workflow",
"Test run",
&status,
"2026-04-05T12:00:00Z",
)
}
#[test]
fn help() {
let context = test_context!();
let mut cmd = context.command();
cmd.args(["create", "--help"]);
fabro_snapshot!(context.filters(), cmd, @"
success: true
exit_code: 0
----- stdout -----
Create a workflow run (allocate run dir, persist spec)
Usage: fabro create [OPTIONS] <WORKFLOW>
Arguments:
<WORKFLOW> Path to a .fabro workflow file or .toml task config
Options:
--json Output as JSON [env: FABRO_JSON=]
--server <SERVER> Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=]
--debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=]
-I, --input <KEY=VALUE> Override a workflow input value (repeatable, format: KEY=VALUE)
--dry-run Execute with simulated LLM backend
--no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true]
--auto-approve Auto-approve all human gates
--quiet Suppress non-essential output [env: FABRO_QUIET=]
--goal <GOAL> Override the workflow goal (available as {{ goal }} in prompts)
--goal-file <GOAL_FILE> Read the workflow goal from a file
--model <MODEL> Override default LLM model
--provider <PROVIDER> Override default LLM provider
-v, --verbose Enable verbose output
--environment <ENVIRONMENT> Named environment for agent tools
--label <KEY=VALUE> Attach a label to this run (repeatable, format: KEY=VALUE)
--parent <RUN> Link this run to an existing orchestration parent run
--preserve-sandbox Keep the sandbox alive after the run finishes (for debugging)
-d, --detach Run the workflow in the background and print the run ID
-h, --help Print help
----- stderr -----
");
}
#[test]
fn create_uses_explicit_server_target_and_prints_remote_run_id() {
let context = test_context!();
let server = MockServer::start();
let run_id = unique_run_id();
let mock = server.mock(|when, then| {
when.method("POST").path("/api/v1/runs");
then.status(201)
.header("Content-Type", "application/json")
.body(run_status_response(run_id.as_str(), "submitted").to_string());
});
let output = context
.create_cmd()
.args([
"--server",
&format!("{}/api/v1", server.base_url()),
"--dry-run",
fixture("simple.fabro").to_str().unwrap(),
])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
mock.assert();
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_uses_configured_server_target_without_server_flag() {
let context = test_context!();
let server = MockServer::start();
let run_id = unique_run_id();
let mock = server.mock(|when, then| {
when.method("POST").path("/api/v1/runs");
then.status(201)
.header("Content-Type", "application/json")
.body(run_status_response(run_id.as_str(), "submitted").to_string());
});
context.set_http_target(&server.base_url());
let output = context
.create_cmd()
.args(["--dry-run", fixture("simple.fabro").to_str().unwrap()])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
mock.assert();
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_parent_resolves_parent_and_sends_parent_id_in_manifest() {
let context = test_context!();
let server = MockServer::start();
let run_id = unique_run_id();
let parent_id = unique_run_id();
let resolve_mock = super::support::mock_resolved_run(&server, "nightly-parent", &parent_id);
let create_mock = server.mock(|when, then| {
when.method("POST")
.path("/api/v1/runs")
.json_body_includes(format!(r#"{{"parent_id":"{parent_id}"}}"#));
then.status(201)
.header("Content-Type", "application/json")
.body(run_status_response(run_id.as_str(), "submitted").to_string());
});
let output = context
.create_cmd()
.args([
"--server",
&format!("{}/api/v1", server.base_url()),
"--dry-run",
"--parent",
"nightly-parent",
fixture("simple.fabro").to_str().unwrap(),
])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
resolve_mock.assert();
create_mock.assert();
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_rejects_storage_dir_flag() {
let context = test_context!();
let output = context
.create_cmd()
.args([
"--storage-dir",
"/tmp/fabro-create",
"--dry-run",
fixture("simple.fabro").to_str().unwrap(),
])
.output()
.expect("command should execute");
assert!(
!output.status.success(),
"command should reject --storage-dir"
);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("unexpected argument '--storage-dir'"));
}
#[test]
fn create_cli_server_target_overrides_configured_server_target() {
let context = test_context!();
let config_server = MockServer::start();
let config_mock = config_server.mock(|when, then| {
when.method("POST").path("/api/v1/runs");
then.status(500)
.body("configured-server-should-not-be-used");
});
let cli_server = MockServer::start();
let run_id = unique_run_id();
let cli_mock = cli_server.mock(|when, then| {
when.method("POST").path("/api/v1/runs");
then.status(201)
.header("Content-Type", "application/json")
.body(run_status_response(run_id.as_str(), "submitted").to_string());
});
context.set_http_target(&config_server.base_url());
let output = context
.create_cmd()
.args([
"--server",
&format!("{}/api/v1", cli_server.base_url()),
"--dry-run",
fixture("simple.fabro").to_str().unwrap(),
])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
cli_mock.assert();
config_mock.assert_calls(0);
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_persists_directory_workflow_slug_and_cached_graph() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("sluggy/workflow.fabro");
context.write_temp(
"sluggy/workflow.fabro",
"\
digraph BarBaz {
start [shape=Mdiamond, label=\"Start\"]
exit [shape=Msquare, label=\"Exit\"]
start -> exit
}
",
);
context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_dir = context.find_run_dir(&run_id);
let state = run_state(&run_dir);
let run = &state.spec;
fabro_json_snapshot!(
context,
serde_json::json!({
"workflow_slug": run.workflow_slug,
"graph_name": run.graph.name,
"cached_graph_lines": state.spec.graph_source.as_ref().expect("graph should exist").lines().collect::<Vec<_>>(),
}),
@r#"
{
"workflow_slug": "sluggy",
"graph_name": "BarBaz",
"cached_graph_lines": [
"digraph BarBaz {",
" start [shape=Mdiamond, label=\"Start\"]",
" exit [shape=Msquare, label=\"Exit\"]",
" start -> exit",
"}"
]
}
"#
);
}
#[test]
fn create_persists_file_stem_slug_for_standalone_file() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("alpha.fabro");
context.write_temp(
"alpha.fabro",
"\
digraph FooWorkflow {
start [shape=Mdiamond, label=\"Start\"]
exit [shape=Msquare, label=\"Exit\"]
start -> exit
}
",
);
context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_dir = context.find_run_dir(&run_id);
let state = run_state(&run_dir);
let run = &state.spec;
fabro_json_snapshot!(
context,
serde_json::json!({
"workflow_slug": run.workflow_slug,
"graph_name": run.graph.name,
"cached_graph_lines": state.spec.graph_source.as_ref().expect("graph should exist").lines().collect::<Vec<_>>(),
}),
@r#"
{
"workflow_slug": "alpha",
"graph_name": "FooWorkflow",
"cached_graph_lines": [
"digraph FooWorkflow {",
" start [shape=Mdiamond, label=\"Start\"]",
" exit [shape=Msquare, label=\"Exit\"]",
" start -> exit",
"}"
]
}
"#
);
}
#[expect(
clippy::disallowed_methods,
reason = "test asserts the raw template source"
)]
#[test]
fn create_persists_requested_overrides_into_store() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let workflow = fixture("simple.fabro");
let mut cmd = context.command();
cmd.args([
"create",
"--dry-run",
"--auto-approve",
"--goal",
"Ship the release",
"--model",
"gpt-5",
"--provider",
"openai",
"--environment",
"default",
"--label",
"env=dev",
"--label",
"team=cli",
"--verbose",
"--preserve-sandbox",
workflow.to_str().unwrap(),
]);
let output = cmd.output().expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let stdout = output_stdout(&output);
let run_id = stdout
.lines()
.find(|line| !line.trim().is_empty())
.map(str::trim)
.expect("create should print a run ID")
.to_string();
let run = resolve_run(&context, &run_id);
let state = run_state(&run.run_dir);
let run_spec = &state.spec;
let labels = json!({
"env": run_spec.labels.get("env"),
"team": run_spec.labels.get("team"),
});
let settings = &run_spec.settings;
let resolved_run = resolved_run(settings);
let compact = json!({
"workflow_slug": run_spec.workflow_slug,
"settings": {
"goal": match resolved_run.goal.as_ref() {
Some(fabro_types::settings::run::RunGoal::Inline(value)) => Some(value.as_source()),
_ => None,
},
"dry_run": resolved_run.execution.mode == fabro_types::settings::run::RunMode::DryRun,
"auto_approve": resolved_run.execution.approval == fabro_types::settings::run::ApprovalMode::Auto,
"llm": {
"model": resolved_run.model.name.clone(),
"provider": resolved_run.model.provider.clone(),
},
"environment": {
"id": resolved_run.environment.id,
"provider": resolved_run.environment.provider.to_string(),
"preserve": resolved_run.environment.lifecycle.preserve,
},
},
"labels": labels,
});
assert_snapshot!(serde_json::to_string_pretty(&compact).unwrap(), @r###"
{
"workflow_slug": "simple",
"settings": {
"goal": "Ship the release",
"dry_run": true,
"auto_approve": true,
"llm": {
"model": "gpt-5",
"provider": "openai"
},
"environment": {
"id": "default",
"provider": "docker",
"preserve": true
}
},
"labels": {
"env": "dev",
"team": "cli"
}
}
"###);
}
#[test]
fn create_json_does_not_imply_auto_approve() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let workflow = fixture("simple.fabro");
let output = context
.command()
.args(["--json", "create", "--dry-run", workflow.to_str().unwrap()])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"command failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let value: serde_json::Value =
serde_json::from_slice(&output.stdout).expect("create JSON should parse");
let run_id = value["run_id"]
.as_str()
.expect("create JSON should include run_id");
let run = resolve_run(&context, run_id);
assert!(
resolved_run(&run_state(&run.run_dir).spec.settings,)
.execution
.approval
!= fabro_types::settings::run::ApprovalMode::Auto
);
}
#[test]
fn create_invalid_workflow_fails_without_creating_run() {
let context = test_context!();
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());
fabro_snapshot!(context.filters(), cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× Validation failed
");
let run_count = run_count_for_test_case(&context);
assert_eq!(
run_count, initial_run_count,
"invalid create should not persist a run for this test case"
);
}
#[test]
fn create_rejects_unbound_template_inputs_without_creating_run() {
let context = test_context!();
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());
fabro_snapshot!(context.filters(), cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× Validation failed
");
let run_count = run_count_for_test_case(&context);
assert_eq!(
run_count, initial_run_count,
"invalid create should not persist a run for this test case"
);
}