diff --git a/AGENTS.md b/AGENTS.md index 4b2de3cf5..4ca4fbeb5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,7 +114,7 @@ Fabro is an AI-powered workflow orchestration platform. Workflows are defined as ### Rust crates (`lib/apps/`, `lib/components/`, and `lib/foundation/`) - **fabro-cli** — CLI entry point. Commands: `run`, `exec`, `serve`, `validate`, `parse`, `cp`, `model`, `doctor`, `install`, `ps`, `system prune` -- **fabro-workflow** — Fabro's workflow definitions: parses Graphviz graphs and layers settings for the read side, creates and archives runs, and holds the run tools and the pull request pipeline. Execution is Petri's, through `fabro-petri` +- **fabro-workflow** — Fabro's platform half of a run: creates a run around Petri's admission (the run's display graph is read off the admitted graph), archives, forks and retries runs, and holds the run tools and the pull request pipeline. Compilation and execution are Petri's, through `fabro-petri` - **fabro-graphviz** — Graphviz DOT parser, the typed graph model, and SVG rendering - **fabro-sandbox** — Local, Docker, and Daytona sandbox providers. `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). Docker is the default runtime provider and creates clone-based `/workspace` containers through the operator's Docker daemon; Daytona uses the same GitHub-only clone-source contract. Docker daemon access is host-root-equivalent and assumes trusted callers/payloads. - **fabro-petri** — Fabro's adapters over Petri, the workflow engine: the one crate that imports the Petri packages (pinned by rev in the workspace `Cargo.toml`), holding the run store over SQLite and the platform adapters @@ -225,7 +225,7 @@ Fabro is an AI-powered workflow orchestration platform. Workflows are defined as ### Rust crates (`lib/apps/`, `lib/components/`, and `lib/foundation/`) - **fabro-cli** — CLI entry point. Commands: `run`, `exec`, `serve`, `validate`, `parse`, `cp`, `model`, `doctor`, `install`, `ps`, `system prune` -- **fabro-workflow** — Fabro's workflow definitions: parses Graphviz graphs and layers settings for the read side, creates and archives runs, and holds the run tools and the pull request pipeline. Execution is Petri's, through `fabro-petri` +- **fabro-workflow** — Fabro's platform half of a run: creates a run around Petri's admission (the run's display graph is read off the admitted graph), archives, forks and retries runs, and holds the run tools and the pull request pipeline. Compilation and execution are Petri's, through `fabro-petri` - **fabro-graphviz** — Graphviz DOT parser, the typed graph model, and SVG rendering - **fabro-sandbox** — Local, Docker, and Daytona sandbox providers. `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). Docker is the default runtime provider and creates clone-based `/workspace` containers through the operator's Docker daemon; Daytona uses the same GitHub-only clone-source contract. Docker daemon access is host-root-equivalent and assumes trusted callers/payloads. - **fabro-petri** — Fabro's adapters over Petri, the workflow engine: the one crate that imports the Petri packages (pinned by rev in the workspace `Cargo.toml`), holding the run store over SQLite and the platform adapters diff --git a/Cargo.lock b/Cargo.lock index 0c2157a9a..187d2e5d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2972,7 +2972,6 @@ dependencies = [ "fabro-redact", "fabro-sandbox", "fabro-store", - "fabro-template", "fabro-test", "fabro-tool", "fabro-types", @@ -2984,7 +2983,6 @@ dependencies = [ "hex", "httpmock", "lithos-llm", - "miette", "pebble-coding-agent", "sandbox-driver", "scopeguard", diff --git a/docs/public/agents/prompts.mdx b/docs/public/agents/prompts.mdx index eed36c6d4..81bbfe9d0 100644 --- a/docs/public/agents/prompts.mdx +++ b/docs/public/agents/prompts.mdx @@ -64,7 +64,7 @@ Before execution, `{{ goal }}` becomes `Add a /health endpoint to the API server | `{{ goal }}` | The graph-level `goal` attribute | | `{{ inputs.name }}` | A value from `[run.inputs]` | -Undefined prompt variables render as empty text and produce a `template_undefined_variable` diagnostic. `fabro validate` reports that diagnostic as a warning; run-style commands promote it to an error before proceeding. Environment variables are not available in prompt templates. +A prompt variable that nothing binds is a diagnostic from the workflow compile. `fabro validate` reports it as a warning (`attractor.unbound_input`) and leaves the text unrendered; run-style commands and `fabro preflight` refuse the workflow with `unsupported.template.unbound_input` before a run is created. Environment variables are not available in prompt templates. Prompt and goal templates can use static MiniJinja includes to share partials: diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index f01dd5595..7c5cfc2b4 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -1424,7 +1424,16 @@ paths: operationId: runPreflight tags: [Runs] summary: Validate Workflow Manifest - description: Validates runtime readiness for a workflow manifest without creating a run. + description: | + Validates runtime readiness for a workflow manifest without creating + a run. The workflow is checked as a run would be admitted: Petri + compiles the bundle with the server's settings, run variables and + model catalog, and every diagnostic carries Petri's code as its + `rule` (`attractor.no_start`, `attractor.model.unknown`, + `unsupported.template.unbound_input`), with Fabro's own + `fabro.model.no_ready_provider` when a model node has no provider + ready to run it. The checks then probe the sandbox, repository + access and GitHub credentials. requestBody: required: true content: @@ -1453,7 +1462,16 @@ paths: operationId: validateRunManifest tags: [Runs] summary: Validate Workflow Manifest - description: Validates workflow structure and diagnostics without runtime readiness checks. + description: | + Validates a workflow manifest without runtime readiness checks. The + workflow is checked as a run would be admitted: Petri compiles the + bundle with the server's settings, run variables and model catalog, + and every diagnostic carries Petri's code as its `rule` + (`attractor.no_start`, `attractor.model.unknown`, + `unsupported.template.unbound_input`), with Fabro's own + `fabro.model.no_ready_provider` when a model node has no provider + ready to run it. `workflow` describes the admitted graph, or the DOT + as written when Petri refused the workflow. requestBody: required: true content: @@ -1482,7 +1500,10 @@ paths: operationId: renderWorkflowGraph tags: [Runs] summary: Render Workflow Graph - description: Validates and renders a workflow manifest as SVG without creating a run. + description: | + Validates and renders a workflow manifest as SVG without creating a + run. The manifest is checked as `POST /validate` checks it; a + workflow Petri refuses is not rendered. requestBody: required: true content: @@ -9958,6 +9979,11 @@ components: $ref: "#/components/schemas/WorkflowDiagnostic" WorkflowDiagnostic: + description: | + One diagnostic about a workflow. `rule` is the stable code of the + check that raised it: Petri's codes (`attractor.*`, `unsupported.*`, + `deprecated.*`, `info.*`, `fabro.hooks.*`) for the compile, and + `fabro.model.no_ready_provider` for Fabro's own provider check. type: object required: - rule @@ -9966,6 +9992,7 @@ components: properties: rule: type: string + description: The stable code of the check that raised the diagnostic. severity: type: string enum: @@ -12644,10 +12671,13 @@ components: settings: $ref: "#/components/schemas/WorkflowSettings" graph: - type: object - additionalProperties: true + $ref: "#/components/schemas/RunGraph" + description: | + The display graph: the workflow Petri admitted, reduced to what + the read side names. The DOT it was written in is `graph_source`. graph_source: type: ["string", "null"] + description: The entrypoint workflow's DOT as written. workflow_slug: type: ["string", "null"] workflow_version_id: @@ -12690,6 +12720,54 @@ components: What Petri admitted for the run at create time: the graphs it executes and resumes from. + RunGraph: + description: | + The display graph of a run: the admitted workflow's name, goal, + stages and edges, read off the graph Petri admitted at create time. + Lowering artifacts (the goal check, a synthetic fan-in) are left out; + the stages are the nodes the workflow declares, imports and + `[run.prepare]` steps included. + type: object + required: [name] + properties: + name: + type: string + description: The workflow's name, the DOT `digraph` name. + goal: + type: string + description: The run's goal as the run displays it; empty when the workflow has none. + nodes: + type: object + description: The stages by node id. + additionalProperties: + $ref: "#/components/schemas/RunGraphNode" + edges: + type: array + description: The routing edges as written, one per arm. + items: + $ref: "#/components/schemas/RunGraphEdge" + + RunGraphNode: + description: One stage of a run's display graph. + type: object + required: [label, kind] + properties: + label: + type: string + description: The node's display label, its `label` attribute or its id. + kind: + $ref: "#/components/schemas/StageHandler" + + RunGraphEdge: + description: One routing edge of a run's display graph. + type: object + required: [from, to] + properties: + from: + type: string + to: + type: string + PetriAdmission: description: | What Petri admitted for a run at create time: the lowered root graph diff --git a/docs/public/workflows/stylesheets.mdx b/docs/public/workflows/stylesheets.mdx index 9a335dc3b..17f4fcf8e 100644 --- a/docs/public/workflows/stylesheets.mdx +++ b/docs/public/workflows/stylesheets.mdx @@ -95,7 +95,7 @@ Fabro uses this order: A `model_stylesheet` on an imported graph is ignored and produces an `imported_model_stylesheet_ignored` warning. Put the stylesheet on the root graph. A root stylesheet can target imported nodes by their generated IDs, classes, or shapes. -If an input or variable is unavailable, `fabro validate` reports `template_undefined_variable`. It skips stylesheet syntax and model checks for that validation pass. Run-style commands treat the same diagnostic as an error before they create or start a run. +If an input or variable is unavailable, `fabro validate` reports `attractor.unbound_input` as a warning and leaves the stylesheet unrendered, so its syntax and model checks wait for the values. Run-style commands refuse the workflow with `unsupported.template.unbound_input` before they create or start a run. ## Selectors diff --git a/docs/public/workflows/variables.mdx b/docs/public/workflows/variables.mdx index 6624a8c92..fe41595de 100644 --- a/docs/public/workflows/variables.mdx +++ b/docs/public/workflows/variables.mdx @@ -163,7 +163,7 @@ digraph Example { That prompt becomes `Create a plan for: Implement the login feature`. -A graph goal cannot contain `{{ goal }}` because that would reference itself. `fabro validate` reports `goal_self_reference` as an error; put the reusable text in an input or server-managed variable instead. +A graph goal cannot contain `{{ goal }}` because that would reference itself: the goal is not bound while it renders, so `fabro validate` reports `attractor.unbound_input` and run-style commands refuse the workflow with `unsupported.template.unbound_input`. Put the reusable text in an input or server-managed variable instead. ## Expansion timing @@ -185,9 +185,7 @@ Fabro renders the graph `goal` first and stores the rendered value back onto the ## Undefined variables -Fabro renders undefined workflow variables as empty text and records a `template_undefined_variable` diagnostic. `fabro validate` reports that diagnostic as a warning so you can validate workflow structure before all inputs are known. Offline validation does not read a server's variable store, so `{{ vars.* }}` references also warn there. Run-style commands such as `fabro run`, `fabro create`, and preflight use the server snapshot and promote any still-undefined reference to an error before proceeding. - -In a `script`, an undefined value records the same diagnostic but leaves the token in place rather than emptying it, so validation output shows what is unbound. +A workflow variable that nothing binds is a diagnostic from the workflow compile. `fabro validate` reports it as a warning (`attractor.unbound_input`) and leaves the text unrendered, so you can validate workflow structure before all inputs are known. Offline validation does not read a server's variable store, so `{{ vars.* }}` references also warn there. Run-style commands such as `fabro run`, `fabro create`, and preflight use the server snapshot and refuse any still-unbound reference with `unsupported.template.unbound_input` before proceeding. ## Template includes diff --git a/lib/apps/fabro-cli/src/commands/run/attach.rs b/lib/apps/fabro-cli/src/commands/run/attach.rs index dc55ce336..379deb3b2 100644 --- a/lib/apps/fabro-cli/src/commands/run/attach.rs +++ b/lib/apps/fabro-cli/src/commands/run/attach.rs @@ -784,7 +784,7 @@ mod tests { let spec = fabro_types::RunSpec { run_id, settings: fabro_types::WorkflowSettings::default(), - graph: fabro_types::Graph::new("test"), + graph: fabro_types::RunGraph::new("test"), graph_source: None, workflow_slug: None, workflow_version_id: None, diff --git a/lib/apps/fabro-cli/src/main.rs b/lib/apps/fabro-cli/src/main.rs index 775dda5ed..d4c13101e 100644 --- a/lib/apps/fabro-cli/src/main.rs +++ b/lib/apps/fabro-cli/src/main.rs @@ -153,13 +153,6 @@ impl CliDiagnostic { show_auth_hint, } } - - fn delegated_diagnostic(&self) -> Option<&dyn miette::Diagnostic> { - self.err.chain().find_map(|err| { - err.downcast_ref::() - .map(|err| err as &dyn miette::Diagnostic) - }) - } } impl Display for CliDiagnostic { @@ -181,33 +174,9 @@ impl std::error::Error for CliDiagnostic { } impl miette::Diagnostic for CliDiagnostic { - fn code<'a>(&'a self) -> Option> { - self.delegated_diagnostic() - .and_then(miette::Diagnostic::code) - } - fn help<'a>(&'a self) -> Option> { - if self.show_auth_hint && exit::exit_class_for(&self.err) == Some(ExitClass::AuthRequired) { - Some(Box::new("Run `fabro auth login` to authenticate.")) - } else { - self.delegated_diagnostic() - .and_then(miette::Diagnostic::help) - } - } - - fn source_code(&self) -> Option<&dyn miette::SourceCode> { - self.delegated_diagnostic() - .and_then(miette::Diagnostic::source_code) - } - - fn labels(&self) -> Option + '_>> { - self.delegated_diagnostic() - .and_then(miette::Diagnostic::labels) - } - - fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - self.delegated_diagnostic() - .and_then(miette::Diagnostic::diagnostic_source) + (self.show_auth_hint && exit::exit_class_for(&self.err) == Some(ExitClass::AuthRequired)) + .then(|| Box::new("Run `fabro auth login` to authenticate.") as Box) } } diff --git a/lib/apps/fabro-cli/tests/it/cmd/inspect.rs b/lib/apps/fabro-cli/tests/it/cmd/inspect.rs index 277691ebe..835be6af6 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/inspect.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/inspect.rs @@ -211,9 +211,9 @@ fn inspect_resolves_selector_via_server_endpoint() { }, "graph": { "name": "Remote Workflow", + "goal": "", "nodes": {}, - "edges": [], - "attrs": {} + "edges": [] }, "workflow_slug": "remote-workflow", "source_directory": "/srv/repo", diff --git a/lib/apps/fabro-cli/tests/it/cmd/preflight.rs b/lib/apps/fabro-cli/tests/it/cmd/preflight.rs index 1bf970ff0..481c81c5d 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/preflight.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/preflight.rs @@ -70,12 +70,8 @@ fn preflight_rejects_unbound_template_inputs() { ----- stderr ----- Workflow: TemplatedUnbound (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound.fabro - Goal: Demo + Goal: Demo {{ inputs.app_dir }} - error: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable) - fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` - error: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` error: [FIXTURES]/templated_unbound.fabro:2:12: the graph `goal` reads `{{ inputs.app_dir }}`, which no input binds (unsupported.template.unbound_input) fix: pass `--input app_dir=VALUE`, or add a default under `[run.inputs]` in workflow.toml error: [FIXTURES]/templated_unbound.fabro:7:25: node `work` `prompt` reads `{{ inputs.app_dir }}`, which no input binds (unsupported.template.unbound_input) diff --git a/lib/apps/fabro-cli/tests/it/cmd/validate.rs b/lib/apps/fabro-cli/tests/it/cmd/validate.rs index f99191b5f..8bf2f2700 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/validate.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/validate.rs @@ -206,10 +206,6 @@ fn bare_fabro_with_unbound_inputs_validates_structurally_with_warning() { ----- stderr ----- Workflow: TemplatedUnbound (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound.fabro - warning: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable) - fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` - warning: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` warning: [FIXTURES]/templated_unbound.fabro:2:12: the graph `goal` reads `{{ inputs.app_dir }}`, which no input binds; it is left unrendered because no inputs were given. Pass `--input app_dir=VALUE` to render it (attractor.unbound_input) warning: [FIXTURES]/templated_unbound.fabro:7:25: node `work` `prompt` reads `{{ inputs.app_dir }}`, which no input binds; it is left unrendered because no inputs were given. Pass `--input app_dir=VALUE` to render it (attractor.unbound_input) Validation: OK @@ -228,8 +224,6 @@ fn unbound_model_stylesheet_input_warns_without_css_error() { ----- stderr ----- Workflow: ModelStylesheetUnbound (3 nodes, 2 edges) Graph: [FIXTURES]/model_stylesheet_unbound.fabro - warning: [FIXTURES]/model_stylesheet_unbound.fabro:4:38: undefined template variable `inputs.effort` in graph attribute `model_stylesheet` (template_undefined_variable) - fix: bind `effort` via `[run.inputs]` in workflow.toml, or pass `--input effort=` warning: [FIXTURES]/model_stylesheet_unbound.fabro:3:9: the `model_stylesheet` reads `{{ inputs.effort }}`, which no input binds; it is left unrendered because no inputs were given. Pass `--input effort=VALUE` to render it (attractor.unbound_input) Validation: OK "); @@ -252,8 +246,6 @@ fn bare_fabro_with_unbound_inputs_in_imported_prompt_validates_structurally_with ----- stderr ----- Workflow: TemplatedUnboundImported (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound_imported/workflow.fabro - warning: [FIXTURES]/templated_unbound_imported/work.md:1:12: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` warning: [FIXTURES]/templated_unbound_imported/workflow.fabro:5:25: node `work` `prompt` reads `{{ inputs.app_dir }}`, which no input binds; it is left unrendered because no inputs were given. Pass `--input app_dir=VALUE` to render it (attractor.unbound_input) Validation: OK "); @@ -275,8 +267,6 @@ fn bare_fabro_with_unbound_inputs_in_template_partial_validates_structurally_wit ----- stderr ----- Workflow: TemplatedUnboundPartial (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound_partial/workflow.fabro - warning: [FIXTURES]/templated_unbound_partial/test-include.partial.md:1:4: undefined template variable `inputs.hello` in node `test_imported_include` attribute `prompt` [node: test_imported_include] (template_undefined_variable) - fix: bind `hello` via `[run.inputs]` in workflow.toml, or pass `--input hello=` error: [FIXTURES]/templated_unbound_partial/workflow.fabro:3:42: node `test_imported_include` `prompt`: template render: could not render include: error in "../../../../../../../..[FIXTURES]/templated_unbound_partial/test-include.partial.md" (in ../../../../../../../..[FIXTURES]/templated_unbound_partial/__petri_root__:1) (attractor.template) × Validation failed "#); diff --git a/lib/apps/fabro-cli/tests/it/support/mod.rs b/lib/apps/fabro-cli/tests/it/support/mod.rs index 7e52a2126..25364a67c 100644 --- a/lib/apps/fabro-cli/tests/it/support/mod.rs +++ b/lib/apps/fabro-cli/tests/it/support/mod.rs @@ -10,7 +10,7 @@ pub(crate) use auth_harness::{ }; pub(crate) use auth_tokens::{TEST_SESSION_SECRET, issue_test_github_jwt, issue_test_worker_jwt}; use fabro_test::{EnvVars, TestContext, preserve_coverage_env}; -use fabro_types::{Graph, RunId, RunSpec, RunStreamItem, WorkflowSettings}; +use fabro_types::{RunGraph, RunId, RunSpec, RunStreamItem, WorkflowSettings}; pub(crate) use mcp_client::McpStdioTestClient; pub(crate) fn run_output_filters(context: &TestContext) -> Vec<(String, String)> { @@ -45,7 +45,7 @@ pub(crate) fn run_projection_json(run_id: &str, status: &serde_json::Value) -> s let spec = RunSpec { run_id, settings: WorkflowSettings::default(), - graph: Graph::new("Remote Workflow"), + graph: RunGraph::new("Remote Workflow"), graph_source: None, workflow_slug: Some("remote-workflow".to_string()), workflow_version_id: None, diff --git a/lib/apps/fabro-server/src/demo/mod.rs b/lib/apps/fabro-server/src/demo/mod.rs index d4ca46711..c48e90058 100644 --- a/lib/apps/fabro-server/src/demo/mod.rs +++ b/lib/apps/fabro-server/src/demo/mod.rs @@ -1630,7 +1630,7 @@ mod runs { /// agent stage carrying the coding agent's fold of `agent_events()`. pub(super) fn run_state() -> fabro_types::RunProjection { use fabro_types::{ - Graph, RunProjection, RunProvenance, RunSpec, StageTiming, WorkflowSettings, + RunGraph, RunProjection, RunProvenance, RunSpec, StageTiming, WorkflowSettings, first_event_seq, }; use pebble_coding_agent::projection::SessionProjection; @@ -1639,7 +1639,7 @@ mod runs { let spec = RunSpec { run_id: demo_run_id(1), settings: WorkflowSettings::default(), - graph: Graph::new("drift-remediation"), + graph: RunGraph::new("drift-remediation"), graph_source: Some(super::DEMO_GRAPH_DOT.to_string()), workflow_slug: Some("implement".to_string()), workflow_version_id: None, diff --git a/lib/apps/fabro-server/src/manifest_validation.rs b/lib/apps/fabro-server/src/manifest_validation.rs index f04b8f954..9dbcb490a 100644 --- a/lib/apps/fabro-server/src/manifest_validation.rs +++ b/lib/apps/fabro-server/src/manifest_validation.rs @@ -1,14 +1,14 @@ use std::collections::HashMap; -use std::path::PathBuf; +use std::path::Path; use anyhow::{Result, anyhow}; use fabro_api::types; -use fabro_config::{RunLayer, SettingsLayer, WorkflowSettingsBuilder}; +use fabro_config::{RunLayer, SettingsLayer, WorkflowSettingsBuilder, project}; use fabro_manifest::CollectedWorkflowClosure; +use fabro_petri::run_graph; use fabro_petri::runtime::RuntimeSpec; -use fabro_workflow::operations::{ValidateInput, WorkflowInput, validate}; -use fabro_workflow::pipeline::TEMPLATE_UNDEFINED_VARIABLE_RULE; +use crate::run_manifest::{ManifestCheck, workflow_shape_of}; use crate::{petri_check, run_intent, run_manifest}; /// Validate a manifest without a model catalog. @@ -17,7 +17,9 @@ use crate::{petri_check, run_intent, run_manifest}; /// client's catalog is its own, not the server's. Judging model and provider /// availability here would reject workflows the server can run, so Petri /// checks the bundle with no model client: the structure, the settings and -/// the templates, with every model node left for the server on create. +/// the templates, with every model node left for the server on create. An +/// input nothing binds is a warning here (`attractor.unbound_input`), since +/// the run's inputs do not exist yet. pub fn validate_manifest( manifest_run_defaults: &RunLayer, manifest: &types::RunManifest, @@ -67,8 +69,8 @@ fn offline_runtime(run: Option<&RunLayer>) -> RuntimeSpec { /// The supplied run layer is complete, including any already resolved inline /// goal. Validation uses only seeded environment defaults, immutable workflow /// settings, and explicit inputs; it performs no store, HTTP, user-settings, -/// project-settings, or model-catalog operation. Undefined template variables -/// are promoted to errors before the response is returned. +/// project-settings, or model-catalog operation. An input nothing binds is +/// an error here (`unsupported.template.unbound_input`), as it is at create. pub fn validate_collected_workflow( closure: &CollectedWorkflowClosure, run_overrides: Option<&RunLayer>, @@ -94,14 +96,6 @@ pub fn validate_collected_workflow( } let mut settings = builder.build().map_err(anyhow::Error::new)?; settings.run.inputs.extend(input_overrides.clone()); - let mut validated = validate(ValidateInput { - workflow: WorkflowInput::Bundled(workflow), - settings: settings.clone(), - vars: HashMap::new(), - cwd: PathBuf::from("/workspace"), - custom_transforms: Vec::new(), - }) - .map_err(anyhow::Error::new)?; let request = petri_check::check_request( &lowered.workflow_bundle, &lowered.entrypoint, @@ -113,26 +107,17 @@ pub fn validate_collected_workflow( ) .map_err(anyhow::Error::new)?; let checked = petri_check::check(&request, true).map_err(anyhow::Error::new)?; - validated.extend_diagnostics(checked.diagnostics); - let mut response = types::ValidateResponse { - ok: !validated.has_errors(), - workflow: run_manifest::workflow_summary(&validated, lowered.entrypoint.as_path()), + let check = ManifestCheck { + graph: checked.admitted.as_ref().map(run_graph::run_graph), + diagnostics: checked.diagnostics, }; - promote_template_undefined_variables_to_errors(&mut response); - Ok(response) -} - -pub fn promote_template_undefined_variables_to_errors(response: &mut types::ValidateResponse) { - let mut promoted = false; - for diagnostic in &mut response.workflow.diagnostics { - if diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE { - diagnostic.severity = types::WorkflowDiagnosticSeverity::Error; - promoted = true; - } - } - if promoted { - response.ok = false; - } + let working_directory = + project::resolve_working_directory_from_run(&settings.run, Path::new("/workspace")); + let shape = workflow_shape_of(&check, &workflow.source, &settings, &working_directory); + Ok(types::ValidateResponse { + ok: !check.has_errors(), + workflow: run_manifest::workflow_summary(&check, &shape, lowered.entrypoint.as_path()), + }) } #[cfg(test)] @@ -150,6 +135,10 @@ mod tests { use super::*; + /// Petri's code for an input a template reads that nothing binds, as + /// the check before a run raises it. + const UNBOUND_INPUT_RULE: &str = "unsupported.template.unbound_input"; + fn write(root: &Path, path: &str, content: &str) { let path = root.join(path); fs::create_dir_all(path.parent().unwrap()).unwrap(); @@ -217,7 +206,7 @@ dockerfile = { path = "Dockerfile" } } #[test] - fn collected_validation_matches_legacy_response_for_equivalent_inputs() { + fn collected_validation_matches_the_manifest_response_for_equivalent_inputs() { let temp = tempfile::tempdir().unwrap(); let workflow = write_complete_fixture(temp.path()); let run = run_overrides("inline goal"); @@ -242,14 +231,13 @@ dockerfile = { path = "Dockerfile" } }) .unwrap(); - let mut legacy = validate_manifest(&RunLayer::default(), &manifest.manifest).unwrap(); - promote_template_undefined_variables_to_errors(&mut legacy); + let from_manifest = validate_manifest(&RunLayer::default(), &manifest.manifest).unwrap(); let collected = validate_collected_workflow(package.closure(), Some(&run), &inputs).unwrap(); assert_eq!( serde_json::to_value(collected).unwrap(), - serde_json::to_value(legacy).unwrap(), + serde_json::to_value(from_manifest).unwrap(), ); } @@ -267,10 +255,15 @@ dockerfile = { path = "Dockerfile" } let missing = validate_collected_workflow(package.closure(), None, &HashMap::new()).unwrap(); assert!(!missing.ok); - assert!(missing.workflow.diagnostics.iter().any(|diagnostic| { - diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE - && diagnostic.severity == types::WorkflowDiagnosticSeverity::Error - })); + assert!( + missing.workflow.diagnostics.iter().any(|diagnostic| { + diagnostic.rule == UNBOUND_INPUT_RULE + && diagnostic.severity == types::WorkflowDiagnosticSeverity::Error + && diagnostic.message.contains("inputs.owner") + }), + "{:?}", + missing.workflow.diagnostics + ); let present = validate_collected_workflow( package.closure(), @@ -283,7 +276,7 @@ dockerfile = { path = "Dockerfile" } .workflow .diagnostics .iter() - .all(|diagnostic| { diagnostic.rule != TEMPLATE_UNDEFINED_VARIABLE_RULE }) + .all(|diagnostic| diagnostic.rule != UNBOUND_INPUT_RULE) ); assert!(present.ok, "{:?}", present.workflow.diagnostics); assert_eq!(present.workflow.goal, "resolved inline goal"); diff --git a/lib/apps/fabro-server/src/run_compiler.rs b/lib/apps/fabro-server/src/run_compiler.rs index 2bafdd33f..650d7e562 100644 --- a/lib/apps/fabro-server/src/run_compiler.rs +++ b/lib/apps/fabro-server/src/run_compiler.rs @@ -7,13 +7,13 @@ //! 1. [`normalize_source`] — resolve the bundle entrypoint and retain the //! admitted workflow settings, whose dockerfile references are already //! inlined. -//! 2. [`layer_settings`] + [`apply_run_variables`] + graph compilation — layer -//! settings from every configured source, substitute the run-scoped variable -//! snapshot, then parse/transform/validate the graph through the -//! fabro-workflow pipeline. -//! 3. [`compile_admitted`] — Petri compiled, linted and pinned models at its -//! admission, so only the Fabro graph the read side displays is parsed here, -//! and the admission is recorded on the run. +//! 2. [`layer_settings`] + [`apply_run_variables`] — layer settings from every +//! configured source and substitute the run-scoped variable snapshot. The +//! prepared run then goes to Petri (`crate::server::petri_runs::admit`), +//! which compiles, lints and pins models: its admitted graph is the graph. +//! 3. [`materialize_admitted`] — record the admission and the display graph +//! read off it on the run, and materialize Fabro's run-level settings (the +//! goal, the pull request block) around them. //! 4. [`assemble_run`] — purely assemble the complete persistence input; no //! field is mutated after assembly. //! @@ -35,14 +35,14 @@ use fabro_config::{ use fabro_types::settings::interp::{InterpString, ResolveError}; use fabro_types::settings::run::{McpServerSettings, RunGoal}; use fabro_types::{ - AutomationRef, GitContext, ManifestPath, PetriAdmission, RunId, RunProvenance, RunTarget, - WorkflowSettings, WorkflowVersionId, + AutomationRef, GitContext, ManifestPath, PetriAdmission, RunGraph, RunId, RunProvenance, + RunTarget, WorkflowSettings, WorkflowVersionId, }; use fabro_util::workspace_glob::{WorkspaceGlob, WorkspaceGlobError}; use fabro_workflow::Error as WorkflowError; use fabro_workflow::operations::{ - self, CreateRunCompileInput, CreateRunPersistenceInput, CreateRunPersistenceMetadata, - MaterializedRun, WorkflowInput, + self, AdmittedRunInput, CreateRunPersistenceInput, CreateRunPersistenceMetadata, + MaterializedRun, }; use fabro_workflow::workflow_bundle::{BundledWorkflow, WorkflowBundle}; use tokio::task; @@ -125,7 +125,8 @@ pub(crate) struct LayeredRun { } /// Variable-substituted stage output. Callers may inspect the resolved -/// settings before policy checks, then move it into [`compile_and_pin`]. +/// settings before policy checks, then hand it to Petri's admission and move +/// it into [`materialize_admitted`]. pub(crate) struct PreparedRun { layered: LayeredRun, vars: HashMap, @@ -185,7 +186,14 @@ impl PreparedRun { } } -/// Model-pinned stage output ready for pure persistence-input assembly. +/// What Petri admitted for a run: the stored graphs the run executes from, +/// and the display graph read off them. +pub(crate) struct AdmittedRun { + pub(crate) admission: PetriAdmission, + pub(crate) graph: RunGraph, +} + +/// Admitted stage output ready for pure persistence-input assembly. pub(crate) struct PinnedRun { materialized: MaterializedRun, metadata: RunMetadata, @@ -210,9 +218,9 @@ pub(crate) enum RunCompilerError { #[error("Run config variable interpolation failed: {0}")] VariableInterpolation(#[from] VariableInterpolationError), - /// Graph compilation or model pinning failed in the workflow engine. The - /// full [`WorkflowError`] is preserved so callers can distinguish - /// validation, parse, and model-selection failures. + /// Petri refused the workflow, or Fabro's materialization around its + /// admission failed. The full [`WorkflowError`] is preserved so callers + /// can distinguish validation and parse failures. #[error(transparent)] Workflow(#[from] WorkflowError), } @@ -393,12 +401,12 @@ pub(crate) fn apply_run_variables( Ok(PreparedRun { layered, vars }) } -/// Stages two and three for a run Petri admitted: parse the Fabro graph -/// the read side displays, with no lint and no model pinning, and record -/// the admission on the run. -pub(crate) async fn compile_admitted( +/// Stage three for a run Petri admitted: record the admission and its +/// display graph on the run, and materialize Fabro's run-level settings +/// around them. Blocking: a `[run.goal]` file is read from disk. +pub(crate) async fn materialize_admitted( prepared: PreparedRun, - admission: PetriAdmission, + admitted: AdmittedRun, ) -> Result { task::spawn_blocking(move || { let PreparedRun { @@ -411,18 +419,20 @@ pub(crate) async fn compile_admitted( cwd, metadata, }, - vars, + // Consumed at admission, as Petri's compile variables. + vars: _, } = prepared; - let compiled = operations::compile_admitted_run(CreateRunCompileInput { - workflow: WorkflowInput::Bundled(workflow), + let AdmittedRun { admission, graph } = admitted; + let materialized = operations::materialize_admitted_run(AdmittedRunInput { settings, - vars, cwd, - workflow_path: Some(entrypoint), - workflow_bundle: Some(workflow_bundle), + graph, + source: workflow.source, + workflow_path: entrypoint, + workflow_bundle, })?; Ok(PinnedRun { - materialized: operations::materialize_admitted_run(compiled), + materialized, metadata, admission, }) @@ -876,21 +886,22 @@ include = ["reports/{{ vars.path }}/*.json"] HashMap::from([("owner".to_string(), "payments".to_string())]), ) .expect("settings should prepare"); - let pinned = compile_admitted(prepared, PetriAdmission::default()) - .await - .expect("the admitted graph should compile"); + let mut graph = RunGraph::new("Test"); + graph.goal = "Graph goal".to_string(); + let pinned = materialize_admitted(prepared, AdmittedRun { + admission: PetriAdmission::default(), + graph, + }) + .await + .expect("the admitted run should materialize"); let persistence = assemble_run(pinned); assert_eq!(persistence.run_id(), run_id); assert_eq!(persistence.workflow_slug(), Some("compiler-boundary")); assert_eq!(persistence.workflow_version_id(), Some(workflow_version_id)); assert_eq!(persistence.automation(), Some(&automation)); - assert_eq!( - persistence - .definition() - .map(|definition| &definition.workflow_path), - Some(&expected_entrypoint) - ); + assert_eq!(persistence.definition().workflow_path, expected_entrypoint); + assert_eq!(persistence.materialized().graph().goal(), "Graph goal"); assert_eq!( persistence.materialized().settings().run.goal.as_ref(), Some(&RunGoal::Inline(InterpString::parse("Graph goal"))) diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index efa8d6646..e5c1254d1 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -2285,7 +2285,7 @@ index 1111111..2222222 160000 fabro_types::RunSpec { run_id: fabro_types::fixtures::RUN_1, settings: fabro_types::WorkflowSettings::default(), - graph: fabro_types::Graph::new("test"), + graph: fabro_types::RunGraph::new("test"), graph_source: None, workflow_slug: None, workflow_version_id: None, diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 83dfa2fe5..fefe049b6 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::future::Future; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -7,17 +7,17 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_api::types; use fabro_config::parse::SettingsSource; +use fabro_config::run::resolve_run_goal_from_namespace; use fabro_config::{ CliLayer, CliOutputLayer, EnvironmentLayer, MergeMap, RunLayer, SettingsLayer, WorkflowSettingsBuilder, parse_input_overrides, parse_labels, project, }; use fabro_github::token_source::{InstallationTokenSource, ResolvedToken, TokenSnapshot}; -use fabro_graphviz::graph::{Graph, is_llm_handler_type}; +use fabro_graphviz::graph::AttrValue; +use fabro_graphviz::parser; use fabro_graphviz::render::apply_direction; -use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::probe::{self, ModelTestStatus}; -use fabro_llm::{FabroClient, selection}; use fabro_petri::check::Launch; +use fabro_petri::run_graph; use fabro_petri::runtime::RuntimeSpec; use fabro_proc::ProcessError; use fabro_sandbox::{ @@ -27,34 +27,33 @@ use fabro_static::EnvVars; use fabro_types::diagnostic::{Diagnostic, Severity}; use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::interp::InterpString; -use fabro_types::settings::run::{McpServerSettings, RunGoal, RunNamespace}; +use fabro_types::settings::run::{McpServerSettings, RunGoal, RunMode, RunNamespace}; use fabro_types::{ - BundledProvider, ManifestPath, RunId, SandboxProviderKind, ServerSettings, WorkflowSettings, + BundledProvider, ManifestPath, RunGraph, RunId, SandboxProviderKind, ServerSettings, + WorkflowSettings, }; use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; use fabro_workflow::Error as WorkflowError; -use fabro_workflow::operations::{ValidateInput, WorkflowInput, validate}; -use fabro_workflow::pipeline::Validated; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; use futures_util::stream::{self, StreamExt}; use lithos_llm::catalog::ProviderId; use tokio::process::Command; +use tokio::task; #[cfg(test)] use tokio::time; use tokio_util::sync::CancellationToken; -use crate::server::AppState; +use crate::server::{AppState, petri_runs}; use crate::{petri_check, run_compiler}; #[derive(Clone)] pub(crate) struct PreparedManifest { - pub cwd: PathBuf, pub git: Option, + /// The entrypoint's DOT as written: what the render endpoint draws. pub root_source: String, pub settings: WorkflowSettings, pub target_path: ManifestPath, pub workflow_bundle: WorkflowBundle, - pub workflow_input: BundledWorkflow, pub source_directory: PathBuf, } @@ -163,31 +162,35 @@ pub(crate) fn prepare_manifest_with_environment_defaults( let source_directory = project::resolve_working_directory_from_run(&settings.run, &cwd); Ok(PreparedManifest { - cwd, git: manifest.git.clone(), root_source, settings, target_path, workflow_bundle, - workflow_input, source_directory, }) } -/// Fabro's own structural pass: parse and transform the root workflow, with -/// the transforms' diagnostics. Enough to render a graph. -pub(crate) fn validate_prepared_manifest_structural( - prepared: &PreparedManifest, -) -> Result { - validate(manifest_validate_input(prepared, HashMap::new())) +/// What Petri said about a prepared manifest: the display graph when it +/// admitted the workflow, and every diagnostic, Petri's and Fabro's. +pub(crate) struct ManifestCheck { + pub(crate) graph: Option, + pub(crate) diagnostics: Vec, } -/// The structural pass, then Petri's check of the whole bundle under -/// `launch` and `runtime`, its diagnostics after the transforms' own. +impl ManifestCheck { + pub(crate) fn has_errors(&self) -> bool { + self.diagnostics + .iter() + .any(|diagnostic| diagnostic.severity == Severity::Error) + } +} + +/// Petri's check of the whole bundle under `launch` and `runtime`. /// `has_ready_provider` false adds Fabro's refusal of a model node no /// provider can run; `unbound_is_warning` keeps an input nothing binds a -/// warning, for a validation before the run's inputs exist. Blocking: -/// Petri lowers the graph synchronously. +/// warning, for a validation before the run's inputs exist. Blocking: Petri +/// lowers the graph synchronously. pub(crate) fn validate_prepared_manifest( prepared: &PreparedManifest, vars: &HashMap, @@ -195,8 +198,7 @@ pub(crate) fn validate_prepared_manifest( runtime: RuntimeSpec, has_ready_provider: bool, unbound_is_warning: bool, -) -> Result { - let mut validated = validate(manifest_validate_input(prepared, vars.clone()))?; +) -> Result { let request = petri_check::check_request( &prepared.workflow_bundle, &prepared.target_path, @@ -207,90 +209,65 @@ pub(crate) fn validate_prepared_manifest( unbound_is_warning, )?; let checked = petri_check::check(&request, has_ready_provider)?; - validated.extend_diagnostics(checked.diagnostics); - Ok(validated) + Ok(ManifestCheck { + graph: checked.admitted.as_ref().map(run_graph::run_graph), + diagnostics: checked.diagnostics, + }) } -/// The model and provider the run's LLM nodes default to: what the settings -/// or the graph name, resolved against the ready providers first and the -/// whole catalog after, as the run's launch binds them. -fn preflight_model( - catalog: &Catalog, - graph: &Graph, - settings: &WorkflowSettings, - ready_providers: &[ProviderId], -) -> Result<(String, ProviderId)> { - let graph_attr = |name: &str| { - graph - .attrs - .get(name) - .and_then(|value| value.as_str()) - .map(str::to_string) - }; - let model = settings - .run - .model - .name - .clone() - .or_else(|| graph_attr("default_model")); - let provider = settings - .run - .model - .provider - .clone() - .or_else(|| graph_attr("default_provider")) - .filter(|provider| !provider.is_empty()) - .map(ProviderId::new); - let eligible = ready_providers.iter().cloned().collect(); - let selected = selection::resolve_selection_with_catalog_fallback( - catalog, - model.as_deref(), - provider.as_ref(), - &eligible, - )?; - Ok((selected.model, selected.provider)) -} - -fn manifest_validate_input( +/// Check a prepared manifest as a run would be admitted: Petri's check with +/// the server's runtime and the model client over the ready providers, on +/// the blocking pool. +pub(crate) async fn check_prepared_manifest( + state: &Arc, prepared: &PreparedManifest, vars: HashMap, -) -> ValidateInput { - ValidateInput { - workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), - settings: prepared.settings.clone(), - vars, - cwd: prepared.cwd.clone(), - custom_transforms: Vec::new(), - } + ready_providers: &[ProviderId], +) -> Result { + let launch = petri_check::launch( + &state.catalog(), + &prepared.settings, + ready_providers, + None, + None, + ); + let dry_run = prepared.settings.run.execution.mode == RunMode::DryRun; + let runtime = petri_runs::runtime_spec(state, ready_providers, dry_run); + let has_ready_provider = !ready_providers.is_empty(); + let prepared = prepared.clone(); + task::spawn_blocking(move || { + validate_prepared_manifest(&prepared, &vars, launch, runtime, has_ready_provider, false) + }) + .await + .map_err(|source| WorkflowError::engine_with_source("manifest check task failed", source))? } pub(crate) async fn run_preflight( state: &AppState, prepared: &PreparedManifest, - validated: &Validated, - llm_result: Result, + check: &ManifestCheck, ) -> Result<(types::PreflightResponse, bool)> { - let (report, checks_ok) = - build_preflight_report(state, prepared, validated, llm_result).await?; - let preflight_ok = !validated.has_errors() && checks_ok; + let shape = workflow_shape(check, prepared); + let (report, checks_ok) = build_preflight_report(state, prepared, check, &shape).await?; + let preflight_ok = !check.has_errors() && checks_ok; Ok(( - preflight_response( - validated, - prepared.target_path.as_path(), - &report, - preflight_ok, - ), + types::PreflightResponse { + ok: preflight_ok, + checks: report_to_api(&report), + workflow: workflow_summary(check, &shape, prepared.target_path.as_path()), + }, preflight_ok, )) } pub(crate) fn validate_response( prepared: &PreparedManifest, - validated: &Validated, + check: &ManifestCheck, ) -> types::ValidateResponse { + let shape = workflow_shape(check, prepared); types::ValidateResponse { - ok: !validated.has_errors(), - workflow: workflow_summary(validated, prepared.target_path.as_path()), + ok: !check.has_errors(), + workflow: workflow_summary(check, &shape, prepared.target_path.as_path()), } } @@ -441,12 +418,11 @@ fn manifest_project_config_path( async fn build_preflight_report( state: &AppState, prepared: &PreparedManifest, - validated: &Validated, - llm_result: Result, + check: &ManifestCheck, + shape: &WorkflowShape, ) -> Result<(CheckReport, bool)> { - let graph = validated.graph(); - let mut checks = base_preflight_checks(prepared, graph); - if validated.has_errors() { + let mut checks = base_preflight_checks(prepared, shape); + if check.has_errors() { return Ok(( CheckReport { title: "Run Preflight".into(), @@ -459,19 +435,6 @@ async fn build_preflight_report( )); } - let catalog = state.catalog(); - let ready_providers = llm_result - .as_ref() - .map(FabroClient::provider_ids) - .unwrap_or_default(); - let (run_model, run_provider) = preflight_model( - catalog.as_ref(), - graph, - &prepared.settings, - &ready_providers, - )?; - let run_provider = run_provider.into_string(); - let (run_model, run_provider) = (run_model.as_str(), run_provider.as_str()); let resolved_run = prepared.settings.run.clone(); let server_settings = state.server_settings(); let github_integration = &server_settings.server.integrations.github; @@ -531,19 +494,10 @@ async fn build_preflight_report( github_app.clone(), ) .await; - let llm_ok = run_llm_check( - &mut checks, - graph, - run_model, - run_provider, - catalog.as_ref(), - llm_result, - ) - .await; let github_token_ok = run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await; - let checks_ok = sandbox_ok && repository_access_ok && llm_ok && github_token_ok; + let checks_ok = sandbox_ok && repository_access_ok && github_token_ok; Ok(( CheckReport { @@ -557,7 +511,7 @@ async fn build_preflight_report( )) } -fn base_preflight_checks(prepared: &PreparedManifest, graph: &Graph) -> Vec { +fn base_preflight_checks(prepared: &PreparedManifest, shape: &WorkflowShape) -> Vec { let setup_command_count = prepared.settings.run.prepare.steps.len(); let repo_summary = prepared.git.as_ref().map_or_else( || "unknown".to_string(), @@ -600,11 +554,11 @@ fn base_preflight_checks(prepared: &PreparedManifest, graph: &Graph) -> Vec, - graph: &Graph, - model: &str, - default_provider: &str, - catalog: &Catalog, - llm_result: Result, -) -> bool { - let mut model_providers = std::collections::BTreeSet::new(); - let mut has_llm_nodes = false; - // Each node's selector resolves against the ready providers first and - // the whole catalog after, as the run's launch binds it: an alias - // becomes the catalog model on the provider that offers it. A selector - // the catalog cannot place stays as written; the checks below say why. - let eligible = llm_result - .as_ref() - .map(FabroClient::provider_ids) - .unwrap_or_default() - .into_iter() - .collect::>(); - - for node in graph.nodes.values() { - if !is_llm_handler_type(node.handler_type()) { - continue; - } - has_llm_nodes = true; - let node_model = node.model().unwrap_or(model); - let node_provider = node.provider().unwrap_or(default_provider); - // Only a provider the node names itself constrains the selection: - // an unqualified alias goes to whichever eligible provider offers it. - let provider = node - .provider() - .filter(|provider| !provider.is_empty()) - .map(ProviderId::new); - let resolved = selection::resolve_selection_with_catalog_fallback( - catalog, - Some(node_model), - provider.as_ref(), - &eligible, - ) - .map_or_else( - |_| (node_model.to_string(), node_provider.to_string()), - |selected| (selected.model, selected.provider.into_string()), - ); - model_providers.insert(resolved); - } - - if !has_llm_nodes { - return true; - } - - match llm_result { - Ok(result) => { - let auth_issues = result.auth_issues; - let registration_issues = result.build_issues; - let client = Arc::new(result.client); - - let mut all_ok = true; - let mut completed_checks: Vec<(usize, CheckResult)> = Vec::new(); - let mut pending_probes = Vec::new(); - for (index, (model_id, provider_name)) in model_providers.iter().enumerate() { - let provider_id = canonical_provider_id(catalog, provider_name); - if let Some((_, issue)) = auth_issues - .iter() - .find(|(candidate, _)| candidate == &provider_id) - { - all_ok = false; - completed_checks.push((index, CheckResult { - name: "LLM".into(), - status: CheckStatus::Warning, - summary: model_id.clone(), - details: vec![CheckDetail::new(format!("Provider: {provider_name}"))], - remediation: Some(issue.to_string()), - })); - } else if let Some(issue) = registration_issues - .iter() - .find(|issue| issue.provider == provider_id) - { - all_ok = false; - completed_checks.push((index, CheckResult { - name: "LLM".into(), - status: CheckStatus::Warning, - summary: model_id.clone(), - details: vec![CheckDetail::new(format!("Provider: {provider_name}"))], - remediation: Some(issue.cause.to_string()), - })); - } else if !client.available_providers().contains(&provider_id) { - all_ok = false; - completed_checks.push((index, CheckResult { - name: "LLM".into(), - status: CheckStatus::Warning, - summary: model_id.clone(), - details: vec![CheckDetail::new(format!("Provider: {provider_name}"))], - remediation: Some(format!( - "Provider \"{provider_name}\" is not configured" - )), - })); - } else { - pending_probes.push(PendingModelProbe { - index, - model_id: model_id.clone(), - provider_name: provider_name.clone(), - }); - } - } - - let mut probe_checks = stream::iter(pending_probes) - .map(|probe| { - let client = Arc::clone(&client); - async move { - let outcome = probe::run_basic_probe( - &client, - &format!("{}/{}", probe.provider_name, probe.model_id), - Duration::from_secs(fabro_types::ModelTestMode::Basic.timeout_secs()), - ) - .await; - let (status, remediation) = if outcome.status == ModelTestStatus::Ok { - (CheckStatus::Pass, None) - } else { - ( - CheckStatus::Error, - Some(format!( - "Model availability probe failed: {}", - outcome - .error_message - .unwrap_or_else(|| "unknown error".to_string()) - )), - ) - }; - (probe.index, CheckResult { - name: "LLM".into(), - status, - summary: probe.model_id, - details: vec![ - CheckDetail::new(format!("Provider: {}", probe.provider_name)), - CheckDetail::new("Probe: basic generation".to_string()), - ], - remediation, - }) - } - }) - .buffer_unordered(MODEL_PREFLIGHT_PROBE_CONCURRENCY) - .collect::>() - .await; - - if probe_checks - .iter() - .any(|(_, check)| check.status != CheckStatus::Pass) - { - all_ok = false; - } - completed_checks.append(&mut probe_checks); - completed_checks.sort_by_key(|(index, _)| *index); - checks.extend(completed_checks.into_iter().map(|(_, check)| check)); - all_ok - } - Err(err) => { - checks.push(CheckResult { - name: "LLM".into(), - status: CheckStatus::Error, - summary: "initialization failed".into(), - details: vec![], - remediation: Some(format!("LLM client init failed: {err}")), - }); - false - } - } -} - -fn canonical_provider_id(catalog: &Catalog, provider_name: &str) -> ProviderId { - catalog.enabled_provider(provider_name).map_or_else( - || ProviderId::new(provider_name), - |provider| provider.id().clone(), - ) -} - async fn run_github_token_check( checks: &mut Vec, prepared: &PreparedManifest, @@ -1535,32 +1305,81 @@ async fn mint_github_token( .await } -fn preflight_response( - validated: &Validated, - target_path: &Path, - report: &CheckReport, - ok: bool, -) -> types::PreflightResponse { - types::PreflightResponse { - ok, - checks: report_to_api(report), - workflow: workflow_summary(validated, target_path), +/// The workflow as the validate and preflight responses describe it: its +/// name, its size and its goal. From the graph Petri admitted when it did; +/// for a refused workflow, from the DOT as written, so the response still +/// names what was checked. The goal is the run's effective goal, as create +/// materializes it: the settings' `run.goal` when the run has one, else the +/// workflow's own. +pub(crate) struct WorkflowShape { + name: String, + nodes: usize, + edges: usize, + goal: String, +} + +pub(crate) fn workflow_shape(check: &ManifestCheck, prepared: &PreparedManifest) -> WorkflowShape { + workflow_shape_of( + check, + &prepared.root_source, + &prepared.settings, + &prepared.source_directory, + ) +} + +pub(crate) fn workflow_shape_of( + check: &ManifestCheck, + root_source: &str, + settings: &WorkflowSettings, + working_directory: &Path, +) -> WorkflowShape { + let mut shape = check.graph.as_ref().map_or_else( + || { + parser::parse(root_source).map_or_else( + |_| WorkflowShape { + name: String::new(), + nodes: 0, + edges: 0, + goal: String::new(), + }, + |graph| WorkflowShape { + goal: graph + .attrs + .get("goal") + .and_then(AttrValue::as_str) + .unwrap_or_default() + .to_string(), + nodes: graph.nodes.len(), + edges: graph.edges.len(), + name: graph.name, + }, + ) + }, + |graph| WorkflowShape { + name: graph.name.clone(), + nodes: graph.nodes.len(), + edges: graph.edges.len(), + goal: graph.goal().to_string(), + }, + ); + if let Ok(Some(resolved)) = resolve_run_goal_from_namespace(&settings.run, working_directory) { + shape.goal = resolved.text; } + shape } pub(crate) fn workflow_summary( - validated: &Validated, + check: &ManifestCheck, + shape: &WorkflowShape, target_path: &Path, ) -> types::PreflightWorkflowSummary { types::PreflightWorkflowSummary { - diagnostics: diagnostics_to_api(validated.diagnostics()), - edges: i64::try_from(validated.graph().edges.len()) - .expect("graph edge count should fit in i64"), - goal: validated.graph().goal().to_string(), + diagnostics: diagnostics_to_api(&check.diagnostics), + edges: i64::try_from(shape.edges).expect("graph edge count should fit in i64"), + goal: shape.goal.clone(), graph_path: Some(target_path.display().to_string()), - name: validated.graph().name.clone(), - nodes: i64::try_from(validated.graph().nodes.len()) - .expect("graph node count should fit in i64"), + name: shape.name.clone(), + nodes: i64::try_from(shape.nodes).expect("graph node count should fit in i64"), } } @@ -1647,13 +1466,13 @@ mod tests { use super::*; - /// Validate as the endpoints do: the structural pass, then Petri's check - /// with the state's model client over `ready_providers`. + /// Validate as the endpoints do: Petri's check with the state's model + /// client over `ready_providers`. fn validate_for_test( state: &AppState, prepared: &PreparedManifest, ready_providers: &[ProviderId], - ) -> Result { + ) -> Result { let launch = petri_check::launch( &state.catalog(), &prepared.settings, @@ -1677,7 +1496,7 @@ mod tests { fn validate_with_catalog( state: &AppState, prepared: &PreparedManifest, - ) -> Result { + ) -> Result { let providers = state .catalog() .enabled_provider_ids() @@ -1798,21 +1617,6 @@ mod tests { ) } - fn openai_compatible_completion(model: &str) -> serde_json::Value { - serde_json::json!({ - "id": "chatcmpl_preflight", - "object": "chat.completion", - "created": 1_700_000_000, - "model": model, - "choices": [{ - "index": 0, - "message": {"role": "assistant", "content": "OK"}, - "finish_reason": "stop" - }], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2} - }) - } - fn ready_moonshot_and_openrouter_state( server: &httpmock::MockServer, ) -> Arc { @@ -1837,55 +1641,12 @@ enabled = true .build() } - async fn preflight_for_model( - state: &Arc, - model: &str, - ) -> (types::PreflightResponse, bool) { - let llm_result = state.resolve_llm_client().await; - let mut ready_providers = llm_result - .as_ref() - .map(FabroClient::provider_ids) - .unwrap_or_default(); - ready_providers.sort(); - assert_eq!(ready_providers, vec![ - ProviderId::new("moonshot"), - ProviderId::new("openrouter") - ]); - - let mut manifest = minimal_manifest(); - manifest.workflows.get_mut("workflow.fabro").unwrap().source = format!( - r#" -digraph Demo {{ - start [shape=Mdiamond] - exit [shape=Msquare] - work [prompt="Do work", model="{model}"] - start -> work -> exit -}} -"# - ); - let prepared = prepare_manifest( - &manifest_run_defaults(Some(&default_settings_fixture())), - &manifest, - ) - .unwrap(); - let validated = validate_for_test(state, &prepared, &ready_providers).unwrap(); - assert!(!validated.has_errors(), "{:?}", validated.diagnostics()); - - run_preflight(state.as_ref(), &prepared, &validated, llm_result) - .await - .unwrap() - } - /// Preflight for a workflow naming `model`, which Petri refuses. async fn preflight_for_refused_model( state: &Arc, model: &str, ) -> (types::PreflightResponse, bool) { - let llm_result = state.resolve_llm_client().await; - let ready_providers = llm_result - .as_ref() - .map(FabroClient::provider_ids) - .unwrap_or_default(); + let (_, ready_providers) = state.resolve_llm_client_with_ready_ids().await; let mut manifest = minimal_manifest(); manifest.workflows.get_mut("workflow.fabro").unwrap().source = format!( r#" @@ -1903,9 +1664,9 @@ digraph Demo {{ ) .unwrap(); let validated = validate_for_test(state, &prepared, &ready_providers).unwrap(); - assert!(validated.has_errors(), "{:?}", validated.diagnostics()); + assert!(validated.has_errors(), "{:?}", validated.diagnostics); - run_preflight(state.as_ref(), &prepared, &validated, llm_result) + run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap() } @@ -1913,10 +1674,9 @@ digraph Demo {{ async fn resolve_and_run_preflight( state: &AppState, prepared: &PreparedManifest, - validated: &Validated, + check: &ManifestCheck, ) -> Result<(types::PreflightResponse, bool)> { - let llm_result = state.resolve_llm_client().await; - run_preflight(state, prepared, validated, llm_result).await + run_preflight(state, prepared, check).await } fn manifest_workflow() -> types::ManifestWorkflow { @@ -2569,7 +2329,7 @@ issues = "read" ) .unwrap(); let validated = validate_with_catalog(&state, &prepared).unwrap(); - assert!(!validated.has_errors(), "{:?}", validated.diagnostics()); + assert!(!validated.has_errors(), "{:?}", validated.diagnostics); let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await @@ -2614,7 +2374,7 @@ issues = "{{ env.GITHUB_ISSUES_PERMISSION }}" ) .unwrap(); let validated = validate_with_catalog(&state, &prepared).unwrap(); - assert!(!validated.has_errors(), "{:?}", validated.diagnostics()); + assert!(!validated.has_errors(), "{:?}", validated.diagnostics); let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await @@ -2669,7 +2429,7 @@ id = "local" .unwrap(); let validated = validate_with_catalog(&state, &prepared).unwrap(); - assert!(!validated.has_errors(), "{:?}", validated.diagnostics()); + assert!(!validated.has_errors(), "{:?}", validated.diagnostics); let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await @@ -2792,139 +2552,18 @@ id = "daytona" } #[tokio::test] - async fn preflight_probes_configured_llm_model_availability() { + async fn preflight_refuses_an_unknown_unqualified_model_before_any_check() { let server = httpmock::MockServer::start_async().await; - let response_mock = server + let any_provider_call = server .mock_async(|when, then| { - when.method(httpmock::Method::POST) - .path("/v1/responses") - .header("authorization", "Bearer test-openai-key"); - then.status(429) - .header("content-type", "application/json") - .json_body(serde_json::json!({ - "error": { - "message": "quota limited", - "type": "rate_limit_error" - } - })); - }) - .await; - let state = crate::test_support::TestAppStateBuilder::new() - .runtime_settings( - crate::test_support::default_test_server_settings(), - RunLayer::default(), - ) - .max_concurrent_runs(5) - .provider_base_url("openai", server.url("/v1")) - .build(); - state - .stores - .vault - .set( - "OPENAI_API_KEY", - "test-openai-key", - fabro_vault::SecretType::Token, - None, - ) - .await - .unwrap(); - - let mut manifest = minimal_manifest(); - manifest.workflows.get_mut("workflow.fabro").unwrap().source = r#" -digraph Demo { - start [shape=Mdiamond] - exit [shape=Msquare] - work [prompt="Do work", model="gpt-54"] - start -> work -> exit -} -"# - .to_string(); - let prepared = prepare_manifest( - &manifest_run_defaults(Some(&default_settings_fixture())), - &manifest, - ) - .unwrap(); - let validated = validate_with_catalog(&state, &prepared).unwrap(); - - let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); - - assert!(!ok); - let llm_check = response.checks.sections[0] - .checks - .iter() - .find(|check| check.name == "LLM" && check.summary == "gpt-5.4") - .expect("preflight should include the configured LLM model"); - assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Error); - assert!( - llm_check - .remediation - .as_deref() - .unwrap_or_default() - .contains("quota limited") - ); - assert!(response_mock.calls_async().await >= 1); - } - - #[tokio::test] - async fn preflight_uses_ready_providers_for_known_shared_alias() { - let server = httpmock::MockServer::start_async().await; - let openrouter_probe = server - .mock_async(|when, then| { - when.method(httpmock::Method::POST) - .path("/openrouter/v1/chat/completions") - .header("authorization", "Bearer test-openrouter-key") - .json_body_includes(r#"{"model":"anthropic/claude-fable-5"}"#); - then.status(200) - .header("content-type", "application/json") - .json_body(openai_compatible_completion("anthropic/claude-fable-5")); - }) - .await; - let state = ready_moonshot_and_openrouter_state(&server); - - let (response, _ok) = preflight_for_model(&state, "claude-fable").await; - - let llm_check = response.checks.sections[0] - .checks - .iter() - .find(|check| check.name == "LLM" && check.summary == "claude-fable-5") - .unwrap_or_else(|| { - panic!( - "preflight should include Claude Fable: {:?}", - response.checks.sections - ) - }); - assert_eq!( - llm_check - .details - .iter() - .map(|detail| detail.text.as_str()) - .find(|detail| detail.starts_with("Provider: ")), - Some("Provider: openrouter") - ); - assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Pass); - openrouter_probe.assert_async().await; - } - - #[tokio::test] - async fn preflight_uses_ready_providers_for_unknown_unqualified_model() { - let server = httpmock::MockServer::start_async().await; - let moonshot_probe = server - .mock_async(|when, then| { - when.method(httpmock::Method::POST) - .path("/moonshot/v1/chat/completions") - .header("authorization", "Bearer test-moonshot-key") - .json_body_includes(r#"{"model":"provider-private-preview"}"#); - then.status(200) - .header("content-type", "application/json") - .json_body(openai_compatible_completion("provider-private-preview")); + when.any_request(); + then.status(500); }) .await; let state = ready_moonshot_and_openrouter_state(&server); // Petri admits no model its catalog lacks, so the workflow is refused - // before any probe runs: no passthrough of an unknown model. + // at its check: no passthrough of an unknown model to a provider. let (response, ok) = preflight_for_refused_model(&state, "provider-private-preview").await; assert!(!ok); @@ -2939,7 +2578,7 @@ digraph Demo { .all(|check| check.name != "LLM"), "no LLM check runs on a refused workflow" ); - assert_eq!(moonshot_probe.calls_async().await, 0); + assert_eq!(any_provider_call.calls_async().await, 0); } #[test] @@ -2964,7 +2603,7 @@ digraph Demo { // Petri refuses the model node: the provider is not in the catalog. let refusal = validated - .diagnostics() + .diagnostics .iter() .find(|diagnostic| diagnostic.severity == Severity::Error) .expect("unknown provider should fail static validation"); @@ -3010,11 +2649,7 @@ digraph Demo { &manifest, ) .unwrap(); - let llm_result = state.resolve_llm_client().await; - let ready_providers = llm_result - .as_ref() - .map(FabroClient::provider_ids) - .unwrap_or_default(); + let (_, ready_providers) = state.resolve_llm_client_with_ready_ids().await; assert!(ready_providers.is_empty()); // With no provider ready there is no model client to resolve the @@ -3024,14 +2659,14 @@ digraph Demo { assert!(validated.has_errors()); assert!( validated - .diagnostics() + .diagnostics .iter() .any(|diagnostic| diagnostic.rule == petri_check::NO_READY_PROVIDER_RULE), "{:?}", - validated.diagnostics() + validated.diagnostics ); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated, llm_result) + let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); assert!(!ok); diff --git a/lib/apps/fabro-server/src/run_title_generation.rs b/lib/apps/fabro-server/src/run_title_generation.rs index dde818ea1..dc153fa61 100644 --- a/lib/apps/fabro-server/src/run_title_generation.rs +++ b/lib/apps/fabro-server/src/run_title_generation.rs @@ -4,7 +4,7 @@ use std::time::Duration; use fabro_llm::{Client, Request}; use fabro_template::{TemplateContext, TemplateError}; -use fabro_types::{Graph, MAX_RUN_TITLE_CHARS, RunId}; +use fabro_types::{MAX_RUN_TITLE_CHARS, RunGraph, RunId}; use fabro_util::error; use lithos_llm::catalog::ProviderId; use serde::Serialize; @@ -149,20 +149,19 @@ pub(crate) struct WorkflowSummary { pub(crate) struct StageSummary { id: String, label: String, - handler_type: Option, + handler_type: String, } -pub(crate) fn workflow_summary(graph: &Graph) -> WorkflowSummary { - let mut stages = graph +pub(crate) fn workflow_summary(graph: &RunGraph) -> WorkflowSummary { + let stages = graph .nodes - .values() - .map(|node| StageSummary { - id: node.id.clone(), - label: node.label().to_string(), - handler_type: node.handler_type().map(str::to_string), + .iter() + .map(|(id, node)| StageSummary { + id: id.clone(), + label: node.label.clone(), + handler_type: node.kind.to_string(), }) .collect::>(); - stages.sort_by(|left, right| left.id.cmp(&right.id)); WorkflowSummary { graph_name: graph.name.clone(), @@ -192,28 +191,36 @@ mod tests { use std::sync::{Arc, Mutex}; use async_trait::async_trait; - use fabro_graphviz::parser; use fabro_llm::adapter::{ProviderAdapter, ResolvedCall}; use fabro_llm::lithos_catalog::AdapterId; use fabro_llm::{Error as LlmError, Response, ResponseStream}; - use fabro_types::RunId; + use fabro_types::{RunGraphEdge, RunGraphNode, RunId, StageHandler}; use lithos_llm::catalog::builtin; use toml::Value as TomlValue; use super::*; - fn title_test_graph() -> fabro_types::Graph { - parser::parse( - r#"digraph Ship { - graph [goal="Deploy API token SECRET_123 to production"] - start [shape=Mdiamond, label="Start"] - plan [shape=box, label="Plan rollout"] - deploy [shape=parallelogram, label="Deploy"] - exit [shape=Msquare, label="Exit"] - start -> plan -> deploy -> exit - }"#, - ) - .unwrap() + fn title_test_graph() -> RunGraph { + let mut graph = RunGraph::new("Ship"); + graph.goal = "Deploy API token SECRET_123 to production".to_string(); + for (id, label, kind) in [ + ("start", "Start", StageHandler::Start), + ("plan", "Plan rollout", StageHandler::Agent), + ("deploy", "Deploy", StageHandler::Command), + ("exit", "Exit", StageHandler::Exit), + ] { + graph.nodes.insert(id.to_string(), RunGraphNode { + label: label.to_string(), + kind, + }); + } + for (from, to) in [("start", "plan"), ("plan", "deploy"), ("deploy", "exit")] { + graph.edges.push(RunGraphEdge { + from: from.to_string(), + to: to.to_string(), + }); + } + graph } /// Strict rendering already fails on a variable the template asks for and diff --git a/lib/apps/fabro-server/src/server/handler/graph.rs b/lib/apps/fabro-server/src/server/handler/graph.rs index f645f2ce0..9208ec7b2 100644 --- a/lib/apps/fabro-server/src/server/handler/graph.rs +++ b/lib/apps/fabro-server/src/server/handler/graph.rs @@ -52,11 +52,26 @@ async fn render_graph_from_manifest( Ok(prepared) => prepared, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; - let validated = match run_manifest::validate_prepared_manifest_structural(&prepared) { - Ok(validated) => validated, + let vars = match state.stores.variables.value_map().await { + Ok(vars) => vars, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; + let (_, ready_providers) = state.resolve_llm_client_with_ready_ids().await; + let check = match run_manifest::check_prepared_manifest( + &state, + &prepared, + vars, + &ready_providers, + ) + .await + { + Ok(check) => check, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; - if validated.has_errors() { + if check.has_errors() { return ApiError::bad_request("Validation failed").into_response(); } diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 8705a43e4..b054b831b 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -28,7 +28,6 @@ use fabro_store::{ RunSummaryListQuery, RunSummarySort, RunSummarySortDirection, RunSummaryVisibility, }; use fabro_types::diagnostic::Severity; -use fabro_types::settings::run::RunMode; use fabro_types::{ AutomationRef, ContextWindowStaleness, ManifestPath, Principal, Run, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, RunStatus, RunStatusKind, RunTarget, @@ -38,12 +37,11 @@ use fabro_types::{ }; use fabro_util::error as error_util; use fabro_util::version::FABRO_VERSION; -use fabro_workflow::pipeline::Validated; use fabro_workflow::{Error as WorkflowError, operations}; use lithos_llm::catalog::ProviderId; use serde::de::IgnoredAny; use strum::VariantArray as _; -use tokio::{fs, task}; +use tokio::fs; use tracing::info; use super::super::{ @@ -64,11 +62,11 @@ use crate::run_intent::{ EnvironmentSelectionError, PreparedIntentTarget, RunIntentAdmissionError, lower_workflow_closure, pin_workflow_environment_authority, prepare_intent_target, }; +use crate::run_manifest; use crate::run_selector::{ResolveRunError, resolve_run_by_selector}; use crate::run_title_generation::{self, GenerateTitleInput, TitlePromptInput, WorkflowSummary}; #[cfg(any(test, feature = "test-support"))] use crate::test_support as server_test_support; -use crate::{petri_check, run_manifest}; pub(super) fn manifest_routes() -> Router> { Router::new() @@ -753,7 +751,7 @@ async fn finalize_created_run( // the run executes. Fabro's own settings resolution ran above. let pinned = match petri_runs::admit(&state, &prepared, &run_materialization_provider_ids).await { - Ok(admission) => run_compiler::compile_admitted(prepared, admission).await, + Ok(admitted) => run_compiler::materialize_admitted(prepared, admitted).await, Err(error) => Err(error), }; let pinned = match pinned { @@ -810,7 +808,7 @@ async fn finalize_created_run( runs.insert( created.run_id, managed_run( - created.persisted.source().to_string(), + created.source.clone(), RunStatus::Submitted, created_at, created.run_dir, @@ -820,7 +818,7 @@ async fn finalize_created_run( } if !explicit_title_supplied && !ready_provider_ids.is_empty() { if let Some(llm_result) = llm_client_for_title { - let run_spec = created.persisted.run_spec(); + let run_spec = &created.spec; let workflow = run_title_generation::workflow_summary(&run_spec.graph); let run_inputs = run_spec.settings.run.inputs.clone(); let title_catalog = state.catalog(); @@ -1213,24 +1211,28 @@ async fn run_preflight( return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let (llm_result, ready_providers) = state.resolve_llm_client_with_ready_ids().await; - let mut validated = - match validate_manifest_on_petri(&state, &prepared, vars, &ready_providers).await { - Ok(validated) => validated, - Err(WorkflowError::Parse(_)) => { - return ApiError::bad_request("Validation failed").into_response(); - } - Err(err) => return ApiError::bad_request(err.to_string()).into_response(), - }; - validated.promote_template_undefined_variables_to_errors(); - let response = - match run_manifest::run_preflight(&state, &prepared, &validated, llm_result).await { - Ok((response, _ok)) => response, - Err(err) => { - return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) - .into_response(); - } - }; + let (_, ready_providers) = state.resolve_llm_client_with_ready_ids().await; + let check = match run_manifest::check_prepared_manifest( + &state, + &prepared, + vars, + &ready_providers, + ) + .await + { + Ok(check) => check, + Err(WorkflowError::Parse(_)) => { + return ApiError::bad_request("Validation failed").into_response(); + } + Err(err) => return ApiError::bad_request(err.to_string()).into_response(), + }; + let response = match run_manifest::run_preflight(&state, &prepared, &check).await { + Ok((response, _ok)) => response, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; (StatusCode::OK, Json(response)).into_response() } @@ -1263,55 +1265,27 @@ async fn validate_run_manifest( .into_response(); } let (_, ready_providers) = state.resolve_llm_client_with_ready_ids().await; - let validated = - match validate_manifest_on_petri(&state, &prepared, vars, &ready_providers).await { - Ok(validated) => validated, - Err(WorkflowError::Parse(_)) => { - return ApiError::bad_request("Validation failed").into_response(); - } - Err(err) => return ApiError::bad_request(err.to_string()).into_response(), - }; + let check = match run_manifest::check_prepared_manifest( + &state, + &prepared, + vars, + &ready_providers, + ) + .await + { + Ok(check) => check, + Err(WorkflowError::Parse(_)) => { + return ApiError::bad_request("Validation failed").into_response(); + } + Err(err) => return ApiError::bad_request(err.to_string()).into_response(), + }; ( StatusCode::OK, - Json(run_manifest::validate_response(&prepared, &validated)), + Json(run_manifest::validate_response(&prepared, &check)), ) .into_response() } -/// Validate a prepared manifest as a run would be admitted: Fabro's -/// structural pass, then Petri's check with the model client over the ready -/// providers, on the blocking pool. -async fn validate_manifest_on_petri( - state: &Arc, - prepared: &run_manifest::PreparedManifest, - vars: HashMap, - ready_providers: &[ProviderId], -) -> Result { - let launch = petri_check::launch( - &state.catalog(), - &prepared.settings, - ready_providers, - None, - None, - ); - let dry_run = prepared.settings.run.execution.mode == RunMode::DryRun; - let runtime = petri_runs::runtime_spec(state, ready_providers, dry_run); - let has_ready_provider = !ready_providers.is_empty(); - let prepared = prepared.clone(); - task::spawn_blocking(move || { - run_manifest::validate_prepared_manifest( - &prepared, - &vars, - launch, - runtime, - has_ready_provider, - false, - ) - }) - .await - .map_err(|source| WorkflowError::engine_with_source("manifest check task failed", source))? -} - async fn snapshot_run_variables( state: &AppState, ) -> Result, VariableError> { diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index a75388808..82781d8a0 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -1062,17 +1062,13 @@ fn build_ask_fabro_run_snapshot(projection: &fabro_types::RunProjection, run_id: let total_non_meta = graph .nodes - .values() - .filter(|node| !is_ask_fabro_meta_node(node)) + .keys() + .filter(|node_id| !graph.is_boundary(node_id)) .count(); let completed_non_meta = projection .iter_stages() .filter(|(stage_id, stage)| { - graph - .nodes - .get(stage_id.node_id()) - .is_none_or(|node| !is_ask_fabro_meta_node(node)) - && stage.effective_state().is_terminal() + !graph.is_boundary(stage_id.node_id()) && stage.effective_state().is_terminal() }) .count(); lines.push(format!( @@ -1081,12 +1077,7 @@ fn build_ask_fabro_run_snapshot(projection: &fabro_types::RunProjection, run_id: let recent_stages = projection .iter_stages() - .filter(|(stage_id, _)| { - graph - .nodes - .get(stage_id.node_id()) - .is_none_or(|node| !is_ask_fabro_meta_node(node)) - }) + .filter(|(stage_id, _)| !graph.is_boundary(stage_id.node_id())) .collect::>(); let recent_stages = recent_stages .iter() @@ -1121,10 +1112,6 @@ fn build_ask_fabro_run_snapshot(projection: &fabro_types::RunProjection, run_id: lines.join("\n") } -fn is_ask_fabro_meta_node(node: &fabro_types::Node) -> bool { - matches!(node.handler_type(), Some("start" | "exit")) -} - fn ask_fabro_stage_summary( stage_id: &fabro_types::StageId, stage: &fabro_types::StageProjection, @@ -1792,24 +1779,21 @@ enabled = true fn ask_fabro_run_snapshot_summarizes_goal_progress_and_recent_stages() { let run_id = RunId::new(); let now = Utc::now(); - let mut graph = fabro_types::Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - fabro_types::AttrValue::String("Ship the feature".to_string()), - ); + let mut graph = fabro_types::RunGraph::new("test"); + graph.goal = "Ship the feature".to_string(); for node_id in ["start", "plan", "code", "test", "review", "deploy", "exit"] { - let mut node = fabro_types::Node::new(node_id); - let shape = match node_id { - "start" => "Mdiamond", - "exit" => "Msquare", - "test" => "parallelogram", - _ => "box", + let kind = match node_id { + "start" => fabro_types::StageHandler::Start, + "exit" => fabro_types::StageHandler::Exit, + "test" => fabro_types::StageHandler::Command, + _ => fabro_types::StageHandler::Agent, }; - node.attrs.insert( - "shape".to_string(), - fabro_types::AttrValue::String(shape.to_string()), - ); - graph.nodes.insert(node_id.to_string(), node); + graph + .nodes + .insert(node_id.to_string(), fabro_types::RunGraphNode { + label: node_id.to_string(), + kind, + }); } let spec = fabro_types::RunSpec { run_id, @@ -1834,13 +1818,7 @@ enabled = true .iter() .enumerate() { - let handler = projection - .spec - .graph - .nodes - .get(*node_id) - .and_then(fabro_types::Node::handler_type) - .and_then(|handler| handler.parse().ok()); + let handler = projection.spec.graph.node(node_id).map(|node| node.kind); let stage = projection.stage_entry( node_id, 1, diff --git a/lib/apps/fabro-server/src/server/handler/usage.rs b/lib/apps/fabro-server/src/server/handler/usage.rs index 1cffc13f2..a793d66f6 100644 --- a/lib/apps/fabro-server/src/server/handler/usage.rs +++ b/lib/apps/fabro-server/src/server/handler/usage.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use chrono::{DateTime, Utc}; use fabro_types::usage_rollup::usage_rollup_from_projection; use fabro_types::{ - Graph, RunProjection, StageHandler, StageId, StageProjection, StageState, StageTiming, + RunGraph, RunProjection, StageHandler, StageId, StageProjection, StageState, StageTiming, usage_is_empty, }; @@ -23,16 +23,13 @@ pub(super) fn routes() -> Router> { fn run_stage_from_projection( stage_id: &StageId, stage: &StageProjection, - graph: &Graph, + graph: &RunGraph, now: DateTime, ) -> RunStage { let handler = stage.handler.unwrap_or_else(|| { - StageHandler::from_handler_type( - graph - .nodes - .get(stage_id.node_id()) - .and_then(|node| node.handler_type()), - ) + graph + .node(stage_id.node_id()) + .map_or(StageHandler::Agent, |node| node.kind) }); let (parallel_group_id, parallel_branch_index) = stage .parallel_branch_id diff --git a/lib/apps/fabro-server/src/server/petri_runs.rs b/lib/apps/fabro-server/src/server/petri_runs.rs index 0cb319dd6..2963cdcd7 100644 --- a/lib/apps/fabro-server/src/server/petri_runs.rs +++ b/lib/apps/fabro-server/src/server/petri_runs.rs @@ -5,7 +5,8 @@ //! and the launch to Petri's `Runtime::check` through `fabro_petri::check`, //! maps Petri's diagnostics onto Fabro's, and stores the admitted graphs in //! the blob store so the run executes and resumes from what was admitted. -//! Petri compiled, linted and pinned models; the legacy compile is skipped. +//! Petri compiled, linted and pinned models; the run's display graph is read +//! off its admitted graph (`fabro_petri::run_graph`). //! //! At execution, a Petri run takes the same path a legacy run does: the //! scheduler launches `fabro run __run-worker` with the worker's token, and @@ -45,13 +46,11 @@ use fabro_petri::platform_records::SqlitePlatformRecords; use fabro_petri::recovery::{self, Recovery, RecoveryRequest}; use fabro_petri::runtime::{self, RuntimeSpec}; use fabro_petri::secrets::VaultSecrets; -use fabro_petri::{SqliteRunStore, admission}; +use fabro_petri::{SqliteRunStore, admission, run_graph}; use fabro_store::platform_records::{RunLifecycleKind, RunLifecycleRecord}; use fabro_types::settings::McpTransport; use fabro_types::settings::run::{ApprovalMode, McpServerSettings, RunMode}; -use fabro_types::{ - FailureReason, PetriAdmission, RunId, RunRunnableSource, RunStatus, RunTarget, SuccessReason, -}; +use fabro_types::{FailureReason, RunId, RunRunnableSource, RunStatus, RunTarget, SuccessReason}; use fabro_util::error as error_util; use fabro_workflow::Error as WorkflowError; use lithos_llm::catalog::ProviderId; @@ -65,7 +64,7 @@ use super::{ }; use crate::petri_check; use crate::petri_runs::PetriRuns; -use crate::run_compiler::{PreparedRun, RunCompilerError}; +use crate::run_compiler::{AdmittedRun, PreparedRun, RunCompilerError}; /// The runtime Petri gets, at create and at execution: the server's run /// defaults and environment catalog as the settings layer, the MCP @@ -245,14 +244,14 @@ fn mcp_catalog_entry(server: &McpServerSettings) -> toml::Table { entry } -/// Petri compiles the run: check the bundle, map the diagnostics, and -/// persist the admitted graphs. A refusal is the same validation error the -/// legacy compiler raised, carrying Petri's diagnostics. +/// Petri compiles the run: check the bundle, map the diagnostics, persist +/// the admitted graphs, and read the display graph off them. A refusal is a +/// validation error carrying Petri's diagnostics. pub(crate) async fn admit( state: &AppState, prepared: &PreparedRun, eligible: &[ProviderId], -) -> Result { +) -> Result { let settings = prepared.settings(); let repository = match prepared.target() { Some(RunTarget::Folder { path }) => Some(path.into()), @@ -301,14 +300,18 @@ pub(crate) async fn admit( "Petri's check admitted no graph and raised no error", )) })?; - admission::persist(&state.store_ref().blobs(), &admitted) + let admission = admission::persist(&state.store_ref().blobs(), &admitted) .await .map_err(|err| { RunCompilerError::Workflow(WorkflowError::engine_with_source( "the admitted graphs could not be stored", err, )) - }) + })?; + Ok(AdmittedRun { + admission, + graph: run_graph::run_graph(&admitted), + }) } /// Execute a Petri run in the server process, under the test override: diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 78e2aa5a0..29182e8da 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4819,8 +4819,11 @@ async fn validate_endpoint_uses_app_state_catalog_for_model_diagnostics() { ); } +/// An input a bundled prompt reads that nothing binds is Petri's +/// `unsupported.template.unbound_input`, positioned at the node attribute +/// that names the prompt file, in the workflow the manifest targets. #[tokio::test] -async fn validate_endpoint_returns_template_source_coordinates() { +async fn validate_endpoint_reports_an_unbound_input_with_petris_code_and_position() { let app = test_app_with(); let dot = r#"digraph ValidatePlan { start [shape=Mdiamond, label="Start"] @@ -4866,18 +4869,17 @@ async fn validate_endpoint_returns_template_source_coordinates() { let diagnostics = body["workflow"]["diagnostics"].as_array().unwrap(); let diagnostic = diagnostics .iter() - .find(|diagnostic| diagnostic["rule"] == "template_undefined_variable") - .expect("expected template diagnostic"); + .find(|diagnostic| diagnostic["rule"] == "unsupported.template.unbound_input") + .unwrap_or_else(|| panic!("expected Petri's unbound input diagnostic: {diagnostics:?}")); - assert_eq!(diagnostic["source_path"], "test.md"); - assert_eq!(diagnostic["line"], 1); - assert_eq!(diagnostic["column"], 4); - assert!( - diagnostic["node_id"] - .as_str() - .unwrap() - .contains("test_imported_prompt") - ); + assert_eq!(diagnostic["severity"], "error"); + assert_eq!(diagnostic["source_path"], "workflow.fabro"); + assert_eq!(diagnostic["line"], 4); + assert_eq!(diagnostic["column"], 43); + let message = diagnostic["message"].as_str().unwrap(); + assert!(message.contains("test_imported_prompt"), "{message}"); + assert!(message.contains("inputs.foo"), "{message}"); + assert_eq!(body["ok"], false); } async fn create_run_for_target(app: &Router, target_path: &str, dot_source: &str) -> String { diff --git a/lib/apps/fabro-server/tests/it/api/variables.rs b/lib/apps/fabro-server/tests/it/api/variables.rs index 04749d401..6f710010e 100644 --- a/lib/apps/fabro-server/tests/it/api/variables.rs +++ b/lib/apps/fabro-server/tests/it/api/variables.rs @@ -270,12 +270,12 @@ async fn run_config_substitutes_variables_before_persisting_settings() { } #[tokio::test] -async fn run_create_interpolates_variables_into_node_prompts() { +async fn run_create_interpolates_variables_into_the_admitted_graph() { let workspace = tempfile::tempdir().unwrap(); // End-to-end through the real run-create path: a server variable resolves - // inside a node `prompt` (a DOT graph attribute the settings substitution - // pass never touches), proving the variable store is snapshotted into the - // template render context at create time. + // inside the graph `goal` (a DOT graph attribute the settings substitution + // pass never touches), proving the variable store is snapshotted into + // Petri's compile variables at create time. let state = test_app_state_with_options(test_settings(), 5); let app = fabro_server::test_support::build_test_router(std::sync::Arc::clone(&state)); @@ -291,7 +291,7 @@ async fn run_create_interpolates_variables_into_node_prompts() { response_status(create_variable, StatusCode::OK, "POST /api/v1/variables").await; let dot = r#"digraph Test { - graph [goal="Ship it"] + graph [goal="Ship {{ vars.SERVICE }}"] start [shape=Mdiamond] work [shape=box, prompt="Service: {{ vars.SERVICE }}"] exit [shape=Msquare] @@ -315,7 +315,9 @@ async fn run_create_interpolates_variables_into_node_prompts() { .expect("create run response should include id"); // The run's stream holds its `run.created` record, whose spec carries - // the fully-rendered graph. The view trails the record, so wait for it. + // the display graph read off Petri's admitted graph: its goal is the + // rendered goal, the same rendering the node prompts went through. The + // view trails the record, so wait for it. state .test_petri_projector() .settle(run_id.parse().expect("run id")) @@ -340,9 +342,12 @@ async fn run_create_interpolates_variables_into_node_prompts() { .find(|item| item["item"]["record"]["kind"] == "run.created") .expect("expected a run.created record"); assert_eq!( - created["item"]["record"]["spec"]["graph"]["nodes"]["work"]["attrs"]["prompt"]["String"], - "Service: billing", - "node prompt should interpolate the run variable; record: {created}" + created["item"]["record"]["spec"]["graph"]["goal"], "Ship billing", + "the admitted goal should interpolate the run variable; record: {created}" + ); + assert_eq!( + created["item"]["record"]["spec"]["graph"]["nodes"]["work"]["kind"], "agent", + "record: {created}" ); } diff --git a/lib/components/fabro-dump/src/lib.rs b/lib/components/fabro-dump/src/lib.rs index 27e979671..f01707dd6 100644 --- a/lib/components/fabro-dump/src/lib.rs +++ b/lib/components/fabro-dump/src/lib.rs @@ -494,12 +494,12 @@ mod tests { use chrono::{TimeZone, Utc}; use fabro_store::{RunProjection, StageId}; - use fabro_types::graph::Graph; use fabro_types::run::RunSpec; use fabro_types::{ - Checkpoint, CheckpointRecord, Conclusion, RunDiff, RunSandbox, RunSandboxInstance, - RunSandboxPlan, RunStatus, SandboxProviderKind, StageCompletion, StageModelUsage, - StageOutcome, StartRecord, SuccessReason, first_event_seq, fixtures, test_support, + Checkpoint, CheckpointRecord, Conclusion, RunDiff, RunGraph, RunSandbox, + RunSandboxInstance, RunSandboxPlan, RunStatus, SandboxProviderKind, StageCompletion, + StageModelUsage, StageOutcome, StartRecord, SuccessReason, first_event_seq, fixtures, + test_support, }; use futures::executor; @@ -507,7 +507,7 @@ mod tests { fn sample_run_spec() -> RunSpec { RunSpec { - graph: Graph::new("ship"), + graph: RunGraph::new("ship"), graph_source: Some("digraph Ship {}".to_string()), workflow_slug: Some("demo".to_string()), source_directory: Some("/tmp/project".to_string()), diff --git a/lib/components/fabro-graphviz/src/error.rs b/lib/components/fabro-graphviz/src/error.rs index b710c1dc7..9950b7738 100644 --- a/lib/components/fabro-graphviz/src/error.rs +++ b/lib/components/fabro-graphviz/src/error.rs @@ -4,9 +4,6 @@ use thiserror::Error as ThisError; pub enum Error { #[error("Parse error: {0}")] Parse(String), - - #[error("Stylesheet error: {0}")] - Stylesheet(String), } pub type Result = std::result::Result; diff --git a/lib/components/fabro-graphviz/src/lib.rs b/lib/components/fabro-graphviz/src/lib.rs index b9482f650..e9b997618 100644 --- a/lib/components/fabro-graphviz/src/lib.rs +++ b/lib/components/fabro-graphviz/src/lib.rs @@ -2,6 +2,5 @@ pub mod error; pub mod graph; pub mod parser; pub mod render; -pub mod stylesheet; pub use error::{Error, Result}; diff --git a/lib/components/fabro-graphviz/src/parser/mod.rs b/lib/components/fabro-graphviz/src/parser/mod.rs index fd924786f..02bcc8df5 100644 --- a/lib/components/fabro-graphviz/src/parser/mod.rs +++ b/lib/components/fabro-graphviz/src/parser/mod.rs @@ -70,8 +70,8 @@ mod tests { assert_eq!(graph.nodes.len(), 4); // start->run_tests, run_tests->report, report->exit assert_eq!(graph.edges.len(), 3); - assert!(graph.find_start_node().is_some()); - assert!(graph.find_exit_node().is_some()); + assert!(graph.nodes.contains_key("start")); + assert!(graph.nodes.contains_key("exit")); } #[test] diff --git a/lib/components/fabro-graphviz/src/parser/semantic.rs b/lib/components/fabro-graphviz/src/parser/semantic.rs index 8c4e48a5e..fca86dafd 100644 --- a/lib/components/fabro-graphviz/src/parser/semantic.rs +++ b/lib/components/fabro-graphviz/src/parser/semantic.rs @@ -482,7 +482,13 @@ mod tests { }; let graph = ast_to_graph(&dot).unwrap(); - assert_eq!(graph.edges[0].weight(), 5); + assert_eq!( + graph.edges[0] + .attrs + .get("weight") + .and_then(AttrValue::as_i64), + Some(5) + ); } #[test] diff --git a/lib/components/fabro-petri/src/lib.rs b/lib/components/fabro-petri/src/lib.rs index c07732ee2..3395031d3 100644 --- a/lib/components/fabro-petri/src/lib.rs +++ b/lib/components/fabro-petri/src/lib.rs @@ -16,6 +16,8 @@ //! its diagnostics come back in a shape Fabro maps onto its own; //! - [`admission`]: the admitted graphs in Fabro's blob store, named on the run //! spec; +//! - [`run_graph`]: the display graph the run spec carries, read off the +//! admitted graph's metadata; //! - [`engine`]: a run executed by Petri, started or resumed, in the run's //! worker process over the HTTP store (or in the server process under its //! test override), with the outcome read from its record; @@ -73,6 +75,7 @@ pub mod platform_records; pub mod projection; pub mod projector; pub mod recovery; +pub mod run_graph; pub mod run_store; pub mod runtime; pub mod secrets; diff --git a/lib/components/fabro-petri/src/run_graph.rs b/lib/components/fabro-petri/src/run_graph.rs new file mode 100644 index 000000000..99885a22e --- /dev/null +++ b/lib/components/fabro-petri/src/run_graph.rs @@ -0,0 +1,274 @@ +//! The display graph of a run, read off what Petri admitted. +//! +//! Petri's lowered graph carries the frontend's metadata on every node +//! (`meta`: `label`, `kind`, `edges`, `synthetic`, see the Fabro handoff's +//! "Identities a host can rely on") and the run's goal and workflow name as +//! graph params. [`run_graph`] reduces that to the [`RunGraph`] the read side +//! stores on the run spec: one node per stage the workflow declares, each +//! with its label and handler kind, and one edge per routing arm as written. +//! +//! Lowering artifacts are left out: the goal check before `exit`, the +//! synthetic fan-in before a plain join, and a duplicate branch target's +//! extra delegate. A parallel branch target stays a stage of the graph: in +//! the parent graph it is the branch delegate (`kind = parallel.branch`, +//! `synthetic: true`, `branch = {fork, target}`), so its own kind is read +//! from the child graph the branch runs, where the target keeps its +//! metadata, and the parallel node's edge to it is the delegate's `branch`. + +use std::str::FromStr; + +use fabro_types::{RunGraph, RunGraphEdge, RunGraphNode, StageHandler}; +use petri_runtime::ir::Graph; +use serde_json::Value; + +use crate::check::Admitted; + +/// The graph param the Attractor lowering stores the workflow's name under. +const WORKFLOW_PARAM: &str = "attractor.workflow"; +/// The graph param the run's goal is stored under. +const GOAL_PARAM: &str = "goal"; + +/// The display graph of the admitted workflow. +#[must_use] +pub fn run_graph(admitted: &Admitted) -> RunGraph { + let root = &admitted.graph; + let mut graph = RunGraph::new(param_text(root, WORKFLOW_PARAM)); + graph.goal = param_text(root, GOAL_PARAM).to_string(); + for node in &root.body.nodes { + let meta = &node.meta; + let name = node.name.as_str(); + let kind = if is_synthetic(meta) { + // A branch delegate stands for the parallel node's edge to its + // target, and for the target itself when it is named after it; + // any other synthetic node is a lowering artifact. + let Some((fork, target)) = branch_of(meta) else { + continue; + }; + graph.edges.push(RunGraphEdge { + from: fork.to_string(), + to: target.to_string(), + }); + if target != name { + continue; + } + admitted + .children + .iter() + .flat_map(|child| &child.body.nodes) + .find(|candidate| candidate.name == target && !is_synthetic(&candidate.meta)) + .map_or(StageHandler::Agent, |candidate| kind_of(&candidate.meta)) + } else { + kind_of(meta) + }; + let label = meta + .get("label") + .and_then(Value::as_str) + .filter(|label| !label.is_empty()) + .unwrap_or(name); + graph.nodes.insert(name.to_string(), RunGraphNode { + label: label.to_string(), + kind, + }); + if let Some(edges) = meta.get("edges").and_then(Value::as_object) { + for entry in edges.values() { + if let Some(to) = entry.get("to").and_then(Value::as_str) { + graph.edges.push(RunGraphEdge { + from: name.to_string(), + to: to.to_string(), + }); + } + } + } + } + // The routing arms are keyed by engine edge id in the metadata; order + // them by the nodes they join so the graph reads the same on every + // build. + graph + .edges + .sort_by(|left, right| (&left.from, &left.to).cmp(&(&right.from, &right.to))); + graph +} + +fn param_text<'a>(graph: &'a Graph, name: &str) -> &'a str { + graph + .params + .get(name) + .and_then(Value::as_str) + .unwrap_or_default() +} + +fn is_synthetic(meta: &Value) -> bool { + meta.get("synthetic").and_then(Value::as_bool) == Some(true) +} + +/// The parallel node and the target a branch delegate stands for. +fn branch_of(meta: &Value) -> Option<(&str, &str)> { + let branch = meta.get("branch")?; + Some(( + branch.get("fork")?.as_str()?, + branch.get("target")?.as_str()?, + )) +} + +/// The node's handler kind from its `meta.kind`: the Attractor names are the +/// stage handler names, and anything else runs as an agent, as Fabro's +/// handler resolution has it. +fn kind_of(meta: &Value) -> StageHandler { + let kind = meta.get("kind").and_then(Value::as_str); + kind.and_then(|kind| StageHandler::from_str(kind).ok()) + .unwrap_or_else(|| StageHandler::from_handler_type(kind)) +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + use crate::check::{self, Bundle, CheckRequest}; + + fn admit(files: &[(&str, &str)]) -> Admitted { + let request = CheckRequest { + bundle: Bundle { + files: files + .iter() + .map(|(path, text)| ((*path).to_string(), (*text).to_string())) + .collect::>(), + entrypoint: "workflow.fabro".to_string(), + project_toml: None, + }, + ..CheckRequest::default() + }; + check::check(&request).unwrap_or_else(|err| panic!("the workflow should admit: {err:?}")) + } + + fn kinds(graph: &RunGraph) -> Vec<(&str, StageHandler)> { + graph + .nodes + .iter() + .map(|(id, node)| (id.as_str(), node.kind)) + .collect() + } + + fn edges(graph: &RunGraph) -> Vec<(&str, &str)> { + graph + .edges + .iter() + .map(|edge| (edge.from.as_str(), edge.to.as_str())) + .collect() + } + + #[test] + fn a_linear_workflow_keeps_its_name_goal_stages_and_edges() { + let admitted = admit(&[( + "workflow.fabro", + r#"digraph Ship { + graph [goal="Ship the feature"] + start [shape=Mdiamond, label="Start"] + plan [label="Plan rollout", prompt="Plan"] + deploy [shape=parallelogram, script="make deploy"] + gate [shape=hexagon, label="Ship?"] + exit [shape=Msquare] + start -> plan -> deploy -> gate + gate -> exit [label="[Y] Yes"] + gate -> plan [label="[N] No"] + }"#, + )]); + + let graph = run_graph(&admitted); + + assert_eq!(graph.name, "Ship"); + assert_eq!(graph.goal(), "Ship the feature"); + assert_eq!(kinds(&graph), vec![ + ("deploy", StageHandler::Command), + ("exit", StageHandler::Exit), + ("gate", StageHandler::Human), + ("plan", StageHandler::Agent), + ("start", StageHandler::Start), + ]); + assert_eq!(graph.node("plan").unwrap().label, "Plan rollout"); + assert_eq!(graph.node("deploy").unwrap().label, "deploy"); + assert_eq!(edges(&graph), vec![ + ("deploy", "gate"), + ("gate", "exit"), + ("gate", "plan"), + ("plan", "deploy"), + ("start", "plan"), + ]); + assert!(graph.is_boundary("start")); + assert!(graph.is_boundary("exit")); + } + + #[test] + fn lowering_artifacts_are_left_out_and_branch_targets_keep_their_kind() { + let admitted = admit(&[( + "workflow.fabro", + r#"digraph Parallel { + graph [goal="Fan out"] + start [shape=Mdiamond] + fan_out [shape=component] + lint [shape=parallelogram, script="make lint"] + test [prompt="Run the tests", goal_gate=true, retry_target="lint"] + join [shape=tripleoctagon] + exit [shape=Msquare] + start -> fan_out + fan_out -> lint + fan_out -> test + lint -> join + test -> join + join -> exit + }"#, + )]); + + let graph = run_graph(&admitted); + + assert_eq!(kinds(&graph), vec![ + ("exit", StageHandler::Exit), + ("fan_out", StageHandler::Parallel), + ("join", StageHandler::ParallelFanIn), + ("lint", StageHandler::Command), + ("start", StageHandler::Start), + ("test", StageHandler::Agent), + ]); + assert_eq!(edges(&graph), vec![ + ("fan_out", "lint"), + ("fan_out", "test"), + ("join", "exit"), + ("lint", "join"), + ("start", "fan_out"), + ("test", "join"), + ]); + } + + #[test] + fn prepare_steps_are_stages_of_the_graph_they_run_in() { + let admitted = admit(&[ + ( + "workflow.fabro", + r#"digraph Prepared { + start [shape=Mdiamond] + work [prompt="Work"] + exit [shape=Msquare] + start -> work -> exit + }"#, + ), + ( + "workflow.toml", + "_version = 1\n[run]\ngoal = \"Settings goal\"\n[[run.prepare.steps]]\nscript = \ + \"make setup\"\n", + ), + ]); + + let graph = run_graph(&admitted); + + assert_eq!(graph.goal(), "Settings goal"); + assert_eq!( + graph.node("run_prepare_1").map(|node| node.kind), + Some(StageHandler::Command) + ); + assert_eq!(edges(&graph), vec![ + ("run_prepare_1", "work"), + ("start", "run_prepare_1"), + ("work", "exit"), + ]); + } +} diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index 14cce156e..79b128031 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -644,10 +644,10 @@ mod tests { use chrono::{DateTime, Utc}; use fabro_types::{ - AutomationRef, BlockedReason, Conclusion, DiffSummary, FailureReason, Graph, PendingReason, - PetriAdmission, PullRequestCreationId, RunDiff, RunId, RunProjection, RunSize, RunSpec, - RunStatus, RunStatusKind, RunTiming, StageOutcome, SuccessReason, WorkflowSettings, - test_support, + AutomationRef, BlockedReason, Conclusion, DiffSummary, FailureReason, PendingReason, + PetriAdmission, PullRequestCreationId, RunDiff, RunGraph, RunId, RunProjection, RunSize, + RunSpec, RunStatus, RunStatusKind, RunTiming, StageOutcome, SuccessReason, + WorkflowSettings, test_support, }; use lithos_llm::types::{Cost, CostSource, TokenCounts, Usage}; use strum::VariantArray as _; @@ -678,7 +678,7 @@ mod tests { RunSpec { run_id, settings: WorkflowSettings::default(), - graph: Graph::new("test"), + graph: RunGraph::new("test"), graph_source: None, workflow_slug: Some("test-workflow".to_string()), workflow_version_id: None, diff --git a/lib/components/fabro-store/tests/serializable_projection.rs b/lib/components/fabro-store/tests/serializable_projection.rs index c0171e60c..094f14577 100644 --- a/lib/components/fabro-store/tests/serializable_projection.rs +++ b/lib/components/fabro-store/tests/serializable_projection.rs @@ -2,19 +2,18 @@ use std::collections::{BTreeMap, HashMap}; use chrono::{TimeZone, Utc}; use fabro_store::{RunProjection, SerializableProjection, StageId}; -use fabro_types::graph::Graph; use fabro_types::run::RunSpec; use fabro_types::{ Checkpoint, CheckpointRecord, InterviewQuestionRecord, ModelUsage, ParallelBranchResult, - QuestionType, RunDiff, RunSandbox, RunSandboxInstance, RunSandboxPlan, RunSandboxRuntime, - RunStatus, SandboxProviderKind, StageCompletion, StageModelUsage, StageOutcome, StartRecord, - first_event_seq, fixtures, test_support, + QuestionType, RunDiff, RunGraph, RunSandbox, RunSandboxInstance, RunSandboxPlan, + RunSandboxRuntime, RunStatus, SandboxProviderKind, StageCompletion, StageModelUsage, + StageOutcome, StartRecord, first_event_seq, fixtures, test_support, }; use serde_json::json; fn sample_run_spec() -> RunSpec { RunSpec { - graph: Graph::new("ship"), + graph: RunGraph::new("ship"), workflow_slug: Some("demo".to_string()), source_directory: Some("/tmp/project".to_string()), labels: HashMap::from([("team".to_string(), "platform".to_string())]), diff --git a/lib/components/fabro-workflow/Cargo.toml b/lib/components/fabro-workflow/Cargo.toml index 40b5b72cc..376415cb0 100644 --- a/lib/components/fabro-workflow/Cargo.toml +++ b/lib/components/fabro-workflow/Cargo.toml @@ -28,7 +28,6 @@ fabro-sandbox = { path = "../fabro-sandbox" } sandbox-driver.workspace = true pebble-coding-agent.workspace = true fabro-github = { path = "../fabro-github" } -fabro-template = { path = "../../foundation/fabro-template" } fabro-tool = { path = "../fabro-tool" } fabro-util = { path = "../../foundation/fabro-util" } fabro-redact.workspace = true @@ -48,7 +47,6 @@ chrono = { workspace = true, features = ["serde"] } dirs = "6" scopeguard = "1" hex.workspace = true -miette.workspace = true git2.workspace = true tracing.workspace = true tempfile = "3" diff --git a/lib/components/fabro-workflow/README.md b/lib/components/fabro-workflow/README.md index 717fe6806..fa3698ea5 100644 --- a/lib/components/fabro-workflow/README.md +++ b/lib/components/fabro-workflow/README.md @@ -1,159 +1,28 @@ # fabro-workflow -A DOT-based pipeline runner for multi-stage AI workflows. Define workflows as Graphviz `digraph` files and execute them with pluggable handlers, conditional routing, human-in-the-loop gates, parallel branching, retry policies, and checkpoint-based recovery. +Fabro's platform half of a workflow run: what Fabro does around the engine. -## Key Concepts +Petri compiles and executes every run. `fabro-petri` is the one crate that +talks to it, and this crate keeps what Fabro itself owns: -- **Graph** -- A directed graph parsed from DOT syntax containing nodes, edges, and attributes. The graph carries a `goal` describing the pipeline's purpose. -- **Node** -- A workflow step. Graphviz shapes map to handler types (e.g., `Mdiamond` = start, `Msquare` = exit, `box` = agent, `tab` = prompt, `diamond` = conditional, `hexagon` = human gate, `component` = parallel). -- **Edge** -- A connection between nodes with optional `condition`, `label`, `weight`, and `fidelity` attributes that control routing. -- **Handler** -- An async trait implementation that executes a node and returns an `Outcome`. Built-in handlers include `StartHandler`, `ExitHandler`, `AgentHandler`, `PromptHandler`, `ConditionalHandler`, `HumanHandler`, `ParallelHandler`, `FanInHandler`, `CommandHandler`, and `SubWorkflowHandler`. -- **Outcome** -- The result of executing a handler, carrying a `StageOutcome` (Success, Fail, PartialSuccess, Retry, Skipped), optional routing hints (`preferred_label`, `suggested_next_ids`), and context updates. -- **Context** -- A thread-safe key-value store shared across pipeline stages, supporting snapshots and isolated cloning for parallel branches. -- **Interviewer** -- A trait for human-in-the-loop interactions. Implementations include `AutoApproveInterviewer` and `ControlInterviewer`. -- **Checkpoint** -- A serializable snapshot of execution state (completed nodes, context values) for crash recovery and resume. +- **`operations`** — creating a run around Petri's admission + (`materialize_admitted_run`, `persist_create_run`), and the other run + operations: fork, rewind, retry, and the timeline they resolve targets on. + The run's display graph (`fabro_types::RunGraph`) is read off the graph + Petri admitted; the DOT the workflow was written in is persisted beside it + as `graph_source`. +- **`workflow_bundle`** — the bundle a run is created from: every workflow + of the version closure with its settings file and its files, and the + `RunDefinition` the run records. +- **`git`**, **`sandbox_git`** — the Git helpers a run's platform effects use, + on the host and inside a sandbox. +- **`pull_request`** — pull request creation for a finished run. +- **`run_tools`**, **`services`** — the run tools an agent session calls. +- **`web_search`** — the built-in web search backend. +- **`run_lookup`** — resolving a run selector to a run. -## Pipeline Definition - -Pipelines are defined using Graphviz DOT syntax: - -```dot -digraph MyPipeline { - graph [goal="Implement and validate a feature"] - rankdir=LR - node [shape=box, timeout="900s"] - - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - plan [label="Plan", prompt="Plan the implementation"] - implement [label="Implement", prompt="Implement the plan"] - validate [label="Validate", prompt="Run tests"] - gate [shape=diamond, label="Tests passing?"] - - start -> plan -> implement -> validate -> gate - gate -> exit [label="Yes", condition="outcome=succeeded"] - gate -> implement [label="No", condition="outcome!=succeeded"] -} -``` - -## Usage - -### Parsing and Validating a Pipeline - -```rust -use fabro_workflow::operations::{create, CreateOptions}; - -let dot_source = r#"digraph Simple { - graph [goal="Run tests"] - start [shape=Mdiamond] - exit [shape=Msquare] - work [shape=box, prompt="Run the test suite"] - start -> work -> exit -}"#; - -let validated = create(dot_source, CreateOptions::default()) - .expect("pipeline should parse"); -validated.raise_on_errors().expect("pipeline should validate"); -let (graph, _, _) = validated.into_parts(); -assert_eq!(graph.name, "Simple"); -assert_eq!(graph.goal(), "Run tests"); -``` - -`operations::create` parses the DOT source, applies built-in transforms (variable expansion, stylesheet application, preamble injection), and returns diagnostics through `Validated`. - -### Running a Pipeline - -```rust -use fabro_workflow::operations::start; -use fabro_workflow::pipeline; - -// Use `operations::start(...)` for the full -// initialize -> execute -> conclude -> publish -> finalize flow. -// Use `pipeline::initialize(...)` + `pipeline::execute(...)` when you need partial lifecycle control. -``` - -### Custom Handlers - -Implement the `Handler` trait to add custom node behavior: - -```rust -use arc_workflows::handler::Handler; -use arc_workflows::context::Context; -use arc_workflows::graph::{Graph, Node}; -use arc_workflows::outcome::Outcome; -use arc_workflows::error::ArcError; -use async_trait::async_trait; -use std::path::Path; - -struct MyHandler; - -#[async_trait] -impl Handler for MyHandler { - async fn execute( - &self, - node: &Node, - context: &Context, - graph: &Graph, - run_dir: &Path, - ) -> Result { - // Custom logic here - Ok(Outcome::success()) - } -} -``` - -### Model Stylesheets - -CSS-like stylesheets control LLM model assignment with specificity-based cascading: - -```dot -digraph Styled { - graph [ - goal="Build feature", - model_stylesheet=" - * { model: claude-sonnet-4-5;} - .code { model: claude-opus-4-6; } - #critical_review { model: gpt-5.2;} - " - ] - // ... -} -``` - -Selectors by specificity: `*` (universal, 0) < `shape` (1) < `.class` (2) < `#id` (3). Explicit node attributes are never overridden. - -### Condition Expressions - -Edge conditions use a simple expression syntax for routing: - -``` -outcome=succeeded -outcome!=failed -outcome=succeeded && context.tests_passed=true -my_flag -``` - -Clauses support `=`, `!=`, and bare key truthiness checks, joined with `&&`. - -### Human-in-the-Loop Gates - -Nodes with `shape=hexagon` or `type="human"` pause execution for human input. Outgoing edge labels become selectable options, with accelerator key parsing for patterns like `[A] Approve` and `F) Fix`. - -### Parallel Execution - -Nodes with `shape=component` fan out to branches concurrently. Branches receive isolated context forks, share the same sandbox checkout, and always finish before the workflow continues. Use `max_parallel` to limit concurrency; concurrent workspace writes are user-managed. - -### Checkpoints and Resume - -The engine saves a checkpoint after each node. Resume from a checkpoint with `engine.run_from_checkpoint(&graph, &config, &checkpoint)`. - -## Architecture - -``` -parser (DOT -> AST -> Graph) - -> transform (variable expansion, stylesheet, preamble) - -> validation (14 lint rules) - -> engine (execution loop with retry, edge selection, goal gates) - -> handler (pluggable node executors) - -> interviewer (human-in-the-loop I/O) -``` +The run records and status vocabulary are `fabro_types`'. Workflow +diagnostics are Petri's: `fabro validate`, `fabro preflight` and the create +handler report Petri's codes (`attractor.*`, `unsupported.*`, `deprecated.*`, +`info.*`), plus Fabro's `fabro.model.no_ready_provider` when a model node has +no provider ready to run it. diff --git a/lib/components/fabro-workflow/src/error.rs b/lib/components/fabro-workflow/src/error.rs index eb7b0ed61..c38ca8ce5 100644 --- a/lib/components/fabro-workflow/src/error.rs +++ b/lib/components/fabro-workflow/src/error.rs @@ -1,64 +1,8 @@ -use std::fmt; -use std::sync::Arc; - use fabro_graphviz::Error as GraphvizError; -use fabro_template::TemplateError; use fabro_types::diagnostic::Diagnostic; -use fabro_types::settings::ResolveError; use fabro_util::error::{SharedError, collect_chain, render_with_causes}; use thiserror::Error as ThisError; -/// A template error shared across clones of the workflow error that carries -/// it, so the miette diagnostic and the source chain survive cloning. -#[derive(Debug, Clone)] -pub struct SharedTemplateError(Arc); - -impl SharedTemplateError { - #[must_use] - pub fn new(error: TemplateError) -> Self { - Self(Arc::new(error)) - } - - #[must_use] - pub fn inner(&self) -> &TemplateError { - &self.0 - } -} - -impl fmt::Display for SharedTemplateError { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&*self.0, formatter) - } -} - -impl std::error::Error for SharedTemplateError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - std::error::Error::source(&*self.0) - } -} - -impl miette::Diagnostic for SharedTemplateError { - fn code<'a>(&'a self) -> Option> { - miette::Diagnostic::code(&*self.0) - } - - fn help<'a>(&'a self) -> Option> { - miette::Diagnostic::help(&*self.0) - } - - fn source_code(&self) -> Option<&dyn miette::SourceCode> { - miette::Diagnostic::source_code(&*self.0) - } - - fn labels(&self) -> Option + '_>> { - miette::Diagnostic::labels(&*self.0) - } - - fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - miette::Diagnostic::diagnostic_source(&*self.0) - } -} - #[derive(ThisError, Debug, Clone)] pub enum Error { #[error("Parse error: {0}")] @@ -70,21 +14,6 @@ pub enum Error { #[error("Validation failed")] ValidationFailed { diagnostics: Vec }, - #[error("Validation error: script interpolation failed in {owner}: {source} ({fix})")] - ScriptInterpolation { - owner: String, - fix: String, - #[source] - source: ResolveError, - }, - - #[error("{message}")] - Template { - message: String, - #[source] - source: SharedTemplateError, - }, - /// Fabro's own platform work around a run failed: a store call, a /// serialization, a spawned task, a Git command. #[error("Engine error: {message}")] @@ -94,9 +23,6 @@ pub enum Error { source: Option, }, - #[error("Stylesheet error: {0}")] - Stylesheet(String), - #[error("I/O error: {0}")] Io(String), @@ -111,13 +37,6 @@ pub enum Error { } impl Error { - pub fn template(message: impl Into, source: TemplateError) -> Self { - Self::Template { - message: message.into(), - source: SharedTemplateError::new(source), - } - } - pub fn engine(message: impl Into) -> Self { Self::Engine { message: message.into(), @@ -145,8 +64,6 @@ impl Error { Self::Engine { source, .. } => source .as_ref() .map_or_else(Vec::new, |source| collect_chain(source)), - Self::Template { source, .. } => collect_chain(source), - Self::ScriptInterpolation { source, .. } => collect_chain(source), _ => Vec::new(), } } @@ -157,43 +74,6 @@ impl Error { } } -impl miette::Diagnostic for Error { - fn code<'a>(&'a self) -> Option> { - match self { - Self::Template { source, .. } => miette::Diagnostic::code(source), - _ => None, - } - } - - fn help<'a>(&'a self) -> Option> { - match self { - Self::Template { source, .. } => miette::Diagnostic::help(source), - _ => None, - } - } - - fn source_code(&self) -> Option<&dyn miette::SourceCode> { - match self { - Self::Template { source, .. } => miette::Diagnostic::source_code(source), - _ => None, - } - } - - fn labels(&self) -> Option + '_>> { - match self { - Self::Template { source, .. } => miette::Diagnostic::labels(source), - _ => None, - } - } - - fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - match self { - Self::Template { source, .. } => Some(source), - _ => None, - } - } -} - impl From for Error { fn from(err: std::io::Error) -> Self { Self::Io(err.to_string()) @@ -204,7 +84,6 @@ impl From for Error { fn from(e: GraphvizError) -> Self { match e { GraphvizError::Parse(msg) => Self::Parse(msg), - GraphvizError::Stylesheet(msg) => Self::Stylesheet(msg), } } } @@ -273,30 +152,6 @@ mod tests { assert_eq!(err.to_string(), "Validation failed"); } - #[test] - fn template_error_variant_preserves_source_chain() { - let template_err = fabro_template::render_named( - "workflow.fabro", - "{{ inputs.missing }}", - &fabro_template::TemplateContext::new(), - ) - .unwrap_err(); - - let err = Error::template("template expansion failed", template_err); - let chain = collect_chain(&err); - - assert!( - chain - .iter() - .any(|part| part.contains("template expansion failed")) - ); - assert!( - chain - .iter() - .any(|part| part.contains("undefined template variable")) - ); - } - #[test] fn engine_error_display() { let err = Error::engine("no outgoing edge"); @@ -373,7 +228,6 @@ mod tests { }, Error::engine("engine err"), Error::engine_with_source("engine err", TestCause("cause")), - Error::Stylesheet("style err".into()), Error::Io("io err".into()), Error::Precondition("precondition".into()), Error::RunNotFound("run".into()), diff --git a/lib/components/fabro-workflow/src/lib.rs b/lib/components/fabro-workflow/src/lib.rs index 1b90af1cb..bc935253d 100644 --- a/lib/components/fabro-workflow/src/lib.rs +++ b/lib/components/fabro-workflow/src/lib.rs @@ -1,14 +1,14 @@ //! Fabro's platform half of a workflow run: what Fabro does around the //! engine. //! -//! Petri executes every run (`fabro-petri` is the seam). This crate keeps -//! what Fabro itself owns: the create-time compile of the Fabro graph the -//! read side displays (`pipeline`, `transforms`, `operations`), the Git -//! helpers a run's platform effects use (`git`, `sandbox_git`), pull -//! request creation (`pull_request`), the run tools an agent session calls -//! (`run_tools`, `services`), the built-in web search backend -//! (`web_search`). The run records and status vocabulary are -//! `fabro_types`'. +//! Petri compiles and executes every run (`fabro-petri` is the seam). This +//! crate keeps what Fabro itself owns: the run's creation around Petri's +//! admission and the other run operations (`operations`), the bundle a run +//! is created from (`workflow_bundle`), the Git helpers a run's platform +//! effects use (`git`, `sandbox_git`), pull request creation +//! (`pull_request`), the run tools an agent session calls (`run_tools`, +//! `services`), the built-in web search backend (`web_search`). The run +//! records and status vocabulary are `fabro_types`'. #![cfg_attr( test, @@ -28,20 +28,15 @@ )] pub mod error; -pub mod file_resolver; pub mod git; pub mod operations; -pub mod pipeline; pub mod pull_request; pub mod run_lookup; pub use error::{Error, Result}; pub use fabro_types::ManifestPath; -pub mod run_materialization; pub mod run_tools; pub mod sandbox_git; pub mod services; -#[doc(hidden)] -pub mod transforms; pub mod web_search; pub mod workflow_bundle; diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 3bac02edf..3455ae490 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -2,46 +2,56 @@ test, expect( clippy::disallowed_methods, - reason = "tests write workflow fixture files synchronously before exercising async creation" + reason = "tests write a goal file synchronously before exercising creation" ) )] +//! Creating a run Petri admitted: the settings Fabro layered, the display +//! graph read off the admitted workflow, and the run's first records. +//! +//! Petri compiles the workflow (`fabro-petri`'s check) and its admission is +//! the graph the run executes. What Fabro adds at create is its own: the +//! run's goal and pull request settings materialized into the resolved +//! settings, the labels, the workflow slug, the bundle the run was created +//! from, and the durable `run.created` and `submitted` records. + use std::collections::HashMap; use std::path::{Path, PathBuf}; use fabro_config::Storage; -use fabro_graphviz::graph::{AttrValue, Graph}; +use fabro_config::project::{resolve_working_directory_from_run, workflow_slug_from_path}; +use fabro_config::run::resolve_run_goal_from_namespace; use fabro_store::platform_records::{ PlatformRecord, RunCreatedRecord, RunLifecycleKind, RunLifecycleRecord, }; use fabro_store::{BlobStore, Database}; -use fabro_template::TemplateContext; +use fabro_types::settings::InterpString; +use fabro_types::settings::run::RunGoal; use fabro_types::{ - AutomationRef, BlobHash, ForkSourceRef, GitContext, ManifestPath, PetriAdmission, RunId, - RunProvenance, RunSpec, RunStatus, RunTarget, WorkflowSettings, WorkflowVersionId, + AutomationRef, BlobHash, ForkSourceRef, GitContext, ManifestPath, PetriAdmission, RunGraph, + RunId, RunProvenance, RunSpec, RunStatus, RunTarget, WorkflowSettings, WorkflowVersionId, }; use tokio::task::spawn_blocking; -use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow}; use crate::error::Error; -use crate::pipeline::types::PersistOptions; -use crate::pipeline::{self, Persisted, TransformOptions, Validated}; -use crate::run_materialization; -use crate::transforms::RenderMode; use crate::workflow_bundle::{RunDefinition, WorkflowBundle}; -/// Inputs needed to resolve and compile a workflow for run creation. +/// What Petri admitted for a run, as Fabro materializes it: the settings +/// Fabro layered and substituted, the display graph read off the admitted +/// workflow, the entrypoint's DOT, and the bundle it came from. #[derive(Debug)] -pub struct CreateRunCompileInput { - pub workflow: WorkflowInput, +pub struct AdmittedRunInput { pub settings: WorkflowSettings, - pub vars: HashMap, pub cwd: PathBuf, - pub workflow_path: Option, - pub workflow_bundle: Option, + pub graph: RunGraph, + /// The entrypoint's DOT as written, persisted as the run's + /// `graph_source`. + pub source: String, + pub workflow_path: ManifestPath, + pub workflow_bundle: WorkflowBundle, } -/// Durable metadata joined to a materialized workflow before persistence. +/// Durable metadata joined to a materialized run before persistence. /// `run_id` is already resolved, and `storage_root` is used to derive the /// run's scratch directory during pure input assembly. #[derive(Debug)] @@ -62,47 +72,27 @@ pub struct CreateRunPersistenceMetadata { pub admission: PetriAdmission, } +/// The run as persisted: its spec, the DOT it displays, and its scratch +/// directory. #[derive(Debug)] pub struct CreatedRun { - pub persisted: Persisted, - pub run_id: RunId, - pub run_dir: PathBuf, - pub dot_path: Option, + pub spec: RunSpec, + /// The entrypoint's DOT, the same text as `spec.graph_source`. + pub source: String, + pub run_id: RunId, + pub run_dir: PathBuf, } -/// Result of resolving, preprocessing, validating, and promoting a workflow -/// for run creation. Model selectors in the graph are resolved, while the run -/// settings still reflect the compiled source and have not been materialized. -pub struct CompiledRun { - validated: Validated, - settings: WorkflowSettings, - raw_source: String, - workflow_slug: Option, - dot_path: Option, - definition: Option, - source_directory: String, - labels: HashMap, -} - -impl CompiledRun { - pub fn validated(&self) -> &Validated { - &self.validated - } - - pub fn settings(&self) -> &WorkflowSettings { - &self.settings - } -} - -/// Compiled workflow with its run-level model settings materialized against -/// the same provider snapshot used during compilation. +/// The admitted run with Fabro's run-level settings materialized: the goal +/// the run displays and runs under, and a pull request block the settings +/// disable dropped. +#[derive(Debug)] pub struct MaterializedRun { - validated: Validated, settings: WorkflowSettings, - raw_source: String, + graph: RunGraph, + source: String, workflow_slug: Option, - dot_path: Option, - definition: Option, + definition: RunDefinition, source_directory: String, labels: HashMap, } @@ -111,6 +101,10 @@ impl MaterializedRun { pub fn settings(&self) -> &WorkflowSettings { &self.settings } + + pub fn graph(&self) -> &RunGraph { + &self.graph + } } /// Complete input for creating a durable run. The run ID and run directory @@ -157,101 +151,59 @@ impl CreateRunPersistenceInput { self.automation.as_ref() } - pub fn definition(&self) -> Option<&RunDefinition> { - self.materialized.definition.as_ref() + pub fn definition(&self) -> &RunDefinition { + &self.materialized.definition } } -/// Stage two for a run another engine admitted: the Fabro graph is parsed -/// and transformed for the read side (the goal, the node count, labels), with -/// no lint rule, no model resolution and no promotion of template -/// diagnostics. The engine that admitted the run judged the workflow; a -/// graph Fabro's own parser cannot read is still refused, since the read side -/// needs one. -pub fn compile_admitted_run(input: CreateRunCompileInput) -> Result { - let CreateRunCompileInput { - workflow, - settings, - vars, +/// Materialize Fabro's run-level settings around the admitted graph. +/// +/// The goal is the settings' `run.goal` when the run has one (the API's +/// goal, or a `[run.goal]` layer, its `file` form read at the working +/// directory), else the goal Petri admitted; it becomes both the graph's +/// displayed goal and the inline `run.goal`, and stays absent when the +/// workflow has none. A pull request block the settings disable is dropped. +pub fn materialize_admitted_run(input: AdmittedRunInput) -> Result { + let AdmittedRunInput { + mut settings, cwd, + mut graph, + source, workflow_path, workflow_bundle, } = input; - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow, - settings, - cwd, - }) - .map_err(|err| Error::Parse(err.to_string()))?; - let settings = resolved.settings; - let labels = settings.combined_labels(); - let definition = match (workflow_path, workflow_bundle) { - (Some(workflow_path), Some(workflow_bundle)) => { - Some(RunDefinition::new(workflow_path, workflow_bundle)) - } - _ => None, - }; - let mut parsed = pipeline::parse(&resolved.raw_source)?; - apply_goal_override(&mut parsed.graph, resolved.goal_override.as_deref()); - let transformed = pipeline::transform(parsed, &TransformOptions { - current_dir: resolved.current_dir.clone(), - file_resolver: resolved.file_resolver.clone(), - template_context: template_context(Some(&settings), vars), - source_name: resolved - .dot_path - .as_ref() - .map(|path| path.display().to_string()), - render_mode: RenderMode::Structural, - custom_transforms: Vec::new(), - })?; - let validated = Validated::new( - transformed.graph, - transformed.source, - transformed.diagnostics, - ); - Ok(CompiledRun { - validated, - settings, - raw_source: resolved.raw_source, - workflow_slug: resolved.workflow_slug, - dot_path: resolved.dot_path, - definition, - source_directory: resolved.working_directory.to_string_lossy().to_string(), - labels, - }) -} - -/// Stage three for a run Petri admitted: no model pinning, since Petri -/// pinned every route at its admission. The run's goal and pull request -/// settings are materialized as they were for every run: the graph's goal -/// becomes the inline `run.goal`, and a disabled pull request block is -/// dropped. -#[must_use] -pub fn materialize_admitted_run(compiled: CompiledRun) -> MaterializedRun { - let CompiledRun { - validated, - mut settings, - raw_source, - workflow_slug, - dot_path, - definition, - source_directory, - labels, - } = compiled; - run_materialization::materialize_goal_and_pull_request(&mut settings, validated.graph()); - MaterializedRun { - validated, - settings, - raw_source, - workflow_slug, - dot_path, - definition, - source_directory, - labels, + let working_directory = resolve_working_directory_from_run(&settings.run, &cwd); + if let Some(resolved) = resolve_run_goal_from_namespace(&settings.run, &working_directory) + .map_err(|err| Error::Parse(err.to_string()))? + { + graph.goal = resolved.text; } + settings.run.goal = if graph.goal.is_empty() { + None + } else { + Some(RunGoal::Inline(InterpString::parse(&graph.goal))) + }; + if settings + .run + .pull_request + .as_ref() + .is_some_and(|pull_request| !pull_request.enabled) + { + settings.run.pull_request = None; + } + let labels = settings.combined_labels(); + Ok(MaterializedRun { + settings, + graph, + source, + workflow_slug: workflow_slug_from_path(workflow_path.as_path()), + definition: RunDefinition::new(workflow_path, workflow_bundle), + source_directory: working_directory.to_string_lossy().into_owned(), + labels, + }) } -/// Assemble all inputs needed for persistence without I/O or recompilation. +/// Assemble all inputs needed for persistence without I/O. pub fn assemble_create_run_persistence_input( materialized: MaterializedRun, metadata: CreateRunPersistenceMetadata, @@ -295,7 +247,8 @@ pub fn assemble_create_run_persistence_input( } } -/// Persist one already-compiled and materialized run without recompiling it. +/// Persist one materialized run: its scratch directory, then its first +/// records. pub async fn persist_create_run( store: &Database, input: CreateRunPersistenceInput, @@ -317,11 +270,10 @@ pub async fn persist_create_run( admission, } = input; let MaterializedRun { - validated, settings, - raw_source, + graph, + source, workflow_slug: _, - dot_path, definition, source_directory, labels, @@ -331,84 +283,81 @@ pub async fn persist_create_run( Some(RunTarget::Folder { path }) => (Some(path.clone()), git), Some(RunTarget::Git(_)) | None => (Some(source_directory), git), }; - let persisted_run_dir = run_dir.clone(); - let persisted = spawn_blocking(move || { - let run_spec = RunSpec { - run_id, - settings, - graph: validated.graph().clone(), - graph_source: Some(validated.source().to_string()), - workflow_slug, - workflow_version_id, - target, - automation, - source_directory, - labels, - provenance, - definition_blob: None, - spec_blob: None, - git, - fork_source_ref, - admission, - }; - pipeline::persist(validated, PersistOptions { - run_dir: persisted_run_dir, - run_spec, + let spec = RunSpec { + run_id, + settings, + graph, + graph_source: Some(source.clone()), + workflow_slug, + workflow_version_id, + target, + automation, + source_directory, + labels, + provenance, + definition_blob: None, + spec_blob: None, + git, + fork_source_ref, + admission, + }; + let scratch = run_dir.clone(); + spawn_blocking(move || { + std::fs::create_dir_all(&scratch).map_err(|err| { + Error::Io(format!( + "creating run directory {}: {err}", + scratch.display() + )) }) }) .await .map_err(|err| Error::engine_with_source("workflow create task failed", err))??; - persist_created_run( + let spec = Box::pin(persist_created_run( store, - &persisted, - &raw_source, - definition.as_ref(), + spec, + &definition, title, parent_id, web_url, - ) + )) .await?; Ok(CreatedRun { - persisted, + spec, + source, run_id, run_dir, - dot_path, }) } /// The run's first records: `run.created` with the spec Fabro built, and /// the `submitted` lifecycle transition. Both wake the run's projector. +/// Returns the spec as recorded, naming its definition and spec blobs. async fn persist_created_run( store: &Database, - persisted: &Persisted, - workflow_source: &str, - accepted_definition: Option<&RunDefinition>, + mut spec: RunSpec, + definition: &RunDefinition, explicit_title: Option, parent_id: Option, web_url: Option, -) -> Result<(), Error> { - let record = persisted.run_spec(); - let definition_bytes = accepted_definition - .map(serde_json::to_vec) - .transpose() +) -> Result { + let definition_bytes = serde_json::to_vec(definition) .map_err(|err| Error::engine_with_source("failed to serialize run definition", err))?; - let spec_bytes = serde_json::to_vec(record) + let spec_bytes = serde_json::to_vec(&spec) .map_err(|err| Error::engine_with_source("failed to serialize run spec", err))?; let blob_store = store.blobs(); let (definition_blob, spec_blob) = tokio::try_join!( - write_optional_blob(&blob_store, definition_bytes.as_deref()), - async { blob_store.write(&spec_bytes).await.map_err(store_error) }, + write_blob(&blob_store, &definition_bytes), + write_blob(&blob_store, &spec_bytes), )?; - let _ = workflow_source; - let title = explicit_title.unwrap_or_else(|| fabro_types::infer_run_title(record.graph.goal())); - let mut spec = record.clone(); - spec.definition_blob = definition_blob; + let title = explicit_title.unwrap_or_else(|| fabro_types::infer_run_title(spec.graph.goal())); + spec.definition_blob = Some(definition_blob); spec.spec_blob = Some(spec_blob); + let run_id = spec.run_id; let created = PlatformRecord::RunCreated(RunCreatedRecord { - spec, + spec: spec.clone(), title: Some(title), parent_id, retried_from: None, @@ -421,66 +370,22 @@ async fn persist_created_run( let platform_records = summaries.platform_records(); for platform_record in [created, submitted] { platform_records - .append(&record.run_id, &platform_record, None) + .append(&run_id, &platform_record, None) .await .map_err(store_error)?; } - summaries.notify_platform_record(record.run_id); - Ok(()) + summaries.notify_platform_record(run_id); + Ok(spec) } -async fn write_optional_blob( - blob_store: &BlobStore, - bytes: Option<&[u8]>, -) -> Result, Error> { - match bytes { - Some(bytes) => blob_store.write(bytes).await.map(Some).map_err(store_error), - None => Ok(None), - } +async fn write_blob(blob_store: &BlobStore, bytes: &[u8]) -> Result { + blob_store.write(bytes).await.map_err(store_error) } fn store_error(err: impl Into) -> Error { Error::engine_with_source("run store operation failed", err) } -/// Parse and transform `dot_source`, and carry the transform diagnostics -/// as the structural validation. Models and lint are Petri's at admission. -pub(super) fn preprocess_and_validate( - dot_source: &str, - goal_override: Option<&str>, - options: &TransformOptions, -) -> Result { - let mut parsed = pipeline::parse(dot_source)?; - apply_goal_override(&mut parsed.graph, goal_override); - - let transformed = pipeline::transform(parsed, options)?; - Ok(pipeline::validate(transformed)) -} - -pub(super) fn template_context( - settings: Option<&WorkflowSettings>, - vars: HashMap, -) -> TemplateContext { - TemplateContext::new() - .with_inputs(run_inputs(settings)) - .with_vars(vars) -} - -fn run_inputs(settings: Option<&WorkflowSettings>) -> HashMap { - settings - .map(|settings| settings.run.inputs.clone()) - .unwrap_or_default() -} - -fn apply_goal_override(graph: &mut Graph, goal_override: Option<&str>) { - if let Some(goal_override) = goal_override { - graph.attrs.insert( - "goal".to_string(), - AttrValue::String(goal_override.to_string()), - ); - } -} - pub fn make_run_dir(scratch_base: &Path, run_id: &RunId) -> PathBuf { fabro_config::RunScratch::for_run(scratch_base, run_id) .root() @@ -491,64 +396,37 @@ pub fn make_run_dir(scratch_base: &Path, run_id: &RunId) -> PathBuf { mod tests { use std::sync::Arc; - use chrono::{Local, TimeZone, Utc}; - use fabro_config::{ - PrepareStep, ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, - RunPrepareLayer, RunPullRequestLayer, WorkflowSettingsBuilder, - }; - use fabro_graphviz::graph::AttrValue; - use fabro_store::Database; + use fabro_config::{RunLayer, WorkflowSettingsBuilder}; use fabro_store::platform_records::StoredPlatformRecord; - use fabro_types::diagnostic::Severity; - use fabro_types::settings::InterpString; - use fabro_types::settings::run::RunMode; - use fabro_types::{PetriAdmission, WorkflowSettings, fixtures, test_support}; - use fabro_util::error::collect_chain; + use fabro_types::settings::interp::ResolveCtx; + use fabro_types::settings::run::PullRequestSettings; + use fabro_types::{RunGraphNode, StageHandler, fixtures, test_support}; use super::*; - use crate::file_resolver::FileResolver; - use crate::operations::{ValidateInput, validate}; - use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE}; - use crate::transforms::Transform; use crate::workflow_bundle::BundledWorkflow; - /// The platform records the create operation appended for the run. - async fn platform_records(store: &Database, run_id: RunId) -> Vec { - store - .run_summary_store() - .platform_records() - .read(&run_id) - .await - .unwrap() + + const DOT: &str = r#"digraph Test { + graph [goal="Graph goal"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#; + + fn workflow_path() -> ManifestPath { + ManifestPath::from_wire("flows/ship.fabro").unwrap() } - /// The `run.created` record of the run. - fn run_created(records: &[StoredPlatformRecord]) -> &RunCreatedRecord { - records - .iter() - .find_map(|stored| match &stored.record { - PlatformRecord::RunCreated(created) => Some(created), - _ => None, - }) - .expect("run.created record should be persisted") + fn graph(goal: &str) -> RunGraph { + let mut graph = RunGraph::new("Test"); + graph.goal = goal.to_string(); + graph.nodes.insert("start".to_string(), RunGraphNode { + label: "start".to_string(), + kind: StageHandler::Start, + }); + graph } - /// The status the run's last lifecycle transition leads to. - fn last_lifecycle_status(records: &[StoredPlatformRecord]) -> Option { - records - .iter() - .rev() - .find_map(|stored| match &stored.record { - PlatformRecord::RunLifecycle(lifecycle) => Some(lifecycle.status), - _ => None, - }) - .flatten() - } - - fn memory_store() -> Arc { - Arc::new(fabro_store::test_support::test_database()) - } - - fn settings_from_run_layer(run: RunLayer) -> WorkflowSettings { + fn settings(run: RunLayer) -> WorkflowSettings { WorkflowSettingsBuilder::new() .server_manifest_defaults( RunLayer::default(), @@ -559,1093 +437,153 @@ mod tests { .expect("settings should resolve") } - fn test_default_settings() -> WorkflowSettings { - WorkflowSettingsBuilder::new() - .server_manifest_defaults( - RunLayer::default(), - fabro_environment::seeded_catalog_layer(), - ) - .build() - .expect("default settings should resolve") - } - - /// A run to create, as the server's compiler hands it to the staged - /// create pipeline: the tests drive the same stages in one call. - #[derive(Clone)] - struct CreateRunInput { - workflow: WorkflowInput, - settings: WorkflowSettings, - vars: HashMap, - cwd: PathBuf, - workflow_slug: Option, - workflow_path: Option, - workflow_bundle: Option, - target: Option, - run_id: Option, - title: Option, - automation: Option, - git: Option, - fork_source_ref: Option, - parent_id: Option, - provenance: RunProvenance, - web_url: Option, - admission: PetriAdmission, - } - - impl CreateRunInput { - fn into_stages( - self, - run_id: RunId, - storage_root: PathBuf, - ) -> (CreateRunCompileInput, CreateRunPersistenceMetadata) { - let Self { - workflow, - settings, - vars, - cwd, - workflow_slug, - workflow_path, - workflow_bundle, - target, - run_id: _, - title, - automation, - git, - fork_source_ref, - parent_id, - provenance, - web_url, - admission, - } = self; - ( - CreateRunCompileInput { - workflow, - settings, - vars, - cwd, - workflow_path, - workflow_bundle, - }, - CreateRunPersistenceMetadata { - run_id, - storage_root, - workflow_slug, - workflow_version_id: None, - target, - title, - automation, - git, - fork_source_ref, - parent_id, - provenance, - web_url, - admission, - }, - ) - } - } - - /// Compile, materialize, assemble and persist `request`: the create - /// pipeline as the server drives it for a run Petri admitted. - async fn create( - store: &Database, - request: CreateRunInput, - storage_root: PathBuf, - ) -> Result { - let run_id = request.run_id.unwrap_or_default(); - let (compile_input, metadata) = request.into_stages(run_id, storage_root); - let compiled = compile_admitted_run(compile_input)?; - let materialized = materialize_admitted_run(compiled); - let input = assemble_create_run_persistence_input(materialized, metadata); - Box::pin(persist_create_run(store, input)).await - } - - fn compile_input(request: &CreateRunInput) -> CreateRunCompileInput { - let (compile_input, _) = request - .clone() - .into_stages(RunId::new(), PathBuf::from("/tmp/storage")); - compile_input - } - - fn persistence_metadata( - request: &CreateRunInput, - run_id: RunId, - storage_root: &Path, - ) -> CreateRunPersistenceMetadata { - let (_, metadata) = request - .clone() - .into_stages(run_id, storage_root.to_path_buf()); - metadata - } - - fn validate_dot(dot_source: &str, settings: WorkflowSettings) -> Validated { - validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: dot_source.to_string(), - base_dir: None, - }, - settings, - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: Vec::new(), - }) - .unwrap() - } - - /// Drive the create-time pipeline with an explicit variable snapshot, the - /// way the server does (`Structural` render mode, undefined vars promoted - /// to errors at run-create). - fn validate_dot_with_vars(dot_source: &str, vars: HashMap) -> Validated { - preprocess_and_validate( - dot_source, - None, - &test_transform_options( - PathBuf::from("."), - None, - RenderMode::Structural, - template_context(Some(&WorkflowSettings::default()), vars), - ), - ) - .unwrap() - } - - /// Catalog-backed TRANSFORM options for the built-in test catalog. - fn test_transform_options( - current_dir: PathBuf, - file_resolver: Option>, - render_mode: RenderMode, - template_context: TemplateContext, - ) -> TransformOptions { - TransformOptions { - current_dir: Some(current_dir), - file_resolver, - template_context, - source_name: Some("workflow.fabro".to_string()), - render_mode, - custom_transforms: Vec::new(), - } - } - - const MINIMAL_DOT: &str = r#"digraph Test { - graph [goal="Build feature"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#; - - #[test] - fn validate_minimal() { - let validated = validate_dot(MINIMAL_DOT, WorkflowSettings::default()); - validated.raise_on_errors().unwrap(); - - assert_eq!(validated.graph().name, "Test"); - assert!(validated.graph().find_start_node().is_some()); - assert!(validated.graph().find_exit_node().is_some()); - } - - #[test] - fn validate_rejects_goal_self_reference() { - // A goal can't reference itself; a prompt can reference the goal. - let dot = r#"digraph Test { - graph [goal="Refine {{ goal }}"] - start [shape=Mdiamond] - work [prompt="Work on {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let validated = validate_dot(dot, WorkflowSettings::default()); - - assert!( - validated.has_errors(), - "goal self-reference should fail validation" - ); - let self_ref: Vec<_> = validated - .diagnostics() - .iter() - .filter(|d| d.rule == GOAL_SELF_REFERENCE_RULE) - .collect(); - assert_eq!( - self_ref.len(), - 1, - "expected one goal self-reference diagnostic, got: {:?}", - validated.diagnostics() - ); - assert_eq!(self_ref[0].severity, Severity::Error); - } - - #[test] - fn validate_with_unbound_inputs_warns_but_succeeds() { - let dot = r#"digraph Test { - graph [goal="Build feature"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", prompt="Work on {{ inputs.app_dir }}"] - start -> work -> exit - }"#; - let validated = validate_dot(dot, WorkflowSettings::default()); - validated.raise_on_errors().unwrap(); - - let diagnostic = validated - .diagnostics() - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected a template_undefined_variable diagnostic"); - assert_eq!(diagnostic.severity, Severity::Warning); - assert!( - diagnostic.message.contains("inputs.app_dir"), - "missing variable in: {}", - diagnostic.message - ); - } - - #[test] - fn vars_resolve_in_node_prompt_through_create_pipeline() { - let dot = r#"digraph Test { - graph [goal="Ship it"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", prompt="Service: {{ vars.SERVICE }}"] - start -> work -> exit - }"#; - let vars = HashMap::from([("SERVICE".to_string(), "billing".to_string())]); - let validated = validate_dot_with_vars(dot, vars); - validated.raise_on_errors().unwrap(); - assert!( - !validated - .diagnostics() - .iter() - .any(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE), - "vars.SERVICE should resolve through the create pipeline; got: {:?}", - validated.diagnostics() - ); - } - - #[test] - fn unknown_var_in_prompt_warns_at_validate_then_errors_at_run_create() { - let dot = r#"digraph Test { - graph [goal="Ship it"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", prompt="Service: {{ vars.MISSING }}"] - start -> work -> exit - }"#; - let mut validated = validate_dot_with_vars(dot, HashMap::new()); - - // `fabro validate` surfaces a warning, not a hard failure. - let diagnostic = validated - .diagnostics() - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected a template_undefined_variable diagnostic"); - assert_eq!(diagnostic.severity, Severity::Warning); - assert!( - diagnostic.message.contains("vars.MISSING"), - "message: {}", - diagnostic.message - ); - - // Run-create promotes the same diagnostic to a hard error. - validated.promote_template_undefined_variables_to_errors(); - assert!(validated.has_errors()); - } - - #[test] - fn unknown_input_in_model_stylesheet_warns_then_errors_at_run_create() { - let dot = r#"digraph Test { - graph [model_stylesheet="* { reasoning_effort: {{ inputs.effort }}; }"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", prompt="Do work"] - start -> work -> exit - }"#; - let mut validated = validate_dot(dot, WorkflowSettings::default()); - - let diagnostic = validated - .diagnostics() - .iter() - .find(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected a template_undefined_variable diagnostic"); - assert_eq!(diagnostic.severity, Severity::Warning); - assert!( - diagnostic - .message - .contains("graph attribute `model_stylesheet`"), - "message: {}", - diagnostic.message - ); - assert!( - validated - .diagnostics() - .iter() - .all(|diagnostic| diagnostic.rule != "stylesheet_syntax") - ); - - validated.promote_template_undefined_variables_to_errors(); - assert!(validated.has_errors()); - assert_eq!( - validated - .diagnostics() - .iter() - .find(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .unwrap() - .severity, - Severity::Error - ); - } - - #[test] - fn vars_resolve_in_command_script_through_create_pipeline() { - let dot = r#"digraph Test { - graph [goal="Ship it"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", shape=parallelogram, script="deploy --stage {{ vars.STAGE }}"] - start -> work -> exit - }"#; - let vars = HashMap::from([("STAGE".to_string(), "staging".to_string())]); - let validated = validate_dot_with_vars(dot, vars); - validated.raise_on_errors().unwrap(); - - assert_eq!( - validated.graph().nodes["work"] - .attrs - .get("script") - .and_then(fabro_graphviz::graph::AttrValue::as_str), - Some("deploy --stage staging"), - ); - } - - /// The script diagnostic must carry the same rule as the prompt one so the - /// existing run-create promotion catches an unbound value before a run - /// executes a half-interpolated command. - #[test] - fn unknown_input_in_script_warns_at_validate_then_errors_at_run_create() { - let dot = r#"digraph Test { - graph [goal="Ship it"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - work [label="Work", shape=parallelogram, script="deploy --stage {{ inputs.stage }}"] - start -> work -> exit - }"#; - let mut validated = validate_dot_with_vars(dot, HashMap::new()); - - let diagnostic = validated - .diagnostics() - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected a template_undefined_variable diagnostic"); - assert_eq!(diagnostic.severity, Severity::Warning); - assert!( - diagnostic.message.contains("inputs.stage"), - "message: {}", - diagnostic.message - ); - - validated.promote_template_undefined_variables_to_errors(); - assert!(validated.has_errors()); - } - - #[test] - fn promote_template_undefined_rule_turns_warning_into_error() { - let dot = r#"digraph Test { - graph [goal="Build {{ inputs.app_dir }}"] - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - start -> exit - }"#; - let mut validated = validate_dot(dot, WorkflowSettings::default()); - assert!(!validated.has_errors()); - - validated.promote_template_undefined_variables_to_errors(); - - assert!(validated.has_errors()); - let diagnostic = validated - .diagnostics() - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected template diagnostic"); - assert_eq!(diagnostic.severity, Severity::Error); - } - - #[test] - fn strict_template_error_for_inline_prompt_names_workflow_file_and_node() { - let dot = r#"digraph ValidatePlan { - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - test_inline_prompt [label="moo" prompt="{{ inputs.foo }}"] - start -> test_inline_prompt -> exit - }"#; - - let result = preprocess_and_validate( - dot, - None, - &test_transform_options( - PathBuf::from("."), - None, - RenderMode::Strict, - template_context(Some(&WorkflowSettings::default()), HashMap::new()), - ), - ); - let Err(err) = result else { - panic!("expected strict mode to hard-fail on unbound inline prompt"); - }; - - let rendered = collect_chain(&err).join(": "); - assert!(rendered.contains("workflow.fabro"), "{rendered}"); - assert!(rendered.contains("test_inline_prompt"), "{rendered}"); - assert!(rendered.contains("prompt"), "{rendered}"); - assert!(!rendered.contains(""), "{rendered}"); - } - - #[test] - fn imported_prompt_template_error_names_prompt_file_and_node() { - let dir = tempfile::tempdir().unwrap(); - let prompt_path = dir.path().join("test.md"); - std::fs::write(&prompt_path, "{{ inputs.foo }}").unwrap(); - let dot = r#"digraph ValidatePlan { - start [shape=Mdiamond, label="Start"] - exit [shape=Msquare, label="Exit"] - test_imported_prompt [label="moo" prompt="@test.md"] - start -> test_imported_prompt -> exit - }"#; - - let result = preprocess_and_validate( - dot, - None, - &test_transform_options( - dir.path().to_path_buf(), - Some(Arc::new(crate::file_resolver::FilesystemFileResolver::new( - None, - ))), - RenderMode::Strict, - template_context(Some(&WorkflowSettings::default()), HashMap::new()), - ), - ); - let Err(err) = result else { - panic!("expected strict mode to hard-fail on unbound imported prompt"); - }; - - let rendered = collect_chain(&err).join(": "); - assert!(rendered.contains("test.md"), "{rendered}"); - assert!(rendered.contains("test_imported_prompt"), "{rendered}"); - assert!(rendered.contains("prompt"), "{rendered}"); - assert!(!rendered.contains(""), "{rendered}"); - } - - #[test] - fn validate_applies_variable_expansion() { - let dot = r#"digraph Test { - graph [goal="Fix bugs"] - start [shape=Mdiamond] - work [prompt="Goal: {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let validated = validate_dot(dot, WorkflowSettings::default()); - validated.raise_on_errors().unwrap(); - - let prompt = validated.graph().nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, "Goal: Fix bugs"); - } - - #[test] - fn validate_does_not_render_source_level_templated_node_ids() { - let dot = r#"digraph Test { - graph [goal="Fix bugs"] - start [shape=Mdiamond] - {{ inputs.step }} [prompt="Do work"] - exit [shape=Msquare] - start -> exit - }"#; - - let result = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: dot.to_string(), - base_dir: None, - }, - settings: settings_from_run_layer({ - let mut inputs = std::collections::HashMap::new(); - inputs.insert("step".to_string(), toml::Value::String("work".to_string())); - RunLayer { - inputs: Some(inputs), - ..RunLayer::default() - } - }), - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: Vec::new(), - }); - - assert!(result.is_err()); - } - - #[test] - fn inline_and_file_prompt_diagnostics_match() { - fn normalized_diagnostics( - validated: &Validated, - ) -> Vec<(String, Severity, String, Option)> { - validated - .diagnostics() - .iter() - .map(|diagnostic| { - ( - diagnostic.rule.clone(), - diagnostic.severity.clone(), - diagnostic.message.clone(), - diagnostic.node_id.clone(), - ) - }) - .collect() - } - - let dir = tempfile::tempdir().unwrap(); - std::fs::write( - dir.path().join("missing.md"), - "Work in {{ inputs.app_dir }}", - ) - .unwrap(); - std::fs::write(dir.path().join("goal.md"), "Goal: {{ goal }}").unwrap(); - - let inline_missing = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: r#"digraph Test { - graph [goal="Demo"] - start [shape=Mdiamond] - work [prompt="Work in {{ inputs.app_dir }}"] - exit [shape=Msquare] - start -> work -> exit - }"# - .to_string(), - base_dir: Some(dir.path().to_path_buf()), - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - let file_missing = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: r#"digraph Test { - graph [goal="Demo"] - start [shape=Mdiamond] - work [prompt="@missing.md"] - exit [shape=Msquare] - start -> work -> exit - }"# - .to_string(), - base_dir: Some(dir.path().to_path_buf()), - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - assert_eq!( - normalized_diagnostics(&inline_missing), - normalized_diagnostics(&file_missing) - ); - - let inline_goal = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: r#"digraph Test { - graph [goal="Ship"] - start [shape=Mdiamond] - work [prompt="Goal: {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"# - .to_string(), - base_dir: Some(dir.path().to_path_buf()), - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - let file_goal = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: r#"digraph Test { - graph [goal="Ship"] - start [shape=Mdiamond] - work [prompt="@goal.md"] - exit [shape=Msquare] - start -> work -> exit - }"# - .to_string(), - base_dir: Some(dir.path().to_path_buf()), - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - assert_eq!( - inline_goal.graph().nodes["work"].attrs.get("prompt"), - file_goal.graph().nodes["work"].attrs.get("prompt") - ); - assert_eq!( - normalized_diagnostics(&inline_goal), - normalized_diagnostics(&file_goal) - ); - } - - #[test] - fn make_run_dir_uses_run_id_timestamp_in_local_time() { - let scratch_base = Path::new("/tmp/scratch"); - let run_id = RunId::from(ulid::Ulid::from_datetime( - Utc.with_ymd_and_hms(2026, 3, 27, 12, 0, 0).unwrap().into(), - )); - let expected_date = run_id - .created_at() - .with_timezone(&Local) - .format("%Y%m%d") - .to_string(); - - assert_eq!( - make_run_dir(scratch_base, &run_id), - scratch_base.join(format!("{expected_date}-{run_id}")) - ); - } - - #[test] - fn validate_applies_stylesheet() { - let dot = r#"digraph Test { - graph [goal="Test", model_stylesheet="* { model: sonnet; }"] - start [shape=Mdiamond] - work [label="Work"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let validated = validate_dot(dot, WorkflowSettings::default()); - validated.raise_on_errors().unwrap(); - - assert_eq!( - validated.graph().nodes["work"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - } - - #[test] - fn validate_applies_config_vars_and_goal_override() { - let dot = r#"digraph Test { - graph [goal="original"] - start [shape=Mdiamond] - work [prompt="{{ inputs.who }}: {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let validated = validate_dot( - dot, - settings_from_run_layer({ - let mut inputs = std::collections::HashMap::new(); - inputs.insert("who".to_string(), toml::Value::String("agent".to_string())); - RunLayer { - goal: Some(RunGoalLayer::Inline(InterpString::parse("override"))), - inputs: Some(inputs), - ..RunLayer::default() - } - }), - ); - validated.raise_on_errors().unwrap(); - - assert_eq!(validated.graph().goal(), "override"); - let prompt = validated.graph().nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, "agent: override"); - } - - #[test] - fn validate_returns_error_on_invalid_dot() { - let result = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: "not a graph".to_string(), - base_dir: None, - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: Vec::new(), - }); - assert!(result.is_err()); - } - - #[test] - fn validate_supports_custom_transforms() { - struct TagTransform; - - impl Transform for TagTransform { - fn apply( - &self, - graph: fabro_graphviz::graph::Graph, - ) -> Result { - let mut graph = graph; - for node in graph.nodes.values_mut() { - node.attrs - .insert("tagged".to_string(), AttrValue::Boolean(true)); - } - - Ok(graph) - } - } - - let validated = validate(ValidateInput { - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: vec![Box::new(TagTransform)], - }) - .unwrap(); - validated.raise_on_errors().unwrap(); - - assert_eq!( - validated.graph().nodes["start"].attrs.get("tagged"), - Some(&AttrValue::Boolean(true)) - ); - } - - #[test] - fn validate_from_file_uses_parent_directory_for_inlining() { - let dir = tempfile::tempdir().unwrap(); - let data_path = dir.path().join("goal.txt"); - let dot_path = dir.path().join("workflow.fabro"); - - std::fs::write(&data_path, "ship it").unwrap(); - std::fs::write( - &dot_path, - r#"digraph Test { - graph [goal="@goal.txt"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#, - ) - .unwrap(); - - let validated = validate(ValidateInput { - workflow: WorkflowInput::Path(dot_path), - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - validated.raise_on_errors().unwrap(); - assert_eq!(validated.graph().goal(), "ship it"); - } - - #[test] - fn validate_from_file_resolves_minijinja_includes_relative_to_prompt_and_goal_files() { - let dir = tempfile::tempdir().unwrap(); - let prompt_dir = dir.path().join("prompts"); - let goal_dir = dir.path().join("goals"); - std::fs::create_dir_all(&prompt_dir).unwrap(); - std::fs::create_dir_all(&goal_dir).unwrap(); - std::fs::write( - prompt_dir.join("prompt.md"), - r#"{% include "prompt.tpl.md" %}"#, - ) - .unwrap(); - std::fs::write(prompt_dir.join("prompt.tpl.md"), "included prompt").unwrap(); - std::fs::write(goal_dir.join("goal.md"), r#"{% include "goal.tpl.md" %}"#).unwrap(); - std::fs::write(goal_dir.join("goal.tpl.md"), "included goal").unwrap(); - - let dot_path = dir.path().join("workflow.fabro"); - std::fs::write( - &dot_path, - r#"digraph Test { - graph [goal="@goals/goal.md"] - start [shape=Mdiamond] - work [prompt="@prompts/prompt.md"] - exit [shape=Msquare] - start -> work -> exit - }"#, - ) - .unwrap(); - - let validated = validate(ValidateInput { - workflow: WorkflowInput::Path(dot_path), - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - custom_transforms: Vec::new(), - }) - .unwrap(); - - validated.raise_on_errors().unwrap(); - assert_eq!(validated.graph().goal(), "included goal"); - assert_eq!( - validated.graph().nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("included prompt") - ); - } - - #[test] - fn validate_from_bundle_resolves_nested_import_files_relative_to_imported_graph() { - let validated = validate(ValidateInput { - workflow: WorkflowInput::Bundled(BundledWorkflow { - path: ManifestPath::from_wire("workflow.fabro").unwrap(), - source: r#"digraph Test { - graph [goal="Ship"] - start [shape=Mdiamond] - validate [import="./child/validate.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"# - .to_string(), - config: None, - files: HashMap::from([ - ( - ManifestPath::from_wire("child/validate.fabro").unwrap(), - r#"digraph Validate { - start [shape=Mdiamond] - lint [prompt="@../prompts/lint.md"] - exit [shape=Msquare] - start -> lint -> exit - }"# - .to_string(), - ), - ( - ManifestPath::from_wire("prompts/lint.md").unwrap(), - "Lint {{ goal }}".to_string(), - ), - ]), - }), - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: Vec::new(), - }) - .unwrap(); - - validated.raise_on_errors().unwrap(); - assert_eq!( - validated.graph().nodes["validate.lint"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Lint Ship") - ); - } - - #[test] - fn validate_from_bundle_resolves_minijinja_includes_in_prompt_and_goal_files() { - let validated = validate(ValidateInput { - workflow: WorkflowInput::Bundled(BundledWorkflow { - path: ManifestPath::from_wire("workflow.fabro").unwrap(), - source: r#"digraph Test { - graph [goal="@goals/goal.md"] - start [shape=Mdiamond] - work [prompt="@prompts/work.md"] - exit [shape=Msquare] - start -> work -> exit - }"# - .to_string(), - config: None, - files: HashMap::from([ - ( - ManifestPath::from_wire("goals/goal.md").unwrap(), - r#"{% include "goal.tpl.md" %}"#.to_string(), - ), - ( - ManifestPath::from_wire("goals/goal.tpl.md").unwrap(), - "Bundled goal".to_string(), - ), - ( - ManifestPath::from_wire("prompts/work.md").unwrap(), - r#"{% include "work.tpl.md" %}"#.to_string(), - ), - ( - ManifestPath::from_wire("prompts/work.tpl.md").unwrap(), - "Bundled prompt".to_string(), - ), - ]), - }), - settings: WorkflowSettings::default(), - vars: HashMap::new(), - cwd: PathBuf::from("."), - custom_transforms: Vec::new(), - }) - .unwrap(); - - validated.raise_on_errors().unwrap(); - assert_eq!(validated.graph().goal(), "Bundled goal"); - assert_eq!( - validated.graph().nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Bundled prompt") - ); - } - - #[test] - fn assemble_create_run_persistence_input_resolves_complete_durable_identity() { - let dir = tempfile::tempdir().unwrap(); - let storage_root = dir.path().join("storage"); - let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), - workflow_source: None, - }; - let request = CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: test_default_settings(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("request-slug".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_1), - title: Some("Assembled run".to_string()), - automation: Some(automation.clone()), - git: None, - fork_source_ref: None, - parent_id: Some(fixtures::RUN_2), - provenance: test_support::test_run_provenance(), - web_url: Some("https://fabro.test/runs/1".to_string()), - }; - let resolved_run_id = fixtures::RUN_64; - - let compiled = compile_admitted_run(compile_input(&request)).unwrap(); - let materialized = materialize_admitted_run(compiled); - let metadata = persistence_metadata(&request, resolved_run_id, &storage_root); - let input = assemble_create_run_persistence_input(materialized, metadata); - - assert_eq!(input.run_id(), resolved_run_id); - assert_eq!( - input.run_dir(), - Storage::new(&storage_root) - .run_scratch(&resolved_run_id) - .root() - ); - assert_eq!(input.workflow_slug(), Some("request-slug")); - assert_eq!(input.automation(), Some(&automation)); - // The settings keep the model they named (none here): Petri pins - // the resolved model in its admitted graph, not in the settings. - assert_eq!( - input.materialized().settings().run.model.name.as_deref(), - None - ); - } - - #[test] - fn compile_admitted_run_exposes_resolved_metadata_and_definition() { - let workflow_path = ManifestPath::from_wire("workflows/main.fabro").unwrap(); + fn admitted(settings: WorkflowSettings, goal: &str, cwd: &Path) -> AdmittedRunInput { let bundled = BundledWorkflow { - path: workflow_path.clone(), - source: MINIMAL_DOT.to_string(), + path: workflow_path(), + source: DOT.to_string(), config: None, files: HashMap::new(), }; - let bundle = WorkflowBundle::new(HashMap::from([(workflow_path.clone(), bundled.clone())])); - let compiled = compile_admitted_run(CreateRunCompileInput { - workflow: WorkflowInput::Bundled(bundled), - settings: test_default_settings(), - vars: HashMap::new(), - cwd: PathBuf::from("/tmp/project"), - workflow_path: Some(workflow_path.clone()), - workflow_bundle: Some(bundle), - }) + AdmittedRunInput { + settings, + cwd: cwd.to_path_buf(), + graph: graph(goal), + source: DOT.to_string(), + workflow_path: workflow_path(), + workflow_bundle: WorkflowBundle::new(HashMap::from([(workflow_path(), bundled)])), + } + } + + fn metadata(run_id: RunId, storage_root: &Path) -> CreateRunPersistenceMetadata { + CreateRunPersistenceMetadata { + run_id, + storage_root: storage_root.to_path_buf(), + workflow_slug: None, + workflow_version_id: None, + target: None, + title: None, + automation: None, + git: None, + fork_source_ref: None, + parent_id: None, + provenance: test_support::test_run_provenance(), + web_url: None, + admission: PetriAdmission::default(), + } + } + + fn inline_goal(settings: &WorkflowSettings) -> Option { + match settings.run.goal.as_ref()? { + RunGoal::Inline(goal) => Some(goal.resolve_with(&mut ResolveCtx::default()).unwrap()), + RunGoal::File(_) => panic!("the materialized goal should be inline"), + } + } + + async fn platform_records(store: &Database, run_id: RunId) -> Vec { + store + .run_summary_store() + .platform_records() + .read(&run_id) + .await + .unwrap() + } + + #[test] + fn the_admitted_goal_becomes_the_inline_run_goal() { + let dir = tempfile::tempdir().unwrap(); + let materialized = materialize_admitted_run(admitted( + settings(RunLayer::default()), + "Graph goal", + dir.path(), + )) .unwrap(); - assert_eq!(compiled.raw_source, MINIMAL_DOT); - assert_eq!(compiled.dot_path.as_deref(), Some(workflow_path.as_path())); - assert_eq!(compiled.labels, compiled.settings().combined_labels()); - let materialized = materialize_admitted_run(compiled); - let input = - assemble_create_run_persistence_input(materialized, CreateRunPersistenceMetadata { - run_id: fixtures::RUN_1, - storage_root: PathBuf::from("/tmp/storage"), - workflow_slug: None, - workflow_version_id: None, - target: None, - title: None, - automation: None, - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - admission: PetriAdmission::default(), - }); - let definition = input - .definition() - .expect("bundled create input should retain a run definition"); - assert_eq!(definition.workflow_path, workflow_path); + assert_eq!(materialized.graph().goal(), "Graph goal"); + assert_eq!( + inline_goal(materialized.settings()).as_deref(), + Some("Graph goal") + ); + assert_eq!(materialized.workflow_slug.as_deref(), Some("ship")); + assert_eq!(materialized.definition.workflow_path, workflow_path()); + assert_eq!( + materialized.source_directory, + dir.path().to_string_lossy().into_owned() + ); + } + + #[test] + fn the_settings_goal_wins_over_the_admitted_goal_and_a_file_goal_is_read() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("goal.md"), "Goal from file").unwrap(); + let inline = settings(RunLayer { + goal: Some(fabro_config::RunGoalLayer::Inline(InterpString::parse( + "Override goal", + ))), + ..RunLayer::default() + }); + let from_file = settings(RunLayer { + goal: Some(fabro_config::RunGoalLayer::File { + file: InterpString::parse("goal.md"), + }), + ..RunLayer::default() + }); + + let inline = materialize_admitted_run(admitted(inline, "Graph goal", dir.path())).unwrap(); + let from_file = + materialize_admitted_run(admitted(from_file, "Graph goal", dir.path())).unwrap(); + + assert_eq!(inline.graph().goal(), "Override goal"); + assert_eq!( + inline_goal(inline.settings()).as_deref(), + Some("Override goal") + ); + assert_eq!(from_file.graph().goal(), "Goal from file"); + assert_eq!( + inline_goal(from_file.settings()).as_deref(), + Some("Goal from file") + ); + } + + #[test] + fn a_workflow_without_a_goal_keeps_none_and_a_disabled_pull_request_is_dropped() { + let dir = tempfile::tempdir().unwrap(); + let mut disabled = settings(RunLayer::default()); + disabled.run.pull_request = Some(PullRequestSettings::default()); + assert!(!disabled.run.pull_request.as_ref().unwrap().enabled); + + let materialized = materialize_admitted_run(admitted(disabled, "", dir.path())).unwrap(); + + assert_eq!(materialized.settings().run.goal, None); + assert_eq!(materialized.settings().run.pull_request, None); } #[tokio::test] - async fn persist_create_run_uses_compiled_graph_without_recompiling_source() { + async fn persisting_records_the_spec_with_the_graph_and_its_source_then_submits() { let dir = tempfile::tempdir().unwrap(); let storage_root = dir.path().join("storage"); - let dot_path = dir.path().join("workflow.fabro"); - let compiled_source = MINIMAL_DOT.replace("Build feature", "Compiled goal"); - std::fs::write(&dot_path, &compiled_source).unwrap(); - let automation = AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), - workflow_source: None, - }; - let request = CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::Path(dot_path.clone()), - settings: test_default_settings(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("compiled-slug".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_2), - title: Some("Compiled run".to_string()), - automation: Some(automation.clone()), - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }; - let compiled = compile_admitted_run(compile_input(&request)).unwrap(); + let materialized = materialize_admitted_run(admitted( + settings(RunLayer::default()), + "## Plan: Ship it\n\nDetails", + dir.path(), + )) + .unwrap(); + let mut metadata = metadata(fixtures::RUN_2, &storage_root); + metadata.workflow_version_id = Some(test_support::test_workflow_version_id()); + let store = Arc::new(fabro_store::test_support::test_database()); - std::fs::write(&dot_path, "this is no longer a graph").unwrap(); - - let materialized = materialize_admitted_run(compiled); - let workflow_version_id = test_support::test_workflow_version_id(); - let mut metadata = persistence_metadata(&request, fixtures::RUN_2, &storage_root); - metadata.workflow_version_id = Some(workflow_version_id); - let input = assemble_create_run_persistence_input(materialized, metadata); - let store = memory_store(); - let created = persist_create_run(store.as_ref(), input).await.unwrap(); + let created = persist_create_run( + store.as_ref(), + assemble_create_run_persistence_input(materialized, metadata), + ) + .await + .unwrap(); assert_eq!(created.run_id, fixtures::RUN_2); - assert_eq!(created.dot_path.as_deref(), Some(dot_path.as_path())); - assert_eq!(created.persisted.graph().goal(), "Compiled goal"); - assert_eq!(created.persisted.source(), compiled_source); + assert_eq!(created.source, DOT); + assert!(created.run_dir.is_dir()); + assert_eq!(created.spec.workflow_slug.as_deref(), Some("ship")); + assert!(created.spec.definition_blob.is_some()); + assert!(created.spec.spec_blob.is_some()); let records = platform_records(&store, fixtures::RUN_2).await; assert_eq!( @@ -1655,17 +593,17 @@ mod tests { .collect::>(), vec!["run.created", "run.lifecycle"] ); - let PlatformRecord::RunCreated(created) = &records[0].record else { + let PlatformRecord::RunCreated(record) = &records[0].record else { panic!("first durable record should be run.created"); }; - assert_eq!(created.spec.graph.goal(), "Compiled goal"); - assert_eq!(created.spec.automation, Some(automation)); - assert_eq!(created.spec.workflow_version_id, Some(workflow_version_id)); + assert_eq!(record.title.as_deref(), Some("Ship it")); + assert_eq!(record.spec.graph.name, "Test"); + assert_eq!(record.spec.graph.goal(), "## Plan: Ship it\n\nDetails"); + assert_eq!(record.spec.graph_source.as_deref(), Some(DOT)); assert_eq!( - created.spec.graph_source.as_deref(), - Some(compiled_source.as_str()) + record.spec.workflow_version_id, + Some(test_support::test_workflow_version_id()) ); - assert!(created.spec.spec_blob.is_some()); let PlatformRecord::RunLifecycle(submitted) = &records[1].record else { panic!("second durable record should be the submitted transition"); }; @@ -1673,545 +611,39 @@ mod tests { assert_eq!(submitted.status, Some(RunStatus::Submitted)); } - #[expect( - clippy::disallowed_methods, - reason = "test asserts the raw template source" - )] #[tokio::test] - async fn create_persists_normalized_config_and_initial_state() { + async fn an_explicit_title_and_the_target_shape_the_recorded_spec() { let dir = tempfile::tempdir().unwrap(); - let storage_root = dir.path().join("storage"); - let store = memory_store(); - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: settings_from_run_layer({ - let mut metadata = HashMap::new(); - metadata.insert("env".to_string(), "test".to_string()); - RunLayer { - goal: Some(RunGoalLayer::Inline(InterpString::parse("override goal"))), - metadata: ReplaceMap::from(metadata), - model: Some(RunModelLayer { - name: Some("sonnet".to_string()), - ..RunModelLayer::default() - }), - pull_request: Some(RunPullRequestLayer { - enabled: Some(false), - ..RunPullRequestLayer::default() - }), - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - } - }), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("slug".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_1), - title: None, - automation: None, - git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - }), - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root.clone(), - ) - .await + let materialized = materialize_admitted_run(admitted( + settings(RunLayer::default()), + "Graph goal", + dir.path(), + )) .unwrap(); - - assert_eq!(created.run_id, fixtures::RUN_1); - assert_eq!(created.persisted.run_spec().graph.goal(), "override goal"); - assert_eq!( - created - .persisted - .run_spec() - .settings - .run - .model - .name - .as_deref(), - Some("sonnet") - ); - assert_eq!( - created - .persisted - .run_spec() - .settings - .run - .model - .provider - .as_deref(), - None - ); - assert_eq!( - match &created.persisted.run_spec().settings.run.goal { - Some(fabro_types::settings::run::RunGoal::Inline(value)) => { - Some(value.as_source()) - } - _ => None, - } - .as_deref(), - Some("override goal") - ); - assert!( - created - .persisted - .run_spec() - .settings - .run - .pull_request - .is_none() - ); - assert_eq!( - created.persisted.run_spec().workflow_slug.as_deref(), - Some("slug") - ); - assert_eq!( - last_lifecycle_status(&platform_records(&store, fixtures::RUN_1).await), - Some(fabro_types::RunStatus::Submitted) - ); - assert_eq!( - created.run_dir, - Storage::new(&storage_root) - .run_scratch(&fixtures::RUN_1) - .root() - .to_path_buf() - ); - assert!(created.run_dir.is_dir()); - } - - #[tokio::test] - async fn create_persists_secret_tokens_in_run_created_settings_source_form() { - let dir = tempfile::tempdir().unwrap(); - let storage_root = dir.path().join("storage"); - let store = memory_store(); - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: settings_from_run_layer(RunLayer { - prepare: Some(RunPrepareLayer { - steps: vec![PrepareStep { - script: None, - command: Some(vec![ - InterpString::parse("deploy"), - InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"), - ]), - env: HashMap::from([( - "DEPLOY_TOKEN".to_string(), - InterpString::parse("{{ secrets.DEPLOY_TOKEN }}"), - )]), - }], - timeout: None, - }), - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - }), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("secret-source".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_1), - title: None, - automation: None, - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root, - ) - .await - .unwrap(); - - let records = platform_records(&store, created.run_id).await; - let step = run_created(&records) - .spec - .settings - .run - .prepare - .steps - .first() - .expect("prepare step should be persisted"); - - let fabro_types::settings::run::PreparedStepRun::Command { command } = &step.run else { - panic!("expected command prepare step"); - }; - assert_eq!(command, &vec![ - "deploy".to_string(), - "{{ secrets.DEPLOY_TOKEN }}".to_string() - ]); - assert_eq!( - step.env.get("DEPLOY_TOKEN").map(String::as_str), - Some("{{ secrets.DEPLOY_TOKEN }}") - ); - } - - #[tokio::test] - async fn create_persists_submitter_source_directory_from_request_cwd() { - let dir = tempfile::tempdir().unwrap(); - let workspace = dir.path().join("workspace"); - std::fs::create_dir_all(&workspace).unwrap(); - let storage_root = dir.path().join("storage"); - - let store = memory_store(); - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: settings_from_run_layer({ - RunLayer { - working_dir: Some("workspace".to_string()), - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - } - }), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: None, - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_2), - title: None, - automation: None, - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root, - ) - .await - .unwrap(); - - assert_eq!( - created.persisted.run_spec().source_directory.as_deref(), - Some(workspace.to_string_lossy().as_ref()) - ); - } - - #[tokio::test] - async fn create_none_target_omits_the_source_directory_projection() { - let dir = tempfile::tempdir().unwrap(); - let storage_root = dir.path().join("storage"); - let store = memory_store(); - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: test_default_settings(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: None, - workflow_path: None, - workflow_bundle: None, - target: Some(RunTarget::None {}), - run_id: Some(fixtures::RUN_2), - title: None, - automation: None, - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root, - ) - .await - .unwrap(); - - assert_eq!(created.persisted.run_spec().source_directory, None); - assert_eq!( - created.persisted.run_spec().target, - Some(RunTarget::None {}) - ); - assert_eq!(created.persisted.run_spec().git, None); - } - - #[tokio::test] - async fn create_folder_target_projects_its_path_over_the_compiler_directory() { - let dir = tempfile::tempdir().unwrap(); - let workspace = dir.path().join("workspace"); - std::fs::create_dir(&workspace).unwrap(); - let canonical = workspace - .canonicalize() - .unwrap() - .to_string_lossy() - .to_string(); - let storage_root = dir.path().join("storage"); - let store = memory_store(); - let git = fabro_types::GitContext { - origin_url: "https://github.com/fabro-sh/fabro".to_string(), + let mut metadata = metadata(fixtures::RUN_3, &dir.path().join("storage")); + metadata.title = Some("Explicit".to_string()); + metadata.target = Some(RunTarget::None {}); + metadata.git = Some(GitContext { + origin_url: "https://github.com/fabro-sh/fabro.git".to_string(), branch: "main".to_string(), sha: None, dirty: fabro_types::DirtyStatus::Clean, - }; - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: test_default_settings(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: None, - workflow_path: None, - workflow_bundle: None, - target: Some(RunTarget::Folder { - path: canonical.clone(), - }), - run_id: Some(fixtures::RUN_2), - title: None, - automation: None, - git: Some(git.clone()), - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root, - ) - .await - .unwrap(); - - assert_eq!( - created.persisted.run_spec().target, - Some(RunTarget::Folder { - path: canonical.clone(), - }) - ); - assert_eq!( - created.persisted.run_spec().source_directory.as_deref(), - Some(canonical.as_str()) - ); - assert_eq!(created.persisted.run_spec().git, Some(git)); - } - - #[tokio::test] - async fn create_persists_repo_origin_url_from_request() { - let dir = tempfile::tempdir().unwrap(); - let storage_root = dir.path().join("storage"); - let store = memory_store(); - let created = create( - &store, - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: dry_run_only_settings(), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: None, - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_2), - title: None, - automation: None, - git: Some(fabro_types::GitContext { - origin_url: "https://github.com/acme/widgets".to_string(), - branch: String::new(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - }), - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_root, - ) - .await - .unwrap(); - - assert_eq!( - created.persisted.run_spec().repo_origin_url(), - Some("https://github.com/acme/widgets") - ); - } - - fn dry_run_only_settings() -> WorkflowSettings { - settings_from_run_layer(RunLayer { - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - }) - } - - fn dry_run_with_storage(_storage_dir: &Path) -> WorkflowSettings { - settings_from_run_layer(RunLayer { - execution: Some(RunExecutionLayer { - mode: Some(RunMode::DryRun), - ..RunExecutionLayer::default() - }), - ..RunLayer::default() - }) - } - - #[tokio::test] - async fn create_hydrates_run_created_event_into_store() { - let dir = tempfile::tempdir().unwrap(); - let storage_dir = dir.path().join("storage"); - std::fs::create_dir_all(storage_dir.join("store")).unwrap(); + }); let store = Arc::new(fabro_store::test_support::test_database()); - let automation = fabro_types::AutomationRef { - id: "nightly".to_string(), - name: Some("Nightly".to_string()), - trigger_id: Some("schedule_1".to_string()), - workflow_source: None, + + let created = persist_create_run( + store.as_ref(), + assemble_create_run_persistence_input(materialized, metadata), + ) + .await + .unwrap(); + + let records = platform_records(&store, fixtures::RUN_3).await; + let PlatformRecord::RunCreated(record) = &records[0].record else { + panic!("first durable record should be run.created"); }; - let created = create( - store.as_ref(), - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: dry_run_with_storage(&storage_dir), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("slug".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_3), - title: None, - automation: Some(automation.clone()), - git: None, - fork_source_ref: None, - parent_id: None, - provenance: test_support::test_run_provenance(), - web_url: None, - }, - storage_dir.clone(), - ) - .await - .unwrap(); - let records = platform_records(&store, created.run_id).await; - - assert_eq!( - records.first().unwrap().record.kind().to_string(), - "run.created" - ); - assert_eq!( - created.persisted.run_spec().automation, - Some(automation.clone()) - ); - assert_eq!(run_created(&records).spec.automation, Some(automation)); - } - - #[tokio::test] - async fn create_hydrates_provenance_into_store_state() { - let dir = tempfile::tempdir().unwrap(); - let storage_dir = dir.path().join("storage"); - std::fs::create_dir_all(storage_dir.join("store")).unwrap(); - let store = Arc::new(fabro_store::test_support::test_database()); - let created = create( - store.as_ref(), - CreateRunInput { - admission: PetriAdmission::default(), - workflow: WorkflowInput::DotSource { - source: MINIMAL_DOT.to_string(), - base_dir: None, - }, - settings: dry_run_with_storage(&storage_dir), - vars: HashMap::new(), - cwd: dir.path().to_path_buf(), - workflow_slug: Some("slug".to_string()), - workflow_path: None, - workflow_bundle: None, - target: None, - run_id: Some(fixtures::RUN_64), - title: None, - automation: None, - git: None, - fork_source_ref: None, - parent_id: None, - provenance: fabro_types::RunProvenance { - server: Some(fabro_types::RunServerProvenance { - version: "0.9.0".to_string(), - }), - client: Some(fabro_types::RunClientProvenance { - user_agent: Some("fabro-cli/0.9.0".to_string()), - name: Some("fabro-cli".to_string()), - version: Some("0.9.0".to_string()), - }), - subject: fabro_types::Principal::user( - fabro_types::IdpIdentity::new("https://github.com", "12345").unwrap(), - "octocat".to_string(), - fabro_types::AuthMethod::Github, - ), - }, - web_url: None, - }, - storage_dir, - ) - .await - .unwrap(); - - let records = platform_records(&store, created.run_id).await; - let provenance = run_created(&records).spec.provenance.clone(); - - assert_eq!(provenance.server.unwrap().version, "0.9.0"); - assert_eq!( - provenance.client.unwrap().name.as_deref(), - Some("fabro-cli") - ); - assert_eq!( - provenance.subject, - fabro_types::Principal::user( - fabro_types::IdpIdentity::new("https://github.com", "12345").unwrap(), - "octocat".to_string(), - fabro_types::AuthMethod::Github, - ) - ); + assert_eq!(record.title.as_deref(), Some("Explicit")); + assert_eq!(created.spec.source_directory, None); + assert_eq!(created.spec.git, None); } } diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index 000b7a6cd..241892f8b 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -142,7 +142,9 @@ pub async fn persist_forked_run(store: &Database, input: &ForkedRunInput<'_>) -> #[cfg(test)] mod tests { use chrono::Utc; - use fabro_types::{FailureReason, Graph, PetriAdmission, RunSpec, WorkflowSettings, fixtures}; + use fabro_types::{ + FailureReason, PetriAdmission, RunGraph, RunSpec, WorkflowSettings, fixtures, + }; use super::*; @@ -152,7 +154,7 @@ mod tests { RunSpec { run_id: fixtures::RUN_1, settings: WorkflowSettings::default(), - graph: Graph::new("source"), + graph: RunGraph::new("source"), graph_source: Some("digraph source { start -> exit }".to_string()), workflow_slug: Some("source".to_string()), workflow_version_id: None, diff --git a/lib/components/fabro-workflow/src/operations/mod.rs b/lib/components/fabro-workflow/src/operations/mod.rs index 6cfe66b24..91aa55155 100644 --- a/lib/components/fabro-workflow/src/operations/mod.rs +++ b/lib/components/fabro-workflow/src/operations/mod.rs @@ -2,14 +2,12 @@ mod create; mod fork; mod retry; mod rewind; -mod source; mod timeline; -mod validate; pub use create::{ - CompiledRun, CreateRunCompileInput, CreateRunPersistenceInput, CreateRunPersistenceMetadata, - CreatedRun, MaterializedRun, assemble_create_run_persistence_input, compile_admitted_run, - make_run_dir, materialize_admitted_run, persist_create_run, + AdmittedRunInput, CreateRunPersistenceInput, CreateRunPersistenceMetadata, CreatedRun, + MaterializedRun, assemble_create_run_persistence_input, make_run_dir, materialize_admitted_run, + persist_create_run, }; use fabro_types::RunId; pub use fork::{ @@ -18,14 +16,11 @@ pub use fork::{ }; pub use retry::{ensure_retryable, reruns_last}; pub use rewind::{ensure_rewindable, superseded_record}; -pub use source::WorkflowInput; pub use timeline::{ ForkTarget, RunTimeline, StageLabel, StageLabels, TimelineEntry, TimelinePosition, }; -pub use validate::{ValidateInput, validate}; pub use crate::error::Error; -pub use crate::transforms::RenderMode; /// The canonical "run is archived — mutation rejected" error message. Shared /// by the server's HTTP guards and the CLI so the user sees the same diff --git a/lib/components/fabro-workflow/src/pull_request.rs b/lib/components/fabro-workflow/src/pull_request.rs index 2bd662c77..8ff79ea28 100644 --- a/lib/components/fabro-workflow/src/pull_request.rs +++ b/lib/components/fabro-workflow/src/pull_request.rs @@ -666,14 +666,13 @@ mod tests { use chrono::Utc; use fabro_auth::VaultCredentialSource; - use fabro_graphviz::graph::Graph; use fabro_llm::adapter::{ProviderAdapter, ResolvedCall}; use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::AdapterId; use fabro_llm::{Response, ResponseStream}; use fabro_types::{ - PetriAdmission, RunProjection, RunSpec, StageSummary, WorkflowSettings, first_event_seq, - fixtures, test_support, + PetriAdmission, RunGraph, RunProjection, RunSpec, StageSummary, WorkflowSettings, + first_event_seq, fixtures, test_support, }; use fabro_vault::{SecretType, Vault}; use httpmock::Method::{GET, POST}; @@ -781,7 +780,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr RunSpec { run_id: fixtures::RUN_1, settings: WorkflowSettings::default(), - graph: Graph::new("test"), + graph: RunGraph::new("test"), graph_source: None, workflow_slug: None, workflow_version_id: None, diff --git a/lib/components/fabro-workflow/src/workflow_bundle.rs b/lib/components/fabro-workflow/src/workflow_bundle.rs index c6d635af9..73eab27f6 100644 --- a/lib/components/fabro-workflow/src/workflow_bundle.rs +++ b/lib/components/fabro-workflow/src/workflow_bundle.rs @@ -1,12 +1,8 @@ use std::collections::HashMap; -use std::path::PathBuf; -use std::sync::Arc; use fabro_types::ManifestPath; use serde::{Deserialize, Serialize}; -use crate::file_resolver::{BundleFileResolver, FileResolver}; - #[derive(Clone, Debug, Serialize, Deserialize)] pub struct ParsedWorkflowConfig { pub path: ManifestPath, @@ -21,18 +17,6 @@ pub struct BundledWorkflow { pub files: HashMap, } -impl BundledWorkflow { - #[must_use] - pub fn file_resolver(&self) -> Arc { - Arc::new(BundleFileResolver::new(self.files.clone())) - } - - #[must_use] - pub fn current_dir(&self) -> PathBuf { - self.path.parent_or_dot().to_path_buf() - } -} - #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct WorkflowBundle { workflows: HashMap, diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index 623f188d2..7823c1762 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -651,6 +651,9 @@ fn main() { ("RunStreamItemKind", "fabro_types::RunStreamItemKind", &[]), ("PetriAdmission", "fabro_types::PetriAdmission", &[]), ("PetriGraphRef", "fabro_types::PetriGraphRef", &[]), + ("RunGraph", "fabro_types::RunGraph", &[]), + ("RunGraphNode", "fabro_types::RunGraphNode", &[]), + ("RunGraphEdge", "fabro_types::RunGraphEdge", &[]), ("PullRequest", "fabro_types::PullRequest", &[]), ("PullRequestLink", "fabro_types::PullRequestLink", &[]), ( diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index d605647da..6961088e7 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -55,20 +55,20 @@ pub mod types { PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, QuestionType, RepositoryRef, ReviewTarget, ReviewTargetKind, Run, RunApproval, RunApprovalState, RunClientProvenance, RunFailure, - RunIntent, RunIntentArgs, RunPairStatusResponse, RunProjection, RunProvenance, - RunRunnableSource, RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, - RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, RunSessionMetadata, RunSize, - RunStreamItem, RunStreamItemKind, RunTarget, SandboxDetails, SandboxInfo, SandboxListMeta, - SandboxListResponse, SandboxProviderKind, SandboxProviderLookupError, SandboxService, - SandboxServiceListResponse, SecretMetadata, SecretType, ServerSettings, SessionDetail, - SessionEvent, SessionEventBody, SessionId, SessionStatus, SessionSummary, SessionTurn, - SkillActivationSource, SkillSummary, StageCompletion, StageContextWindow, - StageContextWindowUnavailableReason, StageHandler, StageId, StageInferenceProjection, - StageModelUsage, StageOutcome, StageProjection, StageState, StageToolBatchProjection, - SystemActorKind, SystemIntegrationStatus, SystemIntegrationsResponse, TodoListProjection, - ToolCategory, ToolSource, ToolSummary, TurnId, UpdateVariableRequest, UserPrincipal, - Variable, VariableListResponse, WorkflowPath, WorkflowSettings, WorkflowVersion, - WorkflowVersionId, + RunGraph, RunGraphEdge, RunGraphNode, RunIntent, RunIntentArgs, RunPairStatusResponse, + RunProjection, RunProvenance, RunRunnableSource, RunSandbox, RunSandboxFailure, + RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, + RunSessionMetadata, RunSize, RunStreamItem, RunStreamItemKind, RunTarget, SandboxDetails, + SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxProviderKind, + SandboxProviderLookupError, SandboxService, SandboxServiceListResponse, SecretMetadata, + SecretType, ServerSettings, SessionDetail, SessionEvent, SessionEventBody, SessionId, + SessionStatus, SessionSummary, SessionTurn, SkillActivationSource, SkillSummary, + StageCompletion, StageContextWindow, StageContextWindowUnavailableReason, StageHandler, + StageId, StageInferenceProjection, StageModelUsage, StageOutcome, StageProjection, + StageState, StageToolBatchProjection, SystemActorKind, SystemIntegrationStatus, + SystemIntegrationsResponse, TodoListProjection, ToolCategory, ToolSource, ToolSummary, + TurnId, UpdateVariableRequest, UserPrincipal, Variable, VariableListResponse, WorkflowPath, + WorkflowSettings, WorkflowVersion, WorkflowVersionId, }; pub use lithos_llm::catalog::{ModelHandle, ProviderId}; pub use lithos_llm::types::{ diff --git a/lib/foundation/fabro-api/tests/run_graph_round_trip.rs b/lib/foundation/fabro-api/tests/run_graph_round_trip.rs new file mode 100644 index 000000000..f1957564a --- /dev/null +++ b/lib/foundation/fabro-api/tests/run_graph_round_trip.rs @@ -0,0 +1,54 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::{ + RunGraph as ApiRunGraph, RunGraphEdge as ApiRunGraphEdge, RunGraphNode as ApiRunGraphNode, +}; +use fabro_types::{RunGraph, RunGraphEdge, RunGraphNode, StageHandler}; +use serde_json::json; + +#[test] +fn the_run_graph_reuses_canonical_types() { + assert_same_type::(); + assert_same_type::(); + assert_same_type::(); +} + +#[test] +fn the_run_graph_round_trips_the_schema_shape() { + let value = json!({ + "name": "Ship", + "goal": "Ship the feature", + "nodes": { + "plan": { "label": "Plan rollout", "kind": "agent" }, + "start": { "label": "Start", "kind": "start" } + }, + "edges": [ + { "from": "start", "to": "plan" } + ] + }); + let graph: RunGraph = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(graph.name, "Ship"); + assert_eq!(graph.goal(), "Ship the feature"); + assert_eq!( + graph.node("plan").map(|node| node.kind), + Some(StageHandler::Agent) + ); + assert_eq!(graph.edges.len(), 1); + assert_eq!(serde_json::to_value(&graph).unwrap(), value); +} + +#[test] +fn a_graph_with_only_a_name_is_the_schema_minimum() { + let graph: RunGraph = serde_json::from_value(json!({ "name": "Bare" })).unwrap(); + assert_eq!(graph, RunGraph::new("Bare")); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/foundation/fabro-types/src/graph.rs b/lib/foundation/fabro-types/src/graph.rs index 3cbbd4906..fe894802c 100644 --- a/lib/foundation/fabro-types/src/graph.rs +++ b/lib/foundation/fabro-types/src/graph.rs @@ -1,86 +1,16 @@ +//! The workflow graph as written: the typed model `fabro_graphviz::parser` +//! produces from a DOT file. +//! +//! This is the graph the bundler and workflow version registration walk to +//! find what a workflow references (`import`, `stack.child_workflow`, +//! `@file` prompts, the goal, the model stylesheet). Petri compiles and +//! admits the workflow; the graph a run displays is [`crate::RunGraph`], +//! read off Petri's admitted graph, not this model. + use std::collections::HashMap; use std::time::Duration; use serde::{Deserialize, Serialize}; -use strum::VariantNames; - -use crate::AgentBackend; - -/// Policy for a failed node when no explicit recovery route matches. -/// -/// Explicit routes (a jump, a matching edge condition, a matching preferred -/// label, or a matching suggested next node) take priority under every -/// policy. The policy decides what happens when none of them match. -#[derive( - Debug, - Clone, - Copy, - Default, - PartialEq, - Eq, - Serialize, - Deserialize, - strum::Display, - strum::EnumString, - strum::IntoStaticStr, - strum::VariantNames, -)] -#[serde(rename_all = "snake_case")] -#[strum(serialize_all = "snake_case")] -pub enum OnFailure { - /// The outcome stays `failed` and may take an unconditional edge. - #[default] - Route, - /// The outcome stays `failed` and skips the unconditional edge, so the - /// run ends unless a retry target applies. - Exit, - /// The outcome becomes `succeeded` and follows normal success routing. - /// The original failure details stay on the outcome for observability. - Succeed, -} - -impl OnFailure { - #[must_use] - pub fn expected_values() -> String { - ::VARIANTS.join(", ") - } -} - -/// A failure routing policy together with the scope that supplied it, so -/// failure messages can name the attribute that stopped routing. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct ResolvedOnFailure { - policy: OnFailure, - scope: AttributeScope, -} - -impl ResolvedOnFailure { - #[must_use] - pub const fn node(policy: OnFailure) -> Self { - Self { - policy, - scope: AttributeScope::Node, - } - } - - #[must_use] - pub const fn graph(policy: OnFailure) -> Self { - Self { - policy, - scope: AttributeScope::Graph, - } - } - - #[must_use] - pub const fn policy(self) -> OnFailure { - self.policy - } - - #[must_use] - pub const fn scope(self) -> AttributeScope { - self.scope - } -} /// Typed attribute values for nodes, edges, and graph-level attributes. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -132,46 +62,6 @@ impl AttrValue { _ => None, } } - - /// Convert any variant to its string representation. - #[must_use] - pub fn to_string_value(&self) -> String { - match self { - Self::String(s) => s.clone(), - Self::Integer(n) => n.to_string(), - Self::Float(f) => f.to_string(), - Self::Boolean(b) => b.to_string(), - Self::Duration(d) => format!("{}ms", d.as_millis()), - } - } -} - -/// Returns true if the handler type is an LLM-based handler (agent or prompt). -#[must_use] -pub fn is_llm_handler_type(handler_type: Option<&str>) -> bool { - matches!(handler_type, Some("agent" | "prompt")) -} - -pub const KNOWN_HANDLER_TYPES: &[&str] = &[ - "start", - "exit", - "agent", - "prompt", - "human", - "conditional", - "parallel", - "parallel.fan_in", - "command", - "tool", - "stack.manager_loop", - "wait", -]; - -/// Returns true if the handler type is part of Fabro's built-in handler -/// vocabulary. -#[must_use] -pub fn is_known_handler_type(handler_type: &str) -> bool { - KNOWN_HANDLER_TYPES.contains(&handler_type) } /// Maps Graphviz shapes to handler type strings (Section 2.8). @@ -193,19 +83,6 @@ pub fn shape_to_handler_type(shape: &str) -> Option<&'static str> { } } -/// Presence and validity of a node attribute whose value names a workflow -/// context key (`for_each`, `stdin_source`). -/// -/// Consumers need three states: the attribute is not set, it is set but not a -/// usable key (non-string or blank), or it carries a key. Modeling this once -/// keeps lint rules and handlers agreeing on what "valid" means. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ContextKeyAttr<'a> { - Absent, - Invalid, - Present(&'a str), -} - /// A node in the workflow graph. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Node { @@ -228,16 +105,10 @@ impl Node { /// Appends a class, ignoring blank names and ones already present. /// - /// Classes accumulate from several sources — the `class` attribute, - /// enclosing subgraphs, and import placeholders — so every caller needs the - /// same de-duplicating append. - /// - /// The name is trimmed, and a name that is empty or only whitespace is - /// dropped. Stylesheet selectors match class names exactly, so a padded - /// name would never match any rule. - /// - /// Order is preserved because the first class is meaningful: it supplies - /// the fallback thread ID for fidelity threading. + /// Classes accumulate from several sources — the `class` attribute and + /// enclosing subgraphs — so every caller needs the same de-duplicating + /// append. The name is trimmed, and a name that is empty or only + /// whitespace is dropped. Order is preserved. pub fn add_class(&mut self, class: &str) { let class = class.trim(); if !class.is_empty() && !self.classes.iter().any(|existing| existing == class) { @@ -249,14 +120,6 @@ impl Node { self.attrs.get(key).and_then(AttrValue::as_str) } - fn bool_attr(&self, key: &str) -> Option { - self.attrs.get(key).and_then(AttrValue::as_bool) - } - - fn int_attr(&self, key: &str) -> Option { - self.attrs.get(key).and_then(AttrValue::as_i64) - } - #[must_use] pub fn label(&self) -> &str { self.str_attr("label").unwrap_or(&self.id) @@ -288,177 +151,6 @@ impl Node { self.str_attr("prompt") } - /// The shell or Python source a command node runs. - #[must_use] - pub fn script(&self) -> Option<&str> { - self.str_attr("script") - } - - /// The prompt a handler should send, falling back to the node label when - /// `prompt` is absent or empty. - #[must_use] - pub fn prompt_or_label(&self) -> &str { - self.prompt() - .filter(|prompt| !prompt.is_empty()) - .unwrap_or_else(|| self.label()) - } - - #[must_use] - pub fn for_each(&self) -> Option<&str> { - self.str_attr("for_each") - } - - #[must_use] - pub fn context_key_attr(&self, name: &str) -> ContextKeyAttr<'_> { - let Some(value) = self.attrs.get(name) else { - return ContextKeyAttr::Absent; - }; - match value.as_str() { - Some(source) if !source.trim().is_empty() => ContextKeyAttr::Present(source), - _ => ContextKeyAttr::Invalid, - } - } - - #[must_use] - pub fn output_schema(&self) -> Option<&str> { - self.str_attr("output_schema") - } - - #[must_use] - pub fn output_retries(&self) -> i64 { - self.int_attr("output_retries").unwrap_or(2).max(0) - } - - #[must_use] - pub fn max_retries(&self) -> Option { - self.int_attr("max_retries") - } - - #[must_use] - pub fn max_visits(&self) -> Option { - self.int_attr("max_visits") - } - - #[must_use] - pub fn goal_gate(&self) -> bool { - self.bool_attr("goal_gate").unwrap_or(false) - } - - #[must_use] - pub fn review_target(&self) -> bool { - self.bool_attr("review_target").unwrap_or(false) - } - - #[must_use] - pub fn retry_target(&self) -> Option<&str> { - self.str_attr("retry_target") - } - - #[must_use] - pub fn fallback_retry_target(&self) -> Option<&str> { - self.str_attr("fallback_retry_target") - } - - /// Node-level failure policy override. `None` means the node inherits - /// the graph-level policy. Invalid values are rejected during workflow - /// validation, so runtime resolution treats them as absent. - /// - /// The deprecated `auto_status=true` attribute is a compatibility alias - /// for `on_failure="succeed"`. An explicit `on_failure` attribute wins. - #[must_use] - pub fn on_failure(&self) -> Option { - match self.attrs.get("on_failure") { - Some(value) => value.as_str().and_then(|value| value.parse().ok()), - None => self.auto_status().then_some(OnFailure::Succeed), - } - } - - #[must_use] - pub fn fidelity(&self) -> Option<&str> { - self.str_attr("fidelity") - } - - #[must_use] - pub fn thread_id(&self) -> Option<&str> { - self.str_attr("thread_id") - } - - pub fn timeout(&self) -> Option { - self.attrs.get("timeout").and_then(AttrValue::as_duration) - } - - #[must_use] - pub fn model(&self) -> Option<&str> { - self.str_attr("model") - } - - #[must_use] - pub fn provider(&self) -> Option<&str> { - self.str_attr("provider") - } - - #[must_use] - pub fn max_tokens(&self) -> Option { - self.int_attr("max_tokens").filter(|&v| v > 0) - } - - #[must_use] - pub fn speed(&self) -> Option<&str> { - self.str_attr("speed") - } - - /// Deprecated spelling of `on_failure="succeed"`. Validation warns when - /// it is present; [`Node::on_failure`] resolves the alias at runtime. - #[must_use] - pub fn auto_status(&self) -> bool { - self.bool_attr("auto_status").unwrap_or(false) - } - - #[must_use] - pub fn allow_partial(&self) -> bool { - self.bool_attr("allow_partial").unwrap_or(false) - } - - #[must_use] - pub fn project_memory(&self) -> bool { - self.bool_attr("project_memory").unwrap_or(true) - } - - #[must_use] - pub fn retry_policy(&self) -> Option<&str> { - self.str_attr("retry_policy") - } - - #[must_use] - pub fn backend(&self) -> Option<&str> { - self.str_attr("backend") - } - - #[must_use] - pub fn agent_backend(&self) -> Option> { - self.backend().map(str::parse) - } - - #[must_use] - pub fn legacy_acp_command_attr(&self) -> Option<&str> { - self.str_attr("acp_command") - } - - #[must_use] - pub fn acp_command_attr(&self) -> Option<&str> { - self.str_attr("acp.command") - } - - #[must_use] - pub fn acp_config_attr(&self) -> Option<&str> { - self.str_attr("acp.config") - } - - #[must_use] - pub fn selection(&self) -> &str { - self.str_attr("selection").unwrap_or("deterministic") - } - /// Resolve the handler type for this node using explicit type or shape /// mapping. #[must_use] @@ -493,14 +185,6 @@ impl Edge { self.attrs.get(key).and_then(AttrValue::as_str) } - fn bool_attr(&self, key: &str) -> Option { - self.attrs.get(key).and_then(AttrValue::as_bool) - } - - fn int_attr(&self, key: &str) -> Option { - self.attrs.get(key).and_then(AttrValue::as_i64) - } - #[must_use] pub fn label(&self) -> Option<&str> { self.str_attr("label") @@ -510,31 +194,6 @@ impl Edge { pub fn condition(&self) -> Option<&str> { self.str_attr("condition") } - - #[must_use] - pub fn weight(&self) -> i64 { - self.int_attr("weight").unwrap_or(0) - } - - #[must_use] - pub fn fidelity(&self) -> Option<&str> { - self.str_attr("fidelity") - } - - #[must_use] - pub fn thread_id(&self) -> Option<&str> { - self.str_attr("thread_id") - } - - #[must_use] - pub fn loop_restart(&self) -> bool { - self.bool_attr("loop_restart").unwrap_or(false) - } - - #[must_use] - pub fn freeform(&self) -> bool { - self.bool_attr("freeform").unwrap_or(false) - } } /// The parsed workflow graph containing nodes, edges, and graph-level @@ -557,44 +216,6 @@ impl Graph { } } - /// Returns all outgoing edges from the given node. - #[must_use] - pub fn outgoing_edges(&self, node_id: &str) -> Vec<&Edge> { - self.edges.iter().filter(|e| e.from == node_id).collect() - } - - /// Returns all incoming edges to the given node. - #[must_use] - pub fn incoming_edges(&self, node_id: &str) -> Vec<&Edge> { - self.edges.iter().filter(|e| e.to == node_id).collect() - } - - /// Find the start node: shape=Mdiamond, or id "start"/"Start". - #[must_use] - pub fn find_start_node(&self) -> Option<&Node> { - // First: look for shape=Mdiamond - let by_shape = self.nodes.values().find(|n| n.shape() == "Mdiamond"); - if by_shape.is_some() { - return by_shape; - } - // Second: look for id "start" or "Start" - self.nodes.get("start").or_else(|| self.nodes.get("Start")) - } - - /// Find the exit node: shape=Msquare, or id "exit"/"Exit". - #[must_use] - pub fn find_exit_node(&self) -> Option<&Node> { - let by_shape = self.nodes.values().find(|n| n.shape() == "Msquare"); - if by_shape.is_some() { - return by_shape; - } - self.nodes - .get("exit") - .or_else(|| self.nodes.get("Exit")) - .or_else(|| self.nodes.get("end")) - .or_else(|| self.nodes.get("End")) - } - /// Graph-level goal attribute. pub fn goal(&self) -> &str { self.attrs @@ -610,100 +231,6 @@ impl Graph { .and_then(AttrValue::as_str) .unwrap_or("") } - - /// Graph-level `default_max_retries` (default 0). - pub fn default_max_retries(&self) -> i64 { - self.attrs - .get("default_max_retries") - .and_then(AttrValue::as_i64) - .unwrap_or(0) - } - - /// Graph-level `retry_target`. - pub fn retry_target(&self) -> Option<&str> { - self.attrs.get("retry_target").and_then(AttrValue::as_str) - } - - /// Graph-level `fallback_retry_target`. - pub fn fallback_retry_target(&self) -> Option<&str> { - self.attrs - .get("fallback_retry_target") - .and_then(AttrValue::as_str) - } - - /// Graph-level failure policy. Invalid values are rejected during - /// workflow validation, so runtime resolution can use the compatibility - /// default. - #[must_use] - pub fn on_failure(&self) -> OnFailure { - self.attrs - .get("on_failure") - .and_then(AttrValue::as_str) - .and_then(|value| value.parse().ok()) - .unwrap_or_default() - } - - /// Effective failure policy for a node. A node-level `on_failure` - /// attribute overrides the graph level; an absent (or invalid, hence - /// validation-rejected) node attribute inherits the graph policy. - #[must_use] - pub fn resolve_on_failure(&self, node: &Node) -> ResolvedOnFailure { - match node.on_failure() { - Some(policy) => ResolvedOnFailure::node(policy), - None => ResolvedOnFailure::graph(self.on_failure()), - } - } - - /// Graph-level `default_fidelity`. - pub fn default_fidelity(&self) -> Option<&str> { - self.attrs - .get("default_fidelity") - .and_then(AttrValue::as_str) - } - - /// Graph-level `default_thread`. - pub fn default_thread(&self) -> Option<&str> { - self.attrs.get("default_thread").and_then(AttrValue::as_str) - } - - /// Graph-level `loop_restart_signature_limit` (default 3). - /// When the same failure signature repeats this many times, the pipeline - /// aborts. - pub fn loop_restart_signature_limit(&self) -> usize { - #[allow( - clippy::cast_possible_truncation, - clippy::cast_sign_loss, - reason = "Values below 1 are filtered out before this usize conversion." - )] - self.attrs - .get("loop_restart_signature_limit") - .and_then(AttrValue::as_i64) - .filter(|&v| v >= 1) - .map_or(3, |v| v as usize) - } - - /// Graph-level `stall_timeout`. Defaults to 1800s. Returns `None` when set - /// to zero (disabled). - pub fn stall_timeout(&self) -> Option { - match self - .attrs - .get("stall_timeout") - .and_then(AttrValue::as_duration) - { - Some(d) if d.is_zero() => None, - Some(d) => Some(d), - None => Some(Duration::from_mins(30)), - } - } - - /// Graph-level `max_node_visits` (default 0 = disabled). - pub fn max_node_visits(&self) -> u64 { - self.attrs - .get("max_node_visits") - .and_then(AttrValue::as_i64) - .and_then(|n| u64::try_from(n).ok()) - .unwrap_or(0) - } } /// Where an attribute appears in a workflow graph. @@ -784,157 +311,20 @@ mod tests { use super::*; #[test] - fn on_failure_parses_and_displays_supported_values() { - assert_eq!("route".parse::().unwrap(), OnFailure::Route); - assert_eq!("exit".parse::().unwrap(), OnFailure::Exit); - assert_eq!("succeed".parse::().unwrap(), OnFailure::Succeed); - assert_eq!(OnFailure::Route.to_string(), "route"); - assert_eq!(OnFailure::Exit.to_string(), "exit"); - assert_eq!(OnFailure::Succeed.to_string(), "succeed"); - assert_eq!(OnFailure::expected_values(), "route, exit, succeed"); - } - - #[test] - fn graph_on_failure_defaults_to_route_and_resolves_explicit_values() { - let mut graph = Graph::new("test"); - assert_eq!(graph.on_failure(), OnFailure::Route); - - graph.attrs.insert( - "on_failure".to_string(), - AttrValue::String("route".to_string()), - ); - assert_eq!(graph.on_failure(), OnFailure::Route); - - graph.attrs.insert( - "on_failure".to_string(), - AttrValue::String("exit".to_string()), - ); - assert_eq!(graph.on_failure(), OnFailure::Exit); - } - - #[test] - fn node_on_failure_parses_valid_values_and_ignores_invalid_ones() { - let mut node = Node::new("work"); - assert_eq!(node.on_failure(), None); - - node.attrs.insert( - "on_failure".to_string(), - AttrValue::String("exit".to_string()), - ); - assert_eq!(node.on_failure(), Some(OnFailure::Exit)); - - node.attrs.insert( - "on_failure".to_string(), - AttrValue::String("stop".to_string()), - ); - assert_eq!(node.on_failure(), None); - - node.attrs - .insert("on_failure".to_string(), AttrValue::Boolean(true)); - assert_eq!(node.on_failure(), None); - } - - #[test] - fn node_auto_status_is_an_alias_for_on_failure_succeed() { - let mut node = Node::new("work"); - node.attrs - .insert("auto_status".to_string(), AttrValue::Boolean(true)); - assert!(node.auto_status()); - assert_eq!(node.on_failure(), Some(OnFailure::Succeed)); - - // An explicit on_failure attribute wins over the alias. - node.attrs.insert( - "on_failure".to_string(), - AttrValue::String("exit".to_string()), - ); - assert_eq!(node.on_failure(), Some(OnFailure::Exit)); - - // auto_status=false does not set a policy. - let mut node = Node::new("work"); - node.attrs - .insert("auto_status".to_string(), AttrValue::Boolean(false)); - assert_eq!(node.on_failure(), None); - } - - #[test] - fn explicit_invalid_on_failure_does_not_fall_back_to_auto_status() { - for value in [ - AttrValue::String("invalid".to_string()), - AttrValue::Boolean(true), - ] { - let mut node = Node::new("work"); - node.attrs - .insert("auto_status".to_string(), AttrValue::Boolean(true)); - node.attrs.insert("on_failure".to_string(), value); - - assert_eq!(node.on_failure(), None); - } - } - - #[test] - fn resolve_on_failure_prefers_node_policy_over_graph_policy() { - let mut graph = Graph::new("test"); - graph.attrs.insert( - "on_failure".to_string(), - AttrValue::String("exit".to_string()), - ); - graph.nodes.insert("bare".to_string(), Node::new("bare")); - let mut invalid = Node::new("invalid"); - invalid.attrs.insert( - "on_failure".to_string(), - AttrValue::String("bogus".to_string()), - ); - graph.nodes.insert("invalid".to_string(), invalid); - let mut route = Node::new("route"); - route.attrs.insert( - "on_failure".to_string(), - AttrValue::String("route".to_string()), - ); - graph.nodes.insert("route".to_string(), route); - - // Node attribute wins over the graph policy. + fn attr_value_accessors_match_their_variant() { assert_eq!( - graph.resolve_on_failure(&graph.nodes["route"]), - ResolvedOnFailure::node(OnFailure::Route) + AttrValue::String("hello".to_string()).as_str(), + Some("hello") ); - // Absent and invalid node attributes inherit the graph policy. - for node_id in ["bare", "invalid"] { - assert_eq!( - graph.resolve_on_failure(&graph.nodes[node_id]), - ResolvedOnFailure::graph(OnFailure::Exit) - ); - } - } - - #[test] - fn attr_value_as_str() { - let val = AttrValue::String("hello".to_string()); - assert_eq!(val.as_str(), Some("hello")); assert_eq!(AttrValue::Integer(1).as_str(), None); - } - - #[test] - fn attr_value_as_i64() { assert_eq!(AttrValue::Integer(42).as_i64(), Some(42)); assert_eq!(AttrValue::String("x".to_string()).as_i64(), None); - } - - #[test] - fn attr_value_as_f64() { assert_eq!(AttrValue::Float(3.15).as_f64(), Some(3.15)); assert_eq!(AttrValue::Integer(1).as_f64(), None); - } - - #[test] - fn attr_value_as_bool() { assert_eq!(AttrValue::Boolean(true).as_bool(), Some(true)); assert_eq!(AttrValue::String("true".to_string()).as_bool(), None); - } - - #[test] - fn attr_value_as_duration() { - let d = Duration::from_secs(10); - assert_eq!(AttrValue::Duration(d).as_duration(), Some(d)); + let ten = Duration::from_secs(10); + assert_eq!(AttrValue::Duration(ten).as_duration(), Some(ten)); assert_eq!(AttrValue::Integer(10).as_duration(), None); } @@ -957,15 +347,6 @@ mod tests { assert_eq!(shape_to_handler_type("unknown"), None); } - #[test] - fn is_llm_handler_type_checks() { - assert!(is_llm_handler_type(Some("agent"))); - assert!(is_llm_handler_type(Some("prompt"))); - assert!(!is_llm_handler_type(Some("command"))); - assert!(!is_llm_handler_type(Some("human"))); - assert!(!is_llm_handler_type(None)); - } - #[test] fn node_defaults() { let node = Node::new("test"); @@ -974,31 +355,8 @@ mod tests { assert_eq!(node.shape(), "box"); assert_eq!(node.node_type(), None); assert_eq!(node.prompt(), None); - assert_eq!(node.script(), None); - assert_eq!(node.for_each(), None); - assert_eq!( - node.context_key_attr("stdin_source"), - ContextKeyAttr::Absent - ); - assert_eq!(node.output_schema(), None); - assert_eq!(node.output_retries(), 2); - assert_eq!(node.max_retries(), None); - assert!(!node.goal_gate()); - assert!(!node.review_target()); - assert_eq!(node.retry_target(), None); - assert_eq!(node.fallback_retry_target(), None); - assert_eq!(node.fidelity(), None); - assert_eq!(node.thread_id(), None); assert!(node.classes.is_empty()); - assert_eq!(node.timeout(), None); - assert_eq!(node.model(), None); - assert_eq!(node.provider(), None); - assert_eq!(node.speed(), None); - assert!(!node.auto_status()); - assert!(!node.allow_partial()); - assert_eq!(node.retry_policy(), None); - assert_eq!(node.max_visits(), None); - assert!(node.project_memory()); + assert_eq!(node.handler_type(), Some("agent")); } #[test] @@ -1034,27 +392,22 @@ mod tests { let node = node_with("plan", &[("prompt", "Plan the work")]); assert_eq!(node.shape(), "box"); assert_eq!(node.handler_type(), Some("agent")); + assert_eq!(node.prompt(), Some("Plan the work")); } #[test] - fn explicit_shape_wins_over_script_inference() { - let node = node_with("odd", &[("shape", "box"), ("script", "cargo build")]); - assert_eq!(node.shape(), "box"); - assert_eq!(node.handler_type(), Some("agent")); - } + fn explicit_shape_or_type_wins_over_script_inference() { + let shaped = node_with("odd", &[("shape", "box"), ("script", "cargo build")]); + assert_eq!(shaped.shape(), "box"); + assert_eq!(shaped.handler_type(), Some("agent")); - #[test] - fn explicit_type_wins_over_script_inference() { - let node = node_with("odd", &[("type", "agent"), ("script", "cargo build")]); - assert_eq!(node.shape(), "box"); - assert_eq!(node.handler_type(), Some("agent")); + let typed = node_with("odd", &[("type", "agent"), ("script", "cargo build")]); + assert_eq!(typed.shape(), "box"); + assert_eq!(typed.handler_type(), Some("agent")); } #[test] fn any_script_attribute_value_infers_command() { - // The command-requires-script lint reports this; inference only asks - // whether the attribute is present so the diagnostic lands on a - // command node rather than a silently-agent one. let empty = node_with("empty", &[("script", "")]); assert_eq!(empty.shape(), "parallelogram"); assert_eq!(empty.handler_type(), Some("command")); @@ -1068,160 +421,30 @@ mod tests { } #[test] - fn legacy_tool_type_resolves_to_command() { - let node = node_with("build", &[("type", "tool")]); - assert_eq!(node.handler_type(), Some("command")); - } - - #[test] - fn node_project_memory_false_overrides_default() { - let mut node = Node::new("x"); - node.attrs - .insert("project_memory".to_string(), AttrValue::Boolean(false)); - assert!(!node.project_memory()); - } - - #[test] - fn node_output_retries_defaults_and_clamps_to_zero() { - let mut node = Node::new("x"); - assert_eq!(node.output_retries(), 2); - - node.attrs - .insert("output_retries".to_string(), AttrValue::Integer(0)); - assert_eq!(node.output_retries(), 0); - - node.attrs - .insert("output_retries".to_string(), AttrValue::Integer(-3)); - assert_eq!(node.output_retries(), 0); - } - - #[test] - fn node_output_schema_returns_string_attr() { - let mut node = Node::new("x"); - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String("routing".to_string()), - ); - - assert_eq!(node.output_schema(), Some("routing")); - } - - #[test] - fn node_prompt_or_label_falls_back_on_absent_and_empty_prompts() { - let mut node = Node::new("review"); - assert_eq!(node.prompt_or_label(), node.label()); - - node.attrs - .insert("prompt".to_string(), AttrValue::String(String::new())); - assert_eq!(node.prompt_or_label(), node.label()); - - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Review the diff.".to_string()), - ); - assert_eq!(node.prompt_or_label(), "Review the diff."); - } - - #[test] - fn node_for_each_returns_context_source() { - let mut node = Node::new("fanout"); - node.attrs.insert( - "for_each".to_string(), - AttrValue::String("context.candidates".to_string()), - ); - - assert_eq!(node.for_each(), Some("context.candidates")); - } - - #[test] - fn node_context_key_attr_classifies_presence_and_validity() { - let mut node = Node::new("merge"); - node.attrs.insert( - "stdin_source".to_string(), - AttrValue::String("context.parallel.results".to_string()), - ); - + fn explicit_types_and_shapes_resolve_handler_types() { assert_eq!( - node.context_key_attr("stdin_source"), - ContextKeyAttr::Present("context.parallel.results") + node_with("build", &[("type", "tool")]).handler_type(), + Some("command") ); - - for invalid in [AttrValue::String(" ".to_string()), AttrValue::Integer(3)] { - node.attrs.insert("stdin_source".to_string(), invalid); - assert_eq!( - node.context_key_attr("stdin_source"), - ContextKeyAttr::Invalid - ); - } - } - - #[test] - fn node_with_attrs() { - let mut node = Node::new("plan"); - node.attrs.insert( - "label".to_string(), - AttrValue::String("Plan step".to_string()), + assert_eq!( + node_with("gate", &[("type", "human")]).handler_type(), + Some("human") ); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("diamond".to_string()), + assert_eq!( + node_with("entry", &[("shape", "Mdiamond")]).handler_type(), + Some("start") ); - node.attrs - .insert("goal_gate".to_string(), AttrValue::Boolean(true)); - node.attrs - .insert("review_target".to_string(), AttrValue::Boolean(true)); - node.attrs - .insert("max_retries".to_string(), AttrValue::Integer(3)); - - assert_eq!(node.label(), "Plan step"); - assert_eq!(node.shape(), "diamond"); - assert!(node.goal_gate()); - assert!(node.review_target()); - assert_eq!(node.max_retries(), Some(3)); + assert_eq!(node_with("odd", &[("shape", "star")]).handler_type(), None); } #[test] - fn node_max_visits_returns_value() { - let mut node = Node::new("test"); - node.attrs - .insert("max_visits".to_string(), AttrValue::Integer(5)); - assert_eq!(node.max_visits(), Some(5)); - } + fn edge_attributes_are_read_as_written() { + let bare = Edge::new("a", "b"); + assert_eq!(bare.from, "a"); + assert_eq!(bare.to, "b"); + assert_eq!(bare.label(), None); + assert_eq!(bare.condition(), None); - #[test] - fn node_handler_type_explicit() { - let mut node = Node::new("gate"); - node.attrs - .insert("type".to_string(), AttrValue::String("human".to_string())); - assert_eq!(node.handler_type(), Some("human")); - } - - #[test] - fn node_handler_type_from_shape() { - let mut node = Node::new("entry"); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - assert_eq!(node.handler_type(), Some("start")); - } - - #[test] - fn edge_defaults() { - let edge = Edge::new("a", "b"); - assert_eq!(edge.from, "a"); - assert_eq!(edge.to, "b"); - assert_eq!(edge.label(), None); - assert_eq!(edge.condition(), None); - assert_eq!(edge.weight(), 0); - assert_eq!(edge.fidelity(), None); - assert_eq!(edge.thread_id(), None); - assert!(!edge.loop_restart()); - assert!(!edge.freeform()); - } - - #[test] - fn edge_with_attrs() { let mut edge = Edge::new("a", "b"); edge.attrs .insert("label".to_string(), AttrValue::String("next".to_string())); @@ -1229,199 +452,27 @@ mod tests { "condition".to_string(), AttrValue::String("outcome=succeeded".to_string()), ); - edge.attrs - .insert("weight".to_string(), AttrValue::Integer(5)); - edge.attrs - .insert("loop_restart".to_string(), AttrValue::Boolean(true)); - edge.attrs - .insert("freeform".to_string(), AttrValue::Boolean(true)); - assert_eq!(edge.label(), Some("next")); assert_eq!(edge.condition(), Some("outcome=succeeded")); - assert_eq!(edge.weight(), 5); - assert!(edge.loop_restart()); - assert!(edge.freeform()); } - fn sample_graph() -> Graph { - let mut g = Graph::new("test_pipeline"); + #[test] + fn graph_goal_and_stylesheet_default_to_empty() { + let mut graph = Graph::new("test"); + assert_eq!(graph.name, "test"); + assert_eq!(graph.goal(), ""); + assert_eq!(graph.model_stylesheet(), ""); - let mut start = Node::new("start"); - start.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - g.nodes.insert("start".to_string(), start); - - let mut exit = Node::new("exit"); - exit.attrs.insert( - "shape".to_string(), - AttrValue::String("Msquare".to_string()), - ); - g.nodes.insert("exit".to_string(), exit); - - let work = Node::new("work"); - g.nodes.insert("work".to_string(), work); - - g.edges.push(Edge::new("start", "work")); - g.edges.push(Edge::new("work", "exit")); - - g.attrs.insert( + graph.attrs.insert( "goal".to_string(), AttrValue::String("Run tests".to_string()), ); - - g - } - - #[test] - fn graph_find_start_node() { - let g = sample_graph(); - let start = g.find_start_node().unwrap(); - assert_eq!(start.id, "start"); - } - - #[test] - fn graph_find_exit_node() { - let g = sample_graph(); - let exit = g.find_exit_node().unwrap(); - assert_eq!(exit.id, "exit"); - } - - #[test] - fn graph_find_exit_by_end_id() { - let mut g = Graph::new("test"); - let node = Node::new("end"); - g.nodes.insert("end".to_string(), node); - let exit = g.find_exit_node().unwrap(); - assert_eq!(exit.id, "end"); - } - - #[test] - fn graph_outgoing_edges() { - let g = sample_graph(); - let edges = g.outgoing_edges("start"); - assert_eq!(edges.len(), 1); - assert_eq!(edges[0].to, "work"); - } - - #[test] - fn graph_incoming_edges() { - let g = sample_graph(); - let edges = g.incoming_edges("exit"); - assert_eq!(edges.len(), 1); - assert_eq!(edges[0].from, "work"); - } - - #[test] - fn graph_goal() { - let g = sample_graph(); - assert_eq!(g.goal(), "Run tests"); - } - - #[test] - fn graph_goal_default() { - let g = Graph::new("empty"); - assert_eq!(g.goal(), ""); - } - - #[test] - fn graph_model_stylesheet_default() { - let g = Graph::new("empty"); - assert_eq!(g.model_stylesheet(), ""); - } - - #[test] - fn graph_default_max_retries() { - let g = Graph::new("empty"); - assert_eq!(g.default_max_retries(), 0); - } - - #[test] - fn graph_find_start_by_id_fallback() { - let mut g = Graph::new("test"); - // No Mdiamond shape, but id is "start" - let node = Node::new("start"); - g.nodes.insert("start".to_string(), node); - assert!(g.find_start_node().is_some()); - } - - #[test] - fn graph_no_start_node() { - let g = Graph::new("empty"); - assert!(g.find_start_node().is_none()); - } - - #[test] - fn graph_stall_timeout_default() { - let g = Graph::new("empty"); - assert_eq!(g.stall_timeout(), Some(Duration::from_mins(30))); - } - - #[test] - fn graph_stall_timeout_set() { - let mut g = Graph::new("test"); - g.attrs.insert( - "stall_timeout".to_string(), - AttrValue::Duration(Duration::from_millis(200)), + graph.attrs.insert( + "model_stylesheet".to_string(), + AttrValue::String("* { model: gpt-5.4; }".to_string()), ); - assert_eq!(g.stall_timeout(), Some(Duration::from_millis(200))); - } - - #[test] - fn graph_stall_timeout_zero_disables() { - let mut g = Graph::new("test"); - g.attrs.insert( - "stall_timeout".to_string(), - AttrValue::Duration(Duration::ZERO), - ); - assert_eq!(g.stall_timeout(), None); - } - - #[test] - fn graph_max_node_visits_default() { - let g = Graph::new("empty"); - assert_eq!(g.max_node_visits(), 0); - } - - #[test] - fn graph_max_node_visits_set() { - let mut g = Graph::new("test"); - g.attrs - .insert("max_node_visits".to_string(), AttrValue::Integer(10)); - assert_eq!(g.max_node_visits(), 10); - } - - #[test] - fn graph_loop_restart_signature_limit_default() { - let g = Graph::new("empty"); - assert_eq!(g.loop_restart_signature_limit(), 3); - } - - #[test] - fn graph_loop_restart_signature_limit_set() { - let mut g = Graph::new("test"); - g.attrs.insert( - "loop_restart_signature_limit".to_string(), - AttrValue::Integer(5), - ); - assert_eq!(g.loop_restart_signature_limit(), 5); - } - - #[test] - fn graph_loop_restart_signature_limit_invalid_falls_back() { - let mut g = Graph::new("test"); - g.attrs.insert( - "loop_restart_signature_limit".to_string(), - AttrValue::Integer(0), - ); - assert_eq!(g.loop_restart_signature_limit(), 3); - - g.attrs.insert( - "loop_restart_signature_limit".to_string(), - AttrValue::Integer(-1), - ); - assert_eq!(g.loop_restart_signature_limit(), 3); + assert_eq!(graph.goal(), "Run tests"); + assert_eq!(graph.model_stylesheet(), "* { model: gpt-5.4; }"); } #[test] diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index d2dfd262b..2563e0c33 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -31,6 +31,7 @@ pub mod pull_request; pub mod repository; pub mod run; pub mod run_failure; +pub mod run_graph; pub mod run_id; pub mod run_intent; pub mod run_projection; @@ -83,10 +84,7 @@ pub use diff::{DiffStats, DiffSummary, RunDiff}; pub use engine::{PetriAdmission, PetriGraphRef}; pub use failure_signature::FailureSignature; pub use git_identity::{GitIdentity, GitIdentitySource}; -pub use graph::{ - AttrValue, AttributeScope, ContextKeyAttr, Edge, Graph, KNOWN_HANDLER_TYPES, Node, OnFailure, - ResolvedOnFailure, is_known_handler_type, is_llm_handler_type, shape_to_handler_type, -}; +pub use graph::{AttrValue, AttributeScope, Edge, Graph, Node, shape_to_handler_type}; pub use input_scalar::{ JsonScalarToTomlError, TomlScalarToJsonError, json_scalar_to_toml_value, toml_scalar_to_json_value, @@ -140,6 +138,7 @@ pub use run::{ RunServerProvenance, RunSpec, }; pub use run_failure::RunFailure; +pub use run_graph::{RunGraph, RunGraphEdge, RunGraphNode}; pub use run_id::{RunId, fixtures}; pub use run_intent::{ GitCoordinateValidationError, GitRunTarget, RunIntent, RunIntentArgs, RunTarget, diff --git a/lib/foundation/fabro-types/src/outcome.rs b/lib/foundation/fabro-types/src/outcome.rs index 43f507317..206971eab 100644 --- a/lib/foundation/fabro-types/src/outcome.rs +++ b/lib/foundation/fabro-types/src/outcome.rs @@ -8,10 +8,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_json::Value; use strum::{Display, EnumString, IntoStaticStr}; -use crate::{ - ExecOutputTail, FailureSignature, ModelUsage, OnFailure, ResolvedOnFailure, StageTiming, - SystemActorKind, -}; +use crate::{ExecOutputTail, FailureSignature, ModelUsage, StageTiming, SystemActorKind}; pub trait OutcomeMeta: Default + Clone + Send + Sync + fmt::Debug + Serialize + DeserializeOwned + 'static @@ -338,90 +335,13 @@ impl Outcome { ..Self::default() } } - - /// Applies a resolved failure policy to this outcome. - /// - /// `succeed` promotes a `failed` outcome and records the policy scope. - /// The original `failure` stays available for durable diagnostics. Other - /// policies and statuses do not change the outcome. - pub fn apply_on_failure(&mut self, policy: ResolvedOnFailure) -> bool { - if policy.policy() != OnFailure::Succeed || !self.status.is_failure() { - return false; - } - self.status = StageOutcome::Succeeded; - let note = format!( - "{} on_failure=succeed promoted a failed outcome to succeeded", - policy.scope() - ); - self.notes = Some(match self.notes.take() { - Some(existing) => format!("{existing}\n{note}"), - None => note, - }); - true - } } #[cfg(test)] mod tests { use serde_json::json; - use super::{FailureCategory, FailureDetail, Outcome, StageOutcome, StageState}; - use crate::{OnFailure, ResolvedOnFailure}; - - #[test] - fn apply_on_failure_keeps_failure_and_records_scope() { - let mut outcome: Outcome = Outcome::fail("boom"); - assert!(outcome.apply_on_failure(ResolvedOnFailure::node(OnFailure::Succeed))); - - assert_eq!(outcome.status, StageOutcome::Succeeded); - assert_eq!( - outcome - .failure - .as_ref() - .map(|failure| failure.message.as_str()), - Some("boom") - ); - assert_eq!( - outcome.notes.as_deref(), - Some("node on_failure=succeed promoted a failed outcome to succeeded") - ); - } - - #[test] - fn apply_on_failure_appends_to_existing_notes() { - let mut outcome: Outcome = Outcome::fail("boom"); - outcome.notes = Some("handler note".to_string()); - assert!(outcome.apply_on_failure(ResolvedOnFailure::graph(OnFailure::Succeed))); - - assert_eq!( - outcome.notes.as_deref(), - Some("handler note\ngraph on_failure=succeed promoted a failed outcome to succeeded") - ); - } - - #[test] - fn apply_on_failure_ignores_non_failed_outcomes() { - let mut partial: Outcome = Outcome::success(); - partial.status = StageOutcome::PartiallySucceeded; - let mut skipped: Outcome = Outcome::skipped("not needed"); - - for outcome in [&mut partial, &mut skipped] { - let before = outcome.clone(); - assert!(!outcome.apply_on_failure(ResolvedOnFailure::node(OnFailure::Succeed))); - assert_eq!(*outcome, before); - } - } - - #[test] - fn apply_on_failure_ignores_non_succeed_policies() { - for policy in [OnFailure::Route, OnFailure::Exit] { - let mut outcome: Outcome = Outcome::fail("boom"); - let before = outcome.clone(); - - assert!(!outcome.apply_on_failure(ResolvedOnFailure::node(policy))); - assert_eq!(outcome, before); - } - } + use super::{FailureCategory, FailureDetail, StageOutcome, StageState}; #[test] fn stage_outcome_failed_serde_is_lossy_for_retry_intent() { diff --git a/lib/foundation/fabro-types/src/run.rs b/lib/foundation/fabro-types/src/run.rs index 996a8849a..43e8890ca 100644 --- a/lib/foundation/fabro-types/src/run.rs +++ b/lib/foundation/fabro-types/src/run.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; use crate::WorkflowSettings; use crate::blob_hash::BlobHash; use crate::engine::PetriAdmission; -use crate::graph::Graph; use crate::principal::Principal; +use crate::run_graph::RunGraph; use crate::run_id::RunId; use crate::run_intent::RunTarget; use crate::run_summary::AutomationRef; @@ -63,7 +63,9 @@ pub struct ForkSourceRef { pub struct RunSpec { pub run_id: RunId, pub settings: WorkflowSettings, - pub graph: Graph, + /// The display graph: what Petri admitted, reduced to what the read + /// side names. The DOT it was written in is `graph_source`. + pub graph: RunGraph, #[serde(default, skip_serializing_if = "Option::is_none")] pub graph_source: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -102,7 +104,7 @@ impl RunSpec { } #[must_use] - pub fn graph(&self) -> &Graph { + pub fn graph(&self) -> &RunGraph { &self.graph } diff --git a/lib/foundation/fabro-types/src/run_graph.rs b/lib/foundation/fabro-types/src/run_graph.rs new file mode 100644 index 000000000..de0e51c9d --- /dev/null +++ b/lib/foundation/fabro-types/src/run_graph.rs @@ -0,0 +1,124 @@ +//! The graph a run displays: what Petri admitted, reduced to the nodes and +//! edges the read side names. +//! +//! Petri lowers and admits every run's workflow at create time +//! (`RunSpec::admission` names the lowered graphs). The read side needs far +//! less than the lowered graph: the workflow's name and goal, each stage's +//! label and handler kind, and the routing edges as written. `RunGraph` is +//! that projection, built once from the admitted graph by `fabro-petri` and +//! stored on the run spec. The DOT the workflow was written in is beside it +//! as `RunSpec::graph_source`. + +use std::collections::BTreeMap; + +use serde::{Deserialize, Serialize}; + +use crate::stage_handler::StageHandler; + +/// The display graph of a run: the admitted workflow's name, goal, stages +/// and edges. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct RunGraph { + /// The workflow's name: the DOT `digraph` name. + pub name: String, + /// The run's goal as the run displays it. + #[serde(default)] + pub goal: String, + /// The stages by node id, in id order. + #[serde(default)] + pub nodes: BTreeMap, + /// The routing edges as written, one per arm. + #[serde(default)] + pub edges: Vec, +} + +/// One stage of the display graph. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RunGraphNode { + /// The node's display label: its `label` attribute, else its id. + pub label: String, + /// The handler the node runs as. + pub kind: StageHandler, +} + +/// One routing edge of the display graph. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RunGraphEdge { + pub from: String, + pub to: String, +} + +impl RunGraph { + #[must_use] + pub fn new(name: impl Into) -> Self { + Self { + name: name.into(), + ..Self::default() + } + } + + /// The run's goal; empty when the workflow has none. + #[must_use] + pub fn goal(&self) -> &str { + &self.goal + } + + #[must_use] + pub fn node(&self, id: &str) -> Option<&RunGraphNode> { + self.nodes.get(id) + } + + /// Whether `id` names a `start` or `exit` boundary: a node that runs no + /// work of its own. The test is the node's kind, never its name. + #[must_use] + pub fn is_boundary(&self, id: &str) -> bool { + self.node(id) + .is_some_and(|node| matches!(node.kind, StageHandler::Start | StageHandler::Exit)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn graph() -> RunGraph { + let mut graph = RunGraph::new("Ship"); + graph.goal = "Ship it".to_string(); + graph.nodes.insert("start".to_string(), RunGraphNode { + label: "Start".to_string(), + kind: StageHandler::Start, + }); + graph.nodes.insert("plan".to_string(), RunGraphNode { + label: "Plan".to_string(), + kind: StageHandler::Agent, + }); + graph.edges.push(RunGraphEdge { + from: "start".to_string(), + to: "plan".to_string(), + }); + graph + } + + #[test] + fn boundaries_are_by_kind_not_name() { + let mut graph = graph(); + assert!(graph.is_boundary("start")); + assert!(!graph.is_boundary("plan")); + assert!(!graph.is_boundary("missing")); + graph.nodes.get_mut("start").unwrap().kind = StageHandler::Agent; + assert!(!graph.is_boundary("start")); + } + + #[test] + fn the_wire_shape_round_trips_and_defaults_the_optional_parts() { + let graph = graph(); + let json = serde_json::to_value(&graph).unwrap(); + assert_eq!(json["nodes"]["plan"]["kind"], "agent"); + assert_eq!(json["edges"][0]["to"], "plan"); + let decoded: RunGraph = serde_json::from_value(json).unwrap(); + assert_eq!(decoded, graph); + + let bare: RunGraph = serde_json::from_value(serde_json::json!({ "name": "Bare" })).unwrap(); + assert_eq!(bare, RunGraph::new("Bare")); + } +} diff --git a/lib/foundation/fabro-types/src/run_projection.rs b/lib/foundation/fabro-types/src/run_projection.rs index 94221a723..339aed897 100644 --- a/lib/foundation/fabro-types/src/run_projection.rs +++ b/lib/foundation/fabro-types/src/run_projection.rs @@ -826,11 +826,7 @@ impl RunProjection { /// is the node's handler type, not its name: a node may be named /// `start` and still do real work. pub fn is_boundary_stage(&self, node_id: &str) -> bool { - self.spec() - .graph() - .nodes - .get(node_id) - .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit"))) + self.spec().graph().is_boundary(node_id) } pub fn status(&self) -> RunStatus { @@ -917,14 +913,12 @@ impl RunProjection { mod title_tests { use chrono::Utc; - use crate::{AttrValue, Graph, RunProjection, RunSpec, test_support}; + use crate::{RunGraph, RunProjection, RunSpec, test_support}; fn projection_with_goal(goal: Option<&str>) -> RunProjection { - let mut graph = Graph::new("test"); + let mut graph = RunGraph::new("test"); if let Some(goal) = goal { - graph - .attrs - .insert("goal".to_string(), AttrValue::String(goal.to_string())); + graph.goal = goal.to_string(); } let spec = RunSpec { diff --git a/lib/foundation/fabro-types/src/test_support.rs b/lib/foundation/fabro-types/src/test_support.rs index a73b5a579..258378102 100644 --- a/lib/foundation/fabro-types/src/test_support.rs +++ b/lib/foundation/fabro-types/src/test_support.rs @@ -4,8 +4,8 @@ use lithos_llm::catalog::{ModelId, builtin}; use lithos_llm::types::{Cost, CostSource, TokenCounts, Usage}; use crate::{ - AuthMethod, BlobHash, Graph, IdpIdentity, ModelRef, ModelUsage, PetriAdmission, PetriGraphRef, - Principal, RunProvenance, RunSpec, WorkflowSettings, WorkflowVersionId, fixtures, + AuthMethod, BlobHash, IdpIdentity, ModelRef, ModelUsage, PetriAdmission, PetriGraphRef, + Principal, RunGraph, RunProvenance, RunSpec, WorkflowSettings, WorkflowVersionId, fixtures, }; /// A fully populated `ModelUsage` for tests: `input_tokens` and @@ -65,7 +65,7 @@ pub fn test_run_spec() -> RunSpec { RunSpec { run_id: fixtures::RUN_1, settings: WorkflowSettings::default(), - graph: Graph::new("test"), + graph: RunGraph::new("test"), graph_source: None, workflow_slug: None, workflow_version_id: None, diff --git a/lib/foundation/fabro-types/src/usage_rollup.rs b/lib/foundation/fabro-types/src/usage_rollup.rs index 14773c941..46c0a6476 100644 --- a/lib/foundation/fabro-types/src/usage_rollup.rs +++ b/lib/foundation/fabro-types/src/usage_rollup.rs @@ -205,8 +205,8 @@ mod tests { use super::usage_rollup_from_projection; use crate::test_support::{self, test_usage}; use crate::{ - AttrValue, Graph, ModelRef, Node, RunProjection, RunSpec, StageCompletion, StageOutcome, - first_event_seq, + ModelRef, RunGraph, RunGraphNode, RunProjection, RunSpec, StageCompletion, StageHandler, + StageOutcome, first_event_seq, }; fn test_projection() -> RunProjection { @@ -438,22 +438,14 @@ mod tests { } fn run_spec_with_boundary_nodes() -> RunSpec { - let mut graph = Graph::new("test"); - graph.nodes.insert("start".to_string(), { - let mut node = Node::new("start"); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - node + let mut graph = RunGraph::new("test"); + graph.nodes.insert("start".to_string(), RunGraphNode { + label: "start".to_string(), + kind: StageHandler::Start, }); - graph.nodes.insert("exit".to_string(), { - let mut node = Node::new("exit"); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("Msquare".to_string()), - ); - node + graph.nodes.insert("exit".to_string(), RunGraphNode { + label: "exit".to_string(), + kind: StageHandler::Exit, }); RunSpec { diff --git a/lib/foundation/fabro-types/tests/run_spec_methods.rs b/lib/foundation/fabro-types/tests/run_spec_methods.rs index 6f6aefaaf..ee1e9b84b 100644 --- a/lib/foundation/fabro-types/tests/run_spec_methods.rs +++ b/lib/foundation/fabro-types/tests/run_spec_methods.rs @@ -1,10 +1,9 @@ use std::collections::HashMap; -use fabro_types::graph::Graph; use fabro_types::run::{DirtyStatus, GitContext, RunSpec}; use fabro_types::settings::{ProjectNamespace, WorkflowNamespace}; use fabro_types::test_support::test_run_spec; -use fabro_types::{WorkflowSettings, fixtures}; +use fabro_types::{RunGraph, WorkflowSettings, fixtures}; fn sample_run_spec() -> RunSpec { let settings = WorkflowSettings { @@ -21,7 +20,7 @@ fn sample_run_spec() -> RunSpec { RunSpec { settings, - graph: Graph::new("ship"), + graph: RunGraph::new("ship"), workflow_slug: Some("demo".to_string()), source_directory: Some("/Users/client/project".to_string()), labels: HashMap::from([("team".to_string(), "platform".to_string())]), @@ -67,7 +66,7 @@ fn run_spec_name_getters_do_not_synthesize_from_graph_or_slug() { 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"); + run_spec.graph = RunGraph::new("GraphName"); assert_eq!(run_spec.workflow_name(), None); assert_eq!(run_spec.project_name(), None); diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index 2d3d3bca5..739a1e312 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -1,13 +1,12 @@ use std::collections::HashMap; -use fabro_types::graph::Graph; use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, RunSpec}; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; use fabro_types::{ - AutomationRef, GitRunTarget, PetriAdmission, ResolvedAutomationGitWorkflowSource, RunTarget, - WorkflowSettings, fixtures, + AutomationRef, GitRunTarget, PetriAdmission, ResolvedAutomationGitWorkflowSource, RunGraph, + RunTarget, WorkflowSettings, fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -21,7 +20,7 @@ fn run_spec_round_trips_templated_settings() { let record = RunSpec { run_id: fixtures::RUN_1, settings: templated_settings(), - graph: Graph::new("ship"), + graph: RunGraph::new("ship"), graph_source: None, workflow_slug: Some("demo".to_string()), workflow_version_id: Some(test_workflow_version_id()), diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 3546fa7d0..ff556cb1a 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -397,6 +397,9 @@ models/run-git-settings.ts models/run-goal-file.ts models/run-goal-inline.ts models/run-goal.ts +models/run-graph-edge.ts +models/run-graph-node.ts +models/run-graph.ts models/run-integrations-github-settings.ts models/run-integrations-settings.ts models/run-intent-args-inputs-value.ts diff --git a/lib/packages/fabro-api-client/src/api/runs-api.ts b/lib/packages/fabro-api-client/src/api/runs-api.ts index 1ab4f4805..cdd8f5c80 100644 --- a/lib/packages/fabro-api-client/src/api/runs-api.ts +++ b/lib/packages/fabro-api-client/src/api/runs-api.ts @@ -955,7 +955,7 @@ export const RunsApiAxiosParamCreator = function (configuration?: Configuration) }; }, /** - * Validates and renders a workflow manifest as SVG without creating a run. + * Validates and renders a workflow manifest as SVG without creating a run. The manifest is checked as `POST /validate` checks it; a workflow Petri refuses is not rendered. * @summary Render Workflow Graph * @param {RenderWorkflowGraphRequest} renderWorkflowGraphRequest * @param {*} [options] Override http request option. @@ -1247,7 +1247,7 @@ export const RunsApiAxiosParamCreator = function (configuration?: Configuration) }; }, /** - * Validates runtime readiness for a workflow manifest without creating a run. + * Validates runtime readiness for a workflow manifest without creating a run. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. The checks then probe the sandbox, repository access and GitHub credentials. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -1536,7 +1536,7 @@ export const RunsApiAxiosParamCreator = function (configuration?: Configuration) }; }, /** - * Validates workflow structure and diagnostics without runtime readiness checks. + * Validates a workflow manifest without runtime readiness checks. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. `workflow` describes the admitted graph, or the DOT as written when Petri refused the workflow. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -1859,7 +1859,7 @@ export const RunsApiFp = function(configuration?: Configuration) { return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); }, /** - * Validates and renders a workflow manifest as SVG without creating a run. + * Validates and renders a workflow manifest as SVG without creating a run. The manifest is checked as `POST /validate` checks it; a workflow Petri refuses is not rendered. * @summary Render Workflow Graph * @param {RenderWorkflowGraphRequest} renderWorkflowGraphRequest * @param {*} [options] Override http request option. @@ -1952,7 +1952,7 @@ export const RunsApiFp = function(configuration?: Configuration) { return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); }, /** - * Validates runtime readiness for a workflow manifest without creating a run. + * Validates runtime readiness for a workflow manifest without creating a run. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. The checks then probe the sandbox, repository access and GitHub credentials. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -2045,7 +2045,7 @@ export const RunsApiFp = function(configuration?: Configuration) { return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); }, /** - * Validates workflow structure and diagnostics without runtime readiness checks. + * Validates a workflow manifest without runtime readiness checks. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. `workflow` describes the admitted graph, or the DOT as written when Petri refused the workflow. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -2280,7 +2280,7 @@ export const RunsApiFactory = function (configuration?: Configuration, basePath? return localVarFp.pauseRun(id, options).then((request) => request(axios, basePath)); }, /** - * Validates and renders a workflow manifest as SVG without creating a run. + * Validates and renders a workflow manifest as SVG without creating a run. The manifest is checked as `POST /validate` checks it; a workflow Petri refuses is not rendered. * @summary Render Workflow Graph * @param {RenderWorkflowGraphRequest} renderWorkflowGraphRequest * @param {*} [options] Override http request option. @@ -2352,7 +2352,7 @@ export const RunsApiFactory = function (configuration?: Configuration, basePath? return localVarFp.rewindRun(id, rewindRequest, options).then((request) => request(axios, basePath)); }, /** - * Validates runtime readiness for a workflow manifest without creating a run. + * Validates runtime readiness for a workflow manifest without creating a run. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. The checks then probe the sandbox, repository access and GitHub credentials. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -2424,7 +2424,7 @@ export const RunsApiFactory = function (configuration?: Configuration, basePath? return localVarFp.updateRun(id, updateRunRequest, options).then((request) => request(axios, basePath)); }, /** - * Validates workflow structure and diagnostics without runtime readiness checks. + * Validates a workflow manifest without runtime readiness checks. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. `workflow` describes the admitted graph, or the DOT as written when Petri refused the workflow. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -2674,7 +2674,7 @@ export class RunsApi extends BaseAPI { } /** - * Validates and renders a workflow manifest as SVG without creating a run. + * Validates and renders a workflow manifest as SVG without creating a run. The manifest is checked as `POST /validate` checks it; a workflow Petri refuses is not rendered. * @summary Render Workflow Graph * @param {RenderWorkflowGraphRequest} renderWorkflowGraphRequest * @param {*} [options] Override http request option. @@ -2753,7 +2753,7 @@ export class RunsApi extends BaseAPI { } /** - * Validates runtime readiness for a workflow manifest without creating a run. + * Validates runtime readiness for a workflow manifest without creating a run. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. The checks then probe the sandbox, repository access and GitHub credentials. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. @@ -2832,7 +2832,7 @@ export class RunsApi extends BaseAPI { } /** - * Validates workflow structure and diagnostics without runtime readiness checks. + * Validates a workflow manifest without runtime readiness checks. The workflow is checked as a run would be admitted: Petri compiles the bundle with the server\'s settings, run variables and model catalog, and every diagnostic carries Petri\'s code as its `rule` (`attractor.no_start`, `attractor.model.unknown`, `unsupported.template.unbound_input`), with Fabro\'s own `fabro.model.no_ready_provider` when a model node has no provider ready to run it. `workflow` describes the admitted graph, or the DOT as written when Petri refused the workflow. * @summary Validate Workflow Manifest * @param {RunManifest} runManifest * @param {*} [options] Override http request option. diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 53baa3c13..6d9ddbb77 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -368,6 +368,9 @@ export * from './run-git-settings'; export * from './run-goal'; export * from './run-goal-file'; export * from './run-goal-inline'; +export * from './run-graph'; +export * from './run-graph-edge'; +export * from './run-graph-node'; export * from './run-integrations-github-settings'; export * from './run-integrations-settings'; export * from './run-intent'; diff --git a/lib/packages/fabro-api-client/src/models/run-graph-edge.ts b/lib/packages/fabro-api-client/src/models/run-graph-edge.ts new file mode 100644 index 000000000..8f6fbb0f4 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/run-graph-edge.ts @@ -0,0 +1,23 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.2.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * One routing edge of a run\'s display graph. + */ +export interface RunGraphEdge { + 'from': string; + 'to': string; +} diff --git a/lib/packages/fabro-api-client/src/models/run-graph-node.ts b/lib/packages/fabro-api-client/src/models/run-graph-node.ts new file mode 100644 index 000000000..18cd51f86 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/run-graph-node.ts @@ -0,0 +1,29 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.2.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +// May contain unused imports in some cases +// @ts-ignore +import type { StageHandler } from './stage-handler'; + +/** + * One stage of a run\'s display graph. + */ +export interface RunGraphNode { + /** + * The node\'s display label, its `label` attribute or its id. + */ + 'label': string; + 'kind': StageHandler; +} diff --git a/lib/packages/fabro-api-client/src/models/run-graph.ts b/lib/packages/fabro-api-client/src/models/run-graph.ts new file mode 100644 index 000000000..c77b11457 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/run-graph.ts @@ -0,0 +1,43 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.2.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +// May contain unused imports in some cases +// @ts-ignore +import type { RunGraphEdge } from './run-graph-edge'; +// May contain unused imports in some cases +// @ts-ignore +import type { RunGraphNode } from './run-graph-node'; + +/** + * The display graph of a run: the admitted workflow\'s name, goal, stages and edges, read off the graph Petri admitted at create time. Lowering artifacts (the goal check, a synthetic fan-in) are left out; the stages are the nodes the workflow declares, imports and `[run.prepare]` steps included. + */ +export interface RunGraph { + /** + * The workflow\'s name, the DOT `digraph` name. + */ + 'name': string; + /** + * The run\'s goal as the run displays it; empty when the workflow has none. + */ + 'goal'?: string; + /** + * The stages by node id. + */ + 'nodes'?: { [key: string]: RunGraphNode; }; + /** + * The routing edges as written, one per arm. + */ + 'edges'?: Array; +} diff --git a/lib/packages/fabro-api-client/src/models/run-spec.ts b/lib/packages/fabro-api-client/src/models/run-spec.ts index 89b62d955..4c3714b2c 100644 --- a/lib/packages/fabro-api-client/src/models/run-spec.ts +++ b/lib/packages/fabro-api-client/src/models/run-spec.ts @@ -27,6 +27,9 @@ import type { GitContext } from './git-context'; import type { PetriAdmission } from './petri-admission'; // May contain unused imports in some cases // @ts-ignore +import type { RunGraph } from './run-graph'; +// May contain unused imports in some cases +// @ts-ignore import type { RunProvenance } from './run-provenance'; // May contain unused imports in some cases // @ts-ignore @@ -41,7 +44,13 @@ import type { WorkflowSettings } from './workflow-settings'; export interface RunSpec { 'run_id': string; 'settings': WorkflowSettings; - 'graph': { [key: string]: any; }; + /** + * The display graph: the workflow Petri admitted, reduced to what the read side names. The DOT it was written in is `graph_source`. + */ + 'graph': RunGraph; + /** + * The entrypoint workflow\'s DOT as written. + */ 'graph_source'?: string | null; 'workflow_slug'?: string | null; /** diff --git a/lib/packages/fabro-api-client/src/models/workflow-diagnostic.ts b/lib/packages/fabro-api-client/src/models/workflow-diagnostic.ts index e0a9686a1..deeff01b2 100644 --- a/lib/packages/fabro-api-client/src/models/workflow-diagnostic.ts +++ b/lib/packages/fabro-api-client/src/models/workflow-diagnostic.ts @@ -17,7 +17,13 @@ // @ts-ignore import type { RelatedWorkflowDiagnostic } from './related-workflow-diagnostic'; +/** + * One diagnostic about a workflow. `rule` is the stable code of the check that raised it: Petri\'s codes (`attractor.*`, `unsupported.*`, `deprecated.*`, `info.*`, `fabro.hooks.*`) for the compile, and `fabro.model.no_ready_provider` for Fabro\'s own provider check. + */ export interface WorkflowDiagnostic { + /** + * The stable code of the check that raised the diagnostic. + */ 'rule': string; 'severity': WorkflowDiagnosticSeverityEnum; 'message': string;