diff --git a/Cargo.lock b/Cargo.lock index f45bc947f..a84eec268 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3173,6 +3173,7 @@ dependencies = [ "fabro-client", "fabro-types", "fabro-util", + "fabro-workflow-version", "futures", "httpmock", "schemars 1.2.1", diff --git a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs index d791334b4..64181a159 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs @@ -1858,7 +1858,7 @@ async fn mcp_workflow_version_validation_happens_before_auth_or_network() { serde_json::json!({"entrypoint":"workflow","files":{}}), ) .await; - assert_eq!(error, "entrypoint must be an exact supplied file key"); + assert_eq!(error, "entrypoint `workflow` is not present in workflow files"); assert_mcp_run_tool_count(&client).await; client .shutdown() diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index bfd5f0162..f020b5828 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -5,6 +5,8 @@ mod local_workflow_package; mod supplied_workflow; +#[cfg(test)] +mod test_support; mod workflow_bundler; mod workflow_version_collector; mod workflow_version_packager; @@ -32,6 +34,7 @@ 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, @@ -39,8 +42,8 @@ pub use crate::local_workflow_package::{ pub use crate::supplied_workflow::collect_supplied_workflow_versions; use crate::workflow_bundler::WorkflowBundler; pub use crate::workflow_version_collector::{ - CollectedWorkflowClosure, MAX_WORKFLOW_VERSION_DEPTH, WorkflowVersionCollectError, - collect_workflow_versions, collect_workflow_versions_at_location, + MAX_WORKFLOW_VERSION_DEPTH, WorkflowVersionCollectError, collect_workflow_versions, + collect_workflow_versions_at_location, }; pub use crate::workflow_version_packager::SuppliedWorkflowVersionPackager; diff --git a/lib/components/fabro-manifest/src/supplied_workflow.rs b/lib/components/fabro-manifest/src/supplied_workflow.rs index 064516f64..64f824328 100644 --- a/lib/components/fabro-manifest/src/supplied_workflow.rs +++ b/lib/components/fabro-manifest/src/supplied_workflow.rs @@ -118,42 +118,11 @@ fn confine_to_supplied( mod tests { use std::path::Path; + use fabro_tool::ValidatedWorkflowVersionCreate as Supplied; use fabro_util::error::collect_chain; use super::*; - - struct Supplied { - entrypoint: WorkflowPath, - files: BTreeMap, - } - - fn supplied(entrypoint: &str, files: &[(&str, &str)]) -> Supplied { - Supplied { - entrypoint: entrypoint.parse().unwrap(), - files: files - .iter() - .map(|(path, content)| (path.parse().unwrap(), (*content).to_string())) - .collect(), - } - } - - fn fixture() -> Supplied { - supplied("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 {}"), - ]) - } + use crate::test_support::{fixture, source as supplied}; fn collect(input: &Supplied) -> CollectedWorkflowClosure { collect_supplied_workflow_versions(&input.entrypoint, &input.files).unwrap() 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 2c56a1310..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, workflow_version_collector}; +use crate::{ + WorkflowVersionCollectError, manifest_path_from_absolute, normalize_absolute_path, + workflow_version_collector, +}; pub(super) struct WorkflowBundler<'a> { package_root: &'a Path, @@ -38,15 +41,6 @@ pub(super) struct CollectedWorkflowSource { pub(super) dependency_keys: BTreeSet, } -/// A referenced file that does not exist under the package root, reported -/// with its package-relative path so callers can surface it without the -/// staging directory or any file content. -#[derive(Debug, thiserror::Error)] -#[error("workflow package file `{path}` is missing")] -pub(super) struct MissingPackageFile { - pub(super) path: String, -} - impl<'a> WorkflowBundler<'a> { pub(super) fn new(package_root: &'a Path, inputs: &'a HashMap) -> Self { Self { @@ -546,7 +540,9 @@ impl<'a> WorkflowBundler<'a> { 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(MissingPackageFile { path }); + return anyhow::Error::new(WorkflowVersionCollectError::MissingPackageFile { + path, + }); } anyhow::Error::new(source).context(format!( "failed to canonicalize workflow package file `{}`", diff --git a/lib/components/fabro-manifest/src/workflow_version_collector.rs b/lib/components/fabro-manifest/src/workflow_version_collector.rs index b3667d808..131f9990c 100644 --- a/lib/components/fabro-manifest/src/workflow_version_collector.rs +++ b/lib/components/fabro-manifest/src/workflow_version_collector.rs @@ -7,12 +7,12 @@ 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, MissingPackageFile, WorkflowBundler, -}; +use crate::workflow_bundler::{CollectedWorkflowSource, CollectedWorkflowSources, WorkflowBundler}; /// Maximum active graph nesting while collecting or assembling a version. /// Bounds native stack use independently of file-count and byte budgets. @@ -31,37 +31,6 @@ pub(super) fn check_workflow_depth( Ok(()) } -/// One locally packaged workflow-version closure in dependency-first order. -#[derive(Debug)] -pub struct CollectedWorkflowClosure { - root_id: WorkflowVersionId, - versions: Vec<(WorkflowVersionId, ValidatedWorkflowVersion)>, -} - -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)) - } - - /// 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)] pub enum WorkflowVersionCollectError { #[error("workflow dependency nesting at `{path}` exceeds {maximum} levels")] @@ -207,22 +176,12 @@ pub fn collect_workflow_versions_at_location( let collected = WorkflowBundler::new(package_root, &inputs) .collect_versions(location) .map_err(|source| { - let source = match source.downcast::() { - Ok(error) => return error, - Err(source) => source, - }; - let missing = source - .chain() - .find_map(|cause| cause.downcast_ref::()); - match missing { - Some(missing) => WorkflowVersionCollectError::MissingPackageFile { - path: missing.path.clone(), - }, - None => WorkflowVersionCollectError::Collect { + source + .downcast::() + .unwrap_or_else(|source| WorkflowVersionCollectError::Collect { path: workflow.to_path_buf(), source, - }, - } + }) })?; VersionAssembler::new(collected).assemble() } @@ -260,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( diff --git a/lib/components/fabro-manifest/src/workflow_version_packager.rs b/lib/components/fabro-manifest/src/workflow_version_packager.rs index c13067f5f..566161a9a 100644 --- a/lib/components/fabro-manifest/src/workflow_version_packager.rs +++ b/lib/components/fabro-manifest/src/workflow_version_packager.rs @@ -3,11 +3,9 @@ use anyhow::Context as _; use async_trait::async_trait; -use fabro_tool::{ - PackagedWorkflowVersions, ToolError, ValidatedWorkflowVersionCreate, WorkflowVersionPackager, -}; +use fabro_tool::{ToolError, ValidatedWorkflowVersionCreate, WorkflowVersionPackager}; use fabro_util::error::collect_chain; -use fabro_workflow_version::WorkflowVersionError; +use fabro_workflow_version::{CollectedWorkflowClosure, WorkflowVersionError}; use tokio::task; use tracing::debug; @@ -24,7 +22,7 @@ impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager { async fn package( &self, source: ValidatedWorkflowVersionCreate, - ) -> anyhow::Result { + ) -> anyhow::Result { let packaged = task::spawn_blocking(move || package_blocking(&source)) .await .context("workflow packaging task failed")??; @@ -40,7 +38,7 @@ impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager { /// must not reach the log at any level. fn package_blocking( source: &ValidatedWorkflowVersionCreate, -) -> Result { +) -> Result { let closure = crate::collect_supplied_workflow_versions(&source.entrypoint, &source.files) .map_err(|err| { debug!( @@ -51,10 +49,7 @@ fn package_blocking( ); ToolError::message(render_packaging_error(&err)) })?; - Ok(PackagedWorkflowVersions { - root_id: closure.root_id(), - versions: closure.into_versions(), - }) + Ok(closure) } /// Render a packaging failure for the tool caller. Every collector variant's @@ -120,30 +115,7 @@ mod tests { } } - 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(), - } - } - - 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", "Review the implementation."), - ("child.fabro", "digraph Child {}"), - ]) - } + use crate::test_support::{fixture, source}; async fn package_error(input: ValidatedWorkflowVersionCreate) -> String { let error = SuppliedWorkflowVersionPackager @@ -246,16 +218,20 @@ mod tests { .package(fixture()) .await .unwrap(); - assert_eq!(packaged.versions.len(), 2); - assert_eq!(packaged.versions[0].entrypoint().as_str(), "child.fabro"); + 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!( - packaged.versions[1].id().unwrap(), - packaged.root_id, + versions[1].id().unwrap(), + packaged.root_id(), "root version must be last" ); - let child_id = packaged.versions[0].id().unwrap(); + let child_id = versions[0].id().unwrap(); assert!( - packaged.versions[1] + versions[1] .workflow_dependencies() .values() .any(|id| *id == child_id) diff --git a/lib/components/fabro-tool/Cargo.toml b/lib/components/fabro-tool/Cargo.toml index 892b7a6a7..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 diff --git a/lib/components/fabro-tool/src/fabro_client.rs b/lib/components/fabro-tool/src/fabro_client.rs index d01ebb1bd..120176183 100644 --- a/lib/components/fabro-tool/src/fabro_client.rs +++ b/lib/components/fabro-tool/src/fabro_client.rs @@ -82,10 +82,12 @@ impl FabroToolBackend for ClientBackend { .as_ref() .ok_or_else(common::workflow_version_tool_unavailable_error)?; let packaged = packager.package(source).await?; - self.client - .register_workflow_versions(&packaged.versions) - .await?; - Ok(packaged.root_id) + 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( @@ -292,22 +294,29 @@ mod tests { use async_trait::async_trait; use fabro_types::{WorkflowVersion, WorkflowVersionId}; + use fabro_workflow_version::{CollectedWorkflowClosure, ValidatedWorkflowVersion}; use serde_json::json; use super::*; - use crate::{ - PackagedWorkflowVersions, ValidatedWorkflowVersionCreate, WorkflowVersionPackager, - }; + use crate::{ValidatedWorkflowVersionCreate, WorkflowVersionPackager}; - struct FixedPackager(PackagedWorkflowVersions); + struct FixedPackager(Vec); #[async_trait] impl WorkflowVersionPackager for FixedPackager { async fn package( &self, _: ValidatedWorkflowVersionCreate, - ) -> anyhow::Result { - Ok(self.0.clone()) + ) -> 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, + )) } } @@ -319,7 +328,14 @@ mod tests { entrypoint.parse().unwrap(), BTreeMap::from([( entrypoint.parse().unwrap(), - format!("digraph {entrypoint} {{}}"), + format!( + "digraph {entrypoint} {{ {} }}", + dependencies + .keys() + .map(|p| format!("child [stack.child_workflow=\"{p}\"]")) + .collect::>() + .join(" ") + ), )]), dependencies, ) @@ -361,12 +377,8 @@ mod tests { }) .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(PackagedWorkflowVersions { - root_id, - versions: vec![child, root.clone()], - })), - ); + 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; diff --git a/lib/components/fabro-tool/src/lib.rs b/lib/components/fabro-tool/src/lib.rs index d3ade2e3e..5145e7608 100644 --- a/lib/components/fabro-tool/src/lib.rs +++ b/lib/components/fabro-tool/src/lib.rs @@ -49,6 +49,6 @@ pub use search::{ search_runs, search_runs_text, }; pub use workflow_version::{ - FabroWorkflowVersionCreateParams, PackagedWorkflowVersions, ValidatedWorkflowVersionCreate, - WorkflowVersionPackager, create_workflow_version, workflow_version_create_text, + 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 index 38be81c8b..5a3e51fcf 100644 --- a/lib/components/fabro-tool/src/workflow_version.rs +++ b/lib/components/fabro-tool/src/workflow_version.rs @@ -3,10 +3,8 @@ use std::sync::Arc; use async_trait::async_trait; use fabro_api::types::CreateWorkflowVersionResponse; -use fabro_types::{ - MAX_WORKFLOW_VERSION_BYTES, MAX_WORKFLOW_VERSION_FILE_BYTES, MAX_WORKFLOW_VERSION_FILES, - WorkflowPath, WorkflowVersion, WorkflowVersionId, -}; +use fabro_types::{MAX_WORKFLOW_VERSION_BYTES, WorkflowPath}; +use fabro_workflow_version::CollectedWorkflowClosure; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -41,26 +39,9 @@ impl TryFrom for ValidatedWorkflowVersionCreat fn try_from(params: FabroWorkflowVersionCreateParams) -> Result { let FabroWorkflowVersionCreateParams { entrypoint, files } = params; - if !files.contains_key(&entrypoint) { - return Err(ToolError::message( - "entrypoint must be an exact supplied file key", - )); - } - if files.len() > MAX_WORKFLOW_VERSION_FILES { - return Err(ToolError::message(format!( - "workflow source exceeds {MAX_WORKFLOW_VERSION_FILES} files" - ))); - } - let mut total = 0; - for content in files.values() { - if content.len() > MAX_WORKFLOW_VERSION_FILE_BYTES { - return Err(ToolError::message(format!( - "workflow source file exceeds {} KiB", - MAX_WORKFLOW_VERSION_FILE_BYTES / 1024 - ))); - } - total += content.len(); - } + 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", @@ -73,15 +54,6 @@ impl TryFrom for ValidatedWorkflowVersionCreat } } -/// The complete validated closure for one supplied source tree. -#[derive(Clone, Debug)] -pub struct PackagedWorkflowVersions { - pub root_id: WorkflowVersionId, - /// Every version in the closure, dependencies before the versions that - /// reference them, so callers can register them in this order. - pub versions: Vec, -} - /// 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 @@ -91,7 +63,7 @@ pub trait WorkflowVersionPackager: Send + Sync { async fn package( &self, source: ValidatedWorkflowVersionCreate, - ) -> anyhow::Result; + ) -> anyhow::Result; } pub async fn create_workflow_version( @@ -114,6 +86,7 @@ pub fn workflow_version_create_text(result: &CreateWorkflowVersionResponse) -> S #[cfg(test)] mod tests { + use fabro_types::{MAX_WORKFLOW_VERSION_FILE_BYTES, MAX_WORKFLOW_VERSION_FILES}; use serde_json::json; use super::*; @@ -227,7 +200,7 @@ mod tests { async fn package( &self, _: ValidatedWorkflowVersionCreate, - ) -> anyhow::Result { + ) -> 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/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index e678a8a1f..2259c3814 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -198,6 +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_source_paths, + 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 a957f1b36..58fb8e0ad 100644 --- a/lib/foundation/fabro-types/src/workflow_version.rs +++ b/lib/foundation/fabro-types/src/workflow_version.rs @@ -122,32 +122,13 @@ 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() } @@ -159,6 +140,36 @@ impl WorkflowVersion { } } +/// 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.