From cff0a2461a3e1ca36bf53beab2e3040a283b94ef Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Mon, 3 Aug 2026 15:09:25 -0400 Subject: [PATCH] Trim unused run creation event fields --- lib/components/fabro-store/src/run_state.rs | 51 +++++++++++++++---- .../fabro-workflow/src/event/convert.rs | 9 ---- .../fabro-workflow/src/event/events.rs | 11 +--- .../fabro-workflow/src/operations/create.rs | 30 ----------- .../fabro-workflow/src/operations/fork.rs | 6 --- .../fabro-workflow/src/operations/retry.rs | 6 --- .../fabro-workflow/src/operations/source.rs | 20 +++----- .../fabro-types/src/run_event/run.rs | 5 -- .../fabro-types/tests/run_event_serde.rs | 47 ++++++++++++++--- 9 files changed, 93 insertions(+), 92 deletions(-) diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index d2042a4c6..05fb4209a 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -2337,7 +2337,6 @@ mod tests { "settings": WorkflowSettings::default(), "graph": Graph::new("test"), "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -2351,6 +2350,48 @@ mod tests { ); } + #[test] + fn historical_run_created_with_removed_fields_replays_into_projection() { + let provenance = test_support::test_run_provenance(); + let event = test_raw_event( + 1, + "run.created", + &json!({ + "title": "Historical run", + "settings": WorkflowSettings::default(), + "graph": Graph::new("historical"), + "workflow_source": "digraph historical { start -> exit }", + "workflow_config": "[workflow]\ngraph = \"workflow.fabro\"", + "labels": {"team": "platform"}, + "run_dir": "/tmp/historical-run", + "source_directory": "/workspace/project", + "workflow_slug": "historical", + "db_prefix": "run_", + "provenance": provenance + }), + None, + ); + + let projection = RunProjection::apply_events(&[event]).unwrap(); + + assert_eq!(projection.title(), "Historical run"); + assert_eq!(projection.spec.graph.name, "historical"); + assert_eq!( + projection.spec.graph_source.as_deref(), + Some("digraph historical { start -> exit }") + ); + assert_eq!( + projection.spec.labels.get("team").map(String::as_str), + Some("platform") + ); + assert_eq!( + projection.spec.source_directory.as_deref(), + Some("/workspace/project") + ); + assert_eq!(projection.spec.workflow_slug.as_deref(), Some("historical")); + assert_eq!(projection.spec.provenance, provenance); + } + #[test] fn run_created_projects_automation_into_spec_and_summary() { let automation = AutomationRef { @@ -2366,7 +2407,6 @@ mod tests { "graph": Graph::new("test"), "automation": automation, "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -2390,7 +2430,6 @@ mod tests { "settings": WorkflowSettings::default(), "graph": Graph::new("test"), "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -2413,7 +2452,6 @@ mod tests { "settings": WorkflowSettings::default(), "graph": Graph::new("test"), "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -2491,7 +2529,6 @@ mod tests { "settings": WorkflowSettings::default(), "graph": Graph::new("test"), "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -4082,7 +4119,6 @@ mod tests { "attrs": { "goal": { "String": "Goal title" } } }, "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -4110,7 +4146,6 @@ mod tests { "attrs": { "goal": { "String": "## Plan: Legacy title\n\nDetails" } } }, "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -4140,7 +4175,6 @@ mod tests { "attrs": { "goal": { "String": "Goal title" } } }, "labels": {}, - "run_dir": "/tmp/run", "provenance": test_support::test_run_provenance() }), None, @@ -4183,7 +4217,6 @@ mod tests { "attrs": {} }, "labels": {}, - "run_dir": "/tmp/run", "source_directory": "/tmp/run", "provenance": test_support::test_run_provenance(), "manifest_blob": manifest_blob diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs index 3a99f6b64..347a308a3 100644 --- a/lib/components/fabro-workflow/src/event/convert.rs +++ b/lib/components/fabro-workflow/src/event/convert.rs @@ -30,13 +30,10 @@ fn event_body_from_event(event: &Event) -> EventBody { settings, graph, workflow_source, - workflow_config, labels, - run_dir, source_directory, workflow_slug, automation, - db_prefix, provenance, manifest_blob, git, @@ -51,13 +48,10 @@ fn event_body_from_event(event: &Event) -> EventBody { .expect("run.created settings should deserialize: value was serialized from a typed struct in this session"), graph: serde_json::from_value(graph.clone()).expect("run.created graph should deserialize: value was serialized from a typed struct in this session"), workflow_source: workflow_source.clone(), - workflow_config: workflow_config.clone(), labels: labels.clone(), - run_dir: run_dir.clone(), source_directory: source_directory.clone(), workflow_slug: workflow_slug.clone(), automation: automation.clone(), - db_prefix: db_prefix.clone(), provenance: provenance.clone(), manifest_blob: *manifest_blob, git: git.clone(), @@ -2657,13 +2651,10 @@ mod tests { settings: serde_json::to_value(WorkflowSettings::default()).unwrap(), graph: serde_json::to_value(Graph::new("test")).unwrap(), workflow_source: None, - workflow_config: None, labels: BTreeMap::default(), - run_dir: "/tmp/run".to_string(), source_directory: Some("/tmp/run".to_string()), workflow_slug: None, automation: Some(automation.clone()), - db_prefix: None, provenance, manifest_blob: None, git: None, diff --git a/lib/components/fabro-workflow/src/event/events.rs b/lib/components/fabro-workflow/src/event/events.rs index 126b23d90..2b05f7acb 100644 --- a/lib/components/fabro-workflow/src/event/events.rs +++ b/lib/components/fabro-workflow/src/event/events.rs @@ -30,18 +30,13 @@ pub enum Event { graph: serde_json::Value, #[serde(default, skip_serializing_if = "Option::is_none")] workflow_source: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - workflow_config: Option, labels: BTreeMap, - run_dir: String, #[serde(default, skip_serializing_if = "Option::is_none")] source_directory: Option, #[serde(default, skip_serializing_if = "Option::is_none")] workflow_slug: Option, #[serde(default, skip_serializing_if = "Option::is_none")] automation: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - db_prefix: Option, provenance: RunProvenance, #[serde(default, skip_serializing_if = "Option::is_none")] manifest_blob: Option, @@ -795,10 +790,8 @@ impl Event { pub fn trace(&self) { use tracing::{debug, error, info, warn}; match self { - Self::RunCreated { - run_id, run_dir, .. - } => { - info!(run_id = %run_id, run_dir, "Run created"); + Self::RunCreated { run_id, .. } => { + info!(run_id = %run_id, "Run created"); } Self::WorkflowRunStarted { name, run_id, .. } => { info!(workflow = name.as_str(), run_id = %run_id, "Workflow run started"); diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 15d4f85b6..ffd29f3b6 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -156,7 +156,6 @@ pub struct CompiledRun { settings: WorkflowSettings, raw_source: String, workflow_slug: Option, - workflow_config: Option, dot_path: Option, definition: Option, source_directory: String, @@ -181,7 +180,6 @@ pub struct MaterializedRun { settings: WorkflowSettings, raw_source: String, workflow_slug: Option, - workflow_config: Option, dot_path: Option, definition: Option, source_directory: String, @@ -290,10 +288,6 @@ pub fn compile_create_run( .map_err(|err| Error::Parse(err.to_string()))?; let settings = resolved.settings; let labels = settings.combined_labels(); - let workflow_config = resolved - .workflow_toml_path - .as_deref() - .and_then(|path| std::fs::read_to_string(path).ok()); let source_name = resolved .dot_path .as_ref() @@ -349,7 +343,6 @@ pub fn compile_create_run( settings, raw_source: resolved.raw_source, workflow_slug: resolved.workflow_slug, - workflow_config, dot_path: resolved.dot_path, definition, source_directory: resolved.working_directory.to_string_lossy().to_string(), @@ -368,7 +361,6 @@ pub fn materialize_create_run( settings, raw_source, workflow_slug, - workflow_config, dot_path, definition, source_directory, @@ -386,7 +378,6 @@ pub fn materialize_create_run( settings, raw_source, workflow_slug, - workflow_config, dot_path, definition, source_directory, @@ -458,7 +449,6 @@ pub async fn persist_create_run( settings, raw_source, workflow_slug: _, - workflow_config, dot_path, definition, source_directory, @@ -493,7 +483,6 @@ pub async fn persist_create_run( store, &persisted, &raw_source, - workflow_config, submitted_manifest_bytes.as_deref(), definition.as_ref(), title, @@ -514,7 +503,6 @@ async fn persist_created_run( store: &Database, persisted: &Persisted, workflow_source: &str, - workflow_config: Option, submitted_manifest_bytes: Option<&[u8]>, accepted_definition: Option<&RunDefinition>, explicit_title: Option, @@ -554,17 +542,14 @@ async fn persist_created_run( .map_err(|err| Error::engine(err.to_string()))?, ), workflow_source: (!workflow_source.is_empty()).then(|| workflow_source.to_string()), - workflow_config, labels: record .labels .clone() .into_iter() .collect::>(), - run_dir: persisted.run_dir().display().to_string(), source_directory: record.source_directory.clone(), workflow_slug: record.workflow_slug.clone(), automation: record.automation.clone(), - db_prefix: None, provenance: record.provenance.clone(), manifest_blob, git: record.git.clone(), @@ -1753,20 +1738,9 @@ reasoning = false web_url: None, }; let catalog = test_catalog(); - let workflow_config_path = dir.path().join("workflow.toml"); - std::fs::write( - &workflow_config_path, - "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", - ) - .unwrap(); let compiled = compile_create_run(compile_input(&request), Arc::clone(&catalog)).unwrap(); - assert_eq!( - compiled.workflow_config.as_deref(), - Some("_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n") - ); std::fs::write(&dot_path, "this is no longer a graph").unwrap(); - std::fs::write(&workflow_config_path, "changed after compilation").unwrap(); let materialized = materialize_create_run(compiled, catalog.as_ref()).unwrap(); let metadata = persistence_metadata(&request, fixtures::RUN_2, &storage_root); @@ -1798,10 +1772,6 @@ reasoning = false created.workflow_source.as_deref(), Some(compiled_source.as_str()) ); - assert_eq!( - created.workflow_config.as_deref(), - Some("_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n") - ); let manifest_blob = created .manifest_blob .as_ref() diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index 559df2746..a82fc5c65 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -156,13 +156,10 @@ async fn persist_forked_run( graph: serde_json::to_value(&spec.graph) .map_err(|err| Error::engine(err.to_string()))?, workflow_source: projection.spec.graph_source.clone(), - workflow_config: None, labels: spec.labels.clone().into_iter().collect(), - run_dir: String::new(), source_directory: spec.source_directory.clone(), workflow_slug: spec.workflow_slug.clone(), automation: spec.automation.clone(), - db_prefix: None, provenance: spec.provenance.clone(), manifest_blob: spec.manifest_blob, git: spec.git.clone(), @@ -378,13 +375,10 @@ mod tests { settings: serde_json::to_value(&settings).unwrap(), graph: serde_json::to_value(&graph).unwrap(), workflow_source: Some("digraph fork_source {}".to_string()), - workflow_config: None, labels: BTreeMap::new(), - run_dir: "/tmp/source".to_string(), source_directory: Some("/client/source".to_string()), workflow_slug: Some("fork-source".to_string()), automation: None, - db_prefix: None, provenance: test_support::test_run_provenance(), manifest_blob: None, git: Some(fabro_types::GitContext { diff --git a/lib/components/fabro-workflow/src/operations/retry.rs b/lib/components/fabro-workflow/src/operations/retry.rs index a52997ccd..d9249ddc8 100644 --- a/lib/components/fabro-workflow/src/operations/retry.rs +++ b/lib/components/fabro-workflow/src/operations/retry.rs @@ -72,13 +72,10 @@ pub async fn retry_run( settings, graph, workflow_source: graph_source, - workflow_config: None, labels: labels.into_iter().collect::>(), - run_dir: String::new(), source_directory, workflow_slug, automation, - db_prefix: None, provenance: input.provenance.clone(), manifest_blob, git, @@ -183,13 +180,10 @@ mod tests { settings: serde_json::to_value(&settings).unwrap(), graph: serde_json::to_value(Graph::new("retry_source")).unwrap(), workflow_source: Some("digraph retry_source { start -> exit }".to_string()), - workflow_config: None, labels: labels.into_iter().collect(), - run_dir: "/tmp/source".to_string(), source_directory: Some("/workspace/source".to_string()), workflow_slug: Some("retry-source".to_string()), automation: None, - db_prefix: None, provenance: provenance("source-user"), manifest_blob, git: Some(git_context()), diff --git a/lib/components/fabro-workflow/src/operations/source.rs b/lib/components/fabro-workflow/src/operations/source.rs index bb00adaca..3e275937c 100644 --- a/lib/components/fabro-workflow/src/operations/source.rs +++ b/lib/components/fabro-workflow/src/operations/source.rs @@ -35,15 +35,14 @@ pub(crate) struct ResolveWorkflowInput { #[derive(Clone)] pub(crate) struct ResolvedWorkflow { - pub raw_source: String, - pub settings: WorkflowSettings, - pub workflow_slug: Option, - pub workflow_toml_path: Option, - pub dot_path: Option, - pub current_dir: Option, - pub file_resolver: Option>, - pub goal_override: Option, - pub working_directory: PathBuf, + pub raw_source: String, + pub settings: WorkflowSettings, + pub workflow_slug: Option, + pub dot_path: Option, + pub current_dir: Option, + pub file_resolver: Option>, + pub goal_override: Option, + pub working_directory: PathBuf, } pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result { @@ -60,7 +59,6 @@ pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result< raw_source, settings, workflow_slug: location.slug, - workflow_toml_path: location.toml, dot_path: Some(location.graph), current_dir: Some(location.dir), file_resolver: Some(Arc::new(FilesystemFileResolver::new(Some( @@ -79,7 +77,6 @@ pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result< raw_source: source, settings, workflow_slug: None, - workflow_toml_path: None, dot_path: None, current_dir: base_dir, file_resolver: has_base_dir.then(|| { @@ -100,7 +97,6 @@ pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result< raw_source: workflow.source.clone(), settings, workflow_slug: workflow_slug_from_path(workflow.path.as_path()), - workflow_toml_path: None, dot_path: Some(workflow.path.as_path().to_path_buf()), current_dir: Some(workflow.current_dir()), file_resolver: Some(workflow.file_resolver()), diff --git a/lib/foundation/fabro-types/src/run_event/run.rs b/lib/foundation/fabro-types/src/run_event/run.rs index 06077171d..d070d8aef 100644 --- a/lib/foundation/fabro-types/src/run_event/run.rs +++ b/lib/foundation/fabro-types/src/run_event/run.rs @@ -17,19 +17,14 @@ pub struct RunCreatedProps { pub graph: Graph, #[serde(default, skip_serializing_if = "Option::is_none")] pub workflow_source: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub workflow_config: Option, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub labels: BTreeMap, - pub run_dir: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub source_directory: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub workflow_slug: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub automation: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub db_prefix: Option, pub provenance: RunProvenance, #[serde(default, skip_serializing_if = "Option::is_none")] pub manifest_blob: Option, diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 5d25d0869..723c5c003 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -22,9 +22,7 @@ fn run_created_props_round_trip_templated_settings() { settings: templated_settings(), graph: Graph::new("ship"), workflow_source: Some("digraph Ship { start -> exit }".to_string()), - workflow_config: Some("[run]\ngoal = \"Ship {{ env.TASK }}\"".to_string()), labels: BTreeMap::from([("team".to_string(), "platform".to_string())]), - run_dir: "/tmp/run".to_string(), source_directory: Some("/Users/client/project".to_string()), workflow_slug: Some("demo".to_string()), automation: Some(AutomationRef { @@ -32,7 +30,6 @@ fn run_created_props_round_trip_templated_settings() { name: Some("Nightly".to_string()), trigger_id: Some("schedule_1".to_string()), }), - db_prefix: Some("run_".to_string()), provenance: test_run_provenance(), manifest_blob: None, git: Some(GitContext { @@ -91,13 +88,10 @@ fn run_created_props_omits_web_url_when_absent() { settings: WorkflowSettings::default(), graph: Graph::new("ship"), workflow_source: None, - workflow_config: None, labels: BTreeMap::new(), - run_dir: "/tmp/run".to_string(), source_directory: None, workflow_slug: None, automation: None, - db_prefix: None, provenance: test_run_provenance(), manifest_blob: None, git: None, @@ -145,6 +139,47 @@ fn run_created_props_defaults_additive_fields_for_legacy_events() { assert_eq!(props.automation, None); } +#[test] +fn run_created_props_accepts_removed_fields_without_reserializing_them() { + let historical = serde_json::json!({ + "title": "Ship task", + "settings": templated_settings(), + "graph": Graph::new("ship"), + "workflow_source": "digraph Ship { start -> exit }", + "workflow_config": "[run]\ngoal = \"Ship {{ env.TASK }}\"", + "labels": {"team": "platform"}, + "run_dir": "/tmp/run", + "source_directory": "/Users/client/project", + "db_prefix": "run_", + "provenance": test_run_provenance() + }); + + let props: RunCreatedProps = + serde_json::from_value(historical).expect("historical props should deserialize"); + let canonical = serde_json::to_value(props).expect("props should serialize canonically"); + + assert_eq!(canonical["title"], "Ship task"); + assert_eq!(canonical["graph"]["name"], "ship"); + assert_eq!( + canonical["workflow_source"], + "digraph Ship { start -> exit }" + ); + assert_eq!(canonical["labels"]["team"], "platform"); + assert_eq!(canonical["source_directory"], "/Users/client/project"); + assert_eq!(canonical["settings"]["run"]["goal"]["type"], "inline"); + assert_eq!( + canonical["settings"]["run"]["goal"]["value"], + "Ship {{ env.TASK }}" + ); + assert!(canonical.get("provenance").is_some()); + for removed in ["workflow_config", "run_dir", "db_prefix"] { + assert!( + canonical.get(removed).is_none(), + "removed field {removed} must not be reserialized: {canonical}" + ); + } +} + #[test] fn run_parent_events_round_trip_parent_ids() { let linked = EventBody::RunParentLinked(RunParentLinkedProps {