Files
narratio/docs/roadmap/implementation.md

482 lines
26 KiB
Markdown

# Runner Lifecycle Refactoring Implementation Plan
## Status And Purpose
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.
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.
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).
## Scope And Fixed Decisions
- 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 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. 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 — Unify Pre-Execution Decisions
**Status: Completed**
### Goal
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/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/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 — Establish The Invocation Runtime And Lock Lifecycle
**Status: Completed**
### Goal
Make invocation state cohesive and make the top-level transaction order obvious
without hiding session-lock ownership.
### Required Work
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: Completed**
### 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.
- 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 5 — Isolate Finalization And Simplify The Lifecycle Owner
**Status: Pending**
### Goal
Complete the runner decomposition so the lifecycle and transaction boundaries
are easier to read than the former monolith.
### Required Work
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/app ./internal/stage ./internal/manifest ./internal/artifacts`
succeeds.
- `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 6 — Remove Global Runner Seams And Close Workstream 5
**Status: Pending**
### Goal
Prove the target architecture through invocation-scoped command tests, accurate
current documentation, and complete repository validation.
### Required Work
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 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$'`. Also run
`git diff --check` and confirm the implementation commit leaves no unintended
worktree changes.
### Exit Criteria
- 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 policies, and Workstream 5 decisions resolve
the behavior, architecture, testability, and sequencing choices required for
implementation.