diff --git a/apps/fabro-web/app/data/runs.test.ts b/apps/fabro-web/app/data/runs.test.ts index 391cb8c4d..c2365637f 100644 --- a/apps/fabro-web/app/data/runs.test.ts +++ b/apps/fabro-web/app/data/runs.test.ts @@ -14,7 +14,7 @@ function makeRun(overrides: Partial = {}): Run { id: "01ABC", goal: "Fix the build", title: "Fix the build", - workflow: { slug: "fix_build", name: "Fix Build" }, + workflow: { slug: "fix_build", name: "Fix Build", graph_name: "FixBuild" }, automation: null, repository: { name: "myrepo", origin_url: null, provider: "unknown" }, created_by: null, @@ -77,7 +77,7 @@ describe("mapRunListItem", () => { const item = mapRunListItem(summary); expect(item.id).toBe("01ABC"); expect(item.title).toBe("Server supplied title"); - expect(item.workflow).toBe("fix_build"); + expect(item.workflow).toBe("Fix Build"); expect(item.repo).toBe("myrepo"); expect(item.sourceDirectory).toBe("/home/user/myrepo"); expect(item.elapsed).toBeDefined(); @@ -107,7 +107,7 @@ describe("mapRunToRunItem", () => { const item = mapRunToRunItem(summary); expect(item.id).toBe("01ABC"); expect(item.title).toBe("Fix the build"); - expect(item.workflow).toBe("fix_build"); + expect(item.workflow).toBe("Fix Build"); expect(item.repo).toBe("myrepo"); expect(item.sourceDirectory).toBe("/home/user/myrepo"); expect(item.elapsed).toBeDefined(); @@ -121,7 +121,7 @@ describe("mapRunToRunItem", () => { id: "01DEF", goal: "", title: "", - workflow: { slug: null, name: "unknown" }, + workflow: { slug: null, name: null, graph_name: null }, source_directory: null, repository: { name: "unknown", origin_url: null, provider: "unknown" }, ...withStatus({ kind: "submitted" }), @@ -143,6 +143,18 @@ describe("mapRunToRunItem", () => { expect(item.sourceDirectory).toBeUndefined(); }); + test("falls back to graph name and slug for workflow labels", () => { + const graphFallback = mapRunToRunItem( + makeRun({ workflow: { slug: "fix_build", name: null, graph_name: "FixBuild" } }), + ); + const slugFallback = mapRunToRunItem( + makeRun({ workflow: { slug: "fix_build", name: null, graph_name: null } }), + ); + + expect(graphFallback.workflow).toBe("FixBuild"); + expect(slugFallback.workflow).toBe("fix_build"); + }); + test("recognizes canonical blocked and queued run statuses", () => { expect(isRunStatus("queued")).toBe(true); expect(isRunStatus("blocked")).toBe(true); diff --git a/apps/fabro-web/app/data/runs.ts b/apps/fabro-web/app/data/runs.ts index 28a0160a8..36b7c2f3c 100644 --- a/apps/fabro-web/app/data/runs.ts +++ b/apps/fabro-web/app/data/runs.ts @@ -85,7 +85,7 @@ export function mapRunListItem(item: Run): RunItem { id: item.id, repo: displayRepoName(item.repository?.name ?? "unknown"), title: displayRunTitle(item.title), - workflow: item.workflow.slug ?? item.workflow.name ?? "unknown", + workflow: item.workflow.name ?? item.workflow.graph_name ?? item.workflow.slug ?? "unknown", column: columnForRun(item) ?? undefined, lifecycleStatus, lifecycleStatusLabel: lifecycleStatusLabel(item.lifecycle.status, item.lifecycle.archived), diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 308cf77c1..55670aab0 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -7386,12 +7386,14 @@ components: WorkflowRef: type: object - required: [slug, name] + required: [slug, name, graph_name] properties: slug: type: ["string", "null"] name: - type: string + type: ["string", "null"] + graph_name: + type: ["string", "null"] AutomationRef: type: object diff --git a/lib/crates/fabro-api/tests/run_summary_round_trip.rs b/lib/crates/fabro-api/tests/run_summary_round_trip.rs index 33f7cb908..83d7ce4e9 100644 --- a/lib/crates/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/crates/fabro-api/tests/run_summary_round_trip.rs @@ -29,8 +29,9 @@ fn run_summary_json_matches_openapi_shape() { title: "API title".to_string(), goal: String::new(), workflow: WorkflowRef { - slug: Some("workflow".to_string()), - name: "workflow".to_string(), + slug: Some("workflow".to_string()), + name: Some("Ship workflow".to_string()), + graph_name: Some("GraphName".to_string()), }, automation: None, repository: Some(RepositoryRef { @@ -89,7 +90,8 @@ fn run_summary_json_matches_openapi_shape() { "goal": "", "workflow": { "slug": "workflow", - "name": "workflow" + "name": "Ship workflow", + "graph_name": "GraphName" }, "automation": null, "repository": { @@ -159,7 +161,8 @@ fn run_summary_deserializes_when_optional_fields_are_absent() { "title": "ship it", "workflow": { "slug": null, - "name": "unnamed" + "name": null, + "graph_name": "GraphName" }, "origin": { "kind": "api" @@ -191,7 +194,8 @@ fn run_summary_deserializes_when_optional_fields_are_absent() { assert_eq!(summary.id, run_id); assert_eq!(summary.children_count, 0); - assert_eq!(summary.workflow.name, "unnamed"); + assert_eq!(summary.workflow.name, None); + assert_eq!(summary.workflow.graph_name.as_deref(), Some("GraphName")); assert_eq!(summary.workflow.slug, None); assert_eq!(summary.goal, "ship it"); assert_eq!(summary.title, "ship it"); diff --git a/lib/crates/fabro-cli/src/commands/runs/list.rs b/lib/crates/fabro-cli/src/commands/runs/list.rs index 120539a74..93ccab216 100644 --- a/lib/crates/fabro-cli/src/commands/runs/list.rs +++ b/lib/crates/fabro-cli/src/commands/runs/list.rs @@ -49,6 +49,7 @@ pub(crate) async fn list_command( "run_id": run.run_id(), "parent_id": run.parent_id(), "workflow_name": run.workflow_name(), + "workflow_graph_name": run.workflow_graph_name(), "workflow_slug": run.workflow_slug(), "status": run.status(), "start_time": run.start_time(), @@ -138,7 +139,7 @@ pub(crate) async fn list_command( ); } row.extend([ - run.workflow_name().cell(), + run.workflow_display_name().cell(), status_cell(run.status(), use_color), dir_display.cell(), duration_display.cell(), diff --git a/lib/crates/fabro-cli/src/server_runs.rs b/lib/crates/fabro-cli/src/server_runs.rs index 4f90bd91e..2a4b20543 100644 --- a/lib/crates/fabro-cli/src/server_runs.rs +++ b/lib/crates/fabro-cli/src/server_runs.rs @@ -25,14 +25,37 @@ impl ServerRunInfo { self.run.parent_id } - pub(crate) fn workflow_name(&self) -> String { - self.run.workflow.name.clone() + pub(crate) fn workflow_name(&self) -> Option<&str> { + self.run.workflow.name.as_deref() + } + + pub(crate) fn workflow_graph_name(&self) -> Option<&str> { + self.run.workflow.graph_name.as_deref() } pub(crate) fn workflow_slug(&self) -> Option<&str> { self.run.workflow.slug.as_deref() } + pub(crate) fn workflow_display_name(&self) -> String { + self.workflow_name() + .or_else(|| self.workflow_graph_name()) + .or_else(|| self.workflow_slug()) + .unwrap_or("-") + .to_string() + } + + pub(crate) fn workflow_matches(&self, pattern: &str) -> bool { + [ + self.workflow_name(), + self.workflow_graph_name(), + self.workflow_slug(), + ] + .into_iter() + .flatten() + .any(|value| value.contains(pattern)) + } + pub(crate) fn status(&self) -> RunStatus { self.run.lifecycle.status } @@ -132,7 +155,7 @@ pub(crate) fn filter_server_runs( start_time.is_empty() || start_time.as_str() < before }) }) - .filter(|run| workflow.is_none_or(|pattern| run.workflow_name().contains(pattern))) + .filter(|run| workflow.is_none_or(|pattern| run.workflow_matches(pattern))) .filter(|run| { labels.iter().all(|(key, value)| { run.labels() diff --git a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs index de9d07400..5c52cded3 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs @@ -144,19 +144,20 @@ fn ps_supports_global_flag_and_env_var() { .expect("ps output should be an array") .iter() .map(|run| { + let nullable_string = |field: &str| { + run.get(field) + .unwrap_or_else(|| panic!("{field} should be present")) + .as_str() + .map(str::to_string) + }; ( run["run_id"] .as_str() .expect("run_id should be present") .to_string(), - run["workflow_name"] - .as_str() - .expect("workflow_name should be present") - .to_string(), - run["workflow_slug"] - .as_str() - .expect("workflow_slug should be present") - .to_string(), + nullable_string("workflow_name"), + nullable_string("workflow_graph_name"), + nullable_string("workflow_slug"), run["goal"] .as_str() .expect("goal should be present") diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs index 4681b9bcc..7ebe3391d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -558,7 +558,8 @@ async fn mcp_create_and_search_manage_real_runs_with_cli_auth() { "run_id": "[RUN_ID]", "parent_id": null, "children_count": 0, - "workflow_name": "Simple", + "workflow_name": null, + "workflow_graph_name": "Simple", "workflow_slug": "simple", "status": "queued", "archived": false, @@ -1164,7 +1165,8 @@ async fn mcp_lifecycle_tools_manage_real_run() { "run_id": "[RUN_ID]", "parent_id": null, "children_count": 0, - "workflow_name": "Simple", + "workflow_name": null, + "workflow_graph_name": "Simple", "workflow_slug": "simple", "status": "failed", "archived": false, diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index 0f82641d7..2377e44f3 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -200,8 +200,9 @@ fn ps_all_json_lists_created_and_completed_runs() { let runs: Vec = serde_json::from_slice(&output.stdout).expect("ps JSON should parse"); assert_eq!(runs.len(), 2, "expected submitted + completed runs"); assert!( - runs.iter().all(|run| run["workflow_name"] == "Simple"), - "all runs should belong to the Simple workflow: {runs:#?}" + runs.iter() + .all(|run| run["workflow_name"].is_null() && run["workflow_graph_name"] == "Simple"), + "all runs should expose Simple as the graph name, not a workflow name: {runs:#?}" ); assert!( runs.iter() @@ -318,7 +319,8 @@ fn ps_filters_by_workflow_and_label() { "workflow+label filter should isolate one run" ); let run = &runs[0]; - assert_eq!(run["workflow_name"], "Simple"); + assert!(run["workflow_name"].is_null()); + assert_eq!(run["workflow_graph_name"], "Simple"); assert_eq!(run["status"]["kind"], "succeeded"); assert_eq!(run["status"]["reason"], "completed"); assert_eq!(run["labels"]["suite"], "alpha"); diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 26a994c3e..660d5534b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -148,7 +148,8 @@ pub(crate) fn remote_run_summary_json( "goal": goal, "workflow": { "slug": workflow_slug, - "name": workflow_name + "name": workflow_name, + "graph_name": null }, "repository": { "name": "repo", diff --git a/lib/crates/fabro-config/src/builders.rs b/lib/crates/fabro-config/src/builders.rs index d74482265..78a67c430 100644 --- a/lib/crates/fabro-config/src/builders.rs +++ b/lib/crates/fabro-config/src/builders.rs @@ -452,6 +452,14 @@ impl WorkflowSettingsBuilder { Ok(self.workflow_layer(layer)) } + pub fn workflow_toml_with_run_layer(self, source: &str, run: RunLayer) -> Result { + let mut layer = source + .parse::() + .map_err(|err| Error::parse("Failed to parse settings file", err))?; + layer.run = Some(run); + Ok(self.workflow_layer(layer)) + } + pub fn workflow_file(self, path: &Path) -> Result { Ok(self.workflow_layer(run::load_run_config(path)?)) } diff --git a/lib/crates/fabro-mcp-server/src/run_tools/common.rs b/lib/crates/fabro-mcp-server/src/run_tools/common.rs index 118b3efa6..f2acfa2fe 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/common.rs +++ b/lib/crates/fabro-mcp-server/src/run_tools/common.rs @@ -33,20 +33,21 @@ pub(super) type ToolResult = Result; #[derive(Debug, Serialize, JsonSchema)] pub(crate) struct RunSummaryResult { - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) children_count: u64, - pub(crate) workflow_name: String, - pub(crate) workflow_slug: Option, - pub(crate) status: String, - pub(crate) archived: bool, - pub(crate) created_at: String, - pub(crate) started_at: Option, - pub(crate) completed_at: Option, - pub(crate) labels: HashMap, - pub(crate) source_directory: Option, - pub(crate) repo_origin_url: Option, - pub(crate) goal: String, + pub(crate) run_id: String, + pub(crate) parent_id: Option, + pub(crate) children_count: u64, + pub(crate) workflow_name: Option, + pub(crate) workflow_graph_name: Option, + pub(crate) workflow_slug: Option, + pub(crate) status: String, + pub(crate) archived: bool, + pub(crate) created_at: String, + pub(crate) started_at: Option, + pub(crate) completed_at: Option, + pub(crate) labels: HashMap, + pub(crate) source_directory: Option, + pub(crate) repo_origin_url: Option, + pub(crate) goal: String, } pub(crate) fn success_result( @@ -91,29 +92,30 @@ pub(super) async fn retrieve_run(client: &Client, run_id: &RunId) -> ToolResult< pub(super) fn run_summary_result(run: &Run) -> RunSummaryResult { RunSummaryResult { - run_id: run.id.to_string(), - parent_id: run.parent_id.map(|parent_id| parent_id.to_string()), - children_count: run.children_count, - workflow_name: run.workflow.name.clone(), - workflow_slug: run.workflow.slug.clone(), - status: run_status_kind(run.lifecycle.status).to_string(), - archived: run.lifecycle.archived, - created_at: run.timestamps.created_at.to_rfc3339(), - started_at: run + run_id: run.id.to_string(), + parent_id: run.parent_id.map(|parent_id| parent_id.to_string()), + children_count: run.children_count, + workflow_name: run.workflow.name.clone(), + workflow_graph_name: run.workflow.graph_name.clone(), + workflow_slug: run.workflow.slug.clone(), + status: run_status_kind(run.lifecycle.status).to_string(), + archived: run.lifecycle.archived, + created_at: run.timestamps.created_at.to_rfc3339(), + started_at: run .timestamps .started_at .map(|timestamp| timestamp.to_rfc3339()), - completed_at: run + completed_at: run .timestamps .completed_at .map(|timestamp| timestamp.to_rfc3339()), - labels: run.labels.clone(), - source_directory: run.source_directory.clone(), - repo_origin_url: run + labels: run.labels.clone(), + source_directory: run.source_directory.clone(), + repo_origin_url: run .repository .as_ref() .and_then(|repository| repository.origin_url.clone()), - goal: run.goal.clone(), + goal: run.goal.clone(), } } @@ -161,8 +163,9 @@ mod tests { title: "test".to_string(), goal: "test".to_string(), workflow: WorkflowRef { - slug: Some("simple".to_string()), - name: "Simple".to_string(), + slug: Some("simple".to_string()), + name: Some("Simple".to_string()), + graph_name: Some("GraphName".to_string()), }, automation: None, repository: None, @@ -200,6 +203,8 @@ mod tests { assert_eq!(summary.parent_id, Some(parent_id.to_string())); assert_eq!(summary.children_count, 3); + assert_eq!(summary.workflow_name.as_deref(), Some("Simple")); + assert_eq!(summary.workflow_graph_name.as_deref(), Some("GraphName")); } fn run_id(raw: &str) -> RunId { diff --git a/lib/crates/fabro-mcp-server/src/run_tools/search.rs b/lib/crates/fabro-mcp-server/src/run_tools/search.rs index dade2f943..ce48f54db 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/search.rs +++ b/lib/crates/fabro-mcp-server/src/run_tools/search.rs @@ -84,21 +84,22 @@ pub(crate) struct SearchRunsResult { #[derive(Debug, Serialize, JsonSchema)] pub(crate) struct SearchRunSummaryResult { - pub(crate) run_id: String, - pub(crate) parent_id: Option, - pub(crate) children_count: u64, - pub(crate) workflow_name: String, - pub(crate) workflow_slug: Option, - pub(crate) status: String, - pub(crate) archived: bool, - pub(crate) created_at: String, - pub(crate) started_at: Option, - pub(crate) completed_at: Option, - pub(crate) labels: HashMap, - pub(crate) source_directory: Option, - pub(crate) repo_origin_url: Option, - pub(crate) goal_preview: String, - pub(crate) goal_truncated: bool, + pub(crate) run_id: String, + pub(crate) parent_id: Option, + pub(crate) children_count: u64, + pub(crate) workflow_name: Option, + pub(crate) workflow_graph_name: Option, + pub(crate) workflow_slug: Option, + pub(crate) status: String, + pub(crate) archived: bool, + pub(crate) created_at: String, + pub(crate) started_at: Option, + pub(crate) completed_at: Option, + pub(crate) labels: HashMap, + pub(crate) source_directory: Option, + pub(crate) repo_origin_url: Option, + pub(crate) goal_preview: String, + pub(crate) goal_truncated: bool, } pub(crate) async fn search_runs( @@ -145,6 +146,7 @@ fn search_run_summary_result(run: &Run) -> SearchRunSummaryResult { parent_id, children_count, workflow_name, + workflow_graph_name, workflow_slug, status, archived, @@ -163,6 +165,7 @@ fn search_run_summary_result(run: &Run) -> SearchRunSummaryResult { parent_id, children_count, workflow_name, + workflow_graph_name, workflow_slug, status, archived, @@ -206,7 +209,9 @@ fn filter_sort_and_page_runs( } if let Some(workflow) = raw.workflow.as_deref() { runs.retain(|run| { - run.workflow.name == workflow || run.workflow.slug.as_deref() == Some(workflow) + run.workflow.name.as_deref() == Some(workflow) + || run.workflow.graph_name.as_deref() == Some(workflow) + || run.workflow.slug.as_deref() == Some(workflow) }); } if let Some(labels) = raw.labels.as_ref() { @@ -363,6 +368,8 @@ mod tests { assert_eq!(summary.parent_id, Some(parent_id.to_string())); assert_eq!(summary.children_count, 4); + assert_eq!(summary.workflow_name.as_deref(), Some("Simple")); + assert_eq!(summary.workflow_graph_name.as_deref(), Some("GraphName")); assert!(summary.goal_truncated); assert!(summary.goal_preview.len() < run.goal.len()); assert!(!summary.goal_preview.contains("tail-marker")); @@ -427,8 +434,9 @@ mod tests { title: "test".to_string(), goal: "test".to_string(), workflow: WorkflowRef { - slug: Some("simple".to_string()), - name: "Simple".to_string(), + slug: Some("simple".to_string()), + name: Some("Simple".to_string()), + graph_name: Some("GraphName".to_string()), }, automation: None, repository: None, diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 7c781cf7f..6e1496332 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -101,7 +101,7 @@ pub(crate) async fn resolve_run( ¶ms.selector, |run| run.id.to_string(), |run| run.workflow.slug.clone(), - |run| Some(run.workflow.name.clone()), + |run| run.workflow.name.clone(), |run| run.timestamps.created_at, |run| run.timestamps.created_at.to_rfc3339(), |run| { @@ -1061,8 +1061,9 @@ mod runs { title: fabro_types::infer_run_title(goal), goal: goal.into(), workflow: WorkflowRef { - slug: Some(workflow_slug.into()), - name: workflow_name.into(), + slug: Some(workflow_slug.into()), + name: Some(workflow_name.into()), + graph_name: None, }, automation: None, repository: Some(RepositoryRef::from_origin_and_source( diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 133482cca..88b6efbd9 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -93,7 +93,6 @@ pub(crate) fn prepare_manifest( let args_overrides = manifest_args_overrides(manifest.args.as_ref()).context("failed to parse manifest args")?; - let workflow_run_layer = root_workflow_run_layer(&workflow_input)?; let mut workflow_settings_builder = WorkflowSettingsBuilder::new().server_run_defaults(manifest_run_defaults.clone()); if let Some(run) = args_overrides.run { @@ -102,9 +101,10 @@ pub(crate) fn prepare_manifest( if let Some(cli) = args_overrides.cli { workflow_settings_builder = workflow_settings_builder.cli_overrides(cli); } - if !workflow_run_layer.eq(&RunLayer::default()) { - workflow_settings_builder = - workflow_settings_builder.workflow_run_layer(workflow_run_layer); + if let Some(config) = workflow_input.config.as_ref() { + let workflow_run_layer = workflow_run_layer_with_resolved_dockerfile(&workflow_input)?; + workflow_settings_builder = workflow_settings_builder + .workflow_toml_with_run_layer(&config.source, workflow_run_layer)?; } for config in manifest .configs @@ -316,11 +316,11 @@ fn workflow_files_from_manifest( Ok(bundled) } -fn root_workflow_run_layer(workflow: &BundledWorkflow) -> Result { - let Some(config) = workflow.config.as_ref() else { - return Ok(RunLayer::default()); - }; - +fn workflow_run_layer_with_resolved_dockerfile(workflow: &BundledWorkflow) -> Result { + let config = workflow + .config + .as_ref() + .expect("workflow config should exist before resolving its run layer"); // Parse via `SettingsLayer` so unknown nested keys (like a stale // `[server.integrations.github.permissions]` after the move to // `[run.integrations.github.permissions]`) trip @@ -1955,6 +1955,126 @@ app_id = "fixture-app-id" assert!(settings_json.pointer("/server").is_none()); } + #[test] + fn prepare_manifest_preserves_bundled_workflow_metadata() { + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().config = + Some(types::ManifestWorkflowConfig { + path: "workflow.toml".to_string(), + source: r#" +_version = 1 + +[workflow] +name = "Ship feature" +description = "Move the feature through review" + +[workflow.metadata] +team = "platform" +priority = "high" + +[run.sandbox] +provider = "local" +"# + .to_string(), + }); + + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + + assert_eq!( + prepared.settings.workflow.name.as_deref(), + Some("Ship feature") + ); + assert_eq!( + prepared.settings.workflow.description.as_deref(), + Some("Move the feature through review") + ); + assert_eq!( + prepared + .settings + .workflow + .metadata + .get("team") + .map(String::as_str), + Some("platform") + ); + assert_eq!( + prepared + .settings + .workflow + .metadata + .get("priority") + .map(String::as_str), + Some("high") + ); + } + + #[test] + fn prepare_manifest_keeps_missing_metadata_names_absent() { + let mut manifest = minimal_manifest(); + manifest.target.identifier = "release-flow".to_string(); + manifest.workflows.get_mut("workflow.fabro").unwrap().source = r" +digraph GraphName { + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit +} +" + .to_string(); + manifest.configs.push(types::ManifestConfig { + path: Some(".fabro/project.toml".to_string()), + source: Some( + r#"_version = 1 + +[project.metadata] +team = "platform" +"# + .to_string(), + ), + type_: types::ManifestConfigType::Project, + }); + + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + + assert_eq!(prepared.settings.workflow.name, None); + assert_eq!(prepared.settings.project.name, None); + } + + #[test] + fn prepare_manifest_preserves_explicit_project_name() { + let mut manifest = minimal_manifest(); + manifest.configs.push(types::ManifestConfig { + path: Some(".fabro/project.toml".to_string()), + source: Some( + r#"_version = 1 + +[project] +name = "Control Plane" +"# + .to_string(), + ), + type_: types::ManifestConfigType::Project, + }); + + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + + assert_eq!( + prepared.settings.project.name.as_deref(), + Some("Control Plane") + ); + } + #[tokio::test] async fn invalid_preflight_returns_diagnostics_without_runtime_checks() { let state = crate::test_support::test_app_state(); @@ -2074,6 +2194,74 @@ provider = "local" ); } + #[test] + fn prepare_manifest_does_not_backfill_missing_project_and_workflow_names() { + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().config = + Some(types::ManifestWorkflowConfig { + path: "workflow.toml".to_string(), + source: "_version = 1\n".to_string(), + }); + manifest.configs.push(types::ManifestConfig { + path: Some("/tmp/project/.fabro/project.toml".to_string()), + source: Some("_version = 1\n".to_string()), + type_: types::ManifestConfigType::Project, + }); + + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + + assert_eq!(prepared.settings.project.name, None); + assert_eq!(prepared.settings.workflow.name, None); + } + + #[test] + fn prepare_manifest_preserves_explicit_project_and_workflow_names() { + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().config = + Some(types::ManifestWorkflowConfig { + path: "workflow.toml".to_string(), + source: r#" +_version = 1 + +[workflow] +name = "Workflow Config Name" +"# + .to_string(), + }); + manifest.configs.push(types::ManifestConfig { + path: Some("/tmp/project/.fabro/project.toml".to_string()), + source: Some( + r#" +_version = 1 + +[project] +name = "Project Config Name" +"# + .to_string(), + ), + type_: types::ManifestConfigType::Project, + }); + + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + + assert_eq!( + prepared.settings.project.name.as_deref(), + Some("Project Config Name") + ); + assert_eq!( + prepared.settings.workflow.name.as_deref(), + Some("Workflow Config Name") + ); + } + #[tokio::test] async fn preflight_daytona_without_github_credentials_returns_report() { let state = crate::test_support::test_app_state(); @@ -2310,15 +2498,16 @@ digraph Demo { ); } - mod root_workflow_run_layer_tests { - //! `root_workflow_run_layer` parses bundled workflow.toml through - //! the strict `SettingsLayer` schema, so unknown fields anywhere in - //! the document trip `deny_unknown_fields`. + mod workflow_run_layer_with_resolved_dockerfile_tests { + //! `workflow_run_layer_with_resolved_dockerfile` parses bundled + //! workflow.toml through the strict `SettingsLayer` schema, so + //! unknown fields anywhere in the document trip + //! `deny_unknown_fields`. use fabro_types::ManifestPath; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig}; - use super::super::root_workflow_run_layer; + use super::super::workflow_run_layer_with_resolved_dockerfile; fn workflow_with_config(source: &str) -> BundledWorkflow { BundledWorkflow { @@ -2343,7 +2532,8 @@ issues = "read" "#, ); - let run = root_workflow_run_layer(&workflow).expect("workflow.toml should parse"); + let run = workflow_run_layer_with_resolved_dockerfile(&workflow) + .expect("workflow.toml should parse"); let github = run .integrations .as_ref() @@ -2367,7 +2557,7 @@ issues = "read" "#, ); - let err = root_workflow_run_layer(&workflow) + let err = workflow_run_layer_with_resolved_dockerfile(&workflow) .expect_err("stale [server.integrations.github.permissions] should be rejected"); let message = format!("{err:#}"); assert!( @@ -2389,8 +2579,8 @@ contents = "read" "#, ); - let run = - root_workflow_run_layer(&workflow).expect("workflow + run blocks should parse"); + let run = workflow_run_layer_with_resolved_dockerfile(&workflow) + .expect("workflow + run blocks should parse"); assert!(run.integrations.is_some()); } } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index ec21af2c2..a52299126 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1364,7 +1364,7 @@ fn build_disk_usage_response( if verbose { run_rows.push(DiskUsageRunRow { run_id: Some(run.run_id().to_string()), - workflow_name: Some(run.workflow_name()), + workflow_name: Some(run.workflow_display_name()), status: Some(run.status().to_string()), start_time: Some(run.start_time()), size_bytes: Some(to_i64(size)), @@ -1459,7 +1459,7 @@ fn build_prune_plan( .map(|run| PruneRunEntry { run_id: Some(run.run_id().to_string()), dir_name: Some(run.dir_name.clone()), - workflow_name: Some(run.workflow_name()), + workflow_name: Some(run.workflow_display_name()), size_bytes: Some(to_i64(dir_size(&run.path))), }) .collect::>(); diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 79abef26a..7fc1a3fd9 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -431,7 +431,7 @@ async fn resolve_run( &query.selector, |run| run.id.to_string(), |run| run.workflow.slug.clone(), - |run| Some(run.workflow.name.clone()), + |run| run.workflow.name.clone(), |run| run.id.created_at(), |run| run.id.created_at().to_rfc3339(), |run| { diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 48d0e9542..9f3925b81 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -3175,6 +3175,28 @@ async fn create_run_for_target(app: &Router, target_path: &str, dot_source: &str body["id"].as_str().unwrap().to_string() } +async fn create_run_for_target_with_workflow_name( + app: &Router, + target_path: &str, + dot_source: &str, + workflow_name: &str, +) -> String { + let mut manifest = manifest_json(target_path, dot_source); + manifest["workflows"][target_path]["config"] = serde_json::json!({ + "path": "workflow.toml", + "source": format!("_version = 1\n\n[workflow]\nname = {workflow_name:?}\n"), + }); + let req = Request::builder() + .method("POST") + .uri(api("/runs")) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_string(&manifest).unwrap())) + .unwrap(); + let response = app.clone().oneshot(req).await.unwrap(); + let body = body_json(response.into_body()).await; + body["id"].as_str().unwrap().to_string() +} + fn named_workflow_dot(name: &str, goal: &str) -> String { format!( r#"digraph {name} {{ @@ -5254,16 +5276,18 @@ async fn resolve_run_prefers_most_recent_exact_workflow_slug_match() { #[tokio::test] async fn resolve_run_prefers_most_recent_collapsed_workflow_name_match() { let app = test_app_with(); - let older_id = create_run_for_target( + let older_id = create_run_for_target_with_workflow_name( &app, "nightly-alpha.fabro", - &named_workflow_dot("Nightly_Build", "older"), + &named_workflow_dot("OlderNightlyGraph", "older"), + "Nightly_Build", ) .await; - let newer_id = create_run_for_target( + let newer_id = create_run_for_target_with_workflow_name( &app, "nightly-beta.fabro", - &named_workflow_dot("Nightly_Build", "newer"), + &named_workflow_dot("NewerNightlyGraph", "newer"), + "Nightly_Build", ) .await; @@ -7330,6 +7354,66 @@ async fn create_run_persists_run_spec() { assert_eq!(run_state.spec.graph.name, "Test"); } +#[tokio::test] +async fn create_run_keeps_missing_project_and_workflow_names_absent() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + + let manifest = serde_json::json!({ + "version": 1, + "cwd": "/tmp/project", + "target": { + "identifier": "workflow.fabro", + "path": "workflow.fabro", + }, + "configs": [ + { + "path": "/tmp/project/.fabro/project.toml", + "source": "_version = 1\n", + "type": "project", + } + ], + "workflows": { + "workflow.fabro": { + "source": "digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + "config": { + "path": "workflow.toml", + "source": "_version = 1\n", + }, + "files": {}, + } + }, + }); + + let response = app + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri(api("/runs")) + .header("content-type", "application/json") + .body(Body::from(manifest.to_string())) + .unwrap(), + ) + .await + .unwrap(); + let body = body_json(response.into_body()).await; + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); + + let run_state = state + .store + .open_run_reader(&run_id) + .await + .unwrap() + .state() + .await + .unwrap(); + + assert_eq!(run_state.spec.settings.project.name.as_deref(), None); + assert_eq!(run_state.spec.settings.workflow.name.as_deref(), None); + assert_eq!(run_state.spec.graph_name(), Some("Demo")); +} + #[tokio::test] async fn stage_artifact_upload_rejects_invalid_filename() { let state = test_app_state(); @@ -10713,7 +10797,8 @@ async fn boards_runs_returns_run_list_items_with_board_columns() { assert!(item["title"].is_string()); assert!(item["repository"].is_object()); assert!(item["workflow"]["slug"].is_string() || item["workflow"]["slug"].is_null()); - assert!(item["workflow"]["name"].is_string()); + assert!(item["workflow"]["name"].is_string() || item["workflow"]["name"].is_null()); + assert!(item["workflow"]["graph_name"].is_string()); assert!(item["labels"].is_object()); assert!(run_json_status(item).is_object()); assert!(item["timestamps"]["created_at"].is_string()); diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 6b4fd2d22..c09c0c3bf 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -626,11 +626,6 @@ fn stage_at_completed_visit<'a>( } pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> Run { - let workflow_name = if state.spec.graph.name.is_empty() { - "unnamed".to_string() - } else { - state.spec.graph.name.clone() - }; let goal = state.spec.graph.goal().to_string(); let diff_summary = state .conclusion @@ -683,8 +678,9 @@ pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> Run { title: state.title().into_owned(), goal, workflow: WorkflowRef { - slug: state.spec.workflow_slug.clone(), - name: workflow_name, + slug: state.spec.workflow_slug.clone(), + name: state.spec.workflow_name().map(ToOwned::to_owned), + graph_name: state.spec.graph_name().map(ToOwned::to_owned), }, automation: None, repository: Some(RepositoryRef::from_origin_and_source( @@ -2038,6 +2034,42 @@ mod tests { ); } + #[test] + fn summary_preserves_absent_workflow_name_and_reports_graph_name() { + let mut state = initialized_projection(); + state.spec = fabro_types::RunSpec { + run_id: fixtures::RUN_1, + settings: WorkflowSettings::default(), + graph: fabro_types::Graph::new("GraphName"), + graph_source: None, + workflow_slug: Some("release-flow".to_string()), + source_directory: Some("/tmp/repo".to_string()), + git: None, + labels: HashMap::new(), + provenance: None, + manifest_blob: None, + definition_blob: None, + fork_source_ref: None, + }; + + let summary = build_summary(&state, &fixtures::RUN_1); + + assert_eq!(summary.workflow.name, None); + assert_eq!(summary.workflow.graph_name.as_deref(), Some("GraphName")); + assert_eq!(summary.workflow.slug.as_deref(), Some("release-flow")); + } + + #[test] + fn summary_uses_explicit_workflow_name() { + let mut state = initialized_projection(); + state.spec.settings.workflow.name = Some("Ship workflow".to_string()); + + let summary = build_summary(&state, &fixtures::RUN_1); + + assert_eq!(summary.workflow.name.as_deref(), Some("Ship workflow")); + assert_eq!(summary.workflow.graph_name.as_deref(), Some("test")); + } + #[test] fn run_created_title_populates_projection_and_summary() { let event = test_raw_event( diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index ae793ee1d..ed01a9e73 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -626,7 +626,8 @@ mod tests { assert_eq!(summary.len(), 2); assert_eq!(summary[0].id, test_run_id("run-2")); assert_eq!(summary[1].id, test_run_id("run-1")); - assert_eq!(summary[1].workflow.name, "night-sky"); + assert_eq!(summary[1].workflow.name, None); + assert_eq!(summary[1].workflow.graph_name.as_deref(), Some("night-sky")); assert_eq!(summary[1].goal, "map the constellations"); assert_eq!(summary[1].lifecycle.status, RunStatus::Succeeded { reason: SuccessReason::Completed, diff --git a/lib/crates/fabro-types/src/run.rs b/lib/crates/fabro-types/src/run.rs index 774c7b73a..269db43ee 100644 --- a/lib/crates/fabro-types/src/run.rs +++ b/lib/crates/fabro-types/src/run.rs @@ -123,6 +123,25 @@ impl RunSpec { self.workflow_slug.as_deref() } + #[must_use] + pub fn workflow_name(&self) -> Option<&str> { + self.settings.workflow.name.as_deref() + } + + #[must_use] + pub fn graph_name(&self) -> Option<&str> { + if self.graph.name.is_empty() { + None + } else { + Some(self.graph.name.as_str()) + } + } + + #[must_use] + pub fn project_name(&self) -> Option<&str> { + self.settings.project.name.as_deref() + } + #[must_use] pub fn source_directory(&self) -> Option<&str> { self.source_directory.as_deref() diff --git a/lib/crates/fabro-types/src/run_summary.rs b/lib/crates/fabro-types/src/run_summary.rs index 3a8a4c0ba..f6e336ebc 100644 --- a/lib/crates/fabro-types/src/run_summary.rs +++ b/lib/crates/fabro-types/src/run_summary.rs @@ -49,8 +49,11 @@ pub struct Run { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct WorkflowRef { #[serde(default)] - pub slug: Option, - pub name: String, + pub slug: Option, + #[serde(default)] + pub name: Option, + #[serde(default)] + pub graph_name: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/tests/run_spec_methods.rs b/lib/crates/fabro-types/tests/run_spec_methods.rs index a203a11c8..f5e8cf25f 100644 --- a/lib/crates/fabro-types/tests/run_spec_methods.rs +++ b/lib/crates/fabro-types/tests/run_spec_methods.rs @@ -2,21 +2,34 @@ use std::collections::HashMap; use fabro_types::graph::Graph; use fabro_types::run::{DirtyStatus, GitContext, PreRunPushOutcome, RunSpec}; +use fabro_types::settings::{ProjectNamespace, WorkflowNamespace}; use fabro_types::{WorkflowSettings, fixtures}; fn sample_run_spec() -> RunSpec { + let settings = WorkflowSettings { + project: ProjectNamespace { + name: Some("Control Plane".to_string()), + ..ProjectNamespace::default() + }, + workflow: WorkflowNamespace { + name: Some("Ship workflow".to_string()), + ..WorkflowNamespace::default() + }, + ..WorkflowSettings::default() + }; + RunSpec { - run_id: fixtures::RUN_1, - settings: WorkflowSettings::default(), - graph: Graph::new("ship"), - graph_source: None, - workflow_slug: Some("demo".to_string()), + run_id: fixtures::RUN_1, + settings, + graph: Graph::new("ship"), + graph_source: None, + workflow_slug: Some("demo".to_string()), source_directory: Some("/Users/client/project".to_string()), - labels: HashMap::from([("team".to_string(), "platform".to_string())]), - provenance: None, - manifest_blob: None, - definition_blob: None, - git: Some(GitContext { + labels: HashMap::from([("team".to_string(), "platform".to_string())]), + provenance: None, + manifest_blob: None, + definition_blob: None, + git: Some(GitContext { origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), sha: Some("abc123".to_string()), @@ -26,7 +39,7 @@ fn sample_run_spec() -> RunSpec { repo_origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), }, }), - fork_source_ref: None, + fork_source_ref: None, } } @@ -36,7 +49,9 @@ fn run_spec_getters_return_declared_fields() { assert_eq!(run_spec.id(), fixtures::RUN_1); assert_eq!(run_spec.graph().name, "ship"); - assert_eq!(run_spec.settings(), &WorkflowSettings::default()); + assert_eq!(run_spec.workflow_name(), Some("Ship workflow")); + assert_eq!(run_spec.graph_name(), Some("ship")); + assert_eq!(run_spec.project_name(), Some("Control Plane")); assert_eq!(run_spec.workflow_slug(), Some("demo")); assert_eq!(run_spec.source_directory(), Some("/Users/client/project")); assert_eq!( @@ -53,3 +68,16 @@ fn run_spec_getters_return_declared_fields() { ); assert_eq!(run_spec.base_branch(), Some("main")); } + +#[test] +fn run_spec_name_getters_do_not_synthesize_from_graph_or_slug() { + let mut run_spec = sample_run_spec(); + run_spec.settings.workflow.name = None; + run_spec.settings.project.name = None; + run_spec.workflow_slug = Some("release-flow".to_string()); + run_spec.graph = Graph::new("GraphName"); + + assert_eq!(run_spec.workflow_name(), None); + assert_eq!(run_spec.project_name(), None); + assert_eq!(run_spec.graph_name(), Some("GraphName")); +} diff --git a/lib/crates/fabro-workflow/src/run_lookup.rs b/lib/crates/fabro-workflow/src/run_lookup.rs index eb5736cbb..40683e230 100644 --- a/lib/crates/fabro-workflow/src/run_lookup.rs +++ b/lib/crates/fabro-workflow/src/run_lookup.rs @@ -62,11 +62,16 @@ impl RunInfo { .expect("RunInfo must have a run id") } - pub fn workflow_name(&self) -> String { - self.summary.as_ref().map_or_else( - || "[no run spec]".to_string(), - |summary| summary.workflow.name.clone(), - ) + pub fn workflow_name(&self) -> Option<&str> { + self.summary + .as_ref() + .and_then(|summary| summary.workflow.name.as_deref()) + } + + pub fn workflow_graph_name(&self) -> Option<&str> { + self.summary + .as_ref() + .and_then(|summary| summary.workflow.graph_name.as_deref()) } pub fn workflow_slug(&self) -> Option<&str> { @@ -75,6 +80,30 @@ impl RunInfo { .and_then(|summary| summary.workflow.slug.as_deref()) } + pub fn workflow_display_name(&self) -> String { + self.summary.as_ref().map_or_else( + || "[no run spec]".to_string(), + |_| { + self.workflow_name() + .or_else(|| self.workflow_graph_name()) + .or_else(|| self.workflow_slug()) + .unwrap_or("-") + .to_string() + }, + ) + } + + fn workflow_matches(&self, pattern: &str) -> bool { + [ + self.workflow_name(), + self.workflow_graph_name(), + self.workflow_slug(), + ] + .into_iter() + .flatten() + .any(|value| value.contains(pattern)) + } + pub fn status(&self) -> RunStatus { self.summary .as_ref() @@ -300,7 +329,7 @@ pub fn filter_runs( } } if let Some(pattern) = workflow { - if !run.workflow_name().contains(pattern) { + if !run.workflow_matches(pattern) { return false; } } @@ -352,7 +381,7 @@ fn resolve_run_from_infos(runs: &[RunInfo], identifier: &str) -> Result "{} created_at={} workflow={} origin={}", run.run_id(), run.run_id().created_at().to_rfc3339(), - run.workflow_name(), + run.workflow_display_name(), run.repo_origin_url().unwrap_or("-") ) }) @@ -376,9 +405,14 @@ fn resolve_run_from_infos(runs: &[RunInfo], identifier: &str) -> Result return true; } } - let name_lower = run.workflow_name().to_lowercase(); - name_lower.contains(&id_lower) - || collapse_separators(&name_lower).contains(&id_collapsed) + [run.workflow_name(), run.workflow_graph_name()] + .into_iter() + .flatten() + .any(|name| { + let name_lower = name.to_lowercase(); + name_lower.contains(&id_lower) + || collapse_separators(&name_lower).contains(&id_collapsed) + }) }) .max_by_key(|run| run.run_id().created_at()); diff --git a/lib/packages/fabro-api-client/src/models/workflow-ref.ts b/lib/packages/fabro-api-client/src/models/workflow-ref.ts index 5eccc32c3..0f3339801 100644 --- a/lib/packages/fabro-api-client/src/models/workflow-ref.ts +++ b/lib/packages/fabro-api-client/src/models/workflow-ref.ts @@ -16,6 +16,7 @@ export interface WorkflowRef { 'slug': string | null; - 'name': string; + 'name': string | null; + 'graph_name': string | null; }