Plan cleanup stage refactor
This commit is contained in:
@@ -1,386 +1,341 @@
|
||||
# Workstream 8 Implementation Plan — Remote Publication And Artifact Locks
|
||||
# Workstream 9 Implementation Plan — Cleanup As The Final Pipeline Stage
|
||||
|
||||
## Status
|
||||
|
||||
Completed implementation for Workstream 8, **Remote publication, per-artifact
|
||||
locks, and publish decomposition**, in [`refactoring.md`](refactoring.md).
|
||||
Workstreams 1 through 7 are complete prerequisites. This plan does not enter
|
||||
Workstream 9's cleanup-stage work.
|
||||
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 8 in [`refactoring.md`](refactoring.md) is authoritative. Its
|
||||
supporting records are H04, H15, H18, and H44 in [`audit.md`](audit.md), C05
|
||||
in [`contract-audit.md`](contract-audit.md), and their rows in
|
||||
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 repeating the audits.
|
||||
than rereading or reinterpreting the complete audits.
|
||||
|
||||
The following decisions are final:
|
||||
|
||||
- Keep `publishStage.Run` as the transaction owner. Split cohesive work into
|
||||
focused files within `internal/stage`; add no framework, distributed
|
||||
transaction, or third-party dependency.
|
||||
- Replace static pipeline locks and the shared remote YAML manifest with one
|
||||
object per normalized stable source at
|
||||
`{session_prefix}/locks/{sha256(source)}.lock`. The digest is lowercase
|
||||
hexadecimal SHA-256 of the exact normalized source bytes.
|
||||
- A lock is strict JSON with `format_version: 1`, `source`, optional
|
||||
`reason`, and an RFC 3339 UTC `created_at`. Define
|
||||
`MaxRemoteLockObjectBytes = 64 << 10` centrally in its package. Reject
|
||||
unknown fields, trailing values, invalid/noncanonical fields, unsupported
|
||||
versions, oversize objects, and payload/key hash mismatches.
|
||||
- `internal/artifacts` owns the payload, identity/key rules, and lock access;
|
||||
`internal/adapters/storage` owns object mechanics; `internal/app` owns
|
||||
commands; and `internal/stage` owns publish use.
|
||||
- Extend `storage.ObjectStore` with idempotent deletion. Listing validates
|
||||
every reserved-prefix object and sorts by source. Add is conditional-create;
|
||||
`--force` makes one generation-matched replacement; remove is idempotent.
|
||||
- Ignore old `locks.yml` objects—do not migrate or delete them. Remove their
|
||||
schema, merge/precedence/retry machinery, runtime injection, and static
|
||||
`pipeline.publish.locks`. Strict config decoding rejects that old field.
|
||||
- The initial lock snapshot controls planning. An unlocked selected output
|
||||
publishes normally. A locked output carries its exact declaration from the
|
||||
prior current commit, if any. A locked output never published remains absent
|
||||
even if configured as required. A lock removed after the initial read may
|
||||
therefore cause one conservative carry.
|
||||
- After immutable objects and the immutable commit are written, reread every
|
||||
initially unlocked relevant source. If any became locked, do not advance the
|
||||
pointer. Attempt objects may remain. A lock created after this final check
|
||||
may race the pointer write; do not add stronger transaction machinery.
|
||||
- A carried `published_output` may reference an earlier run only within the
|
||||
same campaign/session. Preserve source, key, checksum, size, and generation
|
||||
exactly. Derive origin from its validated key; do not add an origin field or
|
||||
copy old bytes. Verify existence, size, and generation without downloading.
|
||||
- Publish metadata and inspection distinguish exactly `published`,
|
||||
`carried_locked`, `locked_unpublished`, `optional_missing`, and
|
||||
`unselected` in one source-sorted `publish_output_outcomes` collection.
|
||||
Preserve upload counts and cleanup-consumed commit/pointer metadata until
|
||||
Workstream 9.
|
||||
- Preserve transaction order: immutable object upload/verification, immutable
|
||||
commit, final lock revalidation, then exactly one current-pointer write.
|
||||
- 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 8, its cited audit sections, and the affected sections of
|
||||
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),
|
||||
[`../internal/storage.md`](../internal/storage.md),
|
||||
[`../config.md`](../config.md), and
|
||||
[`../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/call tracing; use text search for schemas,
|
||||
config, CLI text, docs, fixtures, and removal checks.
|
||||
3. Confirm worktree state and preserve unrelated changes. Do not implement an
|
||||
adjacent workstream.
|
||||
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, keep focused tests passing, use explicit package-private
|
||||
helpers instead of generic managers/frameworks, keep reads bounded and bodies
|
||||
closed, honor contexts, wrap errors with safe object identity, preserve
|
||||
determinism, and keep tests offline. Run `gofmt` on changed Go files and
|
||||
`git diff --check`. Commit only a completed stage with a short plain-English
|
||||
message. Update documentation alongside observable changes.
|
||||
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 — Add Idempotent Object Deletion
|
||||
## Stage 1 — Define Cleanup Evidence, Targets, And Configuration Rules
|
||||
|
||||
**Status:** Completed
|
||||
**Status:** Pending
|
||||
|
||||
### Goal
|
||||
|
||||
Add the sole storage primitive required to remove one independent lock.
|
||||
Create one deterministic, side-effect-free cleanup plan and reject
|
||||
configurations that can never authorize it.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Add `Delete(ctx, key) error` to `storage.ObjectStore`. Absence succeeds;
|
||||
preserve existing key validation and contextual errors.
|
||||
2. Extend the narrow S3 client port and backend using the provider delete-object
|
||||
operation. Propagate cancellation, permission, transport, and other errors.
|
||||
3. Update every fake/implementation. The standard fake removes object and
|
||||
generation state, is idempotent, honors context, and permits injected-error
|
||||
tests consistently with its other operations.
|
||||
4. Add no batch, recursive, or lock-specific storage API.
|
||||
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
|
||||
|
||||
Test success, absence, cancellation, backend failures, and S3 request mapping.
|
||||
Run `go test ./internal/adapters/storage`,
|
||||
`go test ./internal/artifacts ./internal/stage ./internal/app`, and
|
||||
`git diff --check`. All implementations satisfy the narrow idempotent port.
|
||||
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 — Define Independent Lock Objects
|
||||
## Stage 2 — Implement Idempotent Cleanup Stage Behavior
|
||||
|
||||
**Status:** Completed
|
||||
**Status:** Pending
|
||||
|
||||
### Goal
|
||||
|
||||
Establish the complete lock contract before migrating callers.
|
||||
Implement cleanup as an ordinary, independently testable stage without yet
|
||||
changing the canonical pipeline.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Add a cohesive `internal/artifacts` owner for payloads, size bound, canonical
|
||||
source validation, key construction, and strict encoding/decoding.
|
||||
2. Reuse publish's canonical stable-source vocabulary. Reject noncanonical
|
||||
caller input rather than making a second identity. Validate the session
|
||||
prefix before joining the reserved `locks/` path.
|
||||
3. Implement exact lookup and prefix listing. Exact lookup returns an explicit
|
||||
present/absent result. Listing bounds every read, rejects unexpected names,
|
||||
verifies hashes, rejects duplicate sources, and sorts by source.
|
||||
4. Implement conditional creation, one generation-matched forced replacement,
|
||||
and idempotent removal. A replacement conflict is reported for operator
|
||||
retry; do not add an internal retry loop.
|
||||
5. Do not consult static config or `locks.yml`.
|
||||
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
|
||||
|
||||
Table-test canonical keys, strict JSON, size limits, hash mismatches, absence,
|
||||
invalid prefix entries, ordering, add conflicts, force conflicts/replacement,
|
||||
and removal. Run `go test ./internal/artifacts ./internal/adapters/storage` and
|
||||
`git diff --check`. Each lock is independently manageable and validated.
|
||||
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 — Permit Safe Cross-Run Declarations
|
||||
## Stage 3 — Integrate Cleanup Into Canonical Orchestration
|
||||
|
||||
**Status:** Completed
|
||||
**Status:** Pending
|
||||
|
||||
### Goal
|
||||
|
||||
Permit exact older published declarations without weakening session isolation.
|
||||
Make cleanup the normal final stage and remove cleanup from runner
|
||||
finalization.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Add a small key parser proving an immutable object belongs to the expected
|
||||
campaign/session and extracting its run. Reject malformed, mutable,
|
||||
traversal, cross-session, and empty-run keys.
|
||||
2. Relax commit validation only for `published_output`: its key may use the
|
||||
current or an earlier run in the same session. Retain every other placement
|
||||
rule and the existing manifest version unless serialization changes.
|
||||
3. Update restore, previous-state caching, current-output lookup, and
|
||||
status/artifact inspection to open the exact declared key and match by stable
|
||||
source instead of reconstructing a current-run path. Never scan prefixes.
|
||||
4. Preserve duplicate-source and destination-collision enforcement when
|
||||
carried and current declarations coexist; derive visible origin from key.
|
||||
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
|
||||
|
||||
Test same/earlier-run acceptance, malformed/mutable/cross-session rejection,
|
||||
exact-key reads, and source-based inspection. Run
|
||||
`go test ./internal/artifacts ./internal/app ./internal/stage` and
|
||||
`git diff --check`. Only safe same-session published-output carry is enabled.
|
||||
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 — Separate Source And Output Planning
|
||||
## Stage 4 — Remove The Cleanup Obligation From Manifest State
|
||||
|
||||
**Status:** Completed
|
||||
**Status:** Pending
|
||||
|
||||
### Goal
|
||||
|
||||
Make source opening and output-rule resolution navigable without changing
|
||||
publish behavior.
|
||||
Delete the second cleanup state machine and leave stage status as the only
|
||||
durable cleanup progress model.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Move source discovery, metadata construction, safe opening, and validation
|
||||
from the aggregate publish file into one cohesive package-local owner.
|
||||
2. Move output selection, source matching, required/optional rules, destination
|
||||
resolution, and collisions into another cohesive owner.
|
||||
3. Keep `publishStage.Run` as caller/transaction owner with explicit inputs;
|
||||
avoid a mutable catch-all context. Do not integrate locks yet.
|
||||
4. Remove only dead helpers/imports exposed by the mechanical split.
|
||||
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/stage -run Publish`,
|
||||
`go test ./internal/app -run 'Publish|Artifact|Status'`, and
|
||||
`git diff --check`. Existing behavior remains, with clear source/output
|
||||
file-level ownership.
|
||||
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 — Separate Transfer And Commit Mechanics
|
||||
## Stage 5 — Consolidate, Validate, And Close Workstream 9
|
||||
|
||||
**Status:** Completed
|
||||
**Status:** Pending
|
||||
|
||||
### Goal
|
||||
|
||||
Finish decomposition while keeping the transaction visible in one place.
|
||||
Ensure the reduced implementation, tests, and canonical documentation agree,
|
||||
then record the workstream complete.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Move copying/hashing, immutable upload, remote verification, and reusable
|
||||
prior-object verification into a transfer owner.
|
||||
2. Move immutable commit and current-pointer construction, encoding, and writes
|
||||
into a commit owner.
|
||||
3. Keep one explicit ordered path in `publishStage.Run`: plan, transfer,
|
||||
immutable commit, pre-pointer checks, pointer write. Document its durability
|
||||
boundary briefly.
|
||||
4. Delete forwarding wrappers and dead helpers; do not add package-local
|
||||
interfaces without a second implementation.
|
||||
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
|
||||
|
||||
Preserve checksum, size, generation, conditional-write, cancellation, and
|
||||
partial-failure tests. Run `go test ./internal/stage -run Publish`,
|
||||
`go test ./internal/artifacts ./internal/adapters/storage`, and
|
||||
`git diff --check`. The transaction remains easy to audit from `Run`.
|
||||
|
||||
## Stage 6 — Model Locked Output Carry-Forward
|
||||
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
Represent every output outcome and build a coherent commit plan without I/O.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Add one private output-plan/result type using the five fixed statuses. A
|
||||
participating output is a configured rule selected for this invocation;
|
||||
unselected rules are reported but do not enforce required-output presence.
|
||||
2. Plan deterministically by source from selected rules, discovered sources,
|
||||
initial locks, and prior current commit.
|
||||
3. Publish unlocked participants normally. Carry a locked prior declaration
|
||||
byte-for-field. Emit `locked_unpublished` without required-output failure
|
||||
when none exists. Preserve optional-missing and unselected separately.
|
||||
4. Reject duplicate prior sources, invalid declarations, configured destination
|
||||
conflicts, and effective destination collisions. Never synthesize an old
|
||||
declaration.
|
||||
5. Project one source-sorted metadata collection with source/status,
|
||||
destination, exact declared key, and derived origin where applicable; omit
|
||||
secrets and local absolute paths.
|
||||
|
||||
### Verification And Exit Criteria
|
||||
|
||||
Pure table tests cover five states, exact carry, required locked absence, mixed
|
||||
snapshots, selection, ordering, and ambiguities. Run
|
||||
`go test ./internal/stage -run 'Publish|Output'` and `git diff --check`.
|
||||
One plan drives both declarations and metadata.
|
||||
|
||||
## Stage 7 — Integrate Locks Into Publish
|
||||
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
Make independent locks authoritative while preserving the snapshot transaction.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Read relevant locks at planning start and load/validate the prior current
|
||||
commit when a locked source may require carry-forward.
|
||||
2. Use Stage 6's plan. Do not open, hash, or upload a carried or never-published
|
||||
locked source.
|
||||
3. Before carrying, verify the exact prior object exists and matches declared
|
||||
size and generation without download; retain its declared checksum.
|
||||
4. Upload/verify new objects, combine exact carried/new declarations, validate
|
||||
and write the immutable commit, then reread initially unlocked sources. If
|
||||
any is now locked, return a clear error without pointer write; otherwise
|
||||
write the pointer exactly once.
|
||||
5. Remove `Env.RevalidatePublishLocks`, runner injection, and related plumbing
|
||||
once unused. Publish reads authoritative locks through artifact/storage.
|
||||
6. Replace overlapping category arrays with `publish_output_outcomes`.
|
||||
Preserve upload counts, `remote_commit_key`, and
|
||||
`current_commit_pointer_key` for current cleanup behavior.
|
||||
|
||||
### Verification And Exit Criteria
|
||||
|
||||
Test no locks, exact/mixed carry, required locked absence, invalid prior state,
|
||||
removed-after-initial lock, added-before-final lock, abandoned attempts, avoided
|
||||
locked-source work, and exactly one pointer write. Run
|
||||
`go test ./internal/stage -run Publish`,
|
||||
`go test ./internal/stage ./internal/artifacts ./internal/app`, `go test ./...`, and
|
||||
`git diff --check`. Newly observed final locks prevent pointer advancement.
|
||||
|
||||
## Stage 8 — Migrate Commands And Inspection
|
||||
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
Preserve the operator workflow over the independent-object model.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Rewrite lock list/add/remove services and CLI wiring to use artifact lock
|
||||
operations while preserving command names and session selection.
|
||||
2. List validated source-sorted locks. Add reports already locked unless
|
||||
`--force`; force performs one conditional replacement. Remove reports
|
||||
idempotent absence without error. Remove help/output references to
|
||||
precedence and shared manifests.
|
||||
3. Reduce command composition to storage, session prefix, source, reason,
|
||||
clock where needed, and output. Remove config mutation and callbacks.
|
||||
4. Make status/current-artifact inspection show five outcomes and identify
|
||||
carried sources by exact key and derived run. Fetch only declared objects.
|
||||
5. Add command tests for exit behavior, conflicts, idempotent remove,
|
||||
deterministic output, malformed locks, and carried inspection.
|
||||
|
||||
### Verification And Exit Criteria
|
||||
|
||||
Run `go test ./internal/app ./internal/artifacts`,
|
||||
`go test ./cmd/...`, and `git diff --check`. Operators retain simple,
|
||||
accurate lock and inspection commands.
|
||||
|
||||
## Stage 9 — Remove Retired Lock Authorities
|
||||
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
Delete the obsolete control plane after all consumers migrate.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Remove `pipeline.publish.locks`, `PublishLockRule`, central store types,
|
||||
`locks.yml` helpers, YAML I/O, merge/precedence logic, document replacement
|
||||
retries, injection fields, compatibility adapters, and dead tests/fixtures.
|
||||
2. Do not retain migration readers. Remaining historical mentions must be
|
||||
clearly intentional prose.
|
||||
3. If effective-config digest inputs change, advance its internal discriminator
|
||||
so prior successful state invalidates deliberately. Change no user-facing
|
||||
schema version unless repository policy requires it.
|
||||
4. Add a representative strict-decode test proving the old config field is
|
||||
unknown, and search code/docs/examples/fixtures for all retired identifiers.
|
||||
|
||||
### Verification And Exit Criteria
|
||||
|
||||
Run:
|
||||
At minimum run:
|
||||
|
||||
```sh
|
||||
go test ./internal/config ./internal/artifacts ./internal/app ./internal/stage
|
||||
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
|
||||
```
|
||||
|
||||
Independent objects are the sole lock authority.
|
||||
|
||||
## Stage 10 — Align Documentation And Close The Workstream
|
||||
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
Make the simplified implementation the fully documented repository state.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Update canonical config, storage, stage, CLI, operations, and development
|
||||
docs for the payload/key, bounded validation, commands, carry behavior, race
|
||||
boundary, five outcomes, cross-run rule, and transaction sequence. Remove
|
||||
static-lock and `locks.yml` claims.
|
||||
2. Document that old `locks.yml` is ignored and may be manually removed after
|
||||
upgrade verification; add no migration tool.
|
||||
3. Remove stale comments, redundant wrappers/logic, dead fixtures, and obsolete
|
||||
metadata assertions exposed by final review.
|
||||
4. Run all formatting, unit/integration, static-analysis, documentation, and
|
||||
policy checks required by `docs/development.md`. Record genuinely
|
||||
platform-only checks without weakening or silently skipping them.
|
||||
5. Only after checks pass, mark this plan's stages and Workstream 8 completed.
|
||||
Leave Workstream 9 as the next pending scope.
|
||||
|
||||
### Verification And Exit Criteria
|
||||
|
||||
At minimum run `gofmt` on changed Go, `go test ./...`, `go vet ./...`,
|
||||
`go test ./internal/doccheck`, `git diff --check`, and all repository
|
||||
scripts mandated by policy. Code, tests, docs, help, and examples agree, and no
|
||||
Workstream 9 implementation is included.
|
||||
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 audit decisions and fixed choices above make this plan
|
||||
decision-complete.
|
||||
None. The approved audits, refactoring roadmap, and fixed decisions above make
|
||||
this implementation plan decision-complete.
|
||||
|
||||
Reference in New Issue
Block a user