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

Merge pull request #712 from fabro-sh/remove-client-run-id

Browse source 
Make the server the only allocator of run IDs
This commit is contained in:
Bryan Helmkamp 2026-08-03 11:17:47 -04:00 • committed by GitHub
commit 60cf4fdd8f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 162 additions and 252 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
docs/public/api-reference/fabro-api.yaml
View file

@ -9052,10 +9052,6 @@ components:
type: integer
description: Manifest schema version.
example: 1
run_id:
type: ["string", "null"]
description: Optional pre-generated run ID to use instead of allocating a new ULID.
example: "01HV6D7S5YF4Z4B2M7K4N0Q6T9"
parent_id:
type: ["string", "null"]
description: Optional orchestration parent run ID. Fork and rewind lineage use separate fields and should not set this value.

4
lib/apps/fabro-cli/src/args.rs
View file

@ -277,10 +277,6 @@ pub(crate) struct RunArgs {
/// Run the workflow in the background and print the run ID
#[arg(short = 'd', long)]
pub(crate) detach: bool,
/// Pre-generated run ID (used internally by --detach)
#[arg(long, hide = true)]
pub(crate) run_id: Option<String>,
}
#[derive(Args)]

15
lib/apps/fabro-cli/src/commands/preflight.rs
View file

@ -23,15 +23,14 @@ pub(crate) async fn execute(
let cli_args_config = preflight_args_overrides(&args)?;
let manifest = build_run_manifest(ManifestBuildInput {
workflow: args.workflow.clone(),
cwd: ctx.cwd().to_path_buf(),
run_overrides: cli_args_config.run,
cli_overrides: cli_args_config.cli,
input_overrides: cli_args_config.input_overrides,
args: preflight_manifest_args(&args),
workflow: args.workflow.clone(),
cwd: ctx.cwd().to_path_buf(),
run_overrides: cli_args_config.run,
cli_overrides: cli_args_config.cli,
input_overrides: cli_args_config.input_overrides,
args: preflight_manifest_args(&args),
environment_defaults: fabro_environment::seeded_catalog_layer(),
user_settings_path: Some(active_settings_path(None)),
..Default::default()
user_settings_path: Some(active_settings_path(None)),
})?;
let spinner = (!ctx.json_output()).then(|| cyan_spinner("Running checks..."));

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

@ -33,13 +33,6 @@ pub(crate) async fn create_run(
.ok_or_else(|| anyhow::anyhow!("--workflow is required"))?;
let cli_args_config = run_args_overrides(args)?;
let cwd = ctx.cwd().to_path_buf();
let run_id = args
.run_id
.as_deref()
.map(str::parse::<RunId>)
.transpose()
.context("invalid run ID")?;
let mut built = build_run_manifest(ManifestBuildInput {
workflow: workflow_path.clone(),
cwd,
@ -47,7 +40,6 @@ pub(crate) async fn create_run(
cli_overrides: cli_args_config.cli,
input_overrides: cli_args_config.input_overrides,
args: run_manifest_args(args),
run_id,
environment_defaults: fabro_environment::seeded_catalog_layer(),
user_settings_path: Some(active_settings_path(None)),
})?;

11
lib/apps/fabro-cli/tests/it/cmd/attach.rs
View file

@ -15,9 +15,10 @@ use fabro_test::{
use serde_json::Value;
use super::support::{
output_stdout, resolve_run, server_endpoint, wait_for_status, write_gated_workflow,
created_run_id, output_stdout, resolve_run, server_endpoint, wait_for_status,
write_gated_workflow,
};
use crate::support::{run_output_filters, unique_run_id};
use crate::support::run_output_filters;
const SHARED_DAEMON_TIMEOUT: Duration = Duration::from_secs(30);
@ -366,22 +367,20 @@ fn attach_reprompts_invalid_choice_then_accepts_valid_answer() {
fn attach_replays_completed_detached_run() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let run = context
.command()
.args([
"run",
"--dry-run",
"--auto-approve",
"--detach",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(run.get_output());
context
.command()

9
lib/apps/fabro-cli/tests/it/cmd/config.rs
View file

@ -9,8 +9,7 @@ use fabro_test::{fabro_snapshot, test_context};
use httpmock::MockServer;
use predicates::prelude::*;
use super::support::run_state;
use crate::support::unique_run_id;
use super::support::{created_run_id, run_state};
#[test]
fn old_config_show_command_is_rejected() {
@ -310,10 +309,9 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() {
context.ensure_home_server_auth_methods();
let cwd = tempfile::tempdir().unwrap();
let workflow = project.path().join("workflow.toml");
let run_id = unique_run_id();
// Remove FABRO_STORAGE_DIR so the CLI uses storage_dir from settings.toml
context
let create = context
.command()
.env_remove("FABRO_STORAGE_DIR")
.current_dir(cwd.path())
@ -322,12 +320,11 @@ fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() {
"--dry-run",
"--model",
"gpt-5.4-pro",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let runs_dir = storage_dir.join("scratch");
let run_dir = std::fs::read_dir(&runs_dir)

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

@ -4,8 +4,8 @@ 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,
created_run_id, fixture, output_stdout, remote_run_summary_json, resolve_run,
run_count_for_test_case, run_state,
};
use crate::support::unique_run_id;
@ -271,7 +271,6 @@ fn create_cli_server_target_overrides_configured_server_target() {
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(
@ -285,18 +284,17 @@ digraph BarBaz {
",
);
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
let state = run_state(&run_dir);
@ -328,7 +326,6 @@ digraph BarBaz {
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(
@ -342,18 +339,17 @@ digraph FooWorkflow {
",
);
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
let state = run_state(&run_dir);

20
lib/apps/fabro-cli/tests/it/cmd/dump.rs
View file

@ -11,10 +11,10 @@ use fabro_test::{fabro_snapshot, test_context};
use insta::assert_snapshot;
use super::support::{
local_dev_token, server_target, setup_completed_dry_run, setup_seeded_completed_dry_run,
setup_seeded_created_dry_run,
local_dev_token, run_state, server_target, setup_completed_dry_run,
setup_seeded_completed_dry_run, setup_seeded_created_dry_run,
};
use crate::support::{LightweightCli, seed_dev_token_auth, unique_run_id};
use crate::support::{LightweightCli, seed_dev_token_auth};
#[test]
fn help() {
@ -99,11 +99,10 @@ fn dump_exports_large_command_output_backed_by_blob_refs() {
)
.unwrap();
let run_id = unique_run_id();
let mut run_cmd = context.run_cmd();
run_cmd.current_dir(&context.temp_dir);
run_cmd.timeout(Duration::from_secs(30));
run_cmd.args(["--run-id", run_id.as_str(), "--environment", "local"]);
run_cmd.args(["--environment", "local"]);
run_cmd.arg(&workflow);
let run_output = run_cmd.output().expect("command should execute");
assert!(
@ -112,6 +111,7 @@ fn dump_exports_large_command_output_backed_by_blob_refs() {
String::from_utf8_lossy(&run_output.stdout),
String::from_utf8_lossy(&run_output.stderr)
);
let run_id = run_state(&context.single_run_dir()).spec.run_id.to_string();
let mut inspect_cmd = context.command();
inspect_cmd.args(["inspect", "--json", &run_id]);
@ -186,17 +186,10 @@ include = ["assets/**"]
)
.unwrap();
let run_id = unique_run_id();
let mut run_cmd = context.run_cmd();
run_cmd.current_dir(&workspace_dir);
run_cmd.timeout(Duration::from_secs(30));
run_cmd.args([
"--run-id",
run_id.as_str(),
"--environment",
"local",
"run.toml",
]);
run_cmd.args(["--environment", "local", "run.toml"]);
let run_output = run_cmd.output().expect("command should execute");
assert!(
run_output.status.success(),
@ -204,6 +197,7 @@ include = ["assets/**"]
String::from_utf8_lossy(&run_output.stdout),
String::from_utf8_lossy(&run_output.stderr)
);
let run_id = run_state(&context.single_run_dir()).spec.run_id.to_string();
let mut inspect_cmd = context.command();
inspect_cmd.args(["inspect", "--json", &run_id]);

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

@ -9,8 +9,10 @@ use fabro_vault::{SecretType, Vault};
use httpmock::MockServer;
use serde_json::Value;
use super::support::{output_stderr, remote_run_summary_json, wait_for_event_names};
use crate::support::{run_output_filters, run_projection_json, unique_run_id};
use super::support::{
created_run_id, output_stderr, remote_run_summary_json, run_state, wait_for_event_names,
};
use crate::support::{LightweightCli, run_output_filters, run_projection_json, unique_run_id};
fn run_status_response(run_id: &str, status: &str) -> serde_json::Value {
let status = match status {
@ -357,7 +359,6 @@ fn run_uses_vault_credentials_for_worker_execution() {
),
);
context.isolated_server();
let run_id = unique_run_id();
seed_anthropic_vault(&context.storage_dir);
let llm_mock = llm_server.mock(|when, then| {
@ -410,8 +411,6 @@ digraph VaultWorkerLlm {
.env_remove("OPENAI_BASE_URL")
.env_remove("GEMINI_API_KEY")
.args([
"--run-id",
run_id.as_str(),
"--auto-approve",
"--environment",
"local",
@ -436,7 +435,7 @@ digraph VaultWorkerLlm {
);
llm_mock.assert();
wait_for_event_names(&context.find_run_dir(&run_id), &["run.completed"]);
wait_for_event_names(&context.single_run_dir(), &["run.completed"]);
}
#[test]
@ -731,7 +730,6 @@ fn foreground_run_rejects_invalid_workflow_before_creating_remote_run() {
#[test]
fn local_foreground_run_prints_artifact_paths_from_server_artifact_list() {
let context = test_context!();
let run_id = unique_run_id();
let workspace_dir = context.temp_dir.join("artifact-summary");
context.write_temp(
"artifact-summary/workflow.fabro",
@ -767,8 +765,6 @@ include = ["assets/**"]
.current_dir(&workspace_dir)
.env("OPENAI_API_KEY", "test")
.args([
"--run-id",
run_id.as_str(),
"--auto-approve",
"--environment",
"local",
@ -890,25 +886,22 @@ fn dry_run_rejects_goal_and_goal_file_together() {
fn dry_run_persists_event_history_in_store() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
.command()
.run_cmd()
.args([
"run",
"--dry-run",
"--auto-approve",
"--environment",
"local",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_dir = context.find_run_dir(&run_id);
let run_dir = context.single_run_dir();
let run_id = run_state(&run_dir).spec.run_id.to_string();
wait_for_event_names(&run_dir, &["run.completed", "sandbox.stop.completed"]);
let output = context
.command()
@ -991,26 +984,26 @@ fn dry_run_persists_event_history_in_store() {
}
#[test]
fn run_id_passthrough_uses_provided_ulid() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
fn run_rejects_removed_run_id_flag() {
let cli = LightweightCli::new();
context
let output = cli
.command()
.args([
"run",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
"01KRBZW5C00000000000000001",
"workflow.fabro",
])
.assert()
.success();
context.find_run_dir(&run_id);
.failure();
let stderr = String::from_utf8_lossy(&output.get_output().stderr);
assert!(
stderr.contains("unexpected argument '--run-id'"),
"{stderr}"
);
}
#[test]
@ -1100,21 +1093,19 @@ fn detach_prints_ulid_and_exits() {
#[test]
fn detach_creates_run_dir_with_detach_log() {
let context = test_context!();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let run = context
.run_cmd()
.args([
"--detach",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(run.get_output());
let run_dir = context.find_run_dir(&run_id);
fabro_json_snapshot!(

41
lib/apps/fabro-cli/tests/it/cmd/runner.rs
View file

@ -20,8 +20,9 @@ use fabro_types::{EventBody, FailureReason, RunEvent, StageId};
use httpmock::MockServer;
use super::support::{
command_log_text, find_run_dir, local_dev_token, output_stderr, run_events, run_state,
server_endpoint, server_target, wait_for_event_names, wait_for_status, write_gated_workflow,
command_log_text, created_run_id, find_run_dir, local_dev_token, output_stderr, run_events,
run_state, server_endpoint, server_target, wait_for_event_names, wait_for_status,
write_gated_workflow,
};
use crate::support::{issue_test_worker_jwt, seed_dev_token_auth, unique_run_id};
@ -243,7 +244,6 @@ fn worker_requires_fabro_worker_token_env() {
#[test]
fn runner_uses_cached_graph_after_source_deleted() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("workflow.fabro");
context.write_temp(
@ -257,18 +257,17 @@ digraph CachedGraph {
",
);
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
let server = server_target(&context.storage_dir);
@ -298,7 +297,6 @@ digraph CachedGraph {
#[test]
fn runner_local_dry_runs_ignore_github_app_configuration() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("workflow.fabro");
context.write_home(
@ -324,18 +322,17 @@ digraph GitHubApp {
",
);
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
context.write_home(".fabro/settings.toml", "_version = 1\n");
@ -361,7 +358,6 @@ digraph GitHubApp {
#[test]
fn runner_runs_without_run_json_when_run_id_is_explicit() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("workflow.fabro");
context.write_temp(
@ -375,18 +371,17 @@ digraph DetachedStoreOnly {
",
);
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
let server = server_target(&context.storage_dir);
@ -469,7 +464,6 @@ methods = ["dev-token"]
)
.expect("writing leak-probe workflow");
let run_id = unique_run_id();
let dev_token = local_dev_token(&storage_dir).expect("managed server should have a dev token");
let target = ServerTarget::unix_socket_path(&socket_path).expect("socket path should parse");
seed_dev_token_auth(&context.home_dir, &target, &dev_token);
@ -478,8 +472,6 @@ methods = ["dev-token"]
.args([
"--server",
socket_path.to_str().expect("socket path should be UTF-8"),
"--run-id",
run_id.as_str(),
"--detach",
"--auto-approve",
"--environment",
@ -496,6 +488,7 @@ methods = ["dev-token"]
String::from_utf8_lossy(&run_output.stdout),
String::from_utf8_lossy(&run_output.stderr)
);
let run_id = created_run_id(&run_output);
let run_dir = find_run_dir(&storage_dir, &run_id).expect("leak-probe run dir should exist");
wait_for_status(&run_dir, &["succeeded"]);
@ -704,7 +697,6 @@ fn runner_reports_malformed_run_state_without_prefetching_events() {
#[test]
fn detached_run_answers_pending_question_without_interview_scratch_files() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("human-gate.fabro");
context.write_temp(
@ -731,8 +723,6 @@ fn detached_run_answers_pending_question_without_interview_scratch_files() {
.args([
"run",
"--detach",
"--run-id",
run_id.as_str(),
"--environment",
"local",
workflow_path.to_str().unwrap(),
@ -746,6 +736,7 @@ fn detached_run_answers_pending_question_without_interview_scratch_files() {
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let run_id = created_run_id(&output);
let run_dir = context.find_run_dir(&run_id);
let runtime = tokio::runtime::Runtime::new().expect("test runtime should build");
@ -796,7 +787,6 @@ fn detached_run_answers_pending_question_without_interview_scratch_files() {
#[test]
fn detached_run_cancel_reaches_worker_over_control_websocket() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("cancel-over-control-websocket.fabro");
let _gate = write_gated_workflow(
&workflow_path,
@ -809,8 +799,6 @@ fn detached_run_cancel_reaches_worker_over_control_websocket() {
.args([
"run",
"--detach",
"--run-id",
run_id.as_str(),
"--environment",
"local",
workflow_path.to_str().unwrap(),
@ -824,6 +812,7 @@ fn detached_run_cancel_reaches_worker_over_control_websocket() {
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let run_id = created_run_id(&output);
let run_dir = context.find_run_dir(&run_id);
wait_for_event_names(&run_dir, &["run.running"]);
@ -857,23 +846,21 @@ fn detached_run_cancel_reaches_worker_over_control_websocket() {
#[test]
fn worker_exits_after_sigterm_cancel_even_when_stdin_stays_open() {
let context = auth_context();
let run_id = unique_run_id();
let workflow_path = context.temp_dir.join("cancel-gated.fabro");
let _gate = write_gated_workflow(&workflow_path, "cancel_gated", "Wait for cancellation");
context
let create = context
.command()
.args([
"create",
"--auto-approve",
"--environment",
"local",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
let run_dir = context.find_run_dir(&run_id);
let server = server_target(&context.storage_dir);

17
lib/apps/fabro-cli/tests/it/cmd/start.rs
View file

@ -1,7 +1,8 @@
use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context};
use super::support::{output_stdout, resolve_run, wait_for_status, write_gated_workflow};
use crate::support::unique_run_id;
use super::support::{
created_run_id, output_stdout, resolve_run, wait_for_status, write_gated_workflow,
};
const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
@ -37,21 +38,19 @@ fn help() {
fn start_by_run_id_starts_created_run() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
context
.command()
@ -89,21 +88,19 @@ fn start_by_run_id_starts_created_run() {
fn start_by_run_id_starts_created_run_without_run_json_or_status_json() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
context
.command()

57
lib/apps/fabro-cli/tests/it/cmd/support.rs
View file

@ -27,8 +27,6 @@ use httpmock::{Mock, MockServer};
use serde_json::Value;
use shlex::try_quote;
use crate::support::unique_run_id;
const LOCAL_COMMAND_TIMEOUT: Duration = Duration::from_secs(30);
const CI_COMMAND_TIMEOUT: Duration = Duration::from_secs(90);
static NEXT_SEEDED_EVENT_ID: AtomicU64 = AtomicU64::new(1);
@ -105,6 +103,14 @@ pub(crate) fn output_stdout(output: &Output) -> String {
stdout(output)
}
pub(crate) fn created_run_id(output: &Output) -> String {
stdout(output)
.trim()
.parse::<RunId>()
.expect("create command should print a run ID")
.to_string()
}
pub(crate) fn read_text(path: &Path) -> String {
std::fs::read_to_string(path)
.unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display()))
@ -260,18 +266,10 @@ pub(crate) fn setup_seeded_created_dry_run(context: &TestContext) -> RunSetup {
}
fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup {
let run_id = unique_run_id();
let mut cmd = context.run_cmd();
cmd.current_dir(&context.temp_dir);
cmd.timeout(command_timeout());
cmd.args([
"--run-id",
run_id.as_str(),
"--dry-run",
"--auto-approve",
"--environment",
"local",
]);
cmd.args(["--dry-run", "--auto-approve", "--environment", "local"]);
cmd.arg(workflow);
let output = cmd.output().expect("command should execute");
if !output.status.success() {
@ -282,10 +280,7 @@ fn run_completed_dry_run(context: &TestContext, workflow: &Path) -> RunSetup {
stderr(&output)
);
}
let run_setup = RunSetup {
run_dir: context.find_run_dir(&run_id),
run_id,
};
let run_setup = single_run_setup(context);
wait_for_event_names(&run_setup.run_dir, &[
"run.completed",
"sandbox.stop.completed",
@ -322,13 +317,10 @@ fn fast_simple_workflow(context: &TestContext) -> PathBuf {
)]
pub(crate) fn setup_detached_dry_run(context: &TestContext) -> RunSetup {
let workflow = context.install_fixture("simple.fabro");
let run_id = unique_run_id();
let mut cmd = context.run_cmd();
cmd.current_dir(&context.temp_dir);
cmd.timeout(command_timeout());
cmd.args([
"--run-id",
run_id.as_str(),
"--detach",
"--dry-run",
"--auto-approve",
@ -345,7 +337,7 @@ pub(crate) fn setup_detached_dry_run(context: &TestContext) -> RunSetup {
stderr(&output)
);
}
assert_eq!(stdout(&output).trim(), run_id);
let run_id = created_run_id(&output);
let run = resolve_run(context, &run_id);
let deadline = Instant::now() + command_timeout();
while run_events(&run.run_dir).is_empty() {
@ -427,14 +419,11 @@ id = "local"
}
fn run_local_workflow(context: &TestContext, workspace_dir: &Path, workflow: &str) -> RunSetup {
let run_id = unique_run_id();
let mut cmd = context.run_cmd();
cmd.current_dir(workspace_dir);
cmd.timeout(command_timeout());
cmd.env("OPENAI_API_KEY", "test");
cmd.args([
"--run-id",
run_id.as_str(),
"--auto-approve",
"--environment",
"local",
@ -451,10 +440,7 @@ fn run_local_workflow(context: &TestContext, workspace_dir: &Path, workflow: &st
);
}
RunSetup {
run_dir: context.find_run_dir(&run_id),
run_id,
}
single_run_setup(context)
}
pub(crate) fn add_project_workflow(
@ -652,6 +638,12 @@ fn infer_run_id(run_dir: &Path) -> String {
.expect("run directory name should contain run id suffix")
}
fn single_run_setup(context: &TestContext) -> RunSetup {
let run_dir = context.single_run_dir();
let run_id = infer_run_id(&run_dir);
RunSetup { run_id, run_dir }
}
fn block_on<T>(future: impl std::future::Future<Output = T>) -> T {
tokio::runtime::Builder::new_current_thread()
.enable_all()
@ -959,10 +951,8 @@ async fn create_seeded_run(
args: serde_json::Value,
git: Option<serde_json::Value>,
) -> RunSetup {
let run_id = unique_run_id();
let mut manifest = serde_json::json!({
"version": 1,
"run_id": run_id.as_str(),
"cwd": context.temp_dir.display().to_string(),
"target": {
"identifier": target_path,
@ -999,11 +989,12 @@ async fn create_seeded_run(
.json()
.await
.expect("seeded run create response should parse");
assert_eq!(
body["id"].as_str(),
Some(run_id.as_str()),
"seeded run should use requested run id"
);
let run_id = body["id"]
.as_str()
.expect("seeded run create response should contain an id")
.parse::<RunId>()
.expect("seeded run create response id should be valid")
.to_string();
RunSetup {
run_dir: context.find_run_dir(&run_id),

12
lib/apps/fabro-cli/tests/it/cmd/worker_auth.rs
View file

@ -23,7 +23,7 @@ use fabro_store::EventEnvelope;
use fabro_test::{apply_test_isolation, expect_reqwest_json, isolated_storage_dir, test_context};
use fabro_vault::{SecretType, Vault};
use super::support::{find_run_dir, output_stderr, output_stdout};
use super::support::{created_run_id, find_run_dir, output_stderr};
use crate::support::{
TEST_SESSION_SECRET, issue_test_github_jwt, issue_test_worker_jwt, parse_event_envelopes,
unique_run_id,
@ -299,14 +299,11 @@ async fn github_only_server_dispatched_worker_succeeds_without_worker_auth_store
let workflow = context.temp_dir.join("worker-auth.fabro");
write_probe_workflow(&workflow);
let run_id = unique_run_id();
let output = context
.run_cmd()
.args([
"--server",
&target,
"--run-id",
&run_id,
"--detach",
"--dry-run",
"--auto-approve",
@ -323,7 +320,7 @@ async fn github_only_server_dispatched_worker_succeeds_without_worker_auth_store
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(output_stdout(&output).trim(), run_id);
let run_id = created_run_id(&output);
let _run_dir = wait_for_run_dir(&server.storage_dir, &run_id);
let events = wait_for_completed_events(&server.api_base_url, &run_id, &access_token).await;
@ -356,14 +353,11 @@ fn runner_rejects_bogus_worker_token_against_github_only_server() {
let workflow = context.temp_dir.join("worker-auth-negative.fabro");
write_probe_workflow(&workflow);
let run_id = unique_run_id();
let create_output = context
.create_cmd()
.args([
"--server",
&target,
"--run-id",
&run_id,
"--dry-run",
"--auto-approve",
"--environment",
@ -379,7 +373,7 @@ fn runner_rejects_bogus_worker_token_against_github_only_server() {
String::from_utf8_lossy(&create_output.stdout),
String::from_utf8_lossy(&create_output.stderr)
);
assert_eq!(output_stdout(&create_output).trim(), run_id);
let run_id = created_run_id(&create_output);
let run_dir = wait_for_run_dir(&server.storage_dir, &run_id);
let worker_root = tempfile::tempdir_in("/tmp").unwrap();

26
lib/apps/fabro-cli/tests/it/scenario/lifecycle.rs
View file

@ -11,7 +11,7 @@ use fabro_test::{fabro_json_snapshot, test_context};
use serde_json::Value;
use super::{fixture, run_state, timeout_for};
use crate::support::unique_run_id;
use crate::cmd::support::created_run_id;
#[fabro_macros::e2e_test()]
fn local_run_lifecycle() {
@ -116,21 +116,19 @@ fn local_run_lifecycle() {
fn dry_run_create_start_attach_works_with_default_run_lookup() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
context
.command()
@ -181,22 +179,20 @@ fn dry_run_create_start_attach_works_with_default_run_lookup() {
fn dry_run_detach_attach_works_with_default_run_lookup() {
let context = test_context!();
context.ensure_home_server_auth_methods();
let run_id = unique_run_id();
let workflow = context.install_fixture("simple.fabro");
context
let run = context
.command()
.args([
"run",
"--detach",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(run.get_output());
context
.command()
@ -229,7 +225,6 @@ fn completed_run_can_be_attached_by_workflow_slug() {
let project = tempfile::tempdir().unwrap();
let workflow_dir = project.path().join("workflows").join("sluggy");
let workflow_path = workflow_dir.join("workflow.fabro");
let run_id = unique_run_id();
std::fs::create_dir_all(&workflow_dir).unwrap();
std::fs::write(
@ -244,19 +239,18 @@ digraph BarBaz {
)
.unwrap();
context
let create = context
.command()
.current_dir(project.path())
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
context
.command()
.current_dir(project.path())
@ -301,7 +295,6 @@ fn completed_run_can_be_attached_by_file_stem() {
context.ensure_home_server_auth_methods();
let workflow_dir = tempfile::tempdir().unwrap();
let workflow_path = workflow_dir.path().join("alpha.fabro");
let run_id = unique_run_id();
std::fs::write(
&workflow_path,
@ -315,18 +308,17 @@ digraph FooWorkflow {
)
.unwrap();
context
let create = context
.command()
.args([
"create",
"--dry-run",
"--auto-approve",
"--run-id",
run_id.as_str(),
workflow_path.to_str().unwrap(),
])
.assert()
.success();
let run_id = created_run_id(create.get_output());
context
.command()
.args(["start", "alpha"])

21
lib/apps/fabro-server/src/automation_materializer.rs
View file

@ -136,7 +136,6 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
let manifest_input = ManifestFromCheckoutInput {
workflow: input.target.workflow,
run_id: input.run_id,
user_settings_path: input.user_settings_path,
checkout_dir,
git_context: ManifestGitContextInput {
@ -166,7 +165,6 @@ fn parse_target_repository(value: &str) -> Result<GitHubRepositorySlug, RunMater
#[derive(Debug)]
pub(crate) struct ManifestFromCheckoutInput {
workflow: String,
run_id: RunId,
user_settings_path: PathBuf,
checkout_dir: PathBuf,
git_context: ManifestGitContextInput,
@ -185,7 +183,6 @@ fn build_manifest_from_checkout(
) -> Result<AutomationRunMaterialized, RunMaterializeError> {
let ManifestFromCheckoutInput {
workflow,
run_id,
user_settings_path,
checkout_dir,
git_context,
@ -194,7 +191,6 @@ fn build_manifest_from_checkout(
let built = fabro_manifest::build_run_manifest(ManifestBuildInput {
workflow: workflow.into(),
cwd: checkout_dir,
run_id: Some(run_id),
user_settings_path: Some(user_settings_path),
environment_defaults,
..ManifestBuildInput::default()
@ -303,7 +299,7 @@ mod tests {
use std::collections::HashMap;
use std::fs;
use fabro_types::{DirtyStatus, PreRunPushOutcome, RunId};
use fabro_types::{DirtyStatus, PreRunPushOutcome};
use tempfile::TempDir;
use super::*;
@ -334,16 +330,14 @@ mod tests {
.unwrap();
let user_settings_path = temp.path().join("settings.toml");
fs::write(&user_settings_path, "_version = 1\n").unwrap();
let run_id = RunId::new();
let repo = parse_target_repository("workspace-org/app").unwrap();
let sha = "0123456789abcdef0123456789abcdef01234567".to_string();
let materialized = build_manifest_from_checkout(ManifestFromCheckoutInput {
workflow: "demo".to_string(),
run_id,
user_settings_path: user_settings_path.clone(),
checkout_dir: checkout.clone(),
git_context: ManifestGitContextInput {
workflow: "demo".to_string(),
user_settings_path: user_settings_path.clone(),
checkout_dir: checkout.clone(),
git_context: ManifestGitContextInput {
repo,
ref_selector: "release".to_string(),
checked_out_sha: sha.clone(),
@ -352,10 +346,6 @@ mod tests {
})
.expect("manifest should build from checkout");
assert_eq!(
materialized.manifest.run_id.as_deref(),
Some(run_id.to_string().as_str())
);
assert_eq!(materialized.manifest.cwd, checkout.display().to_string());
assert_eq!(
materialized.manifest.target.path,
@ -381,6 +371,7 @@ mod tests {
let submitted_manifest: serde_json::Value =
serde_json::from_slice(&materialized.submitted_manifest_bytes)
.expect("submitted bytes should be a manifest");
assert!(submitted_manifest.get("run_id").is_none());
assert_eq!(
submitted_manifest,
serde_json::to_value(&materialized.manifest).unwrap()

7
lib/apps/fabro-server/src/run_manifest.rs
View file

@ -156,12 +156,6 @@ pub(crate) fn prepare_manifest_with_environment_defaults(
.as_ref()
.map(|title| fabro_types::normalize_explicit_run_title(title.as_str()))
.transpose()?;
manifest
.run_id
.as_deref()
.map(str::parse::<RunId>)
.transpose()
.context("invalid run ID")?;
manifest
.parent_id
.as_deref()
@ -1404,7 +1398,6 @@ mod tests {
git: None,
goal: None,
parent_id: None,
run_id: None,
title: None,
target: types::ManifestTarget {
identifier: "workflow.fabro".to_string(),

4
lib/apps/fabro-server/src/run_tool_manifest.rs
View file

@ -25,7 +25,6 @@ pub fn build_run_tool_manifest(
cli_overrides: Some(CliLayer::default()),
input_overrides: spec.inputs.clone(),
args: run_tool_manifest_args(spec),
run_id: spec.run_id,
environment_defaults: fabro_environment::seeded_catalog_layer(),
user_settings_path: Some(user_settings_path.to_path_buf()),
})
@ -106,7 +105,6 @@ mod tests {
fn create_run_spec(workflow: &str) -> ValidatedCreateRunSpec {
ValidatedCreateRunSpec::try_from(CreateRunSpec {
workflow: workflow.to_string(),
run_id: None,
parent_id: None,
cwd: None,
goal: None,
@ -164,7 +162,6 @@ mod tests {
fn manifest_args_preserve_input_provenance() {
let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
workflow: "simple".to_string(),
run_id: None,
parent_id: None,
cwd: None,
goal: None,
@ -196,7 +193,6 @@ mod tests {
fn run_overrides_preserve_goal_file_as_file_goal() {
let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
workflow: "implement-plan".to_string(),
run_id: None,
parent_id: None,
cwd: None,
goal: None,

10
lib/apps/fabro-server/src/server/handler/runs.rs
View file

@ -550,6 +550,8 @@ async fn create_run(
pub(crate) struct CreateRunFromManifestRequest {
pub(crate) manifest: RunManifest,
pub(crate) submitted_manifest_bytes: Vec<u8>,
/// Run ID preallocated by server-side automation code, never supplied by
/// an HTTP create body.
pub(crate) explicit_run_id: Option<RunId>,
pub(crate) explicit_title_supplied: bool,
pub(crate) actor: Principal,
@ -650,12 +652,6 @@ fn manifest_run_identity(
.as_ref()
.map(|title| fabro_types::normalize_explicit_run_title(title.as_str()))
.transpose()?;
let manifest_run_id = manifest
.run_id
.as_deref()
.map(str::parse::<RunId>)
.transpose()
.context("invalid run ID")?;
let parent_id = manifest
.parent_id
.as_deref()
@ -663,7 +659,7 @@ fn manifest_run_identity(
.transpose()
.context("invalid parent run ID")?;
Ok(ManifestRunIdentity {
run_id: explicit_run_id.or(manifest_run_id),
run_id: explicit_run_id,
parent_id,
title,
})

17
lib/apps/fabro-server/src/server/tests.rs
View file

@ -3545,6 +3545,23 @@ async fn post_run_manifest(app: &Router, manifest: serde_json::Value) -> serde_j
response_json!(response, StatusCode::CREATED).await
}
#[tokio::test]
async fn post_runs_ignores_removed_run_id_input() {
let app = crate::test_support::build_test_router(test_app_state());
let submitted_run_id = RunId::new();
let mut manifest = minimal_manifest_json(MINIMAL_DOT);
manifest["run_id"] = json!(submitted_run_id.to_string());
let created = post_run_manifest(&app, manifest).await;
let allocated_run_id = created["id"]
.as_str()
.expect("create response should contain an id")
.parse::<RunId>()
.expect("create response id should be valid");
assert_ne!(allocated_run_id, submitted_run_id);
}
#[tokio::test]
async fn post_runs_create_regression_keeps_api_behavior_without_automation_metadata() {
let state = TestAppStateBuilder::new()

6
lib/components/fabro-manifest/src/lib.rs
View file

@ -24,9 +24,7 @@ use fabro_template::{
};
use fabro_types::settings::interp::InterpString;
use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode};
use fabro_types::{
DirtyStatus, GitContext, ManifestPath, PreRunPushOutcome, RunId, WorkflowSettings,
};
use fabro_types::{DirtyStatus, GitContext, ManifestPath, PreRunPushOutcome, WorkflowSettings};
use fabro_workflow::git::{
GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status,
};
@ -42,7 +40,6 @@ pub struct ManifestBuildInput {
pub cli_overrides: Option<CliLayer>,
pub input_overrides: HashMap<String, toml::Value>,
pub args: Option<types::ManifestArgs>,
pub run_id: Option<RunId>,
pub environment_defaults: MergeMap<EnvironmentLayer>,
/// Path to the user settings file (for inclusion in
/// `RunManifest.configs`). `None` skips the user config entry.
@ -254,7 +251,6 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
git,
goal,
parent_id: None,
run_id: input.run_id.map(|run_id| run_id.to_string()),
title: None,
target: types::ManifestTarget {
identifier: input.workflow.display().to_string(),

54
lib/components/fabro-tool/src/create.rs
View file

@ -92,13 +92,6 @@ impl JsonSchema for CreateRunSpecInput {
],
"description": "Working directory used to resolve relative workflow paths."
},
"run_id": {
"anyOf": [
{ "type": "string" },
{ "type": "null" }
],
"description": "Optional run id to use for the created run."
},
"parent_id": {
"anyOf": [
{ "type": "string" },
@ -198,7 +191,6 @@ impl JsonSchema for CreateRunSpecInput {
pub struct CreateRunSpec {
pub workflow: String,
pub cwd: Option<PathBuf>,
pub run_id: Option<String>,
pub parent_id: Option<String>,
pub goal: Option<String>,
pub goal_file: Option<PathBuf>,
@ -262,7 +254,6 @@ pub struct ValidatedCreateRuns {
pub struct ValidatedCreateRunSpec {
pub workflow: String,
pub cwd: Option<PathBuf>,
pub run_id: Option<RunId>,
pub parent_id: Option<String>,
pub goal: Option<String>,
pub goal_file: Option<PathBuf>,
@ -304,7 +295,6 @@ impl TryFrom<CreateRunSpecInput> for ValidatedCreateRunSpec {
Self::try_from(CreateRunSpec {
workflow: workflow.to_string(),
cwd: None,
run_id: None,
parent_id: None,
goal: None,
goal_file: None,
@ -328,14 +318,6 @@ impl TryFrom<CreateRunSpec> for ValidatedCreateRunSpec {
type Error = ToolError;
fn try_from(spec: CreateRunSpec) -> Result<Self, Self::Error> {
let run_id = spec
.run_id
.as_deref()
.map(str::parse::<RunId>)
.transpose()
.map_err(|err| {
ToolError::message(format!("run_id must be a valid Fabro run id: {err}"))
})?;
let parent_id = spec
.parent_id
.as_deref()
@ -368,7 +350,6 @@ impl TryFrom<CreateRunSpec> for ValidatedCreateRunSpec {
Ok(Self {
workflow: spec.workflow,
cwd: spec.cwd,
run_id,
parent_id,
goal: spec.goal,
goal_file: spec.goal_file,
@ -532,12 +513,23 @@ mod tests {
);
}
#[test]
fn create_spec_schema_omits_run_id() {
let mut generator = SchemaGenerator::default();
let schema = CreateRunSpecInput::json_schema(&mut generator);
let schema = serde_json::to_value(schema).expect("schema should serialize");
let properties = schema["anyOf"][1]["properties"]
.as_object()
.expect("object form should have properties");
assert!(!properties.contains_key("run_id"));
}
#[test]
fn create_spec_accepts_parent_selector() {
let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
workflow: "simple.fabro".to_string(),
cwd: None,
run_id: None,
parent_id: Some(" nightly-parent ".to_string()),
goal: None,
goal_file: None,
@ -568,7 +560,6 @@ mod tests {
let spec = &params.runs[0];
assert_eq!(spec.workflow, "simple.fabro");
assert_eq!(spec.cwd, None);
assert_eq!(spec.run_id, None);
assert_eq!(spec.parent_id, None);
assert!(spec.inputs.is_empty());
assert!(spec.labels.is_empty());
@ -601,6 +592,23 @@ mod tests {
assert_eq!(spec.start, Some(false));
}
#[test]
fn create_params_ignore_removed_run_id() {
let params: FabroRunCreateParams = serde_json::from_value(json!({
"runs": [{
"workflow": "simple.fabro",
"run_id": "not-a-valid-run-id",
"start": false
}]
}))
.expect("old object form should deserialize with run_id ignored");
let params =
ValidatedCreateRuns::try_from(params).expect("remaining create fields should validate");
assert_eq!(params.runs[0].workflow, "simple.fabro");
assert_eq!(params.runs[0].start, Some(false));
}
#[test]
fn create_params_preserve_goal_file_option() {
let params: FabroRunCreateParams = serde_json::from_value(json!({
@ -683,7 +691,6 @@ mod tests {
CreateRunSpec {
workflow: "simple.fabro".to_string(),
cwd: None,
run_id: None,
parent_id: Some("nightly-parent".to_string()),
goal: None,
goal_file: None,
@ -734,7 +741,6 @@ mod tests {
CreateRunSpecInput::from(CreateRunSpec {
workflow: "simple.fabro".to_string(),
cwd: None,
run_id: None,
parent_id: Some("nightly-parent".to_string()),
goal: None,
goal_file: None,
@ -784,7 +790,6 @@ mod tests {
CreateRunSpec {
workflow: "simple.fabro".to_string(),
cwd: None,
run_id: None,
parent_id: Some(parent_id.to_string()),
goal: None,
goal_file: None,
@ -839,7 +844,6 @@ mod tests {
CreateRunSpec {
workflow: "simple.fabro".to_string(),
cwd: None,
run_id: None,
parent_id: Some(parent_id.to_string()),
goal: None,
goal_file: None,

4
lib/packages/fabro-api-client/src/models/run-manifest.ts generated
View file

@ -40,10 +40,6 @@ export interface RunManifest {
* Manifest schema version.
*/
'version': number;
/**
* Optional pre-generated run ID to use instead of allocating a new ULID.
*/
'run_id'?: string | null;
/**
* Optional orchestration parent run ID. Fork and rewind lineage use separate fields and should not set this value.
*/