From 77c5d9482a1d50bf8644fb2fb8d79ca369319fac Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 16 Oct 2026 19:32:31 +0000 Subject: [PATCH] Close cross-cutting sequence regression gaps --- docs/roadmap/implementation.md | 19 ++- facade_sequence_test.go | 156 ++++++++++++++++++ .../engine/destination_accounting_test.go | 60 +++++++ internal/engine/history_sequence_test.go | 8 + internal/engine/resource_context_test.go | 14 ++ 5 files changed, 256 insertions(+), 1 deletion(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index aff0227..a60d67a 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Implementation Plan: Sequence Placement and Composition -Status: implementation in progress; stages 1–11 completed. +Status: implementation in progress; stages 1–12 completed. ## Purpose and authority @@ -706,6 +706,23 @@ Validation: targeted new regressions, `go test -race ./...`, and `./scripts/check.sh`. Completion: no known evidence, ownership, or accounting gap remains hidden behind a correct final tree. +Completion evidence: cross-cutting regressions now cover a typed main-tier +sequence replacement after underlay item appends, including presence and +zero/null distinctions, callback metadata and resource context, final origins, +removed-item explanation, and historical redaction in explanation and effective +rendering. Additional tests verify shared aggregate node accounting across a +nested item glob and sibling contribution, native file closure after nested item +composition failure, and retained sequence-history applications storing source +fragments rather than cumulative snapshots. Existing sequence placement, strict +diagnostic, version-evidence, and rendering tests continue to cover the other +settled risks. These checks passed with Go `go1.26.9 linux/amd64`: + +- `GOTOOLCHAIN=local GOPROXY=off go test -race ./...` +- `GOTOOLCHAIN=local GOPROXY=off ./scripts/check.sh` +- `git diff --check` + +No environmental limitation affected validation; checks ran on Linux/amd64. + ## Stage 13 — Document implemented contracts and provide a maintained example Scope: consumer documentation and a tested in-repository adoption fixture. diff --git a/facade_sequence_test.go b/facade_sequence_test.go index 71f6ac6..aa3ed3c 100644 --- a/facade_sequence_test.go +++ b/facade_sequence_test.go @@ -1,8 +1,10 @@ package configkit_test import ( + "bytes" "os" "path/filepath" + "strings" "testing" configkit "gitea.maximumdirect.net/eric/ConfigKit" @@ -321,3 +323,157 @@ func TestFacadeSourceInfoPlacementCopiesAndZeroValues(t *testing.T) { t.Fatalf("source evidence changed after caller mutation: %#v", again) } } + +func TestFacadeSequenceReplacementKeepsTypedAndHistoricalEvidenceAligned(t *testing.T) { + type record struct { + ID string `yaml:"id"` + Count int `yaml:"count"` + Enabled bool `yaml:"enabled"` + Empty string `yaml:"empty"` + Nullable *string `yaml:"nullable"` + Token string `yaml:"token"` + Omitted string `yaml:"omitted"` + } + type configuration struct { + Entries []record `yaml:"entries"` + } + + directory := t.TempDir() + mainPath := filepath.Join(directory, "main.yml") + underlayPath := filepath.Join(directory, "underlay.yml") + if err := os.WriteFile(mainPath, []byte(`entries: + - id: current + count: 0 + enabled: false + empty: "" + nullable: null + token: CURRENT-SECRET-CANARY +`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(underlayPath, []byte(`configkit: + imports: + - glob: records/*.yml + destination: [entries] + placement: sequence-item +`), 0o600); err != nil { + t.Fatal(err) + } + recordsDir := filepath.Join(directory, "records") + if err := os.MkdirAll(recordsDir, 0o700); err != nil { + t.Fatal(err) + } + for name, body := range map[string]string{ + "a.yml": "id: old-a\ntoken: OLD-SECRET-CANARY-A\n", + "b.yml": "id: old-b\ntoken: OLD-SECRET-CANARY-B\n", + } { + if err := os.WriteFile(filepath.Join(recordsDir, name), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + + var callbackItems []configkit.SourceInfo + var callbackContexts []configkit.ResourceContext + result, err := configkit.LoadLayered(configkit.Input{Source: configkit.File(mainPath)}, configkit.LayeredOptions[configuration]{ + Underlays: []configkit.Underlay{{Inputs: []configkit.Input{{Source: configkit.File(underlayPath), Imports: true}}}}, + Definition: configkit.Definition[configuration]{ + SequenceCompositions: []configkit.SequenceComposition{{Path: configkit.Keys("entries"), IdentityField: "id"}}, + Secret: func(path configkit.Path) bool { + return path.String() == "/entries/0/token" || path.String() == "/entries/1/token" + }, + Resolve: func(value *configuration, resolution *configkit.Resolution) error { + for _, field := range []string{"count", "enabled", "empty", "nullable"} { + path := configkit.Keys("entries").Index(0).Key(field) + if !resolution.IsExplicit(path) { + t.Errorf("resolver did not see explicit %s", path.String()) + } + } + if resolution.IsExplicit(configkit.Keys("entries").Index(0).Key("omitted")) { + t.Error("resolver treated an omitted field as explicit") + } + info, found := resolution.InputInfo(configkit.Keys("entries").Index(0).Key("count")) + if !found || info.Origin.Layer != configkit.LayerBase { + t.Errorf("resolver current-record input info = %#v, %v", info, found) + } + for _, id := range []string{"source-3", "source-4"} { + source, ok := resolution.Source(id) + if !ok { + t.Errorf("resolver could not query %s", id) + continue + } + context, contextOK := resolution.SourceContext(id) + if !contextOK || context.FS == nil || context.BaseDir != "records" { + t.Errorf("resolver context for %s = %#v, %v", id, context, contextOK) + } + callbackItems = append(callbackItems, source) + callbackContexts = append(callbackContexts, context) + } + return nil + }, + }, + }) + if err != nil { + if loadError, ok := err.(*configkit.LoadError); ok { + t.Fatalf("layered item replacement: %#v", loadError.Diagnostics()) + } + t.Fatalf("layered item replacement: %v", err) + } + if len(result.Config.Entries) != 1 || result.Config.Entries[0].ID != "current" || result.Config.Entries[0].Count != 0 || + result.Config.Entries[0].Enabled || result.Config.Entries[0].Empty != "" || result.Config.Entries[0].Nullable != nil { + t.Fatalf("typed replacement result = %#v", result.Config) + } + if len(callbackItems) != 2 || len(callbackContexts) != 2 { + t.Fatalf("callback sources = %#v; contexts = %#v", callbackItems, callbackContexts) + } + for _, source := range callbackItems { + if source.PlacementMode != configkit.PlacementModeSequenceItem || source.Destination.String() != "/entries" || source.DestinationUnresolved { + t.Errorf("callback item source = %#v", source) + } + } + metadata := result.Metadata() + for _, field := range []string{"count", "enabled", "empty", "nullable"} { + path := configkit.Keys("entries").Index(0).Key(field) + if info, ok := metadata.Lookup(path); !ok || !info.Explicit || (field == "nullable") != info.Null { + t.Errorf("explicit metadata at %s = %#v, %v", path.String(), info, ok) + } + } + if info, ok := metadata.Lookup(configkit.Keys("entries").Index(0).Key("omitted")); ok && info.Explicit { + t.Errorf("metadata marked an omitted zero-valued field explicit: %#v", info) + } + mainID := "" + for _, source := range result.Sources() { + if source.Path == mainPath { + mainID = source.ID + } + } + currentOrigin, ok := metadata.Lookup(configkit.Keys("entries").Index(0).Key("id")) + if !ok || len(currentOrigin.Origin.Locations) != 1 || currentOrigin.Origin.Locations[0].SourceID != mainID || mainID == "" { + t.Fatalf("replacement origin = %#v, main source %q", currentOrigin, mainID) + } + oldRecord, ok := result.Explain(configkit.Keys("entries").Index(1).Key("token")) + if !ok || oldRecord.Effective.Present { + t.Fatalf("removed appended item explanation = %#v, %v", oldRecord, ok) + } + maskedHistory := false + for _, change := range oldRecord.History { + maskedHistory = maskedHistory || change.Before.Redacted || change.After.Redacted + } + if !maskedHistory { + t.Fatal("removed appended item lost historical sensitivity") + } + var effective bytes.Buffer + if err := result.WriteEffective(&effective); err != nil { + t.Fatal(err) + } + var explanation bytes.Buffer + if err := result.WriteExplanation(&explanation, configkit.Keys("entries").Index(1).Key("token")); err != nil { + t.Fatal(err) + } + for _, output := range []string{effective.String(), explanation.String()} { + for _, canary := range []string{"CURRENT-SECRET-CANARY", "OLD-SECRET-CANARY-A", "OLD-SECRET-CANARY-B"} { + if strings.Contains(output, canary) { + t.Fatalf("sensitive sequence value leaked: %s", output) + } + } + } +} diff --git a/internal/engine/destination_accounting_test.go b/internal/engine/destination_accounting_test.go index 7814fb9..56243ab 100644 --- a/internal/engine/destination_accounting_test.go +++ b/internal/engine/destination_accounting_test.go @@ -68,6 +68,66 @@ func TestMultipleMountedInputsSharePlacementCharges(t *testing.T) { } } +func TestNestedItemGlobsShareUnderlayNodeBudget(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + mainPath := writeImportFile(t, directory, "main.yml", `configkit: + imports: + - glob: children/*.yml + destination: [children] + placement: sequence-item +id: parent +`) + fragmentPath := writeImportFile(t, directory, "children/a.yml", "id: child-a\n") + parseNodes := func(path string) int { + t.Helper() + parsed, diagnostics := parseSource(File(path), "fixture", defaultParseLimits()) + if len(diagnostics) != 0 { + t.Fatalf("parse %s: %#v", path, diagnostics) + } + return countInputNodes(parsed.document) + } + destination := Keys("groups") + childDestination := appendDestination(destination, Keys("children")) + otherDestination := Keys("other") + wantNodes := parseNodes(mainPath) + parseNodes(fragmentPath) + countInputNodes(&document.Node{Kind: document.Mapping}) + + destinationWrapperNodeCountForMode(destination, placementModeSequenceItem) + + destinationWrapperNodeCountForMode(childDestination, placementModeSequenceItem) + + destinationWrapperNodeCountForMode(otherDestination, placementModeSequenceItem) + limits := defaultImportLimits() + + for _, test := range []struct { + name string + delta int + wantError bool + }{ + {name: "at aggregate bound"}, + {name: "one node below aggregate bound", delta: 1, wantError: true}, + } { + t.Run(test.name, func(t *testing.T) { + accounting := &importAccounting{nodes: limits.nodes - wantNodes + test.delta} + composition, sources, _, diagnostics := acquireUnderlayGroup(loadGroup{inputs: []loadInput{{ + source: File(mainPath), imports: true, destination: destination, placementMode: placementModeSequenceItem, + }, { + source: Mapping("other", map[string]any{"id": "other"}), destination: otherDestination, placementMode: placementModeSequenceItem, + }}}, 1, nil, accounting) + if test.wantError { + if composition != nil || len(diagnostics) == 0 || diagnostics[0].Code != CodeSourceLimitExceeded { + t.Fatalf("under-budget nested item glob = %#v, %#v", composition, diagnostics) + } + return + } + if composition == nil || len(diagnostics) != 0 || len(sources) != 3 { + t.Fatalf("bounded nested item glob = %#v, sources %#v, diagnostics %#v", composition, sources, diagnostics) + } + if composition.inputNodes != wantNodes || accounting.nodes != limits.nodes+test.delta { + t.Fatalf("nested item node charge = %d/%d, want %d/%d", composition.inputNodes, accounting.nodes, wantNodes, limits.nodes+test.delta) + } + }) + } +} + func TestImportedEmptyBodiesChargeEachDestinationWrapper(t *testing.T) { t.Parallel() diff --git a/internal/engine/history_sequence_test.go b/internal/engine/history_sequence_test.go index 37fdda6..181d556 100644 --- a/internal/engine/history_sequence_test.go +++ b/internal/engine/history_sequence_test.go @@ -17,6 +17,14 @@ func TestSequenceHistoryReplaysAppendAtAssignedIndexes(t *testing.T) { if len(diagnostics) != 0 { t.Fatalf("compose diagnostics = %#v", diagnostics) } + for applicationIndex, application := range composed.applications { + for path, want := range map[string]int{"/records": 1, "/other": 1} { + sequence, present := documentAtPath(application.value, mustParsePath(t, path)) + if !present || sequence.Kind != document.Sequence || len(sequence.Sequence) != want { + t.Errorf("application %d retained %s = %#v, %v; want one source fragment item", applicationIndex, path, sequence, present) + } + } + } working := parseCompositionDocument(t, "defaults", "records: [{id: default}]\nother: [{id: default}]\n") index := new(mappingIndex) diff --git a/internal/engine/resource_context_test.go b/internal/engine/resource_context_test.go index 9d9a874..4834f9a 100644 --- a/internal/engine/resource_context_test.go +++ b/internal/engine/resource_context_test.go @@ -142,6 +142,7 @@ func TestLoadingClosesNativeFilesAcrossPipelineOutcomes(t *testing.T) { name string main string child string + helper string failRead string options Options[config] wantErr bool @@ -150,6 +151,16 @@ func TestLoadingClosesNativeFilesAcrossPipelineOutcomes(t *testing.T) { {name: "acquisition", main: "value: retained\n", failRead: "main.yml", wantErr: true}, {name: "parse", main: "value: [broken\n", wantErr: true}, {name: "composition", main: "configkit: {imports: [{path: child.yml}]}\nvalue: first\n", child: "value: second\n", options: Options[config]{Imports: true}, wantErr: true}, + { + name: "nested item composition", + main: "configkit: {imports: [{path: child.yml, destination: [children], placement: sequence-item}]}\nid: parent\n", + child: "configkit: {imports: [{path: helper.yml}]}\nid: child\nvalue: child\n", + helper: "value: helper\n", + options: Options[config]{ + Imports: true, destination: Keys("groups"), placementMode: placementModeSequenceItem, + }, + wantErr: true, + }, {name: "version", main: "version: wrong\n", options: Options[config]{Definition: Definition[config]{Version: &VersionRule{Path: Keys("version"), Check: func(Version, *Validation) error { return fs.ErrInvalid }}}}, wantErr: true}, {name: "validation", main: "value: retained\n", options: Options[config]{Definition: Definition[config]{Validate: func(_ *config, validation *Validation) { validation.Error(Keys("value"), "rejected") }}}, wantErr: true}, } { @@ -158,6 +169,9 @@ func TestLoadingClosesNativeFilesAcrossPipelineOutcomes(t *testing.T) { if test.child != "" { files["child.yml"] = &fstest.MapFile{Data: []byte(test.child)} } + if test.helper != "" { + files["helper.yml"] = &fstest.MapFile{Data: []byte(test.helper)} + } filesystem := &observedFilesystem{files: files, failRead: test.failRead} _, err := testLoad(filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), test.options) if (err != nil) != test.wantErr {