audita/docs/roadmap/remediation.md

# Library Adoption Review: Required Remediation

**Reviewed:** 2026-09-24, source commit `7cefb3f`.
**Scope:** The twelve implementation commits after `1ad2119`, compared with the
[implementation plan](https://gitea.maximumdirect.net/eric/audita/src/commit/cb405f6/docs/roadmap/implementation.md), [combined target](library-adoption.md),
and [configuration target](configkit.md).
**Status:** R1–R5 addressed and verified on 2026-09-24. The evidence in the
original findings below describes source commit `7cefb3f`; line references are
historical. See the resolution record at the end for the implemented fixes.

## Architecture Assessment

The intended ownership split is substantially implemented. ConfigKit v0.4.0 owns
schema-2 loading, overlays, presence/default resolution, explicit overrides,
credentials, and configuration inspection. One shared PromptKit v0.11.0 engine
owns both roles' profiles, embedded prompt rendering, prepared execution, provider
transport, retry/repair behavior, and per-backend admission. Audita retains domain
configuration, CLI composition, ordered modules, deterministic validators,
proposal/application checks, validation batching/cardinality, reporting, and
artifact lifecycle. The root module requires Go 1.25.5 and has no local replacement.

Legacy loaders, provider transport, template rendering, global/role schedulers,
and the isolated prototype were removed. All nine prompt definitions preserve
the original instruction text after accounting for the new hardening variable
and whitespace. There is no need to revisit the library-adoption design or add
compatibility machinery. The findings below are concrete defects or unfulfilled
parts of the selected final contract.

## R1 — P1: Redact Model-Derived Content in Every Retained Report Artifact

**Category:** Incomplete implementation of Stage 10's redaction boundary.

**Evidence:** Both success and failure paths in
[`runProcess`](../../internal/cli/run.go#L311) write
`BuildCorrectionLedger(...)` directly through `RunDirectory.WriteJSONArtifact`.
The success-path write is at line 348. That writer serializes its input without
redaction. Validator decision/rejection messages contain provider-supplied reason
text. Redacting the separate process report does not protect the ledger.

A controlled HTTP subprocess reproduction returned a schema-valid negative
assessment whose reason contained the synthetic configured backend credential.
Processing succeeded and the process report was redacted, but
`correction-ledger.json` retained the credential verbatim. The existing provider-
error test does not exercise this successful, schema-valid response path.

**Required change:** Apply the configured-secret redaction boundary to the ledger
and other model-derived diagnostic/report outputs before persistence. Audit the
new execution/utilization and selected-profile artifact paths for the same bypass;
metadata-only output is not a substitute for enforcing the selected secret-value
contract. Use all resolved backend credentials, including unselected backends.
Keep processing/domain data separate from its sanitized diagnostic representation.
Coordinate the shared redaction implementation with R2.

**Acceptance:** A controlled provider echoes synthetic credentials in a valid
assessment reason. Neither success nor partial-failure reports, ledgers, interaction
artifacts, or other affected diagnostic outputs contain those values. Include an
unselected backend credential. Preserve valid serialization and domain outcomes;
do not expand tests into exhaustive permutations of every artifact.

## R2 — P1: Remove the Report Redactor's Unredacted Fallback

**Category:** New code-quality/security defect in Stage 10.

**Evidence:** [`redactProcessReport`](../../internal/cli/run.go#L392) replaces secret
substrings in the serialized JSON document, then decodes it back into the report.
If decoding fails, it returns the original report. Replacements can corrupt JSON
syntax or typed values rather than just redact strings.

A focused reproduction loaded an otherwise valid config with the synthetic
credential `2`, then redacted a report containing `config_version: 2` and the string
`echo:2`. Replacing the numeric token made the JSON invalid; the helper returned
the original `echo:2`. Short numeric credentials are accepted by the selected
secret-reference contract, so the implementation cannot silently assume otherwise.
Quotes, escapes, overlapping credentials, and replacement order also need handling
at the serialization boundary.

**Required change:** Redact string content without changing JSON structure or
numeric/boolean types. Never return or write the original sensitive object when
redaction/serialization fails. Share the appropriate string-redaction logic across
report and artifact writers rather than maintaining incompatible versions. Handle
escaped content and overlapping values deliberately; retain typed error causes
without exposing provider error text.

**Acceptance:** Focused tests cover a numeric credential coinciding with report
numbers, quoted/backslash-containing credentials, and overlapping values. Output
remains valid with the original numeric/boolean meanings, secrets are removed,
and a redaction failure cannot emit the original object. Exercise the actual
external and run-local report writing paths with representative coverage.

## R3 — P1: Cancel Remaining Work When a Validator Fails

**Category:** Pre-existing runner behavior left inconsistent with the selected
Stage 8 final state and architecture policy; not newly introduced by PromptKit.

**Evidence:** [`runModulePipeline`](../../internal/framework/runner/runner.go#L383)
puts validation errors into a buffered channel, but reads that channel only after
`vwg.Wait()` (lines 414–421). `setErr` therefore cannot cancel the shared context
in response to a validator failure until all validation workers have finished.
Proposal work can continue launching validation in the meantime.

A controlled two-section reproduction made one validator fail while its peer
waited for context cancellation or an explicit test release. The peer was not
canceled; the pipeline returned its error only after the peer was manually released.
In production, a failed validation operation can thus leave other queued or active
provider calls consuming capacity and money until they finish or reach their own
deadlines. Adapter-level cancellation tests do not cover this runner behavior.

**Required change:** Propagate validation failures to shared cancellation as soon
as they occur, while retaining the originating error, joining active workers, and
collecting available results deterministically. Use a coordinator or properly
synchronized failure state; simply calling the current `setErr` from workers would
also require protecting concurrent reads of `firstErr`. Do not add a scheduler,
outer retry loop, or compatibility layer.

**Acceptance:** A runner integration test with a failing validator and a blocked
peer proves cancellation arrives without releasing the peer manually. Cover queued
provider work and preservation of the originating failure/partial observations.
The affected runner/runtime tests pass under the race detector, and failure emits
no purportedly complete corrected transcript.

## R4 — P2: Separate Module Identity from Section-Specific Diagnostic Names

**Category:** New observation/report attribution defect in Stages 9–10.

**Evidence:** [`validateSectionCandidates`](../../internal/framework/runner/runner.go#L525)
passes `ModuleInstanceForStages` as the validation request's module identity. This
value includes `:section-NNNN`. The
[validator call](../../internal/framework/validators/llm_validators.go#L110) copies
it into `DiagnosticContext.ModuleInstance` and leaves `SectionIndex` unset.
[`BuildUtilization`](../../internal/framework/processreport/llm.go#L25) groups
these records by that field, so proposal and validation calls for one real module
are reported under different module identities.

The subprocess reproduction with one grammar module and one section produced:

| Reported module instance | Proposal calls | Validation calls |
| --- | ---: | ---: |
| `grammar` | 1 | 0 |
| `grammar:section-0000` | 0 | 1 |

The latter is not a configured module instance. Its timing cannot join the real
module result, and the actual module incorrectly reports zero validation calls.
Multi-section runs multiply these spurious entries. Explicit section attribution
is also lost from validation call/event records.

**Required change:** Carry actual module instance and section index separately
through the validator request and structured-call context. Keep section-qualified
artifact names as a separate naming concern so files remain distinct. Group counts
and timing by the actual module instance. Update tests that currently use batch
index as a substitute for section identity.

**Acceptance:** A multi-section workflow with repeated module instances yields one
utilization entry per actual instance, correct proposal/validation totals, and
explicit section/batch identity in validation observations. Artifact names remain
unique and correction-ledger ordering is unchanged.

## R5 — P2: Index Execution Events Instead of Repeatedly Scanning Them

**Category:** New report-building performance/maintainability defect in Stage 10.

**Evidence:** [`BuildLLMExecution`](../../internal/framework/processreport/llm.go#L151)
scans every event for every call. `BuildUtilization` additionally tests every event
against a linear `hasCall` search for each backend's calls. With C calls and E
events, these paths perform O(C × E) work even for one backend. Retries and repairs
increase E. This work occurs after processing, before the CLI completes.

A synthetic review probe using one terminal event per call measured:

| Calls/events | Execution mapping | Utilization mapping |
| ---: | ---: | ---: |
| 1,000 | 14 ms | 6 ms |
| 10,000 | 1.61 s | 0.52 s |
| 20,000 | 8.08 s | 2.42 s |

These are local diagnostic measurements, not performance guarantees; the nested
scans establish the underlying growth independently of machine timing. Actual
snapshots usually contain more than one event per call.

**Required change:** Index calls/events by call ID once and aggregate by backend
and actual module identity using direct lookups. Reuse terminal records for totals
without rescanning all events for each call. Preserve event/call ordering and
knownness/completeness semantics. Straightforward maps are sufficient; no reporting
framework or new persistent cache is needed.

**Acceptance:** Existing report semantics remain intact, including preparation
failures, undispatched calls, and incomplete usage. A local benchmark or profiling
comparison demonstrates removal of the quadratic scans. Do not add fragile
wall-clock timing assertions to the default test suite.

## Review Verification and Limits

The existing root `go test ./...`, focused race tests for LLM/runner/subprocess
packages, `go vet ./...`, and Linux amd64/arm64 builds passed. Temporary review
probes reproduced R1–R4 and measured R5; those probe files were removed afterward.
At review time, the retained suite passed despite these defects. Add durable regression
coverage with the fixes, using the narrowest useful production boundaries.

The initial review changed only this remediation document. No application fixes,
upstream changes, live provider calls, release publication, or compatibility work
were performed. Release-distribution compliance remains the explicitly deferred
release task and is not counted as an implementation defect here.

## Resolution Record

All five findings are resolved without compatibility layers, new scheduling,
or changes to library ownership.

### R1 and R2 — Shared Serialization Boundary

`core/redaction` now sanitizes decoded JSON strings and YAML string nodes,
retaining numeric/boolean types and avoiding mutation of domain objects.
Longest matching credentials take precedence in a single replacement pass;
JSON-escaped credential forms are handled for embedded response content.
Serialization or key-collision failures return no payload. The unredacted report
fallback has been removed.

The CLI supplies all resolved backend credentials to the external report writer
and run directory. Report copies, ledgers, execution/utilization artifacts,
interaction diagnostics, selected profiles, and error text share the redactor.
ConfigKit still owns credential-reference redaction of effective configuration.

Evidence:

- `TestSubprocessRedactsValidModelReasonsOnSuccessAndPartialFailure` exercises
  schema-valid negative assessments containing selected and unselected secrets,
  including quotes/backslashes. Both reports and retained artifacts are checked;
  partial failure preserves earlier decisions and writes no corrected output.
- `TestReportWritersRedactStringsAndFailClosed` exercises both actual report
  writers with credential `2`, preserving numeric configuration version 2 while
  redacting string content, and verifies failed serialization writes no artifact.
- `TestStringRedactionPreservesSerializationAndDomainData` covers overlapping
  secrets, escaped nested content, large numbers, booleans, YAML scalar types,
  unchanged domain data, and failures without an original-payload fallback.

### R3 — Immediate Shared Cancellation

Validation workers record their error and cancel shared work immediately.
All accesses to the originating-error state use a mutex. Proposal collection
stops launching validation after a recorded failure; active workers are joined
and available results retain deterministic collection order.

`TestValidatorFailureCancelsActiveAndPendingProviderWork` uses the real PromptKit
runtime with backend concurrency one and queue capacity one. A failing validator
cancels two peer executions, one active in a controlled transport and one pending,
without manually releasing either. The originating error survives, only one
provider attempt occurs, and both logical calls retain finished observations.
The runner and runtime pass the race detector.

### R4 — Independent Module and Section Identity

Validation requests carry the configured module instance and an explicit section
index. The structured call forwards both; only interaction filenames combine
module, section, validator, and batch into a diagnostic name. The runner test
helper now uses section index rather than batch index to identify sections.

`TestSubprocessRepeatedModulesKeepSectionAttribution` runs two grammar instances
across two sections through the actual CLI/runtime. It verifies exactly two
utilization entries with two proposal and two validation calls each, explicit
section/batch identity, four distinct validation response artifacts, and unchanged
proposal ordering in rejected decisions.

### R5 — Indexed Report Aggregation

Execution mapping indexes events by call ID once. Utilization directly groups
calls/events by backend and indexes modules by real instance, seeded in module
result order. Neither builder scans all events for each logical call.

Existing summary tests and `TestIndexedReportsPreserveOrderAndPartialUsage`
verify pre-dispatch failures, original call/event order, backend grouping, and
known-but-incomplete usage. `BenchmarkExecutionReports` retains a reproducible
1,000/10,000/20,000-call benchmark without timing assertions.

A local three-iteration sample measured 20,000 calls at approximately **43 ms**
for execution mapping and **84 ms** for utilization, compared with the review
probe's 8.08 s and 2.42 s respectively. Timings are illustrative; the direct
indexes establish removal of the quadratic scans independently of machine load.

### Completion Validation

- Full offline suite: `go test ./...`.
- Race checks: runner, LLM runtime, process reporting, redaction, and CLI subprocess
  packages.
- Static analysis: `go vet ./...`.
- Both required Linux architectures: `scripts/build-release.sh` in a temporary
  output directory, without tagging or publishing.
- Documentation/diff whitespace checks.

Release publication and deferred release-distribution work remain outside this
remediation unit.