diff --git a/Cargo.lock b/Cargo.lock
index a29e0d55f..ef6490fd1 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3176,6 +3176,7 @@ dependencies = [
"fabro-workflow-version",
"futures",
"httpmock",
+ "jsonschema",
"schemars 1.2.1",
"serde",
"serde_json",
diff --git a/docs/internal/mcp-server-qa-test-plan.md b/docs/internal/mcp-server-qa-test-plan.md
index c5d7b8e74..920ff0754 100644
--- a/docs/internal/mcp-server-qa-test-plan.md
+++ b/docs/internal/mcp-server-qa-test-plan.md
@@ -1,5 +1,11 @@
# Fabro MCP Server — QA Test Plan
+> Historical manual QA results. The run-create source selectors and flat settings
+> below predate the registered-version contract. Current calls register file contents
+> with `fabro_workflow_version_create`, then pass `workflow_version_id`, an explicit
+> standalone `target`, and nested `args` to `fabro_run_create`. See
+> [the current MCP guide](../public/agents/mcp.mdx) for the supported contract.
+
One-time manual QA pass for the 5 tools exposed by `fabro-mcp-server`. Source of truth: `lib/apps/fabro-mcp-server/src/run_tools/`.
This plan is **not** a template for adding automated test coverage — it exists to drive a single hands-on sweep against a real running server. Tick boxes as scenarios pass; add notes inline for failures or surprising behavior. Open bugs/PRs for issues found; do not port these scenarios into the Rust test suite.
diff --git a/docs/public/agents/mcp.mdx b/docs/public/agents/mcp.mdx
index e769a4bf3..98f1de65d 100644
--- a/docs/public/agents/mcp.mdx
+++ b/docs/public/agents/mcp.mdx
@@ -86,34 +86,58 @@ Registration resolves no runtime secrets, selects no environment, and starts no
execution. It requires a user credential or a worker token with `agent:run_tools`;
ordinary worker tokens and same-run Ask Fabro sessions cannot register versions.
If an upload fails, retry the same contents: previously registered immutable
-versions remain reusable. The existing `fabro_run_create` input is unchanged.
+versions remain reusable. Use the returned ID with `fabro_run_create`.
### Create runs
-For a simple create call, `fabro_run_create` accepts a workflow selector string:
-
-```json
-{ "runs": ["sleeper"] }
-```
-
-Use the object form when you need create options:
+Call `fabro_run_create` with a registered workflow version ID and an independent
+workspace target. You can reuse the same ID for multiple runs without uploading
+again:
```json
{
- "runs": [
- {
- "workflow": "sleeper",
- "auto_approve": true,
- "dry_run": true,
- "goal_file": "plans/ship-it.md",
+ "runs": [{
+ "workflow_version_id": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+ "target": { "kind": "git", "repo": "acme/widgets", "branch": "main" },
+ "environment_id": "production",
+ "goal": "Review the checkout implementation.",
+ "args": {
+ "inputs": { "component": "checkout" },
"labels": { "source": "mcp" },
- "start": true
- }
- ]
+ "auto_approve": false
+ },
+ "start": false
+ }]
}
```
-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.
+Standalone MCP calls require an explicit `target`: Git coordinates, `kind: none`
+for an empty workspace, or a server-local folder for a compatible Local environment.
+A folder path refers to the server's filesystem, not the MCP client's filesystem.
+Workflow content does not determine the target. Native workflow-agent calls may
+omit `target` to inherit the parent's execution target; see [Child Runs](/execution/child-runs).
+Supplying `parent_id` in standalone MCP does not enable that inheritance.
+
+`args` uses the same fields as RunIntent: `inputs`, `labels`, `model`, `provider`,
+`dry_run`, `auto_approve`, and `preserve_sandbox`. Omitted boolean overrides stay
+omitted; explicit `false` is preserved. `environment_id` selects a server environment;
+omission uses the server default. `title`, literal `goal`, and an exact `parent_id`
+are optional. Caller/project/machine run settings are not read by the tool.
+Workflow-owned settings remain in the registered `workflow.toml`.
+
+
+Run creation accepts registered IDs only. Replace workflow strings and inline
+sources with a preceding `fabro_workflow_version_create` call. Move flat run
+options into `args`, use `environment_id` instead of `environment`, and send goal
+text instead of `goal_file`. Obtain local or remote files using the caller's own
+shell/read tools. Neither Fabro tool fetches remote sources or reads a caller path.
+
+
+Creation persists a submitted run. A separate start request follows by default;
+set `start: false` to start later. Batches contain 1–50 items and stop at the first
+failure. If creation succeeded before a start, summary lookup, or later item
+failed, the error includes the already-created run IDs. Inspect those runs before
+retrying to avoid creating duplicates.
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. See [Child Runs](/execution/child-runs) for the orchestration model.
diff --git a/docs/public/execution/child-runs.mdx b/docs/public/execution/child-runs.mdx
index 1b85039c1..95881df18 100644
--- a/docs/public/execution/child-runs.mdx
+++ b/docs/public/execution/child-runs.mdx
@@ -32,6 +32,7 @@ This exposes the same Fabro run tools available through [MCP](/agents/mcp):
| Tool | Purpose |
|---|---|
+| `fabro_workflow_version_create` | Register supplied workflow files and return an immutable version ID |
| `fabro_run_create` | Create one or more child runs, starting them by default |
| `fabro_run_search` | Search runs, including direct children by `parent_id` |
| `fabro_run_get` | Inspect a run without mutating it |
@@ -46,57 +47,69 @@ When a workflow agent calls `fabro_run_create`, Fabro always parents the created
## Create child runs
-The simplest `fabro_run_create` call names a workflow:
+Acquire workflow files with the agent's shell/read tools, then call
+`fabro_workflow_version_create` with their contents:
```json
{
- "runs": ["implement-and-test"]
+ "entrypoint": "workflow.fabro",
+ "files": {
+ "workflow.fabro": "digraph Child { start [shape=Mdiamond] work [prompt=\"@prompt.md\"] exit [shape=Msquare] start -> work -> exit }",
+ "prompt.md": "Implement the requested change and run the relevant tests."
+ }
}
```
-Use the object form to pass run options:
+Use the returned `workflow_version_id` in `fabro_run_create`:
```json
{
- "runs": [
- {
- "workflow": "implement-and-test",
- "goal": "Implement the checkout page refactor and run the test suite.",
- "labels": {
- "lane": "checkout",
- "source": "parent-run"
- },
- "start": true
- }
- ]
+ "runs": [{
+ "workflow_version_id": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+ "goal": "Implement the checkout page refactor and run its tests.",
+ "args": { "labels": { "lane": "checkout", "source": "parent-run" } },
+ "start": true
+ }]
}
```
-The parent can create several children in one call:
+A version can be reused for several children, each with its own goal, target, and
+`args`. When the version already exists, skip registration. Read goal files with
+the agent's read tool and pass literal `goal` text. Workflow names, paths, inline
+source objects, and `goal_file` are no longer run-create inputs.
-```json
-{
- "runs": [
- {
- "workflow": "implementation",
- "goal_file": "plans/api.md",
- "labels": { "lane": "api" }
- },
- {
- "workflow": "implementation",
- "goal_file": "plans/web.md",
- "labels": { "lane": "web" }
- },
- {
- "workflow": "review",
- "goal": "Review the current branch for security and data-integrity risks.",
- "labels": { "lane": "review" }
- }
- ]
-}
-```
+This flow works in Local, Docker, and Daytona environments. The agent can clone a
+remote workflow in its sandbox and submit the resulting contents. Fabro's native
+tool handler executes outside the sandbox, so registration treats file-map keys
+as virtual relative paths and never reads sandbox paths from the worker host.
+Include the workflow's referenced configuration, prompts, and child workflows in
+the supplied tree. See [MCP](/agents/mcp) for package limits.
-By default, `fabro_run_create` requests start for each created run. Set `"start": false` when the parent should create the child now and start it later.
+Workflow content and workspace target are independent. If `target` is omitted,
+a native child inherits the parent's canonical target: `none` or folder as-is,
+and a Git target's repository and current execution branch (normally
+`fabro/run/`). Push parent changes before creating the child; a child
+clone sees that branch's remote HEAD. The parent's original pinned SHA/tag is
+not inherited. If run branches are disabled, the original input branch is used;
+if an enabled execution branch is unavailable, send an explicit target.
+
+An explicit Git, `none`, or folder target overrides inheritance while the current
+run remains the forced parent. Set `sha` on an explicit Git target to pin a child.
+Server admission enforces folder access: Docker/Daytona parents cannot select a
+server-host folder, including by requesting a Local child environment.
+Standalone MCP always requires an explicit target, even with `parent_id`.
+
+Use `environment_id` to choose a server environment; omission uses the server
+default, not the parent's environment. Run overrides belong in canonical `args`:
+`inputs`, `labels`, `model`, `provider`, `auto_approve`, `dry_run`, and
+`preserve_sandbox`. Omission preserves workflow/server defaults; explicit `false`
+is not omitted. The tool does not apply caller, project, or machine run settings.
+Keep workflow-owned configuration in the registered `workflow.toml`.
+
+Creation and start remain separate operations. Set `start: false` to leave a
+child submitted. A batch stops on its first failure; if any runs have already
+been created, their IDs appear in the error so the parent can inspect them
+instead of recreating them blindly.
## Start and approval
diff --git a/lib/apps/fabro-cli/src/commands/mcp/mod.rs b/lib/apps/fabro-cli/src/commands/mcp/mod.rs
index ef5725f72..c6a63a5e1 100644
--- a/lib/apps/fabro-cli/src/commands/mcp/mod.rs
+++ b/lib/apps/fabro-cli/src/commands/mcp/mod.rs
@@ -34,7 +34,6 @@ fn server_settings(
let user_settings = connection_ctx.user_settings().clone();
let storage_dir = connection_ctx.storage_dir().to_path_buf();
let base_config_path = connection_ctx.base_config_path().to_path_buf();
- let config_path = base_config_path.clone();
let client_factory: fabro_mcp_server::FabroClientFactory = std::sync::Arc::new(move || {
let target = target.clone();
let user_settings = user_settings.clone();
@@ -51,11 +50,7 @@ fn server_settings(
});
future
});
- Ok(fabro_mcp_server::FabroMcpServerSettings {
- client_factory,
- config_path,
- cwd: base_ctx.cwd().to_path_buf(),
- })
+ Ok(fabro_mcp_server::FabroMcpServerSettings { client_factory })
}
fn init_settings(args: &McpInitArgs) -> Result {
diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs
index 3b4db80a2..10365ca4a 100644
--- a/lib/apps/fabro-cli/src/commands/run/create.rs
+++ b/lib/apps/fabro-cli/src/commands/run/create.rs
@@ -1,9 +1,9 @@
use std::path::Path;
-use anyhow::{Context as _, anyhow, bail};
+use anyhow::{Context as _, anyhow};
use fabro_config::project;
use fabro_environment::{DEFAULT_ENVIRONMENT_ID, Environment};
-use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget, SandboxProviderKind};
+use fabro_types::{RunId, RunIntent};
use fabro_util::terminal::Styles;
use super::overrides::prepare_intent_overrides;
@@ -71,8 +71,16 @@ pub(crate) async fn create_run(
},
resolve_run_environment(client.as_ref(), args.environment.as_deref()),
)?;
- let (target, dirty_worktree) =
- run_target_for_environment(&environment.settings.provider, &canonical_cwd)?;
+ let configured_repo_origin_url =
+ fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())?;
+ let fabro_manifest::DerivedRunTarget {
+ target,
+ dirty_worktree,
+ } = fabro_manifest::derive_run_target_for_provider(
+ &environment.settings.provider,
+ &canonical_cwd,
+ configured_repo_origin_url.as_deref(),
+ )?;
if dirty_worktree {
fabro_util::printerr!(
ctx.printer(),
@@ -163,86 +171,3 @@ fn warn_untransmitted_settings(
keys.join(", "),
);
}
-
-/// Derives the run target from the caller directory for the environment's
-/// provider. Returns the target plus whether a clone-based observation found a
-/// dirty Git worktree, so the caller can warn about it.
-fn run_target_for_environment(
- provider: &SandboxProviderKind,
- canonical_cwd: &Path,
-) -> anyhow::Result<(RunTarget, bool)> {
- if !provider.clones_workspace() {
- let path = canonical_cwd.to_str().ok_or_else(|| {
- anyhow!(
- "caller working directory is not valid UTF-8: {}",
- canonical_cwd.display()
- )
- })?;
- return Ok((
- RunTarget::Folder {
- path: path.to_string(),
- },
- false,
- ));
- }
- let Some(observation) = fabro_manifest::observe_git_run_target(canonical_cwd, None) else {
- return Ok((none_target_for_unversioned_directory(canonical_cwd)?, false));
- };
- let dirty = observation.legacy_git_context.dirty == DirtyStatus::Dirty;
- let target = observation.run_target.ok_or_else(|| {
- anyhow!("the caller Git checkout cannot be represented as a canonical GitHub run target")
- })?;
- if target.sha.is_none() {
- bail!(
- "the exact local Git commit could not be made available from the canonical GitHub origin; push the commit and try again"
- );
- }
- Ok((RunTarget::Git(target), dirty))
-}
-
-fn none_target_for_unversioned_directory(canonical_cwd: &Path) -> anyhow::Result {
- let repository = match git2::Repository::discover(canonical_cwd) {
- Ok(repository) => repository,
- Err(source) if source.code() == git2::ErrorCode::NotFound => return Ok(RunTarget::None {}),
- Err(source) => {
- return Err(anyhow::Error::new(source)).with_context(|| {
- format!(
- "failed to inspect caller working directory {} for Git metadata",
- canonical_cwd.display()
- )
- });
- }
- };
-
- if repository.is_bare() {
- bail!(
- "the caller directory resolves to a bare Git repository; clone-based runs require a non-bare checkout with an attached branch"
- );
- }
- match repository.head() {
- Err(source)
- if matches!(
- source.code(),
- git2::ErrorCode::UnbornBranch | git2::ErrorCode::NotFound
- ) =>
- {
- bail!(
- "the caller Git checkout has no commits; create a commit before using a clone-based environment"
- );
- }
- Err(source) => {
- return Err(anyhow::Error::new(source))
- .context("failed to inspect the caller Git checkout HEAD");
- }
- Ok(head) if !head.is_branch() => {
- bail!(
- "the caller Git checkout has a detached HEAD; check out a branch before using a clone-based environment"
- );
- }
- Ok(_) => {}
- }
-
- bail!(
- "the caller Git checkout does not have a usable attached branch for a clone-based run target"
- )
-}
diff --git a/lib/apps/fabro-cli/src/commands/run/runner.rs b/lib/apps/fabro-cli/src/commands/run/runner.rs
index 506f2d83a..e96388091 100644
--- a/lib/apps/fabro-cli/src/commands/run/runner.rs
+++ b/lib/apps/fabro-cli/src/commands/run/runner.rs
@@ -5,9 +5,7 @@ use std::time::Duration;
use anyhow::{Context, Result, anyhow};
use async_trait::async_trait;
-use fabro_api::types::RunManifest;
use fabro_client::ServerTarget;
-use fabro_config::user::active_settings_path;
use fabro_config::{ServerSettingsBuilder, Storage};
use fabro_interview::{
AnswerSubmission, ControlInterviewer, WORKER_CONTROL_INVALID_CURSOR_REASON,
@@ -16,7 +14,6 @@ use fabro_interview::{
WorkerControlMessage,
};
use fabro_manifest::SuppliedWorkflowVersionPackager;
-use fabro_server::run_tool_manifest;
use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer};
use fabro_tool::fabro_client::ClientBackend;
use fabro_types::settings::run::{RunMode, RunNamespace};
@@ -98,13 +95,7 @@ pub(crate) async fn execute(
worker_token.to_owned(),
)));
let fabro_run_tools = if fabro_run_tools_enabled_from_worker_token(worker_token) {
- build_fabro_run_tool_services(
- worker_token,
- client.clone_for_reuse(),
- run_id,
- run_spec.source_directory.as_deref(),
- &run_dir,
- )
+ build_fabro_run_tool_services(worker_token, client.clone_for_reuse(), run_id)
} else {
None
};
@@ -230,36 +221,18 @@ fn build_fabro_run_tool_services(
worker_token: &str,
client: fabro_client::Client,
current_run_id: RunId,
- source_directory: Option<&str>,
- run_dir: &Path,
) -> Option {
if worker_token.trim().is_empty() {
return None;
}
let backend = ClientBackend::new(Arc::new(client))
- .with_manifest_builder(Arc::new(WorkerRunManifestBuilder))
.with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager));
Some(FabroRunToolServices {
backend: Arc::new(backend),
current_run_id,
- base_cwd: source_directory.map_or_else(|| run_dir.to_path_buf(), PathBuf::from),
- user_settings_path: active_settings_path(None),
})
}
-struct WorkerRunManifestBuilder;
-
-impl fabro_tool::RunManifestBuilder for WorkerRunManifestBuilder {
- fn build_run_manifest(
- &self,
- spec: &fabro_tool::ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
- ) -> fabro_tool::ToolResult {
- run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path)
- }
-}
-
/// Load the worker's secret vault from the run's storage root.
///
/// A worker always receives the server storage root so it can load the same
diff --git a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs
index c07daff5a..246e7b99b 100644
--- a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs
+++ b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs
@@ -645,7 +645,7 @@ async fn stdio_server_initializes_and_lists_run_tools() {
.find(|(name, _, _)| name == "fabro_run_create")
.map(|(_, _, schema)| schema)
.expect("fabro_run_create tool should be listed");
- assert_create_schema_accepts_string_and_object_specs(create_schema);
+ assert_create_schema_requires_version_ids(create_schema);
client
.shutdown()
.await
@@ -737,15 +737,15 @@ async fn mcp_create_and_search_manage_real_runs_with_cli_auth() {
let client = spawn_mcp_client(&context, &["--server", &target_url]).await;
+ let workflow_version_id = register_mcp_workflow(&client, &workflow).await;
let create = call_tool_json(
&client,
"fabro_run_create",
serde_json::json!({
"runs": [{
- "workflow": workflow,
- "dry_run": true,
- "auto_approve": true,
- "labels": { "source": "mcp-test" }
+ "workflow_version_id": workflow_version_id,
+ "target": { "kind": "none" },
+ "args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-test" }}
}]
}),
)
@@ -781,7 +781,7 @@ async fn mcp_create_and_search_manage_real_runs_with_cli_auth() {
"labels": {
"source": "mcp-test"
},
- "source_directory": "[SOURCE_DIRECTORY]",
+ "source_directory": null,
"repo_origin_url": null,
"goal_preview": "Run tests and report results",
"goal_truncated": false
@@ -804,15 +804,15 @@ async fn mcp_run_tools_use_default_local_server_without_server_flag() {
let workflow = context.install_fixture("simple.fabro");
let client = spawn_mcp_client(&context, &[]).await;
+ let workflow_version_id = register_mcp_workflow(&client, &workflow).await;
let create = call_tool_json(
&client,
"fabro_run_create",
serde_json::json!({
"runs": [{
- "workflow": workflow,
- "dry_run": true,
- "auto_approve": true,
- "labels": { "source": "mcp-default-server-test" },
+ "workflow_version_id": workflow_version_id,
+ "target": { "kind": "none" },
+ "args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-default-server-test" }},
"start": false
}]
}),
@@ -1388,7 +1388,7 @@ async fn mcp_lifecycle_tools_manage_real_run() {
"labels": {
"source": "mcp-test"
},
- "source_directory": "[SOURCE_DIRECTORY]",
+ "source_directory": null,
"repo_origin_url": null,
"goal": "Run tests and report results"
}
@@ -1874,7 +1874,9 @@ async fn mcp_create_validation_errors_happen_before_auth_or_network() {
let context = test_context!();
let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await;
let too_many = (0..51)
- .map(|index| serde_json::json!({ "workflow": format!("wf-{index}.fabro") }))
+ .map(
+ |_| serde_json::json!({"workflow_version_id":"a".repeat(64), "target":{"kind":"none"}}),
+ )
.collect::>();
let empty = call_tool_error_text(
@@ -1894,13 +1896,14 @@ async fn mcp_create_validation_errors_happen_before_auth_or_network() {
"fabro_run_create",
serde_json::json!({
"runs": [{
- "workflow": "simple.fabro",
- "inputs": { "decision": null }
+ "workflow_version_id": "a".repeat(64),
+ "target": {"kind":"none"},
+ "args": {"inputs": { "decision": null }}
}]
}),
)
.await;
- let conflicting_goal_sources = call_tool_error_text(
+ let conflicting_goal_sources = call_tool_parameter_error(
&client,
"fabro_run_create",
serde_json::json!({
@@ -1917,7 +1920,7 @@ async fn mcp_create_validation_errors_happen_before_auth_or_network() {
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.contains("fabro_workflow_version_create"),
"{conflicting_goal_sources}"
);
assert_mcp_run_tool_count(&client).await;
@@ -1928,41 +1931,24 @@ async fn mcp_create_validation_errors_happen_before_auth_or_network() {
}
#[tokio::test(flavor = "multi_thread")]
-async fn mcp_create_string_shorthand_deserializes_before_auth() {
+async fn mcp_create_requires_explicit_target_and_rejects_old_shorthand_before_auth() {
let context = test_context!();
- let harness =
- RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await;
- let target_url = harness.api_target();
- let workflow = context.install_fixture("simple.fabro");
- let client = spawn_mcp_client(&context, &["--server", &target_url]).await;
-
- let result = client
- .call_tool(
- "fabro_run_create",
- serde_json::json!({ "runs": [workflow] }),
- std::time::Duration::from_secs(30),
- )
- .await
- .expect("string shorthand should deserialize and return a tool-level auth error");
- assert_eq!(result.is_error, Some(true), "tool should return error");
- let error = result
- .content
- .first()
- .and_then(|content| serde_json::to_value(content).ok())
- .and_then(|content| content["text"].as_str().map(ToOwned::to_owned))
- .expect("tool error should include text");
- assert!(!error.contains("CreateRunSpec"), "{error}");
- assert!(
- error.contains("Run `fabro auth login` to authenticate."),
- "{error}"
- );
- assert_mcp_run_tool_count(&client).await;
-
- client
- .shutdown()
- .await
- .expect("MCP client should shut down");
- harness.shutdown().await;
+ let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await;
+ let error = call_tool_parameter_error(
+ &client,
+ "fabro_run_create",
+ serde_json::json!({"runs":["simple.fabro"]}),
+ )
+ .await;
+ assert!(error.contains("fabro_workflow_version_create"), "{error}");
+ let error = call_tool_error_text(
+ &client,
+ "fabro_run_create",
+ serde_json::json!({"runs":[{"workflow_version_id":"a".repeat(64)}]}),
+ )
+ .await;
+ assert!(error.contains("explicit target"), "{error}");
+ client.shutdown().await.unwrap();
}
#[tokio::test(flavor = "multi_thread")]
@@ -2770,6 +2756,18 @@ async fn call_tool_json(
.expect("tool result should include structured content")
}
+async fn call_tool_parameter_error(
+ client: &McpClient,
+ name: &str,
+ arguments: serde_json::Value,
+) -> String {
+ let error = client
+ .call_tool(name, arguments, std::time::Duration::from_secs(30))
+ .await
+ .expect_err("invalid parameters should produce an MCP protocol error");
+ error.to_string()
+}
+
async fn call_tool_error_text(
client: &McpClient,
name: &str,
@@ -2788,49 +2786,62 @@ async fn call_tool_error_text(
.expect("tool error should include text")
}
-fn assert_create_schema_accepts_string_and_object_specs(schema: &serde_json::Value) {
- let variants = schema
- .pointer("/properties/runs/items/anyOf")
- .and_then(serde_json::Value::as_array)
- .expect("fabro_run_create runs items should use anyOf");
-
- assert!(
- variants.iter().any(|variant| variant["type"] == "string"),
- "fabro_run_create should advertise workflow string shorthand: {schema}"
- );
- let object_variant = variants
- .iter()
- .find(|variant| variant["type"] == "object")
- .unwrap_or_else(|| {
- panic!("fabro_run_create should advertise object create specs: {schema}")
+fn assert_create_schema_requires_version_ids(schema: &serde_json::Value) {
+ let item = &schema["properties"]["runs"]["items"];
+ let item = item
+ .get("$ref")
+ .and_then(serde_json::Value::as_str)
+ .map_or(item, |reference| {
+ schema
+ .pointer(
+ reference
+ .strip_prefix('#')
+ .expect("schema reference should be local"),
+ )
+ .expect("schema reference should resolve")
});
+ assert_eq!(item["type"], "object");
+ assert_eq!(item["additionalProperties"], false);
assert!(
- 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")
- .and_then(serde_json::Value::as_array)
- .is_some_and(|required| required.iter().any(|field| field == "workflow")),
- "object create spec should require workflow: {schema}"
+ item["required"]
+ .as_array()
+ .expect("create schema should require fields")
+ .iter()
+ .any(|field| field == "workflow_version_id")
);
+ assert!(item["properties"].get("args").is_some());
+ assert!(item["properties"].get("workflow").is_none());
+ assert!(item["properties"].get("goal_file").is_none());
+}
+
+async fn register_mcp_workflow(client: &McpClient, workflow: &Path) -> serde_json::Value {
+ let entrypoint = workflow
+ .file_name()
+ .expect("fixture should have a filename")
+ .to_str()
+ .expect("fixture filename should be UTF-8");
+ let content = fs::read_to_string(workflow).expect("workflow fixture should be readable");
+ let result = call_tool_json(
+ client,
+ "fabro_workflow_version_create",
+ serde_json::json!({
+ "entrypoint": entrypoint, "files": {entrypoint: content}
+ }),
+ )
+ .await;
+ result["workflow_version_id"].clone()
}
async fn create_mcp_run(client: &McpClient, workflow: PathBuf, start: bool) -> String {
+ let workflow_version_id = register_mcp_workflow(client, &workflow).await;
let create = call_tool_json(
client,
"fabro_run_create",
serde_json::json!({
"runs": [{
- "workflow": workflow,
- "dry_run": true,
- "auto_approve": true,
- "labels": { "source": "mcp-test" },
+ "workflow_version_id": workflow_version_id,
+ "target": { "kind": "none" },
+ "args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-test" }},
"start": start
}]
}),
diff --git a/lib/apps/fabro-mcp-server/src/lib.rs b/lib/apps/fabro-mcp-server/src/lib.rs
index d345ec5c1..6a58b29b8 100644
--- a/lib/apps/fabro-mcp-server/src/lib.rs
+++ b/lib/apps/fabro-mcp-server/src/lib.rs
@@ -1,6 +1,5 @@
mod config;
mod executable_monitor;
-mod manifest_builder;
mod server;
use std::future::Future;
@@ -24,8 +23,6 @@ pub type FabroClientFactory = Arc FabroClientFuture + Send + Sync>;
#[derive(Clone)]
pub struct FabroMcpServerSettings {
pub client_factory: FabroClientFactory,
- pub config_path: PathBuf,
- pub cwd: PathBuf,
}
impl std::fmt::Debug for FabroMcpServerSettings {
@@ -33,8 +30,6 @@ impl std::fmt::Debug for FabroMcpServerSettings {
formatter
.debug_struct("FabroMcpServerSettings")
.field("client_factory", &"")
- .field("config_path", &self.config_path)
- .field("cwd", &self.cwd)
.finish()
}
}
diff --git a/lib/apps/fabro-mcp-server/src/manifest_builder.rs b/lib/apps/fabro-mcp-server/src/manifest_builder.rs
deleted file mode 100644
index b09be76de..000000000
--- a/lib/apps/fabro-mcp-server/src/manifest_builder.rs
+++ /dev/null
@@ -1,19 +0,0 @@
-use std::path::Path;
-
-use fabro_api::types;
-use fabro_server::run_tool_manifest;
-use fabro_tool::{RunManifestBuilder, ToolResult, ValidatedCreateRunSpec};
-
-#[derive(Default)]
-pub(crate) struct McpRunManifestBuilder;
-
-impl RunManifestBuilder for McpRunManifestBuilder {
- fn build_run_manifest(
- &self,
- spec: &ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
- ) -> ToolResult {
- run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path)
- }
-}
diff --git a/lib/apps/fabro-mcp-server/src/server.rs b/lib/apps/fabro-mcp-server/src/server.rs
index fba1fc5ac..ee98aa6b0 100644
--- a/lib/apps/fabro-mcp-server/src/server.rs
+++ b/lib/apps/fabro-mcp-server/src/server.rs
@@ -1,4 +1,3 @@
-use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
@@ -18,14 +17,12 @@ use tokio::time;
use tracing::warn;
use crate::executable_monitor::ExecutableMonitor;
-use crate::manifest_builder::McpRunManifestBuilder;
use crate::{FabroMcpServerSettings, SERVER_NAME};
#[derive(Clone)]
pub(crate) struct FabroMcpServer {
settings: Arc,
backend: Arc>>,
- cwd: PathBuf,
tool_router: ToolRouter,
}
@@ -82,11 +79,9 @@ impl ServerHandler for FabroMcpServer {
#[tool_router(router = tool_router)]
impl FabroMcpServer {
pub(crate) fn new(settings: Arc) -> Self {
- let cwd = settings.cwd.clone();
Self {
settings,
backend: Arc::new(OnceCell::new()),
- cwd,
tool_router: Self::tool_router(),
}
}
@@ -115,21 +110,21 @@ impl FabroMcpServer {
#[tool(
name = "fabro_run_create",
- description = "Create one or more Fabro workflow runs, optionally under a parent run, starting them by default."
+ description = "Create runs from registered workflow_version_id values and canonical RunIntent settings. Register contents with fabro_workflow_version_create first. Standalone calls require an explicit target; native workers may inherit the parent target. Starts runs by default."
)]
async fn fabro_run_create(
&self,
params: Parameters,
) -> Result {
- let params = match run_tools::ValidatedCreateRuns::try_from(params.0) {
- Ok(params) => params,
- Err(err) => return Ok(error_result(&err)),
- };
+ let params = params.0;
+ if let Err(err) = params.validate(run_tools::CreateRunOptions::default()) {
+ return Ok(error_result(&err));
+ }
let backend = match self.backend().await {
Ok(backend) => backend,
Err(err) => return Ok(error_result(&err)),
};
- match run_tools::create_runs(backend, &self.cwd, &self.settings.config_path, params).await {
+ match run_tools::create_runs(backend, params).await {
Ok(result) => success_result(&result, run_tools::create_runs_text(&result)),
Err(err) => Ok(error_result(&err)),
}
@@ -274,11 +269,9 @@ impl FabroMcpServer {
.await
.map(|client| {
Arc::new(
- ClientBackend::new(Arc::new(client))
- .with_manifest_builder(Arc::new(McpRunManifestBuilder))
- .with_workflow_version_packager(Arc::new(
- SuppliedWorkflowVersionPackager,
- )),
+ ClientBackend::new(Arc::new(client)).with_workflow_version_packager(
+ Arc::new(SuppliedWorkflowVersionPackager),
+ ),
) as Arc
})
.map_err(|err| run_tools::ToolError::from_anyhow(&err))
@@ -310,7 +303,6 @@ fn error_result(err: &run_tools::ToolError) -> CallToolResult {
#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
- use std::path::PathBuf;
use std::sync::Arc;
use serde_json::Value;
@@ -339,8 +331,6 @@ mod tests {
.await;
let url = mock.url("");
let server = FabroMcpServer::new(Arc::new(FabroMcpServerSettings {
- cwd: PathBuf::from("/does-not-exist"),
- config_path: PathBuf::from("/does-not-exist"),
client_factory: Arc::new(move || {
let url = url.clone();
Box::pin(async move { fabro_client::Client::new_no_proxy(&url) })
@@ -378,8 +368,6 @@ mod tests {
#[test]
fn server_info_reports_fabro_version() {
let settings = FabroMcpServerSettings {
- cwd: PathBuf::from("."),
- config_path: PathBuf::from("fabro.toml"),
client_factory: Arc::new(|| {
Box::pin(async { panic!("client should not be constructed while reading info") })
}),
@@ -394,8 +382,6 @@ mod tests {
#[test]
fn fabro_run_pair_tool_is_registered_with_stage_based_schema() {
let settings = FabroMcpServerSettings {
- cwd: PathBuf::from("."),
- config_path: PathBuf::from("fabro.toml"),
client_factory: Arc::new(|| {
Box::pin(async { panic!("client should not be constructed while listing tools") })
}),
@@ -421,10 +407,8 @@ mod tests {
}
#[test]
- fn fabro_run_create_tool_advertises_string_and_object_run_specs() {
+ fn fabro_run_create_tool_advertises_canonical_version_contract() {
let settings = FabroMcpServerSettings {
- cwd: PathBuf::from("."),
- config_path: PathBuf::from("fabro.toml"),
client_factory: Arc::new(|| {
Box::pin(async { panic!("client should not be constructed while listing tools") })
}),
@@ -436,35 +420,15 @@ mod tests {
.find(|tool| tool.name.as_ref() == "fabro_run_create")
.expect("fabro_run_create should be registered");
let schema = Value::Object(tool.input_schema.as_ref().clone());
- let variants = schema
- .pointer("/properties/runs/items/anyOf")
- .and_then(Value::as_array)
- .expect("runs items should advertise string and object variants");
-
- assert!(
- variants.iter().any(|variant| variant["type"] == "string"),
- "runs items should include workflow string shorthand: {schema}"
- );
- let object_variant = variants
+ let definition = run_tools::tool_definitions()
.iter()
- .find(|variant| variant["type"] == "object")
- .unwrap_or_else(|| {
- panic!("runs items should include object create spec variant: {schema}")
- });
- assert!(
- 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")
- .and_then(Value::as_array)
- .is_some_and(|required| required.iter().any(|name| name == "workflow")),
- "object create spec should require workflow: {schema}"
- );
+ .find(|definition| definition.name == "fabro_run_create")
+ .unwrap();
+ let mut expected = definition.parameters.clone();
+ expected.as_object_mut().unwrap().remove("$schema");
+ let mut actual = schema;
+ actual.as_object_mut().unwrap().remove("$schema");
+ assert_eq!(actual, expected);
+ assert_eq!(tool.description.as_deref(), Some(definition.description));
}
}
diff --git a/lib/apps/fabro-server/src/lib.rs b/lib/apps/fabro-server/src/lib.rs
index 98f7a2c77..5b9ee5447 100644
--- a/lib/apps/fabro-server/src/lib.rs
+++ b/lib/apps/fabro-server/src/lib.rs
@@ -41,7 +41,8 @@ mod run_intent;
mod run_manifest;
mod run_selector;
mod run_title_generation;
-pub mod run_tool_manifest;
+#[cfg(test)]
+mod run_tool_create;
pub mod security_headers;
pub mod serve;
pub mod server;
diff --git a/lib/apps/fabro-server/src/run_intent.rs b/lib/apps/fabro-server/src/run_intent.rs
index 68aa7c7a8..32c6f4eea 100644
--- a/lib/apps/fabro-server/src/run_intent.rs
+++ b/lib/apps/fabro-server/src/run_intent.rs
@@ -7,8 +7,8 @@ use fabro_environment::{EnvironmentId, EnvironmentValidationError};
use fabro_manifest::CollectedWorkflowClosure;
use fabro_types::settings::InterpString;
use fabro_types::{
- GitContext, ManifestPath, RunTarget, SandboxProviderKind, TargetValidationError, WorkflowPath,
- WorkflowVersion, WorkflowVersionId,
+ GitContext, ManifestPath, RunId, RunTarget, SandboxProviderKind, TargetValidationError,
+ WorkflowPath, WorkflowVersion, WorkflowVersionId,
};
use fabro_workflow::git;
use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle};
@@ -40,6 +40,14 @@ pub(crate) enum RunIntentAdmissionError {
#[source]
source: fabro_variable::Error,
},
+ #[error("originating worker run `{run_id}` could not be loaded")]
+ WorkerRun {
+ run_id: RunId,
+ #[source]
+ source: fabro_store::Error,
+ },
+ #[error("originating worker run `{run_id}` was not found")]
+ WorkerRunNotFound { run_id: RunId },
}
#[derive(Debug, Error)]
diff --git a/lib/apps/fabro-server/src/run_tool_create.rs b/lib/apps/fabro-server/src/run_tool_create.rs
new file mode 100644
index 000000000..503236f61
--- /dev/null
+++ b/lib/apps/fabro-server/src/run_tool_create.rs
@@ -0,0 +1,229 @@
+// Integration regression for the native run tool using production Git setup.
+use std::collections::HashMap;
+use std::path::Path;
+use std::process::Command;
+use std::sync::{Arc, Mutex};
+
+use chrono::{TimeZone, Utc};
+use fabro_tool::fabro_client::ClientBackend;
+use fabro_types::{
+ GitRunTarget, Run, RunId, RunLifecycle, RunLinks, RunOrigin, RunProjection, RunStatus,
+ RunTarget, RunTimestamps, WorkflowRef, WorkflowVersionId, test_support,
+};
+use httpmock::Method::{GET, POST};
+use httpmock::{HttpMockRequest, HttpMockResponse, MockServer};
+use serde_json::json;
+use tokio::fs;
+#[expect(
+ clippy::disallowed_methods,
+ reason = "test fixture setup uses the Git CLI against an isolated temporary repository"
+)]
+fn run_git(cwd: &Path, args: &[&str]) {
+ let output = Command::new("git")
+ .args(args)
+ .current_dir(cwd)
+ .output()
+ .expect("git command should run");
+ assert!(
+ output.status.success(),
+ "git {args:?} failed: {}",
+ String::from_utf8_lossy(&output.stderr)
+ );
+}
+
+fn test_parent(target: Option) -> RunProjection {
+ let mut spec = fabro_types::test_support::test_run_spec();
+ spec.target = target;
+ RunProjection::new(String::new(), spec, chrono::Utc::now())
+}
+
+async fn mock_parent<'a>(server: &'a MockServer, parent: &RunProjection) -> httpmock::Mock<'a> {
+ let path = format!("/api/v1/runs/{}/state", parent.spec.id());
+ let body = serde_json::to_value(parent).unwrap();
+ server
+ .mock_async(move |when, then| {
+ when.method(GET).path(path);
+ then.status(200).json_body(body);
+ })
+ .await
+}
+
+#[tokio::test]
+async fn run_create_child_checkout_contains_the_parents_pushed_work() {
+ let temp = tempfile::tempdir().unwrap();
+ let workspace = temp.path().join("parent");
+ let origin = temp.path().join("origin.git");
+ fs::create_dir(&workspace).await.unwrap();
+ run_git(temp.path(), &[
+ "init",
+ "--bare",
+ "--quiet",
+ origin.to_str().unwrap(),
+ ]);
+ run_git(&workspace, &["init", "--quiet", "--initial-branch", "main"]);
+ run_git(&workspace, &["config", "user.name", "Fabro Test"]);
+ run_git(&workspace, &["config", "user.email", "fabro@example.com"]);
+ fs::write(workspace.join("result.txt"), "original")
+ .await
+ .unwrap();
+ run_git(&workspace, &["add", "."]);
+ run_git(&workspace, &["commit", "--quiet", "-m", "initial"]);
+ run_git(&workspace, &[
+ "remote",
+ "add",
+ "origin",
+ origin.to_str().unwrap(),
+ ]);
+ run_git(&workspace, &["push", "--quiet", "origin", "main"]);
+ let base_sha = fabro_workflow::git::head_sha(&workspace).unwrap();
+ let mut parent = test_parent(Some(RunTarget::Git(GitRunTarget {
+ repo: "acme/widgets".to_owned(),
+ branch: "main".to_owned(),
+ tag: Some("v1.0.0".to_owned()),
+ sha: Some(base_sha),
+ })));
+ let sandbox = fabro_sandbox::local_sandbox(&workspace).await.unwrap();
+ // Docker and Daytona use this same setup operation to create the run branch.
+ let git = fabro_sandbox::setup_git(&sandbox, &fabro_sandbox::GitSetupIntent::NewRun {
+ run_id: parent.spec.id().to_string(),
+ })
+ .await
+ .unwrap();
+ parent.start = Some(fabro_types::StartRecord {
+ start_time: chrono::Utc::now(),
+ run_branch: Some(git.run_branch.clone()),
+ base_sha: Some(git.base_sha),
+ });
+ fs::write(workspace.join("result.txt"), "parent implementation")
+ .await
+ .unwrap();
+ run_git(&workspace, &["add", "."]);
+ run_git(&workspace, &["commit", "--quiet", "-m", "implement"]);
+ run_git(&workspace, &["push", "--quiet", "origin", &git.run_branch]);
+ let server = MockServer::start_async().await;
+ let state_request = mock_parent(&server, &parent).await;
+ let client = fabro_client::Client::new_no_proxy(&server.url("")).unwrap();
+ let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
+ let child_id = RunId::new();
+ let submitted = Arc::new(Mutex::new(Vec::::new()));
+ let captured = Arc::clone(&submitted);
+ server
+ .mock_async(move |when, then| {
+ when.method(POST).path("/api/v1/runs");
+ then.respond_with(move |request: &HttpMockRequest| {
+ captured
+ .lock()
+ .unwrap()
+ .push(serde_json::from_str(&request.body_string()).unwrap());
+ HttpMockResponse::builder()
+ .status(201)
+ .header("content-type", "application/json")
+ .body(serde_json::to_string(&run(child_id, None, 0)).unwrap())
+ .build()
+ });
+ })
+ .await;
+ server
+ .mock_async(|when, then| {
+ when.method(GET).path(format!("/api/v1/runs/{child_id}"));
+ then.status(200)
+ .json_body_obj(&run(child_id, Some(parent.spec.id()), 0));
+ })
+ .await;
+ let params = serde_json::from_value(
+ json!({"runs":[{"workflow_version_id":workflow_version_id,"start":false}]}),
+ )
+ .unwrap();
+ fabro_tool::create_runs_with_options(
+ Arc::new(ClientBackend::new(Arc::new(client))),
+ params,
+ fabro_tool::CreateRunOptions {
+ forced_parent_id: Some(parent.spec.id()),
+ },
+ )
+ .await
+ .unwrap();
+ state_request.assert_calls_async(1).await;
+ let intents = submitted.lock().unwrap().clone();
+ assert_eq!(intents.len(), 1);
+ assert_eq!(intents[0].workflow_version_id, workflow_version_id);
+ assert_eq!(intents[0].parent_id, Some(parent.spec.id()));
+ let RunTarget::Git(target) = &intents[0].target else {
+ panic!("child should have a Git target")
+ };
+ assert_eq!(target.repo, "acme/widgets");
+ assert_eq!(target.sha, None);
+ assert_eq!(target.tag, None);
+ let child = temp.path().join("child");
+ run_git(temp.path(), &[
+ "clone",
+ "--quiet",
+ "--branch",
+ &target.branch,
+ origin.to_str().unwrap(),
+ child.to_str().unwrap(),
+ ]);
+ assert_eq!(
+ fs::read_to_string(child.join("result.txt")).await.unwrap(),
+ "parent implementation"
+ );
+}
+
+fn run(run_id: RunId, parent_id: Option, children_count: u64) -> Run {
+ run_with_status(run_id, parent_id, children_count, RunStatus::Submitted)
+}
+
+fn run_with_status(
+ run_id: RunId,
+ parent_id: Option,
+ children_count: u64,
+ status: RunStatus,
+) -> Run {
+ Run {
+ id: run_id,
+ parent_id,
+ children_count,
+ title: "Test run".to_string(),
+ goal: "Test run".to_string(),
+ workflow: WorkflowRef {
+ slug: Some("simple".to_string()),
+ name: Some("Simple".to_string()),
+ graph_name: None,
+ node_count: 0,
+ edge_count: 0,
+ },
+ automation: None,
+ repository: None,
+ created_by: test_support::test_principal(),
+ origin: RunOrigin::default(),
+ labels: HashMap::new(),
+ lifecycle: RunLifecycle {
+ status,
+ approval: None,
+ pending_control: None,
+ queue_position: None,
+ error: None,
+ archived: false,
+ archived_at: None,
+ },
+ sandbox: None,
+ models: Vec::new(),
+ source_directory: Some("/srv/repo".to_string()),
+ timestamps: RunTimestamps {
+ created_at: Utc.with_ymd_and_hms(2026, 4, 5, 12, 0, 0).unwrap(),
+ started_at: None,
+ last_event_at: None,
+ completed_at: None,
+ },
+ timing: None,
+ billing: None,
+ size: fabro_types::RunSize::default(),
+ ask_fabro: fabro_types::AskFabro::default(),
+ diff: None,
+ pull_request: None,
+ current_question: None,
+ superseded_by: None,
+ retried_from: None,
+ links: RunLinks { web: None },
+ }
+}
diff --git a/lib/apps/fabro-server/src/run_tool_manifest.rs b/lib/apps/fabro-server/src/run_tool_manifest.rs
deleted file mode 100644
index a27f92e05..000000000
--- a/lib/apps/fabro-server/src/run_tool_manifest.rs
+++ /dev/null
@@ -1,218 +0,0 @@
-use std::path::{Path, PathBuf};
-
-use fabro_api::types;
-use fabro_config::{CliLayer, RunGoalLayer, RunLayer};
-use fabro_manifest::{ManifestBuildInput, RunOverrideInput};
-use fabro_tool::{ToolError, ToolResult, ValidatedCreateRunSpec};
-use fabro_types::settings::interp::InterpString;
-
-use crate::manifest_validation;
-
-/// Build and validate a run manifest for the `fabro_run_create` tool.
-///
-/// Validation is structural. The caller is a client — an MCP server or a run
-/// worker — whose catalog is its own, not the server's, so judging model and
-/// provider availability here would reject workflows the server can run.
-pub fn build_run_tool_manifest(
- spec: &ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
-) -> ToolResult {
- let built = fabro_manifest::build_run_manifest(ManifestBuildInput {
- workflow: PathBuf::from(&spec.workflow),
- cwd: cwd.to_path_buf(),
- run_overrides: run_tool_run_overrides(spec),
- cli_overrides: Some(CliLayer::default()),
- input_overrides: spec.inputs.clone(),
- args: run_tool_manifest_args(spec),
- environment_defaults: fabro_environment::seeded_catalog_layer(),
- user_settings_path: Some(user_settings_path.to_path_buf()),
- })
- .map_err(|err| ToolError::from_anyhow(&err))?;
-
- let mut validation =
- manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)
- .map_err(|err| ToolError::from_anyhow(&err))?;
- manifest_validation::promote_template_undefined_variables_to_errors(&mut validation);
- if !validation.ok {
- return Err(ToolError::message("workflow manifest validation failed"));
- }
-
- Ok(built.manifest)
-}
-
-pub fn run_tool_manifest_args(spec: &ValidatedCreateRunSpec) -> Option {
- let mut input = spec
- .inputs
- .iter()
- .map(|(key, value)| format!("{key}={value}"))
- .collect::>();
- input.sort();
- let mut label = spec
- .labels
- .iter()
- .map(|(key, value)| format!("{key}={value}"))
- .collect::>();
- label.sort();
-
- let payload = types::ManifestArgs {
- auto_approve: spec.auto_approve.filter(|value| *value),
- dry_run: spec.dry_run.filter(|value| *value),
- input,
- label,
- model: spec.model.clone(),
- preserve_sandbox: spec.preserve_sandbox.filter(|value| *value),
- provider: spec.provider.clone(),
- environment: spec.environment.clone(),
- verbose: None,
- };
- (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload)
-}
-
-pub fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec) -> Option {
- let mut run = fabro_manifest::build_run_overrides(RunOverrideInput {
- goal: spec.goal.as_deref(),
- model: spec.model.as_deref(),
- provider: spec.provider.as_deref(),
- environment: spec.environment.as_deref(),
- preserve_sandbox: spec.preserve_sandbox,
- 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.environment.is_some()
- || run.execution.is_some())
- .then_some(run)
-}
-
-#[cfg(test)]
-mod tests {
- use std::collections::HashMap;
-
- use fabro_tool::CreateRunSpec;
- use serde_json::json;
-
- use super::*;
-
- fn create_run_spec(workflow: &str) -> ValidatedCreateRunSpec {
- ValidatedCreateRunSpec::try_from(CreateRunSpec {
- workflow: workflow.to_string(),
- parent_id: None,
- cwd: None,
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- model: None,
- provider: None,
- environment: None,
- dry_run: None,
- auto_approve: None,
- preserve_sandbox: None,
- start: None,
- })
- .expect("create spec should validate")
- }
-
- /// The tool runs on a client, whose catalog is not the server's, so a
- /// server-owned model must reach the server rather than fail here.
- #[expect(
- clippy::disallowed_methods,
- reason = "sync test writes one workflow fixture before building the manifest"
- )]
- #[test]
- fn server_owned_provider_is_not_rejected_by_tool_manifest_validation() {
- let dir = tempfile::tempdir().expect("temp dir should be created");
- let workflow = dir.path().join("server-model.fabro");
- std::fs::write(
- &workflow,
- r#"digraph ServerModel {
- graph [goal="Use a server-owned model"]
- start [shape=Mdiamond]
- work [prompt="Do work", model="private-model", provider="server-only"]
- exit [shape=Msquare]
- start -> work -> exit
- }"#,
- )
- .expect("workflow fixture should be written");
-
- let manifest = build_run_tool_manifest(
- &create_run_spec(&workflow.to_string_lossy()),
- dir.path(),
- &dir.path().join("settings.toml"),
- )
- .expect("tool validation should leave provider availability to the server");
-
- let encoded = serde_json::to_string(&manifest).expect("manifest should serialize");
- assert!(
- encoded.contains("server-only"),
- "the authored provider should survive into the manifest: {encoded}"
- );
- }
-
- #[test]
- fn manifest_args_preserve_input_provenance() {
- let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
- workflow: "simple".to_string(),
- 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()),
- ]),
- labels: HashMap::new(),
- model: None,
- provider: None,
- environment: None,
- dry_run: None,
- auto_approve: None,
- preserve_sandbox: None,
- start: None,
- })
- .expect("create spec should validate");
- let args = run_tool_manifest_args(&spec).expect("input args should be present");
-
- assert_eq!(args.input, vec![r"count=3", r#"decision="approve""#]);
- }
-
- #[expect(
- clippy::disallowed_methods,
- reason = "test asserts the raw template source"
- )]
- #[test]
- fn run_overrides_preserve_goal_file_as_file_goal() {
- let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
- workflow: "implement-plan".to_string(),
- 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,
- environment: 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");
- }
-}
diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs
index 1066b0cce..b2de8d3c6 100644
--- a/lib/apps/fabro-server/src/server/handler/runs.rs
+++ b/lib/apps/fabro-server/src/server/handler/runs.rs
@@ -641,6 +641,9 @@ pub(crate) async fn create_run_from_intent(
Ok(id) => id,
Err(error) => return run_intent_admission_error(error.into()),
};
+ if let Err(error) = validate_intent_actor_target(&state, &actor, &target).await {
+ return run_intent_admission_error(error);
+ }
let blobs = state.store_ref().blobs();
let version_store = fabro_workflow_version::WorkflowVersionStore::new(blobs);
let closure = match version_store.get_closure(&intent.workflow_version_id).await {
@@ -985,6 +988,7 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response {
match &error {
RunIntentAdmissionError::VersionStore { .. }
| RunIntentAdmissionError::VariableSnapshot { .. }
+ | RunIntentAdmissionError::WorkerRun { .. }
| RunIntentAdmissionError::Environment(EnvironmentSelectionError::CredentialStore {
..
}) => {
@@ -1003,6 +1007,7 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response {
}
RunIntentAdmissionError::Target(_)
| RunIntentAdmissionError::FolderTarget(_)
+ | RunIntentAdmissionError::WorkerRunNotFound { .. }
| RunIntentAdmissionError::Environment(_) => {}
}
@@ -1076,9 +1081,46 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response {
"failed to load run variables",
"variable_store_error",
),
+ RunIntentAdmissionError::WorkerRun { .. } => intent_error(
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "failed to inspect originating worker run",
+ "worker_run_store_error",
+ ),
+ RunIntentAdmissionError::WorkerRunNotFound { .. } => intent_error(
+ StatusCode::NOT_FOUND,
+ "originating worker run not found",
+ "worker_run_not_found",
+ ),
}
}
+async fn validate_intent_actor_target(
+ state: &AppState,
+ actor: &Principal,
+ target: &RunTarget,
+) -> Result<(), RunIntentAdmissionError> {
+ let (Principal::Worker { run_id }, RunTarget::Folder { .. }) = (actor, target) else {
+ return Ok(());
+ };
+ let projection = state
+ .stores
+ .runs
+ .load_run_projection(run_id)
+ .await
+ .map_err(|source| RunIntentAdmissionError::WorkerRun {
+ run_id: *run_id,
+ source,
+ })?
+ .ok_or(RunIntentAdmissionError::WorkerRunNotFound { run_id: *run_id })?;
+ if !projection.spec.settings.run.environment.provider.is_local() {
+ return Err(EnvironmentSelectionError::TargetUnsupported {
+ detail: "folder targets created by a worker require a Local parent environment",
+ }
+ .into());
+ }
+ Ok(())
+}
+
fn select_intent_environment_id(
state: &AppState,
value: &str,
diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs
index 3f63337b5..9b3a01f58 100644
--- a/lib/apps/fabro-server/src/server/handler/sessions.rs
+++ b/lib/apps/fabro-server/src/server/handler/sessions.rs
@@ -1,7 +1,6 @@
use std::collections::HashMap;
use std::convert::Infallible;
use std::fmt::Write as _;
-use std::path::PathBuf;
use std::sync::Arc;
use axum::extract::{Path, Query, State};
@@ -749,10 +748,8 @@ async fn build_agent(
.map_err(AskFabroBuildError::Agent)?;
let backend = ClientBackend::new(Arc::new(api_client)).with_run_scope(run_id);
let services = FabroRunToolServices {
- backend: Arc::new(backend),
- current_run_id: run_id,
- base_cwd: PathBuf::new(),
- user_settings_path: PathBuf::new(),
+ backend: Arc::new(backend),
+ current_run_id: run_id,
};
let run_tools = register_named_fabro_run_tools(&services, ASK_FABRO_RUN_TOOL_NAMES);
let selector = format!("{provider_id}/{model}");
diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs
index a0a4533f6..1b86f0c90 100644
--- a/lib/apps/fabro-server/src/server/tests.rs
+++ b/lib/apps/fabro-server/src/server/tests.rs
@@ -4494,6 +4494,121 @@ enabled = false
);
}
+#[tokio::test]
+async fn run_tools_worker_cannot_select_server_folder_from_clone_based_parent() {
+ let dir = tempfile::tempdir().unwrap();
+ let missing_target = dir.path().join("missing");
+ let (state, app) = jwt_auth_app();
+ let user_token = issue_test_user_jwt();
+ let parent_run_id = create_run_with_bearer(&app, &user_token).await;
+ let worker_token = issue_test_run_tools_worker_token(&parent_run_id);
+ let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await;
+ let mut intent = folder_intent(workflow_version_id, missing_target.to_string_lossy());
+ intent["environment_id"] = json!("local");
+ intent["parent_id"] = json!(parent_run_id);
+
+ let response = app
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/runs",
+ &worker_token,
+ &intent,
+ ))
+ .await
+ .unwrap();
+ let body = response_json!(response, StatusCode::UNPROCESSABLE_ENTITY).await;
+
+ assert_eq!(body["errors"][0]["code"], "target_environment_unsupported");
+ assert_eq!(
+ body["errors"][0]["detail"],
+ "folder targets created by a worker require a Local parent environment"
+ );
+ assert_eq!(
+ state
+ .stores
+ .run_summaries
+ .list_identities()
+ .await
+ .unwrap()
+ .len(),
+ 1,
+ "the rejected child must not be persisted"
+ );
+}
+
+#[tokio::test]
+async fn run_tools_worker_folder_target_from_missing_parent_run_is_not_found() {
+ let dir = tempfile::tempdir().unwrap();
+ let (state, app) = jwt_auth_app();
+ let worker_token = issue_test_run_tools_worker_token(&RunId::new());
+ let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await;
+ let mut intent = folder_intent(workflow_version_id, dir.path().to_string_lossy());
+ intent["environment_id"] = json!("local");
+
+ let response = app
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/runs",
+ &worker_token,
+ &intent,
+ ))
+ .await
+ .unwrap();
+ let body = response_json!(response, StatusCode::NOT_FOUND).await;
+
+ assert_eq!(body["errors"][0]["code"], "worker_run_not_found");
+ assert!(
+ state
+ .stores
+ .run_summaries
+ .list_identities()
+ .await
+ .unwrap()
+ .is_empty()
+ );
+}
+
+#[tokio::test]
+async fn run_tools_worker_can_select_server_folder_from_local_parent() {
+ let dir = tempfile::tempdir().unwrap();
+ let (state, app) = jwt_auth_app();
+ let user_token = issue_test_user_jwt();
+ let workflow_version_id = store_workflow_version(&state, MINIMAL_DOT, None).await;
+ let mut parent_intent = folder_intent(workflow_version_id, dir.path().to_string_lossy());
+ parent_intent["environment_id"] = json!("local");
+
+ let response = app
+ .clone()
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/runs",
+ &user_token,
+ &parent_intent,
+ ))
+ .await
+ .unwrap();
+ let parent = response_json!(response, StatusCode::CREATED).await;
+ let parent_run_id = parent["id"].as_str().unwrap().parse::().unwrap();
+ let worker_token = issue_test_run_tools_worker_token(&parent_run_id);
+ let mut child_intent = folder_intent(workflow_version_id, dir.path().to_string_lossy());
+ child_intent["environment_id"] = json!("local");
+ child_intent["parent_id"] = json!(parent_run_id);
+
+ let response = app
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/runs",
+ &worker_token,
+ &child_intent,
+ ))
+ .await
+ .unwrap();
+ let child = response_json!(response, StatusCode::CREATED).await;
+
+ assert_eq!(child["parent_id"], parent_run_id.to_string());
+ assert_eq!(child["lifecycle"]["status"]["kind"], "submitted");
+}
+
#[tokio::test]
async fn post_runs_run_intent_accepts_none_target_with_ready_daytona_environment() {
let state = TestAppStateBuilder::new()
@@ -19812,3 +19927,57 @@ async fn workflow_version_registration_requires_user_or_run_tools_capability() {
.is_none()
);
}
+
+#[tokio::test]
+async fn run_tools_worker_registers_contents_then_creates_by_version_id() {
+ let (state, app) = jwt_auth_app();
+ let parent_id = create_run_with_bearer(&app, &issue_test_user_jwt()).await;
+ let token = issue_test_run_tools_worker_token(&parent_id);
+ let response = app
+ .clone()
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/workflow-versions",
+ &token,
+ &json!({
+ "entrypoint": "child.fabro",
+ "files": {"child.fabro": MINIMAL_DOT},
+ "workflow_dependencies": {}
+ }),
+ ))
+ .await
+ .unwrap();
+ let registered = response_json!(response, StatusCode::CREATED).await;
+ let response = app
+ .oneshot(json_bearer_request(
+ Method::POST,
+ "/runs",
+ &token,
+ &json!({
+ "workflow_version_id": registered["workflow_version_id"],
+ "target": {"kind": "none"},
+ "args": {"dry_run": true},
+ "parent_id": parent_id,
+ "goal": "A child created from sandbox-supplied contents"
+ }),
+ ))
+ .await
+ .unwrap();
+ let child = response_json!(response, StatusCode::CREATED).await;
+ assert_eq!(child["parent_id"], parent_id.to_string());
+ assert_eq!(child["lifecycle"]["status"]["kind"], "submitted");
+ let child_id = child["id"].as_str().unwrap().parse::().unwrap();
+ let projection = state
+ .stores
+ .runs
+ .load_run_projection(&child_id)
+ .await
+ .unwrap()
+ .unwrap();
+ assert_eq!(
+ serde_json::to_value(projection.spec.workflow_version_id).unwrap(),
+ registered["workflow_version_id"]
+ );
+ assert_eq!(projection.spec.target, Some(RunTarget::None {}));
+ assert!(projection.start.is_none());
+}
diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs
index f020b5828..43a028dd1 100644
--- a/lib/components/fabro-manifest/src/lib.rs
+++ b/lib/components/fabro-manifest/src/lib.rs
@@ -17,10 +17,13 @@ use std::path::{Component, Path, PathBuf};
use anyhow::{Context, Result, anyhow};
use fabro_api::types;
use fabro_config::project::{self, WorkflowLocation, discover_project_config};
-use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace};
+use fabro_config::run::{
+ parse_run_layer_from_settings_toml, resolve_run_goal_from_layer,
+ resolve_run_goal_from_namespace,
+};
use fabro_config::{
CliLayer, EnvironmentLayer, EnvironmentLifecycleLayer, MergeMap, ReplaceMap,
- RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer,
+ RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunScmLayer,
WorkflowSettingsBuilder,
};
use fabro_graphviz::graph::AttrValue;
@@ -31,7 +34,7 @@ use fabro_types::settings::interp::InterpString;
use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode};
use fabro_types::{
DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, RunTarget,
- WorkflowSettings,
+ SandboxProviderKind, WorkflowSettings,
};
use fabro_workflow::git::{self, GitSyncStatus};
pub use fabro_workflow_version::CollectedWorkflowClosure;
@@ -323,9 +326,29 @@ fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal {
#[derive(Clone, Debug)]
pub struct GitRunTargetObservation {
pub run_target: Option,
+ /// Whether the target's exact commit was proven available, and if not, why.
+ pub exact_commit: ExactCommitStatus,
pub legacy_git_context: GitContext,
}
+/// Outcome of proving that the local HEAD commit is available from the
+/// canonical origin.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum ExactCommitStatus {
+ /// A successful push or a direct remote query proved the commit is
+ /// available; the target carries its SHA.
+ Available,
+ /// The local branch has commits the origin does not, and the best-effort
+ /// push did not publish them.
+ Unpublished,
+ /// The local tracking ref matches HEAD, but the origin could not be
+ /// queried to confirm the commit is there.
+ Unverified,
+ /// The workflow configures a `run.scm` repository that is not the
+ /// checkout's origin, so nothing can be proven about that repository.
+ ConfiguredOriginMismatch,
+}
+
/// Observe Git facts without choosing an environment or a non-Git target.
///
/// Outer `None` means `repo_path` is not a usable attached checkout. A
@@ -341,11 +364,9 @@ pub fn observe_git_run_target(
) -> Option {
let local = inspect_local_git(repo_path, configured_repo_origin_url)?;
let legacy_git_context = local.legacy_git_context;
- let mut run_target = github_run_target(
- &legacy_git_context.origin_url,
- &legacy_git_context.branch,
- None,
- );
+ let mut run_target =
+ github_run_target(&legacy_git_context.origin_url, &legacy_git_context.branch);
+ let mut exact_commit = ExactCommitStatus::Unpublished;
if let Some(target) = run_target.as_mut() {
let publish_status = publish_manifest_branch_best_effort(
repo_path,
@@ -353,20 +374,202 @@ pub fn observe_git_run_target(
local.push_origin_url.as_deref(),
configured_repo_origin_url,
);
- target.sha = remotely_available_sha(
+ let (sha, status) = remotely_available_sha(
repo_path,
&legacy_git_context.branch,
legacy_git_context.sha.as_deref(),
publish_status,
);
+ target.sha = sha;
+ exact_commit = status;
}
Some(GitRunTargetObservation {
run_target,
+ exact_commit,
legacy_git_context,
})
}
+/// A canonical run target derived from a caller directory, plus whether a
+/// clone-based observation found uncommitted changes the target excludes.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct DerivedRunTarget {
+ pub target: RunTarget,
+ pub dirty_worktree: bool,
+}
+
+/// Why a caller directory could not be turned into a canonical run target.
+#[derive(Debug, thiserror::Error)]
+pub enum RunTargetDerivationError {
+ #[error("caller working directory is not valid UTF-8: {}", path.display())]
+ NonUtf8Path { path: PathBuf },
+ #[error("the caller Git checkout cannot be represented as a canonical GitHub run target")]
+ Unrepresentable,
+ #[error(
+ "the exact local Git commit could not be made available from the canonical GitHub origin; push the commit and try again"
+ )]
+ Unpublished,
+ #[error(
+ "the canonical GitHub origin could not be queried to confirm the exact local Git commit is available; check network access and credentials for the origin and try again"
+ )]
+ Unverified,
+ #[error(
+ "the workflow configures a run.scm repository that is not the local checkout's origin; run from a checkout of the configured repository"
+ )]
+ ConfiguredOriginMismatch,
+ #[error("failed to inspect caller working directory {} for Git metadata", path.display())]
+ Inspect {
+ path: PathBuf,
+ #[source]
+ source: git2::Error,
+ },
+ #[error(
+ "the caller directory resolves to a bare Git repository; clone-based runs require a non-bare checkout with an attached branch"
+ )]
+ BareRepository,
+ #[error(
+ "the caller Git checkout has no commits; create a commit before using a clone-based environment"
+ )]
+ NoCommits,
+ #[error("failed to inspect the caller Git checkout HEAD")]
+ Head {
+ #[source]
+ source: git2::Error,
+ },
+ #[error(
+ "the caller Git checkout has a detached HEAD; check out a branch before using a clone-based environment"
+ )]
+ DetachedHead,
+ #[error(
+ "the caller Git checkout does not have a usable attached branch for a clone-based run target"
+ )]
+ NoAttachedBranch,
+}
+
+/// Derive the canonical run target for `canonical_cwd` under `provider`, the
+/// way `fabro run` and `fabro create` do.
+///
+/// Non-clone providers run against the caller folder. Clone-based providers
+/// need an attached GitHub checkout whose exact HEAD is available from the
+/// origin, or a directory with no Git metadata at all, which becomes a `none`
+/// target. `configured_repo_origin_url` is the workflow's configured `run.scm`
+/// repository, when any, and takes precedence over the checkout's own origin.
+///
+/// # Errors
+///
+/// Returns the reason a clone-based target could not be derived; every
+/// variant's message is written for the CLI caller.
+pub fn derive_run_target_for_provider(
+ provider: &SandboxProviderKind,
+ canonical_cwd: &Path,
+ configured_repo_origin_url: Option<&str>,
+) -> std::result::Result {
+ if !provider.clones_workspace() {
+ let path = canonical_cwd
+ .to_str()
+ .ok_or_else(|| RunTargetDerivationError::NonUtf8Path {
+ path: canonical_cwd.to_path_buf(),
+ })?;
+ return Ok(DerivedRunTarget {
+ target: RunTarget::Folder {
+ path: path.to_string(),
+ },
+ dirty_worktree: false,
+ });
+ }
+ let Some(observation) = observe_git_run_target(canonical_cwd, configured_repo_origin_url)
+ else {
+ return Ok(DerivedRunTarget {
+ target: none_target_for_unversioned_directory(canonical_cwd)?,
+ dirty_worktree: false,
+ });
+ };
+ let dirty_worktree = observation.legacy_git_context.dirty == DirtyStatus::Dirty;
+ let target = observation
+ .run_target
+ .ok_or(RunTargetDerivationError::Unrepresentable)?;
+ if target.sha.is_none() {
+ return Err(match observation.exact_commit {
+ ExactCommitStatus::Unverified => RunTargetDerivationError::Unverified,
+ ExactCommitStatus::ConfiguredOriginMismatch => {
+ RunTargetDerivationError::ConfiguredOriginMismatch
+ }
+ ExactCommitStatus::Available | ExactCommitStatus::Unpublished => {
+ RunTargetDerivationError::Unpublished
+ }
+ });
+ }
+ Ok(DerivedRunTarget {
+ target: RunTarget::Git(target),
+ dirty_worktree,
+ })
+}
+
+fn none_target_for_unversioned_directory(
+ canonical_cwd: &Path,
+) -> std::result::Result {
+ let repository = match git2::Repository::discover(canonical_cwd) {
+ Ok(repository) => repository,
+ Err(source) if source.code() == git2::ErrorCode::NotFound => return Ok(RunTarget::None {}),
+ Err(source) => {
+ return Err(RunTargetDerivationError::Inspect {
+ path: canonical_cwd.to_path_buf(),
+ source,
+ });
+ }
+ };
+
+ if repository.is_bare() {
+ return Err(RunTargetDerivationError::BareRepository);
+ }
+ let outcome = match repository.head() {
+ Err(source)
+ if matches!(
+ source.code(),
+ git2::ErrorCode::UnbornBranch | git2::ErrorCode::NotFound
+ ) =>
+ {
+ Err(RunTargetDerivationError::NoCommits)
+ }
+ Err(source) => Err(RunTargetDerivationError::Head { source }),
+ Ok(head) if !head.is_branch() => Err(RunTargetDerivationError::DetachedHead),
+ Ok(_) => Err(RunTargetDerivationError::NoAttachedBranch),
+ };
+ outcome
+}
+
+/// The configured `run.scm` GitHub repository for a resolved local workflow.
+/// `workflow.toml` values take precedence over the discovered
+/// `.fabro/project.toml`, field by field, matching run settings layering.
+/// `None` when neither names a repository.
+///
+/// # Errors
+///
+/// Returns an error when either config exists but cannot be read or parsed.
+pub fn configured_repo_origin_url_for_location(
+ location: &WorkflowLocation,
+) -> Result> {
+ let workflow = location
+ .toml
+ .as_deref()
+ .map(read_scm_layer)
+ .transpose()?
+ .unwrap_or_default();
+ let project = discover_project_config(&location.dir)?
+ .as_deref()
+ .map(read_scm_layer)
+ .transpose()?
+ .unwrap_or_default();
+ let scm = RunScmLayer {
+ provider: workflow.provider.or(project.provider),
+ owner: workflow.owner.or(project.owner),
+ repository: workflow.repository.or(project.repository),
+ github: workflow.github.or(project.github),
+ };
+ Ok(configured_repo_origin_url_from_scm_layer(&scm))
+}
+
struct LocalGitObservation {
push_origin_url: Option,
legacy_git_context: GitContext,
@@ -422,14 +625,14 @@ fn build_legacy_git_context(
Some(local.legacy_git_context)
}
-fn github_run_target(origin_url: &str, branch: &str, sha: Option) -> Option {
+fn github_run_target(origin_url: &str, branch: &str) -> Option {
let (owner, repository) = fabro_github::parse_github_owner_repo(origin_url).ok()?;
let slug = GitHubRepositorySlug::try_new(&format!("{owner}/{repository}"))?;
let validated = RunTarget::Git(GitRunTarget {
- repo: slug.to_string(),
+ repo: slug.to_string(),
branch: branch.to_owned(),
- tag: None,
- sha,
+ tag: None,
+ sha: None,
})
.validate()
.ok()?;
@@ -441,15 +644,23 @@ fn github_run_target(origin_url: &str, branch: &str, sha: Option) -> Opt
fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option {
let scm = &settings.run.scm;
- if !scm
- .provider
- .as_deref()
- .is_none_or(|provider| provider.eq_ignore_ascii_case("github"))
- {
+ configured_repo_origin_url_from_scm(
+ scm.provider.as_deref(),
+ scm.owner.as_deref(),
+ scm.repository.as_deref(),
+ )
+}
+
+fn configured_repo_origin_url_from_scm(
+ provider: Option<&str>,
+ owner: Option<&str>,
+ repository: Option<&str>,
+) -> Option {
+ if !provider.is_none_or(|provider| provider.eq_ignore_ascii_case("github")) {
return None;
}
- let owner = scm.owner.as_deref()?;
- let repository = scm.repository.as_deref()?;
+ let owner = owner?;
+ let repository = repository?;
if owner.trim().is_empty() || repository.trim().is_empty() {
return None;
}
@@ -458,6 +669,22 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option {
(!normalized.is_empty()).then_some(normalized)
}
+fn configured_repo_origin_url_from_scm_layer(scm: &RunScmLayer) -> Option {
+ configured_repo_origin_url_from_scm(
+ scm.provider.as_deref(),
+ scm.owner.as_deref(),
+ scm.repository.as_deref(),
+ )
+}
+
+fn read_scm_layer(path: &Path) -> Result {
+ let source = std::fs::read_to_string(path)
+ .with_context(|| format!("failed to read {}", path.display()))?;
+ let run = parse_run_layer_from_settings_toml(&source)
+ .with_context(|| format!("failed to parse run settings from {}", path.display()))?;
+ Ok(run.scm.unwrap_or_default())
+}
+
struct ManifestRepoInfo {
/// The `origin` URL as libgit2 reports it (after `insteadOf` rewrites).
origin_url: Option,
@@ -505,6 +732,9 @@ enum BranchPublishStatus {
TrackingRefMatches,
Pushed,
Unavailable,
+ /// The configured repository is not the checkout's origin, so the push
+ /// is skipped entirely.
+ OriginMismatch,
}
/// Best-effort publication of the local branch so clone-based execution can
@@ -527,7 +757,7 @@ fn publish_manifest_branch_best_effort(
{
let remote = fabro_github::normalize_repo_origin_url(origin_url);
if remote != repo_origin_url {
- return BranchPublishStatus::Unavailable;
+ return BranchPublishStatus::OriginMismatch;
}
}
@@ -547,18 +777,25 @@ fn remotely_available_sha(
branch: &str,
local_sha: Option<&str>,
publish_status: BranchPublishStatus,
-) -> Option {
- let local_sha = local_sha?;
+) -> (Option, ExactCommitStatus) {
+ let Some(local_sha) = local_sha else {
+ return (None, ExactCommitStatus::Unpublished);
+ };
match publish_status {
- BranchPublishStatus::Pushed => Some(local_sha.to_owned()),
+ BranchPublishStatus::Pushed => (Some(local_sha.to_owned()), ExactCommitStatus::Available),
BranchPublishStatus::TrackingRefMatches => {
- git::remote_branch_sha_noninteractive(repo_path, "origin", branch)
- .ok()
- .flatten()
- .filter(|remote_sha| remote_sha == local_sha)
- .map(|_| local_sha.to_owned())
+ match git::remote_branch_sha_noninteractive(repo_path, "origin", branch) {
+ Ok(Some(remote_sha)) if remote_sha == local_sha => {
+ (Some(local_sha.to_owned()), ExactCommitStatus::Available)
+ }
+ Ok(_) => (None, ExactCommitStatus::Unpublished),
+ // The failure may carry raw Git stderr, so it is neither
+ // returned nor logged; the status tells callers what to say.
+ Err(_) => (None, ExactCommitStatus::Unverified),
+ }
}
- BranchPublishStatus::Unavailable => None,
+ BranchPublishStatus::Unavailable => (None, ExactCommitStatus::Unpublished),
+ BranchPublishStatus::OriginMismatch => (None, ExactCommitStatus::ConfiguredOriginMismatch),
}
}
diff --git a/lib/components/fabro-tool/Cargo.toml b/lib/components/fabro-tool/Cargo.toml
index 1e5395b9e..dc2936a3f 100644
--- a/lib/components/fabro-tool/Cargo.toml
+++ b/lib/components/fabro-tool/Cargo.toml
@@ -32,4 +32,5 @@ toml.workspace = true
[dev-dependencies]
fabro-types = { path = "../../foundation/fabro-types", features = ["test-support"] }
httpmock = "0.8"
+jsonschema.workspace = true
tempfile = "3"
diff --git a/lib/components/fabro-tool/src/common.rs b/lib/components/fabro-tool/src/common.rs
index f154759b8..7b92b3a09 100644
--- a/lib/components/fabro-tool/src/common.rs
+++ b/lib/components/fabro-tool/src/common.rs
@@ -1,5 +1,4 @@
use std::collections::HashMap;
-use std::path::Path;
use std::sync::LazyLock;
use async_trait::async_trait;
@@ -55,13 +54,8 @@ pub trait FabroToolBackend: Send + Sync {
Err(workflow_version_tool_unavailable_error())
}
- async fn create_run_from_spec(
- &self,
- spec: &crate::ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
- parent_id: Option,
- ) -> anyhow::Result;
+ async fn create_run_from_intent(&self, intent: fabro_types::RunIntent)
+ -> anyhow::Result;
async fn resolve_run(&self, selector: &str) -> anyhow::Result;
async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result;
@@ -149,15 +143,6 @@ pub(crate) fn workflow_version_tool_unavailable_error() -> anyhow::Error {
.into()
}
-pub trait RunManifestBuilder: Send + Sync {
- fn build_run_manifest(
- &self,
- spec: &crate::ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
- ) -> ToolResult;
-}
-
#[derive(Debug, Serialize, JsonSchema)]
pub struct RunSummaryResult {
pub run_id: String,
@@ -201,7 +186,7 @@ static TOOL_DEFINITIONS: LazyLock> = LazyLock::new(|| {
),
tool_definition::(
FABRO_RUN_CREATE_TOOL_NAME,
- "Create one or more Fabro workflow runs, optionally under a parent run, starting them by default.",
+ "Create runs from registered workflow_version_id values and canonical RunIntent settings. Register contents with fabro_workflow_version_create first. Standalone calls require an explicit target; native workers may inherit the parent target. Starts runs by default.",
),
tool_definition::(
FABRO_RUN_SEARCH_TOOL_NAME,
diff --git a/lib/components/fabro-tool/src/create.rs b/lib/components/fabro-tool/src/create.rs
index 448e5e84f..21449d610 100644
--- a/lib/components/fabro-tool/src/create.rs
+++ b/lib/components/fabro-tool/src/create.rs
@@ -1,368 +1,84 @@
-use std::borrow::Cow;
-use std::collections::HashMap;
-use std::path::{Path, PathBuf};
use std::sync::Arc;
-use fabro_types::RunId;
+use anyhow::Context as _;
+use fabro_types::{RunId, RunIntent, RunIntentArgs, RunProjection, RunTarget, WorkflowVersionId};
use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema};
use serde::{Deserialize, Deserializer, Serialize, de};
-use serde_json::Value;
use super::common::{self, FabroToolBackend, ToolError, ToolResult};
use super::manifest;
#[derive(Debug, Deserialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
pub struct FabroRunCreateParams {
- pub runs: Vec,
+ #[serde(deserialize_with = "deserialize_runs")]
+ #[schemars(length(min = 1, max = 50))]
+ pub runs: Vec,
}
-#[derive(Debug)]
-pub enum CreateRunSpecInput {
- Workflow(String),
- Spec(Box),
-}
-
-impl<'de> Deserialize<'de> for CreateRunSpecInput {
- fn deserialize(deserializer: D) -> Result
- where
- D: Deserializer<'de>,
- {
- let value = Value::deserialize(deserializer)?;
- match value {
- Value::String(workflow) => Ok(Self::Workflow(workflow)),
- Value::Object(_) => CreateRunSpec::deserialize(value)
- .map(Box::new)
- .map(Self::Spec)
- .map_err(de::Error::custom),
- other => Err(de::Error::custom(format!(
- "expected workflow string shorthand or create spec object, got {}",
- json_value_kind(&other)
- ))),
- }
- }
-}
-
-fn json_value_kind(value: &Value) -> &'static str {
- match value {
- Value::Null => "null",
- Value::Bool(_) => "boolean",
- Value::Number(_) => "number",
- Value::String(_) => "string",
- Value::Array(_) => "array",
- Value::Object(_) => "object",
- }
-}
-
-impl From for CreateRunSpecInput {
- fn from(spec: CreateRunSpec) -> Self {
- Self::Spec(Box::new(spec))
- }
-}
-
-impl JsonSchema for CreateRunSpecInput {
- fn inline_schema() -> bool {
- true
- }
-
- fn schema_name() -> Cow<'static, str> {
- "CreateRunSpecInput".into()
- }
-
- fn json_schema(_: &mut SchemaGenerator) -> Schema {
- json_schema!({
- "description": "Fabro run create specification. Use a workflow string shorthand, or an object when setting create options.",
- "anyOf": [
- {
- "type": "string",
- "description": "Workflow selector shorthand. Equivalent to an object with only the workflow field set."
- },
- {
- "type": "object",
- "description": "Full create-run specification.",
- "required": ["workflow"],
- "properties": {
- "workflow": {
- "type": "string",
- "description": "Workflow selector, such as a workflow name or workflow file path."
- },
- "cwd": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "description": "Working directory used to resolve relative workflow paths."
- },
- "parent_id": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "description": "Optional parent run id or selector."
- },
- "goal": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "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.",
- "additionalProperties": {
- "description": "Run input override value. Inputs are TOML-compatible scalar values: string, boolean, integer, or float.",
- "anyOf": [
- { "type": "string" },
- { "type": "boolean" },
- { "type": "integer" },
- { "type": "number" }
- ]
- }
- },
- "labels": {
- "type": "object",
- "description": "Labels to attach to the created run.",
- "additionalProperties": { "type": "string" }
- },
- "dry_run": {
- "anyOf": [
- { "type": "boolean" },
- { "type": "null" }
- ],
- "description": "Whether the run should use dry-run mode."
- },
- "auto_approve": {
- "anyOf": [
- { "type": "boolean" },
- { "type": "null" }
- ],
- "description": "Whether agent approval prompts should be auto-approved."
- },
- "model": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "description": "Model override for the run."
- },
- "provider": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "description": "Provider override for the run."
- },
- "environment": {
- "anyOf": [
- { "type": "string" },
- { "type": "null" }
- ],
- "description": "Named environment slug override for the run."
- },
- "preserve_sandbox": {
- "anyOf": [
- { "type": "boolean" },
- { "type": "null" }
- ],
- "description": "Whether to preserve the sandbox after the run."
- },
- "start": {
- "anyOf": [
- { "type": "boolean" },
- { "type": "null" }
- ],
- "description": "Whether to start the run immediately after creation. Defaults to true."
- }
- }
- }
- ]
- })
- }
+fn deserialize_runs<'de, D: Deserializer<'de>>(
+ deserializer: D,
+) -> Result, D::Error> {
+ Vec::::deserialize(deserializer).map_err(|error| {
+ de::Error::custom(format!(
+ "fabro_run_create requires workflow_version_id and canonical RunIntent fields; register file contents with fabro_workflow_version_create first: {error}"
+ ))
+ })
}
+/// RunIntent fields plus tool-only parent/target defaults and a separate start
+/// request.
#[derive(Debug, Deserialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
pub struct CreateRunSpec {
- pub workflow: String,
- pub cwd: Option,
- pub parent_id: Option,
- pub goal: Option,
- pub goal_file: Option,
+ #[schemars(with = "String")]
+ pub workflow_version_id: WorkflowVersionId,
#[serde(default)]
- pub inputs: HashMap,
+ #[schemars(schema_with = "run_target_schema")]
+ pub target: Option,
#[serde(default)]
- pub labels: HashMap,
- pub dry_run: Option,
- pub auto_approve: Option,
- pub model: Option,
- pub provider: Option,
- pub environment: Option,
- pub preserve_sandbox: Option,
- pub start: Option,
+ #[schemars(schema_with = "run_args_schema")]
+ pub args: RunIntentArgs,
+ pub environment_id: Option,
+ #[schemars(with = "Option")]
+ pub parent_id: Option,
+ pub title: Option,
+ pub goal: Option,
+ pub start: Option,
}
-#[derive(Debug, Deserialize)]
-#[serde(transparent)]
-pub struct RunInputValue(Value);
-
-impl From for RunInputValue {
- fn from(value: Value) -> Self {
- Self(value)
- }
+#[derive(Debug, Clone, Copy, Default)]
+pub struct CreateRunOptions {
+ /// Set only by trusted native worker dispatch, never from tool JSON.
+ pub forced_parent_id: Option,
}
-impl RunInputValue {
- pub(crate) fn into_inner(self) -> Value {
- self.0
- }
-}
-
-impl JsonSchema for RunInputValue {
- fn inline_schema() -> bool {
- true
- }
-
- fn schema_name() -> Cow<'static, str> {
- "RunInputValue".into()
- }
-
- fn json_schema(_: &mut SchemaGenerator) -> Schema {
- json_schema!({
- "description": "Run input override value. Inputs are TOML-compatible scalar values: string, boolean, integer, or float.",
- "anyOf": [
- { "type": "string" },
- { "type": "boolean" },
- { "type": "integer" },
- { "type": "number" }
- ]
- })
- }
-}
-
-#[derive(Debug)]
-pub struct ValidatedCreateRuns {
- pub runs: Vec,
-}
-
-#[derive(Debug)]
-pub struct ValidatedCreateRunSpec {
- pub workflow: String,
- pub cwd: Option,
- pub parent_id: Option,
- pub goal: Option,
- pub goal_file: Option,
- pub inputs: HashMap,
- pub labels: HashMap,
- pub dry_run: Option,
- pub auto_approve: Option,
- pub model: Option,
- pub provider: Option,
- pub environment: Option,
- pub preserve_sandbox: Option,
- pub start: Option,
-}
-
-impl TryFrom for ValidatedCreateRuns {
- type Error = ToolError;
-
- fn try_from(params: FabroRunCreateParams) -> Result {
- common::validate_len("runs", params.runs.len(), 1, 50)?;
- let runs = params
- .runs
- .into_iter()
- .map(ValidatedCreateRunSpec::try_from)
- .collect::, _>>()?;
- Ok(Self { runs })
- }
-}
-
-impl TryFrom for ValidatedCreateRunSpec {
- type Error = ToolError;
-
- fn try_from(spec: CreateRunSpecInput) -> Result {
- match spec {
- CreateRunSpecInput::Workflow(workflow) => {
- let workflow = workflow.trim();
- if workflow.is_empty() {
- return Err(ToolError::message("workflow must not be blank"));
+impl FabroRunCreateParams {
+ pub fn validate(&self, options: CreateRunOptions) -> ToolResult<()> {
+ common::validate_len("runs", self.runs.len(), 1, 50)?;
+ for spec in &self.runs {
+ if let Some(parent) = options.forced_parent_id {
+ if spec.parent_id.is_some_and(|id| id != parent) {
+ return Err(ToolError::message(format!(
+ "parent_id must be omitted or match the current run {parent}"
+ )));
}
- Self::try_from(CreateRunSpec {
- workflow: workflow.to_string(),
- cwd: None,
- parent_id: None,
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: None,
- auto_approve: None,
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: None,
- })
+ } else if spec.target.is_none() {
+ return Err(ToolError::message(
+ "standalone fabro_run_create requires an explicit target; use kind: none for an empty workspace",
+ ));
+ }
+ if let Some(target) = &spec.target {
+ target
+ .clone()
+ .validate()
+ .map_err(|error| ToolError::from_anyhow(&error.into()))?;
+ }
+ for (key, value) in &spec.args.inputs {
+ manifest::json_to_toml_value(key, value)?;
}
- CreateRunSpecInput::Spec(spec) => Self::try_from(*spec),
}
- }
-}
-
-impl TryFrom for ValidatedCreateRunSpec {
- type Error = ToolError;
-
- fn try_from(spec: CreateRunSpec) -> Result {
- let parent_id = spec
- .parent_id
- .as_deref()
- .map(str::trim)
- .filter(|parent_id| !parent_id.is_empty())
- .map(ToOwned::to_owned);
- 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()
- .map(|(key, value)| {
- let value = value.into_inner();
- manifest::json_to_toml_value(&key, &value).map(|value| (key, value))
- })
- .collect::>>()?;
- Ok(Self {
- workflow: spec.workflow,
- cwd: spec.cwd,
- parent_id,
- goal: spec.goal,
- goal_file: spec.goal_file,
- inputs,
- labels: spec.labels,
- dry_run: spec.dry_run,
- auto_approve: spec.auto_approve,
- model: spec.model,
- provider: spec.provider,
- environment: spec.environment,
- preserve_sandbox: spec.preserve_sandbox,
- start: spec.start,
- })
+ Ok(())
}
}
@@ -373,103 +89,117 @@ pub struct CreateRunsResult {
#[derive(Debug, Serialize, JsonSchema)]
pub struct CreatedRunResult {
- pub run_id: String,
- pub parent_id: Option,
- pub children_count: u64,
- pub workflow: String,
- pub start_requested: bool,
- pub status: String,
-}
-
-#[derive(Debug, Clone, Copy, Default)]
-pub struct CreateRunOptions {
- pub forced_parent_id: Option,
+ pub run_id: String,
+ pub parent_id: Option,
+ pub children_count: u64,
+ #[schemars(with = "String")]
+ pub workflow_version_id: WorkflowVersionId,
+ pub start_requested: bool,
+ pub status: String,
}
pub async fn create_runs(
backend: Arc,
- base_cwd: &Path,
- user_settings_path: &Path,
- params: ValidatedCreateRuns,
+ params: FabroRunCreateParams,
) -> ToolResult {
- create_runs_with_options(
- backend,
- base_cwd,
- user_settings_path,
- params,
- CreateRunOptions::default(),
- )
- .await
+ create_runs_with_options(backend, params, CreateRunOptions::default()).await
}
pub async fn create_runs_with_options(
backend: Arc,
- base_cwd: &Path,
- user_settings_path: &Path,
- params: ValidatedCreateRuns,
+ params: FabroRunCreateParams,
options: CreateRunOptions,
) -> ToolResult {
- let mut created = Vec::with_capacity(params.runs.len());
- let mut parent_id_cache = HashMap::::new();
+ params.validate(options)?;
+ let mut runs = Vec::with_capacity(params.runs.len());
+ let mut created_ids = Vec::new();
for spec in params.runs {
- let cwd = spec.cwd.clone().unwrap_or_else(|| base_cwd.to_path_buf());
- let parent_id = if let Some(forced_parent_id) = options.forced_parent_id {
- Some(forced_parent_id)
- } else if let Some(parent_selector) = spec.parent_id.as_deref() {
- Some(
- resolve_parent_run_id(backend.as_ref(), &mut parent_id_cache, parent_selector)
- .await?,
- )
- } else {
- None
- };
- let run_id = backend
- .create_run_from_spec(&spec, &cwd, user_settings_path, parent_id)
- .await
- .map_err(|err| ToolError::from_anyhow(&err))?;
- let start_requested = spec.start.unwrap_or(true);
- let summary = if start_requested {
- backend
- .start_run(&run_id, false)
- .await
- .map_err(|err| ToolError::from_anyhow(&err))?
- } else {
- backend
- .retrieve_run(&run_id)
- .await
- .map_err(|err| ToolError::from_anyhow(&err))?
- };
- created.push(CreatedRunResult {
- run_id: summary.id.to_string(),
- parent_id: summary.parent_id.map(|parent_id| parent_id.to_string()),
- children_count: summary.children_count,
- workflow: spec.workflow,
- start_requested,
- status: summary.lifecycle.status.kind().to_string(),
- });
+ let result: anyhow::Result = async {
+ let parent_id = options.forced_parent_id.or(spec.parent_id);
+ let target = if let Some(target) = spec.target {
+ target
+ } else {
+ let parent = options
+ .forced_parent_id
+ .context("an explicit target is required")?;
+ inherit_parent_target(&backend.get_run_state(&parent).await?)?
+ };
+ let intent = RunIntent {
+ workflow_version_id: spec.workflow_version_id,
+ target,
+ args: spec.args,
+ environment_id: spec.environment_id,
+ parent_id,
+ title: spec.title,
+ goal: spec.goal,
+ };
+ let run_id = backend.create_run_from_intent(intent).await?;
+ created_ids.push(run_id);
+ let start_requested = spec.start.unwrap_or(true);
+ let summary = if start_requested {
+ backend.start_run(&run_id, false).await.with_context(|| {
+ format!("run {run_id} was created but its start request failed")
+ })?
+ } else {
+ backend.retrieve_run(&run_id).await.with_context(|| {
+ format!("run {run_id} was created but retrieving its summary failed")
+ })?
+ };
+ Ok(CreatedRunResult {
+ run_id: run_id.to_string(),
+ parent_id: summary.parent_id.map(|id| id.to_string()),
+ children_count: summary.children_count,
+ workflow_version_id: spec.workflow_version_id,
+ start_requested,
+ status: summary.lifecycle.status.kind().to_string(),
+ })
+ }
+ .await;
+ match result {
+ Ok(run) => runs.push(run),
+ Err(error) => {
+ let error = if created_ids.is_empty() {
+ error
+ } else {
+ error.context(format!(
+ "already created run IDs: {}; inspect these runs before retrying",
+ created_ids
+ .iter()
+ .map(ToString::to_string)
+ .collect::>()
+ .join(", ")
+ ))
+ };
+ return Err(ToolError::from_anyhow(&error));
+ }
+ }
}
- Ok(CreateRunsResult { runs: created })
+ Ok(CreateRunsResult { runs })
}
-async fn resolve_parent_run_id(
- backend: &dyn FabroToolBackend,
- parent_id_cache: &mut HashMap,
- parent_selector: &str,
-) -> ToolResult {
- if let Ok(parent_id) = parent_selector.parse::() {
- return Ok(parent_id);
- }
- if let Some(parent_id) = parent_id_cache.get(parent_selector) {
- return Ok(*parent_id);
- }
-
- let parent_id = backend
- .resolve_run(parent_selector)
- .await
- .map_err(|err| ToolError::from_anyhow(&err))?
- .id;
- parent_id_cache.insert(parent_selector.to_string(), parent_id);
- Ok(parent_id)
+fn inherit_parent_target(parent: &RunProjection) -> anyhow::Result {
+ let target = parent.spec.target.as_ref().context(
+ "the parent run has no canonical target; send an explicit target for this child run",
+ )?;
+ Ok(match target {
+ RunTarget::Git(git) => {
+ let branch = if parent.spec.settings.run.run_branch.enabled {
+ parent.start.as_ref().and_then(|start| start.run_branch.as_ref())
+ .filter(|branch| !branch.trim().is_empty()).context(
+ "the parent run has no execution branch yet; send an explicit target for this child run"
+ )?
+ } else {
+ &git.branch
+ };
+ RunTarget::Git(fabro_types::GitRunTarget {
+ repo: git.repo.clone(),
+ branch: branch.clone(),
+ tag: None,
+ sha: None,
+ })
+ }
+ RunTarget::None {} | RunTarget::Folder { .. } => target.clone(),
+ })
}
pub fn create_runs_text(result: &CreateRunsResult) -> String {
@@ -480,407 +210,388 @@ pub fn create_runs_text(result: &CreateRunsResult) -> String {
)
}
+// Schema metadata for canonical types owned by fabro-types. Runtime values use
+// those types directly; parity tests below prevent tool-schema drift.
+fn run_args_schema(_: &mut SchemaGenerator) -> Schema {
+ json_schema!({
+ "type": "object", "additionalProperties": false,
+ "properties": {
+ "model": {"type": ["string", "null"]},
+ "provider": {"type": ["string", "null"]},
+ "inputs": {"type": "object", "additionalProperties": {"type": ["string", "boolean", "number"]}},
+ "labels": {"type": "object", "additionalProperties": {"type": "string"}},
+ "dry_run": {"type": ["boolean", "null"]},
+ "auto_approve": {"type": ["boolean", "null"]},
+ "preserve_sandbox": {"type": ["boolean", "null"]}
+ }
+ })
+}
+fn run_target_schema(_: &mut SchemaGenerator) -> Schema {
+ json_schema!({
+ "description": "Canonical workspace target. Required for standalone calls; native workers inherit the parent execution target when omitted.",
+ "anyOf": [
+ { "type": "null" },
+ {
+ "type": "object",
+ "required": ["kind", "repo", "branch"],
+ "additionalProperties": false,
+ "properties": {
+ "kind": { "const": "git" },
+ "repo": { "type": "string" },
+ "branch": { "type": "string" },
+ "tag": {
+ "anyOf": [
+ { "type": "string" },
+ { "type": "null" }
+ ]
+ },
+ "sha": {
+ "anyOf": [
+ { "type": "string" },
+ { "type": "null" }
+ ]
+ }
+ }
+ },
+ {
+ "type": "object",
+ "required": ["kind"],
+ "additionalProperties": false,
+ "properties": {
+ "kind": { "const": "none" }
+ }
+ },
+ {
+ "type": "object",
+ "required": ["kind", "path"],
+ "additionalProperties": false,
+ "properties": {
+ "kind": { "const": "folder" },
+ "path": { "type": "string" }
+ }
+ }
+ ]
+ })
+}
+
#[cfg(test)]
mod tests {
- use std::sync::Mutex;
+ use std::collections::HashMap;
- use async_trait::async_trait;
use chrono::{TimeZone, Utc};
- use fabro_api::types;
use fabro_types::{
- EventEnvelope, Run, RunLifecycle, RunLinks, RunOrigin, RunProjection, RunStatus,
- RunTimestamps, WorkflowRef, test_support,
+ GitRunTarget, Run, RunLifecycle, RunLinks, RunOrigin, RunStatus, RunTimestamps,
+ WorkflowRef, test_support,
};
- use schemars::SchemaGenerator;
- use serde_json::json;
+ use httpmock::Method::{GET, POST};
+ use httpmock::MockServer;
+ use serde_json::{Value, json};
use super::*;
+ use crate::fabro_client::ClientBackend;
+
+ fn version_id() -> WorkflowVersionId {
+ fabro_types::BlobHash::new(b"workflow").into()
+ }
+ fn params(value: Value) -> FabroRunCreateParams {
+ FabroRunCreateParams {
+ runs: vec![serde_json::from_value(value).unwrap()],
+ }
+ }
+ fn spec() -> Value {
+ json!({"workflow_version_id": version_id(), "target": {"kind": "none"}, "start": false})
+ }
+ fn backend(server: &MockServer) -> Arc {
+ Arc::new(ClientBackend::new(Arc::new(
+ fabro_client::Client::new_no_proxy(&server.url("")).unwrap(),
+ )))
+ }
+ fn parent(target: RunTarget) -> RunProjection {
+ let mut spec = test_support::test_run_spec();
+ spec.target = Some(target);
+ RunProjection::new(String::new(), spec, Utc::now())
+ }
#[test]
- fn run_input_value_schema_allows_only_json_scalars() {
- let mut generator = SchemaGenerator::default();
- let schema = RunInputValue::json_schema(&mut generator);
- let schema = serde_json::to_value(schema).expect("schema should serialize");
-
+ fn run_create_accepts_canonical_id_target_and_args() {
+ let mut value = spec();
+ value["args"] = json!({"model":"model", "provider":"provider", "inputs":{"s":"x", "b":true,"i":4,"f":1.25}, "labels":{"x":"y"}, "dry_run":false, "auto_approve":true, "preserve_sandbox":false});
+ let p = params(value.clone());
+ p.validate(CreateRunOptions::default()).unwrap();
assert_eq!(
- schema["anyOf"],
- json!([
- { "type": "string" },
- { "type": "boolean" },
- { "type": "integer" },
- { "type": "number" },
- ])
+ serde_json::to_value(&p.runs[0].args).unwrap(),
+ value["args"]
);
+ let p = params(spec());
+ assert_eq!(serde_json::to_value(&p.runs[0].args).unwrap(), json!({}));
+ assert_eq!(p.runs[0].environment_id, None);
}
#[test]
- fn create_spec_schema_omits_run_id() {
- let mut generator = SchemaGenerator::default();
- let schema = CreateRunSpecInput::json_schema(&mut generator);
- let schema = serde_json::to_value(schema).expect("schema should serialize");
- let properties = schema["anyOf"][1]["properties"]
- .as_object()
- .expect("object form should have properties");
-
- assert!(!properties.contains_key("run_id"));
+ fn run_create_schema_and_serde_reject_old_sources_and_accept_canonical_variants() {
+ let schema = serde_json::to_value(schemars::schema_for!(FabroRunCreateParams)).unwrap();
+ let validator = jsonschema::validator_for(&schema).unwrap();
+ for target in [
+ json!({"kind":"none"}),
+ json!({"kind":"folder","path":"/workspace"}),
+ json!({"kind":"git","repo":"acme/repo","branch":"main","sha":"a".repeat(40),"tag":"v1"}),
+ ] {
+ let value = json!({"runs":[{"workflow_version_id": version_id(), "target":target, "args":{"dry_run":false}}]});
+ assert!(validator.is_valid(&value));
+ serde_json::from_value::(value)
+ .unwrap()
+ .validate(CreateRunOptions::default())
+ .unwrap();
+ }
+ for old in [
+ json!("workflow"),
+ json!({"workflow":"workflow"}),
+ json!({"workflow":{"kind":"inline","entrypoint":"x","files":{"x":"content"}}}),
+ ] {
+ let value = json!({"runs":[old]});
+ assert!(!validator.is_valid(&value));
+ let error = serde_json::from_value::(value)
+ .unwrap_err()
+ .to_string();
+ assert!(error.contains("fabro_workflow_version_create"));
+ }
+ for (key, value) in [
+ ("cwd", json!("/tmp")),
+ ("goal_file", json!("goal.md")),
+ ("inputs", json!({})),
+ ("environment", json!("local")),
+ ] {
+ let mut item = spec();
+ item[key] = value;
+ let value = json!({"runs":[item]});
+ assert!(!validator.is_valid(&value));
+ assert!(serde_json::from_value::(value).is_err());
+ }
+ for value in [json!(null), json!([]), json!({})] {
+ let mut item = spec();
+ item["args"] = json!({"inputs":{"bad":value}});
+ assert!(params(item).validate(CreateRunOptions::default()).is_err());
+ }
}
#[test]
- fn create_spec_accepts_parent_selector() {
- let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec {
- workflow: "simple.fabro".to_string(),
- cwd: None,
- parent_id: Some(" nightly-parent ".to_string()),
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: None,
- auto_approve: None,
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: None,
- })
- .expect("parent selectors should validate without requiring exact run ids");
-
- assert_eq!(spec.parent_id.as_deref(), Some("nightly-parent"));
- }
-
- #[test]
- fn create_params_accept_string_shorthand() {
- let params: FabroRunCreateParams = serde_json::from_value(json!({
- "runs": ["simple.fabro"]
- }))
- .expect("string shorthand should deserialize");
-
- let params = ValidatedCreateRuns::try_from(params)
- .expect("string shorthand should validate as workflow selector");
- let spec = ¶ms.runs[0];
- assert_eq!(spec.workflow, "simple.fabro");
- assert_eq!(spec.cwd, None);
- assert_eq!(spec.parent_id, None);
- assert!(spec.inputs.is_empty());
- assert!(spec.labels.is_empty());
- assert_eq!(spec.start, None);
- }
-
- #[test]
- fn create_params_preserve_object_form_options() {
- let params: FabroRunCreateParams = serde_json::from_value(json!({
- "runs": [{
- "workflow": "simple.fabro",
- "dry_run": true,
- "auto_approve": true,
- "labels": { "source": "mcp-test" },
- "start": false
- }]
- }))
- .expect("object form should deserialize");
-
- let params =
- ValidatedCreateRuns::try_from(params).expect("object form should still validate");
- let spec = ¶ms.runs[0];
- assert_eq!(spec.workflow, "simple.fabro");
- assert_eq!(spec.dry_run, Some(true));
- assert_eq!(spec.auto_approve, Some(true));
- assert_eq!(
- spec.labels.get("source").map(String::as_str),
- Some("mcp-test")
- );
- assert_eq!(spec.start, Some(false));
- }
-
- #[test]
- fn create_params_ignore_removed_run_id() {
- let params: FabroRunCreateParams = serde_json::from_value(json!({
- "runs": [{
- "workflow": "simple.fabro",
- "run_id": "not-a-valid-run-id",
- "start": false
- }]
- }))
- .expect("old object form should deserialize with run_id ignored");
-
- let params =
- ValidatedCreateRuns::try_from(params).expect("remaining create fields should validate");
- assert_eq!(params.runs[0].workflow, "simple.fabro");
- assert_eq!(params.runs[0].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 = ¶ms.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");
+ fn run_create_context_validation_requires_standalone_target_and_current_worker_parent() {
+ let mut item = spec();
+ item.as_object_mut().unwrap().remove("target");
+ item["parent_id"] = json!(RunId::new());
assert!(
- err.to_string()
- .contains("goal and goal_file are mutually exclusive"),
- "{err}"
+ params(item.clone())
+ .validate(CreateRunOptions::default())
+ .unwrap_err()
+ .as_str()
+ .contains("explicit target")
);
- }
-
- #[test]
- fn create_params_reject_blank_string_shorthand_workflow() {
- let params: FabroRunCreateParams = serde_json::from_value(json!({
- "runs": [" "]
- }))
- .expect("blank shorthand should deserialize before validation");
-
- let err = ValidatedCreateRuns::try_from(params).expect_err("blank workflow should fail");
- assert!(err.to_string().contains("workflow"), "{err}");
- }
-
- #[test]
- fn create_params_missing_object_workflow_keeps_field_error() {
- let err = serde_json::from_value::(json!({
- "runs": [{ "dry_run": true }]
- }))
- .expect_err("object form without workflow should fail deserialization");
-
+ let options = CreateRunOptions {
+ forced_parent_id: Some(RunId::new()),
+ };
assert!(
- err.to_string().contains("missing field `workflow`"),
- "{err}"
+ params(item.clone())
+ .validate(options)
+ .unwrap_err()
+ .as_str()
+ .contains("current run")
);
+ item["parent_id"] = json!(options.forced_parent_id);
+ params(item).validate(options).unwrap();
+ assert!(
+ FabroRunCreateParams { runs: vec![] }
+ .validate(options)
+ .is_err()
+ );
+ }
+
+ #[test]
+ fn inherited_target_uses_execution_branch_and_requires_execution_state() {
+ let mut p = parent(RunTarget::Git(GitRunTarget {
+ repo: "acme/repo".into(),
+ branch: "main".into(),
+ sha: Some("a".repeat(40)),
+ tag: Some("v1".into()),
+ }));
+ assert!(
+ inherit_parent_target(&p)
+ .unwrap_err()
+ .to_string()
+ .contains("no execution branch")
+ );
+ p.start = Some(fabro_types::StartRecord {
+ start_time: Utc::now(),
+ run_branch: Some("fabro/run/parent".into()),
+ base_sha: None,
+ });
+ let RunTarget::Git(target) = inherit_parent_target(&p).unwrap() else {
+ panic!("expected git")
+ };
+ assert_eq!(target.branch, "fabro/run/parent");
+ assert_eq!(target.sha, None);
+ assert_eq!(target.tag, None);
+ p.spec.settings.run.run_branch.enabled = false;
+ let RunTarget::Git(target) = inherit_parent_target(&p).unwrap() else {
+ panic!("expected git")
+ };
+ assert_eq!(target.branch, "main");
+ assert_eq!(target.sha, None);
+ assert_eq!(target.tag, None);
+ for target in [RunTarget::None {}, RunTarget::Folder {
+ path: "/workspace".into(),
+ }] {
+ p.spec.target = Some(target.clone());
+ assert_eq!(inherit_parent_target(&p).unwrap(), target);
+ }
+ p.spec.target = None;
+ assert!(inherit_parent_target(&p).is_err());
}
#[tokio::test]
- async fn create_runs_resolves_parent_selector_and_sends_parent_id_to_backend() {
- let temp = tempfile::tempdir().expect("tempdir should be created");
- let settings = temp.path().join("settings.toml");
- let child_id = run_id("01KRBZW5C00000000000000001");
- let parent_id = run_id("01KRBZW4DW0000000000000002");
- let backend = Arc::new(MockCreateBackend {
- child_id,
- parent_id,
- created_parent_ids: Mutex::new(Vec::new()),
- resolved_selectors: Mutex::new(Vec::new()),
- started_run_ids: Mutex::new(Vec::new()),
- });
- let params = ValidatedCreateRuns::try_from(FabroRunCreateParams {
- runs: vec![
- CreateRunSpec {
- workflow: "simple.fabro".to_string(),
- cwd: None,
- parent_id: Some("nightly-parent".to_string()),
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: Some(true),
- auto_approve: Some(true),
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: Some(false),
- }
- .into(),
- ],
- })
- .expect("create params should validate");
-
- let result = create_runs(backend.clone(), temp.path(), &settings, params)
- .await
- .expect("run should be created");
-
- assert_eq!(result.runs[0].parent_id, Some(parent_id.to_string()));
- assert_eq!(result.runs[0].children_count, 0);
- assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[
- Some(parent_id)
- ]);
- assert_eq!(backend.resolved_selectors.lock().unwrap().as_slice(), &[
- "nightly-parent".to_string()
- ]);
- }
-
- #[tokio::test]
- async fn create_runs_reuses_parent_selector_resolution_within_batch() {
- let temp = tempfile::tempdir().expect("tempdir should be created");
- let settings = temp.path().join("settings.toml");
- let child_id = run_id("01KRBZW5C00000000000000001");
- let parent_id = run_id("01KRBZW4DW0000000000000002");
- let backend = Arc::new(MockCreateBackend {
- child_id,
- parent_id,
- created_parent_ids: Mutex::new(Vec::new()),
- resolved_selectors: Mutex::new(Vec::new()),
- started_run_ids: Mutex::new(Vec::new()),
- });
- let runs: Vec = (0..2)
- .map(|_| {
- CreateRunSpecInput::from(CreateRunSpec {
- workflow: "simple.fabro".to_string(),
- cwd: None,
- parent_id: Some("nightly-parent".to_string()),
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: Some(true),
- auto_approve: Some(true),
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: Some(false),
- })
+ async fn run_create_submits_canonical_intent_without_registration_or_parent_lookup() {
+ let server = MockServer::start_async().await;
+ let id = RunId::new();
+ let parent_id = RunId::new();
+ let mut item = spec();
+ item["parent_id"] = json!(parent_id);
+ item["title"] = json!("Title");
+ item["goal"] = json!("Literal goal");
+ item["environment_id"] = json!("environment");
+ item["args"] = json!({"dry_run":false});
+ let mut intent = item.clone();
+ intent.as_object_mut().unwrap().remove("start");
+ let create = server
+ .mock_async(|when, then| {
+ when.method(POST)
+ .path("/api/v1/runs")
+ .json_body_obj(&intent);
+ then.status(201).json_body_obj(&run(id, None, 0));
})
- .collect();
- let params = ValidatedCreateRuns::try_from(FabroRunCreateParams { runs })
- .expect("create params should validate");
-
- create_runs(backend.clone(), temp.path(), &settings, params)
- .await
- .expect("runs should be created");
-
- assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[
- Some(parent_id),
- Some(parent_id),
- ]);
- assert_eq!(backend.resolved_selectors.lock().unwrap().as_slice(), &[
- "nightly-parent".to_string()
- ]);
+ .await;
+ server
+ .mock_async(|when, then| {
+ when.method(GET).path(format!("/api/v1/runs/{id}"));
+ then.status(200).json_body_obj(&run(id, Some(parent_id), 0));
+ })
+ .await;
+ let registration = server
+ .mock_async(|when, then| {
+ when.path("/api/v1/workflow-versions");
+ then.status(500);
+ })
+ .await;
+ let state = server
+ .mock_async(|when, then| {
+ when.path(format!("/api/v1/runs/{parent_id}/state"));
+ then.status(500);
+ })
+ .await;
+ let result = create_runs(backend(&server), params(item)).await.unwrap();
+ assert_eq!(result.runs[0].run_id, id.to_string());
+ assert!(!result.runs[0].start_requested);
+ create.assert_calls_async(1).await;
+ registration.assert_calls_async(0).await;
+ state.assert_calls_async(0).await;
}
#[tokio::test]
- async fn create_runs_forced_parent_id_skips_selector_resolution() {
- let temp = tempfile::tempdir().expect("tempdir should be created");
- let settings = temp.path().join("settings.toml");
- let child_id = run_id("01KRBZW5C00000000000000001");
- let parent_id = run_id("01KRBZW4DW0000000000000002");
- let backend = Arc::new(MockCreateBackend {
- child_id,
- parent_id,
- created_parent_ids: Mutex::new(Vec::new()),
- resolved_selectors: Mutex::new(Vec::new()),
- started_run_ids: Mutex::new(Vec::new()),
- });
- let params = ValidatedCreateRuns::try_from(FabroRunCreateParams {
- runs: vec![
- CreateRunSpec {
- workflow: "simple.fabro".to_string(),
- cwd: None,
- parent_id: Some(parent_id.to_string()),
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: Some(true),
- auto_approve: Some(true),
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: Some(false),
- }
- .into(),
- ],
+ async fn run_create_worker_inherits_fresh_parent_state() {
+ let server = MockServer::start_async().await;
+ let id = RunId::new();
+ let p = parent(RunTarget::None {});
+ let parent_id = p.spec.id();
+ let state = server
+ .mock_async(|when, then| {
+ when.method(GET)
+ .path(format!("/api/v1/runs/{parent_id}/state"));
+ then.status(200).json_body_obj(&p);
+ })
+ .await;
+ let create = server
+ .mock_async(|when, then| {
+ when.method(POST).path("/api/v1/runs").json_body(json!({
+ "workflow_version_id": version_id(),
+ "target": {"kind": "none"},
+ "args": {},
+ "parent_id": parent_id
+ }));
+ then.status(201).json_body_obj(&run(id, Some(parent_id), 0));
+ })
+ .await;
+ server
+ .mock_async(|when, then| {
+ when.method(GET).path(format!("/api/v1/runs/{id}"));
+ then.status(200).json_body_obj(&run(id, Some(parent_id), 0));
+ })
+ .await;
+ let mut item = spec();
+ item.as_object_mut().unwrap().remove("target");
+ create_runs_with_options(backend(&server), params(item), CreateRunOptions {
+ forced_parent_id: Some(parent_id),
})
- .expect("create params should validate");
-
- create_runs_with_options(
- backend.clone(),
- temp.path(),
- &settings,
- params,
- CreateRunOptions {
- forced_parent_id: Some(parent_id),
- },
- )
.await
- .expect("run should be created");
-
- assert_eq!(backend.created_parent_ids.lock().unwrap().as_slice(), &[
- Some(parent_id)
- ]);
- assert!(backend.resolved_selectors.lock().unwrap().is_empty());
+ .unwrap();
+ state.assert_calls_async(1).await;
+ create.assert_calls_async(1).await;
}
#[tokio::test]
- async fn create_runs_defaults_to_start_request_and_reports_pending_child_status() {
- let temp = tempfile::tempdir().expect("tempdir should be created");
- let settings = temp.path().join("settings.toml");
- let child_id = run_id("01KRBZW5C00000000000000001");
- let parent_id = run_id("01KRBZW4DW0000000000000002");
- let backend = Arc::new(MockCreateBackend {
- child_id,
- parent_id,
- created_parent_ids: Mutex::new(Vec::new()),
- resolved_selectors: Mutex::new(Vec::new()),
- started_run_ids: Mutex::new(Vec::new()),
- });
- let params = ValidatedCreateRuns::try_from(FabroRunCreateParams {
- runs: vec![
- CreateRunSpec {
- workflow: "simple.fabro".to_string(),
- cwd: None,
- parent_id: Some(parent_id.to_string()),
- goal: None,
- goal_file: None,
- inputs: HashMap::new(),
- labels: HashMap::new(),
- dry_run: Some(true),
- auto_approve: Some(true),
- model: None,
- provider: None,
- environment: None,
- preserve_sandbox: None,
- start: None,
- }
- .into(),
- ],
- })
- .expect("create params should validate");
-
- let result = create_runs(backend.clone(), temp.path(), &settings, params)
+ async fn run_create_start_and_later_batch_failures_report_already_created_ids() {
+ let server = MockServer::start_async().await;
+ let id = RunId::new();
+ server
+ .mock_async(|when, then| {
+ when.method(POST)
+ .path("/api/v1/runs")
+ .json_body_includes(json!({"goal":"first"}).to_string());
+ then.status(201).json_body_obj(&run(id, None, 0));
+ })
+ .await;
+ let start = server
+ .mock_async(|when, then| {
+ when.method(POST).path(format!("/api/v1/runs/{id}/start"));
+ then.status(500);
+ })
+ .await;
+ let mut item = spec();
+ item["goal"] = json!("first");
+ item.as_object_mut().unwrap().remove("start");
+ let error = create_runs(backend(&server), params(item.clone()))
.await
- .expect("run should be created and start requested");
-
- assert!(result.runs[0].start_requested);
- assert_eq!(result.runs[0].status, "pending");
- assert_eq!(backend.started_run_ids.lock().unwrap().as_slice(), &[
- child_id
- ]);
- assert_eq!(
- create_runs_text(&result),
- "created 1 Fabro run(s), start requested for 1"
- );
+ .unwrap_err();
+ assert!(error.as_str().contains(&id.to_string()));
+ assert!(error.as_str().contains("start request failed"));
+ start.assert_calls_async(1).await;
+ item["start"] = json!(false);
+ server
+ .mock_async(|when, then| {
+ when.method(GET).path(format!("/api/v1/runs/{id}"));
+ then.status(200).json_body_obj(&run(id, None, 0));
+ })
+ .await;
+ let failed = server
+ .mock_async(|when, then| {
+ when.method(POST)
+ .path("/api/v1/runs")
+ .json_body_includes(json!({"goal":"second"}).to_string());
+ then.status(400);
+ })
+ .await;
+ let mut second = item.clone();
+ second["goal"] = json!("second");
+ let p = serde_json::from_value(json!({"runs":[item,second]})).unwrap();
+ let error = create_runs(backend(&server), p).await.unwrap_err();
+ assert!(error.as_str().contains(&id.to_string()));
+ failed.assert_calls_async(1).await;
}
- fn run_id(raw: &str) -> RunId {
- raw.parse().expect("test run id should parse")
+ #[tokio::test]
+ async fn run_create_same_run_backend_denies_before_network() {
+ let client = fabro_client::Client::new_no_proxy("http://127.0.0.1:1").unwrap();
+ let backend = Arc::new(ClientBackend::new(Arc::new(client)).with_run_scope(RunId::new()));
+ let error = create_runs(backend, params(spec())).await.unwrap_err();
+ assert!(error.as_str().contains("outside this tool session"));
}
-
fn run(run_id: RunId, parent_id: Option, children_count: u64) -> Run {
run_with_status(run_id, parent_id, children_count, RunStatus::Submitted)
}
@@ -939,145 +650,4 @@ mod tests {
links: RunLinks { web: None },
}
}
-
- struct MockCreateBackend {
- child_id: RunId,
- parent_id: RunId,
- created_parent_ids: Mutex>>,
- resolved_selectors: Mutex>,
- started_run_ids: Mutex>,
- }
-
- #[async_trait]
- impl FabroToolBackend for MockCreateBackend {
- async fn create_run_from_spec(
- &self,
- _spec: &ValidatedCreateRunSpec,
- _cwd: &Path,
- _user_settings_path: &Path,
- parent_id: Option,
- ) -> anyhow::Result {
- self.created_parent_ids.lock().unwrap().push(parent_id);
- Ok(self.child_id)
- }
-
- async fn resolve_run(&self, selector: &str) -> anyhow::Result {
- assert_eq!(selector, "nightly-parent");
- self.resolved_selectors
- .lock()
- .unwrap()
- .push(selector.to_string());
- Ok(run(self.parent_id, None, 1))
- }
-
- async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result {
- assert_eq!(*run_id, self.child_id);
- Ok(run(self.child_id, Some(self.parent_id), 0))
- }
-
- async fn start_run(&self, run_id: &RunId, resume: bool) -> anyhow::Result {
- assert_eq!(*run_id, self.child_id);
- assert!(!resume);
- self.started_run_ids.lock().unwrap().push(*run_id);
- Ok(run_with_status(
- self.child_id,
- Some(self.parent_id),
- 0,
- RunStatus::Pending {
- reason: fabro_types::PendingReason::ApprovalRequired,
- },
- ))
- }
-
- async fn approve_run(&self, _run_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn deny_run(&self, _run_id: &RunId, _reason: Option) -> anyhow::Result {
- unreachable!()
- }
-
- async fn cancel_run(&self, _run_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn interrupt_run(&self, _run_id: &RunId) -> anyhow::Result<()> {
- unreachable!()
- }
-
- async fn steer_run(
- &self,
- _run_id: &RunId,
- _text: String,
- _interrupt: bool,
- ) -> anyhow::Result<()> {
- unreachable!()
- }
-
- async fn archive_run(&self, _run_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn unarchive_run(&self, _run_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn list_store_runs(&self) -> anyhow::Result> {
- unreachable!()
- }
-
- async fn list_store_runs_by_parent(&self, _parent_id: RunId) -> anyhow::Result> {
- unreachable!()
- }
-
- async fn link_run_parent(
- &self,
- _child_id: &RunId,
- _parent_id: &RunId,
- ) -> anyhow::Result {
- unreachable!()
- }
-
- async fn unlink_run_parent(&self, _child_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn get_run_state(&self, _run_id: &RunId) -> anyhow::Result {
- unreachable!()
- }
-
- async fn list_run_events(
- &self,
- _run_id: &RunId,
- _after: Option,
- _limit: Option,
- ) -> anyhow::Result> {
- unreachable!()
- }
-
- async fn list_run_events_until(
- &self,
- _run_id: &RunId,
- _after: Option,
- _limit: usize,
- ) -> anyhow::Result> {
- unreachable!()
- }
-
- async fn list_run_questions(
- &self,
- _run_id: &RunId,
- ) -> anyhow::Result> {
- unreachable!()
- }
-
- async fn submit_run_answer(
- &self,
- _run_id: &RunId,
- _question_id: &str,
- _body: types::SubmitAnswerRequest,
- ) -> anyhow::Result<()> {
- unreachable!()
- }
- }
}
diff --git a/lib/components/fabro-tool/src/fabro_client.rs b/lib/components/fabro-tool/src/fabro_client.rs
index 120176183..f21e99bc5 100644
--- a/lib/components/fabro-tool/src/fabro_client.rs
+++ b/lib/components/fabro-tool/src/fabro_client.rs
@@ -1,19 +1,17 @@
-use std::path::Path;
use std::sync::Arc;
use async_trait::async_trait;
use fabro_api::types;
use fabro_types::{
EventEnvelope, PairId, PairMessageRecord, PairMessageRequest, PairRecord,
- PairTranscriptResponse, Run, RunId, RunPairStatusResponse, RunProjection, StageId,
+ PairTranscriptResponse, Run, RunId, RunIntent, RunPairStatusResponse, RunProjection, StageId,
};
-use crate::{FabroToolBackend, RunManifestBuilder, ToolError, common};
+use crate::{FabroToolBackend, common};
#[derive(Clone)]
pub struct ClientBackend {
client: Arc<::fabro_client::Client>,
- manifest_builder: Option>,
run_scope: Option,
workflow_version_packager: Option>,
}
@@ -23,18 +21,11 @@ impl ClientBackend {
pub fn new(client: Arc<::fabro_client::Client>) -> Self {
Self {
client,
- manifest_builder: None,
run_scope: None,
workflow_version_packager: None,
}
}
- #[must_use]
- pub fn with_manifest_builder(mut self, builder: Arc) -> Self {
- self.manifest_builder = Some(builder);
- self
- }
-
#[must_use]
pub fn with_workflow_version_packager(
mut self,
@@ -90,28 +81,12 @@ impl FabroToolBackend for ClientBackend {
Ok(packaged.root_id())
}
- async fn create_run_from_spec(
- &self,
- spec: &crate::ValidatedCreateRunSpec,
- cwd: &Path,
- user_settings_path: &Path,
- parent_id: Option,
- ) -> anyhow::Result {
- if let Some(parent_id) = parent_id.as_ref() {
- self.ensure_run_scope(parent_id)?;
- }
- let Some(builder) = self.manifest_builder.as_ref() else {
- return Err(ToolError::message(format!(
- "{} is not available",
- crate::FABRO_RUN_CREATE_TOOL_NAME
- ))
- .into());
- };
- let mut manifest = builder
- .build_run_manifest(spec, cwd, user_settings_path)
- .map_err(anyhow::Error::new)?;
- manifest.parent_id = parent_id.map(|run_id| run_id.to_string());
- self.client.create_run_from_manifest(manifest).await
+ async fn create_run_from_intent(&self, intent: RunIntent) -> anyhow::Result {
+ anyhow::ensure!(
+ self.run_scope.is_none(),
+ "run creation is outside this tool session's run scope"
+ );
+ self.client.create_run_from_intent(intent).await
}
async fn resolve_run(&self, selector: &str) -> anyhow::Result {
diff --git a/lib/components/fabro-tool/src/interact.rs b/lib/components/fabro-tool/src/interact.rs
index 503147ece..0757cdb51 100644
--- a/lib/components/fabro-tool/src/interact.rs
+++ b/lib/components/fabro-tool/src/interact.rs
@@ -446,7 +446,6 @@ fn normalize_optional_text(value: Option<&str>) -> Option {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
- use std::path::Path;
use std::sync::{Arc, Mutex};
use async_trait::async_trait;
@@ -742,12 +741,9 @@ mod tests {
#[async_trait]
impl FabroToolBackend for MockInteractBackend {
- async fn create_run_from_spec(
+ async fn create_run_from_intent(
&self,
- _spec: &crate::ValidatedCreateRunSpec,
- _cwd: &Path,
- _user_settings_path: &Path,
- _parent_id: Option,
+ _intent: fabro_types::RunIntent,
) -> anyhow::Result {
unreachable!()
}
diff --git a/lib/components/fabro-tool/src/lib.rs b/lib/components/fabro-tool/src/lib.rs
index 5145e7608..123c64af3 100644
--- a/lib/components/fabro-tool/src/lib.rs
+++ b/lib/components/fabro-tool/src/lib.rs
@@ -20,12 +20,11 @@ pub use common::{
FABRO_RUN_CREATE_TOOL_NAME, FABRO_RUN_EVENTS_TOOL_NAME, FABRO_RUN_GATHER_TOOL_NAME,
FABRO_RUN_GET_TOOL_NAME, FABRO_RUN_INTERACT_TOOL_NAME, FABRO_RUN_PAIR_TOOL_NAME,
FABRO_RUN_SEARCH_TOOL_NAME, FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME, FabroToolBackend,
- RunManifestBuilder, RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions,
+ RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions,
};
pub use create::{
- CreateRunOptions, CreateRunSpec, CreateRunSpecInput, CreateRunsResult, CreatedRunResult,
- FabroRunCreateParams, RunInputValue, ValidatedCreateRunSpec, ValidatedCreateRuns, create_runs,
- create_runs_text, create_runs_with_options,
+ CreateRunOptions, CreateRunSpec, CreateRunsResult, CreatedRunResult, FabroRunCreateParams,
+ create_runs, create_runs_text, create_runs_with_options,
};
pub use events::{
FabroRunEventsParams, RunEventResult, RunEventsAction, RunEventsResult, ValidatedRunEvents,
diff --git a/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs b/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs
index c2903a64a..0152b5d8f 100644
--- a/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs
+++ b/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs
@@ -3,7 +3,6 @@
use std::sync::Arc;
-use fabro_types::RunId;
use pebble_coding_agent::tools::{RegisteredTool, ToolError, ToolSource};
use serde::de::DeserializeOwned;
@@ -77,13 +76,9 @@ pub(crate) async fn execute_fabro_run_tool(
}
fabro_tool::FABRO_RUN_CREATE_TOOL_NAME => {
let params = parse_fabro_tool_args::(name, args)?;
- ensure_current_run_parent(¶ms, services.current_run_id)?;
- let validated = fabro_tool::ValidatedCreateRuns::try_from(params)?;
let result = fabro_tool::create_runs_with_options(
Arc::clone(&services.backend),
- &services.base_cwd,
- &services.user_settings_path,
- validated,
+ params,
fabro_tool::CreateRunOptions {
forced_parent_id: Some(services.current_run_id),
},
@@ -168,34 +163,6 @@ where
.map_err(|err| fabro_tool::ToolError::message(format!("invalid {name} arguments: {err}")))
}
-fn ensure_current_run_parent(
- params: &fabro_tool::FabroRunCreateParams,
- current_run_id: RunId,
-) -> fabro_tool::ToolResult<()> {
- let current_parent = current_run_id.to_string();
- for run in ¶ms.runs {
- let parent_id = match run {
- fabro_tool::CreateRunSpecInput::Workflow(_) => None,
- fabro_tool::CreateRunSpecInput::Spec(spec) => spec.parent_id.as_deref().map(str::trim),
- };
- match parent_id {
- None => {}
- Some("") => {
- return Err(fabro_tool::ToolError::message(
- "parent_id must be omitted or match the current run; blank parent_id is invalid",
- ));
- }
- Some(parent_id) if parent_id == current_parent => {}
- Some(parent_id) => {
- return Err(fabro_tool::ToolError::message(format!(
- "parent_id must be omitted or match the current run {current_parent}; got {parent_id}"
- )));
- }
- }
- }
- Ok(())
-}
-
fn render_fabro_tool_result(summary: &str, result: &T) -> fabro_tool::ToolResult
where
T: serde::Serialize,
@@ -219,6 +186,57 @@ mod tests {
use super::*;
+ #[tokio::test]
+ async fn native_run_create_submits_intent_and_enforces_current_parent() {
+ let server = httpmock::MockServer::start_async().await;
+ let parent_id = fabro_types::RunId::new();
+ let version_id: fabro_types::WorkflowVersionId =
+ fabro_types::BlobHash::new(b"registered workflow").into();
+ let create = server
+ .mock_async(|when, then| {
+ when.method(httpmock::Method::POST)
+ .path("/api/v1/runs")
+ .json_body(json!({
+ "workflow_version_id": version_id,
+ "target": {"kind":"none"},
+ "parent_id": parent_id,
+ "args": {"auto_approve":false}
+ }));
+ // Admission rejection proves the native dispatcher reached the
+ // canonical API without registering or looking up a workflow.
+ then.status(422).body("native admission rejection");
+ })
+ .await;
+ let state = server
+ .mock_async(|when, then| {
+ when.path(format!("/api/v1/runs/{parent_id}/state"));
+ then.status(500);
+ })
+ .await;
+ let client = fabro_client::Client::new_no_proxy(&server.url("")).unwrap();
+ let services = FabroRunToolServices {
+ backend: Arc::new(ClientBackend::new(Arc::new(client))),
+ current_run_id: parent_id,
+ };
+ let name = fabro_tool::FABRO_RUN_CREATE_TOOL_NAME;
+ let mut args = json!({"runs":[{
+ "workflow_version_id":version_id,
+ "target":{"kind":"none"},
+ "args":{"auto_approve":false}
+ }]});
+ let error = execute_fabro_run_tool(name, args.clone(), &services)
+ .await
+ .unwrap_err();
+ assert!(error.to_string().contains("native admission rejection"));
+ args["runs"][0]["parent_id"] = json!(fabro_types::RunId::new());
+ let error = execute_fabro_run_tool(name, args, &services)
+ .await
+ .unwrap_err();
+ assert!(error.to_string().contains("match the current run"));
+ create.assert_calls_async(1).await;
+ state.assert_calls_async(0).await;
+ }
+
struct SingleGraphPackager;
#[async_trait]
@@ -257,13 +275,11 @@ mod tests {
.await;
let client = fabro_client::Client::new_no_proxy(&server.url("")).unwrap();
let services = FabroRunToolServices {
- backend: Arc::new(
+ backend: Arc::new(
ClientBackend::new(Arc::new(client))
.with_workflow_version_packager(Arc::new(SingleGraphPackager)),
),
- current_run_id: "01KRBZW4DW0000000000000002".parse().unwrap(),
- base_cwd: "unused".into(),
- user_settings_path: "unused".into(),
+ current_run_id: "01KRBZW4DW0000000000000002".parse().unwrap(),
};
let name = fabro_tool::FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME;
assert_eq!(register_named_fabro_run_tools(&services, &[name]).len(), 1);
diff --git a/lib/components/fabro-workflow/src/services.rs b/lib/components/fabro-workflow/src/services.rs
index d5f48a5c7..5f6511b43 100644
--- a/lib/components/fabro-workflow/src/services.rs
+++ b/lib/components/fabro-workflow/src/services.rs
@@ -77,10 +77,8 @@ impl RunLocations {
#[derive(Clone)]
pub struct FabroRunToolServices {
- pub backend: Arc,
- pub current_run_id: RunId,
- pub base_cwd: PathBuf,
- pub user_settings_path: PathBuf,
+ pub backend: Arc,
+ pub current_run_id: RunId,
}
/// Services shared across workflow phases.