mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-02 02:13:49 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
fc6fb20bd0
commit
1fda9633e4
4 changed files with 221 additions and 12 deletions
|
|
@ -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<String> {
|
||||
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,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<AppState>, 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<String>,
|
||||
/// 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<String>,
|
||||
/// The local repository the root `start` stage checks out into the
|
||||
/// workspace; `None` starts the run from an empty workspace.
|
||||
pub repository: Option<PathBuf>,
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue