mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-14 23:22:51 +00:00
Merge pull request #832 from fabro-sh/codex/run-intent-run-create
Migrate fabro_run_create to the RunIntent API
This commit is contained in:
Scott Werner
GitHub
commit
2c81e81e5a
27 changed files with 1515 additions and 1622 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -3176,6 +3176,7 @@ dependencies = [
|
|||
"fabro-workflow-version",
|
||||
"futures",
|
||||
"httpmock",
|
||||
"jsonschema",
|
||||
"schemars 1.2.1",
|
||||
"serde",
|
||||
"serde_json",
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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`.
|
||||
|
||||
<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>
|
||||
|
||||
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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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/<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.
|
||||
|
||||
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
|
||||
|
||||
|
|
|
|||
|
|
@ -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> {
|
||||
|
|
|
|||
|
|
@ -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<RunTarget> {
|
||||
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"
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<FabroRunToolServices> {
|
||||
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<RunManifest> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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::<Vec<_>>();
|
||||
|
||||
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
|
||||
}]
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -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<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 {
|
||||
|
|
@ -33,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()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<types::RunManifest> {
|
||||
run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path)
|
||||
}
|
||||
}
|
||||
|
|
@ -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<FabroMcpServerSettings>,
|
||||
backend: Arc<OnceCell<Arc<dyn FabroToolBackend>>>,
|
||||
cwd: PathBuf,
|
||||
tool_router: ToolRouter<Self>,
|
||||
}
|
||||
|
||||
|
|
@ -82,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(),
|
||||
}
|
||||
}
|
||||
|
|
@ -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<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, &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<dyn FabroToolBackend>
|
||||
})
|
||||
.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));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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)]
|
||||
|
|
|
|||
229
lib/apps/fabro-server/src/run_tool_create.rs
Normal file
229
lib/apps/fabro-server/src/run_tool_create.rs
Normal file
|
|
@ -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<RunTarget>) -> 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::<fabro_types::RunIntent>::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<RunId>, 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<RunId>,
|
||||
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 },
|
||||
}
|
||||
}
|
||||
|
|
@ -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<types::RunManifest> {
|
||||
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<types::ManifestArgs> {
|
||||
let mut input = spec
|
||||
.inputs
|
||||
.iter()
|
||||
.map(|(key, value)| format!("{key}={value}"))
|
||||
.collect::<Vec<_>>();
|
||||
input.sort();
|
||||
let mut label = spec
|
||||
.labels
|
||||
.iter()
|
||||
.map(|(key, value)| format!("{key}={value}"))
|
||||
.collect::<Vec<_>>();
|
||||
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<RunLayer> {
|
||||
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");
|
||||
}
|
||||
}
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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}");
|
||||
|
|
|
|||
|
|
@ -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::<RunId>().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::<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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<GitRunTarget>,
|
||||
/// 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<GitRunTargetObservation> {
|
||||
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<DerivedRunTarget, RunTargetDerivationError> {
|
||||
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<RunTarget, RunTargetDerivationError> {
|
||||
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<Option<String>> {
|
||||
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<String>,
|
||||
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<String>) -> Option<GitRunTarget> {
|
||||
fn github_run_target(origin_url: &str, branch: &str) -> Option<GitRunTarget> {
|
||||
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<String>) -> Opt
|
|||
|
||||
fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option<String> {
|
||||
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<String> {
|
||||
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<String> {
|
|||
(!normalized.is_empty()).then_some(normalized)
|
||||
}
|
||||
|
||||
fn configured_repo_origin_url_from_scm_layer(scm: &RunScmLayer) -> Option<String> {
|
||||
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<RunScmLayer> {
|
||||
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<String>,
|
||||
|
|
@ -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<String> {
|
||||
let local_sha = local_sha?;
|
||||
) -> (Option<String>, 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),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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<RunId>,
|
||||
) -> anyhow::Result<RunId>;
|
||||
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>;
|
||||
|
|
@ -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<types::RunManifest>;
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, JsonSchema)]
|
||||
pub struct RunSummaryResult {
|
||||
pub run_id: String,
|
||||
|
|
@ -201,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, 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::<crate::FabroRunSearchParams>(
|
||||
FABRO_RUN_SEARCH_TOOL_NAME,
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -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<Arc<dyn RunManifestBuilder>>,
|
||||
run_scope: Option<RunId>,
|
||||
workflow_version_packager: Option<Arc<dyn crate::WorkflowVersionPackager>>,
|
||||
}
|
||||
|
|
@ -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<dyn RunManifestBuilder>) -> 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<RunId>,
|
||||
) -> anyhow::Result<RunId> {
|
||||
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<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> {
|
||||
|
|
|
|||
|
|
@ -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,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<RunId>,
|
||||
_intent: fabro_types::RunIntent,
|
||||
) -> anyhow::Result<RunId> {
|
||||
unreachable!()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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(¶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<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);
|
||||
|
|
|
|||
|
|
@ -77,10 +77,8 @@ impl RunLocations {
|
|||
|
||||
#[derive(Clone)]
|
||||
pub struct FabroRunToolServices {
|
||||
pub backend: Arc<dyn fabro_tool::FabroToolBackend>,
|
||||
pub current_run_id: RunId,
|
||||
pub base_cwd: PathBuf,
|
||||
pub user_settings_path: PathBuf,
|
||||
pub backend: Arc<dyn fabro_tool::FabroToolBackend>,
|
||||
pub current_run_id: RunId,
|
||||
}
|
||||
|
||||
/// Services shared across workflow phases.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue