Simplify run creation to registered workflow versions

This commit is contained in:
Scott Werner 2026-09-11 17:10:41 -06:00
parent 1e8e1c9a30
commit 21a5e5b86f
28 changed files with 1042 additions and 3558 deletions

View file

@ -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.

View file

@ -86,84 +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. Selectors are compatible only when the MCP process and Fabro operation share the native filesystem:
```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
}]
}
```
For agent-authored content or callers without a shared filesystem, send an inline package by value. `entrypoint` and every `files` key are portable paths inside that package:
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.
```json
{
"runs": [
{
"workflow": {
"kind": "inline",
"entrypoint": "workflows/review/workflow.fabro",
"files": {
"workflows/review/workflow.fabro": "digraph Review { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}
},
"target": {
"kind": "git",
"repo": "acme/widgets",
"branch": "feature/checkout"
}
}
]
}
```
`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`.
You can also reuse an exact immutable workflow version without uploading content again:
<Note>
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.
</Note>
```json
{
"runs": [
{
"workflow": {
"kind": "stored",
"workflow_version_id": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
},
"target": { "kind": "none" },
"start": false
}
]
}
```
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted; for a Git parent that means its repository and current execution branch (normally `fabro/run/<parent-id>`). The original input branch and pinned commit/tag are not inherited. Push the parent's changes before creating the child. If run branches are disabled, the child follows the original input branch; if an enabled run branch is not available yet, provide an explicit target.
Folder targets are available only to standalone MCP and Local workflow agents with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
Standalone MCP calls using a stored workflow ID must supply an explicit `target`: the stored repository configuration is not available for implicit derivation. For selector and inline sources, when `target` is omitted, standalone MCP derives it from the selected environment the way `fabro run` does: a Local environment targets the working directory as a folder, and a Docker or Daytona environment targets the attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin (a directory with no Git metadata runs with no workspace); otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
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. Like selectors, `goal_file` requires a native/shared filesystem; Docker and Daytona agents must send `goal` text by value.
Creation and execution remain separate operations. `fabro_run_create` first creates a durable run, then requests one start by default. Set `start: false` to leave the new run submitted without requesting execution.
`fabro_run_create` transmits only the values in the request. The MCP server's `~/.fabro/settings.toml` `[run]` defaults, and any project or machine run settings, are not applied to the created run, matching `fabro run`. Keep workflow-owned behavior in `workflow.toml`, and pass explicit `auto_approve`, `dry_run`, `model`, `provider`, `environment`, or `preserve_sandbox` values in the spec when a run needs them.
Creation persists a submitted run. A separate start request follows by default;
set `start: false` to start later. Batches contain 150 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.

View file

