From e6eb59537a0b3b7a6d0af5754e27fc4e8773db85 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 11 Sep 2026 15:30:37 -0600 Subject: [PATCH] Move the supplied-content packager next to its collector ServerWorkflowVersionPackager was a pure adapter over fabro_manifest::collect_supplied_workflow_versions that touched no server state, yet it lived in fabro-server and was imported from there by the standalone MCP server and the CLI run worker. fabro-manifest can depend on fabro-tool without a cycle, so the adapter now lives beside the collector as SuppliedWorkflowVersionPackager and fabro-server no longer exports a non-server module for it. The adapter also cloned every version's file map out of a closure it already owned. CollectedWorkflowClosure::into_versions hands the versions over by value inside the blocking task instead. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 4 ++ lib/apps/fabro-cli/src/commands/run/runner.rs | 4 +- lib/apps/fabro-mcp-server/src/server.rs | 4 +- lib/apps/fabro-server/src/lib.rs | 1 - lib/components/fabro-manifest/Cargo.toml | 4 ++ lib/components/fabro-manifest/src/lib.rs | 2 + .../src/workflow_version_collector.rs | 10 +++++ .../src/workflow_version_packager.rs} | 42 ++++++++++--------- 8 files changed, 46 insertions(+), 25 deletions(-) rename lib/{apps/fabro-server/src/workflow_version_tool.rs => components/fabro-manifest/src/workflow_version_packager.rs} (77%) diff --git a/Cargo.lock b/Cargo.lock index 8923ac91a..2de971b13 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,9 @@ dependencies = [ "temp-env", "tempfile", "thiserror 2.0.18", + "tokio", "toml 0.8.23", + "tracing", ] [[package]] diff --git a/lib/apps/fabro-cli/src/commands/run/runner.rs b/lib/apps/fabro-cli/src/commands/run/runner.rs index 36f149e3a..506f2d83a 100644 --- a/lib/apps/fabro-cli/src/commands/run/runner.rs +++ b/lib/apps/fabro-cli/src/commands/run/runner.rs @@ -15,8 +15,8 @@ use fabro_interview::{ WORKER_CONTROL_WS_PING_INTERVAL, WorkerControlDeliveryFrame, WorkerControlEnvelope, WorkerControlMessage, }; +use fabro_manifest::SuppliedWorkflowVersionPackager; use fabro_server::run_tool_manifest; -use fabro_server::workflow_version_tool::ServerWorkflowVersionPackager; use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer}; use fabro_tool::fabro_client::ClientBackend; use fabro_types::settings::run::{RunMode, RunNamespace}; @@ -238,7 +238,7 @@ fn build_fabro_run_tool_services( } let backend = ClientBackend::new(Arc::new(client)) .with_manifest_builder(Arc::new(WorkerRunManifestBuilder)) - .with_workflow_version_packager(Arc::new(ServerWorkflowVersionPackager)); + .with_workflow_version_packager(Arc::new(SuppliedWorkflowVersionPackager)); Some(FabroRunToolServices { backend: Arc::new(backend), current_run_id, diff --git a/lib/apps/fabro-mcp-server/src/server.rs b/lib/apps/fabro-mcp-server/src/server.rs index 914833318..fba1fc5ac 100644 --- a/lib/apps/fabro-mcp-server/src/server.rs +++ b/lib/apps/fabro-mcp-server/src/server.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use std::time::Duration; use anyhow::Result; -use fabro_server::workflow_version_tool::ServerWorkflowVersionPackager; +use fabro_manifest::SuppliedWorkflowVersionPackager; use fabro_tool::fabro_client::ClientBackend; use fabro_tool::{self as run_tools, FabroToolBackend}; use fabro_util::version::FABRO_VERSION; @@ -277,7 +277,7 @@ impl FabroMcpServer { ClientBackend::new(Arc::new(client)) .with_manifest_builder(Arc::new(McpRunManifestBuilder)) .with_workflow_version_packager(Arc::new( - ServerWorkflowVersionPackager, + SuppliedWorkflowVersionPackager, )), ) as Arc }) diff --git a/lib/apps/fabro-server/src/lib.rs b/lib/apps/fabro-server/src/lib.rs index 227855389..98f7a2c77 100644 --- a/lib/apps/fabro-server/src/lib.rs +++ b/lib/apps/fabro-server/src/lib.rs @@ -55,7 +55,6 @@ pub mod web_auth; mod worker_control; mod worker_runtime; mod worker_token; -pub mod workflow_version_tool; pub use error::{ApiError, Error, Result}; pub use run_manifest::workflow_bundle_from_manifest; diff --git a/lib/components/fabro-manifest/Cargo.toml b/lib/components/fabro-manifest/Cargo.toml index dcb395052..62c6b2d0b 100644 --- a/lib/components/fabro-manifest/Cargo.toml +++ b/lib/components/fabro-manifest/Cargo.toml @@ -14,18 +14,22 @@ 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-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 diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 9f46f2be7..bac761ffb 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -7,6 +7,7 @@ mod local_workflow_package; mod supplied_workflow; mod workflow_bundler; mod workflow_version_collector; +mod workflow_version_packager; use std::collections::HashMap; use std::path::{Component, Path, PathBuf}; @@ -41,6 +42,7 @@ pub use crate::workflow_version_collector::{ CollectedWorkflowClosure, 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/workflow_version_collector.rs b/lib/components/fabro-manifest/src/workflow_version_collector.rs index 3c2fbe063..3c7dd95e4 100644 --- a/lib/components/fabro-manifest/src/workflow_version_collector.rs +++ b/lib/components/fabro-manifest/src/workflow_version_collector.rs @@ -31,6 +31,16 @@ impl CollectedWorkflowClosure { ) -> impl Iterator + '_ { self.versions.iter().map(|(id, version)| (*id, version)) } + + /// Consume the closure, yielding every version with dependencies before + /// parents, for callers that hand the versions on without cloning. + #[must_use] + pub fn into_versions(self) -> Vec { + self.versions + .into_iter() + .map(|(_, version)| version.into_version()) + .collect() + } } #[derive(Debug, Error)] diff --git a/lib/apps/fabro-server/src/workflow_version_tool.rs b/lib/components/fabro-manifest/src/workflow_version_packager.rs similarity index 77% rename from lib/apps/fabro-server/src/workflow_version_tool.rs rename to lib/components/fabro-manifest/src/workflow_version_packager.rs index 816c526b5..16a4f51a5 100644 --- a/lib/apps/fabro-server/src/workflow_version_tool.rs +++ b/lib/components/fabro-manifest/src/workflow_version_packager.rs @@ -1,3 +1,6 @@ +//! Application adapter that packages caller-supplied workflow contents for +//! the `fabro_workflow_version_create` tool. + use async_trait::async_trait; use fabro_tool::{ PackagedWorkflowVersions, ToolError, ValidatedWorkflowVersionCreate, WorkflowVersionPackager, @@ -7,35 +10,34 @@ use tracing::warn; /// Packages supplied workflow contents for standalone MCP and capable run /// workers; the backend that owns the API client performs registration. -pub struct ServerWorkflowVersionPackager; +pub struct SuppliedWorkflowVersionPackager; const PACKAGING_FAILED: &str = "workflow source could not be packaged; check configuration, \ syntax, local references, and package limits"; #[async_trait] -impl WorkflowVersionPackager for ServerWorkflowVersionPackager { +impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager { async fn package( &self, source: ValidatedWorkflowVersionCreate, ) -> anyhow::Result { - let closure = task::spawn_blocking(move || { - fabro_manifest::collect_supplied_workflow_versions(&source.entrypoint, &source.files) + task::spawn_blocking(move || { + let closure = + crate::collect_supplied_workflow_versions(&source.entrypoint, &source.files) + .map_err(|err| { + // Parser diagnostics may quote supplied source, so the + // chain stays in the log and only a generic message + // crosses the tool boundary. + warn!(error = %format!("{err:#}"), "workflow version packaging failed"); + ToolError::message(PACKAGING_FAILED) + })?; + Ok(PackagedWorkflowVersions { + root_id: closure.root_id(), + versions: closure.into_versions(), + }) }) .await .map_err(|err| anyhow::anyhow!("workflow packaging task failed: {err}"))? - .map_err(|err| { - // Parser diagnostics may quote supplied source, so the chain stays - // in the log and only a generic message crosses the tool boundary. - warn!(error = %format!("{err:#}"), "workflow version packaging failed"); - ToolError::message(PACKAGING_FAILED) - })?; - Ok(PackagedWorkflowVersions { - root_id: closure.root_id(), - versions: closure - .versions() - .map(|(_, version)| version.version().clone()) - .collect(), - }) } } @@ -70,7 +72,7 @@ mod tests { #[tokio::test] async fn packager_returns_dependencies_before_root() { - let packaged = ServerWorkflowVersionPackager + let packaged = SuppliedWorkflowVersionPackager .package(fixture()) .await .unwrap(); @@ -112,7 +114,7 @@ mod tests { "PRIVATE_CONTENT invalid source", )]), ] { - let error = ServerWorkflowVersionPackager + let error = SuppliedWorkflowVersionPackager .package(input) .await .unwrap_err(); @@ -129,7 +131,7 @@ mod tests { .files .insert("Prompt.md".parse().unwrap(), prompt); assert!( - ServerWorkflowVersionPackager + SuppliedWorkflowVersionPackager .package(wrong_case) .await .is_err(),