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

Remove run IDs from create manifest producers

Browse source
This commit is contained in:
Scott Werner 2026-08-01 11:21:41 -04:00
parent 3a558b225e
commit be91a5ef89
6 changed files with 45 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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..."));

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

@ -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)),
})?;

22
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,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()

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,

7
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,7 @@ 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()),
run_id: None,
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,