Record migration review findings

docs/roadmap/remediation.md

@@ -1,4 +1,106 @@
-# Transcription Checkpoint Review Remediation
+# Audita v2 and ConfigKit Migration Review Remediation
+
+Status: Pending.
+
+Reviewed `83eaa96` through `fae18bf` against the approved clean-cutover decisions,
+[feature roadmap](audita.md), and [implementation plan](implementation.md).
+Review date: 2026-09-25. No production changes were made. The previous completed
+transcription review is retained below as historical context.
+
+## 1. Preserve omitted modules in direct YAML serialization
+
+Priority: P2. Status: Pending.
+
+Location: `internal/config/credential_projection.go:25` (`AuditaConfig.MarshalYAML`),
+especially the plain slice field at line 30 and assignment at line 40.
+
+The direct safe YAML projection passes a nil module slice to yaml.v3, which
+serializes it as `modules: []`. Loading that output produces a non-nil empty
+slice. Under the new Audita contract these are different instructions: nil
+delegates to Audita's configuration/default sequence; empty passes an explicit
+empty module argument and disables correction modules. Serialization therefore
+silently changes behavior for a configuration that is subsequently reloaded.
+
+Evidence: a temporary package test marshaled
+`AuditaConfig{Binary: "audita", Timeout: "3h"}`, unmarshaled it into another
+`AuditaConfig`, and required `Modules == nil`. It failed with output containing
+`modules: []`. The test was removed after reproduction. This concerns direct
+`yaml.Marshal` of the typed configuration; ordinary CLI `config show` uses
+`MarshalEffectivePipeline` through a separate JSON/value-tree projection and
+does not use this faulty path.
+
+Required remediation:
+
+- Make the direct safe YAML projection preserve nil as null or omission and
+  explicit empty as `[]`. Do not simply add `omitempty` to the slice: that would
+  instead erase the explicit empty selection.
+- Keep credential serialization name-only and preserve module order/repetitions.
+- Add a small round-trip regression covering nil, explicit empty, and an ordered
+  repeated selection; retain effective-output inspection coverage without
+  duplicating its whole test matrix.
+
+## 2. Remove the unapproved manifest compatibility decoder
+
+Priority: P2. Status: Pending.
+
+Location: `internal/manifest/manifest.go:79-89`,
+`internal/manifest/store_test.go:78`, and `docs/internal/manifest.md`'s decoding
+contract. Also correct the conflicting migration language in the plan/roadmap.
+
+The new `EffectiveConfigProvenance.UnmarshalJSON` bypasses the manifest store's
+`DisallowUnknownFields` by using permissive `json.Unmarshal` for the entire
+provenance object. It accepts retired `selected_profile` data and silently
+discards it. It also silently accepts unknown current provenance fields,
+including unknown nested selection fields, rather than limiting behavior to
+the renamed label.
+
+The user expressly selected a clean cutover without backward-compatibility
+shims. This adds precisely such a shim and a new historical-input acceptance
+guarantee. The regression test explicitly requires the old selection to load,
+and the internal guide now declares permissive informational-field decoding.
+
+The plan contributed to this divergence: it said existing generic decoding
+would handle historical optional provenance and required old manifests to load.
+That assumption was false; the baseline store already strictly decoded these
+fields. Repetition in the plan, tests, and documentation does not authorize
+overriding the user's clean-cutover decision.
+
+Required remediation:
+
+- Remove the custom compatibility unmarshaler and retain ordinary strict typed
+  decoding for the new `selected_overlay` representation. Do not add dual fields,
+  special retired-field handling, automatic manifest rewrites, or a format-version
+  bump just for this rename.
+- Replace the historical-acceptance test with coverage for current provenance
+  round trips and unknown/retired provenance rejection through the real store.
+- Correct the plan and current documentation to acknowledge the upgrade effect:
+  manifests with retired provenance require deliberate operator migration before
+  loading. Document narrow removal/renaming of retired informational provenance
+  on a backed-up manifest, preserving execution/checkpoint state; do not advise
+  deleting all session progress or rerunning everything.
+- Keep provenance informational: this correction does not authorize adding it
+  to semantic resume keys or invalidating work based on overlay identity alone.
+
+If transparent historical-manifest acceptance is desired instead, obtain an
+explicit exception to the clean-cutover decision before retaining a compatibility
+reader. Do not treat that exception as already granted.
+
+## Review verification
+
+- Existing `go test ./...` passed.
+- Race tests passed for subprocess, Audita, Notarius, config, stage, app, and
+  manifest packages after removing the temporary reproduction.
+- `go vet ./...` passed.
+- The temporary direct-YAML regression failed as described above.
+- Source inspection and the new historical-provenance test establish finding 2;
+  `LocalStore.LoadReader` still uses strict decoding outside the custom hook.
+
+The primary architecture is otherwise in place: ConfigKit v0.4.0 overlays,
+shared named credential forwarding/redaction, a v2-only Audita subprocess
+adapter, ordinary reports as diagnostics, and versioned ordered-module polish
+evidence. No live model/provider runs were performed during this review.
+
+## Historical completed transcription review
 
 Status: Complete.
 
@@ -7,7 +109,7 @@ upload-replay roadmap and the original user request. No production changes were
 made during review. The two failures below were reproduced with temporary tests
 using the existing stage fixtures; those temporary tests were removed afterward.
 
-## 1. Reconcile Aggregate State Even When No Speaker Needs HTTP Work
+### 1. Reconcile Aggregate State Even When No Speaker Needs HTTP Work
 
 Priority: P2. Status: Complete.
 
@@ -54,7 +156,7 @@ an added speaker), plus incomplete/inconsistent aggregate output coverage.
 Exercise the runner skip path and assert zero additional WhisperX requests,
 correct rebuilt outputs, and no removed speaker reaching merge.
 
-## 2. Remove the Unrequested Positive Audio-Size Requirement
+### 2. Remove the Unrequested Positive Audio-Size Requirement
 
 Priority: P2. Status: Complete.
 
@@ -94,7 +196,7 @@ Verification: cover a zero-size record round-trip and successful adapter result
 being checkpointed/reused for that input; retain negative-size rejection and
 ordinary downstream-failure tests.
 
-## Verification After Remediation
+### Verification After Remediation
 
 Completed on 2026-09-22 with Go 1.26.8: focused stage/application tests,
 changed-package race tests, `go test ./...`, `go test -race ./...`,