mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
Apply CLI model and provider overrides to agent execution
This commit is contained in:
parent
4c4bf3df1f
commit
4e71554773
9 changed files with 261 additions and 19 deletions
|
|
@ -44,6 +44,7 @@ mod repo_init;
|
|||
mod resume;
|
||||
mod rm;
|
||||
mod run;
|
||||
mod run_model;
|
||||
mod runner;
|
||||
mod sandbox_cp;
|
||||
mod sandbox_preview;
|
||||
|
|
|
|||
192
lib/apps/fabro-cli/tests/it/cmd/run_model.rs
Normal file
192
lib/apps/fabro-cli/tests/it/cmd/run_model.rs
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
use std::fmt::Write as _;
|
||||
|
||||
use fabro_test::{TestContext, test_context};
|
||||
use httpmock::MockServer;
|
||||
use serde_json::json;
|
||||
|
||||
/// Exercise admission and the real agent against two local providers. Checking
|
||||
/// the request path and wire model catches overrides that only reach run state.
|
||||
fn assert_agent_route(
|
||||
mut context: TestContext,
|
||||
flags: &[&str],
|
||||
graph_defaults: &str,
|
||||
node_settings: &str,
|
||||
expected_provider: &str,
|
||||
expected_model: &str,
|
||||
) {
|
||||
let endpoint = MockServer::start();
|
||||
let mut settings = "_version = 1\n[server.auth]\nmethods = [\"dev-token\"]\n".to_string();
|
||||
for provider in ["primary", "alternate"] {
|
||||
write!(
|
||||
settings,
|
||||
r#"
|
||||
[llm.providers.{provider}]
|
||||
display_name = "{provider}"
|
||||
base_url = "{base}/{provider}/v1"
|
||||
auth = {{ type = "none" }}
|
||||
default_model = "base"
|
||||
[llm.providers.{provider}.metadata.agent]
|
||||
profile = "openai"
|
||||
"#,
|
||||
base = endpoint.base_url(),
|
||||
)
|
||||
.expect("provider settings should format");
|
||||
for model in ["base", "override", "node"] {
|
||||
write!(
|
||||
settings,
|
||||
r#"
|
||||
[llm.providers.{provider}.models.{model}]
|
||||
display_name = "{model}"
|
||||
api_model = "{provider}-{model}-wire"
|
||||
limits = {{ context_tokens = 32000, max_output_tokens = 1000 }}
|
||||
capabilities = {{ text = true, tools = true }}
|
||||
"#,
|
||||
)
|
||||
.expect("model settings should format");
|
||||
}
|
||||
}
|
||||
context.write_home(".fabro/settings.toml", settings);
|
||||
context.isolated_server();
|
||||
context.write_temp(
|
||||
"workflow.toml",
|
||||
r#"_version = 1
|
||||
[workflow]
|
||||
graph = "workflow.fabro"
|
||||
[run.model]
|
||||
provider = "primary"
|
||||
name = "base"
|
||||
[run.pull_request]
|
||||
enabled = false
|
||||
"#,
|
||||
);
|
||||
context.write_temp(
|
||||
"workflow.fabro",
|
||||
format!(
|
||||
r#"digraph ModelSelection {{
|
||||
{graph_defaults}
|
||||
start [shape=Mdiamond];
|
||||
work [shape=box, prompt="Say hello."];
|
||||
{node_settings}
|
||||
exit [shape=Msquare];
|
||||
start -> work -> exit;
|
||||
}}"#,
|
||||
),
|
||||
);
|
||||
|
||||
let wire_model = format!("{expected_provider}-{expected_model}-wire");
|
||||
let chunk = |delta, finish_reason| {
|
||||
json!({
|
||||
"id": "scripted-response", "object": "chat.completion.chunk",
|
||||
"created": 1, "model": wire_model,
|
||||
"choices": [{"index": 0, "delta": delta, "finish_reason": finish_reason}]
|
||||
})
|
||||
};
|
||||
let response = format!(
|
||||
"data: {}\n\ndata: {}\n\ndata: [DONE]\n\n",
|
||||
chunk(
|
||||
json!({"role": "assistant", "content": "Hello."}),
|
||||
json!(null)
|
||||
),
|
||||
chunk(json!({}), json!("stop")),
|
||||
);
|
||||
let expected = endpoint.mock(|when, then| {
|
||||
when.method("POST")
|
||||
.path(format!("/{expected_provider}/v1/chat/completions"))
|
||||
.json_body_includes(json!({"model": wire_model, "stream": true}).to_string());
|
||||
then.status(200)
|
||||
.header("Content-Type", "text/event-stream")
|
||||
.body(&response);
|
||||
});
|
||||
// Respond immediately to a wrong route too, so the regression fails with
|
||||
// the mismatched request instead of waiting through model retry backoff.
|
||||
let unexpected = endpoint.mock(|when, then| {
|
||||
when.method("POST");
|
||||
then.status(200)
|
||||
.header("Content-Type", "text/event-stream")
|
||||
.body(&response);
|
||||
});
|
||||
|
||||
let output = context
|
||||
.run_cmd()
|
||||
.args(["--auto-approve", "--environment", "local"])
|
||||
.args(flags)
|
||||
.arg(context.temp_dir.join("workflow.toml"))
|
||||
.output()
|
||||
.expect("run should execute");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"run failed:\n{}\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
unexpected.assert_calls(0);
|
||||
assert!(
|
||||
expected.calls() > 0,
|
||||
"the agent must call the requested route"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_model_controls_agent_requests() {
|
||||
assert_agent_route(test_context!(), &[], "", "", "primary", "base");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn model_flag_overrides_workflow_model_at_execution() {
|
||||
assert_agent_route(
|
||||
test_context!(),
|
||||
&["--model", "override"],
|
||||
"",
|
||||
"",
|
||||
"primary",
|
||||
"override",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_flag_overrides_workflow_provider_at_execution() {
|
||||
assert_agent_route(
|
||||
test_context!(),
|
||||
&["--provider", "alternate"],
|
||||
"",
|
||||
"",
|
||||
"alternate",
|
||||
"base",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn model_flags_override_workflow_and_graph_defaults_at_execution() {
|
||||
assert_agent_route(
|
||||
test_context!(),
|
||||
&["--model", "override", "--provider", "alternate"],
|
||||
r#"graph [default_model="base", default_provider="primary"];"#,
|
||||
"",
|
||||
"alternate",
|
||||
"override",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_node_model_takes_precedence_over_flags() {
|
||||
assert_agent_route(
|
||||
test_context!(),
|
||||
&["--model", "override", "--provider", "alternate"],
|
||||
"",
|
||||
r#"work [model="node", provider="primary"];"#,
|
||||
"primary",
|
||||
"node",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_stylesheet_model_takes_precedence_over_flags() {
|
||||
assert_agent_route(
|
||||
test_context!(),
|
||||
&["--model", "override", "--provider", "alternate"],
|
||||
"",
|
||||
r##"graph [model_stylesheet="#work { model: node; provider: primary; }"];"##,
|
||||
"primary",
|
||||
"node",
|
||||
);
|
||||
}
|
||||
|
|
@ -103,7 +103,10 @@ pub fn validate_collected_workflow(
|
|||
&lowered.entrypoint,
|
||||
&settings,
|
||||
&HashMap::new(),
|
||||
petri_check::launch_without_catalog(&settings),
|
||||
petri_check::with_model_overrides(
|
||||
petri_check::launch_without_catalog(&settings),
|
||||
run_overrides.and_then(|run| run.model.as_ref()),
|
||||
),
|
||||
offline_runtime(run_overrides),
|
||||
false,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@
|
|||
use std::collections::{BTreeMap, HashMap, HashSet};
|
||||
use std::path::PathBuf;
|
||||
|
||||
use fabro_config::RunModelLayer;
|
||||
use fabro_llm::lithos_catalog::Catalog;
|
||||
use fabro_llm::selection;
|
||||
use fabro_petri::check::{self, Admitted, Bundle, CheckError, CheckRequest, Diagnostic, Launch};
|
||||
|
|
@ -54,6 +55,7 @@ pub(crate) fn launch(
|
|||
environment: environment.map(str::to_owned),
|
||||
goal: launch_goal(settings),
|
||||
repository,
|
||||
..Launch::default()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -81,14 +83,28 @@ fn launch_goal(settings: &WorkflowSettings) -> Option<String> {
|
|||
/// name, for a check away from the server.
|
||||
pub(crate) fn launch_without_catalog(settings: &WorkflowSettings) -> Launch {
|
||||
Launch {
|
||||
model: settings.run.model.name.clone(),
|
||||
provider: settings.run.model.provider.clone(),
|
||||
model: settings.run.model.name.clone(),
|
||||
provider: settings.run.model.provider.clone(),
|
||||
environment: None,
|
||||
goal: launch_goal(settings),
|
||||
repository: None,
|
||||
goal: launch_goal(settings),
|
||||
repository: None,
|
||||
..Launch::default()
|
||||
}
|
||||
}
|
||||
|
||||
/// Preserve explicit model flags separately from defaults resolved from the
|
||||
/// workflow and server, so they can outrank file layers during admission.
|
||||
pub(crate) fn with_model_overrides(
|
||||
mut launch: Launch,
|
||||
overrides: Option<&RunModelLayer>,
|
||||
) -> Launch {
|
||||
if let Some(overrides) = overrides {
|
||||
launch.model_override.clone_from(&overrides.name);
|
||||
launch.provider_override.clone_from(&overrides.provider);
|
||||
}
|
||||
launch
|
||||
}
|
||||
|
||||
/// The check request for `bundle`'s `entrypoint`: every file of every
|
||||
/// workflow in the bundle at its bundle-relative path, the run's inputs and
|
||||
/// variables, the launch and the runtime. `unbound_is_warning` makes a
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ use std::path::PathBuf;
|
|||
use fabro_config::parse::{self, ParseError, SettingsSource};
|
||||
use fabro_config::{
|
||||
EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, MergeMap, RunLayer,
|
||||
SettingsLayer, WorkflowSettingsBuilder,
|
||||
RunModelLayer, SettingsLayer, WorkflowSettingsBuilder,
|
||||
};
|
||||
use fabro_types::settings::interp::{InterpString, ResolveError};
|
||||
use fabro_types::settings::run::{McpServerSettings, RunGoal};
|
||||
|
|
@ -100,6 +100,7 @@ struct RunMetadata {
|
|||
/// The environment the run overrides selected, for Petri's settings
|
||||
/// layer.
|
||||
environment_id: Option<String>,
|
||||
model_overrides: Option<RunModelLayer>,
|
||||
storage_root: PathBuf,
|
||||
workflow_slug: Option<String>,
|
||||
workflow_version_id: Option<WorkflowVersionId>,
|
||||
|
|
@ -174,6 +175,10 @@ impl PreparedRun {
|
|||
self.layered.metadata.environment_id.as_deref()
|
||||
}
|
||||
|
||||
pub(crate) fn model_overrides(&self) -> Option<&RunModelLayer> {
|
||||
self.layered.metadata.model_overrides.as_ref()
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_run_id(mut self) -> (Self, RunId) {
|
||||
let run_id = self.layered.metadata.run_id.unwrap_or_default();
|
||||
self.layered.metadata.run_id = Some(run_id);
|
||||
|
|
@ -316,6 +321,7 @@ pub(crate) fn normalize_source(input: RawRunCompilerInput) -> Result<NormalizedR
|
|||
.as_ref()
|
||||
.and_then(|run| run.environment.as_ref())
|
||||
.and_then(|environment| environment.id.clone());
|
||||
let model_overrides = run_overrides.as_ref().and_then(|run| run.model.clone());
|
||||
|
||||
Ok(NormalizedRun {
|
||||
workflow_bundle,
|
||||
|
|
@ -332,6 +338,7 @@ pub(crate) fn normalize_source(input: RawRunCompilerInput) -> Result<NormalizedR
|
|||
metadata: RunMetadata {
|
||||
run_id,
|
||||
environment_id,
|
||||
model_overrides,
|
||||
storage_root,
|
||||
workflow_slug,
|
||||
workflow_version_id,
|
||||
|
|
@ -460,6 +467,7 @@ pub(crate) fn assemble_run(pinned: PinnedRun) -> CreateRunPersistenceInput {
|
|||
// Consumed at admission, as the launch's environment; the resolved
|
||||
// settings carry the environment the run persists.
|
||||
environment_id: _,
|
||||
model_overrides: _,
|
||||
storage_root,
|
||||
workflow_slug,
|
||||
workflow_version_id,
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ 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,
|
||||
CliLayer, CliOutputLayer, EnvironmentLayer, MergeMap, RunLayer, RunModelLayer, SettingsLayer,
|
||||
WorkflowSettingsBuilder, parse_input_overrides, parse_labels, project,
|
||||
};
|
||||
use fabro_dot::WorkflowGraph;
|
||||
|
|
@ -53,6 +53,7 @@ pub(crate) struct PreparedManifest {
|
|||
/// The entrypoint's DOT as written: what the render endpoint draws.
|
||||
pub root_source: String,
|
||||
pub settings: WorkflowSettings,
|
||||
pub model_overrides: Option<RunModelLayer>,
|
||||
pub target_path: ManifestPath,
|
||||
pub workflow_bundle: WorkflowBundle,
|
||||
pub source_directory: PathBuf,
|
||||
|
|
@ -92,6 +93,10 @@ pub(crate) fn prepare_manifest_with_environment_defaults(
|
|||
|
||||
let args_overrides =
|
||||
manifest_args_overrides(manifest.args.as_ref()).context("failed to parse manifest args")?;
|
||||
let model_overrides = args_overrides
|
||||
.run
|
||||
.as_ref()
|
||||
.and_then(|run| run.model.clone());
|
||||
let mut workflow_settings_builder = WorkflowSettingsBuilder::new()
|
||||
.server_manifest_defaults(
|
||||
manifest_run_defaults.clone(),
|
||||
|
|
@ -166,6 +171,7 @@ pub(crate) fn prepare_manifest_with_environment_defaults(
|
|||
git: manifest.git.clone(),
|
||||
root_source,
|
||||
settings,
|
||||
model_overrides,
|
||||
target_path,
|
||||
workflow_bundle,
|
||||
source_directory,
|
||||
|
|
@ -205,7 +211,7 @@ pub(crate) fn validate_prepared_manifest(
|
|||
&prepared.target_path,
|
||||
&prepared.settings,
|
||||
vars,
|
||||
launch,
|
||||
petri_check::with_model_overrides(launch, prepared.model_overrides.as_ref()),
|
||||
runtime,
|
||||
unbound_is_warning,
|
||||
)?;
|
||||
|
|
|
|||
|
|
@ -275,6 +275,7 @@ pub(crate) async fn admit(
|
|||
repository,
|
||||
);
|
||||
let dry_run = settings.run.execution.mode == RunMode::DryRun;
|
||||
let launch = petri_check::with_model_overrides(launch, prepared.model_overrides());
|
||||
let request = petri_check::check_request(
|
||||
prepared.workflow_bundle(),
|
||||
prepared.entrypoint(),
|
||||
|
|
|
|||
|
|
@ -12,7 +12,9 @@
|
|||
//!
|
||||
//! 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
|
||||
//! below every file layer, `petri.model_override` and `petri.provider_override`
|
||||
//! as explicit selections above file and graph defaults but below node and
|
||||
//! stylesheet choices, `petri.launch_environment` as the environment
|
||||
//! 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
|
||||
|
|
@ -27,7 +29,8 @@ use petri_frontend_attractor::kinds::{AGENT_KIND, PROMPT_KIND};
|
|||
use petri_runtime::LoadError;
|
||||
use petri_runtime::frontend::{
|
||||
self, CompileInputs, LAUNCH_ENVIRONMENT_VAR, LAUNCH_GOAL_VAR, LAUNCH_MODEL_VAR,
|
||||
LAUNCH_PROVIDER_VAR, MapFiles, REPOSITORY_VAR, Severity,
|
||||
LAUNCH_PROVIDER_VAR, MODEL_OVERRIDE_VAR, MapFiles, PROVIDER_OVERRIDE_VAR, REPOSITORY_VAR,
|
||||
Severity,
|
||||
};
|
||||
use petri_runtime::ir::Graph;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
|
@ -69,21 +72,25 @@ impl Bundle {
|
|||
/// them, the environment selection above them, and the repository.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct Launch {
|
||||
pub model: Option<String>,
|
||||
pub provider: Option<String>,
|
||||
pub model: Option<String>,
|
||||
pub provider: Option<String>,
|
||||
/// Explicit run overrides, above workflow and graph defaults but below
|
||||
/// node attributes and stylesheets. Kept separate from catalog defaults.
|
||||
pub model_override: Option<String>,
|
||||
pub provider_override: Option<String>,
|
||||
/// The environment the run selected, by its id in the server's
|
||||
/// catalog, over every layer's `[run.environment]`, as the intent's
|
||||
/// selection overrides the bundle in Fabro's own resolution; `None`
|
||||
/// leaves the layers to select.
|
||||
pub environment: Option<String>,
|
||||
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>,
|
||||
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>,
|
||||
pub repository: Option<PathBuf>,
|
||||
}
|
||||
|
||||
/// One check: the bundle, the run's inputs and variables, the launch and
|
||||
|
|
@ -219,6 +226,13 @@ fn compile_inputs(
|
|||
compile
|
||||
.vars
|
||||
.insert(LAUNCH_PROVIDER_VAR.into(), text(&launch.provider));
|
||||
compile
|
||||
.vars
|
||||
.insert(MODEL_OVERRIDE_VAR.into(), text(&launch.model_override));
|
||||
compile.vars.insert(
|
||||
PROVIDER_OVERRIDE_VAR.into(),
|
||||
text(&launch.provider_override),
|
||||
);
|
||||
if let Some(environment) = &launch.environment {
|
||||
compile.vars.insert(
|
||||
LAUNCH_ENVIRONMENT_VAR.into(),
|
||||
|
|
|
|||
|
|
@ -140,11 +140,12 @@ async fn a_launch_binds_the_repository_and_the_model_default() {
|
|||
inputs: BTreeMap::new(),
|
||||
vars: BTreeMap::new(),
|
||||
launch: Launch {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
provider: None,
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
provider: None,
|
||||
environment: None,
|
||||
goal: None,
|
||||
repository: Some(repository.path().to_path_buf()),
|
||||
goal: None,
|
||||
repository: Some(repository.path().to_path_buf()),
|
||||
..Launch::default()
|
||||
},
|
||||
runtime: RuntimeSpec::default(),
|
||||
unbound_is_warning: false,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue