Plan runner lifecycle refactor
This commit is contained in:
@@ -1,396 +1,481 @@
|
||||
# Workstream 4 Implementation Plan
|
||||
# Runner Lifecycle Refactoring Implementation Plan
|
||||
|
||||
## Purpose And Status
|
||||
## Status And Purpose
|
||||
|
||||
This is the executable plan for [`refactoring.md`](refactoring.md), Workstream
|
||||
4: **One durable local manifest authority**. Finding 2 of
|
||||
[`contract-audit.md`](contract-audit.md) and rows H14, H15, H17, C02, and C03
|
||||
of [`audit-decisions.md`](audit-decisions.md) own the approved decisions.
|
||||
This is the decision-complete implementation plan for Workstream 5, **Runner
|
||||
lifecycle and pre-execution decisions**, in
|
||||
[`refactoring.md`](refactoring.md). Workstreams 1 through 4 are complete.
|
||||
Workstream 5 is the next pending workstream and is the only feature scope of
|
||||
this plan.
|
||||
|
||||
**Status: Completed**
|
||||
The implementation must reduce the change blast and cognitive load of
|
||||
`internal/app.executePlan` without changing the pipeline's observable behavior.
|
||||
It must leave one explicit lifecycle owner, one session lock, one durable
|
||||
session-manifest authority, a narrow shared planning/execution decision model,
|
||||
and invocation-scoped test seams. It must not create a workflow engine, a
|
||||
general dependency-injection framework, or new public extension points.
|
||||
|
||||
All five stages are completed. Workstreams 1 through 4 are complete.
|
||||
Workstreams 5 and later remain out of scope: do not decompose the runner into a
|
||||
new lifecycle type, redesign pre-execution decisions, add per-artifact analyze
|
||||
checkpoints, split publish into separate transactions, add cleanup as a stage,
|
||||
or change remote lock policy while following this plan.
|
||||
The authoritative scope and target state are Workstream 5, the dependency
|
||||
relationships, and the consolidated target state in
|
||||
[`refactoring.md`](refactoring.md). The supporting audit decisions are H04,
|
||||
H07, H14, H15, H17, H34, and H35 in
|
||||
[`audit-decisions.md`](audit-decisions.md), with detailed analysis in
|
||||
[`audit.md`](audit.md).
|
||||
|
||||
## Settled Implementation Decisions
|
||||
## Scope And Fixed Decisions
|
||||
|
||||
- The session manifest is the only durable local progress ledger. It alone
|
||||
records stage status, current artifacts, semantic evidence, diagnostics,
|
||||
failures, resume authority, restore authority, and publish authority.
|
||||
- Delete the structured invocation run manifest completely: its schema and
|
||||
types, store interface and methods, path helper, lifecycle mutations,
|
||||
terminal recovery, analyze projection, publish dependency, fixtures, tests,
|
||||
and current documentation all leave the repository.
|
||||
- Do not replace it with another JSON summary, journal, database, or event log.
|
||||
Historical per-invocation state is not an operational requirement.
|
||||
- Preserve run IDs. Every invocation still receives one before session state is
|
||||
initialized; it remains the identity used by the session manifest,
|
||||
run-local directory, adapter provenance, logs, generated configuration,
|
||||
receipts, immutable publish prefix, command summary, and diagnostics.
|
||||
- Explicitly create the canonical run directory once per invocation after the
|
||||
session lock and identity checks. Removing the initial run-manifest save must
|
||||
not make diagnostic writers responsible for accidentally creating the run
|
||||
root. Directory-creation failure remains an invocation initialization error.
|
||||
- `RunSummary` remains the immediate in-memory command result and retains
|
||||
session ID, run ID, session manifest path, selected stage names, executed
|
||||
stages, and skipped stages. Remove only `RunManifestPath`; do not add a
|
||||
replacement persistent summary.
|
||||
- A run-manifest write can no longer fail an otherwise valid pipeline action.
|
||||
Session-manifest transition and terminal-save failures remain correctness
|
||||
failures and continue to join with the originating error where applicable.
|
||||
- Publish no longer opens or validates `runs/<run_id>/manifest.json`. When
|
||||
`upload_run` is enabled, it derives the current run root canonically and uses
|
||||
the session manifest's recorded output, log, and generated-configuration
|
||||
references as an allowlist. It includes only safe regular files confined to
|
||||
the current run root and ignores records from earlier runs.
|
||||
- New remote commits do not upload or classify a run manifest. Keep
|
||||
`RemoteArtifactTypeRunManifest` as a read-only wire value so already committed
|
||||
and verified remote snapshots remain readable; it must have no local loader,
|
||||
path, special publish branch, or newly emitted declaration. This preserves a
|
||||
durable remote protocol without retaining the local dual-authority model.
|
||||
- Preserve `RemoteArtifactTypeRunFile` for declared run-local diagnostics.
|
||||
Publishing no current-run diagnostics is valid and must not require a
|
||||
placeholder file.
|
||||
- Remove the invocation subset from `AnalyzeStateProjection` and from analyze
|
||||
execution. During this workstream, analyze still returns and persists one
|
||||
validated reconciled session collection at the existing end-of-stage
|
||||
boundary. Do not implement artifact-by-artifact persistence yet.
|
||||
- Preserve the current safe behavior when the end-of-stage analyze manifest
|
||||
save fails: incidental output bytes do not establish authority, prior
|
||||
in-memory analyze authority is restored, and the aggregate stage fails.
|
||||
Workstream 6 will replace this with the approved checkpoint model.
|
||||
- Preserve current stage ordering, force/invalidation rules, self-skip behavior,
|
||||
aggregate transition timing, cleanup obligations, remote commit ordering,
|
||||
and command output except where they directly expose the removed run
|
||||
manifest.
|
||||
- Delete tests whose only contract is historical run-manifest persistence.
|
||||
Rewrite mixed tests around session state, command summaries, run-local
|
||||
diagnostics, or remote commit contents, and consolidate redundant mechanism
|
||||
assertions rather than recreating a second audit record in tests.
|
||||
- Historical release notes and completed audit documents remain unchanged.
|
||||
Current architecture, CLI, operations, troubleshooting, and internal
|
||||
documentation must describe session-only authority.
|
||||
- Mark only Workstream 4 `Complete`, and only after all stages and final
|
||||
validation succeed.
|
||||
- Keep the existing `internal/app` package boundary. Use private types,
|
||||
functions, and focused files; do not create a runner package or expose
|
||||
lifecycle internals.
|
||||
- Keep the session manifest as the sole durable local progress ledger. Run IDs,
|
||||
run directories, diagnostics, and `RunSummary` remain, but no second durable
|
||||
invocation record may be introduced.
|
||||
- Preserve the canonical flat stage order, bounded contiguous execution,
|
||||
dependency-aware invalidation outside the selected range, stage-owned resume
|
||||
validation, self-skip semantics, force semantics, and current analyze
|
||||
projection behavior.
|
||||
- Preserve the two prerequisite checks: a read-only check before persistent
|
||||
mutation and a second check against the manifest while holding the session
|
||||
lock.
|
||||
- Keep lock acquisition and the deferred release visibly paired in the
|
||||
top-level lifecycle function. A release failure remains the final invocation
|
||||
error when no earlier error exists and is joined with an earlier error
|
||||
otherwise.
|
||||
- Preserve manifest ordering: establish identity before adapter construction;
|
||||
persist `running` before invoking a stage; persist terminal stage state before
|
||||
moving to the next stage; and persist terminal invocation failures where the
|
||||
current runner does so. Do not make diagnostics a persistence authority.
|
||||
- Keep actual `Stage.Run` results, result validation, persistence, logging, and
|
||||
failure handling execution-only. Planning may model a successful or
|
||||
configured self-skipped outcome on a cloned manifest, but must never pretend
|
||||
that it observed a runtime result.
|
||||
- The shared pure decision value must describe only whether to run or skip and
|
||||
the pre-execution stale/invalidation work implied by recorded status, force,
|
||||
and normalized resume validation. Small transition helpers may be shared
|
||||
only where planning and execution perform the same manifest mutation.
|
||||
- Preserve analyze's current single end-of-stage projection/save boundary and
|
||||
rollback on save failure. Per-artifact checkpointing belongs to Workstream 6
|
||||
and is out of scope.
|
||||
- Preserve post-publish cleanup and its durable obligation behavior until
|
||||
Workstream 9. This work may move the call into finalization and make its
|
||||
destructive function invocation-scoped, but must not turn cleanup into a
|
||||
stage or change eligibility/retry semantics.
|
||||
- Continue using `stage.Env` for injected stage collaborators. Add only small
|
||||
private dependency values for nondeterministic or external construction
|
||||
boundaries that cannot be represented by `stage.Env`, and pass those values
|
||||
explicitly per command or invocation.
|
||||
- Remove `executeStagesFn` and `runCommandFn`. Replace their tests with private
|
||||
command helpers that accept a pipeline executor value for that call. Do not
|
||||
migrate unrelated restore/config-loader globals in this workstream; their
|
||||
owners should adopt the same narrow pattern when later work touches them.
|
||||
- Replace the runner's mutable clock use with an invocation-scoped clock. A
|
||||
normal immutable `nowUTC()` function may remain for unrelated command owners.
|
||||
Route post-publish deletion through the invocation dependency value and
|
||||
remove `removeRunScopedDirFn`.
|
||||
- Preserve current CLI syntax, output, errors at their stable semantic level,
|
||||
configuration behavior, workspace modes, secret loading/redaction, remote
|
||||
lock revalidation, Linux/macOS/Windows support, and offline operation.
|
||||
- Do not optimize for a particular private file layout or exact helper count.
|
||||
The finished top-level runner must nonetheless read as a short, explicit
|
||||
sequence of preparation, lock ownership, locked initialization, composition,
|
||||
stage execution, and finalization rather than forwarding all work to another
|
||||
monolith.
|
||||
- Mark only Workstream 5 `Complete`, and only after all stages and full
|
||||
validation succeed. Workstream 6 remains the next pending workstream.
|
||||
|
||||
## Instructions For Every Stage
|
||||
|
||||
Before each stage:
|
||||
|
||||
1. Read `docs/development.md`, every file under `docs/policy/`, this plan,
|
||||
Workstream 4 plus the dependency and target sections of
|
||||
[`refactoring.md`](refactoring.md), Finding 2 and the Workstream-4-relevant
|
||||
boundary in Finding 3 of [`contract-audit.md`](contract-audit.md), and rows
|
||||
H14, H15, H17, C02, and C03 of
|
||||
[`audit-decisions.md`](audit-decisions.md).
|
||||
2. Inspect the named implementation and focused tests. Prefer the codebase
|
||||
graph for Go discovery and text search for schemas, paths, fixtures,
|
||||
documentation, and serialized field names.
|
||||
3. Confirm the worktree state and preserve unrelated changes. Do not enter a
|
||||
later workstream merely because adjacent lifecycle, analyze, publish, or
|
||||
cleanup code would benefit from broader refactoring.
|
||||
1. Read `docs/development.md`, every document under `docs/policy/`, this plan,
|
||||
Workstream 5 plus the dependency and consolidated-target sections of
|
||||
[`refactoring.md`](refactoring.md), and audit rows H04, H07, H14, H15, H17,
|
||||
H34, and H35 in [`audit-decisions.md`](audit-decisions.md). Consult the
|
||||
corresponding detailed findings in [`audit.md`](audit.md) when a boundary is
|
||||
unclear.
|
||||
2. Inspect the named production code and focused tests. Prefer the codebase
|
||||
knowledge graph for Go symbol discovery and call tracing; use text search for
|
||||
documentation, fixtures, serialized fields, and global-variable assignments.
|
||||
3. Confirm the worktree state and preserve unrelated changes. Do not enter
|
||||
Workstream 6 or Workstream 9 merely because analyze or cleanup code is
|
||||
adjacent to runner lifecycle code.
|
||||
|
||||
During every stage:
|
||||
|
||||
- Keep the repository compiling and focused tests passing. A temporary
|
||||
invocation projection or unused run-manifest domain type may remain only
|
||||
when the next numbered stage explicitly removes it; do not introduce a new
|
||||
compatibility facade.
|
||||
- Preserve package ownership and acyclic dependency direction. Keep this a
|
||||
deletion-oriented refactor inside the existing `app`, `stage`, `manifest`,
|
||||
and `artifacts` owners.
|
||||
- Preserve the session lock, session-manifest atomic replacement, group
|
||||
workspace modes, bounded I/O, no-follow publish sources, remote verification,
|
||||
secret redaction, and Linux/macOS/Windows support.
|
||||
- Update the canonical current-behavior document owned by a change in the same
|
||||
stage. Do not edit historical release notes or completed audit findings.
|
||||
- Keep tests offline, deterministic, and behavior-focused. Remove tests with
|
||||
the retired contract and retain consequential session-state, data-integrity,
|
||||
recovery, and remote-commit coverage at the narrowest stable boundary.
|
||||
- Run `gofmt` on changed Go files, focused tests, and `git diff --check`. Mark a
|
||||
stage `Completed` only after its exit criteria pass.
|
||||
- Keep the repository compiling and focused tests passing. Do not leave both an
|
||||
old and new runner path active except for a temporary private delegation that
|
||||
the next numbered stage explicitly removes.
|
||||
- Preserve package ownership and acyclic dependency direction. Private state
|
||||
should aggregate already-related values, not become a service locator.
|
||||
- Preserve error wrapping and joining where errors represent distinct failures.
|
||||
Do not freeze incidental complete error strings in new tests.
|
||||
- Keep tests deterministic, offline, behavior-focused, and lean under
|
||||
`docs/policy/testing.md`. Prefer existing real temporary filesystems and
|
||||
current adapter/storage fakes. Do not add mock choreography for private
|
||||
lifecycle helpers.
|
||||
- Update the canonical current-behavior document owned by a changed behavior in
|
||||
the same stage. Pure file movement or private extraction requires no user
|
||||
documentation change.
|
||||
- Run `gofmt` on changed Go files, focused tests for the changed boundary, and
|
||||
`git diff --check`. Mark a stage `Completed` only after its exit criteria pass.
|
||||
|
||||
## Stage 1 — Decouple Publish From The Run Manifest
|
||||
## Stage 1 — Unify Pre-Execution Decisions
|
||||
|
||||
**Status: Completed**
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Make publish consume only the session manifest and current run directory so the
|
||||
runner can stop producing a structured run manifest without losing useful
|
||||
diagnostic uploads.
|
||||
Give planning and execution one pure representation of the decision made before
|
||||
a stage attempt, while keeping planning predictions distinct from observed
|
||||
runtime outcomes.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. In `internal/stage/publish.go`, replace
|
||||
`resolvePublishRunManifestPath` with canonical current-run-root resolution
|
||||
based on resolved campaign, session ID, workspace root, and `m.RunID` using
|
||||
`artifacts.SessionRunRootForCampaign`. Require that exact root to be a real
|
||||
directory and retain existing symlink/non-directory rejection.
|
||||
2. Remove opening, decoding, rewinding, and passing a run-manifest file. Change
|
||||
current-run collection to inspect `manifest.Manifest` stage records and add
|
||||
only declared `ArchivePath`, fallback `LocalPath`, `Logs`, and
|
||||
`GeneratedConfigs` that resolve within the selected current run root.
|
||||
Records referring to canonical session files or an older run must not be
|
||||
uploaded as current-run diagnostics.
|
||||
3. Make an empty current-run diagnostic set valid. Refactor the source-opening
|
||||
helper so it opens each allowlisted current-run file directly, with the
|
||||
existing regular-file/no-follow and optional checksum verification. It must
|
||||
not require a distinguished manifest file.
|
||||
4. Remove the `manifest.json` synthetic run-file entry and the special branch
|
||||
that emits `RemoteArtifactTypeRunManifest`. Continue emitting ordinary
|
||||
current-run diagnostics as `RemoteArtifactTypeRunFile` with deterministic
|
||||
`run:<relative-path>` source identities and destinations.
|
||||
5. Preserve session-manifest commit generation, previous-cache uploads,
|
||||
configured published outputs, immutable upload verification, lock
|
||||
revalidation, commit upload, and pointer-last ordering unchanged.
|
||||
6. Rewrite focused publish tests and helpers to create a run root and populate
|
||||
session-manifest diagnostic references instead of writing a run manifest.
|
||||
Cover current-run logs/generated configuration/declared archive outputs,
|
||||
exclusion of prior-run and session-canonical paths, empty diagnostics,
|
||||
symlink rejection, deterministic ordering, and absence of newly emitted
|
||||
`run_manifest` declarations. Retain existing upload integrity and pointer
|
||||
tests without duplicating them.
|
||||
7. Update `docs/internal/stage-publish.md` and the run-upload portion of
|
||||
`docs/operations.md`: the session manifest is the diagnostic allowlist, no
|
||||
run manifest is uploaded, and `upload_run` may legitimately upload zero
|
||||
run-local diagnostic files.
|
||||
1. In `internal/app/run_control.go` or a focused private file, replace the
|
||||
current action-only decision with an unexported value that records:
|
||||
`run` versus `skip`; whether the selected successful record must first be
|
||||
marked stale; the stage-owned reason for that stale transition; and the
|
||||
application-owned invalidation reason, if any. Its constructor must be pure:
|
||||
it receives the prior manifest status, `force`, and an already normalized
|
||||
resume-validation result and does not mutate state, persist, log, read files,
|
||||
invoke a stage, or obtain the current time.
|
||||
2. Encode these cases explicitly:
|
||||
- a non-forced succeeded record with no validator result or a resumable
|
||||
result skips;
|
||||
- a non-forced succeeded record with a non-resumable result runs after the
|
||||
selected record is staled with the validator's reason and succeeded
|
||||
dependents are invalidated with `staleReasonNotResumable`;
|
||||
- a forced stage runs and invalidates succeeded dependents with
|
||||
`staleReasonForcedReplacement` when the attempt begins; and
|
||||
- absent, failed, stale, or skipped aggregate records run without inventing a
|
||||
resume-stale transition.
|
||||
3. Add a small helper that applies only the identical pre-execution manifest
|
||||
changes described by the decision at a caller-supplied timestamp. Retain the
|
||||
existing fixed invalidation relation and stable application-owned reasons.
|
||||
The caller remains responsible for persistence and error context.
|
||||
4. Use the decision constructor and application helper from both `Plan` and
|
||||
`executePlan`. Each caller may perform stage-owned resume validation before
|
||||
constructing the decision. Preserve analyze planning's read-only artifact
|
||||
details when analyze is already a run candidate; those details are preview
|
||||
information, not an execution outcome.
|
||||
5. Extract only the exact shared attempt/result mutations already identical in
|
||||
both paths: beginning an attempt (`running` plus forced invalidation),
|
||||
applying a configured/runtime self-skip and its conditional invalidation,
|
||||
and applying a predicted/actual success and its conditional invalidation.
|
||||
Require callers to supply the actual or modeled outcome, outputs, semantic
|
||||
fingerprint, prior outcome, and timestamp. Do not put `Stage.Run`, result
|
||||
validation, logging, or manifest saves in these helpers.
|
||||
6. Rewrite `modelPlannedStageRun` around these values or delete it if the
|
||||
resulting focused helpers make it redundant. Keep all planning mutation on
|
||||
the cloned manifest.
|
||||
7. Add table-driven decision tests for every status/force/resume combination
|
||||
and focused parity tests proving that plan and execution produce the same
|
||||
run/skip choice and dependent-stale effects for ordinary skip, non-resumable
|
||||
success, forced replacement, unchanged self-skip, changed self-skip, and a
|
||||
successful result replacing a non-success record. Assert durable state and
|
||||
decision semantics, not private helper call order or exact timestamps.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/stage ./internal/artifacts` succeeds.
|
||||
- Publish succeeds when the current run directory exists but contains no run
|
||||
manifest and no diagnostics.
|
||||
- Only session-manifest-declared files confined to the current run root become
|
||||
`run_file` transfers; older-run and incidental files do not.
|
||||
- New commits contain no `run_manifest` artifact, while remote decoding still
|
||||
accepts that historical wire value.
|
||||
- `go test ./internal/app` succeeds.
|
||||
- `Plan` and `executePlan` no longer independently encode the same
|
||||
pre-execution stale/force decision sequence.
|
||||
- The decision constructor is pure, and actual stage outcomes and persistence
|
||||
remain outside it.
|
||||
- Planning remains read-only and execution behavior is unchanged.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 2 — Make Runner Persistence Session-Only
|
||||
## Stage 2 — Establish The Invocation Runtime And Lock Lifecycle
|
||||
|
||||
**Status: Completed**
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Remove the second persistence transaction from invocation execution while
|
||||
preserving the existing session-ledger lifecycle and immediate command result.
|
||||
Make invocation state cohesive and make the top-level transaction order obvious
|
||||
without hiding session-lock ownership.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Remove `RunOptions.RunManifestStore` and `RunSummary.RunManifestPath`. Keep
|
||||
all remaining summary fields and run ID generation. Update direct callers
|
||||
and CLI rendering only if they refer to the removed path.
|
||||
2. In `executePlan`, delete run-manifest path resolution, construction,
|
||||
identity/config projection, initial save, per-stage action/running/skip/
|
||||
failure/success saves, final success save, and every error path created only
|
||||
by those operations.
|
||||
3. After acquiring the session lock, validating/restoring session state, and
|
||||
resolving invocation identity, explicitly ensure the canonical
|
||||
`SessionRunRootForCampaign` directory with the ordinary workspace directory
|
||||
contract. Do not create `manifest.json` or another placeholder there.
|
||||
4. Replace `persistTerminalFailure` with a narrowly named session-only helper.
|
||||
It must record the terminal failure in the session manifest when available,
|
||||
atomically attempt the session save, preserve the originating error, and
|
||||
join a session-save error without invoking any diagnostic store. Keep the
|
||||
distinct post-publish-cleanup failure behavior until its later workstream.
|
||||
5. Simplify all runner failure paths to the new helper. Preserve the existing
|
||||
order of session identity save, resume validation, running transition,
|
||||
force invalidation, stage execution, success/failure/self-skip transition,
|
||||
session save, logging, and post-publish cleanup.
|
||||
6. Make `applyAnalyzeProjection` session-only at its application boundary so
|
||||
the runner no longer needs a run-manifest value. The invocation subset may
|
||||
remain temporarily inside validation until Stage 3 deletes it.
|
||||
7. Delete run-only helpers such as `invocationIdentity.applyToRunManifest`,
|
||||
`requestedStageNames`, `applyStageResultToRunManifest`,
|
||||
`applyEffectiveConfigProvenanceToRun`, and
|
||||
`setRunStageSemanticConfig` once unused.
|
||||
8. Rewrite runner, terminal-failure, semantic-resume, assembled-workflow, and
|
||||
lifecycle tests that currently inject a `RunStore` or load
|
||||
`RunSummary.RunManifestPath`. Assert the durable outcome through the session
|
||||
manifest and the immediate invocation outcome through `RunSummary`.
|
||||
Preserve coverage for cancellation, collaborator/preflight failures,
|
||||
session-save failure joining, successful and failed stage transitions,
|
||||
self-skip, force invalidation, semantic evidence, distinct run IDs, and
|
||||
run-local diagnostic paths. Delete tests whose sole purpose was run-record
|
||||
creation, terminalization, or historical inspection.
|
||||
9. Update `docs/policy/architecture.md`, `docs/cli.md`, and the runner portions
|
||||
of `docs/internal/manifest.md`: one atomic session-manifest update is the
|
||||
only durable progress transaction; a run ID and run directory remain, but
|
||||
there is no invocation manifest or persistent command summary.
|
||||
1. Introduce an unexported `invocationRuntime` (or equivalently narrow private
|
||||
type) under `internal/app`. It should carry only state already shared across
|
||||
lifecycle phases: context, resolved configuration, bounded plan/stages,
|
||||
normalized options, effective artifacts, run identity and canonical paths,
|
||||
`stage.Env`, manifest store, artifact store, loaded session manifest, and
|
||||
summary accumulators. Do not expose it or let stages depend on it.
|
||||
2. Extract unlocked preparation that performs the current read-only bounded
|
||||
prerequisite inspection, resolves effective artifacts, generates the run
|
||||
ID, resolves invocation identity, initializes/clones `stage.Env` fields, and
|
||||
supplies default manifest/artifact stores and logger. Preserve the rule that
|
||||
injected `Env` values provide collaborators but never an alternate config,
|
||||
selected-artifact set, or effective artifact set.
|
||||
3. Extract layout preparation separately. Keep `EnsureLayoutFor` before lock
|
||||
acquisition as today, verify its manifest path against the canonical
|
||||
resolved path, and return enough state for the locked lifecycle.
|
||||
4. In the short top-level executor, acquire the session lock after unlocked
|
||||
preparation and visibly install one deferred release beside that acquisition.
|
||||
Preserve exact release-error joining behavior. Do not put lock ownership in
|
||||
`invocationRuntime`, a constructor, or a helper whose lifetime is ambiguous.
|
||||
5. Extract locked initialization that checks the incomplete-restore marker,
|
||||
loads and validates or creates the session manifest, rechecks bounded
|
||||
prerequisites, applies identity and effective-configuration provenance,
|
||||
saves the identity transition, ensures the canonical run directory, and
|
||||
loads configured secrets. All steps after manifest availability that
|
||||
currently record a terminal failure must continue to do so through the
|
||||
session-only failure helper.
|
||||
6. Keep pre-lock failures free of persistent mutation and keep all later
|
||||
manifest transitions protected by the same held lock. Do not add rollback,
|
||||
a second lock, or a second manifest authority.
|
||||
7. Adapt existing bounded-run, restore-marker, identity, run-directory,
|
||||
terminal-failure, and lock-release tests to exercise the assembled lifecycle.
|
||||
Add coverage only if the two-check prerequisite ordering or lock-release
|
||||
error behavior is not already credibly protected.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/app ./internal/artifacts ./internal/manifest` succeeds.
|
||||
- The top-level executor visibly performs unlocked preparation, acquires and
|
||||
defers release of exactly one session lock, initializes locked state, and
|
||||
proceeds to execution.
|
||||
- Invalid preflight requests create no persistent invocation state; the locked
|
||||
prerequisite recheck still prevents races with a prior invocation.
|
||||
- The private runtime has cohesive fields and no public or cross-package API.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 3 — Isolate Collaborator Composition
|
||||
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Make production collaborator construction one bounded lifecycle phase that is
|
||||
selective, testable per invocation, and independent of stage transitions.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Define a small unexported invocation dependency value with explicit defaults
|
||||
for the nondeterministic and external construction boundaries used by this
|
||||
runner: UTC time, run-ID generation, default adapter/object-store creation,
|
||||
effective remote-lock loading/revalidation support, and post-publish scoped
|
||||
removal. Include a field only where tests need control over a real external,
|
||||
destructive, clock, or randomness boundary; continue injecting completed
|
||||
stage collaborators through `stage.Env`.
|
||||
2. Add a private executor variant that accepts that dependency value and have
|
||||
the ordinary executor supply production defaults. Normalize missing fields
|
||||
once at the entry boundary so lifecycle methods never fall back to mutable
|
||||
package state. Do not export the dependency type or store it globally.
|
||||
3. Route run-ID creation and every runner timestamp through the invocation
|
||||
dependencies. Convert `nowUTC` to an ordinary function for unrelated
|
||||
command owners if it no longer needs to be mutable; do not broaden this stage
|
||||
into a restore lifecycle refactor.
|
||||
4. Extract collaborator composition from the runner into one operation over the
|
||||
invocation runtime. Preserve current selection gates exactly: WhisperX only
|
||||
for `transcribe`; Seriatim only for `merge`, `normalize`, `trim`, or `render`;
|
||||
Audita only for `polish`; Notarius only when selected extraction requires it;
|
||||
Scriptorium only for `trim` or `analyze`; object storage only when selected
|
||||
stages/effective artifacts require it; remote lock loading only when needed;
|
||||
and the no-op notifier only for `notify`.
|
||||
5. Preserve injected collaborators, secret-loaded environment behavior,
|
||||
effective lock merging, and the publish-time revalidation closure. A
|
||||
composition failure after identity initialization remains a persisted
|
||||
terminal session failure with its existing useful context.
|
||||
6. Route post-publish cleanup's scoped deletion through the invocation
|
||||
dependency value and delete `removeRunScopedDirFn`. Preserve cleanup
|
||||
obligation creation, per-target persistence, retry, and failure behavior.
|
||||
7. Consolidate selective-composition tests into table-driven or focused
|
||||
behavioral cases. Test that an unselected external boundary is not
|
||||
constructed only where avoiding that construction is an operational
|
||||
requirement; do not assert private helper ordering.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/app` succeeds, including composition, remote-lock,
|
||||
cleanup, and bounded-stage tests.
|
||||
- Collaborator construction is absent from the top-level runner and stage loop,
|
||||
but remains an application-owned production-composition concern.
|
||||
- Runner time, randomness, external construction, and destructive cleanup are
|
||||
controllable per invocation without mutable runner globals.
|
||||
- Selected-stage composition and terminal-failure behavior remain unchanged.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 4 — Extract One-Stage Execution
|
||||
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Give one private operation complete ownership of a single stage attempt while
|
||||
preserving every durable transition and failure boundary.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Extract an operation on the invocation runtime that executes exactly one
|
||||
selected stage and returns a small execution-owned outcome sufficient to
|
||||
update the in-memory summary or stop the invocation. It may coordinate the
|
||||
shared decision/transition helpers but must not select the next stage or
|
||||
release the session lock.
|
||||
2. Within that operation, preserve this order:
|
||||
- compute the stage semantic fingerprint;
|
||||
- run stage-owned resume validation when the recorded success is a skip
|
||||
candidate;
|
||||
- build and apply the pure pre-execution decision and persist any
|
||||
non-resumable stale/invalidation transition;
|
||||
- return a skip outcome without invoking the stage when still resumable;
|
||||
- capture prior aggregate/analyze state, begin the attempt, persist
|
||||
`running`, and then invoke `Stage.Run`;
|
||||
- validate the stage result and any analyze projection;
|
||||
- apply failure, self-skip, or success transitions and required dependent
|
||||
invalidation; and
|
||||
- persist the terminal state before returning.
|
||||
3. Preserve failure semantics for semantic fingerprinting, resume validation,
|
||||
invalidation, the pre-run save, stage execution, result validation,
|
||||
self-skip/success saves, and terminal failure persistence. A failed stage or
|
||||
persistence operation stops later stages. Distinct invalidation/save errors
|
||||
remain joined with the originating failure.
|
||||
4. Preserve current analyze behavior exactly: a valid partial failure
|
||||
projection may be applied before recording failure; a successful projection
|
||||
supplies aggregate outputs; and failure to save a successful projection
|
||||
restores prior analyze authority before recording aggregate failure.
|
||||
Introduce no callback/store dependency into `internal/stage`.
|
||||
5. Keep manifest save calls explicit in the one-stage owner. Do not hide them in
|
||||
generic state-machine hooks or teach `manifest.Store` orchestration policy.
|
||||
6. Reduce the outer stage loop to call the one-stage operation, append its
|
||||
executed/skipped classification, and stop on error. Preserve deterministic
|
||||
`StageNames`, `Executed`, and `Skipped` order and current logging semantics.
|
||||
7. Reconcile runner tests around observable manifest state, invocation summary,
|
||||
stage calls, and errors. Preserve coverage for ordinary skip, force,
|
||||
non-resumable state, self-skip, result validation, cancellation, downstream
|
||||
invalidation outside a bounded range, save failures, analyze rollback, and
|
||||
stopping after failure. Remove redundant tests that exist only to pin the old
|
||||
monolithic control flow.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/app ./internal/stage ./internal/manifest` succeeds.
|
||||
- `executePlan` creates no `runs/<run_id>/manifest.json` and exposes no run
|
||||
manifest store seam, while run IDs, run directories, diagnostics, session
|
||||
transitions, and summaries remain available.
|
||||
- A hypothetical diagnostic-record write cannot fail a stage because no such
|
||||
persistence operation exists; session-manifest failures remain visible and
|
||||
authoritative.
|
||||
- Runner behavior tests obtain durable state only from the session manifest.
|
||||
- The stage loop contains no duplicated transition policy and one-stage
|
||||
execution has one clear persistence owner.
|
||||
- A later stage is never attempted after a stage or required manifest save
|
||||
fails, and no failed/interrupted attempt is presented as successful.
|
||||
- Analyze still uses its pre-Workstream-6 aggregate persistence boundary.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 3 — Remove Invocation-Only Analyze Projection
|
||||
## Stage 5 — Isolate Finalization And Simplify The Lifecycle Owner
|
||||
|
||||
**Status: Completed**
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Give analyze one in-memory projection—the reconciled session collection—without
|
||||
prematurely implementing Workstream 6 checkpoint semantics.
|
||||
Complete the runner decomposition so the lifecycle and transaction boundaries
|
||||
are easier to read than the former monolith.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Remove `Invocation` from `stage.AnalyzeStateProjection` and update its
|
||||
comment to describe only the reconciled session authority proposed for the
|
||||
next session-manifest save. Retain the pointer wrapper so “no analyze update”
|
||||
remains distinguishable from a valid empty collection.
|
||||
2. In `internal/stage/analyze.go`, delete the separate invocation-record map and
|
||||
every write, clone, or failure projection associated only with it. Successful
|
||||
and failed analyze results continue to return the complete reconciled
|
||||
session collection under the existing rules.
|
||||
3. Simplify `internal/app/analyze_projection.go`: validate and clone only the
|
||||
session collection, remove invocation/session subset comparison, apply only
|
||||
the session collection, and derive aggregate analyze outputs only from that
|
||||
collection. Retain rejection of projections from non-analyze stages,
|
||||
contradictory ordinary outputs, malformed records, and illegal skipped or
|
||||
error result shapes.
|
||||
4. Preserve `captureAnalyzeState` and `restoreAnalyzeState` for the current
|
||||
single-save boundary. Do not add manifest-store access to the analyze stage,
|
||||
persist inside its artifact loop, or change retry/force behavior in this
|
||||
stage.
|
||||
5. Rewrite `analyze_projection_test.go`, analyze incremental tests, and mixed
|
||||
runner tests around one reconciled session collection. Delete assertions
|
||||
about per-invocation artifact subsets. Retain behavior coverage for partial
|
||||
failure state, unrelated current records, invalid projections, persistence
|
||||
failure rollback, and aggregate output derivation.
|
||||
6. Update `docs/internal/stage-analyze.md` and the analyze-state section of
|
||||
`docs/internal/manifest.md` to describe one session projection and clearly
|
||||
leave per-artifact checkpointing as future roadmap work rather than current
|
||||
behavior.
|
||||
1. Extract finalization that performs the current post-publish cleanup only
|
||||
when publish actually executed, using the runtime's session manifest,
|
||||
manifest path, environment, executed-stage list, and invocation-scoped
|
||||
deletion dependency. Preserve the special cleanup-failure persistence and
|
||||
the fact that a committed remote publish remains authoritative.
|
||||
2. Build `RunSummary` from the runtime's resolved identity and ordered
|
||||
accumulators only after all selected stages and required cleanup complete.
|
||||
Continue returning no success summary on invocation failure.
|
||||
3. Make the top-level executor an explicit sequence: unlocked preparation;
|
||||
layout preparation; visible lock acquisition/deferred release; locked state
|
||||
initialization; collaborator composition; ordered one-stage execution; and
|
||||
finalization/summary. It must retain meaningful error context at each phase
|
||||
and must not delegate that entire sequence to a new monolithic helper.
|
||||
4. Place private lifecycle concerns in focused files according to ownership—for
|
||||
example runtime/preparation, composition, stage execution, and finalization—
|
||||
while keeping exact shared transition logic in `run_control.go`. Remove dead
|
||||
helpers, stale comments, duplicate state, and imports left by extraction.
|
||||
5. Review the resulting runtime for service-locator growth. Remove fields used
|
||||
by only one local operation unless retaining them materially clarifies the
|
||||
lifecycle. Keep collaborators in `stage.Env` and persistence/path policy in
|
||||
their existing owners.
|
||||
6. Run focused assembled-workflow, cleanup, bounded-run, resume, manifest-save,
|
||||
and release-error tests. Add no tests for private file placement, helper
|
||||
count, or line count.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/stage ./internal/app ./internal/manifest` succeeds.
|
||||
- No production analyze type, result, validator, or test distinguishes session
|
||||
state from an invocation artifact subset.
|
||||
- Analyze still performs one end-of-stage session save, retains valid unrelated
|
||||
records, persists its permitted failure projection, and restores prior
|
||||
in-memory authority if that save fails.
|
||||
- No Workstream 6 checkpoint callback or persistence loop has been introduced.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 4 — Delete The Run-Manifest Domain Model
|
||||
|
||||
**Status: Completed**
|
||||
|
||||
### Goal
|
||||
|
||||
Remove the now-unreferenced structured schema, persistence API, canonical file
|
||||
path, and mechanism tests from their owning packages.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Delete `internal/manifest/run_manifest.go`, `RunManifestSchemaVersion`,
|
||||
`RunManifest`, `RunStageRecord`, run status/action enums, constructors,
|
||||
transition methods, and all run-only validation/normalization helpers.
|
||||
2. Remove `manifest.RunStore` and `LocalStore.CreateRun`, `LoadRun`,
|
||||
`LoadRunReader`, and `SaveRun`. Simplify imports and shared validation only
|
||||
where the deleted run code made them unnecessary; do not weaken session
|
||||
manifest validation.
|
||||
3. Delete `artifacts.SessionRunManifestPathForCampaign` while retaining
|
||||
`SessionRunsDirForCampaign` and `SessionRunRootForCampaign` as canonical
|
||||
run-diagnostic and invocation-work paths.
|
||||
4. Delete `run_manifest_test.go` and run-manifest-only store/path/schema tests.
|
||||
Remove run-specific portions of otherwise valuable manifest, artifact,
|
||||
semantic fingerprint, restore, operator, and publish fixtures. Do not replace
|
||||
serialization round-trip tests for a format that no longer exists.
|
||||
5. Keep the remote `run_manifest` artifact enum accepted only by remote commit
|
||||
decoding/validation. Add a concise comment at its declaration explaining
|
||||
that Narratio no longer emits or interprets it and retains the value solely
|
||||
so immutable existing commits remain readable. No production symbol in
|
||||
`manifest`, `app`, or `stage` may refer to it.
|
||||
6. Update package comments or focused internal documentation affected by the
|
||||
domain deletion in the same change.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- `go test ./internal/manifest ./internal/artifacts ./internal/app ./internal/stage`
|
||||
- `go test ./internal/app ./internal/stage ./internal/manifest ./internal/artifacts`
|
||||
succeeds.
|
||||
- Except for the explicitly retained `RemoteArtifactTypeRunManifest` wire
|
||||
value and its remote-decoder compatibility test, searches for `RunManifest`,
|
||||
`RunStore`, `CreateRun`, `LoadRun`, `SaveRun`, `RunManifestPath`,
|
||||
`run_manifest.go`, and the local `runs/<run_id>/manifest.json` contract find
|
||||
no current production or test implementation.
|
||||
- Session manifest schema validation and round trips remain fully covered.
|
||||
- Existing remote commit documents containing the historical `run_manifest`
|
||||
artifact type still decode and validate, but new publish plans cannot emit
|
||||
it.
|
||||
- `executePlan` is a short, readable lifecycle owner with one visibly scoped
|
||||
lock and no nested stage state machine or collaborator-construction block.
|
||||
- Cleanup remains a post-publish obligation with unchanged behavior, awaiting
|
||||
Workstream 9.
|
||||
- The refactor has removed obsolete code rather than retaining parallel old and
|
||||
new lifecycle paths.
|
||||
- `git diff --check` succeeds.
|
||||
|
||||
## Stage 5 — Reconcile Current Documentation And Close Workstream 4
|
||||
## Stage 6 — Remove Global Runner Seams And Close Workstream 5
|
||||
|
||||
**Status: Completed**
|
||||
**Status: Pending**
|
||||
|
||||
### Goal
|
||||
|
||||
Prove the session-only target state across the repository and leave one accurate
|
||||
implementation narrative for the next workstream.
|
||||
Prove the target architecture through invocation-scoped command tests, accurate
|
||||
current documentation, and complete repository validation.
|
||||
|
||||
### Required Work
|
||||
|
||||
1. Search current production code, tests, fixtures, examples, scripts, and
|
||||
nonhistorical documentation for run-manifest types, paths, JSON fields,
|
||||
stores, publish classifications, invocation projections, and dual-save or
|
||||
terminal-synchronization language. Remove stale surfaces or explain the
|
||||
retained run ID/run directory without implying a structured record.
|
||||
2. Reconcile canonical owners, especially `docs/policy/architecture.md`,
|
||||
`docs/cli.md`, `docs/operations.md`, `docs/troubleshooting.md`,
|
||||
`docs/internal/overview.md`, `docs/internal/manifest.md`,
|
||||
`docs/internal/stage-analyze.md`, `docs/internal/stage-extract.md`, and
|
||||
`docs/internal/stage-publish.md`. Keep schema definitions in the manifest
|
||||
owner, filesystem/remote behavior in operations, and implementation details
|
||||
in internal documents rather than duplicating them.
|
||||
3. Do not edit immutable historical release notes or completed audit documents.
|
||||
Roadmap references may retain the phrase “run manifest” when describing the
|
||||
problem and approved removal. Current instructions must not claim the file
|
||||
is produced or required.
|
||||
4. Review the affected test suite under `docs/policy/testing.md`. Consolidate
|
||||
leftover tests that merely encode the deleted write choreography, while
|
||||
retaining meaningful session durability, transition, recovery, current-run
|
||||
diagnostics, remote publication, and representative assembled-workflow
|
||||
1. Introduce the smallest private command-level helper needed to pass a
|
||||
pipeline-executor function for one call through `Execute`, `Run`,
|
||||
`RegenerateArtifacts`, and the single-stage command path. Public command
|
||||
functions continue to use the production executor. The alias must still be
|
||||
exactly equivalent to `run --force --from extract --through analyze` with
|
||||
caller options forwarded under the existing rules.
|
||||
2. Delete `executeStagesFn` and `runCommandFn`. Rewrite affected CLI, profile,
|
||||
remote-session, restore-workflow, assembled-workflow, bounded-run, alias, and
|
||||
artifact-selection tests to call the private command helper with an explicit
|
||||
executor or to exercise the real assembled runner. Do not serialize tests
|
||||
around package-global save/replace/restore choreography.
|
||||
3. Confirm that `removeRunScopedDirFn` is gone and that runner tests do not
|
||||
replace `nowUTC`. Inventory remaining mutable function variables in
|
||||
`internal/app`; leave unrelated configuration-loading and restore seams for
|
||||
their owning future work, but ensure no runner, run-command, alias, or
|
||||
post-publish-cleanup test mutates process-global behavior.
|
||||
4. Review test value under `docs/policy/testing.md`. Consolidate duplicate
|
||||
command-wiring and lifecycle cases exposed by the new narrow seams while
|
||||
retaining consequential lock, persistence, invalidation, recovery,
|
||||
cancellation, external-boundary, and representative assembled-workflow
|
||||
coverage.
|
||||
5. Update Workstream 4 in `refactoring.md` from `Pending` to `Complete` and
|
||||
update every stage plus the purpose/status section of this plan to
|
||||
`Completed` only after the target state and all checks are proven. Do not
|
||||
change later workstream statuses.
|
||||
6. Run the complete repository validation from `docs/development.md`:
|
||||
5. Update current internal documentation where ownership requires it:
|
||||
`docs/internal/overview.md` should summarize the explicit runner lifecycle;
|
||||
`docs/internal/manifest.md` should describe the shared pre-execution decision
|
||||
and single persistence owner without documenting private helper names; and
|
||||
`docs/internal/adapters.md` should describe selective application-owned
|
||||
composition. Update architecture policy only if implementation reveals a
|
||||
normative boundary not already stated. Do not duplicate CLI/configuration
|
||||
contracts or edit historical release documents and completed audit findings.
|
||||
6. Search current code and documentation for stale claims about the monolithic
|
||||
runner, independent plan/run decisions, or mutable runner seams. Remove only
|
||||
inaccurate current-behavior text; roadmap and audit history remain valid.
|
||||
7. Update Workstream 5 in `refactoring.md` from `Pending` to `Complete`, and
|
||||
change all six stage statuses plus this plan's status to completed only after
|
||||
the target state and all checks are proven. Leave Workstream 6 `Pending`.
|
||||
8. Run the complete repository validation from `docs/development.md`:
|
||||
`go test ./...`, `go test -race ./...`, `go vet ./...`, `go build ./...`,
|
||||
`go test ./internal/doccheck`, and
|
||||
`go test ./internal/config -run '^TestExamplesLoadAndValidate$'`. Run
|
||||
`git diff --check` and confirm the worktree contains only intended changes.
|
||||
`go test ./internal/config -run '^TestExamplesLoadAndValidate$'`. Also run
|
||||
`git diff --check` and confirm the implementation commit leaves no unintended
|
||||
worktree changes.
|
||||
|
||||
### Exit Criteria
|
||||
|
||||
- The session manifest is the only structured durable local progress record;
|
||||
no current code or documentation requires a run manifest.
|
||||
- Run IDs, canonical run directories, bounded diagnostics, generated
|
||||
configuration, receipts, immediate summaries, immutable run prefixes, and
|
||||
remote publication remain functional without a second progress transaction.
|
||||
- Publish uses session-manifest evidence, emits no new run-manifest artifact,
|
||||
and still reads verified historical remote commits without interpreting their
|
||||
old diagnostic record.
|
||||
- All repository-wide validation succeeds on the maintained Go toolchain.
|
||||
- Workstream 4 and all five stages are marked `Complete`; Workstream 5 remains
|
||||
the next pending workstream.
|
||||
- All repository-wide validation succeeds on Go 1.26.
|
||||
- Planning and execution share one pure pre-execution decision model while
|
||||
actual results and persistence remain execution-owned.
|
||||
- The runner reads as an explicit lifecycle with one session lock and one
|
||||
session-manifest authority.
|
||||
- Runner, run-command, alias, and cleanup tests use invocation-scoped
|
||||
dependencies rather than mutable package-global function substitution.
|
||||
- Current internal documentation accurately describes the implemented design
|
||||
at the appropriate level.
|
||||
- Workstream 5 and all six stages are marked `Complete`; Workstream 6 is the
|
||||
next pending workstream.
|
||||
- `git status --short` is empty after the implementation commit.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The approved audits, current architecture, and decisions above are
|
||||
sufficient to implement this workstream without further product or policy
|
||||
choices.
|
||||
None. The approved audits, current policies, and Workstream 5 decisions resolve
|
||||
the behavior, architecture, testability, and sequencing choices required for
|
||||
implementation.
|
||||
|
||||
Reference in New Issue
Block a user