14 KiB
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.mddocs/policy/development.mddocs/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:
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 bothconfig.Backendsources andconfig.Destinationdestinations. - 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.
- one from
- Replace separate source/destination SSH and S3
storage.OpenConfigconstruction 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.goto 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:
go test ./internal/app
Acceptance criteria:
run, configured-sourcevalidate, and configured-sourceinspectstill open local, SSH, and S3 sources as before.run --dry-runstill makes SSH known-host writes read-only.- S3 explicit credentials still prefer resolved environment/secrets-directory
values and never call
os.Getenvdirectly 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/linkpackage. - In that package, define the canonical link URL rule:
- URL must parse successfully;
- scheme must be
httporhttps; - 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.
- config validation for
- 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_urlorstate outputs[i].url. - Keep URL path construction and primary URL selection in
internal/publish.
Tests:
- Add table-driven tests for
internal/linkcovering validhttp, validhttps, 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.htmldirectory-style URLs. - Run:
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/stateis 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
validateCommandandinspectCommandas explicit entrypoints. - Keep command help text separate and command-specific.
- Keep
validateInspectModeOKbehavior 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;
--pipelinewithout--config;--bundlewithout--config;--configwithout--pipeline;- invalid
--format.
- Run:
go test ./internal/cli ./internal/app
Acceptance criteria:
validateandinspectstill 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/publishfor:- converting a
publish.Outputtostate.OutputFile; - extracting a managed destination output path;
- converting output slices to state output slices;
- converting output slices to managed path slices.
- converting a
- 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:
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.gofor destination bundle selection and fixed-path helpers;run_warnings.gofor secret and SSH warning helpers;run_output.gofor text and JSON action projection helpers;run_summary.gofor summary counters and summary result projection;run_failures.gofor run failure aggregation and partial-result detection.
- Keep
Run,runConfig, andrunConfigWithBackendFactoryas 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:
go test ./internal/app ./internal/cli
Acceptance criteria:
run.gois 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/storagehelper for walk emission that owns only:- context checks before callback emission;
WalkOptions.Limitcounting;ErrStopWalkhandling;- 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 viaWalkwith non-recursive limit-one traversal. - Update adapter
HasAnyimplementations 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;
ErrStopWalkstops without error;- callback errors are wrapped as storage walk errors;
- context cancellation is honored.
- Add or preserve
HasAnybehavior tests for empty backends, missing prefixes, file prefixes, directory prefixes, and object-prefix cases where applicable. - Run:
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.
HasAnybehavior 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.ValidateManifestto call the public validator. - Update
internal/bundle.ValidateDigestto delegate topkg/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.ValidateDigestcovering 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:
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/testutilfrom production code. - Do not combine this stage with production refactors.
- Prefer small helper names that describe behavior over generic fixture-builder abstractions.
Tests:
- Run:
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:
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.mdand 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 --shortcontains only intentional changes for the final stage.