scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-15 23:32:46 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Align run-tool creation with server admission

Browse source
This commit is contained in:
Scott Werner 2026-09-01 17:26:18 -04:00
parent b38279fc44
commit c2d6dc4a92
6 changed files with 177 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -153,7 +153,7 @@ You can also reuse an exact immutable workflow version without uploading content
}
```
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted; standalone MCP instead derives an attached GitHub checkout when it can do so truthfully and otherwise requires an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted. Standalone MCP derives an attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin; otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Use `goal` for inline goal text or `goal_file` to read the run goal from a file. They are mutually exclusive. Relative `goal_file` paths resolve from the run's `cwd`, or from the MCP server working directory when `cwd` is omitted. Like selectors, `goal_file` requires a native/shared filesystem; Docker and Daytona agents must send `goal` text by value.

35
lib/apps/fabro-cli/tests/it/cmd/mcp.rs
View file

@ -1951,14 +1951,23 @@ async fn mcp_create_string_shorthand_deserializes_before_auth() {
"origin",
"https://github.com/fabro-sh/fabro.git",
]);
let missing_push = format!("file://{}/missing.git", context.temp_dir.display());
let test_origin = context.temp_dir.join("origin.git");
run_git(&context.temp_dir, &[
"remote",
"set-url",
"--push",
"origin",
&missing_push,
"init",
"--bare",
"--quiet",
test_origin
.to_str()
.expect("test origin path should be UTF-8"),
]);
let push_url = format!("file://{}", test_origin.display());
run_git(&context.temp_dir, &[
"remote", "set-url", "--push", "origin", &push_url,
]);
let workflow_version_id =
fabro_manifest::collect_workflow_versions(&workflow, &context.temp_dir)
.expect("workflow fixture should package")
.root_id();
let client = spawn_mcp_client(&context, &["--server", &target_url]).await;
let result = client
@ -1981,6 +1990,20 @@ async fn mcp_create_string_shorthand_deserializes_before_auth() {
error.contains("Run `fabro auth login` to authenticate."),
"{error}"
);
assert!(
harness
.api_requests
.contains("POST /api/v1/workflow-versions"),
"the shorthand request should reach workflow-version authentication"
);
assert!(
!harness.workflow_version_exists(workflow_version_id).await,
"an unauthenticated shorthand request must not register a workflow version"
);
assert!(
!harness.api_requests.contains("POST /api/v1/runs"),
"an unauthenticated shorthand request must not reach run creation"
);
assert_mcp_run_tool_count(&client).await;
client

15
lib/apps/fabro-cli/tests/it/support/auth_harness.rs
View file

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

202
lib/apps/fabro-server/src/run_tool_create.rs
View file

@ -1,12 +1,10 @@
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result, bail};
use async_trait::async_trait;
use fabro_config::RunLayer;
use fabro_manifest::{
CollectedWorkflowClosure, ResolvedLocalWorkflowPackage, RunOverrideInput,
collect_workflow_versions, observe_git_run_target, resolve_local_workflow_package,
CollectedWorkflowClosure, ResolvedLocalWorkflowPackage, collect_workflow_versions,
observe_git_run_target, resolve_local_workflow_package,
};
use fabro_tool::{
CreateRunWorkflowSource, PreparedRunCreate, RunCreateAdapter, ValidatedCreateRunSpec,
@ -16,8 +14,6 @@ use fabro_types::{DirtyStatus, RunTarget};
use tokio::io::AsyncWriteExt;
use tokio::{fs, task};
use crate::manifest_validation;
#[derive(Clone, Debug)]
pub struct ServerRunCreateAdapter {
mode: RunCreateMode,
@ -145,13 +141,6 @@ impl ServerRunCreateAdapter {
"target is required because the local checkout cannot be represented as a GitHub run target"
)
})?;
let mut warnings = Vec::new();
if observation.legacy_git_context.dirty == DirtyStatus::Dirty {
warnings.push(
"the local checkout has uncommitted changes; those changes are excluded from the run target"
.to_string(),
);
}
if observation
.legacy_git_context
.sha
@ -159,8 +148,14 @@ impl ServerRunCreateAdapter {
.is_some_and(|sha| !sha.is_empty())
&& 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"
);
}
let mut warnings = Vec::new();
if observation.legacy_git_context.dirty == DirtyStatus::Dirty {
warnings.push(
"the local HEAD commit is not fetchable and is not pinned; the remote branch will be selected"
"the local checkout has uncommitted changes; those changes are excluded from the run target"
.to_string(),
);
}
@ -221,7 +216,6 @@ impl RunCreateAdapter for ServerRunCreateAdapter {
}
CreateRunWorkflowSource::Inline(source) => collect_inline_workflow(source).await?,
};
validate_local_source(&closure, spec, goal.as_deref())?;
let resolved_target = self.resolve_target(spec, cwd).await?;
let versions = closure
.versions()
@ -286,40 +280,13 @@ async fn collect_inline_workflow(
)
})?;
}
collect_workflow_versions(Path::new(source.entrypoint.as_str()), root.path())
.map_err(anyhow::Error::new)
}
fn validate_local_source(
closure: &CollectedWorkflowClosure,
spec: &ValidatedCreateRunSpec,
goal: Option<&str>,
) -> Result<()> {
let run_overrides = run_tool_run_overrides(spec, goal);
let inputs = spec
.inputs
.iter()
.map(|(key, value)| (key.clone(), value.toml().clone()))
.collect::<HashMap<_, _>>();
let response =
manifest_validation::validate_collected_workflow(closure, run_overrides.as_ref(), &inputs)?;
if !response.ok {
bail!("workflow validation failed");
}
Ok(())
}
fn run_tool_run_overrides(spec: &ValidatedCreateRunSpec, goal: Option<&str>) -> Option<RunLayer> {
fabro_manifest::build_sparse_run_overrides(RunOverrideInput {
goal,
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(),
let entrypoint = source.entrypoint.clone();
task::spawn_blocking(move || {
collect_workflow_versions(Path::new(entrypoint.as_str()), root.path())
.map_err(anyhow::Error::new)
})
.await
.context("inline workflow package collection task failed")?
}
#[cfg(test)]
@ -328,7 +295,8 @@ mod tests {
use std::process::Command;
use std::sync::{Arc, Mutex};
use fabro_tool::{FabroRunCreateParams, ValidatedCreateRuns};
use fabro_tool::fabro_client::ClientBackend;
use fabro_tool::{FabroRunCreateParams, FabroToolBackend as _, ValidatedCreateRuns};
use fabro_types::{GitRunTarget, WorkflowVersion, WorkflowVersionId};
use httpmock::Method::POST;
use httpmock::{HttpMockRequest, HttpMockResponse, MockServer};
@ -585,24 +553,24 @@ mod tests {
}
#[tokio::test]
async fn workflow_version_local_content_is_validated_before_registration() {
let client = no_proxy_client("http://127.0.0.1:9");
async fn workflow_version_is_registered_before_server_admission_rejection() {
let server = MockServer::start_async().await;
let registered = Arc::new(Mutex::new(Vec::new()));
let registration =
dynamic_version_registration_mock(&server, Arc::clone(&registered)).await;
let admission = server
.mock_async(|when, then| {
when.method(POST).path("/api/v1/runs");
then.status(422)
.header("content-type", "text/plain")
.body("server-authoritative workflow rejection");
})
.await;
let client = Arc::new(no_proxy_client(&server.url("")));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, None, None);
let invalid_graph = validated_spec(&json!({
"workflow": {
"kind": "inline",
"entrypoint": "workflow.fabro",
"files": { "workflow.fabro": "this is not a graph" }
},
"target": { "kind": "none" }
}));
adapter
.prepare(&client, &invalid_graph, Path::new("/ignored"))
.await
.expect_err("invalid graph should fail before registration");
let undefined_input = validated_spec(&json!({
let backend =
ClientBackend::new(Arc::clone(&client)).with_run_create_adapter(Arc::new(adapter));
let spec = validated_spec(&json!({
"workflow": {
"kind": "inline",
"entrypoint": "workflow.fabro",
@ -618,11 +586,20 @@ mod tests {
},
"target": { "kind": "none" }
}));
let error = adapter
.prepare(&client, &undefined_input, Path::new("/ignored"))
let error = backend
.create_run_from_spec(&spec, Path::new("/ignored"), None)
.await
.expect_err("undefined input should fail before registration");
assert!(error.to_string().contains("workflow validation failed"));
.expect_err("the server should reject the semantically invalid workflow");
registration.assert_calls_async(1).await;
admission.assert_calls_async(1).await;
assert_eq!(registered.lock().unwrap().len(), 1);
assert!(
error
.to_string()
.contains("server-authoritative workflow rejection"),
"unexpected error: {error:#}"
);
}
#[tokio::test]
@ -684,7 +661,7 @@ mod tests {
}
#[tokio::test]
async fn workflow_version_standalone_git_fallback_reports_excluded_local_bytes() {
async fn workflow_version_standalone_rejects_unavailable_head_before_registration_or_create() {
let temp = tempfile::tempdir().unwrap();
let workspace = temp.path().join("workspace");
fs::create_dir(&workspace).await.unwrap();
@ -711,6 +688,81 @@ mod tests {
run_git(&workspace, &[
"remote", "set-url", "--push", "origin", &missing,
]);
let spec = validated_spec(&json!({
"workflow": {
"kind": "inline",
"entrypoint": "workflow.fabro",
"files": {
"workflow.fabro": "digraph W { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}
}
}));
let server = MockServer::start_async().await;
let registered = Arc::new(Mutex::new(Vec::new()));
let registration =
dynamic_version_registration_mock(&server, Arc::clone(&registered)).await;
let create = server
.mock_async(|when, then| {
when.method(POST).path("/api/v1/runs");
then.status(500);
})
.await;
let client = Arc::new(no_proxy_client(&server.url("")));
let adapter = ServerRunCreateAdapter::standalone(None);
let backend =
ClientBackend::new(Arc::clone(&client)).with_run_create_adapter(Arc::new(adapter));
let error = backend
.create_run_from_spec(&spec, &workspace, None)
.await
.expect_err("an unavailable local HEAD must not degrade to a branch-only target");
registration.assert_calls_async(0).await;
create.assert_calls_async(0).await;
assert!(registered.lock().unwrap().is_empty());
assert!(
error
.to_string()
.contains("exact local Git commit could not be made available"),
"unexpected error: {error:#}"
);
}
#[tokio::test]
async fn workflow_version_standalone_preserves_dirty_warning_for_exact_target() {
let temp = tempfile::tempdir().unwrap();
let workspace = temp.path().join("workspace");
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",
"feature",
]);
run_git(&workspace, &["config", "user.name", "Fabro Test"]);
run_git(&workspace, &["config", "user.email", "fabro@example.com"]);
fs::write(workspace.join("tracked.txt"), "committed")
.await
.unwrap();
run_git(&workspace, &["add", "tracked.txt"]);
run_git(&workspace, &["commit", "--quiet", "-m", "initial"]);
run_git(&workspace, &[
"remote",
"add",
"origin",
"https://github.com/acme/widgets.git",
]);
let push_url = format!("file://{}", origin.display());
run_git(&workspace, &[
"remote", "set-url", "--push", "origin", &push_url,
]);
fs::write(workspace.join("dirty.txt"), "uncommitted")
.await
.unwrap();
@ -732,18 +784,12 @@ mod tests {
};
assert_eq!(target.repo, "acme/widgets");
assert_eq!(target.branch, "feature");
assert_eq!(target.sha, None);
assert!(target.sha.is_some());
assert!(
prepared
.warnings
.iter()
.any(|warning| warning.contains("uncommitted changes"))
);
assert!(
prepared
.warnings
.iter()
.any(|warning| warning.contains("not fetchable") && warning.contains("not pinned"))
);
}
}

8
lib/components/fabro-tool/src/common.rs
View file

@ -62,9 +62,11 @@ pub struct CreateRunSubmission {
/// Trusted producer-local preparation for one tool-created run.
///
/// Implementations acquire permitted workflow bytes and goal files, validate
/// local content, resolve the independent target, and register immutable
/// workflow versions before returning intent-ready fields.
/// Implementations acquire permitted workflow bytes and goal files, enforce
/// producer capabilities and package structure, resolve the independent
/// target, and register immutable workflow versions before returning
/// intent-ready fields. The server remains authoritative for semantic run
/// admission.
#[async_trait]
pub trait RunCreateAdapter: Send + Sync {
async fn prepare(

12
lib/components/fabro-tool/src/create.rs
View file

@ -128,7 +128,7 @@ impl JsonSchema for CreateRunSpecInput {
"description": "Optional parent run id or selector."
},
"target": {
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted; standalone calls require an observable Git checkout when omitted.",
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted; standalone calls require an attached GitHub checkout whose exact local HEAD is available from the canonical origin.",
"anyOf": [
{ "type": "null" },
{
@ -414,7 +414,6 @@ pub struct ValidatedCreateRunSpec {
#[derive(Debug)]
pub struct ValidatedRunInputValue {
json: Value,
toml: toml::Value,
}
impl ValidatedRunInputValue {
@ -422,11 +421,6 @@ impl ValidatedRunInputValue {
pub fn json(&self) -> &Value {
&self.json
}
#[must_use]
pub fn toml(&self) -> &toml::Value {
&self.toml
}
}
impl TryFrom<FabroRunCreateParams> for ValidatedCreateRuns {
@ -501,8 +495,8 @@ impl TryFrom<CreateRunSpec> for ValidatedCreateRunSpec {
.into_iter()
.map(|(key, value)| {
let json = value.into_inner();
manifest::json_to_toml_value(&key, &json)
.map(|toml| (key, ValidatedRunInputValue { json, toml }))
manifest::json_to_toml_value(&key, &json)?;
Ok((key, ValidatedRunInputValue { json }))
})
.collect::<ToolResult<HashMap<_, _>>>()?;
let target = spec