Surface path-only packaging failures to the tool caller

Every packaging failure collapsed into one generic message, so an LLM
caller that omitted a child workflow, referenced a prompt with the
wrong case, or exceeded the canonical size limit could not tell what to
fix. The collector's error type already separates variants whose
messages carry only paths and counts from the ones whose sources quote
supplied content.

collect_supplied_workflow_versions now returns the typed collector
error, with new variants for a referenced file missing from the
package root, a collected file the caller did not supply, and staging
I/O failures. The bundler reports missing files with their
package-relative path so the collector can recognize them. The packager
renders the full cause chain for path-only variants and stops at the
last path-only level, plus a hint, for graph, TOML, and template
failures whose diagnostics quote source.

The tool-side raw byte total remains a cheap lower bound; the canonical
limit now surfaces with its own message instead of the generic one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-09-11 15:34:49 -06:00
parent e6eb59537a
commit 9bc6bf4227
5 changed files with 159 additions and 45 deletions

View file

@ -22,6 +22,7 @@ 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
@ -33,7 +34,6 @@ tracing.workspace = true
[dev-dependencies]
fabro-test.workspace = true
fabro-util = { path = "../../foundation/fabro-util" }
insta.workspace = true
serde_json.workspace = true
temp-env = "0.3"

View file

@ -4,12 +4,13 @@
use std::collections::BTreeMap;
use std::path::Path;
use anyhow::Result;
use fabro_config::project::WorkflowLocation;
use fabro_types::WorkflowPath;
use tempfile::TempDir;
use crate::CollectedWorkflowClosure;
use crate::{CollectedWorkflowClosure, WorkflowVersionCollectError};
type Result<T> = std::result::Result<T, WorkflowVersionCollectError>;
/// Stage `files` in a private temporary directory and collect the workflow
/// closure rooted at `entrypoint` with the same collector used for checkouts.
@ -22,34 +23,49 @@ pub fn collect_supplied_workflow_versions(
) -> Result<CollectedWorkflowClosure> {
let staging = tempfile::Builder::new()
.prefix("fabro-workflow-version-")
.tempdir()?;
.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<WorkflowPath, String>,
staging: &TempDir,
) -> Result<CollectedWorkflowClosure> {
let root = staging.path().canonicalize()?;
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)?;
std::fs::create_dir_all(parent).map_err(stage_error)?;
}
std::fs::write(destination, contents)?;
std::fs::write(destination, contents).map_err(stage_error)?;
}
let entrypoint = Path::new(entrypoint.as_str());
let location = WorkflowLocation::from_exact_path(entrypoint, &root)?;
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)?;
// A case-insensitive host must not satisfy a reference that is missing
// from the supplied tree under its exact key.
for (_, version) in closure.versions() {
for path in version.version().files().keys() {
anyhow::ensure!(
files.contains_key(path),
"collected file `{path}` was not supplied"
);
if !files.contains_key(path) {
return Err(WorkflowVersionCollectError::NotSupplied { path: path.clone() });
}
}
}
Ok(closure)
@ -59,6 +75,8 @@ fn collect_in_staging(
mod tests {
use std::path::Path;
use fabro_util::error::collect_chain;
use super::*;
struct Supplied {
@ -243,7 +261,7 @@ mod tests {
let error = collect_with_staging(&input, staging)
.err()
.unwrap_or_else(|| panic!("accepted invalid fixture {index}"));
let rendered = format!("{error:#}");
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!(
@ -277,9 +295,10 @@ mod tests {
]);
let error =
collect_supplied_workflow_versions(&renamed.entrypoint, &renamed.files).unwrap_err();
let rendered = collect_chain(&error).join(": ");
assert!(
format!("{error:#}").contains("must be `sub/workflow.toml`"),
"{error:#}"
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", &[

View file

@ -38,6 +38,15 @@ pub(super) struct CollectedWorkflowSource {
pub(super) dependency_keys: BTreeSet<String>,
}
/// 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<String, toml::Value>) -> Self {
Self {
@ -512,11 +521,16 @@ 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(MissingPackageFile { path });
}
anyhow::Error::new(source).context(format!(
"failed to canonicalize workflow package file `{}`",
path.display()
)
))
})?;
if !canonical.starts_with(self.package_root) {
bail!(

View file

@ -10,7 +10,9 @@ use fabro_types::{
use fabro_workflow_version::{ValidatedWorkflowVersion, WorkflowVersionError};
use thiserror::Error;
use crate::workflow_bundler::{CollectedWorkflowSource, CollectedWorkflowSources, WorkflowBundler};
use crate::workflow_bundler::{
CollectedWorkflowSource, CollectedWorkflowSources, MissingPackageFile, WorkflowBundler,
};
/// One locally packaged workflow-version closure in dependency-first order.
#[derive(Debug)]
@ -80,6 +82,20 @@ 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 },
#[error("failed to stage supplied workflow files")]
Stage {
#[source]
source: std::io::Error,
},
}
/// Package one workflow and every separately runnable dependency from a local
@ -157,9 +173,19 @@ pub 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| {
let missing = source
.chain()
.find_map(|cause| cause.downcast_ref::<MissingPackageFile>());
match missing {
Some(missing) => WorkflowVersionCollectError::MissingPackageFile {
path: missing.path.clone(),
},
None => WorkflowVersionCollectError::Collect {
path: workflow.to_path_buf(),
source,
},
}
})?;
VersionAssembler::new(collected).assemble()
}

View file

@ -5,15 +5,17 @@ use async_trait::async_trait;
use fabro_tool::{
PackagedWorkflowVersions, ToolError, ValidatedWorkflowVersionCreate, WorkflowVersionPackager,
};
use fabro_workflow_version::WorkflowVersionError;
use tokio::task;
use tracing::warn;
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_FAILED: &str = "workflow source could not be packaged; check configuration, \
syntax, local references, and package limits";
const PACKAGING_HINT: &str = "check configuration, syntax, local references, and package limits";
#[async_trait]
impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager {
@ -25,11 +27,8 @@ impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager {
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)
ToolError::message(render_packaging_error(&err))
})?;
Ok(PackagedWorkflowVersions {
root_id: closure.root_id(),
@ -41,6 +40,34 @@ impl WorkflowVersionPackager for SuppliedWorkflowVersionPackager {
}
}
/// 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 { .. } => true,
WorkflowVersionCollectError::InvalidVersion { source, .. } => matches!(
source,
WorkflowVersionError::GraphParse { .. }
| WorkflowVersionError::Template { .. }
| WorkflowVersionError::Config { .. }
),
_ => false,
};
if !quotes_source {
return fabro_util::error::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 super::*;
@ -70,6 +97,14 @@ mod tests {
])
}
async fn package_error(input: ValidatedWorkflowVersionCreate) -> String {
let error = SuppliedWorkflowVersionPackager
.package(input)
.await
.unwrap_err();
format!("{error:#}")
}
#[tokio::test]
async fn packager_returns_dependencies_before_root() {
let packaged = SuppliedWorkflowVersionPackager
@ -101,27 +136,34 @@ mod tests {
"_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.goal]\nfile = \"missing.md\""
.into(),
);
let mut oversized = fixture();
oversized.files.insert(
"prompt.md".parse().unwrap(),
"\u{1}".repeat(fabro_types::MAX_WORKFLOW_VERSION_FILE_BYTES - 1),
let mut invalid_config = fixture();
invalid_config.files.insert(
"workflow.toml".parse().unwrap(),
"_version = 1\nPRIVATE_CONTENT = [unterminated".into(),
);
for input in [
invalid_root,
oversized,
invalid_config,
source("workflow", &[(
"workflow",
"PRIVATE_CONTENT invalid source",
)]),
] {
let error = SuppliedWorkflowVersionPackager
.package(input)
.await
.unwrap_err();
let rendered = format!("{error:#}");
let rendered = package_error(input).await;
assert!(!rendered.contains("PRIVATE_CONTENT"), "{rendered}");
assert_eq!(rendered, PACKAGING_FAILED);
}
}
#[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
@ -130,12 +172,25 @@ mod tests {
wrong_case
.files
.insert("Prompt.md".parse().unwrap(), prompt);
assert!(
SuppliedWorkflowVersionPackager
.package(wrong_case)
.await
.is_err(),
"a case-insensitive host must not satisfy an exact reference"
// 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}");
}
}