From 874adaaa18565c442ffc99326d682f0cb128ecfe Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 6 Sep 2026 19:58:18 -0500 Subject: [PATCH] Document results of a code maintainability audit for an upcoming implementation sprint --- docs/roadmap/remediation.md | 532 +++++++++++++++++++----------------- 1 file changed, 288 insertions(+), 244 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index e4a6a67..4e3188e 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,311 +1,355 @@ -# Implementation Review Remediation +# Pre-release Maintainability Remediation ## Status and scope -Status: complete. All nine findings have been addressed. Review baseline: `f42ecc70c330a399aa6dada87658694402a515ec`. -This review covers the initial implementation against the -[architecture policy](../policy/architecture.md), [configuration contract](../config.md), -and [implementation plan](implementation.md). The package boundaries and principal -lifecycle were in place at review time; the original findings below are retained -with completion evidence. No production code was changed during the original -review; subsequent remediation is recorded here. All findings below were reproduced through the public API or local -validation tools, with the additional affected call sites identified by inspection. +Status: planned; no stages have been implemented under this roadmap. +Audit baseline: `9b147bfd96aec25e0125dcae66e136e56dc7a670`. -Priorities: P1 means high-impact confidentiality, isolation, or availability; -P2 means a correctness or validation gap that also requires remediation. -Each item below records its completed correction and validation evidence. -Keep current contracts and implementation evidence accurate when fixing these issues; -do not change the agreed semantics simply to accommodate the implementation. +This roadmap owns the agreed maintainability cleanup before v0.1.0. It replaces +the completed implementation-defect remediation record, whose findings, fixes, +and validation evidence remain available in Git history. It does not reopen +those defects or replace the original [implementation plan](implementation.md). -## 1. P1 — Honor secret tags inside supported dynamic concrete values +The audit found worthwhile consolidation and ownership improvements, rather than +a need to redesign ConfigKit. The offline default suite and vet passed at the +audit baseline. That evidence is not validation of future changes or a release +candidate. -Status: complete. Typed projections now retain concrete field sensitivity; final, -baseline and pre-resolution projections feed historical/dependency classification. -Dynamic nested, replaced, deleted and introduced-value regressions pass. A final -audit additionally verified that validators cannot change dynamic field sensitivity -through equal-text type replacement; the added regression failed before the fix -and now passes. Full tests -pass with the temporary canonical-TMPDIR workaround for issue 8; vet, build and -whitespace checks pass. +Read the [development guide](../development.md), +[architecture policy](../policy/architecture.md), +[testing policy](../policy/testing.md), and +[documentation policy](../policy/documentation.md) before implementation. +The [configuration contract](../config.md) owns current semantics; the +[component overview](../internal/overview.md) owns implementation inventory. +Release preparation remains governed by the [release policy](../policy/release.md) +and [maintainer procedure](../release.md). -Evidence: [secret.go](../../secret.go), `schemaSecretRoot` stops at -`descriptorDynamic`; [projection.go](../../projection.go), `dynamicDescriptor` -accepts concrete structs and describes their fields, but their sensitivity is not -carried into the classification pass. +## Execution rules and invariants -Reproduction: define `Sensitive` with a string field -`Token` tagged `yaml:"token" configkit:"secret"`. Return -`Config{Payload: Sensitive{Token: "review-canary"}}` from defaults, where `Payload` -is `any`, and load with `NoSource()`. Loading succeeds. `WriteEffective` prints -`payload.token` in plaintext, and `Explain(Keys("payload", "token"))` returns that -same text with `Redacted=false`. This does not involve a typed `Secret` reference; -it breaks the separate field-tag classification contract for accepted Go values. +Implement the numbered stages in order. Each stage is one reviewable unit of +work: inspect its current owners and tests, make the change, validate it, update +its status and evidence here, and commit the nonempty unit with a short plain +English message. Do not create empty commits. Record any discovered behavioral +defect separately with a reproduction rather than silently changing semantics +under a refactoring stage. -Required work: preserve sensitivity from concrete dynamic descriptors through -projection and historical classification, including values subsequently replaced -or deleted and dependency propagation. Respect existing supported value shapes; -do not silently ignore tags or require callers to duplicate them in a predicate. +Keep the root public package and the existing `internal/document` and +`internal/yaml` packages. Do not introduce dependencies, public API changes, +provider interfaces, configurable merge policies, or a generic visitor framework. +Private file splits do not require new packages or ADRs. Parser-specific types +must remain behind the existing YAML adapter boundary. -Acceptance: canary tests for dynamic structs, nested containers, defaults and -recorded resolution must prove redaction in effective output and final/historical -explanation. Include replacement/deletion and derived dependencies. Typed secret -references must remain prohibited inside dynamic values. +Preserve the implemented contracts, particularly: -## 2. P1 — Reject nonregular source files before a blocking open +- Strict composition remains distinct from intentional profile/CLI overlay. +- Input precedence, final-only typed decoding, retained defaults, canonical + decoding explanation events, and recorded domain resolution remain unchanged. +- Source identity, containment, optional-import rules, limits, diagnostic codes, + source coordinates, ordering, and early callback termination remain intact. +- Semantic paths retain explicit key/index distinctions and historical shapes. +- Returned values and inspection snapshots remain detached; secret values remain + absent from history, rendering, and library-generated diagnostics. +- Resolution runs before secret lookup and validation; ordinary loading never + bootstraps environment variables implicitly. -Status: complete. Selected files, rooted imports and profiles now inspect canonical -targets before opening and retain post-open checks. Deadline-controlled FIFO -subprocess tests cover all three paths. Full tests pass with the issue-8 TMPDIR -workaround; vet, build and whitespace checks pass. +Use focused existing behavioral tests during each stage and the baseline checks +from the development guide before committing production changes. Add tests only +for meaningful gaps exposed by extraction, not to enforce file locations or +private call choreography. Update current owning documentation alongside each +move. Planned filenames below are private implementation destinations, not new +consumer contracts. -Evidence: [source.go](../../source.go), `readSource`, calls `os.Open` before -checking regular-file status. [imports.go](../../imports.go), `readImport`, and -[profile.go](../../profile.go), `readProfileFile`, likewise open before checking. +## Stage 1 — Remove unused overlay value snapshots -Reproduction: create a POSIX FIFO with no writer and call -`Load[map[string]any](File(fifoPath))`. The child process remained blocked until -terminated after two seconds; no nonregular-file diagnostic was returned. Byte -limits do not help because the open itself blocks. +Status: pending. -Required work: inspect canonical targets before opening and reject nonregular -files while retaining containment and post-open checks. Apply the protection to -selected files, imports, and profiles. This fits the existing stable-filesystem -contract and does not require introducing a cancellation API or promising hostile -filesystem race protection. Explicit `Reader` inputs retain their stream behavior. +Evidence: `overlayRecord` in [overlay.go](../../overlay.go) retains `before` and +`after` trees; profile and CLI processing construct them and +`cloneOverlayRecords` clones them. Production consumers use the record's change +and displaced origins, while explanation uses `historyApplication` instead. -Acceptance: subprocess-based FIFO regressions on supported POSIX platforms must -finish promptly with safe nonregular-file diagnostics for all three paths. Keep -regular-file, permitted symlink, containment, and permission tests passing. +Work: -## 3. P1 — Detach mutable values introduced by resolution before publication +1. Reconfirm all field reads before removal. Remove only the unused overlay + snapshot fields and their construction/cloning in `overlay.go` and + [override.go](../../override.go). +2. Preserve `change`, `displaced`, input applications, and their ownership rules. + Do not remove resolution snapshots or retained decoded trees: those have + separate consumers and budget semantics. +3. Remove locals and imports made unnecessary by the deletion. Do not replace + the removed state with a new cache or history representation. -Status: complete. Added post-resolution and publication ownership copies; the latter -preserves already validated secret state. Regressions cover nested maps/slices, -pointers, direct/helper assignments, validator aliases, and one lookup per load. -Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace pass. +Acceptance: profile/CLI provenance, explanation replay, composition independence, +and redaction tests pass unchanged in observable behavior. Source inspection +confirms no unused overlay snapshot construction remains. An allocation benchmark +is not required to justify removing provably unread clones. -Evidence: [resolution.go](../../resolution.go), `Default`/`Normalize`, directly -assign supplied values; `runResolution` projects them but does not clone the typed -object. [load.go](../../load.go) publishes that same object in `Result.Config`. +## Stage 2 — Centralize parser issue translation -Reproduction: a reusable resolver normalizes a map field to one captured map and -records the assignment through `Normalize`. Perform two sequential `NoSource` -loads with this definition. Changing the first result's map changes the second -result's map. Both loads succeed, while the second result's frozen inspection -still shows the earlier value. There is no concurrent callback use or retained -callback context in this reproduction. +Status: pending. -Required work: establish detached typed ownership after application resolution, -covering direct recorded assignments as well as helpers. Preserve resolved-secret -semantics and avoid extra environment reads; cloning only defaults is insufficient. -Ensure validators do not reintroduce equivalent-valued mutable aliases that bypass -the projection-based mutation check and escape into published results. +Evidence: [parse.go](../../parse.go) translates adapter issues inline, while +`importParseDiagnostics` in [imports.go](../../imports.go) repeats the translation +for imports and profiles. -Acceptance: repeated loads with shared resolver-provided maps, slices, pointers, -and nested values remain independent of each other and captured application state. -Keep final projection, secret lookup, and validation ordering consistent. +Work: -## 4. P2 — Reconcile history replay with final-only default application +1. Define one private `parserDiagnostics` helper in `parse.go` that maps adapter + issues to detached public diagnostics, including related locations. +2. Route selected-source, import, and profile parsing through it. Remove the + import-specific translator and keep upstream parser errors private. +3. Return canonically ordered diagnostics from the shared translation boundary. + Preserve existing codes, messages, primary/related coordinates, and source IDs. + Keep per-source parsing budgets and graph accounting at their current owners. -Status: complete. Explanation now records an explicit canonical decoding transition -when replay differs from final typed input, preserving decoded origins and showing -default restoration. Null/scalar-to-map profile and CLI regressions pass at parent -and descendant paths. Full tests pass with the issue-8 TMPDIR workaround; vet, -build and whitespace checks pass. +Acceptance: malformed selected/imported/profile input retains safe, correctly +attributed diagnostics. Use existing parser and import diagnostic tests; add a +small cross-entry regression only if a source path lacks equivalent coverage. -Evidence: [explain.go](../../explain.go), `Explain` initializes replay from defaults -and merges each input into it. Actual decoding applies defaults to the final -composed/overlaid input, so intermediate replacements are not lasting default resets. +## Stage 3 — Share bounded regular-file read mechanics -Reproduction: defaults contain `m: {a: 1}`, base YAML contains `m: null`, and a -selected profile contains `m: {b: 2}`. The successful typed result correctly contains -both `a: 1` and `b: 2`. Explanation of `/m` ends its history at only `b: 2`. -Explanation of `/m/a` reports its removal by the base and never explains its return, -even though its effective value is `1` with a default origin. +Status: pending. -Required work: reconstruct the input-layer history without conflating it with -incremental typed-default application. Explicitly account for effective default -retention/reappearance and canonical decoding so history and final inspection agree. -Preserve visibility of superseded policy-valid inputs, even when they were not -valid for the final destination type. Do not change decoding to retain reset markers. +Evidence: `readSource`, `importTraversal.readImport`, and `readProfileFile` repeat +preflight stat, open, post-open stat, regular-file checks, size checks, and bounded +reads in [source.go](../../source.go), [imports.go](../../imports.go), and +[profile.go](../../profile.go). -Acceptance: cover null-to-map and scalar-to-map overlays above defaulted containers, -including CLI overlays and descendant queries. The historical account must explain -the effective value and origin without implying that a final-present path is absent. +Work: -## 5. P2 — Replay newly created resolution paths +1. Introduce private helpers in `source_file.go` for regular-file opening and + bounded reading. Use narrow function parameters for stat/open operations so + callers can supply `os.Stat`/`os.Open` or root-bound methods. Do not introduce + an exported filesystem interface or substitute unrestricted opening for + `os.Root` access. +2. Keep preflight before open and post-open verification. Close every opened + file on all paths. Preserve the existing treatment of read-only file close + errors; root-directory close diagnostics are a separate responsibility. +3. Return private classified failures with enough distinction for callers to + map errors safely. Optional imports may suppress only the currently allowed + missing-target failures at stat/open, not later read failures. Do not expose + filesystem errors downstream through wrapping. +4. Keep source metadata, canonicalization, containment, per-source/aggregate + budgets, byte accounting, and caller-specific limit diagnostics outside the + shared mechanics. Pass the computed byte limit into the reader and retain + both the size precheck and limit-plus-one read enforcement. +5. Reuse bounded read mechanics for reader/byte sources only where it simplifies + existing code without changing single-use or ownership behavior. Keep secrets + bootstrap independent: its symlink, environment, and reporting policies differ. -Status: complete. Replay inserts new mapping entries/subtrees and coalesces -overlapping parent/child net operations while retaining reasons and dependencies. -Leaf/container regressions cover nested creation, removal and overlapping records. -Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace pass. +Acceptance: source, import, profile, FIFO, containment, symlink, optional-missing, +exact-budget, and reader-ownership tests pass. Retain the deadline-isolated FIFO +regression. Cover private failure classification only where public tests cannot +reliably induce an important filesystem failure. Run race tests for this unit. -Evidence: [explain.go](../../explain.go), `replaceHistoryPath`, returns immediately -when a mapping entry is absent, even when the replacement is a newly created value. +## Stage 4 — Establish shared document traversal ownership -Reproduction: load `m: {}` into a string-keyed integer map. In the resolver, set -`m["new"] = 7` and record `/m/new` with `ChangeDerivedDefault`. Loading succeeds. -`Explain(Keys("m", "new"))` reports effective YAML `7`, but the recorded resolution -change has `After.Present=false` and empty YAML. Parent explanation also misses -that insertion during replay. +Status: pending. -Required work: apply recorded additions as well as replacements and deletions, -including creation of necessary mapping ancestors when justified by the audited -change. Handle overlapping parent/child records without inventing assignment order -or duplicating effects; the contract records net domain changes. +Evidence: `indexDocument`, `walkDocumentPaths`, and `projectedSensitivity` repeat +semantic tree traversal in [load_metadata.go](../../load_metadata.go), +[metadata.go](../../metadata.go), and [secret.go](../../secret.go). General document +lookup currently lives in [load.go](../../load.go). -Acceptance: explain added map entries and nested additions at both leaf and parent -paths, alongside deletion/replacement cases. After-values and final snapshots must -agree while preserving reasons, dependencies, origins and redaction. +Work: -## 6. P2 — Preserve semantic path kinds and reject nonexistent explanation paths +1. Create root-private `document_walk.go` with a nil-safe preorder walker taking + a starting `Path` and a callback receiving both path and node. Visit mappings + in stored order and sequences in index order; construct explicit Key/Index + segments. It visits value nodes, not synthetic mapping-key nodes. +2. Move `indexDocument` and `documentAtPath` to this owner. Implement indexing, + metadata shape collection, and projected sensitivity collection using the + walker where their traversal semantics match. +3. Document callback borrowing: callers must clone paths or nodes when retaining + mutable state. Preserve existing defensive copies and avoid sharing mutable + metadata across results. +4. Keep path-bearing helpers in the root package so `internal/document` does not + depend on the public package. Pure node representation and cloning stay in + `internal/document`. +5. Leave specialized traversals explicit where they need inherited origins, + pruning, node identity deduplication, or different counting. In particular, + input limits count mapping keys and retained-history limits count shared nodes + once; do not implement either using the new walker without those semantics. -Status: complete. Metadata retains value-free effective/historical path shapes; -queries validate explicit segment kinds, and explanation omits absent-to-absent -operations for paths that never existed. Regressions cover wrong-kind and parsed -numeric queries, out-of-range indices, no-output failures and removed descendants. -Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace pass. +Acceptance: numeric map keys versus sequence indices, escaped paths, unknown +paths, historical shape changes, deleted descendants, dynamic secret tags, and +metadata detachment remain protected by existing tests. No public path behavior +or predicate coverage changes. Do not add tests that assert walker call counts. -Evidence: [metadata.go](../../metadata.go) indexes queries solely by `Path.String`, -losing explicit key/index distinctions. [explain.go](../../explain.go), -`applicationOperationPath`, considers an atomic ancestor relevant even when the -requested descendant never existed; an absent-to-absent event then makes `Explain` -report success. +## Stage 5 — Separate typed projection, cloning, and comparison -Reproduction: load `items: [1]`, `m: {"0": 2}`, and `x: 3` into `map[string]any`. -`Metadata.Lookup(Keys("items", "0"))` and `Metadata.Lookup(Keys("m").Index(0))` -both succeed despite the incorrect segment kinds. Their explanation behavior -also disagrees with metadata. `Explain(Keys("x", "missing"))` returns true and -`WriteExplanation` succeeds, although this path never existed. +Status: pending. -Required work: resolve query segment kinds against effective and historical -container shapes before looking up textual indexes. Distinguish a genuinely -removed path from an impossible descendant of an atomic value. Share traversal -semantics across metadata and explanation while allowing parsed numeric tokens -to resolve contextually. +Evidence: [projection.go](../../projection.go) combines roughly 622 lines of +projection, detached cloning, dynamic schema handling, cycle/limit enforcement, +and structural comparison. Value equality deliberately ignores sensitivity, +which validation must check separately. -Acceptance: explicit key/index queries reject incompatible containers; parsed -`/items/0` and `/m/0` still resolve correctly. Never-existing scalar descendants -return false/nil, and their writer returns an error without output. Historically -existing descendants remain explainable after parent replacement. +Work: -## 7. P2 — Reject unsupported inline pointer-to-map schemas safely +1. Keep canonical typed projection in `projection.go`; move cloning into + `clone.go`, shared reflection operation state and cycle helpers into + `value_operation.go`, and value-tree comparison into `document_compare.go`. + Preserve the descriptor-driven approach and per-operation schema cache. +2. Rename the private equality/comparison entry points to make configuration-value + semantics explicit. Document that location and sensitivity metadata are + excluded, mapping order is immaterial, and sequences are compared atomically + for change reporting. Update all callers without changing their semantics. +3. Give validator mutation comparison a named private owner in `validation.go`. + It must combine projected value changes and sensitivity changes, while + retaining private resolved-secret-state auditing as an additional check. + Do not make resolver value comparison start treating sensitivity as a value. +4. Preserve the distinction between cloning unresolved public secret references + and publication cloning of already validated private state. Name/document + the clone mode explicitly rather than leaving an unexplained boolean at + sensitive call sites. +5. Replace custom `cmpString` calls with the standard library comparator and + remove that helper. Do not perform unrelated test-style rewrites. -Status: complete. Schema construction rejects inline map pointers, including -multiple pointer levels, and detects nonterminating inline pointer chains. Public -regressions verify safe errors before defaults run; supported inline coverage stays -green. Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace -checks pass. +Acceptance: projection/cloning tests cover nil versus empty, custom text codecs, +inline fields, dynamic types, cycles, and limits. Validator mutation regressions +must still reject equal-text sensitivity changes and secret-state changes. +Callback ownership and secret lookup tests must prove publication does not share +borrowed values or reread the environment. Run race tests for this unit. -Evidence: [schema.go](../../schema.go), `describeFields`, dereferences inline field -types before choosing the map branch, then stores a map descriptor for a field -whose actual reflected value is a pointer. [projection.go](../../projection.go) -subsequently applies map operations to that pointer. +## Stage 6 — Clarify source assembly and override ownership -Reproduction: a configuration field declared -`Extra *map[string]int` with `yaml:",inline"`, initialized to a non-nil map in -defaults, panics in `Load` with `reflect: call of reflect.Value.Len on ptr to -non-array Value`. This is an internal reflection panic, not an application-hook panic. +Status: pending. -Required work: reject pointer-to-map inline declarations during schema construction -with a safe schema diagnostic. The agreed inline shapes are structs, -pointers-to-structs, and string-keyed maps; do not broaden them accidentally. -Audit nearby descriptor/value-kind assumptions for the same failure class. +Evidence: [imports.go](../../imports.go) mixes filesystem context, directives, +graph traversal, and diagnostics; [override.go](../../override.go) combines value +construction/validation and application. Shared composition cloning is located +inside the override feature, while profile overlay repeats it. -Acceptance: nil and non-nil inline map pointers fail consistently without invoking -default factories or panicking. Supported inline maps and inline struct pointers -continue to load, project, clone and classify correctly. +Work: -## 8. P2 — Fix the baseline test's canonical-path assumption on macOS +1. Keep import traversal and route diagnostics in `imports.go`. Move filesystem + context validation/canonicalization/containment to `filesystem.go`, and + directive structures/parsing/fragment restrictions to `directives.go`. + Give source-independent node counting an appropriate shared document owner, + with its mapping-key accounting documented. +2. Keep override validation and preparation in `override.go`, move parser-neutral + value building to `override_value.go`, and application/displacement mechanics + to `override_apply.go`. +3. Move `cloneComposedDocument` next to `composedDocument` in `compose.go`; use it + for both profile overlay and overrides. Ensure every retained field is copied + according to its existing ownership contract. +4. Keep strict composition, overlay, and history replay separate. Share small + mechanics when identical; do not introduce merge strategy flags or one general + merge engine. Preserve the existing profile-versus-CLI recording behavior. +5. Simplify profile root cleanup using deferred failure-path cleanup plus an + explicit checked success-path close. Preserve primary failure diagnostics and + the existing successful-read root-close error diagnostic; avoid double-close + effects. Import traversal cleanup may remain explicit where already clear. -Status: complete. The source test compares canonical path identity and separately -checks the declared label. Full default and race suites pass on macOS/arm64 using -both the default TMPDIR and an intentionally symlinked TMPDIR. Vet, build and -whitespace checks also pass; no production canonicalization behavior changed. +Acceptance: imports/globs, directive restrictions, source ordering, profiles, +overrides, version restrictions, limits, and composition independence tests pass. +No new cross-package dependencies appear. Root handles close on every path and +profile/CLI provenance remains identical. -Evidence: [source_test.go](../../source_test.go), -`TestFileSourcesReopenAndClassifyFailures`, compares `SourceInfo.Path` directly -with the path under `t.TempDir()`. Successful sources intentionally report a -symlink-resolved canonical path. +## Stage 7 — Extract explicit loader lifecycle phases -Reproduction: both `GOPROXY=off go test ./...` and its `-race` variant fail at this -assertion on the review host, where the temporary path traverses macOS's `/var` -symlink. Both suites pass with `TMPDIR=/private/tmp`. Source contents are correct; -the test expectation is wrong. +Status: pending. -Required work: derive the expected canonical path through the filesystem contract, -and separately verify the declared source label. Do not remove canonicalization -from production code or require developers to override `TMPDIR`. +Evidence: `loadWithSecretLookup` in [load.go](../../load.go) coordinates roughly +200 lines of source assembly, decoding, callbacks, history, secrets, and result +publication, including repeated creation of a composition handoff. -Acceptance: the ordinary documented test and race commands pass with the host's -default temporary directory and with an intentionally symlinked temporary directory. +Work: -## 9. P2 — Make the release checker's Bash prerequisite executable on macOS +1. Keep the exported entry point and a readable linear coordinator in `load.go`. + Move option selection to `options.go`, input assembly to `load_input.go`, and + typed lifecycle/result publication to `load_result.go`. Move version checking + beside the validation machinery rather than keeping it among document helpers. +2. Use small private handoff structs for assembled input and necessary phase + outputs, not a mutable all-purpose loader object or a long list of output + parameters. Reuse one composition initialization helper for profile and CLI + processing, including NoSource override handling and source byte/node totals. +3. Preserve the exact existing order of option validation, parsing/import checks, + schema validation, version checking, profile/CLI application, defaults factory, + typed decoding, resolution, retained-history limit enforcement, secret lookup, + validation, metadata/classification, and result publication. +4. Keep diagnostic/source accumulation explicit. Warnings survive successful + phases; errors stop dependent work and never return a usable partial Result. +5. Preserve both ownership barriers: detach resolver-borrowed values before secret + resolution and validation, then detach equal-valued validator replacements + before publication while preserving validated secret state. Do not remove + either clone as apparently redundant work. +6. Group Result's frozen inspection state only if it clarifies this handoff; do + not add an additional history copy or change caller ownership of Config. -Status: complete. Replaced mapfile with a Bash-3.2-compatible null-delimited array -read loop. Shell orchestration tests cover spaces/newlines and fail-fast behavior. -The complete checker passes under macOS system Bash 3.2 with cached Go 1.25.5 and -1.26.0, including both baselines, race tests, hygiene/docs/examples and all four -production cross-builds. New native Linux execution remains a release gate, not a -claim made by this local check. +Acceptance: lifecycle ordering/early-stop tests, defaults-only loading, all input +combinations, callback ownership, secrets, warning retention, and complete +provenance agreement pass. Keep orchestration visible enough to verify phase +ordering by reading the coordinator. Run the complete race suite. -Evidence: [scripts/release-check.sh](../../scripts/release-check.sh) uses -`mapfile -d ''`, but [the release procedure](../release.md) requires only Bash. -The macOS system Bash is 3.2 and has no `mapfile`; the null-delimiter option also -requires a sufficiently recent newer Bash. +## Stage 8 — Separate explanation replay, snapshots, and rendering -Reproduction: `/bin/bash -c "mapfile -d '' xs