2026-10-14 17:52:14 +00:00
|
|
|
|
# Destination review findings
|
2026-10-14 14:12:18 +00:00
|
|
|
|
|
|
|
|
|
|
Reviewed `d3d468a..945e111` against the approved
|
|
|
|
|
|
[input-destinations roadmap](input-destinations.md),
|
2026-10-15 16:23:39 +00:00
|
|
|
|
input-destination implementation plan (preserved in Git history at
|
2026-10-14 16:53:50 +00:00
|
|
|
|
`315f464:docs/roadmap/implementation.md`), and
|
2026-10-14 14:12:18 +00:00
|
|
|
|
[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)
|
|
|
|
|
|
|
2026-10-14 15:49:51 +00:00
|
|
|
|
Status: complete. Known Mapping value-builder paths are now prefixed with the
|
|
|
|
|
|
normalized destination at acquisition, preserving semantic key/index segments.
|
|
|
|
|
|
Public regressions cover mounted main and underlay failures, escaped keys,
|
|
|
|
|
|
sequence indices, cyclic values, unchanged root paths, and source-constructor
|
|
|
|
|
|
errors retaining source coordinates. The mounted value cases failed before the
|
|
|
|
|
|
fix and pass afterward. Existing directive/syntax checks remain unchanged.
|
|
|
|
|
|
Focused destination/source/diagnostic/import tests, `./scripts/check.sh`, and
|
|
|
|
|
|
`GOTOOLCHAIN=local GOPROXY=off go test -race ./...` passed on
|
|
|
|
|
|
`go1.26.8 linux/amd64`.
|
2026-10-14 14:12:18 +00:00
|
|
|
|
|
|
|
|
|
|
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.
|
2026-10-14 17:52:14 +00:00
|
|
|
|
|
|
|
|
|
|
## Relative import destination review (2026-10-14)
|
|
|
|
|
|
|
|
|
|
|
|
Reviewed `7248a8b..10edd95` against the
|
|
|
|
|
|
[relative-import roadmap](relative-import-destinations.md), completed
|
2026-10-15 16:23:39 +00:00
|
|
|
|
implementation plan (preserved in Git history at
|
|
|
|
|
|
`d9f08ff:docs/roadmap/implementation.md`), and
|
2026-10-14 17:52:14 +00:00
|
|
|
|
[ADR 0012](../adr/0012-relative-import-destinations.md).
|
|
|
|
|
|
|
|
|
|
|
|
The implementation matches the selected architecture: literal string segments
|
|
|
|
|
|
append to each immediate parent's destination; placement uses the existing
|
|
|
|
|
|
strict-composition and final-loading pipeline; source records retain distinct
|
|
|
|
|
|
effective destinations without changing physical resource contexts. No runtime
|
|
|
|
|
|
defect, missing feature behavior, or unintended strengthening of consumer
|
|
|
|
|
|
contracts was identified. The completed input-destination finding above remains
|
2026-10-14 18:05:48 +00:00
|
|
|
|
resolved. The documentation correction identified below is now resolved.
|
2026-10-14 17:52:14 +00:00
|
|
|
|
|
|
|
|
|
|
### 2. Restore ADR 0011's accepted decision text (P3)
|
|
|
|
|
|
|
2026-10-14 18:05:48 +00:00
|
|
|
|
Status: complete. Restored the accepted Decision paragraph verbatim from
|
|
|
|
|
|
`7248a8b`, leaving the extension link in ADR 0011's Status section and ADR 0012
|
|
|
|
|
|
unchanged. A comparison of everything from `## Date` onward is identical to the
|
|
|
|
|
|
baseline; the full ADR diff against `7248a8b` changes only Status. The uncached
|
|
|
|
|
|
local documentation-link check and `git diff --check` passed. The existing
|
|
|
|
|
|
`TestFacadeDestinationMappingFailurePaths` regression also passed, confirming
|
|
|
|
|
|
the earlier completed finding remains addressed. No runtime code, current
|
|
|
|
|
|
configuration/API documentation, or tests were changed.
|
2026-10-14 17:52:14 +00:00
|
|
|
|
|
|
|
|
|
|
Affected: [ADR 0011](../adr/0011-input-destinations.md), lines 34–42 at
|
|
|
|
|
|
`10edd95`. That commit rewrote its accepted Decision paragraph to incorporate
|
|
|
|
|
|
relative import destinations. Stage 1 of the implementation plan explicitly
|
|
|
|
|
|
required preserving that text, and the
|
|
|
|
|
|
[documentation policy](../policy/documentation.md#architecture-decision-records)
|
|
|
|
|
|
treats accepted decision content as immutable. Rewriting it obscures the original
|
|
|
|
|
|
decision and its subsequent extension, despite the accurate current behavior.
|
|
|
|
|
|
|
|
|
|
|
|
Required remediation:
|
|
|
|
|
|
|
|
|
|
|
|
- Restore that Decision paragraph verbatim from `7248a8b` (the same paragraph
|
|
|
|
|
|
remains unchanged at `a8beb3e`). Keep the extension link in ADR 0011's Status
|
|
|
|
|
|
section and retain ADR 0012 as the accepted extension record.
|
|
|
|
|
|
- Leave the implemented relative-import behavior and current configuration/API
|
|
|
|
|
|
documentation unchanged. No additional ADR or runtime tests are needed.
|
|
|
|
|
|
- Confirm the ADR 0011 diff against `7248a8b` changes only its Status section;
|
|
|
|
|
|
run the local documentation-link and whitespace checks.
|
|
|
|
|
|
|
|
|
|
|
|
### Relative import review validation
|
|
|
|
|
|
|
|
|
|
|
|
At `10edd95d4d9060bd46db51e0d6782c12dbda3120`, using
|
|
|
|
|
|
`go1.26.8 linux/amd64`, all of these passed:
|
|
|
|
|
|
|
|
|
|
|
|
- `GOTOOLCHAIN=local ./scripts/check.sh` (formatting, whitespace, tests,
|
|
|
|
|
|
examples, links, package boundaries, vet, and build).
|
|
|
|
|
|
- `GOTOOLCHAIN=local GOPROXY=off go test -race ./...`.
|
|
|
|
|
|
- `GOTOOLCHAIN=local GOPROXY=off go test -count=1 ./...`.
|
|
|
|
|
|
|
|
|
|
|
|
The review changed only this findings document; it did not implement fixes.
|