From 315f464dfc670b405fb54c4eab578c1b8f1e4575 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 14 Oct 2026 15:49:51 +0000 Subject: [PATCH] Correct mounted mapping diagnostic paths --- docs/roadmap/implementation.md | 9 ++++- docs/roadmap/remediation.md | 10 ++++- facade_destination_test.go | 70 ++++++++++++++++++++++++++++++++++ internal/engine/load_input.go | 9 +++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index b6dd1e2..3e0cc39 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -305,8 +305,13 @@ and `./scripts/check.sh` (go1.26.8 linux/amd64); all passed. The mounted conflict, decode, and imported-version assertions use effective application paths with authored source locations; invalid destinations have no physical location; malformed source-root directives retain their authored path and -location. No path-prefix translation was needed because value diagnostics are -already produced from placed trees and directive diagnostics from raw trees. +location. Later value diagnostics are produced from placed trees, while directive +diagnostics retain their raw source coordinates. Post-review remediation also +rebases known Mapping value-builder failure paths, which are produced before +placement. `TestFacadeDestinationMappingFailurePaths` covers mounted main and +underlay failures, escaped keys and sequence-index intent, cycles, unchanged +root paths, and unchanged source-constructor failures. Its mounted value cases +failed before the fix and pass afterward. Done: diagnostics consistently distinguish application paths from source syntax. ## Stage 7 — Complete shared placement accounting diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index a21641b..ad7e4f6 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -14,7 +14,15 @@ No additional unintended strengthening of contracts was identified. ## 1. Rebase known Mapping-source failure paths to the input destination (P2) -Status: open. +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`. Affected: `assembleBaseInputTracked` in [load_input.go](../../internal/engine/load_input.go), lines 27–32 at the reviewed diff --git a/facade_destination_test.go b/facade_destination_test.go index e01fa96..d9614af 100644 --- a/facade_destination_test.go +++ b/facade_destination_test.go @@ -2,12 +2,14 @@ package configkit_test import ( "bytes" + "errors" "fmt" "io" "io/fs" "os" "path" "path/filepath" + "reflect" "strings" "testing" "testing/fstest" @@ -355,6 +357,74 @@ func TestFacadeFailedLoadExposesDetachedSourceDestination(t *testing.T) { } } +func TestFacadeDestinationMappingFailurePaths(t *testing.T) { + t.Parallel() + + cycle := map[string]any{} + cycle["self"] = cycle + for _, fixture := range []struct { + name string + values map[string]any + path func(configkit.Path) configkit.Path + }{ + { + name: "unsupported nested sequence value", + values: map[string]any{"a/b": []any{map[string]any{"~": func() {}}}}, + path: func(base configkit.Path) configkit.Path { return base.Key("a/b").Index(0).Key("~") }, + }, + { + name: "cyclic value", + values: map[string]any{"item": cycle}, + path: func(base configkit.Path) configkit.Path { return base.Key("item").Key("self") }, + }, + { + name: "constructor failure keeps source coordinates", + path: func(configkit.Path) configkit.Path { return configkit.Path{} }, + }, + } { + t.Run(fixture.name, func(t *testing.T) { + for _, role := range []string{"root", "mounted main", "mounted underlay"} { + t.Run(role, func(t *testing.T) { + var destination configkit.Path + if role != "root" { + destination = configkit.Keys("catalog", "entries") + } + wantPath := fixture.path(destination) + input := configkit.Input{Source: configkit.Mapping("invalid mapping", fixture.values), Destination: destination} + var result *configkit.Result[map[string]any] + var err error + if role == "mounted underlay" { + result, err = configkit.LoadLayered(configkit.Input{Source: configkit.NoSource()}, configkit.LayeredOptions[map[string]any]{ + Underlays: []configkit.Underlay{{Inputs: []configkit.Input{input}}}, + }) + } else { + result, err = configkit.LoadLayered[map[string]any](input) + } + var loadError *configkit.LoadError + if result != nil || !errors.As(err, &loadError) { + t.Fatalf("failed load = %#v, %v", result, err) + } + diagnostics := loadError.Diagnostics() + if len(diagnostics) != 1 { + t.Fatalf("diagnostics = %#v", diagnostics) + } + diagnostic := diagnostics[0] + if diagnostic.Code != configkit.CodeSourceInvalid || !reflect.DeepEqual(diagnostic.Path, wantPath) { + t.Fatalf("diagnostic = %#v; want source.invalid at %q", diagnostic, wantPath.String()) + } + if diagnostic.Primary == nil || *diagnostic.Primary != (configkit.Location{SourceID: "source-1"}) || len(diagnostic.Related) != 0 { + t.Fatalf("mapping source locations = %#v, %#v", diagnostic.Primary, diagnostic.Related) + } + sources := loadError.Sources() + if len(sources) != 1 || sources[0].ID != "source-1" || sources[0].Destination.String() != destination.String() { + t.Fatalf("error sources = %#v", sources) + } + }) + } + }) + } +} + func facadeDestinationMap(t *testing.T, value map[string]any, keys ...string) map[string]any { t.Helper() for _, key := range keys { diff --git a/internal/engine/load_input.go b/internal/engine/load_input.go index 449f865..f98ddde 100644 --- a/internal/engine/load_input.go +++ b/internal/engine/load_input.go @@ -29,6 +29,15 @@ func assembleBaseInputTracked[T any](source Source, options Options[T], destinat if len(diagnostics) != 0 { input.sources = sourceInfos(parsed.source) setSourceDestinations(input.sources, destination) + // Mapping value issues already identify application content. Other early + // failures describe source syntax or access, not a placed value. + if source.kind == sourceMapping && source.mapIssue != nil && len(destination.segments) != 0 { + for index := range diagnostics { + relative := diagnostics[index].Path + segments := clonePathSegments(destination.segments, len(relative.segments)) + diagnostics[index].Path = Path{segments: append(segments, relative.segments...)} + } + } return input, diagnostics } if options.Imports {