Files
narratio/docs/roadmap/cleanup.md

13 KiB

Roadmap: Pre-1.0 Code Cleanup

Status: Planned

This roadmap turns the findings in docs/roadmap/audit.md into staged cleanup work for the 1.0 release. It is planning-only. Do not implement these refactors until a stage is explicitly selected for implementation.

The cleanup work must follow the policy documents under docs/policy/, especially these invariants:

  • keep Narratio explicit and stage-driven;
  • do not introduce a generic workflow engine, DAG abstraction, or generic CLI framework;
  • keep external-system details behind adapters;
  • do not move campaign/session/root-prefix semantics into storage adapters;
  • keep AWS SDK types out of app and stage logic;
  • keep path and remote key construction centralized;
  • preserve manifest-driven run state;
  • keep public CLI/config behavior stable unless a stage explicitly says it is an internal naming cleanup.

Non-Goals

  • Do not change public command syntax, config schema, S3 key layout, manifest schema, or artifact source IDs as part of this cleanup.
  • Do not add compatibility aliases or migration logic.
  • Do not rewrite stage execution, manifest state transitions, or adapter contracts.
  • Do not generalize text output into a generic reporting framework.
  • Do not fold S3 audio cache behavior into a generic downloader.
  • Do not move secret loading into storage adapters.

Stage 1: Shared Low-Risk Mechanics

Goal: remove duplicated mechanics that are easy to test and should not affect public behavior.

Implementation decisions:

  • Add one shared helper for safe relative artifact destination normalization.
    • It must reject empty paths, absolute paths, ., .., and traversal outside the artifact/session scope.
    • It must normalize separators to slash-form for artifact and S3 destination logic.
    • It must be dependency-light enough to be called from config validation, app helpers, publish execution, and previous-cache planning.
  • Add one shared object-store temp download helper.
    • It must take context.Context, storage.ObjectStore, a key, and a temp-file pattern.
    • It must create and close the temp file before download, remove the temp file on failed download, and return a cleaned local path on success.
    • It must not infer bucket, campaign, session, run, or root-prefix semantics.
  • Extract shared cleanup target validation for local deletion.
    • Cover scoped directory deletion, scoped file deletion, and removable children under a root.
    • Preserve existing safety rules: reject empty roots/targets, root deletion, outside-root paths, symlinks, and wrong target types.
    • Keep command-specific output in clean and manifest metadata handling in post-publish cleanup.

Expected callers:

  • replace duplicate relative destination normalization in publish execution, helper command rendering, and previous-cache planning;
  • replace duplicate temp download helpers in app and previous-cache code;
  • replace duplicate scoped deletion validation in clean and post-publish cleanup.

Tests:

  • Add focused tests for destination normalization and path traversal rejection.
  • Add temp download tests for success, failed download cleanup, and preserved contextual caller errors.
  • Add shared cleanup validation tests for directories, files, symlinks, missing targets, root deletion, and outside-root targets.
  • Run:
    • go test ./internal/artifacts -v
    • go test ./internal/adapters/storage -v
    • go test ./internal/app -run 'Clean|Post' -v
    • go test ./...

Completion criteria:

  • duplicated low-level mechanics are removed;
  • public behavior and output are unchanged;
  • no stage, command, or config semantics move into storage adapters.

Stage 2: Artifact Source and Published Output Policy

Goal: make artifact source IDs and published-output destination derivation a single shared policy.

