diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index a299410..c733a76 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -503,7 +503,14 @@ these cases. ## 13. Keep Metadata History In Post-Decode CLI Order — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: metadata history is assembled from the pre-CLI source view, with CLI +events appended separately; inheritance is projected before CLI and resolution. +Effective fields still describe post-CLI values. Regressions verify chronological +origins and resolver dependencies for ordinary/layered mapping and record +inheritance, including equal-valued assignments. Verified history/override +coverage and `./scripts/check.sh`. Owners: [`historyWithInheritance`](../../internal/engine/inheritance_history.go) and the source/CLI metadata handoff in diff --git a/internal/engine/inheritance_history.go b/internal/engine/inheritance_history.go index 8731df2..1fbbf90 100644 --- a/internal/engine/inheritance_history.go +++ b/internal/engine/inheritance_history.go @@ -79,7 +79,7 @@ func applyInheritanceMetadata(metadata *Metadata, raw *document.Node, rawOrigins func historyWithInheritance(path Path, stored []Change, operations []inheritanceOperation) []Change { split := len(stored) for index, change := range stored { - if change.Origin.Layer == LayerResolution { + if change.Origin.Layer == LayerCLI || change.Origin.Layer == LayerResolution { split = index break } diff --git a/internal/engine/override_context.go b/internal/engine/override_context.go index 04c743f..4109ed7 100644 --- a/internal/engine/override_context.go +++ b/internal/engine/override_context.go @@ -24,17 +24,20 @@ func preparePostDecodeOverrideContext(input assembledInput, before decodedConfig after.origins = cloneOrigins(assigned.origins) after.final = document.Clone(assigned.final) - metadata := metadataFromDecoded(after) + metadata := metadataFromDecoded(before) cliRecords := overrideMetadataRecords(before, assigned.overrides) var structuralOverlays []overlayRecord if input.inheritance != nil && input.composition != nil { structuralOverlays = input.composition.overlays - applyInheritanceMetadata(&metadata, input.composition.root, input.composition.origins, append(cloneOverlayRecords(structuralOverlays), cliRecords...), input.inheritance) + applyInheritanceMetadata(&metadata, input.composition.root, input.composition.origins, structuralOverlays, input.inheritance) } else if input.composition != nil { structuralOverlays = input.composition.overlays - applyOverlayMetadata(&metadata, append(cloneOverlayRecords(structuralOverlays), cliRecords...)) - } else if len(cliRecords) != 0 { - applyOverlayMetadata(&metadata, cliRecords) + applyOverlayMetadata(&metadata, structuralOverlays) + } + if len(cliRecords) != 0 { + metadata.fields = make(map[string]FieldInfo) + collectFieldInfo(after.final, Path{}, after, metadata.fields) + appendPostDecodeCLIHistory(&metadata, cliRecords) } sources := cloneSourceInfos(input.sources) @@ -80,6 +83,21 @@ func preparePostDecodeOverrideContext(input assembledInput, before decodedConfig } } +// Source history is already assembled, with inheritance projected at query time. +// Reuse overlay event construction for CLI changes without reapplying displaced +// source definitions after those later assignments. +func appendPostDecodeCLIHistory(metadata *Metadata, records []overlayRecord) { + cli := Metadata{fields: metadata.fields} + applyOverlayMetadata(&cli, records) + for pointer, history := range cli.history { + for _, change := range history { + if change.Origin.Layer == LayerCLI { + appendInputChangeEvent(metadata.history, nil, pointer, change) + } + } + } +} + func overrideMetadataRecords(before decodedConfiguration, overrides []preparedOverride) []overlayRecord { if len(overrides) == 0 { return nil diff --git a/internal/engine/override_history_order_test.go b/internal/engine/override_history_order_test.go new file mode 100644 index 0000000..2d9cc60 --- /dev/null +++ b/internal/engine/override_history_order_test.go @@ -0,0 +1,93 @@ +package engine + +import "testing" + +func TestCLIHistoryFollowsInheritanceAndPrecedesResolution(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name string + layered, records, equal, resolve bool + }{ + {name: "ordinary mapping"}, + {name: "layered mapping equal with resolution", layered: true, equal: true, resolve: true}, + {name: "ordinary records equal", records: true, equal: true}, + {name: "layered records with resolution", layered: true, records: true, resolve: true}, + } { + t.Run(test.name, func(t *testing.T) { + body := "base: {value: parent}\nchild: {extends: base, value: authored}\n" + query, parent := Keys("child", "value"), Keys("base", "value") + cliValue := "cli" + if test.equal { + cliValue = "authored" + } + override := Override{Path: query, Value: cliValue, Flag: "--value"} + if test.records { + body = "configkit:\n sequences: [{destination: [items], key: id}]\nitems: [{id: base, value: parent}, {id: child, extends: {id: base}}]\n" + query, parent = Keys("items").Index(1).Key("value"), Keys("items").Index(0).Key("value") + if test.equal { + cliValue = "parent" + } + override = Override{Path: Keys("items"), Value: []any{map[string]any{"id": "base", "value": "parent"}, map[string]any{"id": "child", "value": cliValue}}, Flag: "--items"} + } + definition := Definition[map[string]any]{} + if test.resolve { + definition.Resolve = func(value *map[string]any, resolution *Resolution) error { + recorded := query + if test.records { + (*value)["items"].([]any)[1].(map[string]any)["value"] = "resolved" + recorded = Keys("items") + } else { + (*value)["child"].(map[string]any)["value"] = "resolved" + } + resolution.Record(recorded, ChangeNormalization, "normalize after CLI", parent) + return nil + } + } + source := Bytes("source", []byte(body)) + var loaded *testResult[map[string]any] + var err error + if test.layered { + loaded, err = testGroupedLoad(loadRequest[map[string]any]{main: loadInput{source: source}, definition: definition, overrides: []Override{override}}) + } else { + loaded, err = testLoad(source, Options[map[string]any]{Definition: definition, Overrides: []Override{override}}) + } + if err != nil { + t.Fatal(err) + } + history := loaded.Metadata().History(query) + inherited, cli, resolved := false, false, false + for _, change := range history { + if change.Origin.Operation == ChangeInheritance { + if cli { + t.Fatal("inheritance reported after CLI") + } + inherited = true + } else if change.Origin.Layer == LayerBase && cli { + t.Fatal("source definition reappeared after CLI") + } + if change.Origin.Layer == LayerCLI { + if !inherited || resolved { + t.Fatal("CLI reported outside its lifecycle phase") + } + cli = true + } + if change.Origin.Layer == LayerResolution { + if !cli { + t.Fatal("resolution reported before CLI") + } + resolved = true + if len(change.Dependencies) != 1 || change.Dependencies[0].String() != parent.String() { + t.Fatalf("resolution dependencies = %#v", change.Dependencies) + } + } + } + if !inherited || !cli || resolved != test.resolve { + t.Fatalf("missing lifecycle history: %#v", history) + } + info, _ := loaded.Metadata().Lookup(query) + if history[len(history)-1].Origin.Layer != info.Origin.Layer { + t.Fatal("last history origin disagrees with effective metadata") + } + }) + } +}