distributor/docs/roadmap/cleanup.md

Cleanup Implementation Roadmap

This roadmap converts the findings in docs/roadmap/audit.md into staged cleanup work. It is written for LLM coding agents that will implement one stage at a time.

The cleanup goal is to reduce drift before remote backend work without changing public CLI behavior, config semantics, manifest/state schemas, or local MVP behavior.

Global Cleanup Rules

Every implementation stage must:

1. read AGENTS.md, docs/policy/architecture.md, docs/policy/documentation.md, and this roadmap before editing;
2. implement only the current stage;
3. preserve current public CLI behavior and config behavior unless the stage explicitly says otherwise;
4. keep cleanup behavior-preserving and avoid broad rewrites;
5. add or update focused tests for the changed behavior;
6. run relevant package tests, and run go test ./... when the stage touches cross-package behavior;
7. update implemented internal docs only when an internal contract actually changes;
8. leave user-facing docs unchanged unless public behavior changes;
9. avoid implementing future remote backend features as part of cleanup.

If Go cache or module cache permissions fail, use workspace-safe temporary caches:

GOCACHE=/private/tmp/distributor-gocache GOMODCACHE=/private/tmp/distributor-gomodcache go test ./...

Stage 1: Centralize Storage State Names and Managed Delete Targets

Goal

Make state-file naming and managed deletion target construction canonical before SSH/SFTP and S3 adapters are added.

Implementation

In internal/storage:

• Export the destination state filename as:

const StateFileName = ".distributor.json"

• Update StatePath to use StateFileName.
• Add:

func ManagedBundleTargets(bundlePath string, managedOutputPaths []string) ([]string, error)

Required behavior:

• bundlePath is validated as a prefix, so "" is valid.
• each managed output path is validated as a file path below bundlePath;
• returned targets contain each managed output target followed by the .distributor.json target;
• root bundle path returns output paths unchanged plus .distributor.json;
• nested bundle path returns bundlePath/output plus bundlePath/.distributor.json;
• invalid output paths fail before any backend deletion occurs;
• duplicate output paths do not need to be de-duplicated in this stage.

Update local and fake backends to call storage.ManagedBundleTargets inside DeleteManagedBundle. Keep actual deletion, missing-file handling, directory pruning, and backend-specific error translation in the backend implementations.

Replace code/test references to literal .distributor.json with storage.StateFileName where the code already imports or reasonably can import internal/storage. Do not contort docs or unrelated tests only to remove literals from prose.

Tests

Add or update tests for:

• StatePath("") == ".distributor.json";
• StatePath("bundle") == "bundle/.distributor.json";
• managed targets for root and nested bundle paths;
• invalid managed output path rejection;
• local and fake managed deletion still delete only listed outputs plus state;
• local and fake managed deletion still preserve unlisted files.

Run:

go test ./internal/storage ./internal/storage/fake ./internal/adapters/local ./internal/bundle ./internal/publish

Completion Criteria

• Managed target derivation lives in one storage helper.
• Local and fake backend behavior is unchanged.
• No broad recursive delete behavior is introduced.

Stage 2: Centralize Normalized Source Manifest Validation

Goal

Make internal/bundle the single owner of source manifest semantics, including embedded source manifests in destination state.

Implementation

In internal/bundle, add a model-level validation helper:

func ValidateManifest(manifest Manifest) error

Required behavior:

• validate SchemaVersion == 1;
• validate non-empty ID;
• validate top-level Digest format;
• validate non-zero Created;
• require at least one file;
• validate every file path with ValidateSourcePath;
• validate every file digest format;
• reject negative file sizes;
• reject duplicate logical file paths;
• recompute BundleDigest(manifest.Files) and require it to match manifest.Digest.

Keep ParseManifest responsible for raw JSON parsing, missing-field detection, RFC3339 timestamp parsing, and trailing-data detection. After building the normalized Manifest, call ValidateManifest for model-level validation. Preserve current error substrings where practical, especially for existing tests that assert user-facing diagnostics.

In internal/state, replace validateEmbeddedManifest logic with delegation to bundle.ValidateManifest, wrapping the error as state source.manifest: ... where current callers expect state context.

Do not change the manifest JSON schema, destination state schema, digest algorithm, timestamp normalization policy, or source path policy.

Tests

Add or update tests for:

• bundle.ValidateManifest accepts the existing valid fixture manifest;
• bundle.ValidateManifest rejects bad schema version, empty id, bad digest, zero created time, empty files, unsafe paths, duplicate paths, negative size, and bundle digest mismatch;
• state.Validate rejects the same embedded manifest violations through the shared helper;
• existing manifest parser and destination state parser tests continue to pass.

Run:

go test ./internal/bundle ./internal/state ./internal/publish ./internal/app

Completion Criteria

• Source manifest contract semantics are implemented once in internal/bundle.
• Destination state validation delegates embedded source manifest semantics to internal/bundle.
• Existing local MVP behavior is unchanged.

Stage 3: Centralize Publish and Transform Policy Validation

Goal

Prevent drift between config validation and publish request validation for allowed source/html/transform combinations.

Implementation

Keep ownership in internal/config, because the policy is expressed in config types and used by config validation.

Add a helper such as:

func ValidatePublishTransformPolicy(publish PublishPolicy, transform Transform) error

Required behavior:

• fail when both publish.Source and publish.HTML are false;
• when publish.HTML is true, require transform.MarkdownToHTML != nil;
• when publish.HTML is true, require transform.MarkdownToHTML.Enabled == true;
• when publish.HTML is true, require transform.MarkdownToHTML.Mode == TransformModeSidecar;
• when Markdown-to-HTML is configured and enabled, reject any mode other than TransformModeSidecar;
• when Markdown-to-HTML is configured but disabled, allow empty mode or TransformModeSidecar and reject other modes;
• return concise errors that can be wrapped with config field context.

Update internal/config.Validate to use this helper while preserving contextual error messages such as pipelines[0].destinations[0].transform....

Update internal/publish.validateRequest to use the same helper for programmatic requests. The publish-layer error may be less field-path-specific, but it must remain actionable.

Do not change defaulting behavior in ApplyDefaults.

Tests

Add table tests covering:

• source-only publish allowed;
• html-only publish with enabled sidecar transform allowed;
• source-plus-html with enabled sidecar transform allowed;
• no outputs rejected;
• html without transform rejected;
• html with disabled transform rejected;
• html with wrong mode rejected;
• enabled Markdown transform with wrong mode rejected.

Apply the table at both config validation and publish request validation levels.

Run:

go test ./internal/config ./internal/publish ./internal/app

Completion Criteria

• Effective publish/transform policy is checked through one helper.
• Config-loaded and programmatic publish paths cannot drift on this policy.

Stage 4: Introduce App Backend Factory Wiring

Goal

Move runtime backend construction toward the storage registry before remote backends are implemented.

Implementation

Create an app-level backend factory, preferably in internal/app/backends.go.

Required shape:

• The factory owns a *storage.Registry.
• The default factory registers only the local backend for now.
• Local backend registration maps storage open config key path to local.New(path).
• Source and destination config conversion stays in internal/app; adapter packages must not import config types.
• Unsupported SSH/SFTP and S3 execution must continue to fail clearly as not implemented for execution.

Suggested API:

type backendFactory struct {
    registry *storage.Registry
}

func newBackendFactory() *backendFactory
func (f *backendFactory) openSource(ctx context.Context, source config.Backend) (storage.Backend, error)
func (f *backendFactory) openDestination(ctx context.Context, destination config.Destination) (storage.Backend, error)
func (f *backendFactory) openLocalPath(ctx context.Context, path string) (storage.Backend, error)

Use the factory from:

