Extract workflow bundling from manifest assembly

[swerner]

Summary

• Extract filesystem, config, workflow, import, template, and Dockerfile traversal into a private WorkflowBundler that produces a validated CollectedWorkflowBundle for the current manifest builder.
• Represent the entrypoint plus all transitively referenced child workflows as one bundle, while each CollectedWorkflow owns its graph, optional config, and associated files.
• Assign every collected document a location-independent virtual coordinate derived from logical roots and original relative references, with uniform component rebasing to eliminate parent components.
• Detect conflicting physical aliases and virtual-coordinate collisions while retaining normalized access paths for the compatibility projection.

Compatibility

• Keep the public manifest and API shape unchanged, including cwd-relative keys and leading parent components for external workflows.
• Keep settings layering, goal resolution, input overrides, Git observation, and the existing best-effort branch push outside bundle collection.
• Preserve error source chains for filesystem, parser, config, and template failures.

This change does not add workflow-definition or run-intent types, APIs, stores, or producer migration. Server admission and lifecycle behavior are independent and untouched.

Tests

• Complete legacy manifest characterization covering root and child workflows, configs, imports, file references, template dependencies, Dockerfiles, goal, args, and input overrides.
• Bundler completeness, relocation stability, external sibling workflows, raw reference preservation, component rebasing, deduplication, alias and collision rejection, error chains, and direct-bundler Git isolation.
• Existing external-workflow bundle round-trip and Git push behavior.
• cargo test -p fabro-manifest
• cargo +nightly-2026-04-14 fmt --check --all
• cargo +nightly-2026-04-14 clippy -p fabro-manifest --all-targets -- -D warnings
• Workspace nextest: 7,790 passed; one unrelated Daytona preflight test timed out under full-suite contention and passed immediately in isolation.

[swerner]

Ran a cleanup review (reuse / simplification / efficiency / altitude) over this PR and applied the contained fixes in 0e703a7 (net −123 lines). No behavior change intended; fabro-manifest tests (including the legacy-projection characterization test), clippy -D warnings, and fmt all pass.

Applied

Reuse / duplication

• Deleted the duplicated lexical path-normalization loop: normalize_absolute_path in lib.rs was a byte-identical copy of the component walk inside working_tree.rs's lexically_normalize_access_path. It now delegates to that loop, and both it and manifest_path_from_absolute moved into working_tree.rs so the module dependency points one way (projection → collector) instead of both ways. Also removed a redundant re-normalization call in collect_bundled_file (a no-op on already-normalized output).
• The goal-file and prompt-file branches in collect_workflow_files shared a copy-pasted 15-line "bundle then collect template includes" sequence — extracted into one collect_bundled_template_includes method, which also removes the doubled self.document(bundled) lookups.
• The two identical config-seeding blocks in collect_working_tree became one seed_config_document helper; the two identical config-reader closures in build_run_manifest became one read_source_input function, with the doubled user_settings_path.is_file() check evaluated once.

Simplification

• Replaced ~100 lines of trivial getters on the five Collected* output structs with pub(super) fields (the module's own tests were already bypassing the getters). The CollectedPath newtype keeps its private field since its constructor enforces a real invariant.
• assemble_current_manifest no longer resolves the root workflow twice with two distinct "root workflow missing" errors — the second lookup was provably unreachable.
• CollectionNamespace, a struct used only inside its own single associated function, is now a free function finalize_documents with two local maps.

Efficiency

• assemble_current_manifest now consumes CollectedWorkingTree by value and moves every source string into the wire types instead of .to_owned()-copying the entire bundle contents a second time.
• Removed two full-DraftDocument clones (including complete file sources) that existed only to satisfy the borrow checker in collect_workflow_files and collect_config_dockerfile — they now clone only the access_path.
• verify_recorded_template_dependencies no longer recomputes manifest_path_from_absolute per collected file (O(templates × files)); it round-trips the files-map key through ManifestPath::from_wire, which is exactly how those keys were built.

Noted but not applied

• Cross-crate reuse with fabro-types — normalize_relative_path, virtual_reference_path, and CollectedPath::try_new partially re-implement ManifestPath::normalize_components / from_reference / is_portable_logical_path. Consolidating means widening fabro-types' public API and would make some validations stricter (a behavior change), so it belongs in its own change.
• Deeper design follow-ups — four sound but larger findings that would restructure the representation this PR deliberately shaped: (1) the legacy wire-path derivation is computed independently in both the collector and the assembler; (2) CollectedFileReference links documents by physical path rather than collected coordinate, forcing consumers to reverse-map; (3) project-config-dockerfile placement is done via remove/mutate/re-insert map surgery in the collector rather than in the compatibility projection; (4) coordinate-conflict detection lives in three places instead of one choke point in insert_document. Worth considering as follow-ups.
• Test-only duplication — the git2-based fixture in collector_does_not_push_an_ahead_branch duplicates the CLI-based git test helpers in lib.rs, and the PR mints two copies of a write-fixture helper across the two test modules. Consolidating means a shared #[cfg(test)] support module; left alone.
• ComponentRole::label → strum — the crate has no strum dependency; adding one for a three-variant const fn isn't a win.

🤖 Generated with Claude Code