diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..a21641b --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,81 @@ +# Input destination review findings + +Reviewed `d3d468a..945e111` against the approved +[input-destinations roadmap](input-destinations.md), +[implementation plan](implementation.md), and +[ADR 0011](../adr/0011-input-destinations.md). + +The root-owned destination fields, placement before strict composition, +transitive import placement, effective-path version ownership, final lifecycle, +source records, resource contexts, retained history, and existing disclosure +mechanisms broadly match the selected architecture. The required correction +below concerns an early diagnostic path, not loading or composition semantics. +No additional unintended strengthening of contracts was identified. + +## 1. Rebase known Mapping-source failure paths to the input destination (P2) + +Status: open. + +Affected: `assembleBaseInputTracked` in +[load_input.go](../../internal/engine/load_input.go), lines 27–32 at the reviewed +commit; the Mapping failure originates in `readSource` in +[source.go](../../internal/engine/source.go), lines 206–208. + +`Mapping` snapshots its input through the existing value builder. When a nested +value is unsupported, cyclic, or exceeds a value-builder limit, the retained +issue already identifies a semantic path inside that application mapping. +`readSource` turns it into a structured diagnostic. The early return from +`assembleBaseInputTracked` annotates the source record with its destination but +returns those diagnostic paths unchanged. + +For example, select this input as either main or an underlay: + +```go +configkit.Input{ + Source: configkit.Mapping("bad mapping", map[string]any{ + "item": map[string]any{"bad": func() {}}, + }), + Destination: configkit.Keys("catalog", "entries"), +} +``` + +The load correctly fails with `source.invalid`, but `Diagnostic.Path` is +`/item/bad` rather than `/catalog/entries/item/bad`. The source record correctly +reports `/catalog/entries`, so the two inspection records use inconsistent +application coordinates. A consumer displaying or associating the failure with +the final application shape would point to the wrong configuration field. + +This diverges from implementation decision 10 and Stage 6: existing paths that +genuinely identify application content must be translated to effective paths +exactly once. A Mapping value-builder issue has that information; it is not a +source-read or YAML syntax failure for which a logical child path is unavailable. + +Required remediation: + +- Prefix known application-relative Mapping value-builder diagnostic paths with + the normalized input destination at the acquisition boundary. Preserve key + escaping and sequence-index intent in the existing semantic Path model. +- Keep source IDs, original primary/related locations, codes, value-free + messages, source destinations, and nil-result behavior unchanged. Root + placement must retain its current paths. +- Do not blanket-prefix parse/read failures, source-constructor errors, or + structural import-directive diagnostics. Do not invent logical child paths + for failures that do not identify application content. +- Add bounded regression coverage through the public API for mounted main and + underlay Mapping failures, alongside unchanged root placement. Reuse existing + syntax/directive tests to protect their different coordinate convention. +- Run the focused destination/source/diagnostic checks and the everyday check. + Reconcile the completed plan's diagnostic evidence with this correction; + do not redefine the agreed effective-path behavior to accept the gap. + +## Review validation + +- `GOTOOLCHAIN=local ./scripts/check.sh` passed at `945e111` on + `go1.26.8 linux/amd64` (formatting, whitespace, tests, vet, and build). +- `GOTOOLCHAIN=local GOPROXY=off go test -race ./...` passed. +- `GOTOOLCHAIN=local GOPROXY=off go test -count=1 ./...` passed. +- An additional external-package probe in a disposable checkout outside the + repository failed for both main and underlay placement, observing `/item/bad` + instead of `/catalog/entries/item/bad` with `source.invalid`. +- No production fixes or regression tests were added to this repository during + the review. No performance measurements or new guarantees were inferred.