• app.Run for pipeline sources and destinations;
• app.Validate for direct local path validation;
• app.Inspect for direct local path inspection.

Keep validate and inspect direct-path commands local-only in this stage. Do not add config-driven validation or remote validation.

Keep public error behavior stable enough that current tests continue to assert meaningful substrings. It is acceptable to update exact error text if the new text is clearer and tests assert stable behavior rather than brittle phrasing.

Tests

Add tests for:

• factory opens a local source backend;
• factory opens a local destination backend;
• factory opens a direct local path;
• factory rejects unsupported source backend with a clear execution-not-implemented error;
• factory rejects unsupported destination backend with a clear execution-not-implemented error.

Run:

go test ./internal/app ./internal/cli

Then run:

go test ./...

Completion Criteria

• internal/app no longer directly constructs local backends in multiple command paths.
• Backend construction goes through one app-level factory and storage registry.
• No SSH/SFTP or S3 backend implementation is added.

Stage 5: Introduce Transform Resolver Wiring

Goal

Remove direct Markdown transform construction from internal/publish and make transform selection explicit and testable.

Implementation

Do not make internal/transform import internal/transform/markdown; that would create the wrong dependency direction. The app layer should own default transform registration.

In internal/publish, define a narrow resolver interface:

type TransformerResolver interface {
    Get(name string) (transform.Transformer, bool)
}

Add a resolver field to publish.Request, for example:

Transformers TransformerResolver

Update output planning so HTML generation:

• looks up transform.MarkdownToHTML through the resolver;
• fails clearly if the resolver is nil or the Markdown transformer is not registered;
• uses the resolved transformer to generate outputs.

Create app-level transform registry wiring, preferably in internal/app/transforms.go:

• create a transform.Registry;
• register transform.MarkdownToHTML with markdown.New();
• pass the registry into every publish request created by app.Run.

Update publish tests to use either:

• a tiny fake resolver and fake transformer for publish package tests; or
• a local registry assembled in the test.

Use app tests to prove the real Markdown transformer remains wired for end-to-end local HTML publication.

Tests

Add or update tests for:

• publish planning fails when HTML is requested and no transformer resolver is supplied;
• publish planning fails when Markdown transformer is missing from the resolver;
• publish planning uses a registered transformer;
• app local HTML publication still produces report.html;
• existing Markdown transformer tests remain focused on Markdown rendering.

Run:

go test ./internal/publish ./internal/transform ./internal/transform/markdown ./internal/app

Then run:

go test ./...

Completion Criteria

• internal/publish no longer imports internal/transform/markdown.
• App wiring registers the MVP Markdown transform explicitly.
• Transform behavior and public CLI behavior are unchanged.

Stage 6: Add Focused CLI Preflight Helpers

Goal

Reduce small CLI parsing drift without hiding command behavior behind a large framework.

Implementation

Keep the hand-written standard-library CLI. Do not introduce a new CLI dependency.

Add small helpers in internal/cli, such as:

func parseOptionalPathArg(stderr io.Writer, command string, args []string) (string, bool)
func rejectPositionalArgs(stderr io.Writer, command string, args []string) bool

Use them to simplify:

• validateCommand;
• inspectCommand;
• run positional argument rejection after flag parsing.

Keep each command's help text local to that command. Keep hasHelp, fail, and root dispatch behavior simple and explicit.

Do not add aliases, output modes, config loading for validate/inspect, or new flags.

Tests

Add table tests for:

• validate with zero args returns app-level required-path error;
• validate with one arg succeeds for a valid bundle;
• validate with two args returns usage;
• inspect with zero args returns app-level required-path error;
• inspect with one arg succeeds for a valid bundle;
• inspect with two args returns usage;
• run rejects extra positional args after flags.

Run:

go test ./internal/cli ./internal/app

Completion Criteria

