From 1fda9633e45c6067a9137d61e158c9eedd55c864 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 21 Sep 2026 03:48:35 -0400 Subject: [PATCH] Bind the run's resolved goal into Petri's check An intent's goal override, and a `[run.goal]` layer, reached the run's display graph and its settings but not Petri's check, so the agent stages executed with the workflow's own goal while the run showed the override. The launch now carries the run's resolved goal (the settings' inline `run.goal`, layered as the create path layers it) as Petri's `petri.launch_goal` compile variable, which Petri binds over the bundle's `[run] goal` and the graph's own `goal`, so admission's frozen plan carries the goal the run shows. Co-Authored-By: Claude Fable 5.1 --- lib/apps/fabro-server/src/petri_check.rs | 33 ++++- .../fabro-server/tests/it/scenario/petri.rs | 118 ++++++++++++++++++ lib/components/fabro-petri/src/check.rs | 29 +++-- lib/components/fabro-petri/tests/check.rs | 53 ++++++++ 4 files changed, 221 insertions(+), 12 deletions(-) diff --git a/lib/apps/fabro-server/src/petri_check.rs b/lib/apps/fabro-server/src/petri_check.rs index fd88e1a70..a720f644e 100644 --- a/lib/apps/fabro-server/src/petri_check.rs +++ b/lib/apps/fabro-server/src/petri_check.rs @@ -17,6 +17,7 @@ use fabro_llm::selection; use fabro_petri::check::{self, Admitted, Bundle, CheckError, CheckRequest, Diagnostic, Launch}; use fabro_petri::runtime::RuntimeSpec; use fabro_types::diagnostic::{Diagnostic as FabroDiagnostic, Severity}; +use fabro_types::settings::run::RunGoal; use fabro_types::{ManifestPath, WorkflowSettings}; use fabro_workflow::Error as WorkflowError; use fabro_workflow::workflow_bundle::WorkflowBundle; @@ -26,11 +27,11 @@ use lithos_llm::catalog::ProviderId; pub(crate) const NO_READY_PROVIDER_RULE: &str = "fabro.model.no_ready_provider"; /// The launch Fabro binds around the settings: the run's model and provider -/// below them, and the environment the run selected above them. When the -/// settings name neither model nor provider, the default offering of the -/// eligible providers is bound as the launch model alone: a node that -/// names no model runs on it, and a node that names a model the catalog -/// lacks stays unqualified, so Petri's admission refuses it. +/// below them, and the environment the run selected and the goal the run +/// resolved above them. When the settings name neither model nor provider, the +/// default offering of the eligible providers is bound as the launch model +/// alone: a node that names no model runs on it, and a node that names a model +/// the catalog lacks stays unqualified, so Petri's admission refuses it. pub(crate) fn launch( catalog: &Catalog, settings: &WorkflowSettings, @@ -51,10 +52,31 @@ pub(crate) fn launch( model, provider: settings.run.model.provider.clone(), environment: environment.map(str::to_owned), + goal: launch_goal(settings), repository, } } +/// The goal the run resolved, for Petri to bind over the bundle's layers +/// and the graph's own `goal`: the settings' inline `run.goal`, which the +/// create path has layered (an intent's override over the workflow layer +/// over the server's defaults) and whose workflow-layer `file` form is +/// inlined before layering. A `file` form that survives layering (a server +/// default) is left to the bundle's own `[run.goal]`, which Petri reads +/// itself; the text is not read here, away from the run's working +/// directory. +#[expect( + clippy::disallowed_methods, + reason = "goal text passes through in source form, as `materialize_admitted_run` displays it; \ + Petri renders `{{ inputs.* }}` and `{{ vars.* }}` in it as it renders `[run] goal`" +)] +fn launch_goal(settings: &WorkflowSettings) -> Option { + match settings.run.goal.as_ref()? { + RunGoal::Inline(text) => Some(text.as_source()), + RunGoal::File(_) => None, + } +} + /// The launch with no catalog to pick a default from: what the settings /// name, for a check away from the server. pub(crate) fn launch_without_catalog(settings: &WorkflowSettings) -> Launch { @@ -62,6 +84,7 @@ pub(crate) fn launch_without_catalog(settings: &WorkflowSettings) -> Launch { model: settings.run.model.name.clone(), provider: settings.run.model.provider.clone(), environment: None, + goal: launch_goal(settings), repository: None, } } diff --git a/lib/apps/fabro-server/tests/it/scenario/petri.rs b/lib/apps/fabro-server/tests/it/scenario/petri.rs index 6deba54f1..86300d554 100644 --- a/lib/apps/fabro-server/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-server/tests/it/scenario/petri.rs @@ -1631,3 +1631,121 @@ async fn a_bundle_naming_a_catalog_mcp_server_lists_its_tools_to_the_model() { "the session lists the catalog server's tool under the reference's name: {tools:?}" ); } + +/// The hello bundle's agent stage, run with the given version files and an +/// optional intent goal override, to completion: the run's id, the twin's +/// request-log namespace and the server state. +async fn run_hello_agent( + files: &[(&str, &str)], + goal: Option<&str>, +) -> (Arc, axum::Router, String, String) { + let workspace = tempfile::tempdir().expect("workspace tempdir"); + let twin = twin_openai().await; + let namespace = format!( + "{}::{}::{}", + module_path!(), + line!(), + goal.map_or("file", |_| "override") + ); + TwinScenarios::new(&namespace) + .scenario(TwinScenario::responses(OPENAI_MODEL).text("A limerick, added.")) + .scenario(TwinScenario::responses(OPENAI_MODEL).text("A limerick, added.")) + .load(twin) + .await; + let settings = test_settings(); + let state = TestAppStateBuilder::new() + .runtime_settings(settings.server_settings, settings.manifest_run_defaults) + .max_concurrent_runs(5) + .in_process_execution() + .llm_overlay(llm_overlay_with_provider_base_url( + "openai", + twin.base_url.clone(), + )) + .vault_entries([(EnvVars::OPENAI_API_KEY, namespace.clone())]) + .build(); + let app = test_app_with_scheduler(Arc::clone(&state)); + let version_id = register_version(&app, files).await; + let mut intent = intent(&version_id, workspace.path()); + intent["args"]["model"] = serde_json::json!(OPENAI_MODEL); + if let Some(goal) = goal { + intent["goal"] = serde_json::json!(goal); + } + let run_id = create_and_start_run_from_intent(&app, intent).await; + let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; + let run = run_json(&app, &run_id).await; + assert_eq!(status, "succeeded", "run: {run}"); + // The workspace outlives the run: the run's record names it. + std::mem::forget(workspace); + (state, app, namespace, run_id) +} + +/// The goal the run shows is the goal its stages execute with: `GET +/// /runs/{id}` names it, Petri's admitted graph carries it, and the agent +/// stage's prompt to the model opens with it in place of the graph's own. +async fn assert_run_goal(app: &axum::Router, namespace: &str, run_id: &str, goal: &str) { + let run = run_json(app, run_id).await; + assert_eq!(run["goal"], goal, "the run shows the goal: {run}"); + let graph = admitted_root_graph(app, run_id).await; + assert_eq!( + graph["params"]["goal"], goal, + "Petri admitted the run's goal: {}", + graph["params"] + ); + let logs = twin_openai().await.request_logs(namespace).await; + let prompt = logs["requests"] + .as_array() + .expect("twin request logs are an array") + .iter() + .filter_map(|request| request["input_text"].as_str()) + .find(|input| input.contains("Add a haiku to the README")) + .unwrap_or_else(|| panic!("the agent stage's prompt reached the twin, got {logs}")); + assert!( + prompt.contains(goal), + "the agent's prompt carries the run's goal, got {prompt}" + ); + assert!( + !prompt.contains("Say hello and demonstrate a basic Fabro workflow"), + "the graph's own goal is replaced, got {prompt}" + ); +} + +/// An intent's goal override is bound into Petri's check, so the agent +/// stages execute with the goal the run shows, not the workflow's own. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_goal_override_is_the_goal_the_stages_execute_with() { + const GOAL: &str = "Add a limerick to the README instead of a haiku"; + if host_plugin().is_none() { + return; + } + let [(workflow_path, workflow), (settings_path, settings)] = hello_files(); + let (_state, app, namespace, run_id) = run_hello_agent( + &[(workflow_path, &workflow), (settings_path, &settings)], + Some(GOAL), + ) + .await; + assert_run_goal(&app, &namespace, &run_id, GOAL).await; +} + +/// A `[run.goal] file` layer in the bundle's `workflow.toml` is the run's +/// goal the same way: the file's text is what the run shows and what the +/// agent stage executes with. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_run_goal_file_layer_is_the_goal_the_stages_execute_with() { + const GOAL: &str = "Write a limerick about workflow engines into the README"; + if host_plugin().is_none() { + return; + } + let [(workflow_path, workflow), _] = hello_files(); + let settings = + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.goal]\nfile = \"goal.md\"\n"; + let (_state, app, namespace, run_id) = run_hello_agent( + &[ + (workflow_path, &workflow), + ("workflow.toml", settings), + ("goal.md", GOAL), + ], + None, + ) + .await; + assert_run_goal(&app, &namespace, &run_id, GOAL).await; +} diff --git a/lib/components/fabro-petri/src/check.rs b/lib/components/fabro-petri/src/check.rs index 6686c9aea..62c0523d4 100644 --- a/lib/components/fabro-petri/src/check.rs +++ b/lib/components/fabro-petri/src/check.rs @@ -13,11 +13,12 @@ //! The launch binds the compile variables the Fabro frontend reads: //! `petri.launch_model` and `petri.launch_provider` as the model default //! below every file layer, `petri.launch_environment` as the environment -//! the run selected over every file layer, and `petri.repository` as the -//! repository the root `start` stage checks out. A caller with no local -//! repository binds `null`, and the run starts from an empty workspace. The -//! server's run variables (`{{ vars.NAME }}`) are bound as compile -//! variables beside them. +//! the run selected over every file layer, `petri.launch_goal` as the goal +//! the run resolved over every file layer and the graph's own, and +//! `petri.repository` as the repository the root `start` stage checks out. A +//! caller with no local repository binds `null`, and the run starts from an +//! empty workspace. The server's run variables (`{{ vars.NAME }}`) are bound as +//! compile variables beside them. use std::collections::BTreeMap; use std::path::PathBuf; @@ -25,8 +26,8 @@ use std::path::PathBuf; use petri_frontend_attractor::kinds::{AGENT_KIND, PROMPT_KIND}; use petri_runtime::LoadError; use petri_runtime::frontend::{ - self, CompileInputs, LAUNCH_ENVIRONMENT_VAR, LAUNCH_MODEL_VAR, LAUNCH_PROVIDER_VAR, MapFiles, - REPOSITORY_VAR, Severity, + self, CompileInputs, LAUNCH_ENVIRONMENT_VAR, LAUNCH_GOAL_VAR, LAUNCH_MODEL_VAR, + LAUNCH_PROVIDER_VAR, MapFiles, REPOSITORY_VAR, Severity, }; use petri_runtime::ir::Graph; use serde::{Deserialize, Serialize}; @@ -75,6 +76,11 @@ pub struct Launch { /// selection overrides the bundle in Fabro's own resolution; `None` /// leaves the layers to select. pub environment: Option, + /// The goal the run resolved (the intent's override, else the settings' + /// `[run] goal` from any layer), over the bundle's `[run] goal` and the + /// graph's own `goal`, so the stages execute with the goal the run + /// shows; `None` leaves the bundle's layers and the graph to state it. + pub goal: Option, /// The local repository the root `start` stage checks out into the /// workspace; `None` starts the run from an empty workspace. pub repository: Option, @@ -219,6 +225,15 @@ fn compile_inputs( Value::String(environment.clone()), ); } + if let Some(goal) = launch + .goal + .as_deref() + .filter(|goal| !goal.trim().is_empty()) + { + compile + .vars + .insert(LAUNCH_GOAL_VAR.into(), Value::String(goal.to_owned())); + } // `Runtime::check_source` uses the inputs as given, so the repository // is the host's to bind: the launch's path, or `null` for a run that // starts from an empty workspace. diff --git a/lib/components/fabro-petri/tests/check.rs b/lib/components/fabro-petri/tests/check.rs index 9a78a5672..3ce8a428e 100644 --- a/lib/components/fabro-petri/tests/check.rs +++ b/lib/components/fabro-petri/tests/check.rs @@ -142,6 +142,7 @@ async fn a_launch_binds_the_repository_and_the_model_default() { model: Some("gpt-5.4".to_string()), provider: None, environment: None, + goal: None, repository: Some(repository.path().to_path_buf()), }, runtime: RuntimeSpec::default(), @@ -367,3 +368,55 @@ fn an_unknown_environment_is_refused_and_the_launch_selects_over_the_bundle() { let admitted = check::check(&selected).expect("the launch's selection admits"); assert_eq!(admitted.graph.params["fabro.environment"]["id"], "local"); } + +/// The goal the launch binds (the run's resolved goal: an intent's override, +/// else the settings' `[run] goal`) is the goal Petri admits over the +/// bundle's own `[run] goal` and the graph's `goal`: the admitted graph's +/// `goal` parameter carries it, and so does the agent stage's config, which +/// is the goal its prompt is assembled from. +#[tokio::test] +async fn a_launch_goal_replaces_the_graphs_goal_on_the_admitted_graph_and_its_stages() { + const AGENT_WORKFLOW: &str = r#"digraph Agent { + graph [goal="The graph's goal"] + start [shape=Mdiamond] + exit [shape=Msquare] + work [shape=box, prompt="Do the work"] + start -> work -> exit +}"#; + let settings = format!("{SETTINGS}[run]\ngoal = \"The bundle's goal\"\n"); + let goal_of = |launch: Launch| { + let request = CheckRequest { + launch, + ..request( + bundle(&[ + ("workflow.fabro", AGENT_WORKFLOW), + ("workflow.toml", &settings), + ]), + RuntimeSpec::default(), + ) + }; + let admitted = check::check(&request).expect("the agent bundle is admitted"); + let stage = admitted + .graph + .body + .nodes + .iter() + .find(|node| node.name == "work") + .expect("the agent stage is admitted"); + assert_eq!( + stage.step.config["goal"], admitted.graph.params["goal"], + "the stage executes with the run's goal" + ); + admitted.graph.params["goal"].clone() + }; + + assert_eq!( + goal_of(Launch { + goal: Some("The run's goal override".to_string()), + ..Launch::default() + }), + "The run's goal override" + ); + // Without a launch goal, the bundle's `[run] goal` stands over the graph's. + assert_eq!(goal_of(Launch::default()), "The bundle's goal"); +}