scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-14 23:22:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Fix child target inheritance and inline workflow path validation

Browse source
This commit is contained in:
Scott Werner 2026-09-11 12:40:02 -06:00
parent 6f72d369bf
commit 1e8e1c9a30
9 changed files with 307 additions and 151 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -153,7 +153,11 @@ 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; for a Git parent that means the repository and branch at their current remote HEAD, not the parent's pinned commit, so a child sees commits the parent has pushed. Folder targets are available only to standalone MCP and Local workflow agents with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment. When `target` is omitted, standalone MCP derives it from the selected environment the way `fabro run` does: a Local environment targets the working directory as a folder, and a Docker or Daytona environment targets the attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin (a directory with no Git metadata runs with no workspace); otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Workflow source and run target are independent. Object-form calls can provide an explicit Git, `none`, or folder target. A workflow agent inherits its parent run's canonical target when `target` is omitted; for a Git parent that means its repository and current execution branch (normally `fabro/run/<parent-id>`). The original input branch and pinned commit/tag are not inherited. Push the parent's changes before creating the child. If run branches are disabled, the child follows the original input branch; if an enabled run branch is not available yet, provide an explicit target.
Folder targets are available only to standalone MCP and Local workflow agents with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
Standalone MCP calls using a stored workflow ID must supply an explicit `target`: the stored repository configuration is not available for implicit derivation. For selector and inline sources, when `target` is omitted, standalone MCP derives it from the selected environment the way `fabro run` does: a Local environment targets the working directory as a folder, and a Docker or Daytona environment targets the attached GitHub checkout only when it can prove that the exact local HEAD is available from the canonical origin (a directory with no Git metadata runs with no workspace); otherwise, push the commit or provide an explicit target. Docker and Daytona agents must send inline files or a stored version ID—never send a sandbox path as a selector, because the run worker cannot use that path to read the sandbox's files.
Use `goal` for inline goal text or `goal_file` to read the run goal from a file. They are mutually exclusive. Relative `goal_file` paths resolve from the run's `cwd`, or from the MCP server working directory when `cwd` is omitted. Like selectors, `goal_file` requires a native/shared filesystem; Docker and Daytona agents must send `goal` text by value.

2
docs/public/execution/child-runs.mdx
View file

