The method iterates EnvironmentImageLayer configs, but out of context
"image layers" reads as Docker image layers — a bad collision in
exactly the domain where it appears. Name it for what it yields.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The manifest bundler and workflow-version validation each maintained
their own walk over the graph reference vocabulary (goal files, inline
goal/prompt templates, imports, child workflows, @-file attributes), so
a new reference-bearing attribute had to be added twice or the two
would drift. Add fabro_template::visit_graph_references as the single
per-graph walker — it validates that file references are template-free
and emits typed events — and rewrite both consumers on top of it,
keeping their own IO, resolution, and recursion.
Same for dockerfiles: SettingsLayer::image_layers[_mut] is now the one
definition of where images live in a settings layer, replacing the
three hand-rolled traversals in the run compiler, manifest bundler, and
workflow-version validation.
Behavior note: the manifest bundler now also follows
stack.child_dotfile (already in the shared vocabulary and treated as a
child-workflow reference by workflow-version validation); nothing in
the engine emits or reads that attribute today.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the WorkflowVersion wire type and its structural invariants (file
count and size limits, entrypoint presence, unique keys, path
collisions, canonical form) into fabro-types, so fabro-api replaces the
generated schema type without pulling graph parsing or the template
engine into every API consumer. The wire shape is unchanged.
fabro-workflow-version keeps the expensive graph/config/template
validation behind a ValidatedWorkflowVersion newtype and now owns
WorkflowVersionStore: put only accepts validated versions and get still
re-validates blobs read from shared storage. fabro-store goes back to
being domain-agnostic persistence.
Move the static-reference attribute vocabulary (ReferenceKind,
AttributeScope, reference_kind_for_attribute) to fabro_types::graph and
template-syntax validation to fabro-template, so fabro-graphviz no
longer depends on the template engine.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Validate WorkflowVersion structure once at construction so
canonical_bytes only serializes and enforces the size limit
- Collapse the three pairwise path-collision loops into one over the
combined file and dependency keys
- Make WorkflowPath::is_ancestor_of allocation-free and remove unused
resolve_from_root and error accessors
- Parse WorkflowVersionId via serde into/try_from, delegating length and
charset checks to RunBlobId
- Derive ReferenceKind's Display with strum instead of a hand-written
match
- Drop the fabro-workflow static_reference re-export shim; consumers
import from fabro-graphviz directly
- Collapse duplicate JSON-rejection arms in the workflow-versions
handler
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Route the root workflow through collect_workflow_entry so relative root
arguments are lexically normalized before reading, matching the pre-refactor
behavior: `..` segments no longer resolve through symlinks to a file other
than the one the manifest key names, and `~`-prefixed references are
rejected again. Adds a symlink regression test for the root argument.
Also:
- collect_workflow_entry/collect_workflow_location return the manifest key,
so bundle() no longer recomputes the root key
- hold one FilesystemTemplateStore on the bundler instead of rebuilding it
per template reference
- drop the unused Clone derive on WorkflowScanInput
- replace the hand-rolled JSON literal in the characterization test with an
insta snapshot per the testing strategy
- share one write_file fixture helper between the lib and bundler test
modules
- remove the bundler git-push test; the bundler has no git code path, so the
test could not fail
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Apply cleanup review findings on the collector extraction:
- Deduplicate the lexical path-normalization loop: normalize_absolute_path
now delegates to lexically_normalize_access_path, and it plus
manifest_path_from_absolute live in working_tree.rs so the module
dependency points one way (projection -> collector). Drop the redundant
re-normalization in collect_bundled_file.
- Extract collect_bundled_template_includes to replace the copy-pasted
goal/prompt template-closure sequence, seed_config_document for the
duplicated config seeding, and read_source_input for the duplicated
config reader closures (with the user-settings is_file check hoisted).
- Replace ~100 lines of trivial getters on the Collected* output structs
with pub(super) fields; keep the CollectedPath newtype encapsulated.
- Assemble the manifest by value, moving collected sources into the wire
types instead of deep-copying every file a second time; drop two full
DraftDocument clones that only satisfied the borrow checker; stop
recomputing manifest paths per file in template-dependency verification.
- Resolve the root workflow once in assemble_current_manifest, removing an
unreachable duplicate error path; flatten single-use CollectionNamespace
into a finalize_documents free function.
No behavior change; fabro-manifest tests, clippy, and fmt pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The manifest builder's best-effort pre-run push converted every result
into a PreRunPushOutcome that was serialized into GitContext, expanded
into five OpenAPI union arms, and generated into API clients — but no
production path ever read it; every field read was a test.
Delete the concept while preserving the behavior:
- Drop the PreRunPushOutcome enum and GitContext.push_outcome from
fabro-types; GitContext keeps origin_url, branch, optional sha, and
dirty, which remain real execution inputs and provenance.
- Rename the manifest outcome builder to push_manifest_branch_best_effort,
a side-effect-only helper with the same decision rules: skip without an
origin, skip on configured-repository mismatch, skip when the branch is
already synced, otherwise push noninteractively and discard the result
without failing manifest creation or logging raw Git stderr.
- Prove the push through repository state instead of the deleted enum: a
branch ahead of a local bare origin is pushed during manifest build, a
mismatched configured repository is not, and a failing remote helper
still cannot fail manifest creation.
- Remove push_outcome from GitContext in OpenAPI, delete the five-arm
union schemas, and drop the fabro-api type replacement and re-export.
- Keep one regression proving historical run.created events with a nested
push_outcome still deserialize through ordinary unknown-field tolerance
and reserialize to the reduced shape. No migration or event rewrite.
Old JSON carrying the removed field stays readable. Newly generated
clients omit a field older servers required, so new-client-to-old-server
compatibility is intentionally not promised for this pre-1.0 contract.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop ManifestTarget.identifier (the raw token the user typed) and
ManifestGoal.path (the original goal-file path) from the OpenAPI
manifest schema, the Rust manifest builder, the regenerated Rust and
TypeScript client types, and every canonical test fixture. Neither
field had a production reader: the server selects the workflow by
target.path and consumes only the resolved goal type and text.
Target path, goal type/text, manifest versioning, and submitted-byte
persistence are unchanged. Old request bodies that still carry the
removed properties remain accepted through unknown-field tolerance,
pinned by a dedicated public-route regression test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
