Removed the completed code audit and implementation plan

2026-06-02 14:21:45 -05:00
parent bb276101d2
commit 9aaa1e9426
2 changed files with 0 additions and 920 deletions
--- a/docs/roadmap/audit.md
+++ b/docs/roadmap/audit.md
@@ -1,512 +0,0 @@
 # Code Quality And Deduplication Audit
 ## Executive summary
 Overall code quality is strong for the current project stage. The repository generally follows the documented architecture: CLI parsing stays in `internal/cli`, orchestration stays in `internal/app`, storage behavior is behind `internal/storage`, adapters are thin enough for their protocols, and source manifest semantics are mostly centralized through `pkg/bundle`.
 The top three cleanup targets are:
 1. Backend open-config construction in `internal/app/backends.go`.
 2. HTTP URL validation shared by config and destination state.
 3. Configured-source command scaffolding for `validate` and `inspect`.
 The codebase appears ready for a limited cleanup pass. I did not find a major architectural risk or evidence that a broad rewrite is warranted. The best follow-up work is a sequence of small, behavior-preserving refactors with focused tests.
 This report belongs in `docs/roadmap/audit.md` because it describes future cleanup opportunities rather than implemented behavior.
 ## Repository map reviewed
 Reviewed documentation:
 - `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/app.md`
 - `docs/internal/bundle.md`
 - `docs/internal/config.md`
 - `docs/internal/publish.md`
 - `docs/internal/state.md`
 - `docs/internal/storage.md`
 - `docs/internal/transform.md`
 - `docs/integrations/markdown.md`
 - `docs/roadmap/implementation.md`
 Reviewed implementation areas:
 - `cmd/distributor`
 - `pkg/bundle`
 - `internal/app`
 - `internal/cli`
 - `internal/config`
 - `internal/bundle`
 - `internal/state`
 - `internal/storage`
 - `internal/storage/fake`
 - `internal/adapters/local`
 - `internal/adapters/ssh`
 - `internal/adapters/s3`
 - `internal/publish`
 - `internal/transform`
 - `internal/transform/markdown`
 - `internal/notify`
 - `internal/testutil`
 - `examples`
 Major execution paths reviewed:
 - `run` config load, source discovery, destination selection, publish planning, dry-run, execution, summary, JSON output, and notification handoff.
 - `validate` and `inspect` in local-path and configured-source modes.
 - `manifest create`.
 - Local, SSH/SFTP, S3, and fake storage behavior.
 - Source manifest validation, destination state comparison, Markdown-to-HTML output planning, link generation, and replacement safety.
 No important repository areas were deliberately skipped. There is no `internal/stage`, `internal/modules`, `internal/validators`, `internal/artifacts`, `internal/manifest`, `internal/schema`, or `internal/report` tree in this repository, so those requested audit categories were mapped to the actual bundle, state, config, publish, transform, storage, and CLI packages.
 ## High-confidence deduplication opportunities
 ### 1. Centralize backend open-config construction
 Affected files/packages:
 - `internal/app/backends.go`
 - `internal/config/config.go`
 - `internal/config/defaults.go`
 - `internal/adapters/ssh/options.go`
 - `internal/adapters/s3/options.go`
 - `internal/app/backends_test.go`
 Duplicated or near-duplicated behavior:
 `internal/app/backends.go` has parallel source and destination conversion paths. `sourceOpenConfig` and `destinationOpenConfig` both copy `path`, SSH host/user/port/key/known-hosts/host-key-policy/read-only fields into `storage.OpenConfig` with nearly the same code (`internal/app/backends.go:123`, `internal/app/backends.go:136`, `internal/app/backends.go:166`, `internal/app/backends.go:180`). S3 fields are centralized only partially through `addS3Config` (`internal/app/backends.go:149`).
 Why it matters:
 Source and destination backend shapes are intentionally similar. Any new executable backend field, credential policy, SSH option, S3 option, or dry-run mutation flag must be added to two mapping paths. That creates a realistic drift risk between `run` and configured-source `validate`/`inspect`, because all three paths share source opening but destinations have a separate config struct.
 Recommended refactor:
 Add a small app-local backend spec adapter, for example:
 - `backendOpenSpec` with backend name, path, SSH fields, S3 fields, credentials, and role label.
 - `openConfigForBackend(spec, readOnlyKnownHosts, environment)`.
 - thin constructors such as `sourceBackendSpec(config.Backend)` and `destinationBackendSpec(config.Destination)`.
 Keep this in `internal/app`; do not move concrete adapter construction into `internal/config`. Config should continue to own YAML shape, defaults, and validation only.
 Suggested tests:
 - Update `internal/app/backends_test.go` to assert one shared source/destination conversion table for local, SSH, S3, explicit S3 credentials, and dry-run known-host behavior.
 - Add a regression test that `validate` configured-source mode and `run` source opening produce the same SSH/S3 `storage.OpenConfig` for equivalent source config.
 - Keep adapter-specific tests unchanged.
 Risk level:
 Low to medium. The refactor touches runtime backend opening, but the behavior can be covered with existing backend factory tests and fake backends.
 ### 2. Share HTTP URL validation rules
 Affected files/packages:
 - `internal/config/validate.go`
 - `internal/state/validate.go`
 - `internal/publish/links.go`
 - `internal/config/validate_test.go`
 - `internal/state/distributor_test.go`
 - `internal/publish/links_test.go`
 Duplicated or near-duplicated behavior:
 `validateLinkBaseURL` validates `http`/`https`, host presence, no query string, and no fragment for `links.base_url` (`internal/config/validate.go:201`). `validateStateURL` repeats the same scheme, host, query, and fragment checks for `links.primary_url` and output URLs (`internal/state/validate.go:86`). `publish.OutputURL` parses and mutates a URL independently while assuming config already enforced the base URL shape (`internal/publish/links.go:28`).
 Why it matters:
 These values are related parts of the same public URL contract. If URL policy changes, for example allowing fragments in one place or requiring canonical path handling, config validation and persisted state validation could drift. State validation also returns a raw parse error while config wraps parse failures as `must be a valid URL`, so user-facing and operator-facing diagnostics can diverge.
 Recommended refactor:
 Create a small internal URL policy helper in the narrowest suitable package. Two reasonable homes are:
 - `internal/state`, if state remains the canonical persisted URL contract and config calls a state helper would be acceptable; or
 - a new small package such as `internal/link` if avoiding a config-to-state dependency is preferred.
 The helper should validate absolute HTTP(S) URLs without query or fragment, and optionally expose a base-url-specific wrapper for config error wording. `publish.OutputURL` can continue to build URLs, but should use the same parser/validator for base URLs before mutation.
 Suggested tests:
 - One table-driven URL policy test for valid `http`, valid `https`, missing host, unsupported scheme, query, fragment, and parse failure.
 - Keep config and state tests focused on field context and wiring.
 - Keep `publish/links_test.go` for path semantics and `index.html` directory-style URLs.
 Risk level:
 Low. This is a small policy centralization with existing tests in all affected packages.
 ### 3. Share configured-source CLI scaffolding for `validate` and `inspect`
 Affected files/packages:
 - `internal/cli/validate.go`
 - `internal/cli/inspect.go`
 - `internal/cli/source_mode.go`
 - `internal/app/source_select.go`
 - `internal/cli/root_test.go`
 Duplicated or near-duplicated behavior:
 `validateCommand` and `inspectCommand` define the same `--config`, `--pipeline`, `--bundle`, and `--format` flags; parse output format the same way; parse at most one optional path; call the same `validateInspectModeOK`; and then construct nearly identical app options (`internal/cli/validate.go:17`, `internal/cli/inspect.go:17`). The app layer already centralizes the actual source selection in `selectSourceBundles` and `selectSourceBundlesFromConfig` (`internal/app/source_select.go:28`, `internal/app/source_select.go:60`).
 Why it matters:
 These commands intentionally support the same two modes. Future changes to configured source mode, such as a new source-only diagnostic flag or error wording, would need coordinated edits in both command files and CLI tests.
 Recommended refactor:
 Keep command entrypoints explicit, but introduce a small `parseSourceCommandFlags(command, args, stderr)` helper returning `{path, configPath, pipelineID, bundlePath, outputFormat}`. `validateCommand` and `inspectCommand` would then differ only in help text and the app function they call.
 Do not introduce a CLI framework or a generic command engine.
 Suggested tests:
 - Keep command-specific smoke tests.
 - Add or preserve one table covering local path, too many paths, config without pipeline, pipeline without config, bundle without config, path plus config, invalid format, and JSON format for both commands.
 Risk level:
 Low. This is confined to CLI parsing and has strong existing tests.
 ### 4. Centralize publish output metadata projection
 Affected files/packages:
 - `internal/publish/output.go`
 - `internal/publish/execute.go`
 - `internal/app/run.go`
 - `internal/notify/notify.go`
 - `internal/state/distributor.go`
 Duplicated or near-duplicated behavior:
 The same publish output metadata is projected into several destination structs:
 - `stateOutputs` maps `publish.Output` to `state.OutputFile` (`internal/publish/output.go:100`).
 - `managedOutputPaths` and `existingManagedOutputPaths` extract output paths (`internal/publish/output.go:116`, `internal/publish/execute.go:106`).
 - `runOutputsFromPlan` maps `publish.Output` to JSON output records (`internal/app/run.go:542`).
 - `notifyEvent` maps `publish.Output` to notification output records (`internal/app/run.go:434`).
 Why it matters:
 Output metadata is a central contract: path, kind, source path, transform, URL, digest, and size appear in destination state, JSON output, and notifications. Adding or renaming an output metadata field would require edits in several projection functions. A missed update could make state, JSON, and notification output disagree.
 Recommended refactor:
 Do not force all external output shapes into a single struct, because JSON/state/notification schemas are not identical. Instead, add small package-local helpers with clear names, for example:
 - `publish.Output.StateFile() state.OutputFile`
 - `publish.Output.ManagedPath() string`
 - possibly `publish.Output.PublicMetadata()` if app and notify projections grow.
 Keep app-owned JSON result structs in `internal/app`, but reduce repeated field-by-field copying where the destination schema is identical enough.
 Suggested tests:
 - Add focused publish tests for output-to-state conversion, including URL and generated transform fields.
 - Add an app JSON regression test that generated outputs include `path`, `kind`, `source_path`, `transform`, `url`, `sha256`, and `size`.
 - Add a notification mapping test for generated output metadata.
 Risk level:
 Low. The refactor is mechanical but should be covered because it touches user-visible JSON and destination state.
 ## Medium-confidence opportunities
 ### 1. Extract shared storage walk emission behavior carefully
 Affected files/packages:
 - `internal/adapters/local/backend.go`
 - `internal/adapters/ssh/backend.go`
 - `internal/adapters/s3/backend.go`
 - `internal/storage/backend.go`
 - `internal/storage/fake/backend.go`
 Duplicated or near-duplicated behavior:
 Local, SSH, and S3 backends each implement an inner `emit` function that checks context, applies `WalkOptions.Limit`, increments a visit counter, handles `storage.ErrStopWalk`, and wraps callback errors as storage walk errors (`internal/adapters/local/backend.go:164`, `internal/adapters/ssh/backend.go:212`, `internal/adapters/s3/backend.go:186`). Each backend also implements `HasAny` as `Walk(... Limit: 1)` (`internal/adapters/local/backend.go:221`, `internal/adapters/ssh/backend.go:247`, `internal/adapters/s3/backend.go:225`).
 Why it matters:
 Walk semantics are part of the storage contract. A change to limit behavior or `ErrStopWalk` handling would need repeated edits across adapters. Fake backend semantics already differ slightly because it handles `ErrStopWalk` in its own loop.
 Recommended refactor:
 Consider a tiny storage helper that wraps emission state, such as `storage.NewWalkEmitter(ctx, backendName, opts, fn)`, plus a default `storage.HasAny(ctx, backend, prefix)` helper. Keep traversal mechanics in adapters. Avoid extracting directory walking, SFTP behavior, S3 pagination, or deletion logic.
 Suggested tests:
 - Shared storage contract tests for `WalkOptions.Limit`, callback error wrapping, stopping with `ErrStopWalk`, and `HasAny` for empty prefix, missing prefix, file prefix, and directory/prefix content.
 - Run adapter tests after any change.
 Risk level:
 Medium. The duplication is real, but adapters have protocol-specific walk behavior. Keep the helper narrow.
 ### 2. Unify digest format validation without expanding public API accidentally
 Affected files/packages:
 - `pkg/bundle/digest.go`
 - `internal/bundle/digest.go`
 - `internal/state/validate.go`
 Duplicated or near-duplicated behavior:
 `pkg/bundle` has a private `validateDigest` using a `sha256:<64 lowercase hex>` regex (`pkg/bundle/digest.go:12`). `internal/bundle` repeats the regex to expose `ValidateDigest` for state validation (`internal/bundle/digest.go:10`, `internal/state/validate.go:77`).
 Why it matters:
 Digest format is a source manifest and destination state invariant. If the digest grammar ever changes, the public package and internal state validation must be updated together.
 Recommended refactor:
 Either expose `pkg/bundle.ValidateDigest` as part of the producer-facing API, or keep the internal duplicate if the project does not want that API commitment. If exposed, document it in `pkg/bundle` tests and use it from `internal/bundle`.
 Suggested tests:
 - Move existing digest format tests to cover the public function if exposed.
 - Add state validation coverage that proves state uses the same digest validator.
 Risk level:
 Medium. The code change is small, but adding a public API has compatibility implications.
 ### 3. Reduce local test fixture builders after behavior cleanup
 Affected files/packages:
 - `internal/app/run_test.go`
 - `internal/testutil/fixtures.go`
 - `internal/cli/root_test.go`
 - `internal/publish/force_test.go`
 Duplicated or near-duplicated behavior:
 `internal/testutil` already provides source bundle, fake source bundle, minimal config, fan-out config, destination state, and state-reading helpers. `internal/app/run_test.go` still maintains a local `testBundleOptions`, local source-bundle adapter, several YAML config builders, file assertions, fake-backend assertions, and state helpers (`internal/app/run_test.go:1237`, `internal/app/run_test.go:1275`, `internal/app/run_test.go:1318`, `internal/app/run_test.go:1349`, `internal/app/run_test.go:1410`, `internal/app/run_test.go:1437`). CLI and publish tests also have small local file/fake helpers.
 Why it matters:
 The large app test file is valuable coverage, but local fixture helpers make it easier for config examples and test setup patterns to drift from shared fixtures. This is mostly maintenance friction rather than production risk.
 Recommended refactor:
 After production cleanup is complete, move only generally reusable helpers into `internal/testutil`: config builder variants for publish policy, path mapping, links, and Markdown transform; file assertion helpers; fake backend assertion helpers. Keep highly specific scenario builders local.
 Suggested tests:
 - This is a test-only cleanup. Run `go test ./internal/app ./internal/cli ./internal/publish`.
 - Avoid rewriting test cases and helpers in the same commit if failures would become hard to diagnose.
 Risk level:
 Low to medium. Test-only, but high-volume edits can obscure behavior changes if combined with production refactors.
 ### 4. Split run output formatting from run orchestration
 Affected files/packages:
 - `internal/app/run.go`
 - `docs/internal/app.md`
 - `internal/app/run_test.go`
 - `internal/cli/root_test.go`
 Duplicated or near-duplicated behavior:
 `internal/app/run.go` contains orchestration, destination selection, fixed-path warnings, text plan lines, JSON result types, summary counters, failure aggregation, notification mapping, secret warnings, and SSH warnings in one file. There is some duplication between text and JSON summary/status projection (`internal/app/run.go:591`, `internal/app/run.go:611`) and between plan error line formatting and JSON error action creation (`internal/app/run.go:264`, `internal/app/run.go:494`, `internal/app/run.go:529`).
 Why it matters:
 This is the largest production file and it carries multiple responsibilities. The current boundaries are not wrong, but future run output changes may require touching orchestration code, making regressions more likely.
 Recommended refactor:
 Split `internal/app/run.go` into package-local files by responsibility, for example `run_output.go`, `run_summary.go`, `run_warnings.go`, and `run_selection.go`. Keep the package and public behavior unchanged. This is file organization and small helper extraction, not a new reporting subsystem.
 Suggested tests:
 - Preserve existing app and CLI output tests.
 - Add focused tests for summary text/JSON consistency if the summary helpers are extracted.
 Risk level:
 Medium. Mostly mechanical, but output text and JSON are public behavior.
 ## Boundary and responsibility concerns
 The primary boundary concern is `internal/app/backends.go`. It is acceptable for app-level wiring to import concrete adapters, but app also owns string-keyed conversion from config structs to adapter option maps. This is currently the right package, but the duplicated source/destination mapping should be narrowed through a shared app-local conversion helper.
 `internal/app/run.go` also mixes orchestration with output presentation and notification projection. This is not a boundary violation because app owns top-level command output, but it is a maintenance concern. Keep formatting in `internal/app`; do not move command output into `internal/publish`, because publish should remain focused on destination planning and execution.
 `internal/config` correctly owns config defaults and validation. It should not open backends or resolve storage paths. `ValidatePublishTransformPolicy` being shared with publish planning is appropriate because it prevents config and runtime policy drift.
 `internal/publish` correctly depends on `internal/storage`, `internal/state`, and transform interfaces rather than concrete adapters. Link generation in publish is destination publication behavior and fits the package, but URL validation should share policy with config/state.
 The storage adapters contain some similar code, but most of it is protocol-specific implementation. Do not move filesystem, SFTP, or S3 semantics into core packages.
 ## Path, key, and naming construction review
 Path and naming construction is mostly centralized enough:
 - Source manifest filename is centralized through `pkg/bundle.ManifestName` and re-exported by `internal/bundle`.
 - Destination state filename and path helpers are centralized through `storage.StateFileName`, `storage.StatePath`, and `storage.ManagedBundleTargets` (`internal/storage/path.go:9`, `internal/storage/path.go:38`, `internal/storage/path.go:52`).
 - Runtime storage paths use `storage.Join`, `storage.ValidatePath`, `storage.ValidatePrefix`, and `storage.DisplayPath`.
 - S3 prefix normalization is centralized in `internal/config/s3.go`.
 - Public URL path construction is in `internal/publish/links.go`.
 Exact cleanup areas:
 - `pkg/bundle` has a private `.distributor.json` reserved-name constant while `internal/storage` has `StateFileName` (`pkg/bundle/path.go:9`, `internal/storage/path.go:9`). This is probably intentional because `pkg/bundle` cannot import `internal/storage`, but the reserved name should be documented near the public bundle contract if it is not already clear.
 - `storage.ValidatePath` and `pkg/bundle.ValidateSourcePath` intentionally share similar clean relative slash path rules, but source paths also reject reserved filenames (`pkg/bundle/path.go:11`, `internal/storage/path.go:11`). Do not merge them unless the semantic difference remains explicit.
 - S3 object-key construction should remain in the S3 adapter. Existing `objectKey`, `listPrefix`, and `logicalPathFromKey` helpers are appropriately local to S3 behavior.
 - URL construction is centralized in publish, but URL validation is duplicated and should be cleaned up as described above.
 ## Resolution and catalog review
 There are no stage, module, schema, prompt, profile, artifact catalog, or validator catalogs in this repository.
 Implemented resolution paths are consistent:
 - Backend names are config constants and runtime adapter construction is registered in `internal/app`.
 - Transform names are centralized in `internal/transform`, and app registers Markdown-to-HTML once through `newTransformRegistry`.
 - Configured-source `validate` and `inspect` share `selectSourceBundles` and `selectSourceBundlesFromConfig`, so pipeline lookup, secret loading, source backend opening, bundle narrowing, and warnings are consistent.
 - Destination bundle resolution for `preserve_relative` and `fixed` mappings is centralized in app run selection.
 - Link primary selection is centralized in publish.
 Recommended centralization:
 - Keep transform registration in app. Do not create a plugin or catalog layer.
 - Consider extracting destination selection helpers from `run.go` into a package-local file, but not a new package.
 - Keep configured-source selection shared between validate and inspect; focus cleanup on duplicated CLI flag parsing.
 ## Config and command-loading review
 Config loading is consistent:
 - `run` uses the supplied config path or `config.DefaultConfigPath`.
 - Configured-source `validate` and `inspect` require an explicit `--config`, matching the documented CLI.
 - `config.LoadFile` rejects unknown YAML fields, applies defaults, then validates.
 - `run`, configured-source `validate`, and configured-source `inspect` load `secrets.directory` before opening backends.
 - S3 explicit credentials are resolved through the config environment resolver, not direct `os.Getenv`.
 Intentional differences:
 - Local-path `validate` and `inspect` do not load config.
 - `run` opens destinations; configured-source diagnostics open only the selected source.
 - `run --dry-run` marks SSH known-host handling read-only; configured-source diagnostics do not have a dry-run flag.
 Likely accidental duplication:
 - `validate` and `inspect` CLI flag setup is repeated even though the mode semantics are identical.
 - Backend source/destination open-config conversion is repeated and should be centralized within app-level wiring.
 ## State, manifest, or progress handling review
 Destination state handling is coherent:
 - `.distributor.json` is the sentinel and state record.
 - Publish planning inspects destination state through `storage.StatePath`, parses with `state.Parse`, and compares with `state.Compare`.
 - Replacement safety uses managed outputs from existing state for normal replacement and bounded prefix deletion for forced replacement.
 - Dry-run builds plans and output records without writing outputs or state.
 Manifest handling is coherent:
 - `pkg/bundle` owns public source manifest parsing, digesting, local writing, and local validation.
 - `internal/bundle` reuses the public manifest model for storage-backed discovery and validation.
 - State validation uses the same embedded manifest validation path.
 Potential cleanup:
 - Digest format validation is duplicated between `pkg/bundle` and `internal/bundle`. Decide whether to expose a public digest validator or keep the duplicate as a conscious public API boundary.
 - Output metadata projection into state, JSON output, and notifications should be made less repetitive before new output metadata is added.
 Progress handling:
 - There is no separate run checkpoint or progress store. This is consistent with current docs and architecture.
 ## Refactors to avoid
 Avoid these refactors for the next cleanup pass:
 - A generic workflow engine for `run`, `validate`, `inspect`, and `manifest create`.
 - A CLI framework or broad command abstraction.
 - A broad plugin architecture for backends or transforms.
 - Moving backend construction into `internal/config`.
 - Moving publish planning policy into adapters.
 - Consolidating local, SSH, and S3 deletion or traversal implementations beyond tiny shared walk helpers.
 - Merging `pkg/bundle.ValidateSourcePath` and `storage.ValidatePath` without preserving their different semantics and error contracts.
 - Rewriting destination state schema or source manifest schema as part of deduplication.
 - Replacing explicit app output structs with a generic reporting layer.
 - Large test rewrites in the same commit as production refactors.
 ## Recommended implementation sequence
 1. Centralize backend open-config construction in `internal/app/backends.go`.
 2. Add a shared internal URL policy helper and use it from config, state, and publish link planning.
 3. Extract configured-source CLI flag parsing for `validate` and `inspect`.
 4. Add small publish output projection helpers for state outputs, managed output paths, and notification/JSON metadata where appropriate.
 5. Split `internal/app/run.go` into package-local files for run selection, warnings, summary, and output formatting.
 6. Add a narrow storage walk-emitter or `HasAny` helper only if adapter tests can prove identical semantics.
 7. Decide whether `pkg/bundle` should expose digest validation; if yes, wire `internal/bundle.ValidateDigest` through it.
 8. Move broadly reusable test fixture helpers from `internal/app/run_test.go` into `internal/testutil`.
 9. Run a dead-code and legacy-helper sweep after the above changes, keeping public behavior unchanged.
 Each item should be a separate commit or prompt unless the implementation is very small.
 ## Test strategy
 Tests to run for the full cleanup sequence:
 - `go test ./internal/app ./internal/cli ./internal/config`
 - `go test ./internal/publish ./internal/state`
 - `go test ./internal/storage ./internal/storage/fake ./internal/adapters/local ./internal/adapters/ssh ./internal/adapters/s3`
 - `go test ./internal/bundle ./pkg/bundle`
 - `go test ./...` after cross-package or documentation changes
 Tests to add before or during refactors:
 - Backend open-config equivalence tests for source and destination SSH/S3 mapping.
 - Configured-source `validate`/`inspect` shared CLI parsing table.
 - Shared URL policy table, plus config/state field-context tests.
 - Publish output projection tests for generated outputs with URLs.
 - Run summary text/JSON consistency tests if summary logic is extracted.
 - Storage walk contract tests if a shared walk helper is introduced.
 Validation performed during this audit:
 - `go list ./...` was run with workspace-safe Go cache paths. It required network access to fetch missing modules and completed successfully after approval.
 - No full test suite was run because this was a report-only task and no code behavior was changed.
 ## Appendix: findings not worth acting on
 ### Adapter-specific path/key conversion
 Local `nativePath`, SSH `nativePath`, and S3 `objectKey` all convert logical storage paths into backend-native addresses. They look similar in purpose, but their semantics differ: local must guard filesystem roots and symlink ancestors, SSH must use remote slash paths and SFTP behavior, and S3 must handle bucket/prefix object keys. Keep them local to adapters.
 ### `storage.ValidatePath` and `pkg/bundle.ValidateSourcePath`
 Both enforce clean relative slash paths, but source bundle paths also reject reserved source/destination state filenames and use public producer-facing error wording. Keep them separate unless a future helper can preserve the different contracts.
 ### Explicit command files
 `run`, `validate`, `inspect`, `version`, and `manifest` each having a command file is clear and appropriate. Only the shared configured-source flag parsing for `validate` and `inspect` should be extracted.
 ### Text output and JSON output are intentionally separate shapes
 Text output is human-oriented and JSON output is a stable machine-readable envelope. Do not collapse them into one generic renderer. Small helpers for shared summary data are enough.
 ### Test-local helpers for very specific scenarios
 Some local test helpers make individual scenarios easier to read. Move only broadly reusable builders and assertions into `internal/testutil`; leave one-off scenario setup local.
