diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 9629c35..2a7edf2 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,7 +1,7 @@ # Sequence Implementation Review Remediation -Status: eleven open findings. This is a review record, not an implementation -change or a new release gate. +Status: remediation in progress. Completed findings are marked below; this +record introduces no new release gate. Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`. Implementation range: `bc6b977..0be7cdb`. @@ -33,7 +33,13 @@ adding limits. ## 1. Preserve Retained Explicit Values In Nested CLI Mapping Assignments — P1 -Status: [ ] Open. +Status: [x] Complete. + +Completion: override metadata now reads the actual target subtree through one +shared lookup. Ordinary/layered regressions preserve retained values, including +zero and false, their explicit origins, and absence-based resolver behavior. +Verified `go test ./internal/engine -run 'Override|CLI' -count=1` and +`./scripts/check.sh`. Owner: [`applyDecodedOverrides`](../../internal/engine/override_decode.go), especially lines 59–60 and the presence/origin update helpers. diff --git a/internal/engine/override_decode.go b/internal/engine/override_decode.go index 5cea947..43874d3 100644 --- a/internal/engine/override_decode.go +++ b/internal/engine/override_decode.go @@ -38,7 +38,10 @@ func applyDecodedOverrides( } presence := cloneInputPresence(decoded.presence) origins := cloneOrigins(decoded.origins) - currentProjection := document.Clone(decoded.final) + var lookup *documentLookup + if len(batch.overrides) != 0 { + lookup = buildDocumentLookup(decoded.final) + } for _, override := range batch.overrides { if len(override.path.segments) == 0 { @@ -56,8 +59,11 @@ func applyDecodedOverrides( location := document.Location{SourceID: override.source.ID} recordOverrideAncestors(presence, origins, override.path, location) - updateOverridePresence(presence, currentProjection, override.value, override.path, location) - updateOverrideOrigins(origins, currentProjection, override.value, override.path, location) + // Prepared paths cannot overlap, so other assignments cannot change + // this target's pre-CLI subtree. + existing, _ := documentAtPathWithLookup(decoded.final, override.path, lookup) + updateOverridePresence(presence, existing, override.value, override.path, location) + updateOverrideOrigins(origins, existing, override.value, override.path, location) } final, projectionIssue := projectConfiguration(schema, working, defaultProjectionLimits()) if projectionIssue != nil { diff --git a/internal/engine/override_lifecycle_test.go b/internal/engine/override_lifecycle_test.go index 7a77cfb..6e50544 100644 --- a/internal/engine/override_lifecycle_test.go +++ b/internal/engine/override_lifecycle_test.go @@ -5,6 +5,67 @@ import ( "testing" ) +func TestNestedCLIOverridePreservesRetainedInputPresence(t *testing.T) { + t.Parallel() + type nested struct { + Changed int `yaml:"changed"` + Count int `yaml:"count"` + Zero int `yaml:"zero"` + Enabled bool `yaml:"enabled"` + } + type settings struct { + Nested nested `yaml:"nested"` + } + type config struct { + Settings settings `yaml:"settings"` + } + for _, layered := range []bool{false, true} { + name := "ordinary" + if layered { + name = "layered" + } + t.Run(name, func(t *testing.T) { + source := Bytes("input", []byte("settings:\n nested:\n changed: 1\n count: 2\n zero: 0\n enabled: false\n")) + definition := Definition[config]{Resolve: func(value *config, resolution *Resolution) error { + for _, key := range []string{"count", "zero", "enabled"} { + info, ok := resolution.InputInfo(Keys("settings", "nested", key)) + if !ok || !info.Explicit || info.Origin.Layer != LayerBase { + t.Errorf("retained %s input info = %#v, %v", key, info, ok) + } + } + Default(resolution, Keys("settings", "nested", "count"), &value.Settings.Nested.Count, 99, "fill missing count") + Default(resolution, Keys("settings", "nested", "zero"), &value.Settings.Nested.Zero, 99, "fill missing zero") + Default(resolution, Keys("settings", "nested", "enabled"), &value.Settings.Nested.Enabled, true, "fill missing enabled") + return nil + }} + overrides := []Override{{Path: Keys("settings"), Value: map[string]any{"nested": map[string]any{"changed": 3}}, Flag: "--settings"}} + var result *testResult[config] + var err error + if layered { + result, err = testGroupedLoad(loadRequest[config]{main: loadInput{source: source}, definition: definition, overrides: overrides}) + } else { + result, err = testLoad(source, Options[config]{Definition: definition, Overrides: overrides}) + } + if err != nil { + t.Fatal(err) + } + if got, want := result.Config.Settings.Nested, (nested{Changed: 3, Count: 2}); got != want { + t.Fatalf("resolved nested values = %#v, want %#v", got, want) + } + for _, key := range []string{"count", "zero", "enabled"} { + info, ok := result.Metadata().Lookup(Keys("settings", "nested", key)) + if !ok || !info.Explicit || info.Origin.Layer != LayerBase || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != "source-1" { + t.Errorf("retained %s metadata = %#v, %v", key, info, ok) + } + } + info, ok := result.Metadata().Lookup(Keys("settings", "nested", "changed")) + if !ok || !info.Explicit || info.Origin.Layer != LayerCLI { + t.Errorf("changed metadata = %#v, %v", info, ok) + } + }) + } +} + func TestPostDecodeOverrideLifecycleMatchesOrdinaryAndLayeredLoads(t *testing.T) { t.Parallel() type config struct {