• CLI command bodies are still readable.
• Common preflight parsing behavior is centralized where it is actually shared.
• Public CLI behavior remains unchanged.

Stage 7: Add Test Fixture Helper Foundation

Goal

Create shared test helpers for high-value fixtures without forcing every test to use them immediately.

Implementation

Create internal/testutil for test support used by multiple internal packages.

This package may contain regular Go files even though it is intended only for tests. Production code must not import internal/testutil.

Initial helper coverage:

• valid source bundle data:
  • default id weather.daily.brentwood.2026-05-30;
  • default created time 2026-05-30T11:10:00Z;
  • default files report.md with # Report\nSunny.\n and summary.txt with Summary\n;
• filesystem source bundle writer;
• fake-backend source bundle writer;
• minimal local config writer;
• fan-out local config writer;
• destination state writer;
• destination state reader.

Helpers should return normal project types such as bundle.Manifest, bundle.Bundle, and state.DistributorState.

Do not move edge-case test logic into testutil. Tests for invalid manifests, collisions, symlinks, failures, and backend-specific behavior should remain close to the package being tested.

Tests

Do not add tests for testutil itself unless helpers contain nontrivial logic not covered by consuming tests.

Migrate only one or two low-risk test files in this stage to prove the helpers work. Good candidates:

• internal/publish/execute_test.go;
• internal/transform/markdown/markdown_test.go.

Run:

go test ./internal/testutil ./internal/publish ./internal/transform/markdown

Then run:

go test ./...

Completion Criteria

• A shared fixture foundation exists.
• At least two packages use it successfully.
• The migration is incremental and does not obscure package-specific assertions.

Stage 8: Migrate High-Value Duplicate Test Fixtures

Goal

Reduce the largest remaining test fixture duplication after the helper foundation is proven.

Implementation

Migrate duplicated valid bundle/config/state setup in:

• internal/app/run_test.go;
• internal/cli/root_test.go;
• internal/publish/output_test.go;
• internal/state/distributor_test.go, where helper use improves clarity.

Keep tests local when custom setup makes the behavior clearer than a shared helper. Do not chase 100 percent fixture reuse.

Preserve all existing behavioral assertions.

Tests

Run:

go test ./internal/app ./internal/cli ./internal/publish ./internal/state

Then run:

go test ./...

Completion Criteria

• The largest repeated valid bundle/config/state setup is centralized.
• Edge-case tests remain readable.
• No production code imports internal/testutil.

Stage 9: Final Dead-Code and Low-Value Cleanup Sweep

Goal

Remove or defer remaining low-value cleanup items after the higher-impact centralization work is complete.

Implementation

Review and decide on these items:

• remove app.ErrNotImplemented if it is still unused;
• remove or expand internal/app/pipeline.go if the placeholder Pipeline type is still unused;
• keep internal/logging.Configure if it is still a planned extension point, otherwise remove it only if no code or docs reference it;
• decide whether empty-path display helpers should remain local or move to a single helper;
• leave config.Backend and config.Destination field duplication alone unless remote backend implementation work is starting immediately.

Do not add:

• generic workflow engine;
• plugin architecture;
• broad CLI framework;
• schema rewrites;
• remote backend behavior;
• output/report formatting package unless it is now clearly shared by multiple commands.

Tests

Run the full suite:

go test ./...

If removals affect docs or internal docs, update only implemented-behavior docs.

Completion Criteria

• Obvious dead code is removed or explicitly left in place for a documented reason.
• Remaining duplication is either low-value or intentionally deferred.
• The codebase is ready to resume roadmap work on remote backends.

Deferred Cleanup

Do not implement these as part of the cleanup roadmap unless a later roadmap explicitly promotes them:

• embedding a shared backend config struct into config.Destination;
• generic output/report formatting package;
• broader test fixture migration beyond the high-value repeated fixtures;
• remote backend implementations;
• force overwrite behavior;
• generic pipeline/workflow engine;
• broad plugin system.