Audit LLM runtime and prompt assets

This commit is contained in:
2026-08-08 21:47:51 +00:00
parent f3506240c2
commit ad85d71b0f

View File

@@ -20,9 +20,10 @@ change only roadmap audit documents do not change that production target.
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
reviews have found three High findings, four Medium findings, and thirteen Low
findings, with no production dependency inversion, unbounded framework worker
pool, completion-order-dependent result assembly, or debug-to-cache coupling.
pool, completion-order-dependent result assembly, debug-to-cache coupling, or
model-visible credential material in the embedded LLM assets.
## Finding Index
@@ -45,6 +46,10 @@ Final cross-area ordering is pending synthesis.
| 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 |
| LLM-001 | High | Correctness | Keep raw provider errors inside the adapter |
| LLM-002 | Low | Correctness | Recheck cancellation after scheduler admission |
| LLM-003 | Low | Correctness | Reject duplicate virtual prompt names |
| LLM-004 | Low | Duplication | Share the read-only in-memory filesystem mechanics |
## Findings
@@ -566,6 +571,148 @@ Final cross-area ordering is pending synthesis.
rename failure.
- **Grouping:** Independent.
### LLM Runtime, Prompt Filesystems, And Assets
### LLM-001 — Keep raw provider errors inside the adapter
- **Severity:** High
- **Category:** Correctness
- **Evidence:** Preparation and ordinary execution failures wrap
`redactPromptKitError` with `%w` at
`internal/framework/llm/promptkit_client.go:140``145` and 149172. The
returned `redactedProviderError` replaces bearer-token text only in its
`Error` method, then exposes the original upstream error unchanged through
`Unwrap` at lines 424442. The outer error therefore prints a redacted
diagnostic, but `errors.Unwrap`, `errors.Is`, or `errors.As` can recover the
raw PromptKit/provider error, including its original text and concrete type.
This contradicts the adapter contract in `docs/internal/llm.md:177``190`
and 231236, which requires provider-specific errors and credentials to stay
behind the provider-neutral boundary. Capacity exhaustion avoids this
particular chain by mapping the PromptKit sentinel explicitly and formatting
the sanitized diagnostic with `%v`.
- **Impact:** Any framework collaborator, logger, test helper, or future error
serializer that walks the standard error chain can disclose a bearer
credential that normal string formatting appeared to redact. It can also
couple application code to PromptKit/provider error types despite the
documented neutral transport boundary.
- **Recommendation:** Make sanitized upstream diagnostics opaque: do not
implement `Unwrap` and do not wrap a provider error directly after crossing
the adapter boundary. Classify context cancellation, capacity, invalid
structured output, and any other intentionally supported neutral categories
before sanitization, then expose only the corresponding contracts sentinel
plus credential-redacted text.
- **Preserve:** Keep caller-context precedence, prompt context, the
provider-neutral capacity and invalid-output sentinels, full internal
PromptKit error inspection before adaptation, and useful redacted operational
diagnostics.
- **Validation:** Return a typed upstream error containing a bearer secret from
both preparation and generation. Assert that every reachable error-chain
level and formatted form omits the secret, `errors.As` cannot recover the
upstream type, and `errors.Is` still recognizes only the documented context,
capacity, and invalid-output categories.
- **Grouping:** Independent.
### LLM-002 — Recheck cancellation after scheduler admission
- **Severity:** Low
- **Category:** Correctness
- **Evidence:** A queued scheduler waiter selects between its closed `ready`
channel and `ctx.Done()` at `internal/framework/llm/scheduler.go:50``73`.
`grantQueuedLocked` marks the waiter granted and closes `ready` at lines
103111. If cancellation and that grant become observable together, Go may
choose the ready case, so `Acquire` returns a permit even though the context
is already canceled. `Scheduler.Run` then invokes `fn(ctx)` immediately at
lines 7786 without another cancellation check. The focused queued-
cancellation test cancels before releasing the existing permit and therefore
does not exercise the simultaneous grant/cancel branch.
- **Impact:** A scheduler user that does not independently reject an already-
canceled context can begin backend work after the caller canceled while it
was queued. The production PromptKit path receives the canceled context and
ordinarily stops downstream work, and the deferred release prevents a permit
leak, which limits current severity; the scheduler's own cancellation
contract is nevertheless incomplete.
- **Recommendation:** After admission and before dispatch, recheck `ctx.Err()`
while retaining the deferred permit release. Keep `Acquire`'s locked
grant-versus-remove accounting as the owner of queue state; the extra gate
should only prevent the admitted callback from starting.
- **Preserve:** Retain one process-wide FIFO queue, the fixed concurrency
bound, idempotent release functions, cancellation removal for still-queued
waiters, and permit transfer/release after all errors.
- **Validation:** Add a focused grant/cancel race case with a callback that
records invocation and intentionally ignores its context. Prove that an
already-canceled admitted request returns `context.Canceled`, never invokes
the callback, releases its permit, and allows the next FIFO waiter to run;
retain the concurrency, cancellation, and idempotent-release tests under
`-race`.
- **Grouping:** Independent; this is the provider scheduler boundary rather
than RUN-001's pipeline module-dispatch boundary.
### LLM-003 — Reject duplicate virtual prompt names
- **Severity:** Low
- **Category:** Correctness
- **Evidence:** `promptfs.ModulePromptFS` validates and reads every declared
module file, then assigns its bytes directly into a map at
`internal/framework/promptfs/prompt_fs.go:40``58`; shared files use the same
direct assignment at lines 6080. Two names that are equal after trimming,
leading-slash removal, and cleaning therefore select the same virtual path,
and the later declaration silently replaces the earlier bytes. The asset
registry rejects duplicate flattened roots, but the module manifest boundary
has no equivalent check or focused duplicate test. All fourteen current D&D
manifests declare unique normalized names, so no maintained asset is
presently shadowed.
- **Impact:** A future or programmatically supplied prompt manifest can hash,
register, and execute successfully while using different prompt or shared
fragment bytes than one of its declarations implies. List order silently
decides the winner instead of producing the contextual asset-construction
error expected at startup.
- **Recommendation:** Track each cleaned virtual destination while assembling
`ModulePromptFS` and reject a second declaration before overwriting it. Report
whether the collision is module-owned or shared and include only the safe
virtual name, not file content.
- **Preserve:** Keep module and `sharedassets` namespaces distinct, reject
nested virtual names, read only explicitly declared files, return owned
bytes, and retain manifest order for fingerprinting and diagnostics where it
is semantically relevant.
- **Validation:** Add exact and normalization-equivalent duplicate cases for
module files and for shared files backed by different filesystems. Assert
deterministic contextual rejection while distinct module/shared basenames
remain valid; run the package under `-race`.
- **Grouping:** Independent.
### LLM-004 — Share the read-only in-memory filesystem mechanics
- **Severity:** Low
- **Category:** Duplication
- **Evidence:** `internal/framework/llm/asset_registry.go:279``430` implements
a complete map-backed read-only filesystem—path cleaning, `Open`, `ReadFile`,
`ReadDir`, directory discovery, sorted entries, cursor reads, file metadata,
and modes. `internal/framework/promptfs/prompt_fs.go:84``240` independently
repeats the same mechanics with renamed types. The copies have already
drifted: the asset filesystem represents an empty root as an existing empty
directory while the prompt filesystem reports it missing, and directory
`Read` returns a custom error in one implementation versus `io.EOF` in the
other.
- **Impact:** Filesystem-contract fixes and edge-case tests must be duplicated,
while callers receive subtly different behavior from two byte maps assembled
for the same PromptKit engine. The maintained registries are non-empty, so
the observed drift is a maintenance risk rather than a current prompt load
failure.
- **Recommendation:** Extract one narrow framework-owned read-only byte-map
filesystem with standard `io/fs` behavior and independently owned file
handles. Keep asset flattening, duplicate/root validation, schema selection,
and module/shared manifest construction in their existing domain owners.
- **Preserve:** Retain valid-path enforcement, deterministic directory order,
0444/0555 metadata, immutable source bytes, independent reader offsets,
contextual domain errors before filesystem construction, and the asset
registry's direct owned `ReadFile` result.
- **Validation:** Run `fstest.TestFS` over empty, single-file, and nested trees;
test root and directory `Open`/`ReadDir`/`Read` behavior, file-handle
independence, sorted entries, missing/invalid paths, modes, and mutation
isolation. Retain both asset-registry and `ModulePromptFS` integration tests.
- **Grouping:** Independent; implement after or alongside LLM-003 without
moving manifest collision policy into the generic filesystem.
<!--
Finding template for later audit stages:
@@ -698,6 +845,31 @@ Finding template for later audit stages:
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.
- `PromptKitClient.CompleteStructured` deliberately keeps prompt preparation,
the frozen prepared-detail snapshot, provider execution, PromptKit schema
validation, application decode, profile recording, and debug material
assembly explicit. Their order proves that debug and generation describe one
prepared request and that malformed provider output cannot become a typed
result. LLM-001 changes only the outward error chain; it should not hide this
sequence behind a generic transport pipeline.
- The CLI profile inspector and production PromptKit client intentionally own
separate engines while sharing the same profile-source option construction.
Inspection must resolve configured, fallback, built-in, and local-backend
policy without credentials, provider contact, or capacity admission;
reusing the runtime engine would blur that preflight boundary.
- Module prompt manifests and PromptKit YAML message lists explicitly enumerate
module-owned files, shared fragments, stable reference blocks, transcript
blocks, lane-specific grounding, and final instructions. This duplication is
content policy: it makes exact model-visible byte order and cache-control
breakpoints reviewable per workload. LLM-003 adds collision rejection, and
LLM-004 shares only the underlying filesystem mechanics; neither should
generate manifests dynamically or infer prompt order from filenames.
- The application scheduler and PromptKit backend limits intentionally remain
layered. The scheduler is the one process-wide provider-call bound across
profiles and modules, while PromptKit owns selected-backend capacity and maps
its exhaustion through a provider-neutral sentinel. LLM-002 closes an
admission/cancellation race without combining these limits or moving provider
capacity policy into the pipeline worker pools.
## Areas Reviewed Without Findings
@@ -1069,6 +1241,68 @@ Finding template for later audit stages:
composed lifecycles and primary-error precedence. The uncovered identity,
copy, and writer issues are recorded as STATE-001 through STATE-003.
### LLM Runtime, Prompt Filesystems, And Assets
- **Completion path:** Every production structured completion crosses the
scheduled client and the shared scheduler before the PromptKit adapter. The
adapter forwards the stable session ID directly, renders inputs and variables
once, prepares one frozen execution target, snapshots its details, invokes
that exact prepared target, accepts only PromptKit schema-valid structured
output, decodes into the caller's target, records normalized profile
provenance, and emits caller-owned debug material. Context and capacity are
adapted to provider-neutral categories; LLM-001 records the raw error chain
that still escapes on other provider failures.
- **Scheduling:** Production constructs one scheduler around the one PromptKit
client, so all LLM-backed bindings and validators share the configured
application limit. Queue insertion and grant are FIFO, active counts change
under one mutex, canceled queued entries are removed or return an already
granted permit, and idempotent deferred release covers success and error.
LLM-002 records only the simultaneous grant/cancel dispatch window; no permit
leak or second application scheduler was found.
- **Profiles and cache identity:** Preflight inspection and runtime construction
share configured directory/file, embedded fallback, built-in catalog, and
optional local-backend option builders while retaining separate engines.
Inspection does not load credentials or contact a provider. Checkpoint
fingerprints cover the compiled catalog identity, every configured profile
file's content identity, fallback asset identity, reasoning override, and a
hashed local endpoint without publishing profile contents, paths, endpoint,
environment values, or concurrency limits. Successful manifests record only
selected non-secret profile/provider/model/backend/reasoning provenance.
- **Sessions, bytes, and cache points:** The CLI resolves one stable session ID
before physical run construction and the same value flows through pipeline
requests into PromptKit's native `SessionID`; it is not reconstructed from a
transcript. All maintained extraction prompts render a stable shared system,
identity/reference prefix before the transcript, then lane-specific evidence
and grounding, with the final instructions last. Focused composition tests
assert identical cached prefixes, exact-once input placement, suffix order,
and ephemeral cache controls at reference, transcript, and final-instruction
boundaries.
- **Asset ownership:** The root `assets` package imports only `embed` and
`io/fs` and exposes one read-only filesystem. Module manifests explicitly
flatten only selected module files and shared fragments; the asset registry
clones bytes, rejects duplicate flattened prompt/schema/profile roots,
validates schema manifests, and hashes selected semantic assets. All fourteen
D&D prompt manifests currently use unique normalized virtual names; LLM-003
records that `ModulePromptFS` does not enforce this invariant itself.
- **Model-visible content:** Every maintained PromptKit YAML file, shared
fragment, module instruction/grounding file, private response schema, and
fallback profile was searched and representative complete compositions were
read. The only provider/model-named content is the intentional fallback
profile target. No bearer token, credential variable/value, endpoint,
provider-generated opaque identifier, UUID, digest-copy instruction, or
duplicated model instruction was found in model-visible assets. Durable
private-schema `$id` values remain schema metadata rather than requests to
reproduce internal identifiers.
- **Filesystem mechanics and tests:** Asset and module prompt filesystems
enforce valid scoped paths, sorted directory entries, read-only metadata, and
owned bytes, while prompt/schema loaders are exercised through real
PromptKit preparation. LLM-004 records the duplicated map-filesystem
implementation and its drift. Focused client tests cover direct sessions,
prepared-snapshot consistency, default/explicit/fallback/local profiles,
response validation/decode, capacity mapping, debug redaction, profile
provenance, and scheduled concurrency; scheduler and promptfs tests cover
limits, queued cancellation, release, path scoping, reads, and ownership.
## Validation Record
| Date | Scope | Command or check | Result |
@@ -1102,6 +1336,10 @@ Finding template for later audit stages:
| 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 |
| 2026-08-08 | Audit target integrity before LLM/assets review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
| 2026-08-08 | LLM and asset graph/content review | Exact symbol reads and call traces across scheduled completion, PromptKit adaptation, profiles/preflight/fingerprints, sessions, debug, asset flattening/hashing, prompt filesystems, all fourteen manifests, every prompt YAML, shared/model-visible content, private schemas, and the root asset leaf | LLM-001 through LLM-004 recorded; profile parity, exact prompt order/cache points, asset ownership, and secret-free model-visible content otherwise confirmed |
| 2026-08-08 | LLM and prompt filesystem race tests | `go test -race ./internal/framework/llm ./internal/framework/promptfs` | Pass |
| 2026-08-08 | Composed CLI and D&D prompt-cache tests | `go test ./internal/cli ./internal/modules/dnd/register` | Pass |
## Coverage Matrix
@@ -1113,7 +1351,7 @@ Finding template for later audit stages:
| 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 | — | — | — |
| LLM runtime, prompt filesystems, and assets | Reviewed | LLM internal/integration/configuration/operations docs and related ADRs; scheduled client, scheduler, PromptKit adapter/profile inspector, profile/source fingerprints, redaction, debug, asset registry, schema loader, promptfs implementations and focused tests; root assets, fallback profile, all fourteen module manifests and prompt YAML files, shared/model-visible fragments, private response schemas, and representative composed loaders | Target-integrity check, graph architecture/symbol/call review, content scan and exact prompt-order comparison, focused race tests, composed CLI and D&D prompt-cache tests | LLM-001, LLM-002, LLM-003, LLM-004 |
| Generic and Seriatim modules | Pending | — | — | — |
| Shared D&D types, codecs, and family mechanics | Pending | — | — | — |
| NPC, item, and location registries | Pending | — | — | — |