@ -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,101 +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.
Selector strings and selector paths are available only when the parent agent runs in a genuinely shared Local filesystem context. Docker and Daytona agents must send the workflow bytes inline or reuse an exact stored workflow version; a path inside their sandbox is not a usable selector on the worker host.
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/<parent-id>`). 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.
Send agent-authored workflow content with a strict inline source:
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`.
```json
{
"runs": [
{
"workflow": {
"kind": "inline",
"entrypoint": "child/workflow.fabro",
"files": {
"child/workflow.fabro": "digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}
},
"goal": "Run the workflow content supplied by this parent."
}
]
}
```
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`.
Reuse content already registered with Fabro by supplying its exact immutable ID:
```json
{
"runs": [
{
"workflow": {
"kind": "stored",
"workflow_version_id": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
},
"target": { "kind": "none" },
"start": false
}
]
}
```
Workflow content and workspace target are separate choices. If `target` is omitted, a child inherits the parent's canonical target: a `none` or folder target as-is, and a Git target's repository and current execution branch (normally `fabro/run/<parent-id>`). Push the parent's changes before creating the child; the child checks out that branch's current remote HEAD. The original input branch and pinned commit/tag are not carried over. If run branches are disabled, the child follows the original input branch. If an enabled run branch is not available yet, provide an explicit target. Pass an explicit Git target with a `sha` to pin a child. An explicit Git, `none`, or folder target overrides that inheritance while the current run remains the forced parent. Folder targets require a Local parent with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
`goal_file` is also a shared-filesystem feature. Local agents can read it relative to the operation `cwd`; Docker and Daytona agents must send the resolved `goal` text by value.
Like `fabro run`, `fabro_run_create` transmits only what the request names. Run defaults from the caller's `~/.fabro/settings.toml` `[run]` table, and project or machine run settings, are not applied to the created run. Put workflow-owned behavior in the workflow's `workflow.toml`, and pass explicit `auto_approve`, `dry_run`, `model`, `provider`, `environment`, or `preserve_sandbox` values in the spec when a child needs them. See [Run Configuration](/execution/run-configuration) for the settings layering.
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.
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

View file

@ -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<fabro_mcp_server::McpInitSettings> {

View file

@ -77,7 +77,7 @@ pub(crate) async fn create_run(
target,
dirty_worktree,
} = fabro_manifest::derive_run_target_for_provider(
environment.settings.provider,
&environment.settings.provider,
&canonical_cwd,
configured_repo_origin_url.as_deref(),
)?;

View file

@ -6,7 +6,6 @@ use std::time::Duration;
use anyhow::{Context, Result, anyhow};
use async_trait::async_trait;
use fabro_client::ServerTarget;
use fabro_config::user::default_workflows_dir;
use fabro_config::{ServerSettingsBuilder, Storage};
use fabro_interview::{
AnswerSubmission, ControlInterviewer, WORKER_CONTROL_INVALID_CURSOR_REASON,
@ -15,13 +14,10 @@ use fabro_interview::{
WorkerControlMessage,
};
use fabro_manifest::SuppliedWorkflowVersionPackager;
use fabro_server::run_tool_create::ServerRunCreateAdapter;
use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer};
use fabro_tool::fabro_client::ClientBackend;
use fabro_types::settings::run::{EnvironmentProvider, RunMode, RunNamespace};
use fabro_types::{
ArtifactUpload, BlobHash, EventBody, FailureReason, Principal, RunEvent, RunId,
};
use fabro_types::settings::run::{RunMode, RunNamespace};
use fabro_types::{ArtifactUpload, BlobHash, EventBody, FailureReason, Principal, RunEvent, RunId};
use fabro_vault::{SecretStore, Vault};
use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader};
use fabro_workflow::event::{Emitter, RunEventSink};
@ -99,14 +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.settings.run.environment.provider,
run_spec.source_directory.as_deref(),
&run_dir,
)
build_fabro_run_tool_services(worker_token, client.clone_for_reuse(), run_id)
} else {
None
};
@ -232,24 +221,15 @@ fn build_fabro_run_tool_services(
worker_token: &str,
client: fabro_client::Client,
current_run_id: RunId,
provider: EnvironmentProvider,
source_directory: Option<&str>,
run_dir: &Path,
) -> Option<FabroRunToolServices> {
if worker_token.trim().is_empty() {
return None;
}
let backend = ClientBackend::new(Arc::new(client))
.with_run_create_adapter(Arc::new(ServerRunCreateAdapter::worker(
provider,
current_run_id,
Some(default_workflows_dir()),
)))
.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),
})
}
@ -1189,7 +1169,6 @@ fn install_signal_handlers(
reason = "This test module prefers explicit type paths over extra imports."
)]
mod tests {
use std::path::Path;
use std::sync::Arc;
use std::time::Duration;
@ -1199,14 +1178,13 @@ mod tests {
use fabro_interview::{
AnswerValue, ControlInterviewer, Interviewer, Question, WorkerControlEnvelope,
};
use fabro_server::run_tool_create::ServerRunCreateAdapter;
use fabro_types::run_event::{
InterviewCompletedProps, InterviewStartedProps, RunCompletedProps, RunControlEffectProps,
RunFailedProps, RunStatusTransitionProps,
};
use fabro_types::{
AuthMethod, EventBody, FailureCategory, FailureDetail, FailureReason, IdpIdentity,
Principal, QuestionType, RunFailure, RunTarget, SuccessReason, WorkflowVersionId, fixtures,
Principal, QuestionType, RunFailure, SuccessReason, fixtures,
};
use fabro_vault::{SecretType, Vault};
use fabro_workflow::event::RunEventSink;
@ -1264,45 +1242,6 @@ mod tests {
));
}
#[tokio::test]
async fn fabro_run_create_worker_adapter_preserves_explicit_target_without_host_reads() {
use fabro_tool::{FabroRunCreateParams, RunCreateAdapter, ValidatedCreateRuns};
use fabro_types::settings::run::EnvironmentProvider;
let temp = tempfile::tempdir().unwrap();
let workflow_version_id: WorkflowVersionId =
fabro_types::BlobHash::new(b"stored workflow").into();
let inherited = RunTarget::None {};
let params: FabroRunCreateParams = serde_json::from_value(serde_json::json!({
"runs": [{
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
},
"target": inherited
}]
}))
.unwrap();
let spec = ValidatedCreateRuns::try_from(params)
.unwrap()
.runs
.remove(0);
let adapter = ServerRunCreateAdapter::worker(
EnvironmentProvider::Docker,
fabro_types::RunId::new(),
Some(temp.path().join("workflows")),
);
let client = fabro_client::Client::new_no_proxy("http://127.0.0.1:9").unwrap();
let prepared = adapter
.prepare(&client, &spec, Path::new("/host/that-must-not-be-read"))
.await
.unwrap();
assert_eq!(prepared.workflow_version_id, workflow_version_id);
assert_eq!(prepared.target, inherited);
}
fn worker_token_with_claims(claims: &serde_json::Value) -> String {
jsonwebtoken::encode(
&jsonwebtoken::Header::new(jsonwebtoken::Algorithm::HS256),

View file

@ -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,16 +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,
"workflow_version_id": workflow_version_id,
"target": { "kind": "none" },
"dry_run": true,
"auto_approve": true,
"labels": { "source": "mcp-test" }
"args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-test" }}
}]
}),
)
@ -805,16 +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,
"workflow_version_id": workflow_version_id,
"target": { "kind": "none" },
"dry_run": true,
"auto_approve": true,
"labels": { "source": "mcp-default-server-test" },
"args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-default-server-test" }},
"start": false
}]
}),
@ -1876,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::<Vec<_>>();
let empty = call_tool_error_text(
@ -1896,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!({
@ -1919,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;
@ -1930,93 +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");
context.git_init();
run_git(&context.temp_dir, &["config", "user.name", "Fabro Test"]);
run_git(&context.temp_dir, &[
"config",
"user.email",
"fabro@example.com",
]);
run_git(&context.temp_dir, &["add", "simple.fabro"]);
run_git(&context.temp_dir, &["commit", "--quiet", "-m", "fixture"]);
run_git(&context.temp_dir, &[
"remote",
"add",
"origin",
"https://github.com/fabro-sh/fabro.git",
]);
let test_origin = context.temp_dir.join("origin.git");
run_git(&context.temp_dir, &[
"init",
"--bare",
"--quiet",
test_origin
.to_str()
.expect("test origin path should be UTF-8"),
]);
let push_url = format!("file://{}", test_origin.display());
run_git(&context.temp_dir, &[
"remote", "set-url", "--push", "origin", &push_url,
]);
let workflow_version_id =
fabro_manifest::collect_workflow_versions(&workflow, &context.temp_dir)
.expect("workflow fixture should package")
.root_id();
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!(
harness
.api_requests
.contains("GET /api/v1/environments/default"),
"the shorthand request should reach environment lookup authentication"
);
assert!(
!harness
.api_requests
.contains("POST /api/v1/workflow-versions"),
"an unauthenticated shorthand request must not attempt registration"
);
assert!(
!harness.workflow_version_exists(workflow_version_id).await,
"an unauthenticated shorthand request must not register a workflow version"
);
assert!(
!harness.api_requests.contains("POST /api/v1/runs"),
"an unauthenticated shorthand request must not reach run creation"
);
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")]
@ -2824,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,
@ -2842,50 +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,
"workflow_version_id": workflow_version_id,
"target": { "kind": "none" },
"dry_run": true,
"auto_approve": true,
"labels": { "source": "mcp-test" },
"args": {"dry_run": true, "auto_approve": true, "labels": { "source": "mcp-test" }},
"start": start
}]
}),
@ -2897,19 +2853,6 @@ async fn create_mcp_run(client: &McpClient, workflow: PathBuf, start: bool) -> S
.to_string()
}
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 seed_oauth_auth(
home_dir: &Path,
target: &fabro_client::ServerTarget,

View file

@ -25,9 +25,7 @@ use fabro_server::jwt_auth::resolve_auth_mode_with_lookup;
use fabro_server::server::{RouterOptions, build_router_with_options};
use fabro_server::test_support::TestAppStateBuilder;
use fabro_static::EnvVars;
use fabro_store::Database;
use fabro_test::{GitHubAppState, TestContext, apply_test_isolation};
use fabro_types::WorkflowVersionId;
use serde_json::Value;
use tokio::net::TcpListener;
use tokio::sync::oneshot;
@ -44,7 +42,6 @@ pub(crate) const TEST_DEV_TOKEN: &str =
pub(crate) struct RealAuthHarness {
pub(crate) api_base_url: String,
api_server: RunningHttpServer,
store: Arc<Database>,
twin: fabro_test::TwinGitHub,
pub(crate) api_requests: ListenerRequestLog,
}
@ -87,13 +84,11 @@ impl RealAuthHarness {
if let Some(token) = dev_token.clone() {
secrets.insert("FABRO_DEV_TOKEN".to_string(), token);
}
let (store, artifact_store) = fabro_server::test_support::test_store_bundle();
let state = TestAppStateBuilder::new()
.runtime_settings(settings, RunLayer::default())
.max_concurrent_runs(5)
.env_lookup(|_| None)
.server_secret_env(secrets)
.store_bundle(Arc::clone(&store), artifact_store)
.vault_entries([
("GITHUB_APP_CLIENT_SECRET", github_client_secret.as_str()),
(EnvVars::OPENAI_API_KEY, "test-openai-api-key"),
@ -118,7 +113,6 @@ impl RealAuthHarness {
Self {
api_base_url,
api_server,
store,
twin,
api_requests,
}
@ -128,15 +122,6 @@ impl RealAuthHarness {
format!("{}/api/v1", self.api_base_url)
}
pub(crate) async fn workflow_version_exists(&self, id: WorkflowVersionId) -> bool {
let blob_hash = id.into();
self.store
.blobs()
.exists(&blob_hash)
.await
.expect("workflow-version blob lookup should succeed")
}
pub(crate) async fn shutdown(self) {
self.api_server.shutdown().await;
self.twin.shutdown().await;

View file

@ -23,8 +23,6 @@ pub type FabroClientFactory = Arc<dyn Fn() -> 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 {
@ -32,8 +30,6 @@ impl std::fmt::Debug for FabroMcpServerSettings {
formatter
.debug_struct("FabroMcpServerSettings")
.field("client_factory", &"<factory>")
.field("config_path", &self.config_path)
.field("cwd", &self.cwd)
.finish()
}
}

View file

@ -1,11 +1,8 @@
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use anyhow::Result;
use fabro_config::user;
use fabro_manifest::SuppliedWorkflowVersionPackager;
use fabro_server::run_tool_create::ServerRunCreateAdapter;
use fabro_tool::fabro_client::ClientBackend;
use fabro_tool::{self as run_tools, FabroToolBackend};
use fabro_util::version::FABRO_VERSION;
@ -26,7 +23,6 @@ use crate::{FabroMcpServerSettings, SERVER_NAME};
pub(crate) struct FabroMcpServer {
settings: Arc<FabroMcpServerSettings>,
backend: Arc<OnceCell<Arc<dyn FabroToolBackend>>>,
cwd: PathBuf,
tool_router: ToolRouter<Self>,
}
@ -83,11 +79,9 @@ impl ServerHandler for FabroMcpServer {
#[tool_router(router = tool_router)]
impl FabroMcpServer {
pub(crate) fn new(settings: Arc<FabroMcpServerSettings>) -> Self {
let cwd = settings.cwd.clone();
Self {
settings,
backend: Arc::new(OnceCell::new()),
cwd,
tool_router: Self::tool_router(),
}
}
@ -116,21 +110,21 @@ impl FabroMcpServer {
#[tool(
name = "fabro_run_create",
description = "Create one or more Fabro workflow runs from a native selector, inline files, or an exact stored workflow version, optionally under a parent run and 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<run_tools::FabroRunCreateParams>,
) -> Result<CallToolResult, ErrorData> {
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, 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)),
}
@ -275,15 +269,9 @@ impl FabroMcpServer {
.await
.map(|client| {
Arc::new(
ClientBackend::new(Arc::new(client))
.with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::standalone(Some(
user::default_workflows_dir(),
)),
))
.with_workflow_version_packager(Arc::new(
SuppliedWorkflowVersionPackager,
)),
ClientBackend::new(Arc::new(client)).with_workflow_version_packager(
Arc::new(SuppliedWorkflowVersionPackager),
),
) as Arc<dyn FabroToolBackend>
})
.map_err(|err| run_tools::ToolError::from_anyhow(&err))
@ -315,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;
@ -344,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) })
@ -383,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") })
}),
@ -399,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") })
}),
@ -426,10 +407,8 @@ mod tests {
}
#[test]
fn fabro_run_create_tool_advertises_complete_workflow_source_and_target_grammar() {
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") })
}),
@ -441,59 +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_eq!(object_variant["additionalProperties"], false);
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}"
);
let workflow_variants = object_variant
.pointer("/properties/workflow/anyOf")
.and_then(Value::as_array)
.expect("workflow should advertise selector, inline, and stored sources");
assert!(
workflow_variants
.iter()
.any(|variant| variant["type"] == "string")
);
for kind in ["inline", "stored"] {
assert!(workflow_variants.iter().any(|variant| {
variant.pointer("/properties/kind/const") == Some(&Value::from(kind))
}));
}
let target_variants = object_variant
.pointer("/properties/target/anyOf")
.and_then(Value::as_array)
.expect("target should advertise the canonical target union");
for kind in ["git", "none", "folder"] {
assert!(target_variants.iter().any(|variant| {
variant.pointer("/properties/kind/const") == Some(&Value::from(kind))
}));
}
.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));
}
}

View file

@ -41,7 +41,8 @@ mod run_intent;
mod run_manifest;
mod run_selector;
mod run_title_generation;
pub mod run_tool_create;
#[cfg(test)]
mod run_tool_create;
pub mod security_headers;
pub mod serve;
pub mod server;

File diff suppressed because it is too large Load diff

View file

@ -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};
@ -751,7 +750,6 @@ async fn build_agent(
let services = FabroRunToolServices {
backend: Arc::new(backend),
current_run_id: run_id,
base_cwd: PathBuf::new(),
};
let run_tools = register_named_fabro_run_tools(&services, ASK_FABRO_RUN_TOOL_NAMES);
let selector = format!("{provider_id}/{model}");

View file

@ -120,7 +120,6 @@ mod tests {
};
use crate::server;
use crate::test_support::{self, TestAppStateBuilder};
use crate::worker_token::{WorkerScopeSet, issue_worker_token, issue_worker_token_with_scopes};
const GRAPH: &str = "digraph W { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }";
@ -162,79 +161,6 @@ mod tests {
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
}
#[tokio::test]
async fn workflow_version_registration_authorization_is_narrow() {
let state = TestAppStateBuilder::new().build();
let app = server::build_router(Arc::clone(&state), test_support::test_auth_mode());
let run_id = fabro_types::RunId::new();
let scoped = issue_worker_token_with_scopes(
state.worker_token_keys(),
&run_id,
WorkerScopeSet::run_worker_with_agent_run_tools(),
)
.unwrap();
let ordinary = issue_worker_token(state.worker_token_keys(), &run_id).unwrap();
let authorized = app
.clone()
.oneshot(
Request::builder()
.method(Method::POST)
.uri("/api/v1/workflow-versions")
.header(header::CONTENT_TYPE, "application/json")
.header(header::AUTHORIZATION, format!("Bearer {scoped}"))
.body(Body::from(serde_json::to_vec(&version(GRAPH)).unwrap()))
.unwrap(),
)
.await
.unwrap();
fabro_test::expect_axum_status(
authorized,
StatusCode::CREATED,
"scoped worker POST /api/v1/workflow-versions",
)
.await;
let forbidden = app
.clone()
.oneshot(
Request::builder()
.method(Method::POST)
.uri("/api/v1/workflow-versions")
.header(header::CONTENT_TYPE, "application/json")
.header(header::AUTHORIZATION, format!("Bearer {ordinary}"))
.body(Body::from(serde_json::to_vec(&version(GRAPH)).unwrap()))
.unwrap(),
)
.await
.unwrap();
fabro_test::expect_axum_status(
forbidden,
StatusCode::FORBIDDEN,
"ordinary worker POST /api/v1/workflow-versions",
)
.await;
let malformed = app
.oneshot(
Request::builder()
.method(Method::POST)
.uri("/api/v1/workflow-versions")
.header(header::CONTENT_TYPE, "application/json")
.header(header::AUTHORIZATION, "Bearer not-a-worker-token")
.body(Body::from(serde_json::to_vec(&version(GRAPH)).unwrap()))
.unwrap(),
)
.await
.unwrap();
fabro_test::expect_axum_status(
malformed,
StatusCode::UNAUTHORIZED,
"malformed worker POST /api/v1/workflow-versions",
)
.await;
}
#[tokio::test]
async fn valid_and_equivalent_requests_return_the_same_id() {
let state = TestAppStateBuilder::new().build();

View file

@ -19927,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::<RunId>().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());
}

View file

@ -31,12 +31,10 @@ use fabro_graphviz::parser;
use fabro_template::validate_static_reference;
use fabro_types::graph::ReferenceKind;
use fabro_types::settings::interp::InterpString;
use fabro_types::settings::run::{
ApprovalMode, EnvironmentProvider, ResolvedGoalSource, ResolvedRunGoal, RunMode,
};
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;
@ -49,7 +47,6 @@ use crate::workflow_bundler::WorkflowBundler;
pub use crate::workflow_version_collector::{
MAX_WORKFLOW_VERSION_DEPTH, WorkflowVersionCollectError, collect_workflow_versions,
collect_workflow_versions_at_location,
collect_inline_workflow_versions,
};
pub use crate::workflow_version_packager::SuppliedWorkflowVersionPackager;
@ -418,7 +415,7 @@ pub enum RunTargetDerivationError {
)]
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 or pass an explicit target"
"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())]
@ -462,13 +459,13 @@ pub enum RunTargetDerivationError {
/// # Errors
///
/// Returns the reason a clone-based target could not be derived; every
/// variant's message is written for the caller of the run tool or CLI.
/// variant's message is written for the CLI caller.
pub fn derive_run_target_for_provider(
provider: EnvironmentProvider,
provider: &SandboxProviderKind,
canonical_cwd: &Path,
configured_repo_origin_url: Option<&str>,
) -> std::result::Result<DerivedRunTarget, RunTargetDerivationError> {
if !provider.is_clone_based() {
if !provider.clones_workspace() {
let path = canonical_cwd
.to_str()
.ok_or_else(|| RunTargetDerivationError::NonUtf8Path {
@ -542,21 +539,6 @@ fn none_target_for_unversioned_directory(
outcome
}
/// The workflow's configured `run.scm` GitHub repository as a normalized
/// origin URL, read from `workflow.toml` source text. `None` when the config
/// names no GitHub repository.
///
/// # Errors
///
/// Returns an error when the source cannot be parsed or resolved.
pub fn configured_repo_origin_url_from_workflow_toml(source: &str) -> Result<Option<String>> {
let run = parse_run_layer_from_settings_toml(source)
.context("failed to parse run settings from workflow.toml")?;
Ok(configured_repo_origin_url_from_scm_layer(
&run.scm.unwrap_or_default(),
))
}
/// 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.
@ -885,28 +867,6 @@ pub(crate) mod test_fixtures {
#[cfg(test)]
mod tests {
#[test]
fn configured_repo_origin_url_reads_run_scm_from_workflow_toml() {
let configured = super::configured_repo_origin_url_from_workflow_toml(
"_version = 1\n[run.scm]\nowner = \"acme\"\nrepository = \"widgets\"\n",
)
.unwrap();
assert_eq!(
configured.as_deref(),
Some("https://github.com/acme/widgets")
);
let unconfigured =
super::configured_repo_origin_url_from_workflow_toml("_version = 1\n").unwrap();
assert_eq!(unconfigured, None);
let other_provider = super::configured_repo_origin_url_from_workflow_toml(
"_version = 1\n[run.scm]\nprovider = \"gitlab\"\nowner = \"acme\"\nrepository = \"widgets\"\n",
)
.unwrap();
assert_eq!(other_provider, None);
}
use fabro_workflow::git::head_sha;
use super::*;

View file

@ -66,11 +66,6 @@ impl ResolvedLocalWorkflowPackage {
pub fn closure(&self) -> &CollectedWorkflowClosure {
&self.closure
}
#[must_use]
pub fn into_closure(self) -> CollectedWorkflowClosure {
self.closure
}
}
/// Resolve producer-readable workflow bytes under one stable local source

View file

@ -186,79 +186,6 @@ pub fn collect_workflow_versions_at_location(
VersionAssembler::new(collected).assemble()
}
/// Package a workflow whose bytes arrive by value. `entrypoint` is an exact
/// key of `files`; unlike a checkout selector, an extensionless entrypoint
/// is never rewritten to a `.fabro/workflows/<name>/workflow.toml` lookup.
/// The files are staged in a private temporary root only for the duration of
/// collection, so the resulting closure's paths are rooted at the file map.
///
/// # Errors
///
/// Returns a collision error before touching the filesystem when two paths
/// cannot coexist on one filesystem; staging and collection failures are
/// reported against the entrypoint.
pub fn collect_inline_workflow_versions(
entrypoint: &WorkflowPath,
files: &BTreeMap<WorkflowPath, String>,
) -> Result<CollectedWorkflowClosure, WorkflowVersionCollectError> {
if !files.contains_key(entrypoint) {
return Err(WorkflowVersionCollectError::MissingWorkflow {
path: entrypoint.to_string(),
});
}
fabro_types::validate_workflow_source_paths(files.keys()).map_err(|error| match error {
WorkflowVersionShapeError::PathCollision { second, .. } => {
WorkflowVersionCollectError::PathCollision {
entrypoint: entrypoint.clone(),
path: second,
}
}
source => WorkflowVersionCollectError::InvalidShape {
entrypoint: entrypoint.clone(),
source,
},
})?;
let collect_error = |source: anyhow::Error| WorkflowVersionCollectError::Collect {
path: PathBuf::from(entrypoint.as_str()),
source,
};
let root = tempfile::tempdir().map_err(|source| {
collect_error(
anyhow::Error::new(source).context("failed to create private inline workflow root"),
)
})?;
for (path, content) in files {
let destination = root.path().join(path.as_str());
if let Some(parent) = destination.parent() {
std::fs::create_dir_all(parent).map_err(|source| {
collect_error(anyhow::Error::new(source).context(format!(
"failed to create inline workflow directory for `{path}`"
)))
})?;
}
std::fs::write(&destination, content).map_err(|source| {
collect_error(
anyhow::Error::new(source)
.context(format!("failed to write inline workflow file `{path}`")),
)
})?;
}
let package_root = root.path().canonicalize().map_err(|source| {
collect_error(
anyhow::Error::new(source).context("failed to canonicalize the inline workflow root"),
)
})?;
let location = WorkflowLocation::from_exact_path(Path::new(entrypoint.as_str()), &package_root)
.map_err(|source| collect_error(source.into()))?;
let location = canonicalize_location(location, |path, source| {
collect_error(anyhow::Error::new(source).context(format!(
"failed to canonicalize inline workflow path {}",
path.display()
)))
})?;
collect_workflow_versions_at_location(&location, &package_root, Path::new(entrypoint.as_str()))
}
fn repository_workflow_path(workflow: &Path) -> PathBuf {
if workflow.is_relative() && workflow.extension().is_none() {
Path::new(".fabro/workflows")
@ -498,43 +425,6 @@ dockerfile = { path = "Dockerfile" }
);
}
#[test]
fn inline_collection_uses_the_exact_entrypoint_and_rejects_collisions_before_staging() {
let files = BTreeMap::from([
(
WorkflowPath::new("review").unwrap(),
"digraph Review {}".to_string(),
),
(
WorkflowPath::new("notes/detail.md").unwrap(),
"detail".to_string(),
),
]);
let closure =
collect_inline_workflow_versions(&WorkflowPath::new("review").unwrap(), &files)
.unwrap();
let (_, root) = closure.versions().next().unwrap();
assert_eq!(root.version().entrypoint().as_str(), "review");
for (file, descendant) in [
("a", "a/b.md"),
("A", "a/b.md"),
("dir/File", "DIR/file/child.md"),
("Prompt.md", "prompt.md"),
] {
let entrypoint = WorkflowPath::new(file).unwrap();
let colliding = BTreeMap::from([
(entrypoint.clone(), "digraph A {}".to_string()),
(WorkflowPath::new(descendant).unwrap(), "b".to_string()),
]);
let error = collect_inline_workflow_versions(&entrypoint, &colliding).unwrap_err();
assert!(
matches!(error, WorkflowVersionCollectError::PathCollision { .. }),
"unexpected error: {error:#}"
);
}
}
#[test]
fn packages_named_workflow_without_project_config() {
let temp = tempfile::tempdir().unwrap();

View file

@ -1,5 +1,4 @@
use std::collections::HashMap;
use std::path::Path;
use std::sync::LazyLock;
use async_trait::async_trait;
@ -7,7 +6,7 @@ use chrono::{DateTime, NaiveDate, Utc};
use fabro_api::types;
use fabro_types::{
PairId, PairMessageRecord, PairMessageRequest, PairRecord, PairTranscriptResponse, Run, RunId,
RunPairStatusResponse, RunTarget, StageId, WorkflowVersionId,
RunPairStatusResponse, StageId,
};
use fabro_util::exit::{self, ExitClass};
use schemars::JsonSchema;
@ -46,37 +45,6 @@ impl std::error::Error for ToolError {}
pub type ToolResult<T> = Result<T, ToolError>;
#[derive(Debug)]
pub struct PreparedRunCreate {
pub workflow_version_id: WorkflowVersionId,
pub target: RunTarget,
pub goal: Option<String>,
pub warnings: Vec<String>,
}
#[derive(Debug)]
pub struct CreateRunSubmission {
pub run_id: RunId,
pub warnings: Vec<String>,
}
/// Trusted producer-local preparation for one tool-created run.
///
/// Implementations acquire permitted workflow bytes and goal files, enforce
/// producer capabilities and package structure, resolve the independent
/// target, and register immutable workflow versions before returning
/// intent-ready fields. The server remains authoritative for semantic run
/// admission.
#[async_trait]
pub trait RunCreateAdapter: Send + Sync {
async fn prepare(
&self,
client: &fabro_client::Client,
spec: &crate::ValidatedCreateRunSpec,
cwd: &Path,
) -> anyhow::Result<PreparedRunCreate>;
}
#[async_trait]
pub trait FabroToolBackend: Send + Sync {
async fn create_workflow_version(
@ -86,12 +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,
parent_id: Option<RunId>,
) -> anyhow::Result<CreateRunSubmission>;
async fn create_run_from_intent(&self, intent: fabro_types::RunIntent)
-> anyhow::Result<RunId>;
async fn resolve_run(&self, selector: &str) -> anyhow::Result<Run>;
async fn retrieve_run(&self, run_id: &RunId) -> anyhow::Result<Run>;
@ -222,7 +186,7 @@ static TOOL_DEFINITIONS: LazyLock<Vec<ToolDefinition>> = LazyLock::new(|| {
),
tool_definition::<crate::FabroRunCreateParams>(
FABRO_RUN_CREATE_TOOL_NAME,
"Create one or more Fabro workflow runs from a native selector, inline files, or an exact stored workflow version, optionally under a parent run and 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::<crate::FabroRunSearchParams>(
FABRO_RUN_SEARCH_TOOL_NAME,
@ -350,7 +314,6 @@ mod tests {
use fabro_types::{
RunLifecycle, RunLinks, RunOrigin, RunStatus, RunTimestamps, WorkflowRef, test_support,
};
use serde_json::Value;
use super::*;
@ -390,44 +353,6 @@ mod tests {
);
}
#[test]
fn create_tool_definition_advertises_complete_workflow_source_and_target_grammar() {
let definition = tool_definitions()
.iter()
.find(|definition| definition.name == FABRO_RUN_CREATE_TOOL_NAME)
.expect("create tool should be in the shared catalog");
let variants = definition
.parameters
.pointer("/properties/runs/items/anyOf")
.and_then(Value::as_array)
.expect("run item union should be present");
assert!(variants.iter().any(|variant| variant["type"] == "string"));
let object = variants
.iter()
.find(|variant| variant["type"] == "object")
.expect("object create specification should be present");
assert_eq!(object["additionalProperties"], false);
let workflow = object
.pointer("/properties/workflow/anyOf")
.and_then(Value::as_array)
.expect("workflow source union should be present");
assert!(workflow.iter().any(|variant| variant["type"] == "string"));
for kind in ["inline", "stored"] {
assert!(workflow.iter().any(|variant| {
variant.pointer("/properties/kind/const") == Some(&Value::from(kind))
}));
}
let target = object
.pointer("/properties/target/anyOf")
.and_then(Value::as_array)
.expect("target union should be present");
for kind in ["git", "none", "folder"] {
assert!(target.iter().any(|variant| {
variant.pointer("/properties/kind/const") == Some(&Value::from(kind))
}));
}
}
#[test]
fn pair_tool_definition_exposes_pair_schema() {
let definition = tool_definitions()

File diff suppressed because it is too large Load diff

View file

@ -1,23 +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, RunIntent, RunIntentArgs, RunPairStatusResponse,
RunProjection, StageId,
PairTranscriptResponse, Run, RunId, RunIntent, RunPairStatusResponse, RunProjection, StageId,
};
use crate::{
CreateRunSubmission, FabroToolBackend, PreparedRunCreate, RunCreateAdapter, ToolError,
ValidatedCreateRunSpec, common,
};
use crate::{FabroToolBackend, common};
#[derive(Clone)]
pub struct ClientBackend {
client: Arc<::fabro_client::Client>,
run_create_adapter: Option<Arc<dyn RunCreateAdapter>>,
run_scope: Option<RunId>,
workflow_version_packager: Option<Arc<dyn crate::WorkflowVersionPackager>>,
}
@ -27,18 +21,11 @@ impl ClientBackend {
pub fn new(client: Arc<::fabro_client::Client>) -> Self {
Self {
client,
run_create_adapter: None,
run_scope: None,
workflow_version_packager: None,
}
}
#[must_use]
pub fn with_run_create_adapter(mut self, adapter: Arc<dyn RunCreateAdapter>) -> Self {
self.run_create_adapter = Some(adapter);
self
}
#[must_use]
pub fn with_workflow_version_packager(
mut self,
@ -68,41 +55,6 @@ impl ClientBackend {
}
}
fn run_intent_from_spec(
spec: &ValidatedCreateRunSpec,
prepared: PreparedRunCreate,
parent_id: Option<RunId>,
) -> (RunIntent, Vec<String>) {
let PreparedRunCreate {
workflow_version_id,
target,
goal,
warnings,
} = prepared;
let intent = RunIntent {
workflow_version_id,
target,
args: RunIntentArgs {
model: spec.model.clone(),
provider: spec.provider.clone(),
inputs: spec
.inputs
.iter()
.map(|(key, value)| (key.clone(), value.json().clone()))
.collect(),
labels: spec.labels.clone(),
dry_run: spec.dry_run,
auto_approve: spec.auto_approve,
preserve_sandbox: spec.preserve_sandbox,
},
environment_id: spec.environment.clone(),
parent_id,
title: None,
goal,
};
(intent, warnings)
}
#[async_trait]
impl FabroToolBackend for ClientBackend {
/// Package the supplied tree, then register dependencies before parents.
@ -129,26 +81,12 @@ impl FabroToolBackend for ClientBackend {
Ok(packaged.root_id())
}
async fn create_run_from_spec(
&self,
spec: &crate::ValidatedCreateRunSpec,
cwd: &Path,
parent_id: Option<RunId>,
) -> anyhow::Result<CreateRunSubmission> {
if let Some(parent_id) = parent_id.as_ref() {
self.ensure_run_scope(parent_id)?;
}
let Some(adapter) = self.run_create_adapter.as_ref() else {
return Err(ToolError::message(format!(
"{} is not available",
crate::FABRO_RUN_CREATE_TOOL_NAME
))
.into());
};
let prepared = adapter.prepare(&self.client, spec, cwd).await?;
let (intent, warnings) = run_intent_from_spec(spec, prepared, parent_id);
let run_id = self.client.create_run_from_intent(intent).await?;
Ok(CreateRunSubmission { run_id, warnings })
async fn create_run_from_intent(&self, intent: RunIntent) -> anyhow::Result<RunId> {
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<Run> {

View file

@ -446,7 +446,6 @@ fn normalize_optional_text(value: Option<&str>) -> Option<String> {
#[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,10 @@ 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,
_parent_id: Option<RunId>,
) -> anyhow::Result<crate::CreateRunSubmission> {
_intent: fabro_types::RunIntent,
) -> anyhow::Result<RunId> {
unreachable!()
}

View file

@ -17,17 +17,14 @@ mod search;
mod workflow_version;
pub use common::{
CreateRunSubmission, 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, PreparedRunCreate, RunCreateAdapter, RunSummaryResult, ToolDefinition,
ToolError, ToolResult, tool_definitions,
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,
RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions,
};
pub use create::{
CreateRunOptions, CreateRunSpec, CreateRunSpecInput, CreateRunWorkflowSource, CreateRunsResult,
CreatedRunResult, FabroRunCreateParams, InlineWorkflowSource, RunInputValue,
ValidatedCreateRunSpec, ValidatedCreateRuns, ValidatedRunInputValue, 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,

View file

@ -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::<fabro_tool::FabroRunCreateParams>(name, args)?;
ensure_current_run_parent(&params, 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 &params.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<T>(summary: &str, result: &T) -> fabro_tool::ToolResult<String>
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);

View file

@ -79,7 +79,6 @@ impl RunLocations {
pub struct FabroRunToolServices {
pub backend: Arc<dyn fabro_tool::FabroToolBackend>,
pub current_run_id: RunId,
pub base_cwd: PathBuf,
}
/// Services shared across workflow phases.

View file

@ -23,10 +23,6 @@ pub fn default_storage_dir() -> PathBuf {
Home::from_env().root().join("storage")
}
pub fn default_workflows_dir() -> PathBuf {
Home::from_env().workflows_dir()
}
pub fn default_socket_path() -> PathBuf {
Home::from_env().root().join("fabro.sock")
}
@ -81,8 +77,8 @@ mod tests {
use temp_env::with_var;
use super::{
SETTINGS_CONFIG_FILENAME, active_settings_path, active_settings_path_with_lookup,
default_settings_path, default_socket_path, default_storage_dir, default_workflows_dir,
SETTINGS_CONFIG_FILENAME, active_settings_path_with_lookup, default_settings_path,
default_socket_path, default_storage_dir,
};
#[test]
@ -95,29 +91,10 @@ mod tests {
home.join(".fabro").join(SETTINGS_CONFIG_FILENAME)
);
assert_eq!(default_storage_dir(), home.join(".fabro/storage"));
assert_eq!(default_workflows_dir(), home.join(".fabro/workflows"));
assert_eq!(default_socket_path(), home.join(".fabro/fabro.sock"));
});
}
#[test]
fn workflows_path_uses_fabro_home_when_config_is_elsewhere() {
let dir = tempfile::tempdir().unwrap();
let fabro_home = dir.path().join("fabro-home");
let custom_config = dir.path().join("config/settings.toml");
with_var(EnvVars::FABRO_HOME, Some(fabro_home.as_os_str()), || {
with_var(
EnvVars::FABRO_CONFIG,
Some(custom_config.as_os_str()),
|| {
assert_eq!(active_settings_path(None), custom_config);
assert_eq!(default_workflows_dir(), fabro_home.join("workflows"));
},
);
});
}
#[test]
fn active_settings_path_honors_fabro_config_env() {
let dir = tempfile::tempdir().unwrap();

View file

@ -198,6 +198,6 @@ pub use workflow_path::{
pub use workflow_version::{
MAX_WORKFLOW_VERSION_BYTES, MAX_WORKFLOW_VERSION_DEPENDENCIES, MAX_WORKFLOW_VERSION_FILE_BYTES,
MAX_WORKFLOW_VERSION_FILES, WorkflowVersion, WorkflowVersionShapeError,
validate_workflow_files, validate_workflow_source_paths, validate_workflow_path_collisions,
validate_workflow_files, validate_workflow_source_paths,
};
pub use workflow_version_id::{WorkflowVersionId, WorkflowVersionIdParseError};

View file

@ -194,13 +194,6 @@ pub fn validate_workflow_source_paths<'a>(
})
}
/// Reject exact duplicate paths and file/directory ancestor collisions.
pub fn validate_workflow_path_collisions<'a>(
paths: impl IntoIterator<Item = &'a WorkflowPath>,
) -> Result<(), WorkflowVersionShapeError> {
validate_path_collisions(paths, Cow::Borrowed)
}
/// Detect colliding paths under a comparison key: identical keys, or a key
/// that names an ancestor directory of another.
fn validate_path_collisions<'a>(