diff --git a/docs/roadmap/audit-findings.md b/docs/roadmap/audit-findings.md index 061ab5d..94c7859 100644 --- a/docs/roadmap/audit-findings.md +++ b/docs/roadmap/audit-findings.md @@ -107,7 +107,7 @@ These are prioritization signals, not findings: | 7: adapters and shared support | complete | All HTTP, subprocess, notification, storage, audio, shared-model, and diagnostic boundaries were compared with their contracts and production callers. Focused normal and race commands passed. Confirmed `COR-016`, `RSK-011` through `RSK-014`, `EFF-002`, and `ARC-004`; added `ARC-005`, `DUP-005`, `COM-003`, and `TST-007`, and refined `TST-001`. | | 8: ordinary stages | complete | Prepare, transcribe, merge, polish, normalize, trim, and render were traced from resolved inputs through adapters, validation, run-local/canonical outputs, diagnostics, and manifest recording. The focused normal command passed; the required race command reproduced only `TST-001`. Confirmed `COR-017` through `COR-019`; added `ARC-006`, `DUP-006`, `COM-004`, and `TST-008`, and refined `ARC-001`, `ARC-002`, `ARC-005`, `RSK-013`, and scenario 8. | | 9: extraction | complete | Configuration, transcript resolution, fingerprinting, Notarius execution, receipt/index/lane validation, immutable promotion, manifest advertisement, catalog hydration, resume, and explicit analyze/publish consumption were traced as one slice. The exact focused command passed. Confirmed `COR-020`; added `DUP-007`, `SIM-002`, and `TST-009`, and refined `ARC-001`, `RSK-013`, and scenario 3. | -| 10: analyze and dependencies | not_started | Assigned artifact dependency/source and selection scenario below. | +| 10: analyze and dependencies | complete | All five source-policy families, seven built-in catalog entries, configured-artifact execution/reuse, dependency validation/order, previous-cache locality, lifecycle, and publish selection were traced as one slice. The exact focused command passed. Confirmed `COR-021` through `COR-024` and `RSK-015`; added `ARC-007`, `DUP-008`, `SIM-003`, `COM-005`, and `TST-010`; and refined `ARC-001`, `ARC-002`, `RSK-013`, and scenario 9. | | 11: maintainability | not_started | Seeded by graph complexity, similarity, and fan-in signals only. | | 12: test policy | not_started | Seeded by intended owners and baseline execution observations. | | 13: synthesis | not_started | No final ranking or accepted-risk decisions yet. | @@ -125,7 +125,7 @@ has been mapped but not behaviorally audited. | Prepare and audio | `internal/stage`, `internal/audio`, `internal/previouscache` | 8 | reviewed | | Transcript stages | `internal/stage` plus tool adapters | 8 | reviewed | | Extraction | `internal/stage`, Notarius adapter, `internal/fileops` | 9 | reviewed | -| Analyze and artifact dependencies | `internal/stage`, `internal/artifacts`, `internal/artifactpolicy` | 10 | assigned | +| Analyze and artifact dependencies | `internal/stage`, `internal/artifacts`, `internal/artifactpolicy` | 10 | reviewed | | Publish and cleanup | `internal/stage`, `internal/app` | 4 | reviewed | | Manifest state | `internal/manifest`, transition policy in `internal/app` | 2 | reviewed | | Artifacts, paths, and policy | `internal/artifacts`, `internal/artifactpolicy`, `internal/pathsafe` | 3 (resolution consumption revisited in 10) | reviewed | @@ -185,7 +185,7 @@ succeeded downstream work, while force pre-stales succeeded downstream work. | 6 `trim` | Manifest final transcript or configured canonical fallback -> final-trimmed transcript and, when enabled, bounds | Trim, bounds, Scriptorium, and Seriatim fields; both runners when enabled | Disabled trim copies and validates the normalized transcript, then succeeds; no explicit self-skip or resume validator | Enabled bounds/trim results validate before canonical materialization; render-debug and subprocess logs/config are diagnostics, not outputs. | | 7 `extract` | Final-trimmed source -> immutable index and configured lane outputs | Notarius executable/config/pipeline/timeout/output contracts; `notarius.Runner` | Disabled is explicit `notarius_disabled` self-skip; only current `ResumeValidator`; obsolete reruns, unsafe validation errors. The validator does not bind reuse to the current trimmed-transcript bytes (`COR-020`). | Validates the run-local receipt/index/configured lanes, promotes the complete regular-file bundle to a unique no-replace destination, rechecks promoted checksums, and records exact checksums/contracts/provenance. Only configured lanes are selectable; index and unconfigured bundle members remain audit state. | | 8 `render` | Manifest final/final-trimmed JSON or canonical fallback -> two Markdown transcripts | Render and Seriatim fields; `seriatim.Runner` | Disabled returns a zero-disposition no-output result, therefore durable success rather than explicit self-skip; later enablement needs force; no validator | Enabled run-local text validates non-empty before either canonical result is recorded; logs/config are diagnostics. Focused wording is imprecise under `COM-004`. | -| 9 `analyze` | Dynamic built-in, prepared, extraction, configured, and previous sources -> selected configured artifact outputs | Scriptorium artifact graph/selection; `scriptorium.Runner` | Missing config or no executable artifacts returns success with skip metadata; no validator | Topological run-local generation/reuse, validation, canonical outputs, deterministic metadata; static `Declares` omits dynamic outputs and several input families. | +| 9 `analyze` | Dynamic built-in, prepared, extraction, configured, and previous sources -> selected configured artifact outputs | Scriptorium artifact graph/selection; `scriptorium.Runner` | Missing config or no executable artifacts returns ordinary success with skip metadata, not self-skip; later configuration enablement needs force; no validator | Selected artifacts run in stable topological order; non-executable configured outputs may be reused; each generated run-local output is validated, materialized canonically, and exposed to later dependents. Static `Declares` omits dynamic outputs and several input families. Optional built-ins, explicit selection/prerequisite planning, guidance, and inert input fields have confirmed defects below. | | 10 `publish` | Session/run state, selected output rules, locks, previous cache -> remote run/output/current objects | Publish/storage/selection fields; `storage.ObjectStore` | Disabled publish or run upload returns ordinary success with skip metadata, not a self-skip; force cannot bypass locks; no validator | Deterministic uploads; `current/manifest.json` before `current/run_id.txt`; post-commit local metadata gates cleanup. Static prerequisites omit extract because disabled extraction is valid and lane resolution enforces required extraction state when selected. | | 11 `notify` | No implemented persisted pipeline input/output | Optional `notify.Sender`; default no-op | Ordinary succeeded-record skip; no explicit self-skip or validator | Placeholder metadata and optional notification call; no returned output. `Declares` nevertheless advertises placeholder input/output paths. | @@ -355,6 +355,48 @@ workers, and `COR-018` confirms that parent cancellation before an adapter records an error can be mistaken for successful zero or partial output. Runner reporting remains the Stage 2 dual-ledger behavior. +Scenario 9 is resolved. Without an explicit filter, configured artifacts are +executable exactly when enabled. With a non-empty filter, catalog registration +makes exactly the named configured keys executable even when their `enabled` +field is false; every other configured artifact becomes non-executable. A +non-executable artifact is available for dependency/input reuse only when its +configured canonical output is an existing non-empty file. That includes both +truly disabled artifacts and enabled-but-unselected artifacts, although both +receive provenance named `filesystem.disabled_artifact_output`. A selected +artifact may depend on an unselected artifact only through that reusable local +file. Selected dependencies are ordered before dependents; independent ready +nodes and final results are lexically stable. Cycles in the enabled graph are +rejected by configuration and cycles in an explicitly selected graph are +rejected again at runtime. The preflight's first unavailable-dependency error +is not stable when several selected nodes fail, which is `RSK-015`. + +Built-in transcript/bounds entries resolve manifest-first then canonical, +prepared stable inputs resolve fixed `inputs/*.yml` paths, extraction entries +hydrate only from a complete compatible current extract record, configured +entries resolve only through catalog availability, and previous-session entries +resolve only from manifest-backed or filesystem `previous/` cache state. No +analyze resolver calls the object store. Required/optional behavior is coherent +for prepared, extraction, configured, previous, polished-transcript, and bounds +sources, but normalized/trimmed/Markdown built-ins bypass the optional policy +(`COR-021`). Previous-input preparation scans enabled artifacts only while +selection can execute disabled artifacts (`COR-022`), and its missing-input +guidance is not an executable CLI command (`COR-023`). Input-level `artifact` +and `path` fields are accepted and documented but never reach resolution or the +adapter (`COR-024`). + +Each successful Scriptorium invocation validates a run-local non-empty output, +materializes it to the configured canonical path, and marks it generated for +later selected dependents. Generated/reused metadata and logs/configuration are +sorted or deduplicated deterministically. Publish builds an independent +availability catalog and filters only publish rules sourced from +`narratio.artifact.`: an unselected configured rule is skipped even when +required, while built-in and explicit extraction rules are unaffected. A +selected configured rule must be locally available; publication never causes +analyze execution. Missing/no-executable analyze returns ordinary successful +no-output metadata so the pipeline can continue to publish; it is durably +reused until forced, closing analyze's behavior portion of `ARC-002` and leaving +the wording gap in `COM-005`. + ## Preliminary Risk-To-Test Matrix This matrix identifies intended owners only. It makes no sufficiency judgment. @@ -418,6 +460,15 @@ Stage 9 test observations for this matrix: | Reuse and downstream invalidation | Stage and runner tests cover immediate and cross-invocation reuse, disabled-to-enabled reconsideration, configuration-value changes, missing/tampered payloads, source/contract/provenance mismatches, forced replacement, failed retry, unsafe resume errors, and changed-outcome invalidation. | No test changes the direct trimmed transcript beneath an otherwise succeeded record, so `COR-020` remains green. Same-path external dependency changes are documented force cases; one lifecycle contract test is sufficient if an explicit external revision mechanism is added. | | Catalog and explicit consumption | Catalog tables require one complete current bundle and reject unsafe, incomplete, mismatched, incidental, or tampered state. Analyze and publish tests prove only explicitly configured lanes are passed/uploaded and invalid required lanes fail before execution/upload. | Resume and catalog independently encode much of the same bundle proof (`DUP-007`); future shared evidence tests should preserve resume's obsolete-versus-unsafe result and catalog's fail-closed all-or-none behavior. | +Stage 10 test observations for this matrix: + +| Risk | Existing focused protection | Gap or disposition | +| --- | --- | --- | +| Source-family resolution | Analyze tests cover successful built-in transcript variants, prepared stable inputs, configured generated/reused outputs, extraction lanes, previous-cache manifest/fallback paths, required failures, and optional absence for prepared/configured/extraction/previous sources. Artifact-policy/config tables protect accepted identities. | No missing optional normalized, trimmed, or Markdown built-in case exists, so `COR-021` remains green. The focused analyze document also omits extraction and successful no-output lifecycle consequences (`COM-005`). | +| Selection, reuse, and dependency order | Catalog tests explicitly require selection to override `enabled`; analyze tests cover one selection filter, generated/reused metadata, independent lexical order, selected dependencies, cycles, and unavailable reused dependencies. Publish tests cover selected/unselected configured rules and prove built-in/extraction rules are unaffected. | No assembled test selects a disabled artifact, crosses that choice into previous-requirement preparation, or creates several simultaneously unavailable dependencies. `COR-022`, `RSK-015`, `ARC-007`, and `TST-010` own those seams. | +| Previous-session locality and guidance | Requirement collection covers enabled/disabled, deduplication, required-wins, and stable ordering. Prepare/restore/app tests cover planned requirements; analyze proves manifest/fallback local resolution and explicitly asserts no object-store call. | Selection is absent from the collector API (`COR-022`), while the guidance test asserts only a fragment of the malformed command and therefore preserves `COR-023`. | +| Scriptorium request and output materialization | Stage tests inspect exact named input paths, vars, generated/reused metadata, render-debug, logs/configuration, and canonical materialization. Adapter tests assert deterministic flags and generated invocation configuration. | Input `artifact`/`path` fields have no consumer or request representation (`COR-024`); no stage-side link/non-regular/oversized result case covers analyze, extending `RSK-013`; and the broad execution/resolution shape remains `SIM-003`. | + ## Path, Artifact, Filesystem, And Lock Conclusions ### Canonical ownership and normalization @@ -1107,8 +1158,8 @@ render deliberately returns successful no-output metadata so the pipeline can continue without Markdown; like any previously succeeded stage, later enablement requires force. Those ordinary-stage outcomes are coherent and resolve their part of `ARC-002`; the focused render document's generic “skips” -wording is retained as `COM-004`. Analyze's remaining no-op outcome stays in -Stage 10. +wording is retained as `COM-004`. Analyze's no-op outcome was deferred to Stage +10, which confirmed the same durable-success behavior under `COM-005`. ### Similarity classification @@ -1218,6 +1269,116 @@ file fully again for JSON validation. This broadens the already confirmed `RSK-013`; safe regular-file shape is stronger here than in ordinary adapters, but an external multi-gigabyte JSON lane can still exhaust memory. +## Analyze And Artifact-Dependency Conclusions + +### Catalog state, selection, and reuse + +The runtime catalog keeps registration, executability, availability, and +provenance separate. Analyze registers every built-in, configured artifact, +and configured extraction source. Extraction availability is manifest-backed +and all-or-none. Configured availability is different: entries executable in +the current invocation are initially unavailable, while every non-executable +entry with a valid non-empty canonical output is marked reusable from disk. +After one selected artifact succeeds, its canonical materialized output is +marked generated so later selected dependents can consume it in the same run. + +Selection is authoritative over the enabled flag inside catalog registration, +and a catalog unit test explicitly preserves that behavior. With no selection, +enabled controls execution. With a selection, exact membership controls +execution and may activate a disabled artifact or suppress an enabled one. +The CLI validates only that selected names exist. Configuration, previous- +requirement planning, and focused documentation do not consistently share this +rule: disabled artifacts need not have an executable prompt/output contract, +previous requirements scan enabled artifacts only, and the artifact document +defines executable as both selected and enabled. `ARC-007` owns the unresolved +authority choice, while `COR-022` records the already observable prerequisite +failure. + +Reuse deliberately does not require an old analyze success record: a configured +non-executable output is accepted by canonical path and non-empty-file shape. +This supports operator-prepared or preserved disabled dependencies, but it is +not freshness validation. Enabled-but-unselected files receive provenance named +for disabled outputs, so metadata cannot distinguish those two causes. This is +an observability consequence of `ARC-007`, not a separate correctness defect. +Publish registers the same configured identities but independently marks any +existing canonical configured output available, then applies selection only to +configured-source publish rules. It neither executes artifacts nor treats +selection as a filter for built-in/extraction sources. + +### Source resolution and required policy + +Artifact policy classifies built-in, prepared-stable, extraction, configured, +and previous-artifact families before analyze resolves them. The built-in +catalog contains base, polished, final, final-trimmed, both rendered Markdown +variants, and bounds. Built-ins prefer manifest output records and then their +canonical path; the resolver applies transcript JSON, non-empty Markdown, or +bounds JSON validation. Stable inputs map to fixed prepared files. Extraction +and configured sources require catalog availability. Previous sources use a +separate resolver that prefers matching manifest input records and falls back +to local `previous/` paths; neither path reaches object storage. + +Optional absence is consistently omitted for prepared stable, extraction, +configured, previous, polished-transcript, and bounds sources. Final, +final-trimmed, and both Markdown cases instead return guidance errors inside +the source-specific branch before the caller can apply `required=false`; this +is `COR-021`. Required prepared and extraction errors identify the producing +stage/configuration. Configured absence is clear but has no repair command. +Required previous absence emits a syntactically invalid and sessionless command +(`COR-023`). The accepted `artifact` and `path` input fields are not consulted +by policy, resolution, execution, either adapter request, generated invocation +configuration, or previous planning; `COR-024` owns that silent contract. + +### Dependency order, execution, and lifecycle + +Configuration validates dependency identities, self-reference, configured- +source/`depends_on` agreement, referenced output paths, and cycles among enabled +artifacts. Analyze validates the executable subgraph again. A dependency +outside that subgraph must already be catalog-available; a dependency inside it +becomes a directed edge. Edges and ready nodes are sorted, so successful order +is deterministic and dependencies precede dependents. The initial unavailable- +dependency pass ranges over a map, however, so several invalid selected nodes +can produce different first errors (`RSK-015`). Re-sorting the ready slice on +every insertion is more work than necessary, but configured artifact counts are +small and no material efficiency defect was established. + +For each plan, input names are sorted, optional omissions and reused sources are +recorded, variables are normalized with Narratio's sticky session identifier, +and optional render-debug completes before the run request. The result must be +non-empty, then run-local bytes are materialized to the canonical artifact path. +Logs/generated configs are deduplicated and sorted, execution metadata follows +plan order, and reused metadata is stable. Output checks still inherit the +link-following, non-regular, and unbounded acquisition risk in `RSK-013`. +Failure after an earlier artifact materializes can leave that canonical output, +but no stage result is advertised; an ordinary retry executes the same selected +set again rather than implicitly resuming mid-graph. + +Missing Scriptorium configuration, an empty artifact map, or no executable +entries returns zero-disposition metadata and is recorded as success by the +runner. That permits full-pipeline continuation and satisfies publish's analyze +prerequisite, but the success is reused after later configuration changes until +forced. The dedicated `analyze` command force-runs, mitigating the explicit +operator workflow. This settles analyze's state behavior for `ARC-002`; the +focused document's generic “skips” wording and omitted extraction family remain +`COM-005`. + +### Representation and ownership + +`analyzeStage.Run` coordinates catalog, planning, transcript-reference +diagnostics, execution, catalog mutation, and aggregation. Its artifact helper +is 239 lines with ten parameters; the source resolver is 84 lines with six +parameters and two unused contextual parameters; the dependency orderer is 86 +lines with ten loops. These metrics support only the narrow `SIM-003`: a typed +execution context/plan and source-resolution result could make state ownership +visible without introducing a generic stage framework or collapsing distinct +source guidance. `resolveInputPathForRead` has no production or test caller and +belongs in that review. + +Analyze, publish, and app operator helpers also repeat built-in/configured/ +extraction catalog registration and extraction hydration. Their final +availability policies differ intentionally, but the shared registration +mechanics and source-definition construction are one policy repeated in three +places; `DUP-008` assigns a narrow common bootstrap decision to Stage 11. + ## Confirmed Findings ### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity @@ -1627,8 +1788,8 @@ but an external multi-gigabyte JSON lane can still exhaust memory. default-path/fallback tests remain as compatibility cases. - Dependencies: this mapping is also required for the committed restore scope in `COR-008`. Stage 6 confirmed that custom destinations are supported and - validated, so remediation must preserve them; Stage 10 owns later artifact- - consumer semantics, not remote identity selection. + validated, so remediation must preserve them; Stage 10 later confirmed that + artifact-consumer semantics do not repair remote identity selection. ### `COR-012`: strict YAML loading silently ignores a valid trailing document @@ -1913,6 +2074,135 @@ but an external multi-gigabyte JSON lane can still exhaust memory. validation structure. Same-path Notarius files remain the documented scenario-3 force limitation. +### `COR-021`: optional normalized, trimmed, and Markdown analyze inputs fail when absent + +- Category: confirmed correctness defect. +- Locations/invariant: `internal/stage/analyze.go` in + `resolveScriptoriumInput`. Every Scriptorium input with `required: false` + must be omitted when its valid source is unavailable, independent of source + family; `required: true` owns source-specific failure and repair guidance. +- Evidence: after the catalog resolver returns `ErrSessionArtifactNotFound`, + extraction and configured branches consult `inputCfg.Required`, while + polished and default built-ins return unresolved for the caller to apply the + flag. The cases for `narratio.transcript.final`, `final_trimmed`, + `final_markdown`, and `final_trimmed_markdown` instead return errors + unconditionally. The caller therefore never reaches its optional-omission + branch. Focused tests cover successful values and required missing values for + these sources, but no optional missing case. +- Realistic scenario: one artifact can use an optional rendered transcript to + improve a prompt while remaining valid from a prepared or extraction input. + On a session without render output, analyze fails instead of invoking + Scriptorium without that optional input. +- Impact/likelihood/confidence: medium workflow-availability impact; moderate + likelihood for optional prompt enrichment; high confidence from the direct + branch ordering and the repository-wide required/optional contract. +- Estimated remediation scope and owner: small analyze source-resolution + change. Return source-specific errors only when required, otherwise return an + unresolved optional result. Preserve the distinct normalize/trim/render + guidance for required inputs. +- Test changes: table all built-in transcript/bounds identities as missing with + both required values, asserting omission for optional and the correct producer + guidance for required. Avoid duplicating successful resolver tests. +- Dependencies: use `SIM-003` only to make the decision shape clearer; do not + merge the source-specific guidance policies. + +### `COR-022`: explicit artifact selection bypasses previous-input prerequisite planning + +- Category: confirmed cross-boundary correctness defect. +- Locations/invariant: `internal/artifacts/catalog.go` in + `RegisterConfiguredArtifacts`, `internal/artifacts/previous_requirements.go` + in `CollectPreviousArtifactRequirements`, and its app/prepare/restore/status/ + validation callers. Every artifact that analyze can execute must contribute + its required previous-session inputs to prerequisite planning. +- Evidence: a non-empty selected set makes membership authoritative over + `Enabled`; a focused catalog test explicitly selects and makes a disabled + artifact executable. CLI validation accepts any configured key regardless of + enabled state. Requirement collection has no selection input and skips every + disabled artifact. Consequently prepare, restore planning, object-store + composition, artifact listing, status, and validation all omit dependencies + that analyze will require from an explicitly selected disabled artifact. +- Realistic scenario: an operator keeps an occasional artifact disabled but + selects it for one run. It requires a previous-session recap. The run does + not compose/fetch/prepare that requirement, and analyze fails. Following its + prepare guidance still cannot populate the file while the artifact remains + disabled. +- Impact/likelihood/confidence: medium-to-high workflow correctness impact; + moderate likelihood because explicit selection is the natural one-off path; + high confidence from the tested selection override and collector signature. +- Estimated remediation scope and owner: medium app/artifact-planning change + after `ARC-007` chooses authority. If selection can activate disabled entries, + derive one effective artifact set and pass it to every prerequisite consumer. + If selection must intersect enabled entries, reject disabled selections + before planning. Do not let analyze and prepare recompute different sets. +- Test changes: an assembled run/prepare/analyze test must select a disabled + artifact with a required previous source and prove composition, planning, + materialization, and execution agree. Also cover optional previous input and + no-selection enabled behavior. +- Dependencies: `COR-010` concerns readiness checks for requirements that were + collected; this finding owns requirements omitted by selection. `ARC-007` + owns the product choice, and `TST-010` owns regression placement. + +### `COR-023`: required previous-input failure recommends an invalid command + +- Category: confirmed correctness/operability defect. +- Locations/invariant: `internal/stage/analyze.go` in + `resolveScriptoriumInput`, with syntax owned by `docs/cli.md` and + `internal/app` command parsing. Actionable failure guidance must name an + executable command for the current session. +- Evidence: the error says `run narratio run-stage --force prepare`. The actual + grammar is `narratio run-stage prepare --force`; the emitted + form places a flag where the required stage argument belongs, reverses the + stage/flag order, adds an extraneous leading `run`, and omits the session ID. + The existing test checks only the fragment `run-stage --force prepare`, so it + codifies rather than catches the bad syntax. Prepared-stable and Markdown + guidance already include the session ID in the correct order. +- Realistic scenario: analyze fails on an absent required previous artifact and + the operator copies the suggested recovery command. Parsing fails before + prepare runs, extending an already blocked recovery path. +- Impact/likelihood/confidence: low data impact but direct recovery/operability + impact; high likelihood whenever this failure occurs; high confidence from + exact CLI grammar and emitted text. +- Estimated remediation scope and owner: tiny analyze message change. Emit + `narratio run-stage prepare --force`, or route producer guidance + through a small command formatter if `SIM-003` establishes one. Include the + session ID already available in `paths`. +- Test changes: assert the complete command string, not an invalid fragment. + A parser round-trip is optional if a shared formatter is introduced. +- Dependencies: `COR-022` can make prepare itself omit the dependency; repair + both before claiming selected-disabled recovery is actionable. + +### `COR-024`: documented Scriptorium input passthrough fields are silently discarded + +- Category: confirmed configuration-contract defect. +- Locations/invariant: `internal/config/config.go` in + `ScriptoriumInputConfig`, `docs/config.md` in Scriptorium artifact entries, + `internal/stage/analyze.go`, and `internal/adapters/scriptorium`. Accepted and + documented operator fields must affect behavior or be rejected/reserved + explicitly. +- Evidence: the strict schema accepts `artifact` and `path`, and configuration + documents both as optional passthrough adapter fields. Graph-augmented exact + searches find no read of `inputCfg.Artifact` or `inputCfg.Path`. Resolution + consumes only `Source` and `Required`; execution sends only a map from input + name to resolved filesystem path. Neither Scriptorium request type nor its + CLI/generated invocation representation has fields for the accepted values. +- Realistic scenario: an operator sets either field based on the maintained + configuration reference to select an upstream adapter artifact/path. Config + loading and validation succeed, but the invocation is identical to one where + the fields were absent, with no warning that intent was lost. +- Impact/likelihood/confidence: medium configuration-trust impact; low-to- + moderate use likelihood because no maintained example uses the fields; high + confidence that current values are inert, with intended upstream semantics + intentionally left unresolved. +- Estimated remediation scope and owner: product/config-and-adapter decision. + Define and implement exact Scriptorium wire semantics if the feature is + supported; otherwise remove the fields from schema/documentation or reject + non-empty values with a migration message. Do not guess how `artifact` and + `path` combine with canonical `source` resolution. +- Test changes: once authority is chosen, assert end-to-end adapter invocation + semantics or strict rejection. A decode-only test is insufficient. +- Dependencies: `COM-005` owns other analyze documentation gaps. `ARC-007` + concerns selection, not these inert per-input fields. + ### `RSK-001`: invocation audit records can remain indefinitely `running` - Category: confirmed correctness/operational risk. @@ -2391,6 +2681,37 @@ but an external multi-gigabyte JSON lane can still exhaust memory. - Dependencies: snapshot/generation authority remains `COR-008`; this finding concerns completion of one list operation only. +### `RSK-015`: analyze dependency preflight chooses errors nondeterministically + +- Category: confirmed determinism/operability risk. +- Locations/invariant: `internal/stage/analyze.go` in + `orderSelectedScriptoriumArtifacts`. Given one configuration, selection, and + filesystem state, dependency validation should return one stable diagnostic + before any adapter or output mutation. +- Evidence: selected names are copied into a map, then the initial unknown/ + unavailable dependency preflight ranges directly over that map and returns + the first failure. Go map order is unspecified. When two selected artifacts + each have an unavailable unselected dependency, either artifact can own the + returned error across executions. Later graph construction also ranges over + maps, but it sorts edges and ready nodes before observable successful order; + the nondeterminism is confined to preflight diagnostics. +- Realistic scenario: a configuration deploy omits several reused dependency + files. Repeated CI or operator invocations report different first blockers, + making logs, snapshots, and one-at-a-time recovery unstable even though no + external state changed. +- Impact/likelihood/confidence: low execution-integrity impact but moderate + diagnostic/reproducibility impact; moderate multi-error likelihood; high + confidence from explicit map iteration before first return. +- Estimated remediation scope and owner: tiny analyze ordering change. Iterate + the already sorted selected slice or sort map keys before preflight. Preserve + lexical successful topological ordering; do not introduce a generic graph + package solely for this repair. +- Test changes: construct at least two independently unavailable dependencies + from intentionally shuffled insertion order and assert one stable exact + error across repetitions. +- Dependencies: `SIM-003` may provide an indexed plan, but this correction does + not depend on structural refactoring. + ### `EFF-001`: restore repeatedly downloads the same objects during planning and execution - Category: confirmed efficiency and clarity issue. @@ -2518,9 +2839,9 @@ register above. interface as authoritative and make incorrect dependency or readiness decisions. Current likelihood appears low because the method has no production caller. -- Confirmation owners: Stages 8-9 completed ordinary/extraction rows; Stage 10 - owns analyze, then Stage 11 owns interface purpose/simplification. - Smallest plausible outcome may be clearer +- Confirmation owners: Stages 8-10 completed ordinary, extraction, and analyze + rows. Stage 11 owns interface purpose/simplification. Smallest plausible + outcome may be clearer naming/documentation, a complete contract, or removal; do not choose yet. ### `ARC-002`: disabled-stage “skip” terminology spans two different durable outcomes @@ -2540,8 +2861,11 @@ register above. cleanup gate, while `COM-002` owns its incorrect “self-skip” documentation. Stage 8 confirmed disabled trim is real successful copy processing and disabled render is deliberately successful with no output; `COM-004` owns - render wording. Stage 10 must decide analyze's remaining no-op behavior before - this architectural candidate can close. + render wording. Stage 10 confirmed absent/no-executable analyze is likewise + successful with no output so the pipeline can continue and publish's + prerequisite can be satisfied; `COM-005` owns its wording. Stage 11 can now + settle shared lifecycle vocabulary without changing those behaviors by + accident. ### `ARC-003`: committed and local manifests give `current_pointer_written` different meanings @@ -2607,6 +2931,33 @@ register above. add one contract test across affected fakes/adapters rather than duplicating every stage case. +### `ARC-007`: enabled and selected artifact authority is split across boundaries + +- Category: architectural boundary/ownership candidate. +- Evidence: catalog registration makes `enabled` authoritative only when no + explicit selection exists; a non-empty selection replaces it, and a unit test + requires a selected disabled artifact to become executable. CLI validation + accepts that choice. Configuration requires prompt/output fields and checks + cycles only for enabled artifacts, previous-requirement collection scans only + enabled artifacts, while `docs/internal/artifacts.md` defines executable as + both selected and enabled. Publish uses selection as a filter over configured + rules but availability ignores enabled state. +- Contract tension: `enabled` can mean default execution, complete executable + configuration, prerequisite participation, or publication availability, + depending on the boundary. `selected` can mean an override or a filter. No + single effective-artifact-set owner states which interpretation is canonical. +- Realistic risk: `COR-022` is the concrete prior-input failure. A selected + disabled artifact can also reach runtime with fields that configuration did + not require, while an enabled-but-unselected artifact is reported with + disabled-output provenance. Future validation/publish changes can widen the + disagreement. +- Confirmation owner: Stage 11 should choose and name one rule. If selection is + a one-off override, compute a typed effective set before configuration- + dependent composition/planning and validate selected entries as executable. + If it is only a filter, reject selected disabled entries and intersect the + sets everywhere. Preserve publish's documented rule that built-in/extraction + sources are unaffected, and do not overload availability with executability. + ### `TST-001`: full race baseline fails in the concurrent transcribe test - Category: test-suite execution candidate. @@ -2776,6 +3127,28 @@ register above. contract/provenance checks, no-follow regular files, checksums, and catalog's JSON validity. Do not move lifecycle decisions into `fileops`. +### `DUP-008`: runtime catalog bootstrap policy is repeated across three consumers + +- Category: duplication/ownership candidate. +- Evidence: `buildAnalyzeRuntimeArtifactCatalog`, + `buildPublishRuntimeArtifactCatalog`, and `buildHelperArtifactCatalog` each + create a catalog, register built-ins, translate Scriptorium configuration to + configured definitions, register extraction definitions, and conditionally + hydrate extraction evidence. Analyze and publish additionally repeat local + configured-output path resolution/availability loops. Their final policies + intentionally differ: analyze applies executable selection and reuses only + non-executable outputs; publish needs every existing configured source before + applying output selection; operator helpers primarily render identities. +- Realistic risk: a new built-in/source family, extraction evidence rule, or + configured-definition field is registered in one consumer and omitted from + another. Analyze can then accept a source that publish/status cannot render, + or publication can expose availability analyze classifies differently. +- Confirmation owner: Stage 11 should extract only common deterministic + registration/definition bootstrap, returning a catalog that callers enrich + with explicit availability/executability policy. Coordinate extraction proof + mechanics with `DUP-007` but do not collapse resume, analyze reuse, publish + selection, and operator rendering into one mode-heavy builder. + ### `TST-003`: filesystem safety tests omit destination and lock-recovery boundaries - Category: test-suite sufficiency candidate. @@ -2921,6 +3294,31 @@ register above. end-to-end framework; coordinate manifest-persistence faults with `TST-002` and destination ancestry with `TST-003`/`COR-003`. +### `TST-010`: analyze tests omit effective-selection and optional built-in boundaries + +- Category: test-suite sufficiency candidate. +- Evidence: analyze, catalog, artifact-policy, configuration, previous- + requirement, publish, and command tests strongly cover ordinary source + success, required/optional absence for four source families, generated/reused + dependencies, successful lexical order, cycles, unavailable dependencies, + selection propagation, local-only previous resolution, and publish filtering. + They do not cover missing optional normalized/trimmed/Markdown built-ins, + execution of a selected disabled artifact, prerequisite planning for that + effective selection, multiple simultaneous dependency errors, or any + behavioral use/rejection of accepted input `artifact`/`path` fields. The + previous-guidance test asserts only the malformed command fragment. +- Realistic risk: fixes for `COR-021` through `COR-024`, `RSK-015`, and + `ARC-007` can be partial while each narrow package suite stays green. A broad + end-to-end matrix would duplicate strong catalog/source tests and obscure + which boundary owns a failure. +- Confirmation owner: Stage 12 should add a built-in required/optional table at + analyze, one assembled selected-disabled previous-requirement workflow at app + or prepare/analyze composition, one deterministic multi-error dependency + case, exact actionable guidance, and either adapter propagation or strict + rejection for the passthrough fields after their contract is chosen. Reuse + existing fixtures for publish/source success and do not retest every + artifact-policy spelling. + ### `SIM-001`: runner terminalization and persistence ordering lack a narrow owner - Category: simplification candidate. @@ -2957,6 +3355,29 @@ register above. non-resumable outcome and every unsafe error, and keep `ValidateResume` as the lifecycle mapper rather than introducing a generic validation framework. +### `SIM-003`: analyze execution passes a wide context through repeated resolution branches + +- Category: simplification candidate. +- Evidence: `executeAnalyzeArtifact` spans 239 lines and accepts ten parameters + for environment, manifest, session paths/identity, run layout, plan, + transcript diagnostics, and mutable catalog state. `resolveScriptoriumInput` + adds six parameters and 84 lines/cognitive complexity 71; `inputName` and + `sessionDir` are unused there. It interleaves family dispatch, availability, + required/optional decisions, previous-cache provenance, and operator guidance. + `orderSelectedScriptoriumArtifacts` separately rebuilds selected sets and + graph indexes. `resolveInputPathForRead` has no caller. +- Realistic risk: another source family or guidance rule can bypass optional + semantics as in `COR-021`, while new per-run context widens signatures and + makes test fixtures construct states that production never uses. A generic + resolver abstraction would create the opposite risk by erasing intentionally + different source policies. +- Confirmation owner: Stage 11 should consider a small typed analyze execution + context, an indexed effective plan, and a resolution result carrying path, + optional absence, provenance, and typed unavailability. Keep policy dispatch + explicit and source-specific guidance close to its producer. Remove the dead + helper and unused parameters if confirmed; do not create a generic stage or + graph framework, and coordinate set authority with `ARC-007`. + ### `COM-001`: dual-ledger save order and partial-failure policy lack rationale - Category: comment/clarity candidate. @@ -3020,6 +3441,31 @@ register above. vocabulary shared with `COM-002`; no production behavior change is indicated by Stage 8. +### `COM-005`: analyze documentation omits one source family and durable no-op consequences + +- Category: documentation/clarity candidate. +- Evidence: `docs/internal/stage-analyze.md` lists built-in, prepared, + configured, and previous-session sources but omits supported + `narratio.extraction.` inputs. It says missing configuration/no + executable artifacts “skips with metadata,” while the result has no skipped + disposition: both manifests record succeeded, publish's prerequisite is + satisfied, and ordinary later runs reuse that success until force. The same + document says the runtime catalog contains only built-ins and configured + artifacts despite extraction registration/hydration. Separately, + `docs/internal/artifacts.md` says executable means selected and enabled even + though current selection overrides enabled; that authority question remains + `ARC-007` rather than a wording-only repair. +- Realistic risk: operators omit usable extraction context or expect newly + configured artifacts to run without force; maintainers can change no-op + analyze into an explicit self-skip and break pipeline/publish behavior. The + dedicated `analyze` command force-runs, which mitigates but does not correct + the contract. +- Confirmation owner: Stage 11 documentation pass after `ARC-007` decides + enabled/selection language. Add extraction to catalog/source lists and state + successful no-output persistence, continuation, and force requirements using + the shared `ARC-002` lifecycle vocabulary. Do not describe the inert + `artifact`/`path` fields until `COR-024` chooses implement versus reject. + ## Candidate Classification Log | Candidate signal | Classification | Reason | @@ -3030,7 +3476,7 @@ register above. | Session `last_error` survives a later stage success | rejected as a current-state defect at Stage 2 | No production reader was found; current status and per-stage error are authoritative, so the field can serve as historical context. Reopen only if an operator surface treats it as the active error. | | Minimal loaded-manifest status/timestamp validation | rejected as a standalone Stage 2 finding | Unknown/non-succeeded statuses fail conservatively into execution, nil maps/records are normalized, and no realistic unsafe caller was established. Configured-versus-persisted identity conflict is separately confirmed in `COR-001`. | | Ignored runner lock-release error | confirmed as `RSK-003` at Stage 3 | The lock is an exclusive-create sentinel, not an OS-released lock. An unlink failure leaves the conflicting file while the runner suppresses the error; process death does the same without a release attempt. | -| Two durable meanings of “skip” | runner/publish/ordinary stages confirmed; analyze deferred | Run action/status distinguishes idempotent skip from executed self-skip. Publish and disabled render deliberately return successful no-output metadata (`COM-002`, `COM-004`); disabled trim performs a successful copy; extraction is the true self-skip. Stage 10 owns analyze's remaining no-op outcome before `ARC-002` closes. | +| Two durable meanings of “skip” | behavior confirmed; retained as `ARC-002` vocabulary candidate | Run action/status distinguishes idempotent skip from executed self-skip. Publish, disabled render, and absent/no-executable analyze deliberately return successful no-output metadata (`COM-002`, `COM-004`, `COM-005`); disabled trim performs a successful copy; extraction is the true self-skip. Stage 11 can settle language without assuming one behavior. | | Exported previous-artifact helper accepts traversal | consolidated into `COR-002` | Current production callers normalize first, but the helper's under-root contract is false in isolation. The identity/relative-segment boundary should be repaired once rather than as separate caller bugs. | | Built-in and previous resolvers do not re-hash manifest records | documented trust distinction; deferred to Stages 5 and 10 | Extraction explicitly requires checksum/contract/provenance validation and enforces it. Other source families explicitly use content validation and a previous-cache filesystem fallback; consumer/restore threat models must establish a stronger requirement before this becomes a finding. | | Promotion destination is path-based while source is handle-confined | consolidated into `COR-003` | Source hardening is strong, but destination ancestors share the same symlink/replacement root cause as ordinary writers and cleanup. | @@ -3074,6 +3520,13 @@ register above. | Every durable promoted bundle is advertised or reusable | rejected at Stage 9 | Promotion establishes immutable bytes, not success. A later sync/checksum/result-persistence failure may leave a uniquely named orphan bundle, but session-manifest success is advertisement authority and neither resume nor catalog scans incidental directories. | | Resume and catalog bundle checks are merely coincidental similarity | retained as `DUP-007` | They repeat one exact-set, identity, contract/provenance, path, type, and checksum policy. Their error mapping differs intentionally, so sharing must return structured evidence rather than collapse lifecycle and fail-closed consumer behavior. | | Extraction resume complexity justifies a generic validation framework | rejected; narrow simplification retained as `SIM-002` | The security proof and ordered failure classifications must remain visible. Named evidence decisions can reduce cognitive load without abstracting stage lifecycle or filesystem mechanics behind a generic framework. | +| Optional analyze inputs behave uniformly across source families | confirmed as `COR-021` | Final, final-trimmed, and both Markdown built-ins return hard producer-guidance errors before the caller can honor `required: false`; other missing optional families are omitted. | +| Explicit selection is only a filter over enabled artifacts | retained as `ARC-007`, with concrete mismatch `COR-022` | Catalog tests require selection to override enabled, while configuration, previous planning, and focused docs assume enabled remains authoritative. The product rule needs one owner; current prerequisite omission is already a defect either way. | +| Successful topological order implies deterministic dependency validation | confirmed as `RSK-015` | Edges and ready nodes are sorted for success, but the first unavailable-dependency preflight returns from unsorted selected-set map iteration. | +| Existing configured output needs a prior analyze success to be reusable | rejected as the current contract | Non-executable configured artifacts intentionally use canonical non-empty files without manifest provenance so disabled/operator-prepared dependencies can be reused. Freshness is operator-owned; misleading enabled-but-unselected provenance stays under `ARC-007`. | +| Previous-session resolution can fetch remotely during analyze | rejected at Stage 10 | Analyze uses only manifest-backed and filesystem `previous/` paths. Remote discovery/download belongs to restore/prepare composition, and a boundary test proves the object store is not called. Selection can omit that earlier planning under `COR-022`. | +| Input `artifact` and `path` are adapter passthrough fields | confirmed as `COR-024` | Strict config accepts and documents them, but no production read, request field, CLI argument, or generated invocation field exists; values are silently discarded. | +| Repeated analyze resolution branches justify a generic resolver framework | rejected; narrow simplification retained as `SIM-003` | Source families deliberately differ in authority, provenance, optional absence, and repair guidance. A typed context/result and indexed plan can reduce width without erasing those policies. | ## Unresolved Questions And Follow-Up @@ -3092,11 +3545,12 @@ register above. - Do deployment packaging and service definitions already enforce an umask, ownership, or ACL that reduces `RSK-004`, and where should that contract be documented and tested? -- Is analyze's missing/no-executable-artifact outcome intentionally successful - so pipeline continuation works, and do all operator views describe that - distinction accurately? Publish and render are now resolved as successful - no-output outcomes with documentation gaps; disabled trim is successful copy - processing. +- Should explicit artifact selection override `enabled` for one-off execution, + or filter only enabled artifacts? `ARC-007` must choose one rule before + `COR-022` and related validation/documentation are repaired. +- What exact Scriptorium wire semantics, if any, were intended for input-level + `artifact` and `path`? Until `COR-024` is answered, non-empty values must not + continue to be accepted as meaningful passthrough configuration. - Is `IODecl` intended only for display/tests, or should it own enforceable dependency declarations? - Should remote lock activation be snapshot-based or immediate, and what @@ -3464,7 +3918,7 @@ No accepted risks or final audit conclusions are recorded yet. justified. Current production transformation adapters return their requested output paths, so inconsistent result-path authority remains architectural, not a present production data defect. -- Later-stage assignments: Stage 10 owns analyze's remaining `ARC-002` outcome. +- Later-stage assignments: Stage 10 later resolved analyze's `ARC-002` outcome. Stage 11 owns `ARC-001`, `ARC-005`, `ARC-006`, `DUP-006`, `COM-004`, and the narrow shared-owner decisions. Stage 12 owns the risk-based additions in `TST-001`, `TST-007`, and `TST-008`. @@ -3511,7 +3965,58 @@ No accepted risks or final audit conclusions are recorded yet. uploaded; disabled extraction self-skips coherently; process/receipt/ prepromotion failures advertise no result; a generic validation or stage framework is not justified. -- Later-stage assignments: Stage 10 owns the full analyze source-family and - publish-selection matrix. Stage 11 owns `ARC-001`, `DUP-007`, and `SIM-002`. +- Later-stage assignments: Stage 10 later completed the analyze source-family + and publish-selection matrix. Stage 11 owns `ARC-001`, `DUP-007`, and `SIM-002`. Stage 12 owns the risk-based additions/consolidation in `TST-009` and the extraction reach of `RSK-013` alongside `TST-002`, `TST-003`, and `TST-007`. + +### Stage 10 + +- Entry revision: `083decc` (`Document extraction audit findings`). Commits + since the pinned audit revision modify audit documentation only, so + implementation/test evidence remains pinned to the identity recorded above. +- Contracts and code reviewed: development guide, exact Stage 10 sequence, all + repository policy, overview, analyze/artifacts/publish/manifest internal + documents, Scriptorium integration, configuration, CLI, operations, and + troubleshooting; complete production flow through config validation, + selection propagation, previous-requirement consumers, artifact policy and + catalog/resolvers, analyze planning/execution/materialization, Scriptorium + adapter requests, lifecycle persistence, and publish source filtering. +- Graph/source evidence: scoped stage architecture and hotspot inventory; + searches, snippets, and call/data-flow traces for source classification, + catalog registration/availability, configured selection, previous + requirements, dependency ordering, execution, resolution, and publish/helper + catalog consumers; followed by exact production/test inspection. The main + execution helper measured 239 lines/ten parameters, source resolution 84 + lines/cognitive complexity 71, and dependency ordering 86 lines/ten loops; + these were treated as review signals only after behavior was traced. +- Validation: the exact required command, `go test -count=1 ./internal/stage + ./internal/artifacts ./internal/artifactpolicy ./internal/config + ./internal/adapters/scriptorium ./internal/app`, passed all six packages. + The command completed in 2.01 s wall time. + No live Scriptorium process, credential, remote, destructive, or paid + operation was exercised. +- Contract conclusions: registered, executable, available, generated, and + reused states are distinct. Selection currently overrides enabled state; + non-executable configured files can be reused; successful order is lexical + and dependency-correct; generated outputs become immediately available to + later plans. All source-policy families were traced, and previous-session + consumption is local-only. Publish selection filters configured-source rules + only and does not trigger analyze or suppress built-in/extraction rules. + Missing/no-executable analyze is durable successful no-output execution. +- Findings: confirmed `COR-021`, `COR-022`, `COR-023`, `COR-024`, and + `RSK-015`; added `ARC-007`, `DUP-008`, `SIM-003`, `COM-005`, and `TST-010`; + refined `ARC-001`, `ARC-002`, and `RSK-013`; and resolved scenario 9. +- Explicit no-finding conclusions: unknown source/dependency identities and + enabled cycles are configuration errors; explicit selected cycles are caught + at runtime; successful plan/output/metadata order is deterministic; a missing + unselected dependency cannot be silently executed; configured disk reuse is + an intentional non-manifest freshness contract; extraction hydration remains + fail-closed; previous resolution performs no remote call; publication cannot + execute artifacts; partial failed analyze output is not advertised as stage + success; no material dependency-order efficiency defect or generic resolver/ + graph framework is justified. +- Later-stage assignments: Stage 11 owns `ARC-001`, `ARC-002`, `ARC-007`, + `DUP-008`, `SIM-003`, and `COM-005`, coordinated with earlier candidates. + Stage 12 owns the risk-based additions/consolidation in `TST-010` and the + analyze reach of `RSK-013` alongside existing safe-output test candidates.