diff --git a/lib/apps/fabro-cli/src/commands/preflight.rs b/lib/apps/fabro-cli/src/commands/preflight.rs index c148ddfa9..bfcc62549 100644 --- a/lib/apps/fabro-cli/src/commands/preflight.rs +++ b/lib/apps/fabro-cli/src/commands/preflight.rs @@ -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...")); diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 3268d5b3b..b8bde173f 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -40,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: None, environment_defaults: fabro_environment::seeded_catalog_layer(), user_settings_path: Some(active_settings_path(None)), })?; diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index c7d808b33..77e1c7bac 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -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 Result { 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,7 @@ 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.run_id, None); assert_eq!(materialized.manifest.cwd, checkout.display().to_string()); assert_eq!( materialized.manifest.target.path, @@ -381,6 +372,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() diff --git a/lib/apps/fabro-server/src/run_tool_manifest.rs b/lib/apps/fabro-server/src/run_tool_manifest.rs index 32cb14bd3..a27f92e05 100644 --- a/lib/apps/fabro-server/src/run_tool_manifest.rs +++ b/lib/apps/fabro-server/src/run_tool_manifest.rs @@ -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, diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 91014a781..8f655313d 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -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, pub input_overrides: HashMap, pub args: Option, - pub run_id: Option, pub environment_defaults: MergeMap, /// Path to the user settings file (for inclusion in /// `RunManifest.configs`). `None` skips the user config entry. @@ -254,7 +251,7 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result { git, goal, parent_id: None, - run_id: input.run_id.map(|run_id| run_id.to_string()), + run_id: None, title: None, target: types::ManifestTarget { identifier: input.workflow.display().to_string(), diff --git a/lib/components/fabro-tool/src/create.rs b/lib/components/fabro-tool/src/create.rs index 57e41acd5..448e5e84f 100644 --- a/lib/components/fabro-tool/src/create.rs +++ b/lib/components/fabro-tool/src/create.rs @@ -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, - pub run_id: Option, pub parent_id: Option, pub goal: Option, pub goal_file: Option, @@ -262,7 +254,6 @@ pub struct ValidatedCreateRuns { pub struct ValidatedCreateRunSpec { pub workflow: String, pub cwd: Option, - pub run_id: Option, pub parent_id: Option, pub goal: Option, pub goal_file: Option, @@ -304,7 +295,6 @@ impl TryFrom 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 for ValidatedCreateRunSpec { type Error = ToolError; fn try_from(spec: CreateRunSpec) -> Result { - let run_id = spec - .run_id - .as_deref() - .map(str::parse::) - .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 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 = ¶ms.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,