Files
distributor/docs/roadmap/cleanup.md

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.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:
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:
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.

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:
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:
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:
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:
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:
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:
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:
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.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.