342 lines
17 KiB
Markdown
342 lines
17 KiB
Markdown
# Workstream 9 Implementation Plan — Cleanup As The Final Pipeline Stage
|
|
|
|
## Status
|
|
|
|
Planned implementation for Workstream 9, **Cleanup as the final pipeline
|
|
stage**, in [`refactoring.md`](refactoring.md). Workstreams 1 through 8 are
|
|
complete prerequisites. This plan does not independently undertake Workstream
|
|
10; its test and documentation requirements apply only where this work changes
|
|
the owning behavior.
|
|
|
|
## Authoritative Scope And Fixed Decisions
|
|
|
|
Workstream 9 in [`refactoring.md`](refactoring.md) is authoritative. Its
|
|
supporting records are H20 and H21 in [`audit.md`](audit.md), Findings 2 and 4
|
|
in [`contract-audit.md`](contract-audit.md), and C02 and C04 in
|
|
[`audit-decisions.md`](audit-decisions.md). Read those cited sections rather
|
|
than rereading or reinterpreting the complete audits.
|
|
|
|
The following decisions are final:
|
|
|
|
- Add `cleanup` after `notify` as the twelfth and final canonical stage.
|
|
`publish` commits remote state, `notify` completes delivery behavior, and
|
|
only then may `cleanup` remove local run data.
|
|
- Retain the existing configuration names
|
|
`pipeline.spool.delete_audio_after_publish` and
|
|
`pipeline.workspace.cleanup_after_publish`. They select cleanup-stage
|
|
targets; they no longer cause a runner-finalization side effect.
|
|
- When neither policy is enabled, `cleanup` returns an explicit self-skip with
|
|
no outputs. Self-skips remain reconsidered on later invocations under the
|
|
ordinary runner contract.
|
|
- When either policy is enabled, cleanup requires a succeeded `publish` stage
|
|
whose metadata proves a completed pointer-selected upload: `uploaded=true`,
|
|
`skipped` is not true, and canonical nonempty `published_run_id`,
|
|
`remote_commit_key`, and `current_commit_pointer_key` values agree with the
|
|
manifest's campaign/session remote prefix. Missing, malformed, skipped, or
|
|
disabled publication evidence is an error and no deletion begins.
|
|
- The succeeded publish record is sufficient durable evidence. Cleanup does
|
|
not reread object storage: publish success is recorded only after the pointer
|
|
write, immutable run objects remain durable if a later pointer supersedes
|
|
them, and adding a network dependency would make local retry less reliable.
|
|
- Compute targets from the validated published run ID, manifest campaign and
|
|
session identities, and the currently resolved application-owned workspace
|
|
and spool roots using canonical artifact path helpers. Never use the new
|
|
invocation's `RunID`, `LocalWorkDir`, or `LocalSpoolDir`, and never accept a
|
|
path from publish metadata as a deletion target.
|
|
- Process requested targets deterministically in spool-then-workspace order.
|
|
Use `fileops.RemoveAllUnderRoot`; a missing root or target is successful.
|
|
Preserve refusal of a root itself, traversal, and out-of-root deletion.
|
|
- Do not record per-target progress. If one deletion succeeds and a later one
|
|
fails, the ordinary stage record becomes failed. A later run recomputes the
|
|
same canonical targets, treats the first as already absent, and retries the
|
|
remainder.
|
|
- A successful cleanup produces no canonical artifacts. Its bounded metadata
|
|
may identify the published run and completed policy names, but must not
|
|
become a second cleanup ledger.
|
|
- Cleanup has a semantic fingerprint over the two policy booleans and their
|
|
normalized configured roots. Its resume validator revalidates publish
|
|
evidence and confirms every enabled canonical target is absent. Recreated or
|
|
newly enabled targets make a prior success non-resumable.
|
|
- Reject statically impossible configuration: either cleanup policy requires
|
|
publish to be enabled with run upload enabled. Dynamic absence or corruption
|
|
of publish evidence remains a cleanup-stage error.
|
|
- `notify -> cleanup` is the final direct invalidation edge and `cleanup` has
|
|
no dependents. This makes every earlier prerequisite transitively stale a
|
|
prior cleanup result while retaining the fixed relation separately from
|
|
execution order.
|
|
- A default full run includes cleanup. `--through notify`, any earlier bounded
|
|
range, `narratio publish`, and artifact-regeneration aliases do not perform
|
|
cleanup. `narratio run-stage cleanup SESSION` is the explicit single-stage
|
|
retry path and applies cleanup's own publication-evidence checks.
|
|
- Remove `post_publish_cleanup` completely. Do not migrate, reinterpret, or
|
|
retain its types, validators, runner hooks, restore clearing, fixtures, or
|
|
tests. Keep session manifest schema version `narratio.session-manifest.v1`:
|
|
the retired field was optional, and strict decoding will reject manifests
|
|
that actually contain it without invalidating otherwise canonical v1 state.
|
|
|
|
## Instructions For Every Stage
|
|
|
|
Before each stage:
|
|
|
|
1. Read `docs/development.md`, all documents under `docs/policy/`, this plan,
|
|
Workstream 9, and only its cited audit sections. Read the affected portions
|
|
of [`../internal/overview.md`](../internal/overview.md),
|
|
[`../internal/workspace.md`](../internal/workspace.md),
|
|
[`../internal/manifest.md`](../internal/manifest.md),
|
|
[`../internal/stage-publish.md`](../internal/stage-publish.md),
|
|
[`../config.md`](../config.md), [`../cli.md`](../cli.md), and
|
|
[`../operations.md`](../operations.md).
|
|
2. Inspect affected production code and focused tests. Prefer the codebase
|
|
knowledge graph for Go discovery and call tracing; use text search for
|
|
configuration, schemas, fixtures, documentation, and removal checks.
|
|
3. Confirm worktree state and preserve unrelated changes. Do not implement
|
|
Workstream 10 work that is not made necessary by this cleanup change.
|
|
|
|
During each stage, use ordinary stage and manifest transitions, keep paths
|
|
root-confined, preserve group-owned workspace behavior, avoid raw secrets,
|
|
honor context before destructive work, and keep tests deterministic and
|
|
offline. Prefer real temporary filesystems over private deletion hooks. Add
|
|
tests for meaningful destructive, recovery, range, and persistence boundaries;
|
|
delete tests whose only subject is the retired obligation choreography. Run
|
|
`gofmt` on changed Go files and `git diff --check`. Commit only a completed
|
|
stage with a short plain-English message, and update each canonical document in
|
|
the same stage as the behavior it owns.
|
|
|
|
## Stage 1 — Define Cleanup Evidence, Targets, And Configuration Rules
|
|
|
|
**Status:** Completed
|
|
|
|
### Goal
|
|
|
|
Create one deterministic, side-effect-free cleanup plan and reject
|
|
configurations that can never authorize it.
|
|
|
|
### Required Work
|
|
|
|
1. Add package-private cleanup planning types in `internal/stage`. A plan
|
|
contains the validated published run ID and zero, one, or two root/target/
|
|
policy entries in deterministic spool-then-workspace order. Do not register
|
|
the stage or delete files yet.
|
|
2. If both cleanup flags are false, return a disabled plan without requiring
|
|
publish evidence. Otherwise require and validate the succeeded `publish`
|
|
record and its metadata exactly as fixed above. Use artifact identity/key
|
|
helpers rather than string-prefix approximations; add a narrow helper only
|
|
if the existing remote-key API cannot validate the commit identity.
|
|
3. Construct the spool target with `artifacts.SessionSpoolAudioDir` and the
|
|
workspace target with `artifacts.SessionRunRootForCampaign`, always using
|
|
the publish metadata's run ID. Validate manifest/config identities before
|
|
constructing paths. Do not consume `Manifest.LocalWorkDir` or
|
|
`Manifest.LocalSpoolDir`.
|
|
4. Add cross-field configuration validation so either cleanup flag requires
|
|
effective `publish.enabled=true` and `publish.upload_run=true`. Preserve
|
|
existing defaults and field names. Add focused tests for each impossible
|
|
combination and both valid enabled policies without duplicating exact error
|
|
prose outside the config owner.
|
|
5. Table-test planning for disabled cleanup, each target selection, stable
|
|
ordering, prior-run publication, missing/failed/skipped publish state,
|
|
false or mistyped metadata, invalid run identity, wrong commit key, wrong
|
|
pointer key, and canonical paths. Prove every invalid evidence case produces
|
|
no plan that can be executed.
|
|
6. Update the cleanup fields in `docs/config.md` in this stage to describe the
|
|
new validation and their role as final-stage target selectors. Do not yet
|
|
describe the unregistered stage as implemented elsewhere.
|
|
|
|
### Verification And Exit Criteria
|
|
|
|
Run `go test ./internal/config ./internal/stage -run 'Cleanup|Publish'` and
|
|
`git diff --check`. Cleanup authorization and targets are deterministic and
|
|
cannot be derived from the current invocation or uncommitted publication.
|
|
|
|
## Stage 2 — Implement Idempotent Cleanup Stage Behavior
|
|
|
|
**Status:** Completed
|
|
|
|
### Goal
|
|
|
|
Implement cleanup as an ordinary, independently testable stage without yet
|
|
changing the canonical pipeline.
|
|
|
|
### Required Work
|
|
|
|
1. Add `cleanupStage` with name `cleanup`. Its `Run` obtains the Stage 1 plan,
|
|
self-skips when disabled, checks context before each target, and removes
|
|
each enabled target with the root-scoped filesystem helper.
|
|
2. Make `fileops.RemoveAllUnderRoot` explicitly idempotent when either the
|
|
configured root or target is already absent. Do not weaken rejection of
|
|
root deletion, traversal, or out-of-root targets and do not introduce a
|
|
general filesystem facade.
|
|
3. Return a successful `StageResult` with no outputs, logs, or generated
|
|
configuration. Keep metadata bounded to the published run and completed
|
|
policy identifiers; do not persist target paths, completion flags, or an
|
|
obligation-like structure.
|
|
4. Implement cleanup semantic fingerprinting with a new private versioned
|
|
payload containing normalized roots and policy booleans. Implement
|
|
`ResumeValidator`: disabled cleanup is reconsidered through self-skip rather
|
|
than reusable success; enabled cleanup first validates the current publish
|
|
evidence and then reports resumable only when all planned targets remain
|
|
absent.
|
|
5. Test through the stage contract with `t.TempDir`: disabled self-skip;
|
|
spool-only, workspace-only, and both; preservation of another run and the
|
|
durable cache; already-absent root/target; cancellation before deletion;
|
|
refusal of unsafe scope; first-target success followed by second-target
|
|
failure and successful idempotent retry; no canonical outputs; fingerprint
|
|
changes; and resume after success versus target recreation. Use real
|
|
filesystem state rather than adding mutable package globals or a deletion
|
|
interface solely for tests.
|
|
|
|
### Verification And Exit Criteria
|
|
|
|
Run `go test ./internal/fileops ./internal/stage -run Cleanup`,
|
|
`go test -race ./internal/stage -run Cleanup`, and `git diff --check`. The
|
|
unregistered stage provides safe, idempotent behavior through existing
|
|
filesystem and manifest contracts.
|
|
|
|
## Stage 3 — Integrate Cleanup Into Canonical Orchestration
|
|
|
|
**Status:** Completed
|
|
|
|
### Goal
|
|
|
|
Make cleanup the normal final stage and remove cleanup from runner
|
|
finalization.
|
|
|
|
### Required Work
|
|
|
|
1. Register `cleanupStage{}` after `notify` in `stage.All`; make it selectable
|
|
by the existing planner and `run-stage` command without a special command or
|
|
alias.
|
|
2. Extend the validated invalidation relation with `notify -> cleanup` and an
|
|
empty cleanup classification. Update transitive expectations so every
|
|
earlier stage reaches cleanup while preserving the render/extract sibling
|
|
relationship.
|
|
3. Reduce `finalizeInvocation` to summary construction. Delete its cleanup
|
|
call and cleanup-specific terminal-failure path. Remove `removeScopedDir`
|
|
from `invocationDependencies` and all normalization/test plumbing made dead
|
|
by that removal.
|
|
4. Delete `internal/app/post_publish_cleanup.go`. Retain application cleanup
|
|
helpers only where the explicit `clean` command still uses them; remove or
|
|
rename types and wrappers whose only owner was post-publish cleanup.
|
|
5. Replace `post_publish_cleanup_test.go` with proportionate runner-level
|
|
integration coverage. Prove full order ends in cleanup, notify failure
|
|
prevents cleanup, publish failure prevents cleanup, cleanup failure leaves
|
|
publish succeeded and remote state untouched, a later ordinary full run
|
|
skips reusable upstream work and retries cleanup, and a forced prerequisite
|
|
stales cleanup.
|
|
6. Cover selection semantics: the default full plan includes cleanup;
|
|
`--through notify` and earlier ranges do not delete; `publish` executes only
|
|
publish; direct `run-stage cleanup` works with valid evidence and fails
|
|
safely without it; artifact selection remains accepted only by analyze and
|
|
publish. Update plan, command, assembled-workflow, bounded-prerequisite,
|
|
runner, and stage-count expectations without broad snapshots.
|
|
7. Update current-behavior owners in the same stage: architecture's publish and
|
|
cleanup boundary, the internal overview/stage list and lifecycle,
|
|
`docs/cli.md`, `docs/operations.md`, `docs/internal/workspace.md`, and
|
|
`docs/internal/stage-publish.md`. Add a focused
|
|
`docs/internal/stage-cleanup.md` and link it from the overview. Clearly state
|
|
that the standalone publish command does not invoke cleanup and normal
|
|
resume retries a failed cleanup stage without republishing valid state.
|
|
|
|
### Verification And Exit Criteria
|
|
|
|
Run `go test ./internal/stage ./internal/app`,
|
|
`go test -race ./internal/app -run 'Cleanup|Runner|RunStage|Bounded|Plan'`,
|
|
`go test ./internal/doccheck`, and `git diff --check`. Execution order, range
|
|
behavior, failure attribution, and retry are ordinary stage behavior, with no
|
|
post-loop deletion path.
|
|
|
|
## Stage 4 — Remove The Cleanup Obligation From Manifest State
|
|
|
|
**Status:** Pending
|
|
|
|
### Goal
|
|
|
|
Delete the second cleanup state machine and leave stage status as the only
|
|
durable cleanup progress model.
|
|
|
|
### Required Work
|
|
|
|
1. Remove `CleanupTarget`, `PostPublishCleanup`, and
|
|
`Manifest.PostPublishCleanup`; remove their validation, cloning, test
|
|
helpers, JSON fixtures, and persistence assertions.
|
|
2. Remove restore's special clearing of `post_publish_cleanup`. Restore should
|
|
treat cleanup exactly like other stage state and must not synthesize or
|
|
reset a separate obligation.
|
|
3. Keep `SessionManifestSchemaVersion` at v1. Confirm strict decoding rejects a
|
|
literal `post_publish_cleanup` field as unknown, while canonical v1
|
|
manifests without it still round-trip unchanged. Add no compatibility
|
|
reader, migration code, or silent field discard.
|
|
4. Update `docs/internal/manifest.md`, restore/workspace internals, operations,
|
|
and troubleshooting where they own the removed state or recovery advice.
|
|
Migration guidance should tell operators with an outstanding old obligation
|
|
to finish cleanup with the pre-upgrade binary or verify remote publication,
|
|
remove/regenerate the incompatible local manifest, and then use the cleanup
|
|
stage. Do not present manual JSON editing as the normal migration path.
|
|
5. Search production, tests, fixtures, examples, and non-audit documentation
|
|
for `PostPublishCleanup`, `CleanupTarget`, `post_publish_cleanup`,
|
|
`runPostPublishCleanup`, and `post-publish cleanup obligation`. Remove every
|
|
live reference; historical roadmap/audit records may retain the terms.
|
|
|
|
### Verification And Exit Criteria
|
|
|
|
Run `go test ./internal/manifest ./internal/app ./internal/stage`,
|
|
`go test ./internal/doccheck`, and `git diff --check`. The session manifest
|
|
contains only the ordinary cleanup stage record, and old obligation-bearing
|
|
documents fail strict decoding rather than entering a compatibility path.
|
|
|
|
## Stage 5 — Consolidate, Validate, And Close Workstream 9
|
|
|
|
**Status:** Pending
|
|
|
|
### Goal
|
|
|
|
Ensure the reduced implementation, tests, and canonical documentation agree,
|
|
then record the workstream complete.
|
|
|
|
### Required Work
|
|
|
|
1. Review the complete diff for duplicated evidence/target logic, obsolete
|
|
runner branches, redundant wrappers, stale fixtures, accidental remote
|
|
cleanup reads, and any new per-target persistence. Consolidate only into the
|
|
narrow owners established above.
|
|
2. Confirm the effective behavioral matrix: disabled self-skip; valid publish
|
|
evidence before deletion; final-stage ordering; bounded exclusion; direct
|
|
retry; idempotent absence; partial-deletion recovery; publish success
|
|
surviving cleanup failure; semantic invalidation; and destructive
|
|
confinement.
|
|
3. Check all current-behavior documentation, CLI help, examples, configuration
|
|
diffs, stage lists, and migration guidance. Volatile details must have one
|
|
canonical owner and roadmap documents must not be cited as current runtime
|
|
documentation.
|
|
4. Run every repository-wide validation command from `docs/development.md`,
|
|
plus `git diff --check`. Run the scheduled shuffle/repetition command on the
|
|
affected `internal/app`, `internal/stage`, `internal/manifest`, and
|
|
`internal/fileops` packages. Record platform-only limitations without
|
|
weakening or silently skipping applicable checks.
|
|
5. Only after all checks pass, mark every stage in this plan and Workstream 9
|
|
in `refactoring.md` **Complete**. Leave Workstream 10 as the next pending
|
|
scope and do not prepare or publish a release.
|
|
|
|
### Verification And Exit Criteria
|
|
|
|
At minimum run:
|
|
|
|
```sh
|
|
go test ./...
|
|
go test -race ./...
|
|
go test -race -shuffle=on -count=3 ./internal/app ./internal/stage ./internal/manifest ./internal/fileops
|
|
go vet ./...
|
|
go build ./...
|
|
go test ./internal/doccheck
|
|
go test ./internal/config -run '^TestExamplesLoadAndValidate$'
|
|
git diff --check
|
|
```
|
|
|
|
The worktree contains one final canonical cleanup stage, no special cleanup
|
|
obligation or runner side effect, aligned documentation, and no Workstream 10
|
|
scope beyond changes required by this behavior.
|
|
|
|
## Open Questions
|
|
|
|
None. The approved audits, refactoring roadmap, and fixed decisions above make
|
|
this implementation plan decision-complete.
|