Implementation decisions:

  • Introduce internal/artifactpolicy as the shared source policy package.
    • This package is the long-term home because it avoids config/artifacts import cycles.
    • It may depend on dependency-light model packages, but it must not depend on app, stage, manifest stores, storage adapters, or downstream adapters.
  • Centralize these behaviors in internal/artifactpolicy:
    • classify source IDs as built-in, configured artifact, or previous-session configured artifact;
    • parse configured artifact keys from narratio.artifact.<key>;
    • parse previous-session artifact keys from narratio.previous_session.artifact.<key>;
    • validate configured artifact sources against pipeline.scriptorium.artifacts;
    • validate publish lock/output sources;
    • derive default published destinations for built-in and configured artifact sources;
    • normalize safe relative published-output destinations.
  • Update callers to consume the shared policy:
    • config validation for publish.outputs and publish.locks;
    • publish-stage output resolution;
    • status and artifacts list rendering;
    • locks list/add/remove validation;
    • analyze input source handling;
    • previous-cache candidate planning.
  • Preserve caller-specific policy at call sites.
    • Required vs optional behavior remains in publish, analyze, restore, and previous-cache callers.
    • Locked output behavior remains in publish.
    • Text formatting remains in app commands.
    • Manifest path scanning remains in artifact/previous-cache logic unless directly tied to source policy.

Tests:

  • Add internal/artifactpolicy table tests for source classification, configured artifact validation, previous-session parsing, default destination derivation, and destination normalization.
  • Update internal/config tests so publish outputs and locks validate through the shared policy.
  • Update internal/stage publish tests for selected, unselected, optional, required, and locked output behavior.
  • Update internal/app tests for status, artifacts list, and locks.
  • Update internal/previouscache tests for previous-session candidate ordering.
  • Run:
    • go test ./internal/artifacts -v
    • go test ./internal/config -v
    • go test ./internal/stage -run 'Analyze|Publish' -v
    • go test ./internal/app -run 'Artifacts|Status|Locks' -v
    • go test ./internal/previouscache -v
    • go test ./...

Completion criteria:

  • artifact source vocabulary and destination derivation are no longer reimplemented in config, app, stage, and previous-cache packages;
  • every caller still owns its own missing/required/optional/locked decision;
  • public behavior is unchanged.

Stage 3: Publish Terminology Cleanup

Goal: align internal implementation names with the public publish contract.

Implementation decisions:

  • Rename archive-named internal files, types, helpers, comments, and tests that now implement publish behavior.
  • Replace names such as:
    • archiveStage with publishStage;
    • ResolveArchiveSessionPrefix with publish/current-state terminology;
    • ResolveArchiveRunPrefix with publish/run-history terminology;
    • ResolveArchiveCurrentStateKeys with current-state terminology;
    • runPostArchiveCleanup with post-publish cleanup terminology;
    • staticArchiveLocks with publish lock terminology.
  • Keep the S3 layout stable:
    • {session_prefix}/runs/{run_id}/;
    • {session_prefix}/current/manifest.json;
    • {session_prefix}/current/run_id.txt;
    • {session_prefix}/locks.yml.
  • Keep the public stage name publish.
  • Keep old archive/promote references only where they are historical roadmap context or intentionally describe immutable run history.

Tests and checks:

  • Run:
    • go test ./internal/stage -v
    • go test ./internal/app -v
    • go test ./internal/artifacts -v
    • go test ./...
  • Run stale-term sweeps:
    • rg -n "archive|promote|promoted|promotion" internal docs examples cmd
    • rg -n "ResolveArchive|archiveStage|post_archive|staticArchive|normalizeArchive" internal

Completion criteria:

  • public publish behavior is no longer implemented through archive/promote names;
  • remaining old terms are intentionally historical, test-fixture bucket names, or roadmap-only context;
  • no config, CLI, manifest, or S3 layout changes are introduced.

Stage 4: Session Command Parsing Consolidation

Goal: reduce command-loading drift while keeping command handlers explicit.

Implementation decisions:

  • Add a small app-level parser helper for common session-aware commands.
  • Centralize:
    • common config flags: --config, --campaign, --campaign-file, --session;
    • positional session ID handling;
    • --session-id compatibility;
    • --previous-session-id;
    • optional selected-artifact parsing for commands that support it.
  • Keep command handlers explicit and readable.
  • Do not introduce a generic CLI framework.
  • Treat these as intentional special cases:
    • session init loads pipeline and campaign but not session;
    • clean --all loads pipeline only;
    • status --manifest remains local-manifest mode;
    • run-stage keeps its stage-name positional handling but reuses common flag parsing where practical.
  • Standardize flag help text where commands use the same semantics.

