Create a cleanup roadmap to address the items identified in the audit

docs/roadmap/audit.md

@@ -0,0 +1,592 @@
+# Code Quality and Deduplication Audit
+
+## 1. Executive summary
+
+Overall code quality is strong. The repository has clear package boundaries, good current-behavior documentation, focused adapter packages, and tests close to most implemented behavior. The most important cleanup opportunities are narrow and behavior-preserving rather than architectural.
+
+Top three refactoring targets:
+
+1. HTTP upload admission, body staging, and archive validation are split across `internal/app` and `internal/ingest` in a way that duplicates size and content-type policy and buffers uploads in memory.
+2. Runtime config loading, default config path selection, secret loading, and warning projection are repeated across app entrypoints.
+3. Run orchestration mixes destination processing, failure aggregation, warning recording, and report event ordering in one large loop, making future changes harder to review safely.
+
+The codebase appears ready for a limited cleanup pass. I do not see a major architectural risk that requires a redesign before the next release.
+
+## 2. Repository map reviewed
+
+Reviewed policy and current-behavior documentation:
+
+- `AGENTS.md`
+- `README.md`
+- `docs/policy/architecture.md`
+- `docs/policy/development.md`
+- `docs/policy/documentation.md`
+- `docs/config.md`
+- `docs/cli.md`
+- `docs/operations.md`
+- `docs/troubleshooting.md`
+- `docs/internal/*.md`
+- `docs/roadmap/http.md`
+- `docs/roadmap/implementation.md`
+
+Reviewed implementation areas:
+
+- `cmd/distributor`: executable entrypoint.
+- `internal/cli`: root command, `version`, `run`, `serve`, `validate`, `inspect`, and `manifest create` parsing.
+- `internal/app`: run orchestration, configured source diagnostics, backend factory, manifest creation, CLI output, HTTP upload server, upload coordinator, and pipeline coordinator.
+- `internal/config`: config structs, defaults, validation, quantity parsing, S3/SSH helpers, and secrets resolver.
+- `internal/bundle`: storage-backed source discovery and validation.
+- `pkg/bundle`: public manifest model, digest logic, manifest building, local validation, and local bundle writer.
+- `internal/storage` and `internal/storage/fake`: backend interface, path helpers, walk helpers, typed errors, and fake backend.
+- `internal/adapters/local`, `internal/adapters/ssh`, and `internal/adapters/s3`: runtime storage adapters.
+- `internal/ingest`: HTTP upload archive staging.
+- `internal/publish`: destination planning, output selection, link projection, state writing, cleanup, and force replacement.
+- `internal/state`: destination state parsing, validation, comparison, and JSON projection.
+- `internal/transform` and `internal/transform/markdown`: transform registry and Markdown rendering.
+- `internal/link`, `internal/notify`, `internal/logging`, and `internal/testutil`.
+- `examples`, package tests, and package `testdata`.
+
+Requested areas that are absent as separate packages:
+
+- `internal/stage`
+- `internal/modules`
+- `internal/validators`
+- `internal/artifacts`
+- `internal/manifest`
+- `internal/schema`
+- `internal/report`
+- public `pkg` packages other than `pkg/bundle`
+
+Those absences are consistent with current architecture policy; the corresponding behavior lives in narrower existing packages.
+
+## 3. High-confidence deduplication opportunities
+
+### HTTP upload body handling should be owned by ingestion
+
+Affected files/packages:
+
+- `internal/app/upload_http.go`
+- `internal/app/upload_coordinator.go`
+- `internal/ingest/archive.go`
+- `internal/app/upload_http_test.go`
+- `internal/app/upload_http_integration_test.go`
+- `internal/ingest/archive_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- `internal/app/upload_http.go` validates upload content types in `supportedUploadContentType`, while `internal/ingest/archive.go` validates the same content types in `archiveFormat`.
+- `internal/app/upload_http.go` enforces upload size in `readUploadBody`, while `internal/ingest/archive.go` enforces upload size again in `writeLimited`.
+- The HTTP handler reads the full upload body into memory before submission, then the coordinator passes a `bytes.Reader` to ingestion.
+
+Why it matters:
+
+- The app transport layer now partially owns archive policy that should belong to `internal/ingest`.
+- Large accepted uploads are buffered in memory even though ingestion already has streaming-to-disk mechanics.
+- Future archive formats, content types, or upload limit changes would need coordinated edits in multiple packages.
+
+Recommended refactor:
+
+- Move supported content-type checking behind an ingestion-owned helper, for example `ingest.ValidateContentType` or `ingest.IsSupportedContentType`.
+- Change upload admission so the request body is streamed to staging exactly once before the HTTP handler returns `202 Accepted`.
+- Keep queue-full rejection before reading the body.
+- Queue a staged local bundle root, not an unread request body. This preserves async distribution while keeping HTTP request lifetime separate from later pipeline execution.
+- Keep `UploadCoordinator` responsible for queueing, status, per-pipeline serialization, and execution. Keep `internal/ingest` responsible for archive format, size, extraction, cleanup, and source bundle validation.
+
+Suggested tests:
+
+- HTTP handler rejects full queues without reading the body.
+- HTTP handler streams a valid body to ingestion and returns `202` only after staging succeeds.
+- Unsupported content types are rejected through the ingestion-owned content-type policy.
+- Oversized uploads are rejected without retaining a staged run.
+- Accepted upload status still transitions through queued/running/succeeded or failed without depending on an open HTTP request body.
+
+Risk level:
+
+- Medium. The behavior change is internal but touches admission timing and async execution boundaries. It should be implemented in a focused prompt with existing HTTP integration tests extended first.
+
+### Runtime config and secret setup should have one app-level helper
+
+Affected files/packages:
+
+- `internal/app/run.go`
+- `internal/app/source_select.go`
+- `internal/app/serve.go`
+- `internal/app/backends.go`
+- `internal/app/run_warnings.go`
+- `internal/config`
+- `internal/app/*_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- Defaulting an empty config path to `config.DefaultConfigPath` appears in `Run`, `RunPipeline`, `RunPipelineWithLocalSource`, and `Serve`.
+- Config loading and secret loading are separate repeated steps in `buildRunReport`, `selectSourceBundlesFromConfig`, and `Serve`.
+- Secret conflict warnings are projected in run and configured source diagnostics, while serve loads secrets without using or exposing conflict warning metadata.
+- Backend factory construction from a config environment is repeated through provider plumbing.
+
+Why it matters:
+
+- Config and secret precedence is a public operational policy.
+- A future change to config discovery, secret conflict reporting, or runtime environment construction could drift between `run`, `serve`, `validate`, and `inspect`.
+- Tests for secrets and credential resolution need to cover several entrypoints today.
+
+Recommended refactor:
+
+- Add a small app-level runtime setup helper, for example `loadRuntimeConfig(optionsConfigPath string) (runtimeConfig, error)`.
+- The helper should own default config path selection, `config.LoadFile`, `config.LoadSecretEnvironment`, and conversion of secret conflicts into `OutputWarning` values.
+- Keep config parsing and validation in `internal/config`; the helper should not duplicate config policy.
+- Let `run`, configured `validate`/`inspect`, and `serve` call the helper and then apply command-specific behavior.
+
+Suggested tests:
+
+- One focused app test proving default config path selection remains unchanged where injection permits it.
+- Existing secret conflict JSON/text warning tests for `run`, `validate`, and `inspect`.
+- Serve startup test proving duplicate and missing upload tokens still fail without leaking values.
+- S3 explicit credential tests proving the resolver is still used through the helper.
+
+Risk level:
+
+- Low to medium. This is a straightforward centralization but touches several command entrypoints.
+
+### Run destination processing needs a narrow helper boundary
+
+Affected files/packages:
+
+- `internal/app/run.go`
+- `internal/app/run_output.go`
+- `internal/app/run_failures.go`
+- `internal/app/run_selection.go`
+- `internal/publish`
+- `internal/app/run_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- Destination backend open failures and publish planning/execution failures each manually add `runFailures`, record summary failure counts, append `RunActionRecord`, and append pipeline event indexes.
+- `publish.Build` error handling patches missing `Plan` identity fields inline before converting the plan to a run action.
+- Fixed-path warning emission is interleaved with destination selection and publish plan handling.
+
+Why it matters:
+
+- `run --format json` depends on exact action ordering, warnings, partial failures, and summary counters.
+- Future changes to actions, links, notifications, or HTTP upload reports could accidentally update one failure path but not another.
+- The current loop is correct but dense enough that small behavior changes are hard to review.
+
+Recommended refactor:
+
+- Extract a narrow `runDestination` or `destinationRunner` helper that processes one destination and returns action records, warnings, summary deltas, and failures.
+- Add a helper for recording a destination-scoped failure that updates `runFailures`, `runSummary`, `RunReport.Actions`, and pipeline events in one place.
+- Add a helper that normalizes partial `publish.Plan` identity fields before action projection.
+- Do not introduce a generic workflow engine or stage abstraction.
+
+Suggested tests:
+
+- Preserve existing run text output golden assertions.
+- Preserve JSON partial-result behavior when planning fails after destination processing begins.
+- Add one focused test where destination open fails for multiple selected bundles and verify action records, output errors, and summary counters stay aligned.
+- Add one fixed-path dry-run warning test after extraction to verify event ordering.
+
+Risk level:
+
+- Medium. The refactor is behavior-preserving but touches the most important user-facing workflow.
+
+### Archive path validation duplicates source path policy with a different error surface
+
+Affected files/packages:
+
+- `internal/ingest/archive.go`
+- `pkg/bundle/path.go`
+- `internal/storage/path.go`
+- `internal/ingest/archive_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- `cleanArchivePath`, `pkg/bundle.ValidateSourcePath`, and `storage.ValidatePath` all enforce clean slash-separated relative paths with no backslashes, no absolute paths, and no dot segments.
+- Archive staging needs slightly different policy because directories are allowed and `manifest.json` is allowed only at the root, so the duplication is not completely mechanical.
+
+Why it matters:
+
+- Path safety is high-risk behavior.
+- Future changes to source path rules could miss archive extraction, especially around backslashes, reserved names, or dot segments.
+
+Recommended refactor:
+
+- Keep archive-specific rules in `internal/ingest`, but use a shared path-checking primitive where possible.
+- A good shape is an exported `pkg/bundle.ValidatePathSegmented` only if it fits the public producer API, or an internal helper in ingestion that delegates file-entry validation to `pkg/bundle.ValidateSourcePath` for regular files after handling directory-specific exceptions.
+- Preserve current archive-specific errors and tests.
+
+Suggested tests:
+
+- Table tests shared or mirrored across bundle path validation and archive path cleaning for absolute paths, traversal, backslashes, dot segments, empty names, root `manifest.json`, nested `manifest.json`, and `.distributor.json`.
+- Regression tests proving directories are still accepted in archives but symlinks and hardlinks remain rejected.
+
+Risk level:
+
+- Low to medium. Path validation changes need careful tests, but the desired change can be small.
+
+## 4. Medium-confidence opportunities
+
+### Source and destination backend config shapes could expose a normalized view
+
+Affected files/packages:
+
+- `internal/config/config.go`
+- `internal/config/defaults.go`
+- `internal/config/validate.go`
+- `internal/app/backends.go`
+- `internal/config/*_test.go`
+- `internal/app/backends_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- `config.Backend` and `config.Destination` duplicate backend fields for local, SSH, S3, and credentials.
+- Defaults for source backends and destination backends are implemented in separate functions.
+- App backend opening converts both shapes into `backendOpenSpec`.
+
+Why it matters:
+
+- New backend fields must be added to both YAML structs, defaulting paths, validation paths, app open-spec conversion, docs, and tests.
+- The current pattern is easy to understand but likely to drift as more backend-specific fields are added.
+
+Recommended refactor:
+
+- Keep the YAML shape unchanged for compatibility.
+- Add package-local helpers in `internal/config` that return a normalized backend view for either source or destination.
+- Use that view for shared backend defaulting and validation where it improves clarity.
+- Keep destination-only fields such as `publish`, `transfer`, `links`, and `path_mapping` on `Destination`.
+
+Suggested tests:
+
+- Existing source and destination backend validation tests should continue to pass.
+- Add a table test that validates equivalent local, SSH, and S3 source/destination backend field requirements through the shared view.
+- Add a test that `http_upload` remains source-only.
+
+Risk level:
+
+- Medium. This reduces future drift, but the current duplication is understandable and does not need to be the first cleanup.
+
+### CLI command scaffolding is mostly shared, but manifest create has special parsing
+
+Affected files/packages:
+
+- `internal/cli/run.go`
+- `internal/cli/serve.go`
+- `internal/cli/source_mode.go`
+- `internal/cli/manifest.go`
+- `internal/cli/version.go`
+- `internal/cli/root_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- Several commands repeat `flag.NewFlagSet`, `SetOutput`, help handling, format parsing, and usage exit handling.
+- `manifest create` uses `splitManifestCreateArgs` to allow a positional bundle path before flags, unlike Go's default `flag` behavior.
+
+Why it matters:
+
+- CLI syntax and error behavior are public.
+- A broad CLI helper could accidentally obscure command-specific parsing, but a narrow helper could reduce repeated setup and invalid-format handling.
+
+Recommended refactor:
+
+- Do not introduce a CLI framework.
+- Consider a tiny helper for common `FlagSet` creation and output-format parsing after higher-value app/config cleanup.
+- Keep `manifest create` custom parsing local unless another command needs the same interspersed positional behavior.
+
+Suggested tests:
+
+- Preserve current CLI usage-error tests.
+- Add explicit tests for `manifest create <path> --id x`, `manifest create --id x <path>`, and invalid missing flag values before any parser cleanup.
+
+Risk level:
+
+- Low if kept narrow; medium if over-generalized.
+
+### Output DTOs repeat bundle metadata projection
+
+Affected files/packages:
+
+- `internal/app/validate.go`
+- `internal/app/inspect.go`
+- `internal/app/manifest.go`
+- `internal/app/run_output.go`
+- `internal/app/output.go`
+
+Duplicated or near-duplicated behavior:
+
+- `inspect` and `manifest create` both project bundle file metadata into command-specific JSON structs.
+- `validate`, `inspect`, and `manifest create` each define local result types and file record types.
+- RFC3339 formatting uses both `time.RFC3339` and the equivalent literal layout string.
+
+Why it matters:
+
+- JSON output is now a public interface.
+- Repeated projection can drift in field names, timestamp formatting, or path display rules.
+
+Recommended refactor:
+
+- Add a small app-local projection helper for bundle summaries and manifest file records.
+- Use `time.RFC3339` instead of literal RFC3339 layouts.
+- Keep command-specific result structs where the command output semantics differ.
+
+Suggested tests:
+
+- JSON structural tests for `validate`, `inspect`, and `manifest create` before and after the helper.
+- A timestamp-format assertion using an offset timestamp to confirm current behavior is preserved.
+
+Risk level:
+
+- Low.
+
+### PipelineRunCoordinator overlaps conceptually with UploadCoordinator
+
+Affected files/packages:
+
+- `internal/app/run_coordinator.go`
+- `internal/app/upload_coordinator.go`
+- `docs/internal/app.md`
+- `internal/app/run_coordinator_test.go`
+- `internal/app/upload_coordinator_test.go`
+
+Duplicated or near-duplicated behavior:
+
+- Both coordinators define run records, statuses, timestamps, status transitions, context handling, and active pipeline protection.
+- The upload coordinator additionally queues, stages, expires status records, and serializes same-pipeline upload execution.
+
+Why it matters:
+
+- The concepts are similar enough to confuse future contributors.
+- However, the behavior is not identical: one rejects duplicate active runs, while the other queues accepted uploads.
+
+Recommended refactor:
+
+- Do not merge the coordinators now.
+- Review whether `PipelineRunCoordinator` is still needed as an exported app-level helper. If it is intended for future transports, document that role clearly. If not, remove it and its tests in a separate dead-code cleanup.
+- If both remain, extract only tiny shared timestamp/status helpers if a real third coordinator appears.
+
+Suggested tests:
+
+- If retained, keep existing duplicate-run tests.
+- If removed, run `go test ./internal/app ./internal/cli` and verify no current behavior depended on it.
+
+Risk level:
+
+- Low for documentation clarification, medium for removal because it is exported from an internal package and documented for maintainers.
+
+## 5. Boundary and responsibility concerns
+
+The major boundaries are sound:
+
+- CLI parsing stays in `internal/cli`.
+- Config defaults and validation stay in `internal/config`.
+- Backend-specific filesystem, SFTP, and S3 behavior stays in adapters.
+- Manifest semantics are centralized in `pkg/bundle`, with `internal/bundle` adding storage-backed discovery and validation.
+- Destination state comparison stays in `internal/state`.
+- Publish planning/execution stays in `internal/publish`.
+- Transform implementation is behind `internal/transform`.
+
+Concerns worth addressing:
+
+- HTTP upload request-body staging currently crosses the app/ingest boundary. The app transport layer should not own body buffering and archive size enforcement beyond admission and HTTP status projection.
+- Runtime config setup is app-layer behavior, but it is repeated rather than named. A runtime setup helper would clarify the boundary between `internal/config` and command-specific execution.
+- `internal/app/run.go` owns too many destination-loop details. Extracting a destination processing helper would keep orchestration in app while reducing local complexity.
+
+Recommended homes:
+
+- Upload archive policy: `internal/ingest`.
+- HTTP route/auth/status mapping: `internal/app/upload_http.go`.
+- Queueing/status/execution: `internal/app/upload_coordinator.go`.
+- Runtime config plus secret setup: a small helper in `internal/app`, using `internal/config`.
+- Path and state filenames: keep in `internal/storage`.
+
+## 6. Path, key, and naming construction review
+
+Centralized and healthy areas:
+
+- `storage.StateFileName`, `storage.StatePath`, `storage.ManagedBundleTargets`, `storage.Join`, `storage.DisplayPath`, and logical path validation are used in core publication and tests.
+- S3 object-key mapping is contained in `internal/adapters/s3`.
+- SSH and local native path conversion stay inside their adapters.
+- Link URL construction is isolated in `internal/publish/links.go` and URL validation in `internal/link`.
+- Manifest name and schema version are centralized in `pkg/bundle`, with `internal/bundle` aliases.
+
+Areas needing cleanup:
+
+- Archive path cleaning duplicates much of source/storage path policy and should either delegate to a shared primitive or be tightly covered by mirrored tests.
+- Upload run ID construction is isolated, but the shape is partly policy. Keep tests around `<pipeline_id>.<utc_timestamp>.<random_suffix>` before changing coordinator code.
+- Some app tests still construct destination state and source paths locally. `internal/testutil` already covers many cases; additional helper use should be opportunistic, not a sweeping test rewrite.
+
+## 7. Resolution and catalog review
+
+Named concept resolution is mostly consistent:
+
+- Backend names are defined in `internal/config/defaults.go`.
+- Runtime backend construction is app-owned through `backendFactory` and the storage registry.
+- Transform names are defined in `internal/transform`, and app wiring owns concrete registration.
+- Publish/transform policy combinations use `config.ValidatePublishTransformPolicy`.
+- Configured source selection for `validate` and `inspect` is shared in `source_select.go`.
+
+Potential refinements:
+
+- A normalized backend config view would make backend field resolution less repetitive across source and destination config.
+- Transform and backend registries should remain separate; there is no evidence that a generic registry abstraction would help.
+- No separate catalog package is needed for the current feature set.
+
+## 8. Config and command-loading review
+
+Config loading is reliable and strict:
+
+- YAML known-field checking is enabled.
+- Defaults are applied before validation.
+- Validation collects multiple field errors.
+- Secrets are loaded without mutating `os.Environ`.
+- Explicit S3 credential references use the config-owned resolver.
+
+Likely accidental duplication:
+
+- Default config path selection and `config.LoadFile` are repeated in several app entrypoints.
+- Secret loading is repeated in run, source diagnostics, and serve.
+- Secret conflict warning projection is not represented by one runtime setup result.
+
+Intentional differences:
+
+- `serve` loads upload tokens and does not produce CLI JSON output.
+- `validate` and `inspect` support local-path shortcut mode, while `run` and `serve` are config-driven.
+- `manifest create` is local filesystem producer tooling and does not load app config.
+
+Recommended cleanup:
+
+- Centralize runtime config and secret setup in `internal/app`.
+- Keep CLI flag parsing local to command files.
+- Keep `manifest create` outside runtime config loading.
+
+## 9. State, manifest, or progress handling review
+
+Manifest handling is in good shape:
+
+- `pkg/bundle` owns manifest parsing, digest grammar, source path validation, canonical bundle digest, local manifest building, and local bundle writing.
+- `internal/bundle` delegates normalized manifest semantics to `pkg/bundle` and adds storage-backed validation.
+- Destination state embeds the normalized manifest and validates through `internal/bundle`/`pkg/bundle`.
+
+State handling is in good shape:
+
+- `.distributor.json` parsing, validation, JSON projection, and comparison live in `internal/state`.
+- Publish execution writes destination state only after outputs are written.
+- Managed replacement deletes only state-listed outputs plus `.distributor.json`; forced replacement is explicit and bounded.
+
+Progress/status handling:
+
+- `RunReport` is the core run result model and supports JSON partial-result output.
+- HTTP upload status is memory-only and documented as such.
+- `PipelineRunCoordinator` and `UploadCoordinator` overlap conceptually but have different policies. Avoid merging unless product behavior converges.
+
+Gaps:
+
+- HTTP upload staging currently stores the request body in memory before queueing. This is both a quality gap and a mismatch with the intended ingestion boundary.
+- There is no durable upload status, but this is documented as deferred work and should not be addressed in cleanup.
+
+## 10. Refactors to avoid
+
+Avoid these changes in the cleanup pass:
+
+- Do not introduce a generic workflow engine or stage framework. The current explicit workflow is easier to audit.
+- Do not add a CLI framework. The standard-library CLI is sufficient and policy-approved.
+- Do not merge local, SSH, S3, and fake adapters behind a shared implementation layer. Their semantics differ enough that generic helpers would likely hide important behavior.
+- Do not collapse `pkg/bundle` and `internal/bundle`. The public producer API and storage-backed distributor validation have different responsibilities.
+- Do not move destination state comparison into `publish` or app orchestration.
+- Do not redesign JSON output envelopes while doing cleanup.
+- Do not add durable queues, retry workers, HTTP TLS, zstd, or browser UI under the banner of refactoring. These are feature work.
+- Do not rewrite tests wholesale to use a new fixture system. Add helpers only where they reduce immediate duplication around changed code.
+
+## 11. Recommended implementation sequence
+
+1. HTTP upload staging boundary cleanup.
+   - Move supported content-type policy to `internal/ingest`.
+   - Stop buffering accepted uploads in `upload_http.go`.
+   - Queue staged bundle roots rather than request bodies.
+   - Extend HTTP upload tests first.
+
+2. Runtime config setup helper.
+   - Add an app-level helper for default config path, config load, secret load, environment resolver, and secret warnings.
+   - Use it from `run`, configured `validate`/`inspect`, and `serve` where applicable.
+   - Preserve command-specific behavior.
+
+3. Run destination processing extraction.
+   - Add small helpers for destination-scoped failure recording and plan identity normalization.
+   - Extract one-destination processing only if the helper remains readable.
+   - Preserve action ordering and report output.
+
+4. Backend config normalized view.
+   - Add source/destination backend view helpers in `internal/config`.
+   - Use them for defaulting and validation if tests show the shape remains clear.
+   - Keep YAML structs and public config unchanged.
+
+5. Bundle output projection cleanup.
+   - Add app-local helpers for file record and bundle summary projection.
+   - Use `time.RFC3339` consistently.
+   - Preserve command-specific JSON field names.
+
+6. Archive/source path validation test alignment.
+   - Add mirrored path safety tests around ingestion and bundle path validation.
+   - Only centralize code if the helper does not blur archive directory semantics.
+
+7. Coordinator intent cleanup.
+   - Decide whether `PipelineRunCoordinator` is retained for internal future use.
+   - If retained, clarify comments/docs. If removed, do it as a separate dead-code commit.
+
+8. Test helper cleanup.
+   - Expand `internal/testutil` only for repeated setup touched by the previous refactors.
+   - Avoid moving every test fixture.
+
+## 12. Test strategy
+
+Tests to add before refactoring:
+
+- HTTP upload handler test proving queue-full rejection does not consume the body.
+- HTTP upload test proving accepted upload staging completes before `202 Accepted`.
+- Ingestion content-type policy tests exposed through the new helper.
+- Run report test covering destination open failure for multiple selected bundles.
+- CLI JSON tests for `inspect` and `manifest create` timestamp formatting before projection cleanup.
+
+Tests to run with each cleanup stage:
+
+- HTTP upload cleanup: `go test ./internal/ingest ./internal/app ./internal/cli`
+- Config setup cleanup: `go test ./internal/config ./internal/app ./internal/cli`
+- Run processing cleanup: `go test ./internal/app ./internal/publish ./internal/state`
+- Backend config view cleanup: `go test ./internal/config ./internal/app`
+- Output projection cleanup: `go test ./internal/app ./internal/cli`
+- Path validation cleanup: `go test ./pkg/bundle ./internal/bundle ./internal/ingest ./internal/storage`
+- Final cleanup validation: `go test ./...`
+
+Useful read-only checks:
+
+- `rg -n "LoadFile\\(|LoadSecretEnvironment\\(|DefaultConfigPath" internal/app internal/cli`
+- `rg -n "application/x-tar|application/gzip|application/x-gzip" internal docs`
+- `rg -n "2006-01-02T15:04:05Z07:00" internal pkg`
+- `rg -n "manifest.json|\\.distributor.json|StatePath|DisplayPath" internal pkg`
+
+## 13. Appendix: findings not worth acting on
+
+Adapter `ReadFile` and `WriteFile` wrappers:
+
+- Local, SSH, S3, and fake backends each implement byte helpers in terms of stream helpers. This is small duplication but appropriate because each adapter owns error translation and metadata semantics.
+
+Adapter traversal implementation:
+
+- Local filesystem walking, SFTP walking, and S3 pagination look similar at the interface level but are semantically different. Keep traversal mechanics in adapters and shared callback behavior in `storage.WalkEmitter`.
+
+State and manifest raw JSON parsing:
+
+- `pkg/bundle` and `internal/state` both parse raw JSON with pointer fields to detect missing required fields. The schemas and error contexts differ, so a generic required-field parser would not be worth the complexity.
+
+CLI help text:
+
+- Help text repeats command names and flags. This is acceptable in a small hand-written CLI and keeps command files readable.
+
+Test fixture strings:
+
+- Some tests inline YAML snippets or expected output strings despite `internal/testutil`. Inline data is often clearer for edge cases. Only centralize fixture setup when tests are already being changed for a behavior-preserving refactor.
+
+HTTP JSON response helpers:
+
+- HTTP API responses use simple JSON objects rather than the CLI JSON envelope. This is intentional because HTTP status codes and route-specific responses are not the same public interface as CLI command output.
+
+Public and internal bundle validation:
+
+- `pkg/bundle.ValidateBundle` is local-filesystem producer validation; `internal/bundle.Validate` is storage-backed distributor validation. Keep both, with shared manifest semantics delegated through `pkg/bundle`.

docs/roadmap/cleanup.md

@@ -0,0 +1,400 @@
+# Code Quality Cleanup Roadmap
+
+## Current Baseline
+
+The codebase has completed the local, SSH/SFTP, S3, public bundle package,
+manifest creation, JSON output, path mapping, link generation, and HTTP upload
+work documented in the current user and internal docs.
+
+The audit in `docs/roadmap/audit.md` found no major architectural risk. The
+remaining cleanup work should be narrow, behavior-preserving, and focused on
+reducing drift in upload staging, runtime config setup, run reporting, backend
+config handling, output projection, path validation tests, and internal
+coordination code.
+
+One intentional behavior change is part of this cleanup roadmap: malformed
+authenticated upload archives should be rejected before `202 Accepted`, rather
+than accepted and later marked failed. Valid staged uploads should still run
+asynchronously after admission.
+
+## Cleanup Principles
+
+- Preserve public CLI behavior, config schema, manifest schema, destination
+  state schema, backend behavior, and JSON envelopes unless a stage explicitly
+  says otherwise.
+- Keep config parsing and validation in `internal/config`.
+- Keep CLI parsing in `internal/cli`.
+- Keep upload archive policy in `internal/ingest`; keep HTTP routing,
+  authentication, and status projection in `internal/app`.
+- Keep backend-specific filesystem, SSH/SFTP, and S3 behavior in adapter
+  packages.
+- Prefer small package-local helpers over broad abstractions.
+- Add or strengthen tests before refactoring behavior that affects public
+  output, upload admission, path safety, or run reporting.
+
+## Active Cleanup Stages
+
+Implement these stages in order. Each stage should be small enough for one
+implementation prompt and should leave the repository passing the listed focused
+tests before moving to the next stage.
+
+## Stage 1: HTTP Upload Staging Boundary
+
+Goal:
+
+Move archive validation and upload body staging fully behind `internal/ingest`,
+stop app-layer full-body buffering, and reject malformed archives before
+returning `202 Accepted`.
+
+Implementation scope:
+
+- Add an ingestion-owned content-type helper, such as
+  `ValidateContentType(contentType string) error`, and remove duplicated
+  content-type policy from the HTTP handler.
+- Replace the current handler-side `readUploadBody` buffering with streaming
+  staging through `internal/ingest`.
+- Introduce a two-step upload coordinator admission model:
+  - reserve a run id and queue slot before consuming the request body;
+  - stage and validate the archive using that reserved run id;
+  - enqueue only a successfully staged local bundle root for async execution.
+- Keep queue-full rejection before reading the body.
+- Preserve `401` for missing or invalid bearer tokens, `415` for unsupported
+  content type, `413` for oversized uploads, and `503` for a full queue.
+- Return a pre-acceptance `400` for malformed tar/gzip content or invalid
+  staged bundles.
+- Preserve async queued/running/succeeded/failed status after a valid staged
+  bundle is accepted.
+- Do not add durable queues, idempotency keys, zstd, or new routes.
+
+Current-behavior documentation updates:
+
+- Update `docs/cli.md`, `docs/config.md`, `docs/operations.md`,
+  `docs/troubleshooting.md`, `docs/internal/app.md`, and
+  `docs/internal/ingest.md` only as needed to describe the new
+  pre-acceptance failure boundary.
+
+Tests:
+
+- `go test ./internal/ingest ./internal/app ./internal/cli`
+- Queue-full upload rejection does not read the request body.
+- Unsupported content type is rejected through ingestion-owned policy.
+- Oversized uploads return `413` and do not retain a staged run.
+- Malformed tar/gzip content returns `400` before a run id is issued.
+- Valid tar and tar.gz uploads return `202` after staging and still transition
+  through async status.
+- HTTP responses and status records do not leak bearer tokens or secret values.
+
+Completion criteria:
+
+- `internal/app` no longer buffers the full upload body before staging.
+- A valid accepted upload has a committed staged bundle root before the `202`
+  response is sent.
+- Invalid archive content cannot create an accepted run id.
+
+## Stage 2: Runtime Config And Secret Setup Helper
+
+Goal:
+
+Centralize runtime config path resolution, config loading, secret loading,
+environment resolver creation, and secret-conflict warning projection in one
+app-layer helper.
+
+Implementation scope:
+
+- Add a small `internal/app` runtime setup helper that:
+  - defaults an empty config path to `config.DefaultConfigPath`;
+  - calls `config.LoadFile`;
+  - calls `config.LoadSecretEnvironment`;
+  - exposes the loaded config, config path, `config.Environment`, and
+    `[]OutputWarning` for secret conflicts.
+- Use the helper from `Run`, `RunPipeline`, `RunPipelineWithLocalSource`,
+  configured `Validate`/`Inspect`, and `Serve` where applicable.
+- Keep `manifest create` outside runtime config loading.
+- Keep YAML structs, defaults, validation, and secret-directory parsing in
+  `internal/config`.
+- Preserve app test injection points for backend factories and upload handler
+  tests.
+
+Current-behavior documentation updates:
+
+- Update `docs/internal/app.md` if helper boundaries or flow descriptions
+  change. User-facing docs should not change unless observable behavior changes.
+
+Tests:
+
+- `go test ./internal/config ./internal/app ./internal/cli`
+- Default config path behavior remains unchanged.
+- Secret conflict warnings still appear in text and JSON output for `run`,
+  configured `validate`, and configured `inspect`.
+- `serve` still fails startup safely for missing, empty, or duplicate upload
+  tokens without leaking values.
+- Explicit S3 credential references still resolve through the config-owned
+  environment resolver.
+
+Completion criteria:
+
+- Runtime commands no longer repeat config path defaulting and secret loading.
+- Config policy remains owned by `internal/config`.
+
+## Stage 3: Run Destination Processing Extraction
+
+Goal:
+
+Reduce complexity in the main run loop while preserving run report behavior,
+warning ordering, action ordering, failure aggregation, and text/JSON output.
+
+Implementation scope:
+
+- Extract narrow helpers from `internal/app/run.go` for destination-scoped
+  processing.
+- Centralize destination-scoped failure recording so one helper updates
+  `runFailures`, `runSummary`, `RunReport.Actions`, and pipeline events.
+- Centralize normalization of partial `publish.Plan` identity fields before
+  converting plans to run action records.
+- Keep app orchestration explicit; do not introduce a generic workflow engine,
+  stage framework, or broad runner abstraction.
+- Preserve independent destination fan-out and partial-result behavior.
+
+Current-behavior documentation updates:
+
+- Update `docs/internal/app.md` only if helper names or package layout
+  descriptions materially change.
+
+Tests:
+
+- `go test ./internal/app ./internal/publish ./internal/state`
+- Destination open failures for multiple selected bundles keep action records,
+  output errors, summary counters, and pipeline events aligned.
+- JSON partial-result output remains unchanged when destination planning or
+  execution fails after a report exists.
+- Fixed-path dry-run warnings appear in the same order as before.
+- Existing run text output assertions continue to pass.
+
+Completion criteria:
+
+- `run.go` delegates destination-scoped record/failure bookkeeping to helpers.
+- No public output shape or ordering changes.
+
+## Stage 4: Backend Config Normalized View
+
+Goal:
+
+Reduce source/destination backend config drift while preserving the current YAML
+schema and public config behavior.
+
+Implementation scope:
+
+- Add package-local normalized backend view helpers in `internal/config` for
+  source and destination backend fields.
+- Use the normalized view to reduce duplication in backend defaulting and
+  validation where it remains clearer than the current paired code.
+- Keep `config.Backend` and `config.Destination` YAML structs and tags
+  unchanged.
+- Preserve destination-only policy fields on `Destination`.
+- Preserve `http_upload` as source-only and invalid for destinations.
+- Update app backend opening only if the normalized view provides clearer
+  handoff without leaking config internals.
+
+Current-behavior documentation updates:
+
+- None expected unless internal docs mention the old paired implementation
+  shape in a way that becomes misleading.
+
+Tests:
+
+- `go test ./internal/config ./internal/app`
+- Equivalent local, SSH, and S3 source/destination validation remains
+  consistent.
+- Defaults for SSH port/host key policy, S3 region/prefix/force-path-style, and
+  HTTP upload staging fields remain unchanged.
+- `http_upload` remains valid only for sources.
+
+Completion criteria:
+
+- Adding a future backend field has one obvious defaulting/validation path.
+- Public config files and examples continue to load unchanged.
+
+## Stage 5: Command Output Projection Cleanup
+
+Goal:
+
+Reduce drift in bundle and file metadata projection for app command JSON
+results.
+
+Implementation scope:
+
+- Add small app-local projection helpers for bundle summaries and manifest file
+  records used by `validate`, `inspect`, and `manifest create`.
+- Use `time.RFC3339` consistently instead of equivalent literal layouts.
+- Preserve existing JSON envelope fields, command names, command-specific result
+  field names, text output, and fatal error behavior.
+- Do not redesign CLI JSON output or HTTP JSON responses.
+
+Current-behavior documentation updates:
+
+- None expected unless tests reveal current docs are stale.
+
+Tests:
+
+- `go test ./internal/app ./internal/cli`
+- JSON output for `validate`, `inspect`, and `manifest create` remains
+  structurally stable.
+- RFC3339 timestamps remain unchanged, including offset-preserving source
+  timestamps where current behavior preserves them.
+- Text output remains unchanged.
+
+Completion criteria:
+
+- Bundle/file projection logic is shared where semantics match.
+- Command-specific result structs remain easy to read.
+
+## Stage 6: Archive And Source Path Validation Alignment
+
+Goal:
+
+Protect path safety by aligning archive path tests with source and storage path
+policy, without blurring archive-specific rules.
+
+Implementation scope:
+
+- Add mirrored path-safety table tests around `internal/ingest`, `pkg/bundle`,
+  `internal/bundle`, and `internal/storage` where useful.
+- Keep archive-specific directory handling, root-level manifest rules, duplicate
+  file rejection, symlink rejection, hardlink rejection, and special-entry
+  rejection in `internal/ingest`.
+- Centralize code only if the helper can preserve clear archive semantics and
+  current error behavior.
+- Do not add new public `pkg/bundle` APIs unless the existing public API cannot
+  safely support the needed shared behavior.
+
+Current-behavior documentation updates:
+
+- None expected unless implementation changes error boundaries or internal
+  package descriptions.
+
+Tests:
+
+- `go test ./pkg/bundle ./internal/bundle ./internal/ingest ./internal/storage`
+- Absolute paths, traversal, backslashes, empty paths, dot segments, nested
+  manifests, `.distributor.json` handling, symlinks, hardlinks, devices, and
+  sockets remain covered.
+- Archive directories remain accepted where safe.
+
+Completion criteria:
+
+- Path safety policy has regression coverage across archive staging, source
+  bundle validation, and storage logical path validation.
+- Any code sharing is smaller and clearer than the duplicated logic it replaces.
+
+## Stage 7: Pipeline Run Coordinator Removal
+
+Goal:
+
+Remove the currently unused internal `PipelineRunCoordinator` to avoid
+maintaining two similar coordination concepts.
+
+Implementation scope:
+
+- Delete `PipelineRunCoordinator`, `PipelineRunRecord`,
+  `DuplicatePipelineRunError`, related helpers, and their tests.
+- Remove or rewrite `docs/internal/app.md` sections that describe the removed
+  coordinator.
+- Keep `UploadCoordinator`; do not merge upload queueing with the removed
+  duplicate-run coordinator.
+- Before deletion, confirm with `rg` that production code does not reference
+  `NewPipelineRunCoordinator`, `PipelineRunCoordinator`, or
+  `DuplicatePipelineRunError`.
+
+Current-behavior documentation updates:
+
+- Update `docs/internal/app.md` because it currently documents the coordinator
+  as an internal implemented component.
+
+Tests:
+
+- `go test ./internal/app ./internal/cli`
+- `rg -n "PipelineRunCoordinator|NewPipelineRunCoordinator|DuplicatePipelineRunError" internal docs`
+  should show no stale references after removal.
+
+Completion criteria:
+
+- No production, test, or internal documentation references remain for the
+  removed coordinator.
+- Upload coordination behavior is unchanged.
+
+## Stage 8: Narrow CLI And Test Helper Cleanup
+
+Goal:
+
+Apply only low-risk CLI setup and test fixture cleanup that remains useful after
+the earlier stages.
+
+Implementation scope:
+
+- Add tiny CLI helpers for repeated `flag.FlagSet` setup or output-format
+  parsing only where command behavior remains obvious.
+- Keep the standard-library CLI; do not introduce a CLI framework.
+- Keep `manifest create` interspersed positional parsing local unless another
+  command now needs the same parsing behavior.
+- Expand `internal/testutil` only for repeated setup touched by earlier stages.
+- Do not rewrite tests wholesale just to use shared helpers.
+
+Current-behavior documentation updates:
+
+- None expected unless CLI help or syntax changes. This stage should avoid such
+  changes.
+
+Tests:
+
+- `go test ./internal/cli ./internal/app`
+- CLI usage-error tests remain stable.
+- `manifest create <path> --id x`, `manifest create --id x <path>`, missing
+  flag values, invalid `--format`, and help output remain covered.
+
+Completion criteria:
+
+- Remaining CLI/test cleanup is small, readable, and behavior-preserving.
+- No public CLI syntax or output changes.
+
+## Refactors To Avoid
+
+- Do not introduce a generic workflow engine or stage framework.
+- Do not add a CLI framework.
+- Do not merge local, SSH, S3, and fake backend adapter implementations.
+- Do not collapse `pkg/bundle` and `internal/bundle`.
+- Do not move destination state comparison into `internal/publish` or
+  `internal/app`.
+- Do not redesign CLI JSON envelopes.
+- Do not change HTTP JSON response shapes except where Stage 1 requires
+  pre-acceptance error behavior.
+- Do not add durable upload queues, retry workers, zstd support, in-app TLS,
+  idempotency keys, browser UI, or other feature work.
+- Do not rewrite tests wholesale to use new fixture helpers.
+
+## Validation
+
+After each implementation stage, run the stage-specific tests listed above.
+
+After all cleanup stages:
+
+```sh
+go test ./...
+```
+
+Recommended consistency checks:
+
+```sh
+rg -n "LoadFile\\(|LoadSecretEnvironment\\(|DefaultConfigPath" internal/app internal/cli
+rg -n "application/x-tar|application/gzip|application/x-gzip" internal docs
+rg -n "2006-01-02T15:04:05Z07:00" internal pkg
+rg -n "PipelineRunCoordinator|NewPipelineRunCoordinator|DuplicatePipelineRunError" internal docs
+```
+
+The cleanup is complete when:
+
+- all staged tests and `go test ./...` pass;
+- current-behavior docs describe the implemented Stage 1 upload failure
+  boundary;
+- `docs/roadmap/audit.md` findings have either been addressed or consciously
+  left in place as noted in this cleanup roadmap;
+- no completed cleanup behavior is documented only as future work.