scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-12 23:02:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

feat(mcp): support goal files in run create

Browse source 
Allow fabro_run_create object specs to pass goal_file, reject goal and goal_file together, and preserve file-sourced goal semantics when building run manifests.
This commit is contained in:
Bryan Helmkamp 2026-05-22 14:27:42 -04:00
parent c2d950da38
commit 511371bd9a
No known key found for this signature in database
5 changed files with 138 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
docs/public/agents/mcp.mdx
View file

@ -49,6 +49,7 @@ Use the object form when you need create options:
"workflow": "sleeper",
"auto_approve": true,
"dry_run": true,
"goal_file": "plans/ship-it.md",
"labels": { "source": "mcp" },
"start": true
}
@ -56,6 +57,8 @@ Use the object form when you need create options:
}
```
Use `goal` for inline goal text or `goal_file` to read the run goal from a file. They are mutually exclusive. Relative `goal_file` paths resolve from the run's `cwd`, or from the MCP server working directory when `cwd` is omitted.
Run summaries returned by the MCP server include parent metadata. Use `parent_id` on `fabro_run_create` to create a child run, `parent_id` on `fabro_run_search` to list direct children, and the `link_parent` or `unlink_parent` actions on `fabro_run_interact` to change an existing run's parent.
## Fabro agents as MCP clients

20
lib/crates/fabro-cli/tests/it/cmd/mcp.rs
View file

@ -1489,10 +1489,26 @@ async fn mcp_create_validation_errors_happen_before_auth_or_network() {
}),
)
.await;
let conflicting_goal_sources = call_tool_error_text(
&client,
"fabro_run_create",
serde_json::json!({
"runs": [{
"workflow": "simple.fabro",
"goal": "inline goal",
"goal_file": "plans/goal.md"
}]
}),
)
.await;
assert!(empty.contains("runs"), "{empty}");
assert!(many.contains("runs"), "{many}");
assert!(null.contains("decision"), "{null}");
assert!(
conflicting_goal_sources.contains("goal and goal_file are mutually exclusive"),
"{conflicting_goal_sources}"
);
assert_eq!(client.list_tools().await.unwrap().len(), 6);
client
.shutdown()
@ -2267,6 +2283,10 @@ fn assert_create_schema_accepts_string_and_object_specs(schema: &serde_json::Val
object_variant.pointer("/properties/workflow").is_some(),
"object create spec should include workflow property: {schema}"
);
assert!(
object_variant.pointer("/properties/goal_file").is_some(),
"object create spec should include goal_file property: {schema}"
);
assert!(
object_variant
.get("required")

4
lib/crates/fabro-mcp-server/src/server.rs
View file

@ -293,6 +293,10 @@ mod tests {
object_variant.pointer("/properties/workflow").is_some(),
"object create spec should expose workflow property: {schema}"
);
assert!(
object_variant.pointer("/properties/goal_file").is_some(),
"object create spec should expose goal_file property: {schema}"
);
assert!(
object_variant
.get("required")

47
lib/crates/fabro-server/src/run_tool_manifest.rs
View file

@ -2,10 +2,11 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use fabro_api::types;
use fabro_config::{CliLayer, RunLayer};
use fabro_config::{CliLayer, RunGoalLayer, RunLayer};
use fabro_manifest::{ManifestBuildInput, RunOverrideInput};
use fabro_model::Catalog;
use fabro_tool::{ToolError, ToolResult, ValidatedCreateRunSpec};
use fabro_types::settings::interp::InterpString;
use crate::manifest_validation;
@ -68,7 +69,7 @@ pub fn run_tool_manifest_args(spec: &ValidatedCreateRunSpec) -> Option<types::Ma
}
pub fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec) -> Option<RunLayer> {
fabro_manifest::build_sparse_run_overrides(RunOverrideInput {
let mut run = fabro_manifest::build_run_overrides(RunOverrideInput {
goal: spec.goal.as_deref(),
model: spec.model.as_deref(),
provider: spec.provider.as_deref(),
@ -78,7 +79,18 @@ pub fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec) -> Option<RunLayer>
dry_run: spec.dry_run,
auto_approve: spec.auto_approve,
labels: spec.labels.clone(),
})
});
if let Some(goal_file) = spec.goal_file.as_ref() {
run.goal = Some(RunGoalLayer::File {
file: InterpString::parse(&goal_file.to_string_lossy()),
});
}
(run.goal.is_some()
|| !run.metadata.is_empty()
|| run.model.is_some()
|| run.sandbox.is_some()
|| run.execution.is_some())
.then_some(run)
}
#[cfg(test)]
@ -98,6 +110,7 @@ mod tests {
parent_id: None,
cwd: None,
goal: None,
goal_file: None,
inputs: HashMap::from([
("count".to_string(), json!(3).into()),
("decision".to_string(), json!("approve").into()),
@ -116,4 +129,32 @@ mod tests {
assert_eq!(args.input, vec![r"count=3", r#"decision="approve""#]);
}
#[test]
fn run_overrides_preserve_goal_file_as_file_goal() {
let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
workflow: "implement-plan".to_string(),
run_id: None,
parent_id: None,
cwd: None,
goal: None,
goal_file: Some(PathBuf::from("plans/ship-it.md")),
inputs: HashMap::new(),
labels: HashMap::new(),
model: None,
provider: None,
sandbox: None,
dry_run: None,
auto_approve: None,
preserve_sandbox: None,
start: None,
})
.expect("create spec with goal_file should validate");
let run = run_tool_run_overrides(&spec).expect("goal_file should produce run overrides");
let Some(fabro_config::RunGoalLayer::File { file }) = run.goal else {
panic!("goal_file should become a file goal override");
};
assert_eq!(file.as_source(), "plans/ship-it.md");
}
}

67
lib/crates/fabro-tool/src/create.rs
View file

@ -113,6 +113,13 @@ impl JsonSchema for CreateRunSpecInput {
],
"description": "Optional goal override for the run."
},
"goal_file": {
"anyOf": [
{ "type": "string" },
{ "type": "null" }
],
"description": "Read the run goal from a file. Mutually exclusive with goal. Relative paths are resolved from the run cwd."
},
"inputs": {
"type": "object",
"description": "Workflow input overrides keyed by input name.",
@ -194,6 +201,7 @@ pub struct CreateRunSpec {
pub run_id: Option<String>,
pub parent_id: Option<String>,
pub goal: Option<String>,
pub goal_file: Option<PathBuf>,
#[serde(default)]
pub inputs: HashMap<String, RunInputValue>,
#[serde(default)]
@ -257,6 +265,7 @@ pub struct ValidatedCreateRunSpec {
pub run_id: Option<RunId>,
pub parent_id: Option<String>,
pub goal: Option<String>,
pub goal_file: Option<PathBuf>,
pub inputs: HashMap<String, toml::Value>,
pub labels: HashMap<String, String>,
pub dry_run: Option<bool>,
@ -298,6 +307,7 @@ impl TryFrom<CreateRunSpecInput> for ValidatedCreateRunSpec {
run_id: None,
parent_id: None,
goal: None,
goal_file: None,
inputs: HashMap::new(),
labels: HashMap::new(),
dry_run: None,
@ -335,6 +345,18 @@ impl TryFrom<CreateRunSpec> for ValidatedCreateRunSpec {
if spec.parent_id.is_some() && parent_id.is_none() {
return Err(ToolError::message("parent_id must not be blank"));
}
if spec.goal.is_some() && spec.goal_file.is_some() {
return Err(ToolError::message(
"goal and goal_file are mutually exclusive; use exactly one",
));
}
if spec
.goal_file
.as_ref()
.is_some_and(|path| path.as_os_str().is_empty())
{
return Err(ToolError::message("goal_file must not be blank"));
}
let inputs = spec
.inputs
.into_iter()
@ -349,6 +371,7 @@ impl TryFrom<CreateRunSpec> for ValidatedCreateRunSpec {
run_id,
parent_id,
goal: spec.goal,
goal_file: spec.goal_file,
inputs,
labels: spec.labels,
dry_run: spec.dry_run,
@ -517,6 +540,7 @@ mod tests {
run_id: None,
parent_id: Some(" nightly-parent ".to_string()),
goal: None,
goal_file: None,
inputs: HashMap::new(),
labels: HashMap::new(),
dry_run: None,
@ -577,6 +601,46 @@ mod tests {
assert_eq!(spec.start, Some(false));
}
#[test]
fn create_params_preserve_goal_file_option() {
let params: FabroRunCreateParams = serde_json::from_value(json!({
"runs": [{
"workflow": "implement-plan",
"goal_file": "plans/ship-it.md",
"start": false
}]
}))
.expect("object form with goal_file should deserialize");
let params = ValidatedCreateRuns::try_from(params).expect("goal_file should validate");
let spec = &params.runs[0];
assert_eq!(spec.goal, None);
assert_eq!(
spec.goal_file.as_deref(),
Some(Path::new("plans/ship-it.md"))
);
}
#[test]
fn create_params_reject_goal_and_goal_file_together() {
let params: FabroRunCreateParams = serde_json::from_value(json!({
"runs": [{
"workflow": "implement-plan",
"goal": "inline goal",
"goal_file": "plans/ship-it.md"
}]
}))
.expect("object form with both goal forms should deserialize before validation");
let err = ValidatedCreateRuns::try_from(params)
.expect_err("goal and goal_file should be mutually exclusive");
assert!(
err.to_string()
.contains("goal and goal_file are mutually exclusive"),
"{err}"
);
}
#[test]
fn create_params_reject_blank_string_shorthand_workflow() {
let params: FabroRunCreateParams = serde_json::from_value(json!({
@ -621,6 +685,7 @@ mod tests {
run_id: None,
parent_id: Some("nightly-parent".to_string()),
goal: None,
goal_file: None,
inputs: HashMap::new(),
labels: HashMap::new(),
dry_run: Some(true),
@ -670,6 +735,7 @@ mod tests {
run_id: None,
parent_id: Some("nightly-parent".to_string()),
goal: None,
goal_file: None,
inputs: HashMap::new(),
labels: HashMap::new(),
dry_run: Some(true),
@ -718,6 +784,7 @@ mod tests {
run_id: None,
parent_id: Some(parent_id.to_string()),
goal: None,
goal_file: None,
inputs: HashMap::new(),
labels: HashMap::new(),
dry_run: Some(true),