Audit reference materialization and ordered handoffs
This commit is contained in:
@@ -19,8 +19,8 @@ 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, and pipeline composition reviews have found one Medium
|
||||
correctness finding and four Low findings, with no production dependency
|
||||
configuration/CLI, pipeline composition, and reference/handoff reviews have
|
||||
found two Medium findings and seven Low findings, with no production dependency
|
||||
inversion or unsafe typed-erasure boundary.
|
||||
|
||||
## Finding Index
|
||||
@@ -34,6 +34,9 @@ Final cross-area ordering is pending synthesis.
|
||||
| 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 |
|
||||
|
||||
## Findings
|
||||
|
||||
@@ -192,6 +195,103 @@ Final cross-area ordering is pending synthesis.
|
||||
`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 128–146,
|
||||
`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 210–232. 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.
|
||||
|
||||
<!--
|
||||
Finding template for later audit stages:
|
||||
|
||||
@@ -261,13 +361,25 @@ Finding template for later audit stages:
|
||||
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 owned by the next audit area.
|
||||
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.
|
||||
|
||||
## Areas Reviewed Without Findings
|
||||
|
||||
@@ -431,6 +543,54 @@ Finding template for later audit stages:
|
||||
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.
|
||||
|
||||
## Validation Record
|
||||
|
||||
| Date | Scope | Command or check | Result |
|
||||
@@ -451,6 +611,10 @@ Finding template for later audit stages:
|
||||
| 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 |
|
||||
|
||||
## Coverage Matrix
|
||||
|
||||
@@ -459,7 +623,7 @@ Finding template for later audit stages:
|
||||
| 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 | Pending | — | — | — |
|
||||
| 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 | Pending | — | — | — |
|
||||
| State, checkpoints, debugging, and file safety | Pending | — | — | — |
|
||||
| LLM runtime, prompt filesystems, and assets | Pending | — | — | — |
|
||||
|
||||
Reference in New Issue
Block a user