diff --git a/Cargo.lock b/Cargo.lock index 58a7ae412..df85b11db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2754,12 +2754,14 @@ name = "fabro-manifest" version = "0.354.0-nightly.0" dependencies = [ "anyhow", + "async-trait", "fabro-api", "fabro-config", "fabro-github", "fabro-graphviz", "fabro-template", "fabro-test", + "fabro-tool", "fabro-types", "fabro-util", "fabro-workflow", @@ -2770,7 +2772,10 @@ dependencies = [ "temp-env", "tempfile", "thiserror 2.0.18", + "tokio", "toml 0.8.23", + "tracing", + "tracing-subscriber", ] [[package]] @@ -3168,7 +3173,9 @@ dependencies = [ "fabro-client", "fabro-types", "fabro-util", + "fabro-workflow-version", "futures", + "httpmock", "schemars 1.2.1", "serde", "serde_json", @@ -3214,6 +3221,8 @@ dependencies = [ "thiserror 2.0.18", "toml 0.8.23", "ulid", + "unicase", + "unicode-normalization", "url", ] @@ -3304,6 +3313,7 @@ dependencies = [ "fabro-api", "fabro-auth", "fabro-checkpoint", + "fabro-client", "fabro-config", "fabro-core", "fabro-dump", @@ -3328,6 +3338,7 @@ dependencies = [ "fabro-validate", "fabro-vault", "fabro-workflow", + "fabro-workflow-version", "futures", "git2", "hex", diff --git a/Cargo.toml b/Cargo.toml index 8bbbfb212..20d1ecf54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,8 @@ console = "0.15" dialoguer = "0.12" git2 = { version = "0.20", default-features = false, features = ["vendored-libgit2", "vendored-openssl", "https"] } tracing = "0.1" +unicase = "2" +unicode-normalization = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter"] } tracing-appender = "0.2" rmcp = { version = "1.4", default-features = false } diff --git a/docs/public/agents/mcp.mdx b/docs/public/agents/mcp.mdx index b0920090f..e769a4bf3 100644 --- a/docs/public/agents/mcp.mdx +++ b/docs/public/agents/mcp.mdx @@ -5,7 +5,7 @@ description: "Connect MCP tools to agents and expose Fabro runs to MCP clients" MCP ([Model Context Protocol](https://modelcontextprotocol.io/)) lets you connect external tool servers to Fabro agents. An MCP server exposes tools over a standardized protocol — databases, APIs, file systems, custom services — and Fabro discovers and registers them automatically. Agents call MCP tools the same way they call built-in tools. -Fabro can also run as an MCP server. MCP clients can use Fabro's run-management tools to create, inspect, control, wait for, and read events from workflow runs through the authenticated `fabro` CLI. +Fabro can also run as an MCP server. MCP clients can register reusable workflow versions and use Fabro's run-management tools to create, inspect, control, wait for, and read events from workflow runs through the authenticated `fabro` CLI. Workflow agents can opt in to that same run-management tool catalog with `[run.agent] fabro_tools = true`. This is not the same as configuring external MCP servers for the agent. When a workflow agent calls `fabro_run_create`, created runs are always [child runs](/execution/child-runs) of the current run; an explicit `parent_id` must match the current run ID. @@ -39,6 +39,7 @@ fabro mcp init claude --name fabro-testing --server https://fabro-testing.exampl | Tool | Purpose | |---|---| +| `fabro_workflow_version_create` | Register supplied workflow contents and local dependencies as an immutable version ID, without creating a run. | | `fabro_run_create` | Create one or more workflow runs, optionally under a parent run, starting them by default. | | `fabro_run_search` | Search runs by ID, parent, workflow, labels, status, archive state, and creation time. | | `fabro_run_get` | Read-only inspection of a run: returns its summary, projection, and pending questions without mutating state. | @@ -47,6 +48,48 @@ fabro mcp init claude --name fabro-testing --server https://fabro-testing.exampl | `fabro_run_pair` | Inspect, start, message, end, or read transcript for a live run pairing session. | | `fabro_run_events` | List, inspect, or search stored events for a run. | +### Register workflow contents from a sandbox + +Use shell and read tools in your sandbox to acquire the workflow and all its local +config, graph, prompt, script, import, and child-workflow files. For example, clone +a repository with your sandbox's shell tool, then read `workflow.fabro` and its +referenced `prompt.md`. Submit the actual contents: + +```json +{ + "entrypoint": "workflow.fabro", + "files": { + "workflow.fabro": "digraph W { start [shape=Mdiamond] work [prompt=\"@prompt.md\"] exit [shape=Msquare] start -> work -> exit }", + "prompt.md": "Review the implementation." + } +} +``` + +Call `fabro_workflow_version_create` with this object and keep the returned +`workflow_version_id`. You can reuse it in a `RunIntent` submitted through the +[Create Run API](/api-reference/runs/create-run). Registration packages and uploads +child workflows before their parents; callers do not calculate dependency IDs. + +`entrypoint` is an exact supplied key, including when it has no extension. File +values are text, never host paths or URLs to fetch. Missing references and paths +that escape the supplied tree fail. The source tree is limited to 512 files, +512 KiB per file, and 2 MiB of text; each resulting serialized version must also +fit the existing 2 MiB API limit. Case-insensitive file and ancestor collisions +are rejected before staging. Graph nesting through child workflows and imports +is limited to 64 levels, including the entrypoint. A supplied sibling +`workflow.toml` must be valid even when a graph is the entrypoint; a valid config +that selects another graph is omitted. Collection follows declared file references; command +`script` values remain literal text, and paths embedded in shell commands are not +inspected or acquired. + +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. + +### Create runs + For a simple create call, `fabro_run_create` accepts a workflow selector string: ```json diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 9ec0dbb2d..96ca10147 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -1086,6 +1086,9 @@ paths: description: >- Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. + Requires an authenticated user or a worker token with the `agent:run_tools` + capability. Ordinary worker tokens cannot register versions. Registration + creates no run and starts no execution. requestBody: required: true content: diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 1a617e977..79637e882 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -509,7 +509,7 @@ Configure workflow agent behavior that is not tied to a single stage. fabro_tools = true ``` -`fabro_tools` defaults to `false`. Set it to `true` only for runs whose agents should be able to use the same Fabro run-management MCP tool catalog exposed to human MCP clients: create, search, get, interact, gather, events, and pair. +`fabro_tools` defaults to `false`. Set it to `true` only for runs whose agents should be able to use the same Fabro run-management MCP tool catalog exposed to human MCP clients: workflow version registration, create, search, get, interact, gather, events, and pair. One workflow-agent exception is intentional: `fabro_run_create` always creates [child runs](/execution/child-runs) parented to the current run. If an agent supplies `parent_id`, it must match the current run ID. diff --git a/lib/apps/fabro-cli/src/commands/run/runner.rs b/lib/apps/fabro-cli/src/commands/run/runner.rs index 16776bc88..506f2d83a 100644 --- a/lib/apps/fabro-cli/src/commands/run/runner.rs +++ b/lib/apps/fabro-cli/src/commands/run/runner.rs @@ -15,6 +15,7 @@ use fabro_interview::{ WORKER_CONTROL_WS_PING_INTERVAL, WorkerControlDeliveryFrame, WorkerControlEnvelope, WorkerControlMessage, }; +use fabro_manifest::SuppliedWorkflowVersionPackager; use fabro_server::run_tool_manifest; use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer}; use fabro_tool::fabro_client::ClientBackend; @@ -236,7 +237,8 @@ fn build_fabro_run_tool_services( return None; } let backend = ClientBackend::new(Arc::new(client)) - .with_manifest_builder(Arc::new(WorkerRunManifestBuilder)); + .with_manifest_builder(Arc::new(WorkerRunManifestBuilder)) + .with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)); Some(FabroRunToolServices { backend: Arc::new(backend), current_run_id, diff --git a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs index 552134dcb..c07daff5a 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs @@ -38,6 +38,7 @@ const MCP_RUN_TOOL_NAMES: &[&str] = &[ "fabro_run_interact", "fabro_run_pair", "fabro_run_search", + "fabro_workflow_version_create", ]; async fn assert_mcp_run_tool_count(client: &McpClient) { @@ -1847,6 +1848,27 @@ async fn mcp_get_rejects_blank_run_id_before_auth_or_network() { .expect("MCP client should shut down"); } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_workflow_version_validation_happens_before_auth_or_network() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + let error = call_tool_error_text( + &client, + "fabro_workflow_version_create", + serde_json::json!({"entrypoint":"workflow","files":{}}), + ) + .await; + assert_eq!( + error, + "entrypoint `workflow` is not present in workflow files" + ); + assert_mcp_run_tool_count(&client).await; + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + #[tokio::test(flavor = "multi_thread")] async fn mcp_create_validation_errors_happen_before_auth_or_network() { let context = test_context!(); diff --git a/lib/apps/fabro-mcp-server/src/server.rs b/lib/apps/fabro-mcp-server/src/server.rs index 6a9c9941d..fba1fc5ac 100644 --- a/lib/apps/fabro-mcp-server/src/server.rs +++ b/lib/apps/fabro-mcp-server/src/server.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use std::time::Duration; use anyhow::Result; +use fabro_manifest::SuppliedWorkflowVersionPackager; use fabro_tool::fabro_client::ClientBackend; use fabro_tool::{self as run_tools, FabroToolBackend}; use fabro_util::version::FABRO_VERSION; @@ -74,7 +75,7 @@ impl ServerHandler for FabroMcpServer { fn get_info(&self) -> ServerInfo { ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) .with_server_info(Implementation::new(SERVER_NAME, FABRO_VERSION).with_title("Fabro")) - .with_instructions("Use these tools to create, inspect, control, wait for, and read events from Fabro workflow runs.") + .with_instructions("Use these tools to register workflow versions and create, inspect, control, wait for, and read events from Fabro workflow runs.") } } @@ -90,6 +91,28 @@ impl FabroMcpServer { } } + #[tool( + name = "fabro_workflow_version_create", + description = "Register supplied workflow file contents and all local dependencies as a reusable immutable workflow version ID. Obtain files with shell/read tools first; this does not create or start a run." + )] + async fn fabro_workflow_version_create( + &self, + params: Parameters, + ) -> Result { + let source = match run_tools::ValidatedWorkflowVersionCreate::try_from(params.0) { + Ok(source) => source, + Err(err) => 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_workflow_version(backend, source).await { + Ok(result) => success_result(&result, run_tools::workflow_version_create_text(&result)), + Err(err) => Ok(error_result(&err)), + } + } + #[tool( name = "fabro_run_create", description = "Create one or more Fabro workflow runs, optionally under a parent run, starting them by default." @@ -252,7 +275,10 @@ impl FabroMcpServer { .map(|client| { Arc::new( ClientBackend::new(Arc::new(client)) - .with_manifest_builder(Arc::new(McpRunManifestBuilder)), + .with_manifest_builder(Arc::new(McpRunManifestBuilder)) + .with_workflow_version_packager(Arc::new( + SuppliedWorkflowVersionPackager, + )), ) as Arc }) .map_err(|err| run_tools::ToolError::from_anyhow(&err)) @@ -283,6 +309,7 @@ fn error_result(err: &run_tools::ToolError) -> CallToolResult { #[cfg(test)] mod tests { + use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Arc; @@ -291,6 +318,63 @@ mod tests { use super::*; use crate::FabroMcpServerSettings; + #[tokio::test] + async fn workflow_version_mcp_surface_matches_catalog_and_returns_minimal_result() { + let mock = httpmock::MockServer::start_async().await; + let version = fabro_types::WorkflowVersion::new( + "workflow".parse().unwrap(), + BTreeMap::from([("workflow".parse().unwrap(), "digraph W {}".to_string())]), + BTreeMap::new(), + ) + .unwrap(); + let id = version.id().unwrap(); + let upload = mock + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/api/v1/workflow-versions") + .json_body_obj(&version); + then.status(201) + .json_body(serde_json::json!({"workflow_version_id":id})); + }) + .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) }) + }), + })); + let tools = server.tool_router.list_all(); + let tool = tools + .iter() + .find(|tool| tool.name == "fabro_workflow_version_create") + .unwrap(); + let definition = run_tools::tool_definitions() + .iter() + .find(|tool| tool.name == "fabro_workflow_version_create") + .unwrap(); + let mut expected = definition.parameters.clone(); + expected.as_object_mut().unwrap().remove("$schema"); + let mut actual = Value::Object(tool.input_schema.as_ref().clone()); + actual.as_object_mut().unwrap().remove("$schema"); + assert_eq!(actual, expected); + assert_eq!(tool.description.as_deref(), Some(definition.description)); + let params = + serde_json::json!({"entrypoint":"workflow","files":{"workflow":"digraph W {}"}}); + let result = server + .fabro_workflow_version_create(Parameters(serde_json::from_value(params).unwrap())) + .await + .unwrap(); + assert_eq!( + result.structured_content, + Some(serde_json::json!({"workflow_version_id":id})) + ); + assert_ne!(result.is_error, Some(true)); + upload.assert_calls_async(1).await; + } + #[test] fn server_info_reports_fabro_version() { let settings = FabroMcpServerSettings { diff --git a/lib/apps/fabro-server/src/server/handler/workflow_versions.rs b/lib/apps/fabro-server/src/server/handler/workflow_versions.rs index 69845679b..cd8daafc7 100644 --- a/lib/apps/fabro-server/src/server/handler/workflow_versions.rs +++ b/lib/apps/fabro-server/src/server/handler/workflow_versions.rs @@ -10,8 +10,9 @@ use fabro_workflow_version::{ }; use super::super::{ - ApiError, AppState, IntoResponse, Json, RequiredUser, Response, Router, State, StatusCode, post, + ApiError, AppState, IntoResponse, Json, Response, Router, State, StatusCode, post, }; +use crate::principal_middleware::RequiredRunManagementActor; const INVALID_JSON_CODE: &str = "invalid_json"; const INVALID_VERSION_CODE: &str = "workflow_version_invalid"; @@ -26,7 +27,7 @@ pub(super) fn routes() -> Router> { } async fn create_workflow_version( - _auth: RequiredUser, + _auth: RequiredRunManagementActor, State(state): State>, payload: Result, JsonRejection>, ) -> Result { diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 3be6e1a29..a0a4533f6 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -19738,3 +19738,77 @@ fn validate_github_slug_rejects_overlong() { let long = "a".repeat(40); assert!(super::validate_github_slug("owner", &long, 39).is_err()); } + +#[tokio::test] +async fn workflow_version_registration_requires_user_or_run_tools_capability() { + let (state, app) = jwt_auth_app(); + let run_id = RunId::new(); + let body = json!({ + "entrypoint": "workflow.fabro", + "files": {"workflow.fabro": "digraph W {}"}, + "workflow_dependencies": {}, + }); + for (token, expected) in [ + (issue_test_user_jwt(), StatusCode::CREATED), + ( + issue_test_run_tools_worker_token(&run_id), + StatusCode::CREATED, + ), + (issue_test_worker_token(&run_id), StatusCode::FORBIDDEN), + ] { + let response = app + .clone() + .oneshot(json_bearer_request( + Method::POST, + "/workflow-versions", + &token, + &body, + )) + .await + .unwrap(); + fabro_test::expect_axum_status(response, expected, "POST /workflow-versions actor matrix") + .await; + } + for body in [serde_json::to_string(&body).unwrap(), "{".to_string()] { + let response = app + .clone() + .oneshot( + Request::builder() + .method(Method::POST) + .uri(api("/workflow-versions")) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(body)) + .unwrap(), + ) + .await + .unwrap(); + fabro_test::expect_axum_status( + response, + StatusCode::UNAUTHORIZED, + "anonymous POST /workflow-versions", + ) + .await; + } + let response = app + .oneshot(bearer_request( + Method::GET, + "/runs", + &issue_test_user_jwt(), + Body::empty(), + )) + .await + .unwrap(); + let listed = + fabro_test::expect_axum_json(response, StatusCode::OK, "GET /runs after registration") + .await; + assert_eq!(listed["data"], json!([])); + assert!( + state + .stores + .runs + .load_run_projection(&run_id) + .await + .unwrap() + .is_none() + ); +} diff --git a/lib/components/fabro-manifest/Cargo.toml b/lib/components/fabro-manifest/Cargo.toml index 1ac7e1eca..edc222c0f 100644 --- a/lib/components/fabro-manifest/Cargo.toml +++ b/lib/components/fabro-manifest/Cargo.toml @@ -14,22 +14,27 @@ workspace = true [dependencies] anyhow.workspace = true +async-trait.workspace = true fabro-api = { path = "../../foundation/fabro-api" } fabro-config = { path = "../../foundation/fabro-config" } fabro-github = { path = "../fabro-github" } fabro-graphviz = { path = "../fabro-graphviz" } fabro-template = { path = "../../foundation/fabro-template" } +fabro-tool = { path = "../fabro-tool" } fabro-types = { path = "../../foundation/fabro-types" } +fabro-util = { path = "../../foundation/fabro-util" } fabro-workflow = { path = "../fabro-workflow" } fabro-workflow-version = { path = "../fabro-workflow-version" } git2.workspace = true +tempfile = "3" thiserror.workspace = true +tokio.workspace = true toml.workspace = true +tracing.workspace = true [dev-dependencies] fabro-test.workspace = true -fabro-util = { path = "../../foundation/fabro-util" } +tracing-subscriber.workspace = true insta.workspace = true serde_json.workspace = true -tempfile = "3" temp-env = "0.3" diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index db317abb1..f020b5828 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -4,8 +4,12 @@ )] mod local_workflow_package; +mod supplied_workflow; +#[cfg(test)] +mod test_support; mod workflow_bundler; mod workflow_version_collector; +mod workflow_version_packager; use std::collections::HashMap; use std::path::{Component, Path, PathBuf}; @@ -30,14 +34,18 @@ use fabro_types::{ WorkflowSettings, }; use fabro_workflow::git::{self, GitSyncStatus}; +pub use fabro_workflow_version::CollectedWorkflowClosure; pub use crate::local_workflow_package::{ LocalWorkflowPackageError, ResolvedLocalWorkflowPackage, resolve_local_workflow_package, }; +pub use crate::supplied_workflow::collect_supplied_workflow_versions; use crate::workflow_bundler::WorkflowBundler; pub use crate::workflow_version_collector::{ - CollectedWorkflowClosure, WorkflowVersionCollectError, collect_workflow_versions, + MAX_WORKFLOW_VERSION_DEPTH, WorkflowVersionCollectError, collect_workflow_versions, + collect_workflow_versions_at_location, }; +pub use crate::workflow_version_packager::SuppliedWorkflowVersionPackager; #[derive(Debug, Default)] pub struct ManifestBuildInput { diff --git a/lib/components/fabro-manifest/src/supplied_workflow.rs b/lib/components/fabro-manifest/src/supplied_workflow.rs new file mode 100644 index 000000000..64f824328 --- /dev/null +++ b/lib/components/fabro-manifest/src/supplied_workflow.rs @@ -0,0 +1,439 @@ +//! Package workflow versions from caller-supplied file contents instead of a +//! checkout on disk. + +use std::collections::BTreeMap; +use std::path::Path; + +use fabro_config::project::WorkflowLocation; +use fabro_types::WorkflowPath; +use tempfile::TempDir; + +use crate::{CollectedWorkflowClosure, WorkflowVersionCollectError}; + +type Result = std::result::Result; + +/// Stage `files` in a private temporary directory and collect the workflow +/// closure rooted at `entrypoint` with the same collector used for checkouts. +/// Only supplied files can satisfy references; the staging directory is +/// removed on every return path. Every dependency is validated before this +/// returns and nothing is registered. +pub fn collect_supplied_workflow_versions( + entrypoint: &WorkflowPath, + files: &BTreeMap, +) -> Result { + let staging = tempfile::Builder::new() + .prefix("fabro-workflow-version-") + .tempdir() + .map_err(stage_error)?; + collect_in_staging(entrypoint, files, &staging) +} + +fn stage_error(source: std::io::Error) -> WorkflowVersionCollectError { + WorkflowVersionCollectError::Stage { source } +} + +fn collect_in_staging( + entrypoint: &WorkflowPath, + files: &BTreeMap, + staging: &TempDir, +) -> Result { + let root = staging.path().canonicalize().map_err(stage_error)?; + for (path, contents) in files { + let destination = root.join(path.as_str()); + if let Some(parent) = destination.parent() { + std::fs::create_dir_all(parent).map_err(stage_error)?; + } + std::fs::write(destination, contents).map_err(stage_error)?; + } + let entrypoint = Path::new(entrypoint.as_str()); + let location = + WorkflowLocation::from_exact_path(entrypoint, &root).map_err(|source| match source { + fabro_config::Error::WorkflowNotFound(_) => { + WorkflowVersionCollectError::WorkflowNotFound { + path: entrypoint.to_path_buf(), + } + } + source => WorkflowVersionCollectError::Collect { + path: entrypoint.to_path_buf(), + source: source.into(), + }, + })?; + let closure = crate::collect_workflow_versions_at_location(&location, &root, entrypoint)?; + for (_, version) in closure.versions() { + confine_to_supplied(version.version(), files, &root)?; + } + Ok(closure) +} + +/// The collector resolves references against the staged tree, so its result +/// can depend on the host filesystem's case and normalization rules. Reject +/// every version whose collected files are not exactly the supplied keys, and +/// pin the one implicit probe (the sibling `workflow.toml`) to the supplied +/// map so the same request packages identically on every host. +fn confine_to_supplied( + version: &fabro_types::WorkflowVersion, + files: &BTreeMap, + root: &Path, +) -> Result<()> { + // A supplied sibling config attaches only when its `[workflow].graph` + // selects this entrypoint, exactly as for a checkout; several graphs may + // share one directory. When no exact sibling was supplied, the probe must + // not find one either. + let config_path = version.config_path(); + if !files.contains_key(&config_path) { + let alias = files.keys().find(|path| { + fabro_types::validate_workflow_source_paths([*path, &config_path]).is_err() + }); + if let Some(alias) = alias { + // A case-insensitive host would probe this key as the config and + // a case-sensitive one would not; neither outcome is what was + // asked for. + return Err(WorkflowVersionCollectError::ConfigAlias { + config_path, + alias: alias.clone(), + }); + } + } else if !version.files().contains_key(&config_path) { + // Graph discovery deliberately ignores sibling configs that it cannot + // load. A supplied config may be omitted only if it is valid and selects + // a different graph; malformed settings must not silently disappear. + WorkflowLocation::from_exact_path(Path::new(config_path.as_str()), root).map_err( + |source| WorkflowVersionCollectError::InvalidSuppliedConfig { + path: config_path, + source: Box::new(source), + }, + )?; + } + // A case-insensitive host must not satisfy a reference that is missing + // from the supplied tree under its exact key. + for path in version.files().keys() { + if !files.contains_key(path) { + return Err(WorkflowVersionCollectError::NotSupplied { path: path.clone() }); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use fabro_tool::ValidatedWorkflowVersionCreate as Supplied; + use fabro_util::error::collect_chain; + + use super::*; + use crate::test_support::{fixture, source as supplied}; + + fn collect(input: &Supplied) -> CollectedWorkflowClosure { + collect_supplied_workflow_versions(&input.entrypoint, &input.files).unwrap() + } + + /// Consumes `staging` so the tests can assert cleanup after return. + fn collect_with_staging( + input: &Supplied, + staging: TempDir, + ) -> Result { + let result = collect_in_staging(&input.entrypoint, &input.files, &staging); + drop(staging); + result + } + + #[test] + fn supplied_content_matches_checkout_collector_and_cleans_staging() { + for input in [ + supplied("workflow.fabro", &[("workflow.fabro", "digraph W {}")]), + fixture(), + ] { + let source = tempfile::tempdir().unwrap(); + for (path, content) in &input.files { + std::fs::write(source.path().join(path.as_str()), content).unwrap(); + } + let expected = crate::collect_workflow_versions( + Path::new(input.entrypoint.as_str()), + source.path(), + ) + .unwrap(); + let staging = tempfile::tempdir().unwrap(); + let path = staging.path().to_owned(); + let actual = collect_with_staging(&input, staging).unwrap(); + assert!(!path.exists()); + assert_eq!(actual.root_id(), expected.root_id()); + assert_eq!( + actual + .versions() + .map(|(_, v)| v.version()) + .collect::>(), + expected + .versions() + .map(|(_, v)| v.version()) + .collect::>() + ); + } + } + + #[test] + fn exact_extensionless_entrypoint_and_child_ignore_selectors() { + let input = supplied("workflow", &[ + ( + "workflow", + r#"digraph W { child [stack.child_workflow="child"] }"#, + ), + ("child", "digraph Child {}"), + ( + ".fabro/project.toml", + "malformed project config must not be read", + ), + ( + ".fabro/workflows/workflow/workflow.toml", + "misleading named workflow", + ), + ]); + let closure = collect(&input); + let versions = closure.versions().collect::>(); + assert_eq!(versions.len(), 2); + assert_eq!(versions[1].1.version().entrypoint().as_str(), "workflow"); + assert_eq!(versions[0].1.version().entrypoint().as_str(), "child"); + } + + #[test] + fn rejects_missing_and_escaping_references_and_cleans_failure() { + let parent = tempfile::tempdir().unwrap(); + std::fs::write( + parent.path().join("outside.md"), + "host content must never satisfy a reference", + ) + .unwrap(); + std::fs::write(parent.path().join("child.fabro"), "digraph Host {}").unwrap(); + // Malformed on purpose: a parser that reaches this file would quote it. + std::fs::write( + parent.path().join("secret.toml"), + "HOST_SECRET = [unterminated", + ) + .unwrap(); + std::fs::write( + parent.path().join("workflow.toml"), + "HOST_SECRET = [unterminated", + ) + .unwrap(); + for (index, input) in [ + supplied("workflow.fabro", &[ + ("workflow.fabro", r#"digraph W { p [prompt="@prompt.md"] }"#), + ("Prompt.md", "wrong case"), + ]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [prompt="@../outside.md"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [prompt="@sub/../../outside.md"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [prompt="@outside.md"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [output_schema="@../outside.md"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [stack.child_workflow="../child.fabro"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [stack.child_workflow="sub/../../child.fabro"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [stack.child_workflow="../secret.toml"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [stack.child_workflow="../graph.fabro"] }"#, + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + r#"digraph W { p [stack.child_workflow="missing"] }"#, + )]), + supplied("workflow.toml", &[( + "workflow.toml", + "_version = 1\n[workflow]\ngraph = \"../child.fabro\"\n", + )]), + supplied("workflow.fabro", &[( + "workflow.fabro", + "invalid source PRIVATE_CONTENT", + )]), + ] + .into_iter() + .enumerate() + { + let staging = tempfile::tempdir_in(parent.path()).unwrap(); + let path = staging.path().to_owned(); + let error = collect_with_staging(&input, staging) + .err() + .unwrap_or_else(|| panic!("accepted invalid fixture {index}")); + let rendered = collect_chain(&error).join(": "); + // Escaping references must fail before any host file is opened, + // so no host diagnostic (parse error, exists-vs-missing) leaks. + assert!( + !rendered.contains("HOST_SECRET") && !rendered.contains("secret.toml:"), + "fixture {index} read a host file: {rendered}" + ); + assert!(!path.exists()); + } + } + + #[test] + fn config_entrypoint_must_be_workflow_toml_beside_its_graph() { + let config = "_version = 1\n[workflow]\ngraph = \"g.fabro\"\n[run]\ngoal = \"hello\"\n"; + let accepted = supplied("sub/workflow.toml", &[ + ("sub/workflow.toml", config), + ("sub/g.fabro", "digraph W {}"), + ]); + let root = collect(&accepted) + .versions() + .last() + .unwrap() + .1 + .version() + .clone(); + assert!(root.files().contains_key(&root.config_path())); + + // Same tree under another config name: runtime would never read it. + let renamed = supplied("sub/run.toml", &[ + ("sub/run.toml", config), + ("sub/g.fabro", "digraph W {}"), + ]); + let error = + collect_supplied_workflow_versions(&renamed.entrypoint, &renamed.files).unwrap_err(); + let rendered = collect_chain(&error).join(": "); + assert!( + rendered.contains("must be `sub/workflow.toml`"), + "{rendered}" + ); + // A config that selects a graph in another directory is not its sibling. + let elsewhere = supplied("sub/workflow.toml", &[ + ( + "sub/workflow.toml", + "_version = 1\n[workflow]\ngraph = \"../g.fabro\"\n", + ), + ("g.fabro", "digraph W {}"), + ]); + assert!( + collect_supplied_workflow_versions(&elsewhere.entrypoint, &elsewhere.files).is_err() + ); + } + + #[test] + fn sibling_config_resolution_does_not_depend_on_the_host_filesystem() { + let graph = r#"digraph W { child [stack.child_workflow="sub/child.fabro"] }"#; + let selects = |target: &str| format!("_version = 1\n[workflow]\ngraph = \"{target}\"\n"); + + // Exact sibling configs attach to the root and to the child. + let attached = supplied("workflow.fabro", &[ + ("workflow.fabro", graph), + ("workflow.toml", &selects("workflow.fabro")), + ("sub/child.fabro", "digraph Child {}"), + ("sub/workflow.toml", &selects("child.fabro")), + ]); + for (_, version) in collect(&attached).versions() { + let version = version.version(); + assert!( + version.files().contains_key(&version.config_path()), + "{} lost its config", + version.entrypoint() + ); + } + + // A case variant of the implicit probe name is rejected everywhere, + // instead of attaching on APFS and vanishing on ext4. + for (entrypoint, files) in [ + ("workflow.fabro", vec![ + ("workflow.fabro", graph.to_owned()), + ("Workflow.toml", selects("workflow.fabro")), + ("sub/child.fabro", "digraph Child {}".to_owned()), + ]), + ("workflow.fabro", vec![ + ("workflow.fabro", graph.to_owned()), + ("sub/child.fabro", "digraph Child {}".to_owned()), + ("sub/WORKFLOW.toml", selects("child.fabro")), + ]), + ] { + let files = files + .iter() + .map(|(path, content)| (*path, content.as_str())) + .collect::>(); + let input = supplied(entrypoint, &files); + let error = + collect_supplied_workflow_versions(&input.entrypoint, &input.files).unwrap_err(); + assert!( + matches!(error, WorkflowVersionCollectError::ConfigAlias { .. }), + "{error}" + ); + } + + // Several graphs may share a directory: a sibling config attaches only + // to the graph it selects, as for a checkout. + let shared_dir = supplied("workflow.fabro", &[ + ("workflow.fabro", graph), + ("sub/child.fabro", "digraph Child {}"), + ("sub/other.fabro", "digraph Other {}"), + ("sub/workflow.toml", &selects("other.fabro")), + ]); + let closure = collect(&shared_dir); + let (_, child) = closure.versions().next().unwrap(); + assert_eq!(child.version().entrypoint().as_str(), "sub/child.fabro"); + assert!( + !child + .version() + .files() + .contains_key(&child.version().config_path()) + ); + } + + #[test] + fn preserves_literal_scripts_without_executing() { + let directory = tempfile::tempdir().unwrap(); + let marker = directory.path().join("must-not-exist"); + // Script is literal command text in Fabro, not an @file import. + let graph = format!( + "digraph W {{ command [script=\"touch {}\"] }}", + marker.display() + ); + let input = supplied("workflow", &[("workflow", &graph)]); + let closure = collect(&input); + let root = closure.versions().last().unwrap().1.version(); + assert_eq!(root.files()[&"workflow".parse().unwrap()], graph); + assert!(!marker.exists()); + } + + #[test] + fn map_order_is_irrelevant_and_reachable_changes_change_ids() { + let input = fixture(); + let first = collect(&input); + let mut reordered = Supplied { + entrypoint: input.entrypoint.clone(), + files: input.files.into_iter().rev().collect(), + }; + assert_eq!(first.root_id(), collect(&reordered).root_id()); + reordered + .files + .insert("prompt.md".parse().unwrap(), "changed".into()); + let changed = collect(&reordered); + assert_ne!(first.root_id(), changed.root_id()); + assert_eq!( + first.versions().next().unwrap().0, + changed.versions().next().unwrap().0 + ); + reordered + .files + .insert("child.fabro".parse().unwrap(), "digraph Changed {}".into()); + let changed_child = collect(&reordered); + assert_ne!(changed.root_id(), changed_child.root_id()); + assert_ne!( + changed.versions().next().unwrap().0, + changed_child.versions().next().unwrap().0 + ); + } +} diff --git a/lib/components/fabro-manifest/src/test_support.rs b/lib/components/fabro-manifest/src/test_support.rs new file mode 100644 index 000000000..8ee12307a --- /dev/null +++ b/lib/components/fabro-manifest/src/test_support.rs @@ -0,0 +1,29 @@ +use fabro_tool::ValidatedWorkflowVersionCreate; + +pub(super) fn source(entrypoint: &str, files: &[(&str, &str)]) -> ValidatedWorkflowVersionCreate { + ValidatedWorkflowVersionCreate { + entrypoint: entrypoint.parse().unwrap(), + files: files + .iter() + .map(|(path, content)| (path.parse().unwrap(), (*content).to_string())) + .collect(), + } +} + +pub(super) fn fixture() -> ValidatedWorkflowVersionCreate { + source("workflow.toml", &[ + ( + "workflow.toml", + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ), + ( + "workflow.fabro", + r#"digraph W { p [prompt="@prompt.md"] child [stack.child_workflow="child.fabro"] }"#, + ), + ( + "prompt.md", + "Keep {{ secrets.TEST }} and {{ env.TEST }} for runtime.", + ), + ("child.fabro", "digraph Child {}"), + ]) +} diff --git a/lib/components/fabro-manifest/src/workflow_bundler.rs b/lib/components/fabro-manifest/src/workflow_bundler.rs index e01eb629d..da1588bcd 100644 --- a/lib/components/fabro-manifest/src/workflow_bundler.rs +++ b/lib/components/fabro-manifest/src/workflow_bundler.rs @@ -17,7 +17,10 @@ use fabro_template::{ use fabro_types::ManifestPath; use fabro_types::graph::ReferenceKind; -use crate::{manifest_path_from_absolute, normalize_absolute_path}; +use crate::{ + WorkflowVersionCollectError, manifest_path_from_absolute, normalize_absolute_path, + workflow_version_collector, +}; pub(super) struct WorkflowBundler<'a> { package_root: &'a Path, @@ -55,7 +58,7 @@ impl<'a> WorkflowBundler<'a> { workflow: &Path, project_config: Option<(&ManifestPath, &str)>, ) -> Result> { - let root_key = self.collect_workflow_entry(workflow, self.package_root)?; + let root_key = self.collect_workflow_entry(workflow, self.package_root, 1)?; if let Some((config_path, source)) = project_config { let mut root = self @@ -80,7 +83,7 @@ impl<'a> WorkflowBundler<'a> { root: &WorkflowLocation, ) -> Result { self.workflow_version_projection = true; - let root_key = self.collect_workflow_location(root)?; + let root_key = self.collect_workflow_location(root, 1)?; Ok(CollectedWorkflowSources { root_key, workflows: self.workflows, @@ -88,18 +91,38 @@ impl<'a> WorkflowBundler<'a> { } /// Collects the workflow at `location` and returns its manifest key. - fn collect_workflow_location(&mut self, location: &WorkflowLocation) -> Result { + fn collect_workflow_location( + &mut self, + location: &WorkflowLocation, + depth: usize, + ) -> Result { let dot_path = manifest_path_from_absolute(&location.graph, self.package_root)?; let dot_key = dot_path.to_string(); if !self.visited_workflows.insert(dot_key.clone()) { return Ok(dot_key); } + if self.workflow_version_projection { + workflow_version_collector::check_workflow_depth(depth, &dot_key)?; + } let source = self.read_package_file(&location.graph)?; let config = if let Some(workflow_toml_path) = location.toml.as_ref() { + let config_path = manifest_path_from_absolute(workflow_toml_path, self.package_root)?; + if self.workflow_version_projection { + // A version's config is read at run time from the fixed + // sibling path only, so a config file under any other name + // would be registered and then silently ignored. + let expected = dot_path.parent_or_dot().join("workflow.toml"); + if config_path.as_path() != expected { + bail!( + "workflow configuration `{config_path}` must be `{}` beside its graph \ + `{dot_path}`", + expected.display() + ); + } + } Some(types::ManifestWorkflowConfig { - path: manifest_path_from_absolute(workflow_toml_path, self.package_root)? - .to_string(), + path: config_path.to_string(), source: self.read_package_file(workflow_toml_path)?, }) } else { @@ -125,6 +148,7 @@ impl<'a> WorkflowBundler<'a> { &mut visited_imports, &mut dependency_keys, GraphPosition::Entrypoint, + depth, )?; self.workflows @@ -143,9 +167,18 @@ impl<'a> WorkflowBundler<'a> { /// Relative workflow references with an extension are lexically /// normalized (`..` segments resolved without consulting the filesystem, /// `~` rejected) before resolution, so the file read matches the manifest - /// key. Returns the collected workflow's manifest key. - fn collect_workflow_entry(&mut self, workflow: &Path, resolve_from: &Path) -> Result { - let normalized_workflow = if workflow.extension().is_some() && workflow.is_relative() { + /// key. Workflow-version projection normalizes every reference and + /// resolves it as an exact path inside the package root, with no + /// workflow-name lookup. Returns the collected workflow's manifest key. + fn collect_workflow_entry( + &mut self, + workflow: &Path, + resolve_from: &Path, + depth: usize, + ) -> Result { + let normalize = self.workflow_version_projection + || (workflow.extension().is_some() && workflow.is_relative()); + let normalized = if normalize { normalize_absolute_path(resolve_from, &workflow.to_string_lossy()).ok_or_else(|| { anyhow!( "unsupported manifest workflow reference: {}", @@ -155,8 +188,23 @@ impl<'a> WorkflowBundler<'a> { } else { workflow.to_path_buf() }; - let location = WorkflowLocation::resolve(&normalized_workflow, resolve_from)?; - self.collect_workflow_location(&location) + let location = if self.workflow_version_projection { + // Location resolution probes and parses config files, so reject + // references that leave the package root before it can touch a + // host file. `ManifestPath::from_absolute` accepts `..`-prefixed + // results and is not a containment check. + if !normalized.starts_with(self.package_root) { + bail!( + "workflow reference `{}` escapes source root `{}`", + workflow.display(), + self.package_root.display() + ); + } + WorkflowLocation::from_exact_path(&normalized, self.package_root)? + } else { + WorkflowLocation::resolve(&normalized, resolve_from)? + }; + self.collect_workflow_location(&location, depth) } fn collect_workflow_files( @@ -166,7 +214,14 @@ impl<'a> WorkflowBundler<'a> { visited_imports: &mut HashSet, dependency_keys: &mut BTreeSet, position: GraphPosition, + depth: usize, ) -> Result<()> { + if self.workflow_version_projection { + workflow_version_collector::check_workflow_depth( + depth, + &workflow.dot_path.to_string(), + )?; + } let graph = parser::parse(&workflow.source) .with_context(|| format!("Failed to parse {}", workflow.absolute_dot_path.display()))?; let workflow_base_dir = workflow @@ -264,12 +319,13 @@ impl<'a> WorkflowBundler<'a> { visited_imports, dependency_keys, GraphPosition::Imported, + depth + 1, )?; } } for child in children { let dependency_key = - self.collect_workflow_entry(Path::new(child), workflow_base_dir)?; + self.collect_workflow_entry(Path::new(child), workflow_base_dir, depth + 1)?; dependency_keys.insert(dependency_key); } @@ -480,11 +536,18 @@ impl<'a> WorkflowBundler<'a> { return std::fs::read_to_string(path) .with_context(|| format!("Failed to read {}", path.display())); } - let canonical = path.canonicalize().with_context(|| { - format!( + let canonical = path.canonicalize().map_err(|source| { + if source.kind() == std::io::ErrorKind::NotFound { + let path = ManifestPath::from_absolute(path, self.package_root) + .map_or_else(|| path.display().to_string(), |path| path.to_string()); + return anyhow::Error::new(WorkflowVersionCollectError::MissingPackageFile { + path, + }); + } + anyhow::Error::new(source).context(format!( "failed to canonicalize workflow package file `{}`", path.display() - ) + )) })?; if !canonical.starts_with(self.package_root) { bail!( diff --git a/lib/components/fabro-manifest/src/workflow_version_collector.rs b/lib/components/fabro-manifest/src/workflow_version_collector.rs index a0fb815b6..131f9990c 100644 --- a/lib/components/fabro-manifest/src/workflow_version_collector.rs +++ b/lib/components/fabro-manifest/src/workflow_version_collector.rs @@ -7,34 +7,34 @@ use fabro_types::{ WorkflowPath, WorkflowPathParseError, WorkflowVersion, WorkflowVersionId, WorkflowVersionShapeError, }; -use fabro_workflow_version::{ValidatedWorkflowVersion, WorkflowVersionError}; +use fabro_workflow_version::{ + CollectedWorkflowClosure, ValidatedWorkflowVersion, WorkflowVersionError, +}; use thiserror::Error; use crate::workflow_bundler::{CollectedWorkflowSource, CollectedWorkflowSources, WorkflowBundler}; -/// One locally packaged workflow-version closure in dependency-first order. -#[derive(Debug)] -pub struct CollectedWorkflowClosure { - root_id: WorkflowVersionId, - versions: Vec<(WorkflowVersionId, ValidatedWorkflowVersion)>, -} +/// Maximum active graph nesting while collecting or assembling a version. +/// Bounds native stack use independently of file-count and byte budgets. +pub const MAX_WORKFLOW_VERSION_DEPTH: usize = 64; -impl CollectedWorkflowClosure { - #[must_use] - pub fn root_id(&self) -> WorkflowVersionId { - self.root_id - } - - /// Iterate over every unique version with dependencies before parents. - pub fn versions( - &self, - ) -> impl Iterator + '_ { - self.versions.iter().map(|(id, version)| (*id, version)) +pub(super) fn check_workflow_depth( + depth: usize, + path: &str, +) -> Result<(), WorkflowVersionCollectError> { + if depth > MAX_WORKFLOW_VERSION_DEPTH { + return Err(WorkflowVersionCollectError::DepthExceeded { + path: path.to_owned(), + maximum: MAX_WORKFLOW_VERSION_DEPTH, + }); } + Ok(()) } #[derive(Debug, Error)] pub enum WorkflowVersionCollectError { + #[error("workflow dependency nesting at `{path}` exceeds {maximum} levels")] + DepthExceeded { path: String, maximum: usize }, #[error("workflow `{path}` was not found")] WorkflowNotFound { path: PathBuf }, #[error("failed to collect workflow `{path}`")] @@ -70,6 +70,34 @@ pub enum WorkflowVersionCollectError { DependencyCycle { path: WorkflowPath }, #[error("collected workflow dependency `{path}` is missing")] MissingWorkflow { path: String }, + /// A referenced file is absent from the package root. Surfaced separately + /// from [`Self::Collect`] because the path is the whole message. + #[error("referenced file `{path}` is missing from the workflow source")] + MissingPackageFile { path: String }, + /// Supplied-content packaging: the collector read a file whose exact key + /// the caller did not supply (a case-insensitive host satisfied a + /// reference that differs from the supplied key). + #[error("collected file `{path}` was not supplied")] + NotSupplied { path: WorkflowPath }, + /// Supplied-content packaging: a supplied key differs from the implicit + /// sibling config name `{config_path}` only by case or normalization, so + /// hosts with different filesystem rules would package different trees. + #[error("supplied file `{alias}` aliases the workflow config name `{config_path}`")] + ConfigAlias { + config_path: WorkflowPath, + alias: WorkflowPath, + }, + #[error("supplied workflow configuration `{path}` is invalid")] + InvalidSuppliedConfig { + path: WorkflowPath, + #[source] + source: Box, + }, + #[error("failed to stage supplied workflow files")] + Stage { + #[source] + source: std::io::Error, + }, } /// Package one workflow and every separately runnable dependency from a local @@ -137,7 +165,9 @@ pub(super) fn canonicalize_location( }) } -pub(super) fn collect_workflow_versions_at_location( +/// Collect an already resolved exact location inside a canonical package root. +/// Every dependency is validated before this returns; no registration occurs. +pub fn collect_workflow_versions_at_location( location: &WorkflowLocation, package_root: &Path, workflow: &Path, @@ -145,9 +175,13 @@ pub(super) fn collect_workflow_versions_at_location( let inputs = HashMap::new(); let collected = WorkflowBundler::new(package_root, &inputs) .collect_versions(location) - .map_err(|source| WorkflowVersionCollectError::Collect { - path: workflow.to_path_buf(), - source, + .map_err(|source| { + source + .downcast::() + .unwrap_or_else(|source| WorkflowVersionCollectError::Collect { + path: workflow.to_path_buf(), + source, + }) })?; VersionAssembler::new(collected).assemble() } @@ -185,10 +219,10 @@ impl VersionAssembler { fn assemble(mut self) -> Result { let root_key = std::mem::take(&mut self.root_key); let root_id = self.assemble_one(&root_key)?; - Ok(CollectedWorkflowClosure { + Ok(CollectedWorkflowClosure::from_dependency_order( root_id, - versions: self.versions, - }) + self.versions, + )) } fn assemble_one( @@ -198,6 +232,7 @@ impl VersionAssembler { if let Some(id) = self.ids.get(key) { return Ok(*id); } + check_workflow_depth(self.visiting.len() + 1, key)?; if !self.visiting.insert(key.to_owned()) { return Err(WorkflowVersionCollectError::DependencyCycle { path: workflow_path(key)?, @@ -303,6 +338,44 @@ mod tests { use super::*; + #[test] + fn version_assembly_bounds_its_own_dependency_traversal() { + // Collection and assembly visit shared dependencies in different orders. + // Assembly must bound its stack even if collection already cached nodes. + for count in [MAX_WORKFLOW_VERSION_DEPTH, MAX_WORKFLOW_VERSION_DEPTH + 1] { + let workflows = (0..count) + .map(|index| { + let child = (index + 1 < count).then(|| format!("f{}.fabro", index + 1)); + let source = child.as_ref().map_or_else( + || "digraph W {}".to_owned(), + |child| format!("digraph W {{ child [stack.child_workflow=\"{child}\"] }}"), + ); + (format!("f{index}.fabro"), CollectedWorkflowSource { + workflow: types::ManifestWorkflow { + config: None, + files: HashMap::new(), + source, + }, + dependency_keys: child.into_iter().collect(), + }) + }) + .collect(); + let result = VersionAssembler::new(CollectedWorkflowSources { + root_key: "f0.fabro".to_owned(), + workflows, + }) + .assemble(); + if count == MAX_WORKFLOW_VERSION_DEPTH { + assert_eq!(result.unwrap().versions().count(), count); + } else { + assert!(matches!( + result, + Err(WorkflowVersionCollectError::DepthExceeded { .. }) + )); + } + } + } + fn write(root: &Path, path: &str, content: &str) { let path = root.join(path); fs::create_dir_all(path.parent().expect("fixture path should have a parent")).unwrap(); diff --git a/lib/components/fabro-manifest/src/workflow_version_packager.rs b/lib/components/fabro-manifest/src/workflow_version_packager.rs new file mode 100644 index 000000000..566161a9a --- /dev/null +++ b/lib/components/fabro-manifest/src/workflow_version_packager.rs @@ -0,0 +1,349 @@ +//! Application adapter that packages caller-supplied workflow contents for +//! the `fabro_workflow_version_create` tool. + +use anyhow::Context as _; +use async_trait::async_trait; +use fabro_tool::{ToolError, ValidatedWorkflowVersionCreate, WorkflowVersionPackager}; +use fabro_util::error::collect_chain; +use fabro_workflow_version::{CollectedWorkflowClosure, WorkflowVersionError}; +use tokio::task; +use tracing::debug; + +use crate::WorkflowVersionCollectError; + +/// Packages supplied workflow contents for standalone MCP and capable run +/// workers; the backend that owns the API client performs registration. +pub struct SuppliedWorkflowVersionPackager; + +const PACKAGING_HINT: &str = "check configuration, syntax, local references, and package limits"; + +#[async_trait] +impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager { + async fn package( + &self, + source: ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + let packaged = task::spawn_blocking(move || package_blocking(&source)) + .await + .context("workflow packaging task failed")??; + Ok(packaged) + } +} + +/// Stage, collect, and validate on the calling thread. +/// +/// Packaging failures are expected input errors, so they log at DEBUG. The +/// event carries the collector error's own message only: parser and template +/// diagnostics further down the chain quote supplied file contents, which +/// must not reach the log at any level. +fn package_blocking( + source: &ValidatedWorkflowVersionCreate, +) -> Result { + let closure = crate::collect_supplied_workflow_versions(&source.entrypoint, &source.files) + .map_err(|err| { + debug!( + entrypoint = %source.entrypoint, + file_count = source.files.len(), + error = %err, + "workflow version packaging failed" + ); + ToolError::message(render_packaging_error(&err)) + })?; + Ok(closure) +} + +/// Render a packaging failure for the tool caller. Every collector variant's +/// own message names paths and counts only, so most render their full cause +/// chain and the caller can fix the input. The graph parser, TOML parser, and +/// template engine quote the offending source in their diagnostics, so +/// failures that reach them stop at the last path-only level and add a hint. +fn render_packaging_error(err: &WorkflowVersionCollectError) -> String { + let quotes_source = match err { + WorkflowVersionCollectError::Collect { .. } + | WorkflowVersionCollectError::InvalidSuppliedConfig { .. } => true, + WorkflowVersionCollectError::InvalidVersion { source, .. } => matches!( + source, + WorkflowVersionError::GraphParse { .. } + | WorkflowVersionError::Template { .. } + | WorkflowVersionError::Config { .. } + ), + _ => false, + }; + if !quotes_source { + return collect_chain(err).join(": "); + } + let summary = match err { + // `WorkflowVersionError` names the offending path; only its source + // quotes content. + WorkflowVersionCollectError::InvalidVersion { source, .. } => format!("{err}: {source}"), + _ => err.to_string(), + }; + format!("{summary}; {PACKAGING_HINT}") +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + #[expect( + clippy::disallowed_types, + reason = "test log capture writes synchronously into memory" + )] + use std::io::Write; + use std::sync::{Arc, Mutex}; + + use tracing::{Level, subscriber}; + + use super::*; + + #[derive(Clone, Default)] + struct CapturedLog(Arc>>); + + impl Write for CapturedLog { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl CapturedLog { + fn text(&self) -> String { + String::from_utf8(self.0.lock().unwrap().clone()).unwrap() + } + } + + use crate::test_support::{fixture, source}; + + async fn package_error(input: ValidatedWorkflowVersionCreate) -> String { + let error = SuppliedWorkflowVersionPackager + .package(input) + .await + .unwrap_err(); + format!("{error:#}") + } + + #[tokio::test] + async fn deeply_nested_graphs_return_errors_on_the_packaging_thread() { + for kind in ["child", "import", "mixed"] { + for count in [ + crate::MAX_WORKFLOW_VERSION_DEPTH, + crate::MAX_WORKFLOW_VERSION_DEPTH + 1, + 384, + 512, + ] { + let files: BTreeMap<_, _> = (0..count) + .map(|index| { + let attribute = if kind == "import" || (kind == "mixed" && index % 2 == 0) { + "import" + } else { + "stack.child_workflow" + }; + let graph = if index + 1 < count { + format!( + "digraph W {{ node{index} [{attribute}=\"f{}.fabro\"] }}", + index + 1 + ) + } else { + "digraph W {}".to_owned() + }; + (format!("f{index}.fabro").parse().unwrap(), graph) + }) + .collect(); + let input = ValidatedWorkflowVersionCreate::try_from( + fabro_tool::FabroWorkflowVersionCreateParams { + entrypoint: "f0.fabro".parse().unwrap(), + files, + }, + ) + .unwrap(); + let result = SuppliedWorkflowVersionPackager.package(input).await; + if count == crate::MAX_WORKFLOW_VERSION_DEPTH { + assert!(result.is_ok(), "{kind} at limit: {result:?}"); + } else { + let error = result.unwrap_err(); + assert!( + error.to_string().contains("exceeds 64 levels"), + "{kind}/{count}: {error:#}" + ); + } + } + } + } + + #[tokio::test] + async fn invalid_supplied_sibling_configs_fail_without_quoting_source() { + for config in [ + "_version = 1\nPRIVATE_CONTENT = [unterminated", + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run]\ngoal = \"PRIVATE_CONTENT\"\nunknown_setting = true\n", + ] { + for child in [false, true] { + let input = if child { + source("root.fabro", &[ + ( + "root.fabro", + r#"digraph W { child [stack.child_workflow="sub/workflow.fabro"] }"#, + ), + ("sub/workflow.fabro", "digraph Child {}"), + ("sub/workflow.toml", config), + ]) + } else { + source("workflow.fabro", &[ + ("workflow.fabro", "digraph W {}"), + ("workflow.toml", config), + ]) + }; + let error = package_error(input).await; + let path = if child { + "sub/workflow.toml" + } else { + "workflow.toml" + }; + assert!( + error.contains(&format!( + "supplied workflow configuration `{path}` is invalid" + )), + "{error}" + ); + assert!(!error.contains("PRIVATE_CONTENT"), "{error}"); + } + } + } + + #[tokio::test] + async fn packager_returns_dependencies_before_root() { + let packaged = SuppliedWorkflowVersionPackager + .package(fixture()) + .await + .unwrap(); + let versions = packaged + .versions() + .map(|(_, v)| v.version()) + .collect::>(); + assert_eq!(versions.len(), 2); + assert_eq!(versions[0].entrypoint().as_str(), "child.fabro"); + assert_eq!( + versions[1].id().unwrap(), + packaged.root_id(), + "root version must be last" + ); + let child_id = versions[0].id().unwrap(); + assert!( + versions[1] + .workflow_dependencies() + .values() + .any(|id| *id == child_id) + ); + } + + #[tokio::test] + async fn packaging_errors_never_quote_supplied_source() { + let mut invalid_root = fixture(); + // Child is valid, but the root fails after its dependency is assembled. + invalid_root.files.insert( + "workflow.toml".parse().unwrap(), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.goal]\nfile = \"missing.md\"" + .into(), + ); + let mut invalid_config = fixture(); + invalid_config.files.insert( + "workflow.toml".parse().unwrap(), + "_version = 1\nPRIVATE_CONTENT = [unterminated".into(), + ); + for input in [ + invalid_root, + invalid_config, + source("workflow", &[( + "workflow", + "PRIVATE_CONTENT invalid source", + )]), + ] { + let rendered = package_error(input).await; + assert!(!rendered.contains("PRIVATE_CONTENT"), "{rendered}"); + } + } + + #[test] + fn packaging_failure_log_never_carries_supplied_source() { + let log = CapturedLog::default(); + let writer = log.clone(); + let subscriber = tracing_subscriber::fmt() + .with_max_level(Level::TRACE) + .with_ansi(false) + .with_writer(move || writer.clone()) + .finish(); + let inputs = [ + source("workflow", &[( + "workflow", + "PRIVATE_CONTENT invalid source", + )]), + source("workflow.toml", &[( + "workflow.toml", + "_version = 1\nPRIVATE_CONTENT = [unterminated", + )]), + source("workflow.fabro", &[ + ("workflow.fabro", "digraph W {}"), + ( + "workflow.toml", + "_version = 1\nPRIVATE_CONTENT = [unterminated", + ), + ]), + ]; + // The guard is load-bearing: the full chain does quote the source. + let leaky = + crate::collect_supplied_workflow_versions(&inputs[0].entrypoint, &inputs[0].files) + .unwrap_err(); + assert!(collect_chain(&leaky).join(": ").contains("PRIVATE_CONTENT")); + subscriber::with_default(subscriber, || { + for input in &inputs { + package_blocking(input).unwrap_err(); + } + }); + let text = log.text(); + assert!(text.contains("workflow version packaging failed"), "{text}"); + assert!(!text.contains("PRIVATE_CONTENT"), "{text}"); + assert!(text.contains("DEBUG"), "{text}"); + } + + #[tokio::test] + async fn path_only_failures_tell_the_caller_what_to_fix() { + let mut missing_child = fixture(); + missing_child.files.remove(&"child.fabro".parse().unwrap()); + let rendered = package_error(missing_child).await; + assert!( + rendered.contains("`child.fabro`") && rendered.contains("missing"), + "{rendered}" + ); + + let mut wrong_case = fixture(); + let prompt = wrong_case + .files + .remove(&"prompt.md".parse().unwrap()) + .unwrap(); + wrong_case + .files + .insert("Prompt.md".parse().unwrap(), prompt); + // A case-insensitive host reads the file and reports the unsupplied + // key; a case-sensitive host reports it missing. Either names the + // path the graph asked for. + let rendered = package_error(wrong_case).await; + assert!(rendered.contains("`prompt.md`"), "{rendered}"); + + let mut oversized = fixture(); + oversized.files.insert( + "prompt.md".parse().unwrap(), + "\u{1}".repeat(fabro_types::MAX_WORKFLOW_VERSION_FILE_BYTES - 1), + ); + let rendered = package_error(oversized).await; + assert!(rendered.contains("canonical bytes"), "{rendered}"); + + let rendered = package_error(source("workflow", &[( + "workflow", + "PRIVATE_CONTENT invalid source", + )])) + .await; + assert!(rendered.ends_with(PACKAGING_HINT), "{rendered}"); + } +} diff --git a/lib/components/fabro-tool/Cargo.toml b/lib/components/fabro-tool/Cargo.toml index 0d3a2862d..1e5395b9e 100644 --- a/lib/components/fabro-tool/Cargo.toml +++ b/lib/components/fabro-tool/Cargo.toml @@ -20,6 +20,7 @@ fabro-api = { path = "../../foundation/fabro-api" } fabro-client = { path = "../../foundation/fabro-client" } fabro-types = { path = "../../foundation/fabro-types" } fabro-util = { path = "../../foundation/fabro-util" } +fabro-workflow-version = { path = "../fabro-workflow-version" } futures.workspace = true schemars = "1.2.1" serde.workspace = true @@ -30,4 +31,5 @@ toml.workspace = true [dev-dependencies] fabro-types = { path = "../../foundation/fabro-types", features = ["test-support"] } +httpmock = "0.8" tempfile = "3" diff --git a/lib/components/fabro-tool/src/common.rs b/lib/components/fabro-tool/src/common.rs index 159ec426d..f154759b8 100644 --- a/lib/components/fabro-tool/src/common.rs +++ b/lib/components/fabro-tool/src/common.rs @@ -48,6 +48,13 @@ pub type ToolResult = Result; #[async_trait] pub trait FabroToolBackend: Send + Sync { + async fn create_workflow_version( + &self, + _source: crate::ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + Err(workflow_version_tool_unavailable_error()) + } + async fn create_run_from_spec( &self, spec: &crate::ValidatedCreateRunSpec, @@ -135,6 +142,13 @@ fn pair_tool_unavailable_error() -> anyhow::Error { ToolError::message(format!("{FABRO_RUN_PAIR_TOOL_NAME} is not available")).into() } +pub(crate) fn workflow_version_tool_unavailable_error() -> anyhow::Error { + ToolError::message(format!( + "{FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME} is not available" + )) + .into() +} + pub trait RunManifestBuilder: Send + Sync { fn build_run_manifest( &self, @@ -170,6 +184,7 @@ pub struct ToolDefinition { pub parameters: Value, } +pub const FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME: &str = "fabro_workflow_version_create"; pub const FABRO_RUN_CREATE_TOOL_NAME: &str = "fabro_run_create"; pub const FABRO_RUN_SEARCH_TOOL_NAME: &str = "fabro_run_search"; pub const FABRO_RUN_GET_TOOL_NAME: &str = "fabro_run_get"; @@ -180,6 +195,10 @@ pub const FABRO_RUN_PAIR_TOOL_NAME: &str = "fabro_run_pair"; static TOOL_DEFINITIONS: LazyLock> = LazyLock::new(|| { vec![ + tool_definition::( + FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME, + "Register supplied workflow file contents and all local dependencies as a reusable immutable workflow version ID. Obtain files with shell/read tools first; this does not create or start a run.", + ), tool_definition::( FABRO_RUN_CREATE_TOOL_NAME, "Create one or more Fabro workflow runs, optionally under a parent run, starting them by default.", @@ -323,6 +342,7 @@ mod tests { #[test] fn shared_tool_definitions_include_run_management_catalog() { assert_eq!(shared_tool_names(), vec![ + FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME, FABRO_RUN_CREATE_TOOL_NAME, FABRO_RUN_SEARCH_TOOL_NAME, FABRO_RUN_GET_TOOL_NAME, @@ -333,6 +353,21 @@ mod tests { ]); } + #[test] + fn workflow_version_create_has_strict_content_schema() { + let definition = tool_definitions() + .iter() + .find(|definition| definition.name == FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME) + .expect("workflow version creation should be in the shared catalog"); + let schema = &definition.parameters; + assert_eq!(schema["additionalProperties"], false); + assert_eq!(schema["properties"].as_object().unwrap().len(), 2); + assert_eq!( + schema["required"], + serde_json::json!(["entrypoint", "files"]) + ); + } + #[test] fn pair_tool_definition_exposes_pair_schema() { let definition = tool_definitions() diff --git a/lib/components/fabro-tool/src/fabro_client.rs b/lib/components/fabro-tool/src/fabro_client.rs index 59b935a85..120176183 100644 --- a/lib/components/fabro-tool/src/fabro_client.rs +++ b/lib/components/fabro-tool/src/fabro_client.rs @@ -8,13 +8,14 @@ use fabro_types::{ PairTranscriptResponse, Run, RunId, RunPairStatusResponse, RunProjection, StageId, }; -use crate::{FabroToolBackend, RunManifestBuilder, ToolError}; +use crate::{FabroToolBackend, RunManifestBuilder, ToolError, common}; #[derive(Clone)] pub struct ClientBackend { - client: Arc<::fabro_client::Client>, - manifest_builder: Option>, - run_scope: Option, + client: Arc<::fabro_client::Client>, + manifest_builder: Option>, + run_scope: Option, + workflow_version_packager: Option>, } impl ClientBackend { @@ -24,6 +25,7 @@ impl ClientBackend { client, manifest_builder: None, run_scope: None, + workflow_version_packager: None, } } @@ -33,6 +35,15 @@ impl ClientBackend { self } + #[must_use] + pub fn with_workflow_version_packager( + mut self, + packager: Arc, + ) -> Self { + self.workflow_version_packager = Some(packager); + self + } + /// Restrict this backend to a single run. /// /// Ask Fabro sessions use this with a same-run worker token so accidental @@ -55,6 +66,30 @@ impl ClientBackend { #[async_trait] impl FabroToolBackend for ClientBackend { + /// Package the supplied tree, then register dependencies before parents. + /// Versions are immutable and content-addressed, so a failed upload can be + /// retried with the same contents without cleanup. + async fn create_workflow_version( + &self, + source: crate::ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + anyhow::ensure!( + self.run_scope.is_none(), + "workflow version creation is outside this tool session's run scope" + ); + let packager = self + .workflow_version_packager + .as_ref() + .ok_or_else(common::workflow_version_tool_unavailable_error)?; + let packaged = packager.package(source).await?; + let versions = packaged + .versions() + .map(|(_, v)| v.version()) + .collect::>(); + self.client.register_workflow_versions(versions).await?; + Ok(packaged.root_id()) + } + async fn create_run_from_spec( &self, spec: &crate::ValidatedCreateRunSpec, @@ -252,3 +287,135 @@ impl FabroToolBackend for ClientBackend { .await } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use async_trait::async_trait; + use fabro_types::{WorkflowVersion, WorkflowVersionId}; + use fabro_workflow_version::{CollectedWorkflowClosure, ValidatedWorkflowVersion}; + use serde_json::json; + + use super::*; + use crate::{ValidatedWorkflowVersionCreate, WorkflowVersionPackager}; + + struct FixedPackager(Vec); + + #[async_trait] + impl WorkflowVersionPackager for FixedPackager { + async fn package( + &self, + _: ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + let versions = self + .0 + .iter() + .map(|v| Ok((v.id()?, ValidatedWorkflowVersion::new(v.clone())?))) + .collect::>>()?; + Ok(CollectedWorkflowClosure::from_dependency_order( + versions.last().unwrap().0, + versions, + )) + } + } + + fn version( + entrypoint: &str, + dependencies: BTreeMap, + ) -> WorkflowVersion { + WorkflowVersion::new( + entrypoint.parse().unwrap(), + BTreeMap::from([( + entrypoint.parse().unwrap(), + format!( + "digraph {entrypoint} {{ {} }}", + dependencies + .keys() + .map(|p| format!("child [stack.child_workflow=\"{p}\"]")) + .collect::>() + .join(" ") + ), + )]), + dependencies, + ) + .unwrap() + } + + fn source() -> ValidatedWorkflowVersionCreate { + ValidatedWorkflowVersionCreate { + entrypoint: "root".parse().unwrap(), + files: BTreeMap::new(), + } + } + + #[tokio::test] + async fn create_workflow_version_registers_packaged_closure_and_retries_after_failure() { + let server = httpmock::MockServer::start_async().await; + let child = version("child", BTreeMap::new()); + let child_id = child.id().unwrap(); + let root = version( + "root", + BTreeMap::from([("child".parse().unwrap(), child_id)]), + ); + let root_id = root.id().unwrap(); + let child_upload = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/api/v1/workflow-versions") + .json_body_obj(&child); + then.status(201) + .json_body(json!({"workflow_version_id": child_id})); + }) + .await; + let failed_root = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/api/v1/workflow-versions") + .json_body_obj(&root); + then.status(400); + }) + .await; + let client = ::fabro_client::Client::new_no_proxy(&server.url("")).unwrap(); + let backend = ClientBackend::new(Arc::new(client)) + .with_workflow_version_packager(Arc::new(FixedPackager(vec![child, root.clone()]))); + + assert!(backend.create_workflow_version(source()).await.is_err()); + child_upload.assert_calls_async(1).await; + failed_root.assert_calls_async(1).await; + + // Immutable content: retrying re-sends the child and completes the root. + failed_root.delete_async().await; + let root_upload = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/api/v1/workflow-versions") + .json_body_obj(&root); + then.status(201) + .json_body(json!({"workflow_version_id": root_id})); + }) + .await; + assert_eq!( + backend.create_workflow_version(source()).await.unwrap(), + root_id + ); + child_upload.assert_calls_async(2).await; + root_upload.assert_calls_async(1).await; + } + + #[tokio::test] + async fn create_workflow_version_without_packager_is_unavailable() { + let client = ::fabro_client::Client::new_no_proxy("http://127.0.0.1:1").unwrap(); + let error = ClientBackend::new(Arc::new(client)) + .create_workflow_version(source()) + .await + .unwrap_err(); + assert_eq!( + error.to_string(), + format!( + "{} is not available", + crate::FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME + ) + ); + } +} diff --git a/lib/components/fabro-tool/src/lib.rs b/lib/components/fabro-tool/src/lib.rs index e854c26e3..5145e7608 100644 --- a/lib/components/fabro-tool/src/lib.rs +++ b/lib/components/fabro-tool/src/lib.rs @@ -14,12 +14,13 @@ mod interact; mod manifest; mod pair; mod search; +mod workflow_version; 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, FabroToolBackend, RunManifestBuilder, RunSummaryResult, - ToolDefinition, ToolError, ToolResult, tool_definitions, + FABRO_RUN_SEARCH_TOOL_NAME, FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME, FabroToolBackend, + RunManifestBuilder, RunSummaryResult, ToolDefinition, ToolError, ToolResult, tool_definitions, }; pub use create::{ CreateRunOptions, CreateRunSpec, CreateRunSpecInput, CreateRunsResult, CreatedRunResult, @@ -47,3 +48,7 @@ pub use search::{ FabroRunSearchParams, SearchRunSummaryResult, SearchRunsResult, ValidatedSearchRuns, search_runs, search_runs_text, }; +pub use workflow_version::{ + FabroWorkflowVersionCreateParams, ValidatedWorkflowVersionCreate, WorkflowVersionPackager, + create_workflow_version, workflow_version_create_text, +}; diff --git a/lib/components/fabro-tool/src/workflow_version.rs b/lib/components/fabro-tool/src/workflow_version.rs new file mode 100644 index 000000000..5a3e51fcf --- /dev/null +++ b/lib/components/fabro-tool/src/workflow_version.rs @@ -0,0 +1,207 @@ +use std::collections::BTreeMap; +use std::sync::Arc; + +use async_trait::async_trait; +use fabro_api::types::CreateWorkflowVersionResponse; +use fabro_types::{MAX_WORKFLOW_VERSION_BYTES, WorkflowPath}; +use fabro_workflow_version::CollectedWorkflowClosure; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::{FabroToolBackend, ToolError, ToolResult}; + +/// Caller-supplied source content, before packaging resolves workflow +/// dependencies. +#[derive(Clone, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct FabroWorkflowVersionCreateParams { + /// Exact package-relative key of the graph or workflow configuration file. + #[schemars(with = "String")] + pub entrypoint: WorkflowPath, + /// All local dependencies, keyed by package-relative path. Values are text + /// contents. Tool arguments arrive as an already-parsed JSON value on + /// every production route, so duplicate keys have collapsed (last wins) + /// before this type sees them; there is no byte-level guard to add here. + #[schemars(with = "BTreeMap")] + pub files: BTreeMap, +} + +/// A supplied source tree whose entrypoint, budgets, and portable path +/// collisions have been checked, so it is safe to stage on a filesystem. +#[derive(Clone, Debug)] +pub struct ValidatedWorkflowVersionCreate { + pub entrypoint: WorkflowPath, + pub files: BTreeMap, +} + +impl TryFrom for ValidatedWorkflowVersionCreate { + type Error = ToolError; + + fn try_from(params: FabroWorkflowVersionCreateParams) -> Result { + let FabroWorkflowVersionCreateParams { entrypoint, files } = params; + fabro_types::validate_workflow_files(&entrypoint, &files) + .map_err(|err| ToolError::message(err.to_string()))?; + let total: usize = files.values().map(String::len).sum(); + if total > MAX_WORKFLOW_VERSION_BYTES { + return Err(ToolError::message(format!( + "workflow source exceeds {} MiB", + MAX_WORKFLOW_VERSION_BYTES / (1024 * 1024) + ))); + } + fabro_types::validate_workflow_source_paths(files.keys()) + .map_err(|_| ToolError::message("workflow source paths collide"))?; + Ok(Self { entrypoint, files }) + } +} + +/// Application seam for packaging supplied content. The manifest crates that +/// own collection depend on this crate, so the packager is injected instead. +/// Implementations confine reads to supplied files and validate the entire +/// closure before returning. +#[async_trait] +pub trait WorkflowVersionPackager: Send + Sync { + async fn package( + &self, + source: ValidatedWorkflowVersionCreate, + ) -> anyhow::Result; +} + +pub async fn create_workflow_version( + backend: Arc, + source: ValidatedWorkflowVersionCreate, +) -> ToolResult { + let workflow_version_id = backend + .create_workflow_version(source) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + Ok(CreateWorkflowVersionResponse { + workflow_version_id, + }) +} + +#[must_use] +pub fn workflow_version_create_text(result: &CreateWorkflowVersionResponse) -> String { + format!("Registered workflow version {}", result.workflow_version_id) +} + +#[cfg(test)] +mod tests { + use fabro_types::{MAX_WORKFLOW_VERSION_FILE_BYTES, MAX_WORKFLOW_VERSION_FILES}; + use serde_json::json; + + use super::*; + use crate::fabro_client::ClientBackend; + + fn validate(value: serde_json::Value) -> ToolResult { + let params: FabroWorkflowVersionCreateParams = serde_json::from_value(value).unwrap(); + ValidatedWorkflowVersionCreate::try_from(params) + } + + #[test] + fn workflow_version_request_rejects_unknown_fields_and_invalid_paths() { + let valid = json!({"entrypoint": "workflow", "files": {"workflow": "digraph W {}"}}); + validate(valid.clone()).unwrap(); + for field in [ + "cwd", + "url", + "workflow", + "environment", + "parent_id", + "workflow_dependencies", + ] { + let mut value = valid.clone(); + value[field] = json!("unexpected"); + assert!(serde_json::from_value::(value).is_err()); + } + for path in [ + "../workflow", + "/workflow", + "a/../workflow", + "a//b", + "a\\b", + "~/workflow", + "", + ] { + for value in [ + json!({"entrypoint":path,"files":{"workflow":"x"}}), + json!({"entrypoint":"workflow","files":{path:"x"}}), + ] { + assert!(serde_json::from_value::(value).is_err()); + } + } + } + + #[test] + fn workflow_version_source_enforces_presence_collisions_and_budgets() { + assert!( + validate(json!({"entrypoint":"missing","files":{"workflow":"digraph W {}"}})).is_err() + ); + for files in [ + json!({"A":"x","a":"y"}), + json!({"A":"x","a/b.md":"y"}), + json!({"a":"x","A/b.md":"y"}), + ] { + let mut files = files.as_object().unwrap().clone(); + files.insert("workflow".into(), json!("digraph W {}")); + assert!(validate(json!({"entrypoint":"workflow","files":files})).is_err()); + } + let oversized_file = FabroWorkflowVersionCreateParams { + entrypoint: "workflow".parse().unwrap(), + files: BTreeMap::from([( + "workflow".parse().unwrap(), + "x".repeat(MAX_WORKFLOW_VERSION_FILE_BYTES + 1), + )]), + }; + assert!(ValidatedWorkflowVersionCreate::try_from(oversized_file).is_err()); + let mut too_many_files = FabroWorkflowVersionCreateParams { + entrypoint: "workflow".parse().unwrap(), + files: (0..MAX_WORKFLOW_VERSION_FILES) + .map(|i| (format!("file{i}").parse().unwrap(), String::new())) + .collect(), + }; + too_many_files + .files + .insert(too_many_files.entrypoint.clone(), String::new()); + assert!(ValidatedWorkflowVersionCreate::try_from(too_many_files).is_err()); + let mut oversized_total = FabroWorkflowVersionCreateParams { + entrypoint: "workflow".parse().unwrap(), + files: (0..5) + .map(|i| { + ( + format!("file{i}").parse().unwrap(), + "x".repeat(MAX_WORKFLOW_VERSION_FILE_BYTES), + ) + }) + .collect(), + }; + oversized_total + .files + .insert(oversized_total.entrypoint.clone(), String::new()); + assert!(ValidatedWorkflowVersionCreate::try_from(oversized_total).is_err()); + } + + #[tokio::test] + async fn workflow_version_same_run_backend_denies_before_packaging() { + let client = fabro_client::Client::new_no_proxy("http://127.0.0.1:1").unwrap(); + let backend = ClientBackend::new(Arc::new(client)) + .with_workflow_version_packager(Arc::new(UnreachablePackager)) + .with_run_scope("01KRBZW4DW0000000000000002".parse().unwrap()); + let source = + validate(json!({"entrypoint":"workflow","files":{"workflow":"digraph W {}"}})).unwrap(); + let error = create_workflow_version(Arc::new(backend), source) + .await + .unwrap_err(); + assert!(error.as_str().contains("run scope")); + } + + struct UnreachablePackager; + #[async_trait] + impl WorkflowVersionPackager for UnreachablePackager { + async fn package( + &self, + _: ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + panic!("scoped backend must not invoke the packager") + } + } +} diff --git a/lib/components/fabro-workflow-version/src/closure.rs b/lib/components/fabro-workflow-version/src/closure.rs new file mode 100644 index 000000000..0e438cba1 --- /dev/null +++ b/lib/components/fabro-workflow-version/src/closure.rs @@ -0,0 +1,38 @@ +use fabro_types::WorkflowVersionId; + +use crate::ValidatedWorkflowVersion; + +/// Validated workflow versions in dependency-first order, with the root last. +/// Owns source contents; consumers borrow versions instead of cloning them. +#[derive(Debug)] +pub struct CollectedWorkflowClosure { + root_id: WorkflowVersionId, + versions: Vec<(WorkflowVersionId, ValidatedWorkflowVersion)>, +} + +impl CollectedWorkflowClosure { + /// Assemble the result of a collector that has already ordered and + /// validated the dependency graph. The caller supplies matching IDs, + /// unique versions, and dependencies before parents, with `root_id` + /// identifying the last entry. This preserves the collector's ordering + /// without traversing or hashing again. + #[must_use] + pub fn from_dependency_order( + root_id: WorkflowVersionId, + versions: Vec<(WorkflowVersionId, ValidatedWorkflowVersion)>, + ) -> Self { + Self { root_id, versions } + } + + #[must_use] + pub fn root_id(&self) -> WorkflowVersionId { + self.root_id + } + + /// Iterate over every version with dependencies before parents. + pub fn versions( + &self, + ) -> impl Iterator + '_ { + self.versions.iter().map(|(id, version)| (*id, version)) + } +} diff --git a/lib/components/fabro-workflow-version/src/lib.rs b/lib/components/fabro-workflow-version/src/lib.rs index bf0d89a44..b9f8eb6ed 100644 --- a/lib/components/fabro-workflow-version/src/lib.rs +++ b/lib/components/fabro-workflow-version/src/lib.rs @@ -23,8 +23,9 @@ use fabro_types::settings::InterpString; use fabro_types::{ManifestPath, WorkflowPath, WorkflowPathParseError, WorkflowVersion}; use thiserror::Error; +mod closure; mod store; - +pub use closure::CollectedWorkflowClosure; pub use store::{LoadedWorkflowVersionClosure, WorkflowVersionStore, WorkflowVersionStoreError}; #[derive(Debug, Error)] diff --git a/lib/components/fabro-workflow/Cargo.toml b/lib/components/fabro-workflow/Cargo.toml index 314a53e42..8574796f1 100644 --- a/lib/components/fabro-workflow/Cargo.toml +++ b/lib/components/fabro-workflow/Cargo.toml @@ -77,6 +77,8 @@ tempfile = "3" toml.workspace = true fabro-vault = { path = "../../foundation/fabro-vault" } [dev-dependencies] +fabro-client = { path = "../../foundation/fabro-client" } +fabro-workflow-version = { path = "../fabro-workflow-version" } fabro-llm = { path = "../fabro-llm", features = ["test-support"] } fabro-store = { path = "../fabro-store", features = ["test-support"] } fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } diff --git a/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs b/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs index 67d3df89f..c2903a64a 100644 --- a/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs +++ b/lib/components/fabro-workflow/src/handler/llm/fabro_tools.rs @@ -1,4 +1,4 @@ -//! The Fabro run tools (`fabro_run_*`) as application tools a pebble coding +//! The Fabro workflow and run tools as application tools a pebble coding //! agent can call. use std::sync::Arc; @@ -66,6 +66,15 @@ pub(crate) async fn execute_fabro_run_tool( services: &FabroRunToolServices, ) -> fabro_tool::ToolResult { match name { + fabro_tool::FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME => { + let params = + parse_fabro_tool_args::(name, args)?; + let source = fabro_tool::ValidatedWorkflowVersionCreate::try_from(params)?; + let result = + fabro_tool::create_workflow_version(Arc::clone(&services.backend), source).await?; + let summary = fabro_tool::workflow_version_create_text(&result); + render_fabro_tool_result(&summary, &result) + } fabro_tool::FABRO_RUN_CREATE_TOOL_NAME => { let params = parse_fabro_tool_args::(name, args)?; ensure_current_run_parent(¶ms, services.current_run_id)?; @@ -196,3 +205,89 @@ where })?; Ok(format!("{summary}\n{json}")) } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use async_trait::async_trait; + use fabro_tool::fabro_client::ClientBackend; + use fabro_tool::{ValidatedWorkflowVersionCreate, WorkflowVersionPackager}; + use fabro_types::WorkflowVersion; + use fabro_workflow_version::{CollectedWorkflowClosure, ValidatedWorkflowVersion}; + use serde_json::json; + + use super::*; + + struct SingleGraphPackager; + + #[async_trait] + impl WorkflowVersionPackager for SingleGraphPackager { + async fn package( + &self, + source: ValidatedWorkflowVersionCreate, + ) -> anyhow::Result { + let version = WorkflowVersion::new(source.entrypoint, source.files, BTreeMap::new())?; + let id = version.id()?; + Ok(CollectedWorkflowClosure::from_dependency_order(id, vec![( + id, + ValidatedWorkflowVersion::new(version)?, + )])) + } + } + + #[tokio::test] + async fn workflow_version_native_dispatch_registers_and_returns_version() { + let server = httpmock::MockServer::start_async().await; + let version = WorkflowVersion::new( + "workflow".parse().unwrap(), + BTreeMap::from([("workflow".parse().unwrap(), "digraph W {}".into())]), + BTreeMap::new(), + ) + .unwrap(); + let id = version.id().unwrap(); + let upload = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/api/v1/workflow-versions") + .json_body_obj(&version); + then.status(201) + .json_body(json!({"workflow_version_id": id})); + }) + .await; + let client = fabro_client::Client::new_no_proxy(&server.url("")).unwrap(); + let services = FabroRunToolServices { + 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(), + }; + let name = fabro_tool::FABRO_WORKFLOW_VERSION_CREATE_TOOL_NAME; + assert_eq!(register_named_fabro_run_tools(&services, &[name]).len(), 1); + let output = execute_fabro_run_tool( + name, + json!({"entrypoint":"workflow", "files":{"workflow":"digraph W {}"}}), + &services, + ) + .await + .unwrap(); + let (summary, body) = output.split_once('\n').unwrap(); + assert_eq!(summary, format!("Registered workflow version {id}")); + assert_eq!( + serde_json::from_str::(body).unwrap(), + json!({"workflow_version_id": id}) + ); + let error = execute_fabro_run_tool( + name, + json!({"entrypoint":"missing", "files":{"workflow":"digraph W {}"}}), + &services, + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("not present")); + upload.assert_calls_async(1).await; + } +} diff --git a/lib/foundation/fabro-config/src/project.rs b/lib/foundation/fabro-config/src/project.rs index 4fc58e61e..f61ae2bff 100644 --- a/lib/foundation/fabro-config/src/project.rs +++ b/lib/foundation/fabro-config/src/project.rs @@ -46,11 +46,22 @@ impl WorkflowLocation { /// graph file (e.g. `workflow.fabro`); the three forms produce the same /// shape. pub fn resolve(arg: &Path, cwd: &Path) -> Result { - let resolved = resolve_workflow_arg_from(arg, cwd)?; - if resolved.extension().is_some_and(|ext| ext == "toml") { - Self::from_toml(resolved) + Self::from_resolved_path(resolve_workflow_arg_from(arg, cwd)?) + } + + /// Resolve an exact file path without workflow-name lookup. Relative paths + /// are interpreted against the supplied directory only. + pub fn from_exact_path(path: &Path, directory: &Path) -> Result { + Self::from_resolved_path(directory.join(path)) + } + + /// Dispatch a resolved file path on its extension: `workflow.toml` loads + /// run config, anything else is treated as a graph file. + fn from_resolved_path(path: PathBuf) -> Result { + if path.extension().is_some_and(|ext| ext == "toml") { + Self::from_toml(path) } else { - Ok(Self::from_graph(resolved)) + Ok(Self::from_graph(path)) } } diff --git a/lib/foundation/fabro-types/Cargo.toml b/lib/foundation/fabro-types/Cargo.toml index 2e777317b..1fb90fb25 100644 --- a/lib/foundation/fabro-types/Cargo.toml +++ b/lib/foundation/fabro-types/Cargo.toml @@ -33,6 +33,8 @@ strum.workspace = true thiserror.workspace = true toml.workspace = true ulid.workspace = true +unicase.workspace = true +unicode-normalization.workspace = true url.workspace = true [dev-dependencies] diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index b4b26fe74..2259c3814 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -198,5 +198,6 @@ pub use workflow_path::{ pub use workflow_version::{ MAX_WORKFLOW_VERSION_BYTES, MAX_WORKFLOW_VERSION_DEPENDENCIES, MAX_WORKFLOW_VERSION_FILE_BYTES, MAX_WORKFLOW_VERSION_FILES, WorkflowVersion, WorkflowVersionShapeError, + validate_workflow_files, validate_workflow_source_paths, }; pub use workflow_version_id::{WorkflowVersionId, WorkflowVersionIdParseError}; diff --git a/lib/foundation/fabro-types/src/workflow_version.rs b/lib/foundation/fabro-types/src/workflow_version.rs index 125544aa3..58fb8e0ad 100644 --- a/lib/foundation/fabro-types/src/workflow_version.rs +++ b/lib/foundation/fabro-types/src/workflow_version.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; use std::fmt; use std::marker::PhantomData; @@ -5,6 +6,8 @@ use std::marker::PhantomData; use serde::de::{Error as _, MapAccess, Visitor}; use serde::{Deserialize, Deserializer, Serialize}; use thiserror::Error; +use unicase::UniCase; +use unicode_normalization::UnicodeNormalization as _; use crate::{BlobHash, WorkflowPath, WorkflowVersionId}; @@ -119,61 +122,110 @@ impl WorkflowVersion { } fn validate_shape(&self) -> Result<(), WorkflowVersionShapeError> { - if self.files.len() > MAX_WORKFLOW_VERSION_FILES { - return Err(WorkflowVersionShapeError::TooManyFiles { - actual: self.files.len(), - maximum: MAX_WORKFLOW_VERSION_FILES, - }); - } + validate_workflow_files(&self.entrypoint, &self.files)?; if self.workflow_dependencies.len() > MAX_WORKFLOW_VERSION_DEPENDENCIES { return Err(WorkflowVersionShapeError::TooManyWorkflowDependencies { actual: self.workflow_dependencies.len(), maximum: MAX_WORKFLOW_VERSION_DEPENDENCIES, }); } - for (path, content) in &self.files { - if content.len() > MAX_WORKFLOW_VERSION_FILE_BYTES { - return Err(WorkflowVersionShapeError::FileTooLarge { - path: path.clone(), - actual: content.len(), - maximum: MAX_WORKFLOW_VERSION_FILE_BYTES, - }); - } - } - if !self.files.contains_key(&self.entrypoint) { - return Err(WorkflowVersionShapeError::MissingEntrypoint { - path: self.entrypoint.clone(), - }); - } self.validate_path_collisions() } fn validate_path_collisions(&self) -> Result<(), WorkflowVersionShapeError> { - // Keys are unique within each map, so equality can only collide - // across files and workflow dependencies. - let mut by_text = - HashMap::with_capacity(self.files.len() + self.workflow_dependencies.len()); - for path in self.files.keys().chain(self.workflow_dependencies.keys()) { - if let Some(existing) = by_text.insert(path.as_str(), path) { + validate_path_collisions( + self.files.keys().chain(self.workflow_dependencies.keys()), + Cow::Borrowed, + ) + } +} + +/// Validate the file limits and entrypoint shared by source trees and versions. +/// Aggregate source bytes, canonical bytes, and path policies are checked +/// separately. +pub fn validate_workflow_files( + entrypoint: &WorkflowPath, + files: &BTreeMap, +) -> Result<(), WorkflowVersionShapeError> { + if files.len() > MAX_WORKFLOW_VERSION_FILES { + return Err(WorkflowVersionShapeError::TooManyFiles { + actual: files.len(), + maximum: MAX_WORKFLOW_VERSION_FILES, + }); + } + for (path, content) in files { + if content.len() > MAX_WORKFLOW_VERSION_FILE_BYTES { + return Err(WorkflowVersionShapeError::FileTooLarge { + path: path.clone(), + actual: content.len(), + maximum: MAX_WORKFLOW_VERSION_FILE_BYTES, + }); + } + } + if !files.contains_key(entrypoint) { + return Err(WorkflowVersionShapeError::MissingEntrypoint { + path: entrypoint.clone(), + }); + } + Ok(()) +} + +/// Reject file and directory aliases before materializing a portable source +/// tree, including Unicode case folding and normalization. Canonical versions +/// themselves retain their exact, case-sensitive semantics. +pub fn validate_workflow_source_paths<'a>( + paths: impl IntoIterator, +) -> Result<(), WorkflowVersionShapeError> { + validate_path_collisions(paths, |text| { + if text.is_ascii() { + Cow::Owned(text.to_ascii_lowercase()) + } else { + // Normalize before folding: case folding is not closed under + // canonical equivalence, so folding a decomposed sequence and + // folding its precomposed form can yield different strings. + let normalized: String = text.nfc().collect(); + Cow::Owned( + UniCase::unicode(normalized) + .to_folded_case() + .nfc() + .collect(), + ) + } + }) +} + +/// Detect colliding paths under a comparison key: identical keys, or a key +/// that names an ancestor directory of another. +fn validate_path_collisions<'a>( + paths: impl IntoIterator, + key: impl Fn(&'a str) -> Cow<'a, str>, +) -> Result<(), WorkflowVersionShapeError> { + let keyed: Vec<(Cow<'a, str>, &WorkflowPath)> = paths + .into_iter() + .map(|path| (key(path.as_str()), path)) + .collect(); + let mut by_text = HashMap::with_capacity(keyed.len()); + for (text, path) in &keyed { + if let Some(existing) = by_text.insert(text.as_ref(), *path) { + return Err(WorkflowVersionShapeError::PathCollision { + first: existing.clone(), + second: (*path).clone(), + }); + } + } + // Walk the input order, not the map, so the reported pair is stable when + // more than one ancestor collision exists. + for (text, path) in &keyed { + for (index, _) in text.match_indices('/') { + if let Some(ancestor) = by_text.get(&text[..index]) { return Err(WorkflowVersionShapeError::PathCollision { - first: existing.clone(), - second: path.clone(), + first: (*ancestor).clone(), + second: (*path).clone(), }); } } - for path in self.files.keys().chain(self.workflow_dependencies.keys()) { - let text = path.as_str(); - for (index, _) in text.match_indices('/') { - if let Some(ancestor) = by_text.get(&text[..index]) { - return Err(WorkflowVersionShapeError::PathCollision { - first: (*ancestor).clone(), - second: path.clone(), - }); - } - } - } - Ok(()) } + Ok(()) } impl<'de> Deserialize<'de> for WorkflowVersion { @@ -509,3 +561,60 @@ mod tests { assert!(serde_json::from_str::(duplicate).is_err()); } } + +#[cfg(test)] +mod source_path_tests { + use super::*; + + #[test] + fn ancestor_collision_reports_the_first_pair_in_input_order() { + let paths = ["assets", "assets/item.txt", "libs", "libs/child.fabro"] + .map(|path| WorkflowPath::new(path).unwrap()); + for _ in 0..32 { + let error = validate_workflow_source_paths(paths.iter()).unwrap_err(); + assert_eq!( + error.to_string(), + "workflow paths collide: `assets` and `assets/item.txt`" + ); + let version = WorkflowVersion::new( + paths[1].clone(), + paths.iter().map(|p| (p.clone(), String::new())).collect(), + BTreeMap::new(), + ) + .unwrap_err(); + assert_eq!(version.to_string(), error.to_string()); + } + } + + #[test] + fn workflow_source_collisions_are_portable_in_both_orders() { + for pair in [ + ["A", "a"], + ["A", "a/b.md"], + ["a", "A/b.md"], + ["é", "É/b"], + ["ΟΣ", "οσ/b"], + ["é", "e\u{301}/b"], + ["Straße", "STRASSE/b"], + // Canonically equivalent, but folding before normalizing yields + // different keys (U+03B1 U+03AF vs U+03AC U+03B9). + ["α\u{345}\u{301}.md", "\u{1FB4}.md"], + ] { + let paths = pair.map(|path| WorkflowPath::new(path).unwrap()); + assert!(validate_workflow_source_paths(paths.iter()).is_err()); + assert!(validate_workflow_source_paths(paths.iter().rev()).is_err()); + } + let version = WorkflowVersion::new( + WorkflowPath::new("A").unwrap(), + BTreeMap::from([ + (WorkflowPath::new("A").unwrap(), "x".into()), + (WorkflowPath::new("a").unwrap(), "y".into()), + ]), + BTreeMap::new(), + ); + assert!( + version.is_ok(), + "canonical versions keep exact path semantics" + ); + } +} diff --git a/lib/packages/fabro-api-client/src/api/workflow-versions-api.ts b/lib/packages/fabro-api-client/src/api/workflow-versions-api.ts index 4285c1849..14173fac5 100644 --- a/lib/packages/fabro-api-client/src/api/workflow-versions-api.ts +++ b/lib/packages/fabro-api-client/src/api/workflow-versions-api.ts @@ -33,7 +33,7 @@ import type { WorkflowVersion } from '../models'; export const WorkflowVersionsApiAxiosParamCreator = function (configuration?: Configuration) { return { /** - * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. + * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. Requires an authenticated user or a worker token with the `agent:run_tools` capability. Ordinary worker tokens cannot register versions. Registration creates no run and starts no execution. * @summary Create Workflow Version * @param {WorkflowVersion} workflowVersion * @param {*} [options] Override http request option. @@ -83,7 +83,7 @@ export const WorkflowVersionsApiFp = function(configuration?: Configuration) { const localVarAxiosParamCreator = WorkflowVersionsApiAxiosParamCreator(configuration) return { /** - * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. + * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. Requires an authenticated user or a worker token with the `agent:run_tools` capability. Ordinary worker tokens cannot register versions. Registration creates no run and starts no execution. * @summary Create Workflow Version * @param {WorkflowVersion} workflowVersion * @param {*} [options] Override http request option. @@ -105,7 +105,7 @@ export const WorkflowVersionsApiFactory = function (configuration?: Configuratio const localVarFp = WorkflowVersionsApiFp(configuration) return { /** - * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. + * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. Requires an authenticated user or a worker token with the `agent:run_tools` capability. Ordinary worker tokens cannot register versions. Registration creates no run and starts no execution. * @summary Create Workflow Version * @param {WorkflowVersion} workflowVersion * @param {*} [options] Override http request option. @@ -122,7 +122,7 @@ export const WorkflowVersionsApiFactory = function (configuration?: Configuratio */ export class WorkflowVersionsApi extends BaseAPI { /** - * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. + * Validates and stores an immutable workflow package in content-addressed storage. Repeating the same canonical content returns the same identifier. Requires an authenticated user or a worker token with the `agent:run_tools` capability. Ordinary worker tokens cannot register versions. Registration creates no run and starts no execution. * @summary Create Workflow Version * @param {WorkflowVersion} workflowVersion * @param {*} [options] Override http request option.