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