Tests:

  • Update app command tests for:
    • positional session ID;
    • --session-id;
    • positional/flag mismatch;
    • missing session ID;
    • --previous-session-id;
    • unsupported --artifacts by command/stage;
    • unchanged behavior for session init, clean --all, and status --manifest.
  • Run:
    • go test ./internal/app -run 'Run|RunStage|Analyze|Publish|Restore|Clean|Session' -v
    • go test ./internal/app -v
    • go test ./...

Completion criteria:

  • shared session flag/session ID behavior has one implementation;
  • command handlers remain command-specific;
  • public command syntax and output stay unchanged.

Stage 5: Remote Current-State Mechanics

Goal: centralize remote current-state loading mechanics without hiding caller policy.

Implementation decisions:

  • Extract narrow helpers for remote current state.
    • Load current run pointer through storage.ObjectStore.
    • Load and decode current manifest through storage.ObjectStore.
    • Validate campaign, session, and run identity when requested by the caller.
    • Return typed missing-state errors.
  • Preserve caller policy:
    • restore treats missing or invalid current state as an error;
    • previous-cache hydration fails for required previous artifacts and skips optional missing artifacts;
    • status reports missing remote state as state, not command failure;
    • session validate emits findings and fails only for error findings.
  • Keep all remote key construction in internal/artifacts.
  • Keep object-store initialization in internal/app.
  • Do not add storage adapter knowledge of campaigns, sessions, runs, root prefixes, current state, or manifests.

Tests:

  • Add helper tests for:
    • missing current run pointer;
    • missing current manifest;
    • empty run pointer;
    • malformed manifest;
    • campaign mismatch;
    • session mismatch;
    • run ID mismatch.
  • Update restore, previous-cache, status, and session validate tests to prove their caller-specific behavior is unchanged.
  • Run:
    • go test ./internal/app -run 'Restore|Status|SessionValidate' -v
    • go test ./internal/previouscache -v
    • go test ./...

Completion criteria:

  • low-level remote current-state mechanics are shared;
  • missing-state behavior remains caller-specific;
  • storage adapter boundaries remain unchanged.

Stage 6: Operator Helper File Split and Final Sweep

Goal: improve maintainability after shared policy and mechanics are already centralized.

Implementation decisions:

  • Split the large operator helper implementation by command or responsibility.
  • Suggested file grouping:
    • session init;
    • session validate;
    • status;
    • artifacts list;
    • locks;
    • helper findings;
    • helper artifact rendering.
  • Do not change command syntax, text output, config loading, remote loading, lock behavior, or artifact catalog behavior during the split.
  • Keep output formatting text-only and command-specific unless a concrete inconsistency remains after the split.
  • Update roadmap status notes after each completed stage.

Tests and checks:

  • Run:
    • go test ./internal/app -v
    • go test ./...
  • Final searches:
    • rg -n "archive|promote|promoted|promotion" internal docs examples cmd
    • rg -n "narratio.transcript.merged|narratio.transcript.full|narratio.transcript.trimmed" internal docs examples
    • rg -n "previous_session_artifact|promote_artifacts|pipeline.archive" internal docs examples

Completion criteria:

  • operator helper code is easier to navigate;
  • stale implementation terminology is removed or intentionally documented;
  • no behavior changes are introduced by file organization.

Overall Validation

After each implementation stage:

  • run the focused tests listed for that stage;
  • run go test ./...;
  • run git status --short;
  • update this roadmap to mark the completed stage implemented only after code, tests, and documentation are aligned.

Assumptions

  • This roadmap is a cleanup plan, not a feature plan.
  • Stages may be implemented as separate prompts/commits.
  • internal/artifactpolicy is the chosen home for shared source policy.
  • Shared object-store temp download helpers must not learn Narratio session semantics.
  • Public behavior must remain stable unless a stage explicitly says it is internal terminology cleanup.