--- a/docs/roadmap/cleanup.md
+++ b/docs/roadmap/cleanup.md
@@ -1,408 +0,0 @@
 # Cleanup Implementation Roadmap
 This roadmap turns the audit findings in `docs/roadmap/audit.md` into staged
 implementation work for LLM coding agents.
 Each stage is intended to be implemented in order. Treat each stage as a
 separate prompt or commit unless the change is trivially small. Before starting
 any stage, read:
 - `docs/policy/architecture.md`
 - `docs/policy/development.md`
 - `docs/policy/documentation.md`
 ## Global Rules
 - Preserve public CLI behavior, config semantics, source manifest schema,
  destination state schema, implemented backend behavior, and documented output
  contracts unless a stage explicitly says otherwise.
 - Do not introduce external dependencies.
 - Do not create a generic workflow engine, CLI framework, plugin system, broad
  adapter abstraction, or generic reporting layer.
 - Keep concrete backend construction in `internal/app`.
 - Keep backend-specific filesystem, SSH/SFTP, and S3 details inside adapters.
 - Keep user-facing documentation unchanged unless public behavior changes.
  These stages are intended to be behavior-preserving.
 - After each stage, run the tests listed for that stage. If Go cache
  permissions fail in a restricted environment, use:
 ```sh
 GOCACHE=/private/tmp/distributor-gocache GOMODCACHE=/private/tmp/distributor-gomodcache go test <packages>
 ```
 ## Stage 1: Backend Open-Config Unification
 Goal: make source and destination backend opening use one app-local conversion
 path while preserving current runtime behavior.
 Implement:
 - In `internal/app`, introduce a package-local backend-open spec that can
  represent both `config.Backend` sources and `config.Destination`
  destinations.
 - The spec must include backend name, path, SSH fields, S3 fields, credentials,
  and a role label for error wording where needed.
 - Add two thin constructors:
  - one from `config.Backend`;
  - one from `config.Destination`.
 - Replace separate source/destination SSH and S3 `storage.OpenConfig`
  construction with one helper that:
  - sets the local path key;
  - copies SSH host, user, port, key file, known-hosts path, host-key policy,
    and read-only known-hosts flag;
  - copies S3 endpoint, bucket, prefix, region, force-path-style setting, and
    resolved explicit credentials;
  - resolves explicit S3 credentials only through `config.Environment`.
 - Keep adapter registration and concrete adapter imports in `internal/app`.
 - Do not move backend opening, credential resolution, or runtime support checks
  into `internal/config`.
 Tests:
 - Update `internal/app/backends_test.go` to assert equivalent source and
  destination conversion for local, SSH, S3, S3 explicit credentials, and
  dry-run known-host behavior.
 - Add or preserve a regression test proving configured-source validation and
  run source opening produce equivalent SSH/S3 open config for the same source
  settings.
 - Run:
 ```sh
 go test ./internal/app
 ```
 Acceptance criteria:
 - `run`, configured-source `validate`, and configured-source `inspect` still
  open local, SSH, and S3 sources as before.
 - `run --dry-run` still makes SSH known-host writes read-only.
 - S3 explicit credentials still prefer resolved environment/secrets-directory
  values and never call `os.Getenv` directly from backend wiring.
 ## Stage 2: Shared Link URL Policy
 Goal: centralize HTTP URL validation for configured links and persisted link
 state.
 Implement:
 - Add a small `internal/link` package.
 - In that package, define the canonical link URL rule:
  - URL must parse successfully;
  - scheme must be `http` or `https`;
  - host must be present;
  - query string must be absent;
  - fragment must be absent.
 - Use the helper from:
  - config validation for `links.base_url`;
  - state validation for `links.primary_url`;
  - state validation for output `url`;
  - publish link planning before building output URLs.
 - Preserve existing field-context error messages from config and state callers
  where tests assert them. The helper may return concise common errors, while
  callers add context such as `links.base_url` or `state outputs[i].url`.
 - Keep URL path construction and primary URL selection in `internal/publish`.
 Tests:
 - Add table-driven tests for `internal/link` covering valid `http`, valid
  `https`, missing host, unsupported scheme, query string, fragment, and parse
  failure.
 - Keep config tests focused on config field context and accepted primary-link
  policies.
 - Keep state tests focused on persisted state validation context.
 - Keep publish tests focused on URL path semantics and `index.html`
  directory-style URLs.
 - Run:
 ```sh
 go test ./internal/config ./internal/state ./internal/publish
 ```
 Acceptance criteria:
 - Config, state, and publish all enforce the same link URL policy.
 - Existing public URL generation behavior is unchanged.
 - No config package dependency on `internal/state` is introduced.
 ## Stage 3: Configured-Source CLI Parsing
 Goal: remove duplicate `validate` and `inspect` configured-source flag parsing
 without obscuring command behavior.
 Implement:
 - In `internal/cli`, add a small helper that parses the shared source-diagnostic
  command flags for a command name:
  - `--config`;
  - `--pipeline`;
  - `--bundle`;
  - `--format`;
  - at most one optional local path.
 - The helper should return a simple parsed value containing path, config path,
  pipeline id, bundle path, and normalized output format.
 - Keep `validateCommand` and `inspectCommand` as explicit entrypoints.
 - Keep command help text separate and command-specific.
 - Keep `validateInspectModeOK` behavior or fold it into the shared helper with
  the same error semantics.
 - Do not introduce a CLI framework, generic command engine, or new root command
  dispatch model.
 Tests:
 - Add or preserve CLI tests for both commands covering:
  - local path mode;
  - configured source mode;
  - configured source JSON output;
  - too many local paths;
  - local path combined with config flags;
  - `--pipeline` without `--config`;
  - `--bundle` without `--config`;
  - `--config` without `--pipeline`;
  - invalid `--format`.
 - Run:
 ```sh
 go test ./internal/cli ./internal/app
 ```
 Acceptance criteria:
 - `validate` and `inspect` still support the exact documented modes.
 - Help output remains command-specific.
 - User-facing error wording remains compatible with current tests.
 ## Stage 4: Publish Output Projection Helpers
 Goal: reduce drift when publish output metadata is projected into state, run
 JSON output, managed cleanup paths, and notifications.
 Implement:
 - Add package-local helper methods or functions in `internal/publish` for:
  - converting a `publish.Output` to `state.OutputFile`;
  - extracting a managed destination output path;
  - converting output slices to state output slices;
  - converting output slices to managed path slices.
 - Replace existing duplicate state-output and managed-path projections with the
  new helpers.
 - Keep app-owned JSON result structs in `internal/app`.
 - Keep notify-owned event structs in `internal/notify`.
 - In `internal/app`, use publish helpers only where they reduce duplicate field
  mapping without changing JSON or notification schemas.
 - Do not collapse state, JSON, and notification schemas into one shared public
  type.
 Tests:
 - Add publish tests for output-to-state conversion, including source output,
  generated output, transform name, URL, digest, and size.
 - Add or preserve app JSON regression coverage proving generated output records
  include path, kind, source path, transform, URL, digest, and size.
 - Add or preserve notification mapping coverage for generated output metadata.
 - Run:
 ```sh
 go test ./internal/publish ./internal/app
 ```
 Acceptance criteria:
 - Destination state output metadata is unchanged.
 - Run JSON output metadata is unchanged.
 - Notification event metadata is unchanged.
 - Managed cleanup still deletes only recorded output paths plus state through
  existing storage safety helpers.
 ## Stage 5: Run Orchestration File Split
 Goal: make `internal/app/run.go` easier to maintain by moving package-local
 helpers into focused files without changing behavior.
 Implement:
 - Split current run-related helpers into package-local files such as:
  - `run_selection.go` for destination bundle selection and fixed-path helpers;
  - `run_warnings.go` for secret and SSH warning helpers;
  - `run_output.go` for text and JSON action projection helpers;
  - `run_summary.go` for summary counters and summary result projection;
  - `run_failures.go` for run failure aggregation and partial-result detection.
 - Keep `Run`, `runConfig`, and `runConfigWithBackendFactory` as the primary
  orchestration functions.
 - Do not change exported APIs, text output, JSON output, warning wording,
  summary counters, failure aggregation, fixed-path selection, notification
  behavior, or dry-run behavior.
 - Do not create a separate reporting package.
 Tests:
 - Add focused tests for summary text and JSON projection if extracting summary
  code exposes a natural package-local test seam.
 - Preserve existing app and CLI output tests.
 - Run:
 ```sh
 go test ./internal/app ./internal/cli
 ```
 Acceptance criteria:
 - `run.go` is smaller and orchestration-focused.
 - All run output remains byte-for-byte compatible where tests currently assert
  exact output.
 - JSON partial-failure behavior remains unchanged.
 ## Stage 6: Storage Walk Helper
 Goal: centralize the narrow shared parts of storage walk callback handling
 without moving backend traversal semantics out of adapters.
 Implement:
 - Add an `internal/storage` helper for walk emission that owns only:
  - context checks before callback emission;
  - `WalkOptions.Limit` counting;
  - `ErrStopWalk` handling;
  - callback error wrapping as storage walk errors.
 - Use this helper in local, SSH, and S3 adapters only where the semantics match.
 - Add `storage.HasAny(ctx, backend, prefix)` as a shared helper implemented via
  `Walk` with non-recursive limit-one traversal.
 - Update adapter `HasAny` implementations to call the shared helper if doing so
  preserves current behavior.
 - Do not centralize:
  - local filesystem walking;
  - SSH/SFTP directory walking;
  - S3 pagination or object listing;
  - deletion behavior;
  - native path or object key construction;
  - symlink handling.
 Tests:
 - Add storage-level tests for the walk emitter or helper behavior:
  - limit stops after the expected number of emissions;
  - `ErrStopWalk` stops without error;
  - callback errors are wrapped as storage walk errors;
  - context cancellation is honored.
 - Add or preserve `HasAny` behavior tests for empty backends, missing prefixes,
  file prefixes, directory prefixes, and object-prefix cases where applicable.
 - Run:
 ```sh
 go test ./internal/storage ./internal/storage/fake ./internal/adapters/local ./internal/adapters/ssh ./internal/adapters/s3
 ```
 Acceptance criteria:
 - Adapter traversal order and recursive/non-recursive behavior are unchanged.
 - Backend-specific path, key, symlink, deletion, and pagination logic remains
  adapter-local.
 - `HasAny` behavior is consistent with existing tests.
 ## Stage 7: Digest Validation Centralization
 Goal: make `pkg/bundle` the canonical home for digest format validation.
 Implement:
 - Expose `pkg/bundle.ValidateDigest(value string) error`.
 - Use the same lowercase `sha256:<64 hex>` rule currently used by manifest
  validation.
 - Update `pkg/bundle.ValidateManifest` to call the public validator.
 - Update `internal/bundle.ValidateDigest` to delegate to
  `pkg/bundle.ValidateDigest`.
 - Keep digest formatting and bundle digest generation unchanged.
 - Treat this as a public producer-facing API addition. Do not remove or rename
  existing public APIs.
 Tests:
 - Add public package tests for `pkg/bundle.ValidateDigest` covering valid
  digest, uppercase hex rejection, missing prefix, wrong algorithm, short hex,
  long hex, and non-hex characters.
 - Add or preserve state validation coverage proving destination state output
  SHA-256 validation uses the same rule.
 - Run:
 ```sh
 go test ./pkg/bundle ./internal/bundle ./internal/state
 ```
 Acceptance criteria:
 - There is one canonical digest validation rule.
 - Producer-facing code can validate digest strings directly.
 - Existing manifest and state validation behavior is unchanged.
 ## Stage 8: Test Fixture Cleanup
 Goal: move broadly reusable test helpers into `internal/testutil` after
 production refactors are complete.
 Implement:
 - Move only reusable helpers into `internal/testutil`, such as:
  - local config builders for publish policy, path mapping, links, and Markdown
    transform combinations;
  - file assertion helpers;
  - fake backend file assertion helpers;
  - state read/write helpers already duplicated in package tests.
 - Keep scenario-specific setup local to the package test that uses it.
 - Do not import `internal/testutil` from production code.
 - Do not combine this stage with production refactors.
 - Prefer small helper names that describe behavior over generic fixture-builder
  abstractions.
 Tests:
 - Run:
 ```sh
 go test ./internal/app ./internal/cli ./internal/publish
 ```
 Acceptance criteria:
 - Test behavior and coverage are unchanged.
 - Test helpers reduce repeated setup without hiding scenario-specific behavior.
 - No production package imports `internal/testutil`.
 ## Stage 9: Dead-Code And Legacy Sweep
 Goal: remove obsolete helpers left behind by the previous stages.
 Implement:
 - Search for unused or now-duplicative helpers after stages 1 through 8.
 - Remove only clearly obsolete code.
 - Keep this limited to dead code, duplicate helpers, and stale comments created
  by earlier cleanup.
 - Do not perform unrelated refactors.
 - Do not change user-facing docs unless a previous stage intentionally changed
  public behavior.
 Tests:
 - Run:
 ```sh
 go test ./...
 ```
 Acceptance criteria:
 - Full test suite passes.
 - No stale helper names remain from the old implementation paths.
 - No public behavior, schemas, or documented workflows change.
 ## Final Cleanup Acceptance
 After all stages are complete:
 - Run `go test ./...`.
 - Re-read `docs/roadmap/audit.md` and confirm every high-confidence and
  medium-confidence opportunity has been addressed or intentionally deferred in
  a short note.
 - Confirm no refactor-to-avoid item from the audit was introduced.
 - Confirm `git status --short` contains only intentional changes for the final
  stage.