Files
notarius/docs/roadmap/audit.md

1124 lines
75 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Codebase Audit
## Audit Metadata
- **Production target:** `92e89076a268089e703978fb9d7176200e93344c`
- **Branch at target:** `main`
- **Audit date:** 2026-08-08
- **Go version:** `go1.26.5 linux/amd64`
- **PromptKit version:** `gitea.maximumdirect.net/eric/promptkit v0.5.0`
- **Knowledge-graph project:** `notarius-audit-92e8907`
- **Knowledge-graph target:** branch `main`, head
`92e89076a268089e703978fb9d7176200e93344c`
- **Initial worktree:** Clean. There were no pre-existing production or roadmap
changes to record.
The commit above is the production snapshot under audit. Later commits that
change only roadmap audit documents do not change that production target.
## Executive Summary
Pending final synthesis. The initial baseline is healthy. The architecture,
configuration/CLI, pipeline composition, reference/handoff, runtime, and state
reviews have found two High findings, four Medium findings, and ten Low
findings, with no production dependency inversion, unbounded framework worker
pool, completion-order-dependent result assembly, or debug-to-cache coupling.
## Finding Index
Final cross-area ordering is pending synthesis.
| ID | Severity | Category | Title |
| --- | --- | --- | --- |
| ARCH-001 | Low | Documentation/Comments | Repair broken ADR cross-references |
| CFGCLI-001 | Medium | Correctness | Reject additional YAML documents |
| CFGCLI-002 | Low | Correctness | Reject a blank command-level LLM profile |
| PIPE-001 | Low | Correctness | Reject normalized module-reference collisions in the resolver |
| PIPE-002 | Low | Efficiency | Clone construction inputs once per builder boundary |
| REF-001 | Medium | Efficiency | Bound reference reads before allocating the file |
| REF-002 | Low | Efficiency | Index accepted outputs once per ordered handoff |
| REF-003 | Low | Correctness | Include canonical size in generated-reference fingerprints |
| RUN-001 | High | Correctness | Check cancellation at unguarded dispatch boundaries |
| RUN-002 | High | Correctness | Isolate typed validator values from stage output |
| RUN-003 | Medium | Correctness | Preserve warnings from the terminal rejected attempt |
| RUN-004 | Low | Documentation/Comments | Document the lane collector's liveness invariant |
| STATE-001 | Medium | Correctness | Preserve distinct identities in state paths |
| STATE-002 | Low | Efficiency | Remove redundant post-decode clones from canonical codecs |
| STATE-003 | Low | Duplication | Publish output through the confined file writer |
## Findings
### Architecture And Dependency Boundaries
### ARCH-001 — Repair broken ADR cross-references
- **Severity:** Low
- **Category:** Documentation/Comments
- **Evidence:**
`docs/adr/0012-resolve-opaque-entity-identifiers-deterministically.md:15`
links ADR-0003 as `0003-strongly-typed-stage-interfaces.md`, and line 17
links ADR-0009 as
`0009-prefer-minimal-evidence-grounded-extraction-artifacts.md`. Neither file
exists. The maintained files are
`0003-typed-interfaces-with-two-zone-data-model.md` and
`0009-minimal-evidence-grounded-extraction-artifacts.md`.
- **Impact:** Readers and documentation tooling cannot follow ADR-0012 to the
two architectural decisions it explicitly relies on. Runtime behavior is
unaffected.
- **Recommendation:** Correct only the two relative link targets in ADR-0012.
- **Preserve:** Keep the accepted decision text and its intended references to
ADR-0003 and ADR-0009 unchanged.
- **Validation:** Run a relative Markdown-link check across `docs/adr/` and
confirm both targets resolve; verify the change contains no decision-text
edits.
- **Grouping:** Independent.
### Configuration And CLI Composition
### CFGCLI-001 — Reject additional YAML documents
- **Severity:** Medium
- **Category:** Correctness
- **Evidence:** `internal/core/config/file_config.go:327``353`
constructs a `yaml.Decoder`, enables `KnownFields`, and calls `Decode` only
once. `yaml.Decoder.Decode` reads the next YAML document, so input such as a
valid version 4 configuration followed by `---` and another configuration is
accepted with the second document ignored. The strict file tests in
`internal/core/config/file_config_contract_test.go` cover malformed values,
unknown fields, and duplicate normalized identifiers, but not an additional
document.
- **Impact:** An operator can append a syntactically valid configuration
document, receive a successful validation result, and then run with only the
first document. Settings in the ignored document—including operational or
pipeline settings—have no effect without a diagnostic, contradicting the
documented strict single-file model.
- **Recommendation:** After decoding `FileConfig`, decode once more and require
`io.EOF`; reject any second document, including an empty or malformed one,
with a contextual configuration error.
- **Preserve:** Keep version gating before the full strict decode, unknown-field
and duplicate-key rejection, and the existing defaults → file → environment
precedence unchanged.
- **Validation:** Add focused parser cases for a second valid document, a
second malformed document, and ordinary trailing whitespace/comments; run
`go test ./internal/core/config ./internal/cli`.
- **Grouping:** Independent.
### CFGCLI-002 — Reject a blank command-level LLM profile
- **Severity:** Low
- **Category:** Correctness
- **Evidence:** `internal/cli/run.go:149``166` registers `--llm-profile` as a
plain string flag, while the command's presence-aware empty-value checks
cover session ID, reasoning effort, output/debug directories, and recompute
step but not this flag. `runPipelineCommand` passes the resulting string to
`Config.Resolve`; `internal/framework/pipeline/profile.go:1255``1277` trims
an empty override and treats it as absent. The run contract tests cover a
valid override and an unknown non-empty profile, but not an explicitly
supplied blank value.
- **Impact:** A shell expansion such as `--llm-profile "$PROFILE"` with an
unset or blank value succeeds by silently using binding, pipeline, or
PromptKit defaults. The run can therefore use a different model/profile than
the operator explicitly intended to select.
- **Recommendation:** Make the CLI flag presence-aware and reject an explicitly
supplied empty or whitespace-only profile ID as command syntax before config
loading or physical-state allocation.
- **Preserve:** Keep a genuinely omitted override optional, trim non-empty IDs,
retain command → binding → pipeline → PromptKit precedence, and continue to
apply overrides only to selected LLM-backed bindings and validators.
- **Validation:** Add command-contract cases for blank and whitespace-only
values that assert exit status 2 and no state allocation, plus retain the
valid and unknown-profile run cases; run `go test ./internal/cli`.
- **Grouping:** Independent.
### Pipeline Resolution, Preparation, And Typed Registries
### PIPE-001 — Reject normalized module-reference collisions in the resolver
- **Severity:** Low
- **Category:** Correctness
- **Evidence:** `internal/framework/pipeline/profile.go:1239``1252` sends every
module binding's reference map through `normalizeReferenceMap`. That helper,
at lines 13611377, trims each key but overwrites
`rawByNormalized[trimmedKey]` without checking whether another raw key already
produced the same identity. A programmatic binding containing both `slot`
and ` slot ` therefore retains whichever raw key is visited last by Go's map
iteration, then silently emits only one resolved binding. The later strict
reference resolver sees only the collapsed map and cannot diagnose the
collision. `internal/core/config/validation.go:256``266` correctly rejects
this shape for validated file/config flows, but direct `ResolvePipeline`
callers do not pass through that owner and there is no focused framework
regression case.
- **Impact:** A programmatically assembled profile can resolve successfully to
different external paths or generated selectors across processes from the
same ambiguous input. The normal CLI configuration path is protected by
upstream validation, which limits current production exposure, but the
resolver's own contract is nondeterministic.
- **Recommendation:** Make binding reference normalization return an error for
empty or duplicate trimmed keys before constructing the normalized map, and
propagate stage/lane context through `resolveBinding` callers. Avoid relying
on the config layer to make the framework resolver deterministic.
- **Preserve:** Keep whitespace normalization, exact external-versus-generated
source validation, sorted resolved bindings, local-over-pipeline precedence,
and the config layer's earlier contextual diagnostics.
- **Validation:** Add focused `ResolvePipeline` cases for whitespace-equivalent
chunk, extract, merge, and normalize reference keys, including different
source forms, and assert deterministic contextual rejection; run
`go test ./internal/framework/pipeline ./internal/core/config`.
- **Grouping:** Independent.
### PIPE-002 — Clone construction inputs once per builder boundary
- **Severity:** Low
- **Category:** Efficiency
- **Evidence:** `Prepare` and `prepareLane` clone binding option maps while
forming requests (`internal/framework/pipeline/prepare.go:102``104` and
202206), and `prepareValidatorChain` does the same at lines 258263.
Registry/build boundaries then clone the complete request again. Typed
extractors, mergers, normalizers, and validators add another clone inside
their registered erased-builder adapters
(`extractor_registry.go:56`, `merger_registry.go:68``74`,
`normalizer_registry.go:63``69`, and `validator_registry.go:101``108`),
after `buildErasedModule` or `buildPreparedValidator` already called
`cloneBuildRequest` (`prepare.go:272``299` and 325327). Each request clone
deep-copies materialized reference content as well as options, so typed
builders receive two reference copies and as many as three option copies;
untyped stage and validator builders use fewer copies.
- **Impact:** Every preparation repeats allocation and byte copying for bounded
external references and nested options, with the highest cost and a
different ownership path specifically for typed lanes and validators. The
work is run-construction-time rather than a concurrent operation hot path,
so the issue is low severity.
- **Recommendation:** Designate one private construction invocation as the
ownership boundary and clone the complete `BuildRequest` exactly there.
Store raw builders or remove the caller-side clone consistently so all stage
and validator registry variants follow the same single-copy rule.
- **Preserve:** Builders must continue to receive independently owned options,
reference maps, slot slices, metadata, and content bytes; preparation must
retain its own immutable resolved/reference state; nil, key/name, execution
class, and exact artifact-type checks must remain contextual errors.
- **Validation:** Extend construction hooks to mutate nested options and
reference bytes for typed and untyped modules/validators, assert no aliasing
with resolved or sibling requests, and use allocation/byte-copy observations
or a focused benchmark to confirm a single defensive copy; run
`go test ./internal/framework/contracts ./internal/framework/pipeline`.
- **Grouping:** Independent.
### References And Ordered Handoffs
### REF-001 — Bound reference reads before allocating the file
- **Severity:** Medium
- **Category:** Efficiency
- **Evidence:** `internal/framework/pipeline/references.go:88``164` implements
the external-reference materialization boundary. At lines 128146,
`materializeReferenceTarget` calls `os.ReadFile(path)` before comparing the
resulting allocation with `ReferenceSlot.MaxBytes`. The focused
`TestMaterializeReferencesEnforcesMaxBytes` case uses a nine-byte file and
verifies the post-read diagnostic, but does not prove that reads are bounded
by the declared three-byte limit.
- **Impact:** A mistakenly selected very large file, growing file, device, or
named pipe can consume memory far beyond the slot's advertised bound before
the framework rejects it. CLI reference overrides expose the same path, so a
local operator error can terminate the process instead of producing the
intended bounded validation failure.
- **Recommendation:** Open the path and read through a limit of
`MaxBytes + 1` when a positive maximum is declared, rejecting an extra byte
before retaining or cloning content. A regular-file size precheck may improve
diagnostics, but the bounded reader must remain authoritative for changing or
non-regular inputs. Preserve the existing unbounded behavior only for slots
that explicitly declare no maximum.
- **Preserve:** Keep config-relative versus working-directory-relative path
resolution, UTF-8 and media-type validation, empty-file warnings, canonical
digest/size/origin metadata, contextual errors without content, and owned
reference bytes.
- **Validation:** Add a reader or file fixture that proves no more than
`MaxBytes + 1` bytes are consumed, including a non-regular or growing-input
case, while retaining the current UTF-8, media, empty, and ordinary oversize
diagnostics; run `go test ./internal/framework/pipeline ./internal/cli`.
- **Grouping:** Independent.
### REF-002 — Index accepted outputs once per ordered handoff
- **Severity:** Low
- **Category:** Efficiency
- **Evidence:** `buildStepReferenceSets` walks every generated binding in the
receiving step (`internal/framework/pipeline/handoff.go:37``89`). For each
previously unseen producer, `generatedReferenceItem` allocates a `matches`
slice and scans the complete cumulative `outputs` slice to find that
step/lane (`handoff.go:104``133`). Its cache avoids rescanning when several
targets fan out from the same producer, but a step consuming `P` distinct
producers from `O` earlier outputs still performs `P * O` comparisons and up
to `P` temporary allocations.
- **Impact:** Ordered pipelines with many distinct generated dependencies pay
quadratic handoff preparation work before the consumer step can start. The
current production profiles are small and the scan is outside the lane
worker hot path, which limits present impact.
- **Recommendation:** Build one map from normalized `(step ID, lane ID)` to an
explicit zero/one/many accepted-output state at the start of
`buildStepReferenceSets`, then let `generatedReferenceItem` perform a direct
lookup. Keep ambiguity as data in the index so duplicate producer outputs are
still rejected rather than overwritten.
- **Preserve:** Retain exact accepted-output cardinality, codec
decode/re-encode canonicalization, producer lookup, complete schema/media
validation, per-target byte ownership, deterministic contextual errors, and
the rule that no consumer lane starts after a failed handoff.
- **Validation:** Add a many-producer/fanout case that retains missing and
duplicate rejection, then use a focused benchmark or comparison counter to
demonstrate one output-index pass plus direct producer lookups; run
`go test ./internal/framework/pipeline`.
- **Grouping:** Independent; do not combine with PIPE-002, which concerns
construction-request copying rather than runtime producer lookup.
### REF-003 — Include canonical size in generated-reference fingerprints
- **Severity:** Low
- **Category:** Correctness
- **Evidence:** The generated `ReferenceItem` records canonical content length
in `SizeBytes` (`internal/framework/pipeline/handoff.go:154``177`), and the
manifest provenance also retains that value at lines 210232. However,
`generatedReferenceFingerprintIdentity` and
`generatedReferenceDependencies` (`handoff.go:234``287`) hash producer,
kind, complete schema identity, media type, and content digest without the
canonical size. This differs from the documented resume contract in
`docs/internal/state.md:49``54`. The focused fingerprint test changes only
canonical content and does not assert size participation.
- **Impact:** Consumer checkpoint identity does not cover one field that the
handoff and manifest declare part of canonical reference identity. Current
construction derives size directly from canonical bytes, so a practical
stale reuse also requires malformed internal metadata, a digest collision,
or a future producer-path change; the immediate risk is therefore low.
- **Recommendation:** Add `SizeBytes` to the private fingerprint identity and
populate it from the canonical generated item before JSON hashing. Keep the
existing normalized fingerprint name and all current identity fields.
- **Preserve:** Do not weaken the content digest, producer provenance,
artifact kind, complete schema digest/fields, media type, or canonical codec
checks. Continue to keep content bytes out of checkpoints, manifests, debug
summaries, and errors.
- **Validation:** Add a direct dependency-fingerprint case that holds the
other identity fields constant while changing size metadata, retain the
canonical-content sensitivity case, and run
`go test ./internal/framework/pipeline ./internal/modules/integration/...`.
- **Grouping:** Independent.
### Execution, Validation, Retry, And Concurrency
### RUN-001 — Check cancellation at unguarded dispatch boundaries
- **Severity:** High
- **Category:** Correctness
- **Evidence:** `Runner.Run` invokes the input adapter at
`internal/framework/pipeline/runner.go:159``177` without checking
`ctx.Err()` first; the first framework-owned cancellation check on that path
is inside `runWithRetry` at lines 389418, after source work and its
checkpoint/debug side effects. At the other end of the run, ordered lane work
completes at lines 278282, then evidence/debug assembly and
`encoder.Encode` run at lines 297346 without another cancellation check.
The worker and retry paths do check cancellation, but
`TestRunnerReturnsParentCancellationAndStopsQueuedExtracts` covers only
cancellation while two extract calls are active
(`runner_concurrency_test.go:559``585`). A context-ignoring input or output
module can therefore be called with an already-cancelled context; in the
output case it can return files and make the cancelled run report success.
- **Impact:** Work can start after cancellation, and a cancellation arriving
after lane completion or during output debug assembly can still publish a
successful logical output. This violates the documented guarantee that
parent cancellation prevents queued work and output encoding, and it leaves
correctness dependent on every module independently honoring an already
cancelled context.
- **Recommendation:** Add framework-owned cancellation gates at run entry and
immediately before every module dispatch not already protected by
`runWithRetry` or a lane worker, especially input parsing and output encoding.
Recheck after intervening collaborator/debug/evidence work so a cancellation
cannot slip between the gate and the operation, and after an unguarded module
returns so a module that did not observe mid-call cancellation cannot publish
success. Return the parent context error through the existing failed-output
path.
- **Preserve:** Continue to pass the caller context into active operations,
wait for all started lane work, suppress output files on framework failure,
select an actual framework error deterministically when the parent remains
live, and prefer the parent cancellation when it is set.
- **Validation:** Add a pre-cancelled run whose input adapter ignores context
and must not be called, plus a run whose debug recorder cancels immediately
before output dispatch while a context-ignoring encoder records calls, and an
encoder that cancels mid-call but returns files. Assert `context.Canceled`, no
invocation after pre-dispatch cancellation, and no output files in either
output case; retain the active-extract cancellation case and run the package
under `-race`.
- **Grouping:** Independent.
### RUN-002 — Isolate typed validator values from stage output
- **Severity:** High
- **Category:** Correctness
- **Evidence:** Extract, merge, and normalize serialize a canonical candidate
before validation, but pass the operation's original typed value alongside
it; representative extract code is
`internal/framework/pipeline/runner_concurrent.go:424``439`, and merge and
normalize do the same in `runner_typed.go:264``277` and 359392.
`validateTypedArtifact` clones source input, references, metadata, and chunks,
but `requestTarget := target` leaves `target.value` shallow-copied
(`runner_typed.go:458``499`). The registered typed adapter then exact-casts
that value and passes it directly to the validator
(`validator_registry.go:109``119`). Struct values containing slices, maps,
or pointers therefore retain aliases to the stage output. A validator can
mutate what later validators and final checkpoint serialization observe,
while serialized validators and attempt debug continue to describe the
pre-mutation canonical candidate. Candidate-encoding tests verify encode
counts and rejection-before-checkpoint behavior, but no focused test mutates
a typed validator request.
- **Impact:** A buggy validator can corrupt an accepted output, cause validators
in one chain to inspect different values, or make persisted output differ
from the candidate that was serialized for validation and debug. This breaks
the documented immutable whole-output validation contract at all three typed
stages.
- **Recommendation:** Treat the already serialized canonical candidate as the
clone boundary. Decode a fresh exact typed value through the active codec for
each typed validator, or provide an equivalent codec-backed deep clone, and
never expose the stage operation's retained value. Keep serialized validators
on independent schema/content copies of the same candidate.
- **Preserve:** Retain exact Go-type checks, one canonical candidate encode per
attempt, validator declaration order, isolated request metadata/references,
serialized-validator byte ownership, contextual errors, and final encoding
only after the entire chain approves.
- **Validation:** Use an artifact with both slice and map fields. Have the first
typed validator mutate both, then assert that the next typed validator, a
serialized validator, attempt debug, and accepted extract/merge/normalize
output all observe the original canonical value. Run the focused package
tests under `-race` as well as normally.
- **Grouping:** Independent.
### RUN-003 — Preserve warnings from the terminal rejected attempt
- **Severity:** Medium
- **Category:** Correctness
- **Evidence:** The runtime contract says warnings from the final accepted or
rejected attempt are preserved (`docs/internal/pipeline.md:122``126`). Each
retry closure assembles per-attempt operation and validator warnings, but
stores them only on acceptance. Generated chunk-plan rejection returns at
`internal/framework/pipeline/runner_chunk_plan.go:142``150` before assigning
`result.warnings` at line 154, and `Runner.Run` promotes chunk warnings only
for acceptance or a cache hit (`runner.go:228``233`). Extract follows the
same pattern at `runner_concurrent.go:425``457`; `finalizeLaneExtract` skips
directly from a rejected result to the next chunk at lines 469476. Merge
and normalize collect `attemptWarnings` but return terminal rejections before
appending them to `RunOutput` (`runner_typed.go:264``301` and 359415).
`runWithRetry` returns only the last rejection, not its warnings
(`runner.go:389``428`). Existing retry-warning tests prove that discarded
attempts are not promoted, and final-rejection debug tests prove rejection
recording, but neither asserts terminal rejection warnings.
- **Impact:** Module warnings and warnings from validators that approved before
the rejecting validator disappear exactly when the final candidate is
rejected. Operators receive the rejection but lose diagnostics produced by
that terminal attempt, contrary to the documented output contract.
- **Recommendation:** Carry the warnings associated with the last rejection
through the retry result, then promote only that terminal attempt's warnings
at chunk, extract, merge, and normalize rejection handling. Keep warnings
from earlier failed or rejected attempts confined to attempt debug. Coordinate
any persistence-format addition with the later checkpoint/state audit rather
than silently changing state ownership here.
- **Preserve:** Do not promote warnings from attempts superseded by a later
retry, do not turn rejection into a framework error, keep deterministic
lane/chunk warning order, and retain accepted and reused checkpoint warning
behavior.
- **Validation:** Add table-driven chunk, extract, merge, and normalize cases
with distinct first-attempt and final-rejection warning scopes. Assert that
only the final scopes reach `RunOutput`, rejection attempt counts remain
correct, intermediate warnings remain in attempt debug, and shuffled/race
runs preserve order.
- **Grouping:** Independent.
### RUN-004 — Document the lane collector's liveness invariant
- **Severity:** Low
- **Category:** Documentation/Comments
- **Evidence:** `laneEngine.collect` coordinates a dynamically enabled
continuation send, extract-result closure, cancellation, and the
`pending`/`launched`/`completed` counters in
`internal/framework/pipeline/runner_concurrent.go:256``281`, with state
changes split across `handleExtractResult` and `handleCompletion` at lines
283313. The loop has no invariant-level comment. Its non-obvious purpose is
to discard only unlaunched continuations after cancellation while continuing
to drain extract results and exactly one completion from every launched
continuation before the continuation channel is closed and workers are
awaited.
- **Impact:** Current tests and the race run support the implementation, but a
future change to channel buffering, cancellation cleanup, or a counter can
introduce a deadlock or early close without the ownership/liveness rule being
visible at the maintenance point.
- **Recommendation:** Add one short comment immediately above
`laneEngine.collect` stating that it is the sole owner of pending/launch/
completion accounting and must drain closed extract results plus every
launched continuation completion even after cancellation. Do not narrate the
`select` cases.
- **Preserve:** Keep chunk-first/lane-second extract dispatch, bounded extract
and continuation pools, overlap between completed lanes and remaining
extracts, cancellation of undispatched work, and the final worker wait.
- **Validation:** Review the comment against the collector termination
predicate and retain the existing cancellation, reverse-completion,
continuation-overlap, worker-bound, race, and shuffled tests.
- **Grouping:** Independent.
### State, Checkpoints, Debugging, And File Safety
### STATE-001 — Preserve distinct identities in state paths
- **Severity:** Medium
- **Category:** Correctness
- **Evidence:** The resolver accepts any non-empty trimmed ordered-step and
lane IDs (`internal/framework/pipeline/profile.go:341``351` and
13821392). Checkpoint path construction then sends both IDs through
`checkpointPathComponent` (`internal/framework/checkpoint/recorder.go:476`
`508`), which maps `.`, `..`, and every value containing `..` to the single
component `_`. The trace side duplicates that encoder in
`debugPathComponent` (`internal/framework/pipeline/debug.go:37``62`), and
`cleanDebugPath` at lines 364374 additionally applies `path.Clean` before
encoding. Thus valid distinct identities such as `.` and `_`, or `a..b` and
`_`, select the same checkpoint component and can also collapse to the same
debug path. Existing filesystem tests cover traversal and symlink rejection,
but no focused test asserts that accepted identities map injectively.
- **Impact:** Two valid lanes or steps in one resolved pipeline can overwrite
each other's checkpoint manifests and payloads. Exact manifest validation
prevents a mismatched artifact from being silently reused, but ordinary
resume repeatedly invalidates and re-executes the colliding work, and a
required selective-recompute predecessor can become unavailable after its
sibling overwrites it. Requested debug records can likewise overwrite or be
attributed to the wrong identity. Atomic rename does not protect against a
logical-name collision.
- **Recommendation:** Introduce one shared, injective path-component encoding
for accepted application identities. Preserve already-safe components, but
encode reserved dot components and every unsafe rune rather than replacing a
whole value with `_`; do not run identity-bearing debug paths through a
lossy clean operation first. Keep path joining and confinement separate from
identity encoding.
- **Preserve:** Retain trimmed non-empty identity validation, human-readable
ordinary IDs, exact step/lane checks inside checkpoint manifests, narrow
relative state paths, symlink rejection, and recoverable cold execution for
incompatible reconstructible cache entries.
- **Validation:** Add table and uniqueness cases covering `.`, `..`, `_`,
embedded `..`, separators, tildes, and Unicode, then run a two-identity
filesystem checkpoint/resume and debug trace case that proves distinct files
and successful accepted-normalize hydration. Include selective recomputation
so a required predecessor remains reusable.
- **Grouping:** Independent.
### STATE-002 — Remove redundant post-decode clones from canonical codecs
- **Severity:** Low
- **Category:** Efficiency
- **Evidence:** `chunkmap.Codec.Decode` schema-validates and decodes JSON into a
fresh value, canonicalizes that value, then deep-clones it again before
returning (`internal/framework/chunkmap/codec.go:149``171`).
`evidencecontext.Codec.Decode` follows the same sequence
(`internal/framework/evidencecontext/codec.go:79``101`), while its
`canonicalize` already deep-clones the decoded document at lines 169203;
the final `clone` at lines 299320 therefore makes a second complete copy of
context/reference slices, source-unit structs, and nested metadata. Unlike an
encode caller, the JSON decoder retains no caller-owned object graph that
must be protected from canonicalization.
- **Impact:** Hydrating or consuming canonical artifacts performs avoidable
slice and nested-metadata allocations. The evidence-context path can clone a
large source-derived object graph twice after JSON decoding, increasing peak
allocation and latency without adding an ownership boundary.
- **Recommendation:** Separate canonicalization of an already-owned decoded
value from the defensive-copy entry point used by encoders and external
callers. Return the canonical decoded value directly once validation and
normalization succeed; retain exactly one clone wherever caller-owned input
could otherwise be mutated.
- **Preserve:** Keep schema validation, unknown-field and trailing-value
rejection, digest reconstruction, annotation/context normalization,
encode-time caller isolation, and a fully independently owned decode result.
- **Validation:** Retain and extend ownership tests that mutate both the input
value after encode and the returned value after decode, then use focused
allocation assertions or benchmarks for a nested chunk map and evidence
context to confirm that decode no longer performs the redundant deep copy.
- **Grouping:** Independent.
### STATE-003 — Publish output through the confined file writer
- **Severity:** Low
- **Category:** Duplication
- **Evidence:** The CLI's private `writeFileAtomic`
(`internal/cli/run.go:826``856`) independently implements the same
create-temp, write, chmod, close, rename, and cleanup sequence as
`internal/core/fileio.writeAtomic` (`internal/core/fileio/fileio.go:105`
`133`). `writeOutputFiles` calls the CLI copy after its own path checks and
directory creation (`internal/cli/run.go:754``787`), while
`fileio.WriteBytes` additionally centralizes relative-path confinement and
symlink-component rejection. Debug summaries, traces, and checkpoints
already use that core primitive; durable output is the divergent state
writer.
- **Impact:** Atomic-publication fixes must be made and tested in two places,
and the copies have already drifted in their confinement checks. The fresh,
exclusively created run directory limits current output exposure, so this is
a maintenance and defense-in-depth issue rather than an observed escape.
- **Recommendation:** After prevalidating all logical output names and
exclusively creating the run directory, publish each file through
`fileio.WriteBytes` (or a narrowly exported equivalent) using the run
directory as root. Remove the CLI atomic-write copy rather than introducing
another generic filesystem abstraction.
- **Preserve:** Validate every logical name before allocating state, refuse an
existing run directory unchanged, retain a newly created partial bundle on a
later write failure, keep output directory/file modes at 0755/0644, and
preserve logical-name context in errors.
- **Validation:** Retain the existing output ordering, collision, permission,
and partial-bundle cases; add a symlink-component case at the shared writer
boundary and verify no temporary file remains after write, chmod, close, or
rename failure.
- **Grouping:** Independent.
<!--
Finding template for later audit stages:
### AREA-001 — Concise title
- **Severity:** High, Medium, or Low
- **Category:** Correctness, Efficiency, Duplication, Simplicity, Test Quality,
or Documentation/Comments
- **Evidence:** Exact files, symbols, call paths, and observed behavior
- **Impact:** The concrete risk or cost
- **Recommendation:** A bounded implementation direction
- **Preserve:** Invariants and contracts that remediation must retain
- **Validation:** Focused checks that would demonstrate success
- **Grouping:** Independent, or the IDs with which this should be implemented
-->
## Intentional Complexity And Duplication To Preserve
- `internal/modules/generic/register.Register`,
`internal/modules/seriatim/register.Register`, and
`internal/modules/dnd/register.Register` deliberately expose the same small
registrar shape while retaining family-local registration policy and
diagnostics. Combining them would move extension ownership out of the domain
registrars and weaken the composition boundary established by ADR-0004.
- `internal/modules/dnd/register.registerModules`, `registerEvidence`,
`registerValidators`, and `registerDefaultChains` use explicit typed
registration lists. At this architectural pass, that repetition preserves
artifact Go types, module-specific validator order, and registrar-owned
production policy. Later D&D stages may evaluate individual shared
mechanics, but should not replace these lists with a dynamically typed
registration engine.
- `internal/framework/pipeline.RegisterArtifactCodec` and
`exactTypedValue` perform apparently repetitive exact-type checks around
private erasure. The checks deliberately turn incompatible values into
errors at each erased boundary rather than permitting a panic or accepting a
near-matching type, preserving ADR-0003.
- `internal/cli.runPipelineCommand` is a large linear orchestrator, but its
ordering is policy: syntax and config rejection precede run identity and
debug allocation; resolution and profile inspection precede module
preparation and input parsing; framework success precedes durable output;
and terminal debug publication precedes the optional JSON receipt. Existing
helpers isolate reference selection, recomputation, stores, output, result
encoding, and terminal error precedence. A generic lifecycle abstraction
would hide physical-state allocation and publication boundaries; bounded
parsing fixes such as CFGCLI-002 should not reorganize that lifecycle.
- `internal/core/config.validatePipelineProfiles` explicitly walks pipelines,
ordered steps, lanes, bindings, and references. Its nested structure mirrors
the public configuration shape and retains the nearest pipeline/step/lane
context in errors. Replacing it with a reflection-driven validator would
weaken those diagnostics and the presence-aware file-model boundary.
- `internal/cli.selectedReferenceTargets` and `recomputePolicy` perform
explicit resolved-shape traversals for distinct CLI policies: disambiguating
reference selectors against selected module capabilities, and computing the
forward forced/backward reusable checkpoint closure. Keeping these typed
traversals separate avoids adding command syntax or checkpoint policy to the
framework resolver.
- `pipeline.ResolvePipeline` and `resolveArtifactLane` are long, but their
linear sections retain the authoritative composition order: normalize
identity, select lanes, prove capabilities and exact artifact variants,
resolve stage-local references and validator chains, apply effective LLM
profiles, validate options, and only then compute the digest. Splitting these
checks into a generic stage engine would erase the different input, chunk,
typed-lane, validator, and output contracts. PIPE-001 is a bounded
normalization fix and should not reorganize this sequence.
- `pipeline.validateGeneratedBindings` has deeply nested traversal because it
proves a cross-step selector against ordered producer identity, the target's
declared slot, accepted artifact kinds, registered codec, and accepted media
types in one pass. Those checks are distinct static composition invariants;
materialized bytes, runtime handoff construction, and checkpoint hydration
remain separate runtime owners.
- The stage, validator, codec, and evidence registries intentionally use
private typed entries and small stage-specific lookup methods. Their
repetition preserves compile-time generic types until a narrow erased
closure, exact artifact-kind variant selection, and stage-specific
diagnostics. PIPE-002 concerns redundant request copies around those
closures, not the typed registry split itself.
- Generated-reference handoff deliberately decodes and re-encodes an accepted
normalized artifact through the registered producer codec even though fresh
outputs were already serialized. The same boundary also receives hydrated
checkpoint artifacts, so canonicalizing once per unique producer verifies
exact kind, schema, media type, and bytes before fanout. REF-002 removes only
repeated output discovery; it must not bypass this codec check.
- Operation paths clone reference sets for each module request while retaining
a separate pristine set for dependency fingerprints and validators. That
apparent duplication isolates module mutation across chunks and retries and
keeps validators on authoritative inputs. PIPE-002 applies only to adjacent
construction-builder copies and should not remove these operation-time
ownership boundaries.
- `Runner.Run` is a long linear coordinator, but its sequence is the runtime
contract: source state precedes chunk planning; a rejected chunk plan skips
lanes but remains an encodable ordinary outcome; ordered steps are barriers;
evidence is built only from accepted normalized artifacts; and logical output
encoding is last. RUN-001 requires narrow cancellation gates and should not
replace that readable lifecycle with a generic stage engine.
- Extract jobs and lane continuations deliberately use separate pools, each
bounded by the resolved worker count. This permits a completed lane to enter
its serial merge/normalize continuation while other lanes are still
extracting, without letting either class grow with lane or chunk count. The
two pools should not be collapsed merely to make the configured limit a
process-wide semaphore; provider calls have their own shared scheduler.
- Chunk, extract, merge, and normalize retry closures repeat candidate debug,
rejection, and warning mechanics around different typed operations and
checkpoint transitions. Their common retry counter and cancellation policy
appropriately live in `runWithRetry`, while stage-specific serialization,
normalize fallback, and state recording remain explicit. RUN-003 calls for a
small terminal-warning result, not a parameter-heavy generic stage runner.
- `synchronizedCheckpointRecorder`, `synchronizedDebugRecorder`, and
`synchronizedLLMDebugRecorder` intentionally serialize collaborators whose
contracts do not promise concurrent safety. Each adapter owns one lock and
invokes only its wrapped collaborator, so there is no framework lock-order
cycle; removing them would push concurrency requirements into filesystem and
test implementations.
- Filesystem checkpoint recorder and loader methods intentionally repeat the
stage-shaped interface surface. Each small adapter fixes a stage, manifest
status, dependency set, and payload codec before delegating to shared
manifest validation or publication. Collapsing them into a reflection- or
string-driven state engine would weaken typed call sites and contextual
validation; STATE-001 concerns only their shared path-component encoding.
- The chunk-plan store intentionally does not use the general `fileio` writer.
Its source-addressed directory is an independently replaceable cache entry,
and its `os.Root`-relative implementation rejects non-directory digest
entries, non-regular or symlinked plans, named pipes, and replacement races,
while syncing a private temporary file before rename. Those stronger
cache-entry mechanics should remain local rather than being generalized into
output/debug/checkpoint publication.
- Source metadata, accepted chunk maps, and evidence contexts retain separate
clone helpers because their ownership graphs and canonical invariants differ:
source metadata handles nested dynamic values and cycles, chunk maps own
annotations and chunk slices, and evidence contexts own source units and
metadata. STATE-002 removes only copies of an already-owned decoded graph; it
should not replace these helpers with reflection or remove encode-time
isolation.
## Areas Reviewed Without Findings
### Architecture And Dependency Boundaries
- **Composition root:** `internal/cli.newProductionComponents` constructs the
complete registry set and asset registry, then invokes only the generic,
Seriatim, and D&D family registrars. Direct production imports confirm that
`internal/cli` is the only layer importing those registrar packages.
- **Dependency direction:** A direct production import map found no core or
framework package importing `internal/modules`, no module importing
`internal/cli`, no concrete generic or Seriatim module importing D&D, and no
module importing the file-backed checkpoint, chunk-plan, debug, file-I/O, or
debug-bundle implementations. PromptKit is imported directly only by
`internal/framework/llm`.
- **Graph cross-layer calls:** The refreshed graph reported one
framework-to-module edge from `pipeline.Prepare` to a symbol named `request`
in a D&D validator test. Tracing it showed a confidence `0.06` suffix match
from the local closure call in `Prepare`; `trace_path` classified the target
as test-only, and the production import map disproved a dependency. The graph
reported no module-to-CLI calls.
- **Assets leaf:** `assets/package.go` imports only `embed` and `io/fs`, embeds
content, and exposes the read-only `FS() fs.FS` accessor. It contains no
business logic and has no `internal` or PromptKit dependency.
- **Fixed pipeline shape:** `pipeline.ResolvePipeline` resolves input and
chunk once, fixed extract/merge/normalize bindings per artifact lane, and one
output binding. Ordered steps are barriers around those fixed lanes rather
than arbitrary graph topology. `pipeline.Prepare`, `Runner.Run`,
`runPreparedSteps`, and `runLanes` retain that shape through construction and
execution.
- **Typed artifact boundary:** Typed registrations retain the exact Go type for
codecs and lane operations. Private erasure in `RegisterArtifactCodec` and
`exactTypedValue` verifies exact types and returns contextual errors;
normalized values cross into output through serialized artifacts.
- **Physical-state ownership:** The CLI owns root selection, store factories,
and durable file placement (`chunkPlanStoreForRun`, checkpoint/debug setup,
and `writeOutputFiles`). The framework receives collaborator interfaces and
returns logical output files. The generic JSON output module's direct import
of `internal/framework/chunkmap` validates and republishes the accepted
serialized chunk-map contract; it neither chooses a physical root nor writes
files.
- **Accepted architectural decisions:** ADRs 00010005 and 00070012 were read
against the current high-level composition. Apart from ARCH-001, the
composition root, fixed ordered pipeline, typed boundary, domain packaging,
canonical chunk-plan policy, separate state surfaces, checkpoint policy,
evidence rules, workload profile ownership, centralized asset leaf, and
deterministic entity identity boundary have corresponding current owners.
### Configuration And CLI Composition
- **End-to-end command path:** `RunWithOptions` normalizes injectable process
collaborators once and dispatches to `runPipelineCommand`. The run command
parses and normalizes command input, discovers and loads configuration,
applies command overrides, builds the effective catalog, resolves reference
selectors and the pipeline, inspects effective profiles, materializes
references, constructs runtime/state collaborators, invokes `pipeline.Run`,
publishes output files, terminalizes debug state, and only then publishes a
requested JSON receipt.
- **Precedence and resolution:** `loadConfig` enforces explicit `--config` over
`NOTARIUS_CONFIG` over the system default, then applies `Default`, file
configuration, supported environment overrides, and run-only CLI overrides
in order. `Config.Resolve` recomputes derived worker defaults, validates,
clones the selected profile, and delegates catalog-dependent composition to
the framework resolver. Apart from CFGCLI-001 and CFGCLI-002, unknown fields,
malformed values, normalized-key collisions, unknown command flags, and
invalid selected modules/options are rejected at their owning boundary.
- **Profile-source equality:** Validation-time
`validateExplicitPromptKitProfiles` and runtime
`buildProductionLLMClient` pass the same profile directory, profile file,
mapped local backend, and shared fallback asset registry. Effective profile
collection is sorted, deduplicated, and limited to selected LLM-backed
modules and validators, so inspection and runtime selection use the resolved
profile values rather than recomputing inheritance.
- **Session identity:** `resolvePromptSessionID` uses a versioned SHA-256 value
over the trimmed resolved input-module key, a separator, and exact raw input
bytes. It contains no pipeline ID, reference, profile, retry, input path,
working directory, or run ID; an explicit non-empty session replaces the
generated value. Run contracts verify the same effective session reaches all
prompt-facing requests, manifests, debug metadata, and checkpoint identity.
- **Reference and recomputation controls:** CLI reference selectors are
resolved only against selected chunk/extract/merge/normalize capabilities
before the authoritative effective resolution. Recompute policy is derived
after reference materialization, forces the requested step and transitive
consumers, and requires reusable checkpoints for non-forced transitive
producers. Focused contract tests exercise selector ambiguity, lane
selection, ordered dependency closure, and execution behavior.
- **Publication and terminal outcomes:** Syntax/config failures before run
identity allocate no output or debug state. After debug allocation,
resolution, profile, preparation, input, framework, partial-summary, and
output failures all pass through `failPipelineCommand`. `terminalize` writes
a run report once, preserves an existing primary failure over report/error-log
failures, promotes a success-report failure to primary, and reports other
persistence failures secondarily. Framework cancellation follows the same
wrapped primary-error path. Durable outputs are attempted only after runner
success; a JSON result is encoded before output publication but written to
stdout only after output and debug terminalization. A receipt-delivery
failure leaves already published bundles intact and returns failure.
- **Test ownership:** Configuration tests own strict file/env application,
structural validation, effective cloning/digests, and redaction. CLI command,
run, reference, recomputation, session, production, example, result, and state
contracts assert process-level ordering and side effects rather than merely
repeating lower-level resolver assertions. The two uncovered command/parser
cases are recorded as CFGCLI-001 and CFGCLI-002.
### Pipeline Resolution, Preparation, And Typed Registries
- **Static composition:** `ResolvePipeline` rejects mixed legacy/ordered
shapes, empty and duplicate normalized step/lane identities, invalid
invocation filtering, unknown modules, missing capabilities, incompatible
artifact variants, unsupported lane-level validators, invalid generated
selectors, and missing output capabilities before producing a resolved
value. Apart from PIPE-001's direct-call collision, selected lanes and steps
have stable sorted/order-preserving identities.
- **Effective policy and identity:** Execution classes come from normalized
registry specs. Command override → binding → pipeline LLM-profile precedence
applies only to selected LLM-backed modules and validators; deterministic
bindings reject explicit profiles. Module and validator option validators
receive owned maps before `resolvedPipelineDigest` hashes the complete
effective composition. Digest tests cover map canonicalization, validator
policy, artifact schema identity, effective profiles, and exclusion of the
digest field itself.
- **Typed registry boundary:** Extractor registrations retain one exact Go type;
merger, normalizer, and typed-validator registrations select an exact
module/artifact-kind variant; codecs validate complete schema/media identity;
and evidence projectors must match the active codec type. Every erased
operation checks the implementation or value type and returns an error rather
than asserting or panicking. Kind-neutral `Spec` methods are confined to
catalog inspection, while behavior-sensitive resolution uses exact variant
lookups.
- **Construction boundary:** `Prepare` validates the resolved shape and needed
registries, clones retained bindings, options, validator chains, reference
targets, schemas, and bytes, then constructs input, chunker, chunk validators,
every ordered typed lane and local validator chain, output, and the optional
evidence plan before returning. Implementations and operations remain
private; public prepared bindings/lanes are separate clones. Focused tests
verify deterministic construction order, late failure before input parsing,
nil and identity rejection, generated-selector retention, and independent
builder reference inputs. PIPE-002 records only the extra adjacent copies.
- **Checkpoint supplements:** Preparation collects component-provided semantic
fingerprints only after the complete implementation set exists. Scopes
include stage, globally unique lane identity, module, and validator position;
empty or duplicate values fail preparation, results are sorted, and the
accessor returns a defensive copy. Scheduling limits and diagnostics are not
included. Resolved composition—including options, reference selectors,
effective profiles, retries, and validator order—remains owned by the
resolved digest rather than being redundantly restated as component
fingerprints.
- **Registry comparison:** Input, chunker, and output registries consistently
normalize keys/specs, reject nil validators/builders, validate options on
owned maps, clone stored specs, sort discovery output, and verify constructed
identity. Typed stage and validator registries add only the exact-type and
artifact-variant mechanics their contracts require. Validator-chain lookup
distinguishes absent, default, explicit replacement, and explicit empty
chains while returning defensive copies. No dead compatibility path or safe
consolidation was found beyond the copy reduction in PIPE-002.
- **Test ownership:** Contract tests cover clone/serialization boundaries;
registry tests cover invalid registration, sorted/defensive discovery,
strict options, exact types, schema compatibility, and evidence ownership;
resolution tests cover heterogeneous variants and effective identity; and
preparation tests own all-before-parse construction and fingerprint
collection. The missing module-reference collision case is recorded in
PIPE-001 rather than as a separate test-only finding.
### References And Ordered Handoffs
- **External path:** Pipeline defaults are filtered to declared slots; local
bindings override eligible defaults; CLI overrides apply to one exact final
target; and unbind removes only an external binding before required-slot
enforcement. Selected legacy lanes determine eligible targets, while
explicit ordered steps reject `--only`. Config-relative and CLI-relative
paths remain distinct, materialized values retain digest/media/size/origin,
preparation supplies owned construction inputs, and operation requests get
independent content. REF-001 records the only missing bound in this path.
- **Generated path:** Pipeline-level selectors are rejected. Ordered local or
step bindings must name a declared earlier step and selected lane whose exact
artifact kind, registered codec media type, and target slot constraints are
compatible. Because all edges point strictly backward, forward references
and cycles are rejected during resolution. At the step barrier, exactly one
accepted normalized output is decoded and re-encoded through its codec;
missing, duplicate, wrong-kind, wrong-schema/media, oversized, rejected, or
unavailable producer state stops the consumer before execution. REF-002
concerns only the repeated lookup used to establish that cardinality.
- **Resume and recomputation:** Ordinary resume progressively compares
generated dependency fingerprints on extract, merge, and normalize
checkpoints. Selective recomputation instead forces the selected step and
transitive consumers while requiring unforced transitive producers to supply
accepted normalize state without extract/merge files or dependency matches.
Hydration validates stored provenance and canonical bytes through the active
codec before publishing an owned normalized output. REF-003 records the
omitted size field in the ordinary consumer fingerprint.
- **Provenance and evidence:** External manifests contain paths, digests,
media, sizes, and binding sources; generated manifests contain bounded
producer, codec/schema, digest, and size identity without content or a fake
path. Runtime stage debug envelopes omit reference sets, contract JSON omits
`ReferenceItem.Content`, and focused assembled-module tests confirm generated
references ground operations without becoming source evidence.
- **Complexity ownership:** Static target resolution keeps precedence,
unbinding, and required-slot policy together for contextual errors;
`validateGeneratedBindings` owns cross-step static compatibility; and
`buildStepReferenceSets` owns the runtime barrier. Apart from REF-002's
repeated full-output scan, their explicit traversals preserve distinct
invariants more clearly than a generic graph or reflection engine.
- **Test ownership:** Profile and CLI reference contracts cover defaults,
local precedence, unbinding, required slots, selected targets, ambiguity,
and typed producer compatibility. Reference materialization tests cover
origin, UTF-8, media, size diagnostics, warnings, and provenance; handoff and
runner tests cover canonical fanout, invalid/missing producers, ordering,
fingerprints, and accepted checkpoint hydration; assembled integration tests
cover semantic generated-reference consumers. REF-001 and REF-003 identify
the two unproved identity/resource details.
### Execution, Validation, Retry, And Concurrency
- **Runtime state diagram:** The audited framework lifecycle is:
```text
input validation / owned metadata
-> source load or parse -> document validation
-> chunk load or plan -> materialize -> whole-plan validator chain
-> rejected: terminal ordinary outcome ----------------------+
-> accepted: for each ordered step |
-> generated handoff barrier |
-> initialize lane checkpoint states |
-> dispatch extract jobs (chunk first, lane second) |
-> operation -> canonical candidate -> validators |
-> retry | rejected chunk | accepted chunk |
-> completed lane enters bounded continuation |
-> merge -> validators -> retry/reject |
-> normalize -> validators -> retry/reject |
-> await all started work -> stable lane merge -> barrier |
+---------------------------------------------------------------+
-> manifest/evidence -> logical output encoder
any framework error or parent cancellation
-> cancel derived work -> stop dispatch -> drain/await started work
-> stable error selection -> failed output (no logical files)
```
- **Ordering and bounded work:** `dispatchExtractJobs` enumerates chunks first
and lanes second. One fixed extract pool and one fixed continuation pool are
each bounded by `ExtractWorkers`; a lane continuation is serial merge then
normalize, while different completed lanes may overlap remaining extracts.
Results are indexed by lane and chunk, extracts are sorted by chunk index,
lane outputs merge in prepared order, and framework errors sort by stage,
lane, and chunk after child cancellation errors are filtered. Reverse-
completion, continuation-overlap, continuation-bound, provider-bound, and
stable-error tests confirm these contracts independently of goroutine finish
order.
- **Cancellation and liveness:** The lane engine checks cancellation before
starting queued extract and continuation work, stops dispatch, clears
unlaunched continuations, drains worker results, receives every launched
completion, and waits for both pools. The retry loop checks before an attempt
and after failed/rejected attempts. Parent cancellation takes precedence over
collected child cancellation, while an actual framework failure is selected
deterministically when the parent remains live. RUN-001 records the two
unguarded outer dispatch boundaries; no additional worker leak or deadlock
path was found. RUN-004 records the missing collector invariant comment.
- **Terminal outcomes:** Chunk, extract, merge, and normalize validators inspect
whole candidates, not partial streams. A rejection is collected as ordinary
output: a rejected chunk plan skips all lanes, a rejected extract chunk does
not prevent accepted chunks in that lane from merging, and merge/normalize
rejection terminates only that lane. A framework error cancels sibling work,
blocks later ordered steps, and returns through `failOutput`, which leaves no
logical output files. RUN-002 records the typed-value ownership violation;
serialized validators receive owned canonical bytes.
- **Retries and diagnostics:** Operation and complete validator-chain execution
share one retry budget. Debug persistence failure is terminal rather than
retried; cancellation stops retries; rejections record their final attempt;
and normalize retry directives use the same budget before validating their
final fallback. Candidate serialization precedes validation and accepted
checkpoint serialization follows it, so invalid candidates are not recorded
as success. Intermediate-attempt warnings remain only in attempt debug as
intended; RUN-003 records loss of the final rejected attempt's warnings.
- **Collaborator synchronization:** Runner-local wrappers serialize checkpoint,
debug, and LLM-debug collaborators without acquiring multiple framework
locks at once. Module/provider concurrency remains separately bounded by the
shared LLM scheduler. No duplicate recording or permit leak was found in the
audited execution layer; durable checkpoint transitions and filesystem
atomicity remain in the next audit area.
- **Focused test review:** Concurrency tests exercise chunk-first dispatch,
reverse completion, extract and continuation bounds, intended overlap,
provider limits, stable stage/lane error priority, ordinary rejection, parent
cancellation, and stopped queued extracts. Retry, rejection, candidate-
encoding, session, manifest, typed-checkpoint, debug-attempt, accepted-
checkpoint, and generated-handoff tests cover the remaining execution
branches. The consequential missing runtime cases are included in RUN-001
through RUN-003 rather than duplicated as test-only findings.
### State, Checkpoints, Debugging, And File Safety
- **Independent state lifecycles:** `runPipelineCommand` selects output,
chunk-plan, checkpoint, and debug roots only in the CLI. Bypass returns before
resolving or constructing a chunk-plan store; disabled checkpoints construct
only no-op collaborators and reject resume; enabled recording always creates
a recorder but creates a loader only for resume; and debug allocation occurs
only after an explicit debug request. None of these roots enters a module or
another state family's identity, and debug records are never read by reuse
code.
- **Chunk-plan cache:** The store accepts only a canonical lowercase SHA-256
source digest as its key, opens the exact digest directory through `os.Root`,
rejects symlink/non-directory digest entries and symlink/non-regular plan
files, strictly decodes one schema-versioned JSON value, revalidates plan
provenance and digest, and returns typed missing/invalid/hit decisions. A
reused plan is cloned, materialized against the current document, and passed
through the whole-plan validators. Only a newly accepted plan is published;
random 0600 temporary files are synced and renamed within the confined 0700
entry. Missing, invalid, or deleted entries are reconstructible.
- **Checkpoint identity and ordinary resume:** Identity hashes the complete
resolved pipeline, raw input, selected lanes, runtime overrides, external and
generated reference provenance, observed profile/runtime provenance, and
prepared component fingerprints. The directory uses readable/prefix
components, while every manifest retains and validates the complete identity
digest and exact stage/step/lane/module/dependencies. Payload publication
precedes a succeeded manifest, and the manifest retains payload digests, so
interruption or a changed dependency produces execution/invalidation rather
than stale reuse. STATE-001 records the remaining non-injective step/lane
component mapping.
- **Selective recomputation:** The CLI computes a forward forced closure and a
backward set of required reusable producers. The runner applies
`forced_recompute` before decoding an otherwise reusable artifact. Required
producers use only an accepted workspace-v3 normalize manifest with exact
invocation, producer provenance, artifact digest, active codec/schema/media,
and canonical bytes; they deliberately do not require extract/merge state or
current consumer dependencies. A failed required load records its typed
decision and stops before producer or consumer execution.
- **Decisions and diagnostics:** Checkpoint readers assign category and reason
code at the validation branch for missing, path, read, decode, schema,
identity, stage, dependency, payload, digest, and reuse outcomes. Runner
hydration adds codec and canonicality codes, forced policy adds the recompute
code, and required-predecessor errors name only stable step/lane identity and
code. Event detail is selected from code-owned text, UTF-8 normalized, and
bounded rather than copied from a cache error or caller payload.
- **File safety and recovery:** Output names are all validated before exclusive
run-directory creation; existing output/debug run directories are refused;
later output failure intentionally retains the new partial bundle. General
state writes use same-directory temporary files and atomic rename after
narrow relative-path checks, with 0700/0600 cache/debug modes and 0755/0644
output modes. General file I/O rejects existing symlink components; the
chunk-plan store additionally uses root-relative no-follow opens and
race-oriented entry rechecks. No production path automatically moves,
deletes, or cleans output, cache, or completed debug state; debug allocation
removes only its just-created partial leaf on setup failure.
- **Debug and terminalization:** A bundle owns distinct summary and trace
directories. Summary records use redacted invocation/configuration and
bounded checkpoint decisions; trace requests receive cloned application
payloads and no environment enumeration. Debug write failures fail the
requested run but cannot affect checkpoint identity or reuse. Guarded
terminalization writes one report, preserves an existing primary error over
report failure, attempts one error log, and joins persistence failures only
as secondary diagnostics. STATE-001 records trace-name collisions, not a
cache dependency or redaction leak.
- **Canonical ownership:** Source digesting uses canonical document/plan
serialization and cycle-aware metadata cloning. Chunk-map and evidence-
context codecs strictly validate schema, reject unknown fields and trailing
values, canonicalize nested identities/annotations/context, and return owned
graphs. Their encode-time clones are required ownership boundaries;
STATE-002 records only the redundant copies after decoding already-owned
JSON. STATE-003 records the one extractable duplicate writer; checkpoint
stage adapters, cache-specific `os.Root` mechanics, and type-specific clone
helpers remain intentional.
- **Focused test review:** File-I/O and debug-bundle tests cover confinement,
atomic replacement, symlinks, modes, refusal, cleanup, and redaction.
Checkpoint tests cover identity, manifests, interrupted publication,
categories/reason bounds, dependency invalidation, accepted normalize, and
codec canonicality. Chunk-plan tests cover corrupt entries, races, named
pipes, strict decode, materialization, and permissions; chunk-map and
evidence-context tests cover canonical round trips and ownership. CLI cache,
recomputation, state-hardening, run-contract, and terminal tests cover the
composed lifecycles and primary-error precedence. The uncovered identity,
copy, and writer issues are recorded as STATE-001 through STATE-003.
## Validation Record
| Date | Scope | Command or check | Result |
| --- | --- | --- | --- |
| 2026-08-08 | Initial worktree | `git status --short` | Pass; no output |
| 2026-08-08 | Knowledge graph | Full index as `notarius-audit-92e8907` | Pass; 8,322 nodes and 47,887 edges; branch/head matched the production target |
| 2026-08-08 | Baseline tests | `go test ./...` | Pass |
| 2026-08-08 | Baseline static analysis | `go vet ./...` | Pass |
| 2026-08-08 | Baseline build | `go build ./cmd/notarius` | Pass |
| 2026-08-08 | Baseline whitespace | `git diff --check` | Pass |
| 2026-08-08 | Production imports | Direct `go list` import-edge audit plus graph call tracing | Pass; no production dependency inversion found |
| 2026-08-08 | Accepted ADR links | Relative Markdown-link target scan under `docs/adr/` | Two unresolved targets recorded as ARCH-001 |
| 2026-08-08 | Audit target integrity before configuration/CLI review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | YAML decoder contract | `go doc gopkg.in/yaml.v3.Decoder.Decode` and `ParseFileConfigYAML` call trace | Decode consumes the next document; no EOF/second-document check, recorded as CFGCLI-001 |
| 2026-08-08 | Focused configuration and CLI tests | `go test ./internal/core/config ./internal/cli` | Pass |
| 2026-08-08 | Focused configuration and CLI static analysis | `go vet ./internal/core/config ./internal/cli` | Pass |
| 2026-08-08 | Audit target integrity before pipeline composition review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | Pipeline graph review | Architecture, complexity query, exact symbol reads, and call traces for `ResolvePipeline`, binding normalization, typed registries, `Prepare`, and checkpoint fingerprints | Pass; PIPE-001 and PIPE-002 recorded; generated handoff execution deferred to the next area |
| 2026-08-08 | Focused contracts and pipeline tests | `go test ./internal/framework/contracts ./internal/framework/pipeline` | Pass |
| 2026-08-08 | Focused contracts and pipeline static analysis | `go vet ./internal/framework/contracts ./internal/framework/pipeline` | Pass |
| 2026-08-08 | Audit target integrity before references/handoffs review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | Reference and handoff graph review | Architecture, exact symbol reads, and call traces across external materialization, target resolution, operation cloning, generated codec handoff, consumer fingerprints, ordered execution, and accepted-checkpoint hydration | REF-001, REF-002, and REF-003 recorded; no content/evidence leak found |
| 2026-08-08 | Focused pipeline and CLI tests | `go test ./internal/framework/pipeline ./internal/cli` | Pass |
| 2026-08-08 | Assembled integration tests | `go test ./internal/modules/integration/...` | Pass |
| 2026-08-08 | Audit target integrity before runtime review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | Runtime graph and state-machine review | Exact symbol reads, complexity inspection, call traces, cancellation-search coverage, and focused concurrency/retry/rejection/session/checkpoint/debug/manifest/handoff test review | RUN-001 through RUN-004 recorded; bounded work, stable assembly, and worker draining otherwise confirmed |
| 2026-08-08 | Pipeline race tests | `go test -race ./internal/framework/pipeline` | Pass |
| 2026-08-08 | Fresh pipeline tests | `go test -count=1 ./internal/framework/pipeline` | Pass |
| 2026-08-08 | Shuffled pipeline tests | `go test -shuffle=on ./internal/framework/pipeline` | Pass |
| 2026-08-08 | Audit target integrity before state review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | State graph and lifecycle review | Exact symbol reads, complexity and call traces across CLI root construction, core file I/O/debug allocation, chunk-plan storage, checkpoint identity/loading/recording, runner reuse decisions, trace/summary writers, terminalization, and canonical codecs | STATE-001 through STATE-003 recorded; independent lifecycle, typed decisions, accepted hydration, debug isolation, and primary-error precedence otherwise confirmed |
| 2026-08-08 | State collaborator race tests | `go test -race ./internal/core/fileio ./internal/core/debugbundle ./internal/framework/checkpoint ./internal/framework/chunkplan ./internal/framework/chunkmap ./internal/framework/debug ./internal/framework/evidencecontext` | Pass |
| 2026-08-08 | CLI state and terminal tests | `go test ./internal/cli` | Pass |
## Coverage Matrix
| Audit area | Status | Packages and documents inspected | Validation run | Finding IDs |
| --- | --- | --- | --- | --- |
| Architecture and dependency boundaries | Reviewed | Architecture, documentation, and testing policies; internal overview; accepted ADRs; `internal/cli/catalog.go`; production registrars; root `assets` package; representative pipeline, typed-codec, output, and state-owner symbols | Full baseline, fresh graph, direct import map, cross-layer call traces, ADR link scan | ARCH-001 |
| Configuration and CLI composition | Reviewed | `docs/config.md`, `docs/cli.md`, `docs/operations.md`, internal configuration/CLI docs; `internal/core/config/`; CLI run, catalog, session, profile, result, and terminal owners; focused config, command, run, reference, recomputation, session, production, example, result, and state tests | Target-integrity check, graph call/data-owner traces, YAML decoder contract, focused tests and vet | CFGCLI-001, CFGCLI-002 |
| Pipeline resolution, preparation, and typed registries | Reviewed | Internal pipeline/module docs; `internal/framework/contracts/`; pipeline profile, options, module, construction, preparation, fingerprint, stage/validator/chain/codec/evidence registry implementations and focused tests | Target-integrity check, graph architecture/complexity/call traces, focused tests and vet | PIPE-001, PIPE-002 |
| References and ordered handoffs | Reviewed | Reference and ordered-step sections of configuration, internal pipeline, and state docs; pipeline reference resolution/materialization, preparation ownership, generated handoff, consumer fingerprint, checkpoint hydration, and runner barriers; CLI selector/recomputation owners; focused profile, reference, handoff, checkpoint, recomputation, and assembled integration tests | Target-integrity check, graph architecture/complexity/call traces, focused pipeline/CLI tests, assembled integration tests | REF-001, REF-002, REF-003 |
| Execution, validation, retry, and concurrency | Reviewed | Internal pipeline runtime contract; runner, chunk planning/validation, concurrent lane engine, typed execution/validation, retry/normalize, synchronization, output suppression, and focused concurrency, retry, rejection, session, typed-checkpoint, debug, manifest, candidate-encoding, and handoff tests | Target-integrity check, graph state-machine/call/complexity review, race tests, fresh tests, shuffled tests | RUN-001, RUN-002, RUN-003, RUN-004 |
| State, checkpoints, debugging, and file safety | Reviewed | State and operations docs plus architecture state/security policy; CLI output, cache-root, checkpoint identity, recomputation, debug allocation, and terminal owners; core file I/O, debug bundle, and source digest/clone helpers; framework checkpoint, chunk-plan, chunk-map, debug, evidence-context implementations and focused tests | Target-integrity check, graph architecture/complexity/call traces, state collaborator race tests, CLI tests | STATE-001, STATE-002, STATE-003 |
| LLM runtime, prompt filesystems, and assets | Pending | — | — | — |
| Generic and Seriatim modules | Pending | — | — | — |
| Shared D&D types, codecs, and family mechanics | Pending | — | — | — |
| NPC, item, and location registries | Pending | — | — | — |
| NPC, item, and location occurrences | Pending | — | — | — |
| Spells, scene chunking, and scene descriptions | Pending | — | — | — |
| Combat turns and enemy events | Pending | — | — | — |
| Test ownership, comments, and final synthesis | Pending | — | — | — |