diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 84fa09b..a66fcfb 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -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//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:` 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//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//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.