diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index ea25462..c4eefef 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,11 +2,13 @@ ## Status And Audience -Complete. +Remediation planned. Stages 1 through 9 are complete; Stages 10 through 14 +address gaps found during the post-implementation architecture review. -This plan is written for a GPT-5.6 Terra coding agent. Implement the stages in -strict numerical order. Do not skip ahead, combine stages merely to reduce the -number of prompts, or describe unfinished behavior as current functionality. +This plan is written for a GPT-5.6 Terra coding agent. Implement incomplete +stages in strict numerical order. Do not skip ahead, combine stages merely to +reduce the number of prompts, or describe unfinished behavior as current +functionality. The target state and acceptance criteria are owned by [notarius-extract-stage.md](notarius-extract-stage.md). This document owns the @@ -25,6 +27,11 @@ implementation sequence. | Stage 7 | Complete | | Stage 8 | Complete | | Stage 9 | Complete | +| Stage 10 | Complete | +| Stage 11 | Pending | +| Stage 12 | Pending | +| Stage 13 | Pending | +| Stage 14 | Pending | After completing and validating a stage, update only that stage's row to `Complete` and record any material deviation in the relevant stage section. @@ -32,7 +39,7 @@ Do not mark a stage complete while required tests or exit criteria remain. ## Working Rules -Before Stage 1, read: +Before Stage 10, read: - `docs/development.md` and its task-specific references; - `docs/roadmap/notarius-extract-stage.md` completely; @@ -40,538 +47,314 @@ Before Stage 1, read: receipt, JSON-output, and lane-contract documentation; and - the focused Narratio documents and tests named by the current stage. -For every stage: +For every incomplete stage: 1. Inspect the current implementation before editing. Prefer the codebase knowledge graph for code discovery as required by `AGENTS.md`. 2. Preserve unrelated worktree changes. -3. Implement the complete stage scope, including focused tests. Do not leave - TODO implementations, compatibility shims without owners, or knowingly - dead code for a later stage. -4. Use real internal collaborators in tests where fast and deterministic; fake - only subprocess, remote, clock, randomness, or other external boundaries. -5. Keep tests offline and independent of real Notarius, PromptKit, LLM - providers, credentials, and mutable services. -6. Run the focused package tests listed for the stage. Fix failures before - proceeding. -7. Run `go test ./...` after every stage. Also run `go vet ./...` and - `go build ./cmd/narratio` when the stage changes shared contracts, - composition, CLI behavior, or documentation examples. -8. Keep proposed behavior in `docs/roadmap/` until Stage 9. Intermediate code - is development work and must not cause current-behavior documentation to - claim that the end-to-end feature is complete. +3. Implement the complete stage scope, including focused regression tests. Do + not leave TODO implementations or defer a known part of the stage. +4. Keep tests offline and independent of real Notarius, PromptKit, LLM + providers, credentials, network services, and mutable external state. +5. Run the focused package tests listed for the stage, then `go test ./...`. +6. Run `go vet ./...` and `go build ./cmd/narratio` whenever shared contracts, + composition, CLI behavior, portability, or documentation examples change. +7. Do not broaden the feature beyond the target architecture while fixing a + remediation. Prefer a general runner or filesystem invariant when the defect + is general, but avoid unrelated cleanup. -Intermediate stages must compile and pass the repository test suite. The work -is not release-ready until Stage 9 is complete. +## Completed Stages 1-9 Summary -## Stage 1: Establish Shared Stage-Outcome And Artifact-Provenance Contracts +1. **Stage 1 — Shared contracts:** Added explicit stage self-skip semantics, + artifact source identity, contract metadata, external provenance, and + backward-compatible manifest serialization. +2. **Stage 2 — Configuration and source policy:** Added strict optional + Notarius configuration, deterministic defaults/path resolution, output-lane + contracts, and the `narratio.extraction.` source family. +3. **Stage 3 — Immutable promotion:** Added regular-file-only bundle copying, + symlink and special-file rejection, bounded permissions, syncing, cleanup, + and atomic no-replace installation on Linux. +4. **Stage 4 — Notarius adapter:** Added the subprocess boundary, exact CLI + invocation, separate receipt/diagnostic streams, bounded tolerant decoding, + path confinement, and generic receipt/index discovery. +5. **Stage 5 — Extract execution:** Added final-trimmed transcript consumption, + configured required-lane validation, immutable bundle promotion, checksums, + manifest-ready outputs, provenance, diagnostics, and invocation + fingerprinting. +6. **Stage 6 — Lifecycle and resume:** Registered `extract` between `trim` and + `render`, added full and single-stage CLI support, resume validation, force + ordering, and old-manifest compatibility. +7. **Stage 7 — Analyze integration:** Added shared manifest-backed extraction + catalog hydration and explicit required/optional Scriptorium input + resolution without directory scanning or implicit inputs. +8. **Stage 8 — Publish and inspection integration:** Added explicit extraction + publish rules, metadata round-tripping, restore compatibility, and operator + availability reporting. +9. **Stage 9 — Documentation and examples:** Added maintained ten-lane and + subset examples, integration and maintainer documentation, operational + guidance, and repository-wide validation. -Introduce the reusable internal contracts needed by extraction without adding -Notarius configuration or a registered stage. +## Stage 10: Correct Downstream Invalidation For Every Replaced Stage Result + +Fix runner semantics so a newly produced extraction result cannot coexist with +downstream stages that are still considered current from an older result. Make +the correction at the general stage-runner boundary rather than adding an +`extract`-specific exception. Implementation: -1. Under `internal/artifactmodel`, add optional neutral models for: - - contract metadata: `media_type`, `schema_id`, `schema_version`, and - optional `module_key`; - - external provenance: `system`, `run_id`, `pipeline_id`, and - `artifact_id`. -2. Extend `artifacts.Ref` with: - - optional explicit `SourceID`; - - optional contract metadata; and - - optional external provenance. -3. Extend `manifest.ArtifactRecord` with the same nested optional metadata. - Use backward-compatible `omitempty` JSON fields. Old manifests must load - and round-trip without fabricated metadata. -4. Refactor `internal/app.mapResultOutputs` to prefer `Ref.SourceID` when set, - copy both metadata structures, and retain existing output-kind and analyze - inference only as fallback behavior. -5. Add an explicit `StageResult` disposition with succeeded and skipped - values. Preserve zero-value success so existing stages do not need - mechanical edits. -6. Add a stable skip-reason field. Reject or fail safely if a skipped result - contains outputs. -7. Teach `executeStages` to persist a self-skipped stage as skipped in both - session and run manifests, apply bounded result metadata and diagnostics, - clear any older session-stage outputs, and continue to later stages. - Distinguish this from the runner's existing decision to skip an already - succeeded stage. -8. Keep successful and failed behavior unchanged for existing stages. +1. In `internal/app`, capture the prior session-stage outcome before changing a + selected stage to running. Preserve enough prior state to distinguish a + reused result, a repeated identical self-skip, and a replaced result. +2. Define replacement and invalidation behavior as follows: + - an already-succeeded stage skipped without execution does not invalidate + anything; + - a resume-validation error still stops without mutating the succeeded stage + or downstream stages; + - a non-resumable succeeded result is marked stale and invalidates succeeded + downstream stages before rerun, preserving the current behavior; + - any forced execution invalidates succeeded downstream stages even if the + execution later self-skips or fails; + - a non-forced execution that changes a skipped, failed, stale, pending, or + absent stage to succeeded invalidates succeeded downstream stages; + - an executed failure invalidates succeeded downstream stages before the + failed manifest state is committed; and + - a self-skip invalidates succeeded downstream stages only when it replaces + a different effective outcome. Repeating the same stable skip reason with + no outputs must not make every disabled full-pipeline invocation rerun all + downstream stages. +3. Persist invalidation with the same session-manifest transition that makes the + upstream replacement observable. Do not leave a committed failed or skipped + upstream result with succeeded downstream records that depend on its former + output. +4. Continue to use canonical stage order for invalidation. For `extract`, the + downstream set remains `render`, `analyze`, `publish`, and `notify`. +5. Recheck each precomputed stage decision immediately before acting on it. + When an earlier stage invalidates a later succeeded stage during the same + full run, the later stage must execute instead of honoring its original skip + decision. +6. Use stable bounded stale reasons that distinguish forced replacement, + changed upstream result, upstream failure, and upstream self-skip where that + distinction is operationally useful. +7. Remove or consolidate the old success-only force invalidation path after the + new invariant owns all replacement cases. Do not invalidate stages earlier + than the replaced stage. Tests: -- `internal/artifactmodel`: JSON behavior for complete and omitted metadata. -- `internal/manifest`: old manifest compatibility and self-skip clearing of - earlier outputs. -- `internal/app`: explicit source ID precedence, fallback inference, - provenance copying, self-skip persistence, continuation after self-skip, - and rejection of skipped results containing outputs. +- extend `internal/app/extract_lifecycle_test.go` with a full-plan or focused + multi-stage fixture in which disabled `extract` and downstream `analyze` first + complete, Notarius is enabled, and the next ordinary run executes both + `extract` and the previously succeeded downstream stages; +- a failed extract followed by a successful ordinary retry invalidates and + reruns previously succeeded downstream stages; +- `run-stage extract --force` followed by `notarius_disabled` leaves downstream + succeeded stages stale; +- forced extraction failure leaves downstream succeeded stages stale; +- repeated identical disabled self-skip does not repeatedly stale downstream + stages; +- successful resume reuse does not invalidate downstream stages; +- resume-validation error preserves both the prior extract success and + downstream state; and +- forcing `render` still does not stale `extract`. Exit criteria: -- Existing stage outputs and manifests remain compatible. -- A synthetic stage can self-skip without being recorded as succeeded. -- A later invocation will reconsider a self-skipped stage. -- `go test ./...`, `go vet ./...`, and `go build ./cmd/narratio` pass. - -## Stage 2: Add Notarius Configuration And Extraction Source Policy - -Add strict configuration and stable source identity, but do not invoke -Notarius or register `extract` yet. - -Implementation: - -1. Add an optional `Notarius *NotariusConfig` field to `PipelineConfig`. -2. Define: - - `enabled` with false default; - - `binary` with `notarius` default; - - `config_path`; - - `pipeline_id`; - - `timeout` with `3h` default; - - optional `working_directory`; and - - `outputs map[string]NotariusOutputConfig`. -3. Define each output with required `lane_id`, `media_type`, `schema_id`, and - `schema_version`, plus optional `module_key`. -4. Apply defaults centrally. When enabled, resolve `config_path` to an - absolute path using the same configuration-source semantics as comparable - adapter paths. Default `working_directory` to the resolved config file's - directory, and resolve an explicitly configured working directory to an - absolute path. -5. Validate enabled configuration before stage execution: - - non-empty binary, config path, pipeline ID, and output map; - - positive duration; - - non-empty output contract fields; - - output keys accepted by the existing configured-artifact key policy; - - unique normalized extraction source IDs; - - unique lane IDs; - - no collision with built-in, configured Scriptorium, or previous-session - source identities. -6. Add `SourceKindExtraction` and helpers for the exact - `narratio.extraction.` family in `internal/artifactpolicy` and - `internal/artifacts`. Do not accept arbitrary unconfigured extraction - sources at configuration validation boundaries. -7. Extend Scriptorium input and publish source validation to accept only - extraction keys declared by the same effective pipeline configuration. -8. Do not expose a Notarius session-ID setting, lane-selection flag, prompt, - profile, model, retry, or reference configuration. - -Tests: - -- strict YAML decoding and unknown-field rejection; -- omitted and disabled behavior; -- defaults and path resolution from pipeline-file location; -- enabled required fields and positive timeout; -- invalid/safe keys, normalized collisions, duplicate lanes, incomplete - contracts, and source-family collisions; -- valid and unknown extraction sources in Scriptorium inputs and publish - rules; and -- existing configurations and maintained examples continue to load. - -Exit criteria: - -- A complete Notarius configuration loads and normalizes deterministically. -- Invalid downstream extraction references fail during config validation. -- No runtime code invokes Notarius yet. -- `go test ./internal/config ./internal/artifactpolicy ./internal/artifacts` - and `go test ./...` pass. - -## Stage 3: Add Safe Immutable Directory Promotion - -Implement the filesystem primitive used to promote a validated external bundle -without exposing a partial or mixed durable directory. - -Implementation: - -1. Add a narrowly named directory-promotion operation to `internal/fileops`. - It accepts an existing source directory and a new destination directory. -2. Require a non-empty source and destination. The destination must not exist. -3. Create a temporary sibling of the destination so final installation uses a - same-filesystem rename. -4. Recursively walk with `Lstat` semantics: - - permit directories and regular files only; - - reject symlinks, devices, sockets, named pipes, and other special files; - - never follow a symlink; - - use deterministic lexical traversal; - - preserve relative layout; and - - use bounded safe permissions rather than copying unsafe mode bits. -5. Copy and sync each regular file, sync the completed temporary tree where - practical, and rename the temporary directory to the unique destination. -6. On any failure, remove only the operation's validated temporary sibling. - Never remove or overwrite the source, destination, workspace root, or a - broad caller-provided directory. -7. Return an error if a concurrent actor creates the destination before final - installation. - -Tests using `t.TempDir()`: - -- nested regular-tree success and byte-for-byte preservation; -- deterministic layout; -- destination already exists; -- source is not a directory; -- symlink to a file, symlink to a directory, and escaping symlink rejection; -- representative special-file rejection where portable; -- copy/install failure cleanup with no partial destination; and -- source preservation after success and failure. - -Exit criteria: - -- A promoted directory appears only as a complete unique tree. -- The operation cannot follow symlinks or replace an existing destination. -- `go test ./internal/fileops` and `go test ./...` pass. - -## Stage 4: Implement The Notarius Adapter - -Create the external subprocess boundary independently of stage policy. - -Implementation: - -1. Add `internal/adapters/notarius` with: - - a `Runner` interface; - - transport-neutral request and result models; - - receipt, index, lane-descriptor, and pipeline-wide-descriptor models; - - a production subprocess runner; and - - a small configurable fake for stage tests. -2. Build exactly: - - ```text - notarius run --config --input - --output-dir --json - ``` - - Use separate argument elements, absolute supplied paths, the configured - working directory, inherited environment, and Narratio's shared subprocess - timeout/cancellation behavior. Do not pass `--session-id`. -3. Write stdout directly to the configured receipt path and stderr to the - configured log path. Do not combine streams. -4. On nonzero exit, cancellation, or timeout, return the subprocess error and - do not parse stdout. -5. After exit zero, read receipt bytes with an explicit upper bound, decode - `notarius.run-result.v1`, tolerate unknown fields, and validate required - fields and matching pipeline ID. -6. Require an absolute `output_directory` and confine it beneath the absolute - output root from the request. Confine the relative `index_file` beneath the - returned directory using Narratio's root-scoped path helpers. Reject - absolute logical paths, traversal, symlinks at consumed paths, and - non-regular files. -7. Decode the supported production JSON index tolerantly. Validate required - management paths and lane descriptors, reject duplicate lane IDs, and - confine every lane and pipeline-wide descriptor file beneath the bundle - root. -8. Load `rejected.json` and `warnings.json` with explicit size bounds and - tolerant decoding. Return only their generic summaries, including lane ID - and reason code where present; do not read lane payload bodies or interpret - D&D fields. Keep free-form external messages bounded for immediate errors - and out of manifest metadata. -9. Return descriptors and paths without deciding which lanes are required or - whether their configured contracts match. -10. Do not implement automatic `notarius config validate` execution. - -Tests: - -- exact argument order and omission of `--session-id`; -- working directory, environment inheritance, stream separation, cancellation, - timeout, and nonzero exit; -- stdout ignored after failure; -- receipt size limit, malformed JSON, unsupported version, missing fields, - pipeline mismatch, and tolerated unknown fields; -- output-directory escape from the requested output root; -- index malformed/unsupported shapes, duplicate lanes, missing management - paths, and tolerated optional fields; -- malformed or oversized rejection/warning documents and tolerated unknown - summary fields; -- absolute logical paths, lexical traversal, root-prefix confusion, symlink - paths, and descriptor escape attempts; and -- valid lane plus chunk-map/evidence-context discovery. - -Use a test helper subprocess or injected shared runner. Do not require a real -Notarius binary. - -Exit criteria: - -- The adapter fully enforces the generic Notarius subprocess and discovery - boundary without D&D policy. -- `go test ./internal/adapters/notarius ./internal/adapters/subprocess` and - `go test ./...` pass. - -## Stage 5: Implement Extract-Stage Execution And Bundle Materialization - -Implement the stage behind direct package tests, but do not yet add it to the -canonical `stage.All()` registry. - -Implementation: - -1. Add `Notarius notarius.Runner` to `stage.Env` and default production - composition in `internal/app/runner.go` when enabled extraction is selected. -2. Add canonical path helpers for: - - the run-local extract directory; - - receipt and stderr paths; - - the run-local Notarius output root; and - - immutable `artifacts/notarius/` destinations. -3. Implement `extractStage` with name `extract` and declarations matching its - final-trimmed input and structured outputs. -4. When configuration is absent or disabled, return the explicit skipped - result with `notarius_disabled` and no outputs. -5. Resolve `narratio.transcript.final_trimmed` through the manifest-aware - resolver. Do not fall back to guessed filenames outside that resolver. -6. Invoke the adapter once with resolved absolute paths. -7. Apply required-output policy: - - find each configured output by exact lane ID; - - require exactly one descriptor; - - compare media type, schema ID/version, and optional module key; - - treat rejected or absent configured lanes as failure; - - tolerate unconfigured lanes; - - require each selected file to be regular, non-empty, and syntactically - valid JSON; and - - compute a staging checksum for later copy verification. -8. Validate the complete bundle tree for promotion, then use Stage 3's helper - to copy it to the unique immutable destination. -9. Re-resolve the promoted index and configured descriptor paths rather than - retaining staging paths. Compute authoritative SHA-256 checksums from the - promoted files and fail if promoted bytes differ from the validated source. -10. Return one `artifacts.Ref` per configured lane with explicit extraction - source ID, checksum, contract metadata, and Notarius external provenance. -11. Return the promoted `index.json` as a `notarius_index` output without a - selectable source ID. -12. Return bounded stage metadata containing durable bundle root, receipt and - diagnostic paths, receipt counts/status, rejection/warning summaries, the - producing Narratio run ID, and a deterministic fingerprint of the resolved - binary, config path, pipeline ID, timeout, working directory, and sorted - output-contract map. -13. Do not delete run-local diagnostics or failed bundles. - -Tests: - -- disabled self-skip; -- missing/invalid final-trimmed input before adapter invocation; -- exact adapter request construction; -- required, missing, rejected, duplicate, incompatible, and unconfigured - lanes; -- non-empty and JSON-syntax validation without D&D decoding; -- immutable promotion and promoted-path re-resolution; -- checksums, explicit source IDs, nested metadata, and external provenance; -- adapter and promotion failures do not return successful outputs; and -- complete bundle files are preserved while unknown files are not registered - as sources. - -Exit criteria: - -- Direct extract-stage execution produces a complete manifest-ready result - using only a fake Notarius runner. -- The stage is not yet in normal CLI plans. -- `go test ./internal/stage ./internal/artifacts ./internal/fileops` and - `go test ./...` pass. - -## Stage 6: Integrate Canonical Ordering, CLI Lifecycle, Force, And Resume - -Make `extract` a real pipeline stage and complete its orchestration semantics. - -Implementation: - -1. Register `extractStage` in `stage.All()` immediately after `trim` and before - `render`. -2. Update all hard-coded or user-visible stage inventories, stage-name - validation, CLI help, planning, prerequisite checks, and test fixtures. -3. Confirm full and single-stage commands can select `extract`. Do not add a - lane-selection flag and do not broaden `--artifacts` beyond Scriptorium - artifact selection. -4. Add an optional stage resume-validation contract. Use a result containing: - - resumable boolean; and - - bounded non-resumable reason. - Reserve an ordinary error for cases where a safe decision cannot be made. -5. Before skipping a succeeded stage, `executeStages` invokes the optional - validator. A non-resumable result marks the session stage stale with its - reason and schedules it to run. A validation error stops execution without - mutating the succeeded record. -6. Implement extraction resume validation against: - - enabled state; - - stored invocation/config-contract fingerprint; - - succeeded extract record; - - immutable bundle and index paths; - - exact configured source set; - - regular confined lane files; - - checksums; - - contracts; and - - Notarius external provenance. - Require the producing Narratio run ID recorded in extract metadata, output - records, and the immutable bundle path to agree. Do not compare it with the - session manifest's top-level `run_id`, which identifies the latest - invocation and may legitimately differ after resume. -7. Do not inspect or hash Notarius's transitive reference/profile closure. -8. Verify canonical downstream invalidation: - - force through `trim` stales extract and later succeeded stages; - - force `extract` stales render and later succeeded stages; - - force `render` does not stale extract. -9. Verify existing session manifests without an extract record normalize and - execute the new stage rather than failing migration. - -Tests: - -- exact canonical order and CLI stage inventory; -- full-plan and `run-stage extract` behavior; -- disabled skip followed by later enablement; -- successful resume skip; -- missing, tampered, incompatible, or config-mismatched outputs trigger stale - and rerun; -- unsafe validation errors stop without corrupting prior success; -- force and downstream invalidation in each direction named above; and -- old manifest compatibility. - -Exit criteria: - -- Normal execution includes `extract` in the agreed position. -- Disabled and resume behavior cannot make obsolete outputs current. -- `go test ./internal/stage ./internal/app ./internal/manifest`, +- Enabling, retrying, forcing, failing, or disabling extraction cannot leave an + incompatible succeeded analyze/publish result current. +- Repeated unchanged disabled extraction remains inexpensive and stable. +- `go test ./internal/app ./internal/stage ./internal/manifest`, `go test ./...`, `go vet ./...`, and `go build ./cmd/narratio` pass. -## Stage 7: Integrate Extraction Sources With The Runtime Catalog And Analyze +## Stage 11: Exclude Run-Local Notarius Bundles From Implicit Publication -Make succeeded extraction lanes selectable by configured Scriptorium -artifacts. +Enforce the existing explicit-only publication boundary while preserving +ordinary run-record diagnostics. Implementation: -1. Extend `ArtifactCatalog` with extraction definitions registered from the - effective Notarius output map. -2. Add one shared manifest-backed hydration operation for extraction sources. - It accepts session paths, manifest, and configured extraction definitions - and marks entries available only after verifying the roadmap's full - successful-stage, source, path, checksum, contract, and provenance rules. -3. Add a distinct extraction provenance value such as - `manifest.current_extract_run`. -4. Extend `ResolveSessionArtifactWithCatalog` to resolve configured extraction - sources through catalog availability. It must never scan the Notarius - artifact directory or accept an incidental file. -5. Refactor catalog construction enough that extraction registration and - hydration are reusable by analyze and publish. Do not create a generic - workflow abstraction or move Scriptorium execution policy into artifacts. -6. Update analyze catalog creation to include effective extraction - definitions and current manifest state before planning Scriptorium inputs. -7. Preserve required/optional input behavior. Required unavailable extraction - inputs fail with actionable extract configuration/rerun guidance; optional - inputs are omitted normally. -8. Do not automatically add extraction inputs to any configured artifact. +1. Update publish run-file collection so the canonical run-local + `extract/notarius-output/` subtree is never included in the run-record upload. + Match the exact slash-normalized relative subtree; do not use a broad + substring rule that could suppress unrelated files. +2. Keep the Notarius receipt and stderr files eligible for the existing run + archive. They are diagnostics, not the published bundle. Preserve current + audio exclusion and all unrelated run-file behavior. +3. Do not scan or upload the durable `artifacts/notarius//` directory. + A configured lane may still be uploaded only through an explicit + `pipeline.publish.outputs` rule resolved through the artifact catalog. +4. Preserve locks, required/optional output handling, upload ordering, current + manifest publication, and the final current-run pointer commit. +5. Update publish metadata so excluded staging-bundle files are not counted in + `run_files_uploaded` or listed in `run_uploaded_paths`. Tests: -- registration, lookup, deterministic ordering, and source-family collision; -- successful current-manifest hydration; -- missing stage, skipped, failed, stale, interrupted, missing-source, - internally inconsistent producer identity, incompatible-contract, - missing-file, tampered-checksum, and unsafe-path rejection; -- required and optional analyze input behavior; -- selected Scriptorium artifact execution receives only explicitly configured - extraction inputs; and -- no object-store calls or directory scanning occur during analyze - extraction resolution. +- materialize a realistic current-run `extract/notarius-output//` + tree containing index, management, lane, pipeline-wide, and unknown regular + files, then prove none are uploaded under the run prefix; +- prove receipt and stderr diagnostics in the same extract directory remain + ordinary run uploads; +- prove no Notarius bundle member is uploaded when there is no explicit + extraction publish rule; +- prove one explicit extraction rule uploads only its durable selected lane to + the configured session destination; and +- retain assertions for commit-marker order, locks, previous-cache publication, + and unrelated run files. Exit criteria: -- A fake successful extract run can feed selected JSON files into a fake - Scriptorium analysis run. -- No failed or incidental bundle can become available through the catalog. -- `go test ./internal/artifacts ./internal/stage ./internal/config` and - `go test ./...` pass. - -## Stage 8: Integrate Extraction Sources With Publish And Operator Inspection - -Complete downstream artifact handling without automatically publishing the -bundle. - -Implementation: - -1. Use Stage 7's shared extraction catalog registration and hydration in - publish planning and source resolution. -2. Permit explicit publish rules whose source is a configured extraction - source. Preserve existing destination policy, lock behavior, required - output policy, selected-Scriptorium behavior, and remote commit order. -3. Do not publish the complete Notarius bundle unless individual explicit - rules name selectable sources. `notarius_index` remains non-selectable in - the initial feature. -4. Ensure the published session manifest retains extraction artifact contract - and external provenance metadata through ordinary serialization. -5. Extend existing artifact/status/inspection output only where necessary so - configured extraction sources report planned, available, unavailable, and - published states consistently. Do not expose payload contents. -6. Confirm restore safely round-trips explicitly published extraction files - through existing manifest and confined-path behavior; add code only if a - real incompatibility is found. - -Tests: - -- successful explicit lane publication; -- required missing or invalid extraction source failure; -- optional/unselected behavior remains consistent; -- publish lock and commit-marker order remain unchanged; -- `--artifacts` does not partially select Notarius lanes; -- manifest metadata survives publish serialization and restore loading; and -- operator inspection never reads or prints extraction payload bodies. - -Exit criteria: - -- Explicit extraction publish rules work through the shared catalog. -- No implicit whole-bundle publication is introduced. +- No run-local or durable Notarius bundle is published wholesale. +- Explicit configured lane publication remains functional and auditable. - `go test ./internal/stage ./internal/app ./internal/artifacts` and `go test ./...` pass. -## Stage 9: Add Maintained Examples, Current Documentation, And Final Validation +## Stage 12: Enforce Canonical Notarius Management-File Semantics -Finish the public and maintainer contract only after the implementation is -complete. +Make the adapter's accepted production contract agree with extract, resume, and +catalog invariants. Implementation: -1. Read Notarius's current published lane contracts and add exact media type, - schema ID, schema version, and module-key constraints for all ten lanes to - a maintained Narratio complete example. Do not guess or copy stale roadmap - placeholders. -2. Add a small maintained Scriptorium example that consumes a purpose-specific - subset of extraction sources rather than all ten. -3. Add `docs/integrations/notarius.md` as Narratio's external-consumer - contract. Link to Notarius's canonical documents instead of duplicating - complete schemas. -4. Add `docs/internal/stage-extract.md` for implemented stage mechanics, - lifecycle, resume validation, failure behavior, and focused tests. -5. Update the canonical owners listed in the roadmap's Documentation - Deliverables section. Keep configuration fields/defaults only in - `docs/config.md`, commands only in `docs/cli.md`, physical layout and force - procedures only in `docs/operations.md`, and implementation mechanics only - in internal documents. -6. Update every implemented stage inventory to show `extract` between `trim` - and `render`. -7. Add troubleshooting guidance for missing Notarius, nonzero exit, - receipt/index incompatibility, required-lane rejection, resume - invalidation, and the requirement to force extraction after changing - Notarius's transitive configuration inputs. -8. Review the roadmap status. If every acceptance criterion is implemented, - mark it complete or move completed planning material according to repository - convention without deleting useful decision context prematurely. -9. Check all changed links, examples, commands, field names, defaults, schemas, - and paths against the implementation. +1. Re-read Notarius's current `run-result.md` and `json-output.md` before editing + and keep tolerant decoding of unknown fields within the supported schema. +2. For `notarius.run-result.v1` using the production JSON output, require the + receipt's logical `index_file` to be exactly `index.json`. A different safe + relative path is semantically incompatible and must fail adapter discovery, + not produce a successful result that later becomes non-resumable. +3. Require the production index management fields to be exactly: + - `manifest_file: manifest.json`; + - `rejected_file: rejected.json`; and + - `warnings_file: warnings.json`. +4. Retain all existing confinement, regular-file, symlink, size-limit, and + tolerant unknown-field checks after the exact semantic checks. Do not tighten + optional lane descriptor fields beyond Notarius's published contract. +5. Keep the canonical index invariant shared by stage output, resume validation, + and catalog hydration. Avoid introducing a second configurable or inferred + management-path model. +6. Return errors that identify the incompatible field and observed value without + including transcript-derived payload content. -Validation: +Tests: -- focused example/configuration validation tests; -- `go test ./...`; -- `go vet ./...`; -- `go build ./cmd/narratio`; -- inspect `git diff --check`; -- confirm ordinary tests use no live Notarius, PromptKit, LLM provider, - credentials, network service, or mutable external state; and -- confirm no secrets or private campaign content were added. +- reject alternate but confined values such as `nested/index.json`, + `./index.json`, and renamed management files; +- continue rejecting absolute paths, traversal, symlinks, and missing files; +- continue accepting canonical documents with unknown optional fields; +- prove a successful adapter result promotes to canonical `index.json`; and +- prove a successful extract record is immediately accepted by resume validation + and catalog hydration without normalization or path rewriting. Exit criteria: -- Every feature-roadmap acceptance criterion is satisfied. -- Maintained examples load under strict configuration validation. -- Current-behavior documentation accurately describes the shipped feature - without duplicating Notarius-owned schema definitions. -- The repository-wide validation suite passes. +- Every adapter success satisfies the canonical management-file assumptions of + extraction, resume, and catalog code. +- `go test ./internal/adapters/notarius ./internal/stage ./internal/artifacts` + and `go test ./...` pass. + +## Stage 13: Complete And Document The Atomic-Promotion Platform Contract + +Replace the accidental Linux-only behavior with an explicit, tested support +boundary. Support Linux, macOS, and Windows; fail early and clearly on other +platforms until they gain an atomic no-replace primitive. + +Implementation: + +1. Keep Linux installation based on `renameat2(RENAME_NOREPLACE)`. +2. Add a Darwin implementation using `renamex_np(RENAME_EXCL)` from + `golang.org/x/sys/unix`. +3. Add a Windows implementation using the no-replace form of + `golang.org/x/sys/windows.MoveFileEx`. The sibling temporary directory keeps + the move on one volume; do not set the replace-existing flag. +4. Split directory-sync behavior by platform where necessary. Unsupported + directory syncing may be treated as best effort only for documented + platform-specific unsupported-operation errors; do not suppress ordinary I/O + or permission failures. +5. Add a small build-specific capability check so an unsupported platform fails + before copying the bundle into a temporary tree. Retain safe cleanup if any + later platform operation fails. +6. Preserve the core invariant on every supported platform: a concurrent actor + that creates the destination wins or causes a clean error; Narratio never + replaces that destination. +7. Document Linux, macOS, and Windows as the supported atomic-promotion + platforms and the explicit extraction limitation on other operating systems. + Do not imply that all of Narratio has a broader support guarantee than its + existing documentation establishes. + +Tests and validation: + +- retain the portable promotion conformance tests for successful nested copy, + source preservation, existing destination, cleanup, symlink rejection, and + install collision; +- add platform-specific no-replace tests that run on their native CI platform; +- cross-compile the fileops tests for Linux, Darwin, and Windows; +- confirm the unsupported-platform implementation returns its capability error + before creating a temporary sibling; and +- run `go test ./internal/fileops`, `go test ./...`, `go vet ./...`, and + `go build ./cmd/narratio` on the development platform. + +Exit criteria: + +- Directory promotion is functional and no-replace on Linux, macOS, and + Windows rather than merely compiling there. +- Unsupported platforms fail before expensive copying and have an explicit + documented boundary. +- All available native and cross-compilation checks pass. + +## Stage 14: Reconcile Documentation And Perform Final Remediation Validation + +Make current-behavior documentation match the corrected implementation and +close the remediation effort only after end-to-end verification. + +Implementation: + +1. Correct `docs/internal/stage-extract.md` to say that omitted or disabled + Notarius explicitly self-skips with `notarius_disabled`; do not describe the + manifest result as succeeded. +2. Replace the nonexistent `internal/stage/extract_resume_test.go` reference with + the actual focused test owner, or create that file only if tests were + intentionally reorganized during remediation. +3. Update operations, publish, and extraction documentation to state that: + - changed extraction outcomes stale affected downstream stages; + - repeated identical disabled self-skip does not cause perpetual reruns; + - the run-local Notarius bundle is excluded from run-record upload; and + - only explicit configured extraction lanes are published. +4. Update platform-support documentation from Stage 13 and troubleshooting for + unsupported atomic promotion. +5. Reconcile `docs/roadmap/notarius-extract-stage.md` with the remediated current + state. Mark the feature complete only if every original and remediation + acceptance criterion is satisfied. +6. Recheck maintained examples, internal links, stage inventories, command + examples, field names, schema identities, defaults, and paths against code. +7. Keep roadmap history concise; do not restore the former detailed completed + Stage 1-9 instructions to this file. + +Validation: + +- run focused lifecycle, adapter, fileops, catalog, analyze, publish, restore, + and operator tests without live external services; +- run focused race tests covering the changed extraction, artifact, publish, + fileops, and runner paths; +- run `go test -count=1 ./...`; +- run `go vet ./...`; +- run `go build ./cmd/narratio`; +- run the Stage 13 cross-compilation checks; +- run `git diff --check`; and +- confirm no secrets, private campaign content, generated binaries, or test + artifacts were added to the repository. + +Exit criteria: + +- All post-implementation review findings are corrected or explicitly bounded + by the documented platform contract. +- Current documentation accurately describes stage outcomes, invalidation, + publication, adapter compatibility, and supported platforms. +- Every progress row is `Complete`, the repository-wide validation suite passes, + and the target feature roadmap can truthfully remain complete. ## Open Questions -None. The target architecture, defaults, lifecycle behavior, compatibility -boundary, storage layout, source identity, downstream selection model, and -implementation order are decision-complete for implementation. If repository -inspection exposes a contradiction with an implemented invariant, stop at the -affected stage and document the concrete conflict rather than silently -changing this plan's target behavior. +None. The remediation stages adopt the long-term-maintainable defaults: general +runner invalidation semantics, explicit-only bundle publication, exact current +Notarius management-file semantics, and atomic promotion support on Linux, +macOS, and Windows with an explicit early failure elsewhere. diff --git a/internal/app/analyze_artifacts_commands_test.go b/internal/app/analyze_artifacts_commands_test.go index d881dd0..c65727f 100644 --- a/internal/app/analyze_artifacts_commands_test.go +++ b/internal/app/analyze_artifacts_commands_test.go @@ -130,6 +130,7 @@ func TestRunArtifactsWithSucceededAnalyzeSkipsUnlessForced(t *testing.T) { for _, stageName := range []string{"prepare", "transcribe", "merge", "polish", "normalize", "trim", "render", "analyze", "publish", "notify"} { seed.MarkStageSucceeded(stageName, time.Date(2026, 5, 3, 10, 1, 0, 0, time.UTC), nil) } + seed.MarkStageSkipped("extract", time.Date(2026, 5, 3, 10, 1, 0, 0, time.UTC), "notarius_disabled") if err := store.Save(context.Background(), manifestPath, seed); err != nil { t.Fatalf("save manifest: %v", err) } diff --git a/internal/app/extract_lifecycle_test.go b/internal/app/extract_lifecycle_test.go index ff08755..f36c191 100644 --- a/internal/app/extract_lifecycle_test.go +++ b/internal/app/extract_lifecycle_test.go @@ -3,6 +3,7 @@ package app import ( "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -19,12 +20,17 @@ import ( ) type materializingNotariusRunner struct { - cfg *config.NotariusConfig - requests []notarius.RunRequest + cfg *config.NotariusConfig + requests []notarius.RunRequest + failuresRemaining int } func (r *materializingNotariusRunner) Run(_ context.Context, req notarius.RunRequest) (notarius.RunResult, error) { r.requests = append(r.requests, req) + if r.failuresRemaining > 0 { + r.failuresRemaining-- + return notarius.RunResult{}, errors.New("notarius execution failed") + } externalRunID := fmt.Sprintf("notarius-run-%d", len(r.requests)) bundle := filepath.Join(req.OutputRoot, externalRunID) lanesDir := filepath.Join(bundle, "lanes") @@ -94,9 +100,121 @@ func TestExtractLifecycleDisabledThenEnabled(t *testing.T) { } } +func TestExtractLifecycleChangedOutcomeRerunsSucceededDownstream(t *testing.T) { + cfg, env, runner := extractionLifecycleFixture(t, false) + analyzeRuns := 0 + plan := extractionLifecyclePlan(t, &analyzeRuns) + + if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}); err != nil { + t.Fatalf("disabled executeStages() error = %v", err) + } + if analyzeRuns != 1 || len(runner.requests) != 0 { + t.Fatalf("disabled run analyze=%d Notarius=%d, want 1 and 0", analyzeRuns, len(runner.requests)) + } + + cfg.Pipeline.Notarius.Enabled = true + summary, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}) + if err != nil { + t.Fatalf("enabled executeStages() error = %v", err) + } + if analyzeRuns != 2 || len(runner.requests) != 1 { + t.Fatalf("enabled run analyze=%d Notarius=%d, want 2 and 1", analyzeRuns, len(runner.requests)) + } + if len(summary.Executed) != 2 || len(summary.Skipped) != 0 { + t.Fatalf("enabled summary = %#v, want extract and analyze executed", summary) + } +} + +func TestExtractLifecycleFailureInvalidatesAndOrdinaryRetryRerunsDownstream(t *testing.T) { + cfg, env, runner := extractionLifecycleFixture(t, true) + runner.failuresRemaining = 1 + markLifecycleStageSucceeded(t, cfg, "analyze") + analyzeRuns := 0 + plan := extractionLifecyclePlan(t, &analyzeRuns) + + if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}); err == nil || !strings.Contains(err.Error(), "notarius execution failed") { + t.Fatalf("failed executeStages() error = %v", err) + } + failed := loadLifecycleManifest(t, cfg) + if failed.Stages["extract"].Status != manifest.StatusFailed || failed.Stages["analyze"].Status != manifest.StatusStale { + t.Fatalf("failed lifecycle extract=%#v analyze=%#v", failed.Stages["extract"], failed.Stages["analyze"]) + } + if failed.Stages["analyze"].Error == nil || failed.Stages["analyze"].Error.Message != staleReasonFailure { + t.Fatalf("analyze stale reason = %#v, want %q", failed.Stages["analyze"].Error, staleReasonFailure) + } + + summary, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}) + if err != nil { + t.Fatalf("retry executeStages() error = %v", err) + } + if analyzeRuns != 1 || len(runner.requests) != 2 || len(summary.Executed) != 2 { + t.Fatalf("retry analyze=%d Notarius=%d summary=%#v", analyzeRuns, len(runner.requests), summary) + } +} + +func TestExtractLifecycleForcedSelfSkipInvalidatesDownstream(t *testing.T) { + cfg, env, _ := extractionLifecycleFixture(t, false) + markLifecycleStageSucceeded(t, cfg, "analyze") + plan, _ := BuildSingleStagePlan("extract") + + if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env, Force: true}); err != nil { + t.Fatalf("executeStages() error = %v", err) + } + loaded := loadLifecycleManifest(t, cfg) + if loaded.Stages["extract"].Status != manifest.StatusSkipped || loaded.Stages["analyze"].Status != manifest.StatusStale { + t.Fatalf("forced self-skip extract=%#v analyze=%#v", loaded.Stages["extract"], loaded.Stages["analyze"]) + } + if loaded.Stages["analyze"].Error == nil || loaded.Stages["analyze"].Error.Message != staleReasonForcedReplacement { + t.Fatalf("analyze stale reason = %#v, want %q", loaded.Stages["analyze"].Error, staleReasonForcedReplacement) + } +} + +func TestExtractLifecycleForcedFailureInvalidatesDownstream(t *testing.T) { + cfg, env, runner := extractionLifecycleFixture(t, true) + runner.failuresRemaining = 1 + markLifecycleStageSucceeded(t, cfg, "analyze") + plan, _ := BuildSingleStagePlan("extract") + + if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env, Force: true}); err == nil { + t.Fatal("executeStages() error = nil, want forced extraction failure") + } + loaded := loadLifecycleManifest(t, cfg) + if loaded.Stages["extract"].Status != manifest.StatusFailed || loaded.Stages["analyze"].Status != manifest.StatusStale { + t.Fatalf("forced failure extract=%#v analyze=%#v", loaded.Stages["extract"], loaded.Stages["analyze"]) + } + if loaded.Stages["analyze"].Error == nil || loaded.Stages["analyze"].Error.Message != staleReasonForcedReplacement { + t.Fatalf("analyze stale reason = %#v, want %q", loaded.Stages["analyze"].Error, staleReasonForcedReplacement) + } +} + +func TestExtractLifecycleRepeatedSelfSkipPreservesSucceededDownstream(t *testing.T) { + cfg, env, runner := extractionLifecycleFixture(t, false) + analyzeRuns := 0 + plan := extractionLifecyclePlan(t, &analyzeRuns) + + if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}); err != nil { + t.Fatalf("first executeStages() error = %v", err) + } + second, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}) + if err != nil { + t.Fatalf("second executeStages() error = %v", err) + } + if analyzeRuns != 1 || len(runner.requests) != 0 { + t.Fatalf("repeated disabled run analyze=%d Notarius=%d, want 1 and 0", analyzeRuns, len(runner.requests)) + } + if len(second.Executed) != 1 || len(second.Skipped) != 2 { + t.Fatalf("second summary = %#v, want executed self-skip and skipped analyze", second) + } + loaded := loadLifecycleManifest(t, cfg) + if loaded.Stages["analyze"].Status != manifest.StatusSucceeded { + t.Fatalf("analyze = %#v, want succeeded", loaded.Stages["analyze"]) + } +} + func TestExtractLifecycleSkipsCurrentResumableResult(t *testing.T) { cfg, env, runner := extractionLifecycleFixture(t, true) - plan, _ := BuildSingleStagePlan("extract") + analyzeRuns := 0 + plan := extractionLifecyclePlan(t, &analyzeRuns) if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}); err != nil { t.Fatalf("first executeStages() error = %v", err) } @@ -104,8 +222,8 @@ func TestExtractLifecycleSkipsCurrentResumableResult(t *testing.T) { if err != nil { t.Fatalf("resume executeStages() error = %v", err) } - if len(resumed.Executed) != 0 || len(resumed.Skipped) != 1 || resumed.Skipped[0] != "extract" || len(runner.requests) != 1 { - t.Fatalf("resume summary = %#v requests=%d", resumed, len(runner.requests)) + if len(resumed.Executed) != 0 || len(resumed.Skipped) != 2 || len(runner.requests) != 1 || analyzeRuns != 1 { + t.Fatalf("resume summary = %#v requests=%d analyze=%d", resumed, len(runner.requests), analyzeRuns) } } @@ -162,7 +280,8 @@ func TestExtractLifecycleResumesAndRerunsObsoleteResults(t *testing.T) { func TestExtractLifecycleUnsafeResumeErrorPreservesSuccess(t *testing.T) { cfg, env, runner := extractionLifecycleFixture(t, true) - plan, _ := BuildSingleStagePlan("extract") + analyzeRuns := 0 + plan := extractionLifecyclePlan(t, &analyzeRuns) first, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}) if err != nil { t.Fatalf("first executeStages() error = %v", err) @@ -176,7 +295,10 @@ func TestExtractLifecycleUnsafeResumeErrorPreservesSuccess(t *testing.T) { if err := store.Save(context.Background(), first.ManifestPath, persisted); err != nil { t.Fatalf("Save() error = %v", err) } - before, _ := json.Marshal(persisted.Stages["extract"]) + before, _ := json.Marshal(map[string]*manifest.StageRecord{ + "extract": persisted.Stages["extract"], + "analyze": persisted.Stages["analyze"], + }) if _, err := executeStages(context.Background(), cfg, plan, RunOptions{Env: env}); err == nil || !strings.Contains(err.Error(), "unsafe") { t.Fatalf("executeStages() error = %v, want unsafe resume failure", err) @@ -185,9 +307,39 @@ func TestExtractLifecycleUnsafeResumeErrorPreservesSuccess(t *testing.T) { if err != nil { t.Fatalf("Load(after) error = %v", err) } - after, _ := json.Marshal(afterManifest.Stages["extract"]) - if string(before) != string(after) || len(runner.requests) != 1 { - t.Fatalf("successful extract record changed: before=%s after=%s requests=%d", before, after, len(runner.requests)) + after, _ := json.Marshal(map[string]*manifest.StageRecord{ + "extract": afterManifest.Stages["extract"], + "analyze": afterManifest.Stages["analyze"], + }) + if string(before) != string(after) || len(runner.requests) != 1 || analyzeRuns != 1 { + t.Fatalf("successful records changed: before=%s after=%s requests=%d analyze=%d", before, after, len(runner.requests), analyzeRuns) + } +} + +func extractionLifecyclePlan(t *testing.T, analyzeRuns *int) []stage.Stage { + t.Helper() + plan, err := BuildSingleStagePlan("extract") + if err != nil { + t.Fatalf("BuildSingleStagePlan(extract) error = %v", err) + } + return append(plan, countingStage{name: "analyze", runs: analyzeRuns}) +} + +func loadLifecycleManifest(t *testing.T, cfg *config.Config) *manifest.Manifest { + t.Helper() + loaded, err := (&manifest.LocalStore{}).Load(context.Background(), manifestPathFor(cfg)) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + return loaded +} + +func markLifecycleStageSucceeded(t *testing.T, cfg *config.Config, name string) { + t.Helper() + loaded := loadLifecycleManifest(t, cfg) + loaded.MarkStageSucceeded(name, time.Now().UTC(), nil) + if err := (&manifest.LocalStore{}).Save(context.Background(), manifestPathFor(cfg), loaded); err != nil { + t.Fatalf("Save() error = %v", err) } } diff --git a/internal/app/run_control.go b/internal/app/run_control.go index 8dbfcff..d7d504c 100644 --- a/internal/app/run_control.go +++ b/internal/app/run_control.go @@ -3,6 +3,7 @@ package app import ( "context" "fmt" + "strings" "time" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" @@ -23,22 +24,40 @@ type stageDecision struct { Action stageAction } +const ( + staleReasonForcedReplacement = "upstream stage was force-run" + staleReasonChangedResult = "upstream stage result changed" + staleReasonFailure = "upstream stage failed" + staleReasonSelfSkip = "upstream stage self-skipped" + staleReasonNotResumable = "upstream stage result was not resumable" +) + +type priorStageOutcome struct { + exists bool + status manifest.StageStatus + skipReason string + outputs int +} + func decideStageActions(stages []stage.Stage, m *manifest.Manifest, force bool) []stageDecision { out := make([]stageDecision, 0, len(stages)) for _, s := range stages { - action := stageActionRun - // TODO: incorporate stale detection once checksum/input change tracking is implemented. - if !force && stageSucceeded(m, s.Name()) { - action = stageActionSkip - } out = append(out, stageDecision{ Stage: s, - Action: action, + Action: decideStageAction(s, m, force), }) } return out } +func decideStageAction(s stage.Stage, m *manifest.Manifest, force bool) stageAction { + // TODO: incorporate stale detection once checksum/input change tracking is implemented. + if !force && stageSucceeded(m, s.Name()) { + return stageActionSkip + } + return stageActionRun +} + func stageSucceeded(m *manifest.Manifest, name string) bool { if m == nil || m.Stages == nil { return false @@ -47,6 +66,29 @@ func stageSucceeded(m *manifest.Manifest, name string) bool { return sr != nil && sr.Status == manifest.StatusSucceeded } +func capturePriorStageOutcome(m *manifest.Manifest, name string) priorStageOutcome { + if m == nil || m.Stages == nil || m.Stages[name] == nil { + return priorStageOutcome{} + } + record := m.Stages[name] + outcome := priorStageOutcome{ + exists: true, + status: record.Status, + outputs: len(record.Outputs), + } + if record.Error != nil && record.Error.Code == "skipped" { + outcome.skipReason = record.Error.Message + } + return outcome +} + +func (o priorStageOutcome) isSameSelfSkip(reason string) bool { + return o.exists && + o.status == manifest.StatusSkipped && + o.outputs == 0 && + o.skipReason == strings.TrimSpace(reason) +} + func loadManifestIfPresent(ctx context.Context, cfg *config.Config) (*manifest.Manifest, error) { path := artifacts.SessionManifestPathForCampaign( cfg.Pipeline.Workspace.Root, @@ -91,10 +133,6 @@ func downstreamStageNames(stageName string) []string { return nil } -func invalidateDownstreamSucceededStages(m *manifest.Manifest, upstreamStage string, at time.Time) []string { - return invalidateDownstreamSucceededStagesWithReason(m, upstreamStage, at, "upstream stage rerun with force") -} - func invalidateDownstreamSucceededStagesWithReason(m *manifest.Manifest, upstreamStage string, at time.Time, reason string) []string { if m == nil || m.Stages == nil { return nil diff --git a/internal/app/run_control_test.go b/internal/app/run_control_test.go index 31d536e..7ccf7cf 100644 --- a/internal/app/run_control_test.go +++ b/internal/app/run_control_test.go @@ -43,7 +43,7 @@ func TestDownstreamStageNames(t *testing.T) { } } -func TestInvalidateDownstreamSucceededStages(t *testing.T) { +func TestInvalidateDownstreamSucceededStagesWithReason(t *testing.T) { now := time.Now().UTC() m := manifest.New("2026-05-03", now) m.MarkStageSucceeded("prepare", now, nil) @@ -58,10 +58,10 @@ func TestInvalidateDownstreamSucceededStages(t *testing.T) { m.MarkStageSucceeded("publish", now, nil) m.MarkStageSucceeded("notify", now, nil) - got := invalidateDownstreamSucceededStages(m, "polish", now.Add(1*time.Second)) + got := invalidateDownstreamSucceededStagesWithReason(m, "polish", now.Add(1*time.Second), staleReasonChangedResult) want := []string{"normalize", "trim", "extract", "render", "publish", "notify"} if !reflect.DeepEqual(got, want) { - t.Fatalf("invalidateDownstreamSucceededStages() = %#v, want %#v", got, want) + t.Fatalf("invalidateDownstreamSucceededStagesWithReason() = %#v, want %#v", got, want) } for _, stageName := range want { @@ -93,7 +93,7 @@ func TestExtractionPositionControlsForceInvalidation(t *testing.T) { for _, name := range canonicalStageNames() { m.MarkStageSucceeded(name, now, nil) } - got := invalidateDownstreamSucceededStages(m, test.upstream, now.Add(time.Second)) + got := invalidateDownstreamSucceededStagesWithReason(m, test.upstream, now.Add(time.Second), staleReasonForcedReplacement) if !reflect.DeepEqual(got, test.want) { t.Fatalf("invalidated = %#v, want %#v", got, test.want) } diff --git a/internal/app/run_stage_test.go b/internal/app/run_stage_test.go index 7bef0e6..eb50aa3 100644 --- a/internal/app/run_stage_test.go +++ b/internal/app/run_stage_test.go @@ -59,6 +59,7 @@ func TestRunNoRemainingStagesRecordsSkippedStages(t *testing.T) { for _, name := range []string{"prepare", "transcribe", "merge", "polish", "normalize", "trim", "render", "analyze", "publish", "notify"} { m.MarkStageSucceeded(name, time.Date(2026, 5, 3, 10, 1, 0, 0, time.UTC), nil) } + m.MarkStageSkipped("extract", time.Date(2026, 5, 3, 10, 1, 0, 0, time.UTC), "notarius_disabled") if err := store.Save(context.Background(), manifestPath, m); err != nil { t.Fatalf("save manifest: %v", err) } @@ -76,7 +77,7 @@ func TestRunNoRemainingStagesRecordsSkippedStages(t *testing.T) { t.Fatalf("load migrated manifest: %v", err) } if loaded.Stages["extract"] == nil || loaded.Stages["extract"].Status != manifest.StatusSkipped { - t.Fatalf("legacy manifest extract record = %#v, want newly executed disabled skip", loaded.Stages["extract"]) + t.Fatalf("extract record = %#v, want stable disabled skip", loaded.Stages["extract"]) } } diff --git a/internal/app/runner.go b/internal/app/runner.go index e701a2b..0af4ab6 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -171,9 +171,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage for _, d := range decisions { s := d.Stage runNames = append(runNames, s.Name()) - if d.Action == stageActionSkip && !stageSucceeded(m, s.Name()) { - d.Action = stageActionRun - } + d.Action = decideStageAction(s, m, opts.Force) if d.Action == stageActionSkip { if validator, ok := s.(stage.ResumeValidator); ok { @@ -186,7 +184,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage staleAt := nowUTC() m.MarkStageStale(s.Name(), staleAt, validation.Reason) invalidateDownstreamSucceededStagesWithReason( - m, s.Name(), staleAt, "upstream stage result was not resumable", + m, s.Name(), staleAt, staleReasonNotResumable, ) if err := env.ManifestStore.Save(ctx, manifestPath, m); err != nil { return nil, fmt.Errorf("save manifest after resume validation for stage %q: %w", s.Name(), err) @@ -209,6 +207,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage continue } executed = append(executed, s.Name()) + priorOutcome := capturePriorStageOutcome(m, s.Name()) now := nowUTC() runManifest.SetStageAction(s.Name(), manifest.RunStageActionRun, now) @@ -217,6 +216,9 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage return nil, fmt.Errorf("save run manifest before stage %q: %w", s.Name(), err) } m.MarkStageRunning(s.Name(), now) + if opts.Force { + invalidateDownstreamSucceededStagesWithReason(m, s.Name(), now, staleReasonForcedReplacement) + } env.Logger.Info("starting stage", "stage", s.Name()) if err := env.ManifestStore.Save(ctx, manifestPath, m); err != nil { return nil, fmt.Errorf("save manifest before stage %q: %w", s.Name(), err) @@ -230,6 +232,7 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage if err != nil { failedAt := nowUTC() m.MarkStageFailed(s.Name(), failedAt, err.Error()) + invalidateDownstreamSucceededStagesWithReason(m, s.Name(), failedAt, staleReasonFailure) if saveErr := env.ManifestStore.Save(ctx, manifestPath, m); saveErr != nil { return nil, fmt.Errorf("stage %q failed (%v) and manifest save failed (%v)", s.Name(), err, saveErr) } @@ -247,6 +250,9 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage m.MarkStageSkipped(s.Name(), skippedAt, result.SkipReason) clearStageResultDetails(m.Stages[s.Name()]) applyStageResultToManifest(m, s.Name(), result) + if !priorOutcome.isSameSelfSkip(result.SkipReason) { + invalidateDownstreamSucceededStagesWithReason(m, s.Name(), skippedAt, staleReasonSelfSkip) + } if err := env.ManifestStore.Save(ctx, manifestPath, m); err != nil { return nil, fmt.Errorf("save manifest after self-skip %q: %w", s.Name(), err) } @@ -265,8 +271,8 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage succeededAt := nowUTC() m.MarkStageSucceeded(s.Name(), succeededAt, outputs) applyStageResultToManifest(m, s.Name(), result) - if opts.Force { - invalidateDownstreamSucceededStages(m, s.Name(), succeededAt) + if !priorOutcome.exists || priorOutcome.status != manifest.StatusSucceeded { + invalidateDownstreamSucceededStagesWithReason(m, s.Name(), succeededAt, staleReasonChangedResult) } if err := env.ManifestStore.Save(ctx, manifestPath, m); err != nil {