@ -134,7 +134,7 @@ Reuse content already registered with Fabro by supplying its exact immutable ID:
}
```
Workflow content and workspace target are separate choices. If `target` is omitted, a child inherits the parent's canonical target: a `none` or folder target as-is, and a Git target's repository and branch. The child checks out the branch's current remote HEAD, so commits the parent has pushed are visible to it; the parent's pinned commit and tag are not carried over. Pass an explicit Git target with a `sha` to pin a child. An explicit Git, `none`, or folder target overrides that inheritance while the current run remains the forced parent. Folder targets require a Local parent with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
Workflow content and workspace target are separate choices. If `target` is omitted, a child inherits the parent's canonical target: a `none` or folder target as-is, and a Git target's repository and current execution branch (normally `fabro/run/<parent-id>`). Push the parent's changes before creating the child; the child checks out that branch's current remote HEAD. The original input branch and pinned commit/tag are not carried over. If run branches are disabled, the child follows the original input branch. If an enabled run branch is not available yet, provide an explicit target. Pass an explicit Git target with a `sha` to pin a child. An explicit Git, `none`, or folder target overrides that inheritance while the current run remains the forced parent. Folder targets require a Local parent with a shared host filesystem; Docker and Daytona parents cannot select a server-host folder, even by naming a Local child environment.
`goal_file` is also a shared-filesystem feature. Local agents can read it relative to the operation `cwd`; Docker and Daytona agents must send the resolved `goal` text by value.

21
lib/apps/fabro-cli/src/commands/run/runner.rs
View file

@ -20,7 +20,7 @@ use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer};
use fabro_tool::fabro_client::ClientBackend;
use fabro_types::settings::run::{EnvironmentProvider, RunMode, RunNamespace};
use fabro_types::{
ArtifactUpload, BlobHash, EventBody, FailureReason, Principal, RunEvent, RunId, RunTarget,
ArtifactUpload, BlobHash, EventBody, FailureReason, Principal, RunEvent, RunId,
};
use fabro_vault::{SecretStore, Vault};
use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader};
@ -104,7 +104,6 @@ pub(crate) async fn execute(
client.clone_for_reuse(),
run_id,
run_spec.settings.run.environment.provider,
run_spec.target.clone(),
run_spec.source_directory.as_deref(),
&run_dir,
)
@ -234,16 +233,19 @@ fn build_fabro_run_tool_services(
client: fabro_client::Client,
current_run_id: RunId,
provider: EnvironmentProvider,
inherited_target: Option<RunTarget>,
source_directory: Option<&str>,
run_dir: &Path,
) -> Option<FabroRunToolServices> {
if worker_token.trim().is_empty() {
return None;
}
let backend = ClientBackend::new(Arc::new(client)).with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::worker(provider, inherited_target, Some(default_workflows_dir())),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager));
let backend = ClientBackend::new(Arc::new(client))
.with_run_create_adapter(Arc::new(ServerRunCreateAdapter::worker(
provider,
current_run_id,
Some(default_workflows_dir()),
)))
.with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager));
Some(FabroRunToolServices {
backend: Arc::new(backend),
current_run_id,
@ -1263,7 +1265,7 @@ mod tests {
}
#[tokio::test]
async fn fabro_run_create_worker_adapter_uses_runtime_provider_and_canonical_target() {
async fn fabro_run_create_worker_adapter_preserves_explicit_target_without_host_reads() {
use fabro_tool::{FabroRunCreateParams, RunCreateAdapter, ValidatedCreateRuns};
use fabro_types::settings::run::EnvironmentProvider;
@ -1276,7 +1278,8 @@ mod tests {
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
}
},
"target": inherited
}]
}))
.unwrap();
@ -1286,7 +1289,7 @@ mod tests {
.remove(0);
let adapter = ServerRunCreateAdapter::worker(
EnvironmentProvider::Docker,
Some(inherited.clone()),
fabro_types::RunId::new(),
Some(temp.path().join("workflows")),
);
let client = fabro_client::Client::new_no_proxy("http://127.0.0.1:9").unwrap();

16
lib/apps/fabro-mcp-server/src/server.rs
View file

@ -3,8 +3,8 @@ use std::sync::Arc;
use std::time::Duration;
use anyhow::Result;
use fabro_manifest::SuppliedWorkflowVersionPackager;
use fabro_config::user;
use fabro_manifest::SuppliedWorkflowVersionPackager;
use fabro_server::run_tool_create::ServerRunCreateAdapter;
use fabro_tool::fabro_client::ClientBackend;
use fabro_tool::{self as run_tools, FabroToolBackend};
@ -275,11 +275,15 @@ impl FabroMcpServer {
.await
.map(|client| {
Arc::new(
ClientBackend::new(Arc::new(client)).with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::standalone(Some(
user::default_workflows_dir(),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)),
)).with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)),
ClientBackend::new(Arc::new(client))
.with_run_create_adapter(Arc::new(
ServerRunCreateAdapter::standalone(Some(
user::default_workflows_dir(),
)),
))
.with_workflow_version_packager(Arc::new(
SuppliedWorkflowVersionPackager,
)),
) as Arc<dyn FabroToolBackend>
})
.map_err(|err| run_tools::ToolError::from_anyhow(&err))

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

@ -11,8 +11,8 @@ use fabro_manifest::{
use fabro_tool::{
CreateRunWorkflowSource, PreparedRunCreate, RunCreateAdapter, ValidatedCreateRunSpec,
};
use fabro_types::RunTarget;
use fabro_types::settings::run::EnvironmentProvider;
use fabro_types::{RunId, RunProjection, RunTarget};
use tokio::{fs, task};
#[derive(Clone, Debug)]
@ -27,7 +27,7 @@ enum RunCreateMode {
},
Worker {
provider: EnvironmentProvider,
inherited_target: Option<RunTarget>,
parent_run_id: RunId,
user_workflows_root: Option<PathBuf>,
},
}
@ -45,13 +45,13 @@ impl ServerRunCreateAdapter {
#[must_use]
pub fn worker(
provider: EnvironmentProvider,
inherited_target: Option<RunTarget>,
parent_run_id: RunId,
user_workflows_root: Option<PathBuf>,
) -> Self {
Self {
mode: RunCreateMode::Worker {
provider,
inherited_target,
parent_run_id,
user_workflows_root,
},
}
@ -119,19 +119,16 @@ impl ServerRunCreateAdapter {
}
match &self.mode {
RunCreateMode::Worker {
inherited_target: Some(target),
..
} => Ok(ResolvedTarget {
target: inherit_parent_target(target),
warnings: Vec::new(),
}),
RunCreateMode::Worker {
inherited_target: None,
..
} => bail!(
"the parent run has no canonical target; send an explicit target for this child run"
),
RunCreateMode::Worker { parent_run_id, .. } => {
let parent = client
.get_run_state(parent_run_id)
.await
.context("could not retrieve the parent run's current execution state")?;
Ok(ResolvedTarget {
target: inherit_parent_target(&parent)?,
warnings: Vec::new(),
})
}
RunCreateMode::Standalone { .. } => {
// Standalone callers derive the target the same way `fabro run`
// does: from the selected environment's provider. Local
@ -225,8 +222,14 @@ impl RunCreateAdapter for ServerRunCreateAdapter {
CreateRunWorkflowSource::Stored {
workflow_version_id,
} => {
// A stored version's config is not available locally, so
// derivation uses the checkout's own origin.
// Stored configuration is not available through the client
// API. Never substitute the caller's checkout for a workflow's
// configured repository merely because it was supplied by ID.
if matches!(self.mode, RunCreateMode::Standalone { .. }) && spec.target.is_none() {
bail!(
"standalone stored workflow sources require an explicit target; the stored workflow's repository configuration is not available for target derivation"
);
}
let resolved_target = self.resolve_target(client, spec, cwd, None).await?;
return Ok(PreparedRunCreate {
workflow_version_id: *workflow_version_id,
@ -263,23 +266,32 @@ struct ResolvedTarget {
warnings: Vec<String>,
}
/// The target a child inherits when it omits its own. A parent's Git target
/// is pinned to the commit admitted for the parent, and clone-based providers
/// never fall back to branch HEAD, so carrying that pin forward would hide
/// every commit the parent has since pushed from a child meant to review or
/// continue that work. The child follows the parent's branch instead; the
/// pinned commit and tag stay on the parent only. Folder and none targets are
/// inherited as-is.
fn inherit_parent_target(parent: &RunTarget) -> RunTarget {
match parent {
RunTarget::Git(git) => RunTarget::Git(fabro_types::GitRunTarget {
repo: git.repo.clone(),
branch: git.branch.clone(),
tag: None,
sha: None,
}),
RunTarget::None {} | RunTarget::Folder { .. } => parent.clone(),
}
/// Follow the parent's execution branch, not the branch it originally cloned.
/// A missing execution branch must not silently substitute unrelated source
/// content. When run branches are disabled, execution stays on the input
/// branch.
fn inherit_parent_target(parent: &RunProjection) -> Result<RunTarget> {
let target = parent.spec.target.as_ref().context(
"the parent run has no canonical target; send an explicit target for this child run",
)?;
Ok(match target {
RunTarget::Git(git) => {
let branch = if parent.spec.settings.run.run_branch.enabled {
parent.start.as_ref().and_then(|start| start.run_branch.as_ref()).context(
"the parent run has no execution branch yet; send an explicit target for this child run"
)?
} else {
&git.branch
};
RunTarget::Git(fabro_types::GitRunTarget {
repo: git.repo.clone(),
branch: branch.clone(),
tag: None,
sha: None,
})
}
RunTarget::None {} | RunTarget::Folder { .. } => target.clone(),
})
}
/// Collect an inline workflow from its supplied bytes. The entrypoint is an
@ -450,7 +462,8 @@ mod tests {
"target": { "kind": "none" },
"start": false
}));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, RunId::new(), None);
let prepared = adapter
.prepare(&client, &spec, Path::new("/host/that-must-not-be-read"))
@ -504,7 +517,8 @@ mod tests {
},
"target": { "kind": "none" }
}));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, RunId::new(), None);
let prepared = adapter
.prepare(&client, &spec, Path::new("/host/that-must-not-be-read"))
@ -517,45 +531,170 @@ mod tests {
assert_eq!(registered[0].entrypoint().as_str(), "review");
}
#[tokio::test]
async fn workflow_version_stored_create_skips_registration_and_inherits_worker_branch() {
let client = no_proxy_client("http://127.0.0.1:9");
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let inherited = RunTarget::Git(GitRunTarget {
repo: "fabro-sh/fabro".to_string(),
branch: "main".to_string(),
tag: Some("v1.0.0".to_string()),
sha: Some("0123456789abcdef0123456789abcdef01234567".to_string()),
});
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
}
}));
let adapter = ServerRunCreateAdapter::worker(
EnvironmentProvider::Docker,
Some(inherited.clone()),
None,
);
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())
}
let prepared = adapter
.prepare(&client, &spec, Path::new("/ignored"))
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 workflow_version_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();
assert_eq!(prepared.workflow_version_id, workflow_version_id);
// The child follows the parent's branch so commits the parent pushed
// are visible; the parent's pinned commit and tag are not inherited.
assert_eq!(
prepared.target,
RunTarget::Git(GitRunTarget {
repo: "fabro-sh/fabro".to_string(),
branch: "main".to_string(),
tag: None,
sha: None,
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),
})));
// Construct the adapter before execution starts, exactly as the worker does.
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, parent.spec.id(), None);
let sandbox = fabro_sandbox::LocalSandbox::new(workspace.clone());
// Docker and Daytona use this same setup operation to create the run branch.
let git =
fabro_sandbox::setup_git_via_exec(&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 = no_proxy_client(&server.url(""));
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let spec = validated_spec(
&json!({ "workflow": { "kind": "stored", "workflow_version_id": workflow_version_id } }),
);
let prepared = adapter
.prepare(&client, &spec, Path::new("/must-not-be-read"))
.await
.unwrap();
state_request.assert_calls_async(1).await;
assert_eq!(prepared.workflow_version_id, workflow_version_id);
let RunTarget::Git(target) = prepared.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"
);
}
#[test]
fn inherited_target_requires_execution_state_unless_run_branches_are_disabled() {
let mut parent = test_parent(Some(RunTarget::Git(GitRunTarget {
repo: "acme/widgets".to_owned(),
branch: "main".to_owned(),
tag: None,
sha: None,
})));
assert!(
inherit_parent_target(&parent)
.unwrap_err()
.to_string()
.contains("no execution branch yet")
);
parent.spec.settings.run.run_branch.enabled = false;
assert_eq!(
inherit_parent_target(&parent).unwrap(),
parent.spec.target.clone().unwrap()
);
for target in [RunTarget::None {}, RunTarget::Folder {
path: "/shared/workspace".to_owned(),
}] {
parent.spec.target = Some(target.clone());
assert_eq!(inherit_parent_target(&parent).unwrap(), target);
}
}
#[tokio::test]
async fn workflow_version_standalone_stored_source_requires_target_without_reading_cwd() {
let client = no_proxy_client("http://127.0.0.1:9");
let adapter = ServerRunCreateAdapter::standalone(None);
let workflow_version_id: WorkflowVersionId =
fabro_types::BlobHash::new(b"stored with repository config").into();
let mut spec = validated_spec(
&json!({ "workflow": { "kind": "stored", "workflow_version_id": workflow_version_id } }),
);
let error = adapter
.prepare(&client, &spec, Path::new("/must-not-be-read"))
.await
.unwrap_err();
assert!(
error
.to_string()
.contains("stored workflow sources require an explicit target")
);
spec.target = Some(RunTarget::Git(GitRunTarget {
repo: "acme/upstream".to_owned(),
branch: "main".to_owned(),
tag: None,
sha: None,
}));
let prepared = adapter
.prepare(&client, &spec, Path::new("/must-not-be-read"))
.await
.unwrap();
assert_eq!(prepared.target, spec.target.unwrap());
assert_eq!(prepared.workflow_version_id, workflow_version_id);
}
#[tokio::test]
@ -589,7 +728,8 @@ mod tests {
"workflow": "demo",
"target": { "kind": "none" }
}));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Local, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Local, RunId::new(), None);
let prepared = adapter
.prepare(&client, &spec, &operation_cwd)
@ -622,7 +762,8 @@ mod tests {
.await
.unwrap();
let client = no_proxy_client("http://127.0.0.1:9");
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Daytona, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Daytona, RunId::new(), None);
let selector = validated_spec(&json!({
"workflow": "same-name.fabro",
@ -667,7 +808,7 @@ mod tests {
}));
for provider in [EnvironmentProvider::Docker, EnvironmentProvider::Daytona] {
let adapter = ServerRunCreateAdapter::worker(provider, None, None);
let adapter = ServerRunCreateAdapter::worker(provider, RunId::new(), None);
let error = adapter
.prepare(&client, &spec, Path::new("/ignored"))
.await
@ -696,7 +837,8 @@ mod tests {
},
"target": target
}));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Local, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Local, RunId::new(), None);
let prepared = adapter
.prepare(&client, &spec, Path::new("/ignored"))
@ -721,7 +863,8 @@ mod tests {
})
.await;
let client = Arc::new(no_proxy_client(&server.url("")));
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, None, None);
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, RunId::new(), None);
let backend =
ClientBackend::new(Arc::clone(&client)).with_run_create_adapter(Arc::new(adapter));
let spec = validated_spec(&json!({
@ -758,8 +901,12 @@ mod tests {
#[tokio::test]
async fn workflow_version_target_failure_precedes_registration() {
let client = no_proxy_client("http://127.0.0.1:9");
let adapter = ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, None, None);
let parent = test_parent(None);
let server = MockServer::start_async().await;
let state_request = mock_parent(&server, &parent).await;
let client = no_proxy_client(&server.url(""));
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Docker, parent.spec.id(), None);
let spec = validated_spec(&json!({
"workflow": {
"kind": "inline",
@ -775,6 +922,7 @@ mod tests {
.await
.expect_err("missing inherited target should fail before registration");
state_request.assert_calls_async(1).await;
assert!(
error
.to_string()
@ -799,11 +947,8 @@ mod tests {
"target": { "kind": "none" },
"goal_file": "goal.md"
}));
let inherited = RunTarget::Folder {
path: "/parent/workspace".to_string(),
};
let adapter =
ServerRunCreateAdapter::worker(EnvironmentProvider::Local, Some(inherited), None);
ServerRunCreateAdapter::worker(EnvironmentProvider::Local, RunId::new(), None);
let prepared = adapter.prepare(&client, &spec, temp.path()).await.unwrap();
@ -922,22 +1067,25 @@ mod tests {
.await
.unwrap();
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
"kind": "inline", "entrypoint": "main.fabro", "files": {
"main.fabro": "digraph W { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}
},
"environment": "sandbox"
}));
let server = MockServer::start_async().await;
let environment = mock_environment(&server, "sandbox", "daytona").await;
let registration =
dynamic_version_registration_mock(&server, Arc::new(Mutex::new(Vec::new()))).await;
let client = no_proxy_client(&server.url(""));
let adapter = ServerRunCreateAdapter::standalone(None);
let prepared = adapter.prepare(&client, &spec, &workspace).await.unwrap();
environment.assert_calls_async(1).await;
registration.assert_calls_async(1).await;
let RunTarget::Git(target) = prepared.target else {
panic!("standalone attached Git checkout should derive a Git target");
};
@ -957,20 +1105,21 @@ mod tests {
let temp = tempfile::tempdir().unwrap();
let workspace = temp.path().join("plain");
fs::create_dir(&workspace).await.unwrap();
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
},
"workflow": { "kind": "inline", "entrypoint": "main.fabro", "files": {
"main.fabro": "digraph W { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}},
"environment": "local"
}));
let server = MockServer::start_async().await;
mock_environment(&server, "local", "local").await;
let registration =
dynamic_version_registration_mock(&server, Arc::new(Mutex::new(Vec::new()))).await;
let client = no_proxy_client(&server.url(""));
let adapter = ServerRunCreateAdapter::standalone(None);
let prepared = adapter.prepare(&client, &spec, &workspace).await.unwrap();
registration.assert_calls_async(1).await;
let expected = workspace.canonicalize().unwrap();
assert_eq!(prepared.target, RunTarget::Folder {
@ -984,19 +1133,22 @@ mod tests {
let temp = tempfile::tempdir().unwrap();
let workspace = temp.path().join("plain");
fs::create_dir(&workspace).await.unwrap();
let workflow_version_id: WorkflowVersionId = fabro_types::BlobHash::new(b"stored").into();
let spec = validated_spec(&json!({
"workflow": {
"kind": "stored",
"workflow_version_id": workflow_version_id
"kind": "inline", "entrypoint": "main.fabro", "files": {
"main.fabro": "digraph W { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
}
}
}));
let server = MockServer::start_async().await;
mock_environment(&server, "default", "docker").await;
let registration =
dynamic_version_registration_mock(&server, Arc::new(Mutex::new(Vec::new()))).await;
let client = no_proxy_client(&server.url(""));
let adapter = ServerRunCreateAdapter::standalone(None);
let prepared = adapter.prepare(&client, &spec, &workspace).await.unwrap();
registration.assert_calls_async(1).await;
assert_eq!(prepared.target, RunTarget::None {});
}

31
lib/components/fabro-manifest/src/workflow_version_collector.rs
View file

@ -206,7 +206,7 @@ pub fn collect_inline_workflow_versions(
path: entrypoint.to_string(),
});
}
fabro_types::validate_workflow_path_collisions(files.keys()).map_err(|error| match error {
fabro_types::validate_workflow_source_paths(files.keys()).map_err(|error| match error {
WorkflowVersionShapeError::PathCollision { second, .. } => {
WorkflowVersionCollectError::PathCollision {
entrypoint: entrypoint.clone(),
@ -248,7 +248,7 @@ pub fn collect_inline_workflow_versions(
anyhow::Error::new(source).context("failed to canonicalize the inline workflow root"),
)
})?;
let location = WorkflowLocation::from_exact_path(package_root.join(entrypoint.as_str()))
let location = WorkflowLocation::from_exact_path(Path::new(entrypoint.as_str()), &package_root)
.map_err(|source| collect_error(source.into()))?;
let location = canonicalize_location(location, |path, source| {
collect_error(anyhow::Error::new(source).context(format!(
@ -516,16 +516,23 @@ dockerfile = { path = "Dockerfile" }
let (_, root) = closure.versions().next().unwrap();
assert_eq!(root.version().entrypoint().as_str(), "review");
let colliding = BTreeMap::from([
(WorkflowPath::new("a").unwrap(), "digraph A {}".to_string()),
(WorkflowPath::new("a/b.md").unwrap(), "b".to_string()),
]);
let error = collect_inline_workflow_versions(&WorkflowPath::new("a").unwrap(), &colliding)
.unwrap_err();
assert!(
matches!(error, WorkflowVersionCollectError::PathCollision { .. }),
"unexpected error: {error:#}"
);
for (file, descendant) in [
("a", "a/b.md"),
("A", "a/b.md"),
("dir/File", "DIR/file/child.md"),
("Prompt.md", "prompt.md"),
] {
let entrypoint = WorkflowPath::new(file).unwrap();
let colliding = BTreeMap::from([
(entrypoint.clone(), "digraph A {}".to_string()),
(WorkflowPath::new(descendant).unwrap(), "b".to_string()),
]);
let error = collect_inline_workflow_versions(&entrypoint, &colliding).unwrap_err();
assert!(
matches!(error, WorkflowVersionCollectError::PathCollision { .. }),
"unexpected error: {error:#}"
);
}
}
#[test]

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

@ -131,7 +131,7 @@ impl JsonSchema for CreateRunWorkflowSource {
/// spelled out here and pinned by the serde parity test below.
fn run_target_schema(_: &mut SchemaGenerator) -> Schema {
json_schema!({
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted (a Git parent contributes its repository and branch, not its pinned commit or tag, so the child sees commits the parent has pushed); standalone calls derive it from the selected environment: Local environments target the working directory folder, and clone-based environments require an attached GitHub checkout whose exact local HEAD is available from the canonical origin. Folder targets require a standalone or Local-worker filesystem context.",
"description": "Canonical run workspace target. Worker calls inherit the parent target when omitted (a Git parent contributes its repository and current execution branch from run state, not its original input branch or pinned commit/tag; changes must be pushed before creating the child); standalone stored-ID calls require an explicit target; standalone selector and inline calls derive it from the selected environment: Local environments target the working directory folder, and clone-based environments require an attached GitHub checkout whose exact local HEAD is available from the canonical origin. Folder targets require a standalone or Local-worker filesystem context.",
"anyOf": [
{ "type": "null" },
{
@ -490,9 +490,8 @@ fn validate_workflow_source(
source.entrypoint
)));
}
fabro_types::validate_workflow_path_collisions(source.files.keys())
fabro_types::validate_workflow_source_paths(source.files.keys())
.map_err(|err| ToolError::message(format!("inline workflow {err}")))?;
validate_inline_paths_distinct_ignoring_case(&source.files)?;
let mut total_bytes = 0usize;
for (path, content) in &source.files {
let bytes = content.len();
@ -518,24 +517,6 @@ fn validate_workflow_source(
}
}
/// Inline files are staged on, and later checked out to, filesystems that may
/// be case-insensitive, so two paths that differ only by case would silently
/// overwrite each other there. Reject them up front with a clear message
/// instead of surfacing a platform-dependent I/O error later.
fn validate_inline_paths_distinct_ignoring_case(
files: &BTreeMap<WorkflowPath, String>,
) -> ToolResult<()> {
let mut seen: HashMap<String, &WorkflowPath> = HashMap::with_capacity(files.len());
for path in files.keys() {
if let Some(existing) = seen.insert(path.as_str().to_lowercase(), path) {
return Err(ToolError::message(format!(
"inline workflow files `{existing}` and `{path}` differ only by case; workflow files must stay distinct on case-insensitive filesystems"
)));
}
}
Ok(())
}
#[derive(Debug, Serialize, JsonSchema)]
pub struct CreateRunsResult {
pub runs: Vec<CreatedRunResult>,
@ -917,9 +898,14 @@ mod tests {
for (files, expected) in [
(json!({ "a": "x", "a/b.md": "y" }), "paths collide"),
(json!({ "A": "x", "a/b.md": "y" }), "paths collide"),
(
json!({ "dir/File": "x", "DIR/file/child.md": "y" }),
"paths collide",
),
(
json!({ "main.fabro": "digraph W {}", "Prompt.md": "x", "prompt.md": "y" }),
"differ only by case",
"paths collide",
),
] {
let entrypoint = files

6
lib/components/fabro-tool/src/fabro_client.rs
View file

@ -16,9 +16,9 @@ use crate::{
#[derive(Clone)]
pub struct ClientBackend {
client: Arc<::fabro_client::Client>,
run_create_adapter: Option<Arc<dyn RunCreateAdapter>>,
run_scope: Option<RunId>,
client: Arc<::fabro_client::Client>,
run_create_adapter: Option<Arc<dyn RunCreateAdapter>>,
run_scope: Option<RunId>,
workflow_version_packager: Option<Arc<dyn crate::WorkflowVersionPackager>>,
}

6
lib/components/fabro-tool/src/lib.rs
View file

@ -19,9 +19,9 @@ mod workflow_version;
pub use common::{
CreateRunSubmission, FABRO_RUN_CREATE_TOOL_NAME, FABRO_RUN_EVENTS_TOOL_NAME,
FABRO_RUN_GATHER_TOOL_NAME, FABRO_RUN_GET_TOOL_NAME, FABRO_RUN_INTERACT_TOOL_NAME,
FABRO_RUN_PAIR_TOOL_NAME, FABRO_RUN_SEARCH_TOOL_NAME, FabroToolBackend, PreparedRunCreate,
RunCreateAdapter, RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions,
FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME,
FABRO_RUN_PAIR_TOOL_NAME, FABRO_RUN_SEARCH_TOOL_NAME, FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME,
FabroToolBackend, PreparedRunCreate, RunCreateAdapter, RunSummaryResult, ToolDefinition,
ToolError, ToolResult, tool_definitions,
};
pub use create::{
CreateRunOptions, CreateRunSpec, CreateRunSpecInput, CreateRunWorkflowSource, CreateRunsResult,