diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 9a9ff2e..a21a907 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -169,7 +169,14 @@ matches, and nested helpers. Check version-evidence projection where applicable. ## 5. Honor Identity Reset On Actual Inheritance Replacement — P2 -Status: [ ] Open. +Status: [x] Complete. + +Completion: inheritance resolves identity against actual child-value presence, +discarding parent YAML identity for an explicit replacement while preserving +agreement for retained values and authoritative application constraints. Added +policy-state and public-load regressions for changed/unkeyed/non-sequence +replacements, application agreement/conflict, and mode-only/append/upsert cases. +Verified focused inheritance tests and `./scripts/check.sh`. Owners: [`inheritanceCollectionResolution` and `mergeInheritanceCollectionPolicies`](../../internal/engine/inheritance_collection.go), especially lines 79–80 and 121. diff --git a/internal/engine/inheritance_collection.go b/internal/engine/inheritance_collection.go index 3896fc4..3b327e2 100644 --- a/internal/engine/inheritance_collection.go +++ b/internal/engine/inheritance_collection.go @@ -59,6 +59,7 @@ func inheritanceCollectionResolution( path Path, lowerScope, higherScope collectionPolicyScope, applicationPolicy sequencePolicy, + incoming *document.Node, ) (collectionPolicyResolution, collectionPolicy, collectionPolicy, bool, []Diagnostic) { lower, hasLower := lowerScope.at(path) higher, hasHigher := higherScope.at(path) @@ -72,6 +73,11 @@ func inheritanceCollectionResolution( lowerForResolution.explicitMode = 0 lowerForResolution.explicitModePresent = false lowerForResolution.modeLocation = nil + if incoming != nil && higher.explicitModePresent && higher.explicitMode == sequenceModeReplace { + // Actual replacement discards the parent's YAML identity before + // agreement. A mode-only declaration still governs retained records. + lowerForResolution = collectionPolicy{} + } var lowerPointer *collectionPolicy if hasLower { lowerPointer = &lowerForResolution @@ -85,7 +91,7 @@ func inheritanceCollectionResolution( func mergeInheritanceCollectionPolicies( parent, child collectionPolicyScope, parentRootPath Path, - parentValue *document.Node, + parentValue, childValue *document.Node, applicationPolicy sequencePolicy, ) (collectionPolicyScope, []Diagnostic) { result := cloneCollectionPolicyScopeForInheritance(parent) @@ -98,7 +104,13 @@ func mergeInheritanceCollectionPolicies( } var diagnostics []Diagnostic for _, path := range paths { - resolution, lower, _, active, issues := inheritanceCollectionResolution(path, parent, child, applicationPolicy) + relative, inside := relativeHistoryPath(parentRootPath, path) + var retained, incoming *document.Node + if inside { + retained, _ = documentAtPath(parentValue, relative) + incoming, _ = documentAtPath(childValue, relative) + } + resolution, lower, _, active, issues := inheritanceCollectionResolution(path, parent, child, applicationPolicy, incoming) if len(issues) != 0 { diagnostics = append(diagnostics, issues...) continue @@ -113,15 +125,14 @@ func mergeInheritanceCollectionPolicies( lower.modeLocation = nil lowerPointer = &lower } - relative, inside := relativeHistoryPath(parentRootPath, path) - var retained *document.Node - if inside { - retained, _ = documentAtPath(parentValue, relative) - } - policy, foundDiagnostics := collectionPolicyWithoutValue(lowerPointer, resolution.policy, retained, path) - if len(foundDiagnostics) != 0 { - diagnostics = append(diagnostics, foundDiagnostics...) - continue + policy := resolution.policy + if incoming == nil || resolution.operation != sequenceModeReplace { + var foundDiagnostics []Diagnostic + policy, foundDiagnostics = collectionPolicyWithoutValue(lowerPointer, resolution.policy, retained, path) + if len(foundDiagnostics) != 0 { + diagnostics = append(diagnostics, foundDiagnostics...) + continue + } } if higher, ok := child.at(path); ok { policy = inheritanceRecordPoliciesFromHigher(policy, higher) diff --git a/internal/engine/inheritance_collection_test.go b/internal/engine/inheritance_collection_test.go index 64affad..81a8c1e 100644 --- a/internal/engine/inheritance_collection_test.go +++ b/internal/engine/inheritance_collection_test.go @@ -1,11 +1,99 @@ package engine import ( + "reflect" "testing" "gitea.maximumdirect.net/eric/ConfigKit/internal/document" ) +func TestInheritanceReplacementResetsOnlyReplacedYamlIdentity(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name, key, value, applicationKey string + mode sequenceMode + wantFields []string + conflict bool + }{ + {name: "changed key", key: "name", value: "[{name: child}]", mode: sequenceModeReplace, wantFields: []string{"name"}}, + {name: "unkeyed replacement", value: "[{name: child}]", mode: sequenceModeReplace}, + {name: "non-sequence replacement", value: "42", mode: sequenceModeReplace}, + {name: "agreeing application", key: "name", value: "[{name: child}]", applicationKey: "name", mode: sequenceModeReplace, wantFields: []string{"name"}}, + {name: "conflicting application", key: "name", value: "[{name: child}]", applicationKey: "id", mode: sequenceModeReplace, conflict: true}, + {name: "application still validates replacement", value: "[{name: child}]", applicationKey: "id", mode: sequenceModeReplace, conflict: true}, + {name: "mode-only key change", key: "name", mode: sequenceModeReplace, conflict: true}, + {name: "mode-only keeps identity", mode: sequenceModeReplace, wantFields: []string{"id"}}, + {name: "append key change", key: "name", value: "[{name: child}]", mode: sequenceModeAppend, conflict: true}, + {name: "upsert key change", key: "name", value: "[{name: child}]", mode: sequenceModeUpsert, conflict: true}, + } { + t.Run(test.name, func(t *testing.T) { + input := "base: {items: [{id: parent}]}\nchild:\n extends: base\n" + if test.value != "" { + input += " items: " + test.value + "\n" + } + root := parseItemPolicyDocument(t, "replacement", input) + policies := collectionPolicyScope{} + policies.set(Keys("base", "items"), collectionPolicy{identityFields: []string{"id"}, identityPresent: true}) + higher := collectionPolicy{explicitMode: test.mode, explicitModePresent: true} + if test.key != "" { + higher.identityFields, higher.identityPresent = []string{test.key}, true + } + policies.set(Keys("child", "items"), higher) + application := sequencePolicy{} + if test.applicationKey != "" { + application = mustCompileSequenceIdentityPolicy(t, []sequenceIdentityRegistration{{ + selector: sequenceIdentitySelector{literalSequenceSelector("child"), literalSequenceSelector("items")}, identityFields: []string{test.applicationKey}, + }}) + } + expanded, diagnostics := expandInheritanceWithCollectionRules(root, nil, policies, application, defaultProjectionLimits()) + if test.conflict { + wantCode := CodeSequenceConflict + if test.applicationKey != "" && test.key == "" { + wantCode = CodeSequenceInvalid + } + if len(diagnostics) == 0 || diagnostics[0].Code != wantCode || diagnostics[0].Path.String() != "/child/items" && diagnostics[0].Path.String() != "/child/items/0/id" { + t.Fatalf("missing identity/constraint failure: %#v", diagnostics) + } + return + } + if len(diagnostics) != 0 { + t.Fatal(diagnostics) + } + policy, found := expanded.collectionPolicies.at(Keys("child", "items")) + if !found || !reflect.DeepEqual(policy.identityFields, test.wantFields) || policy.identityPresent != (test.wantFields != nil) { + t.Fatalf("effective YAML identity = %#v, want %v", policy, test.wantFields) + } + if test.value != "" { + actual, _ := documentAtPath(expanded.root, Keys("child", "items")) + want, _ := documentAtPath(root, Keys("child", "items")) + if !configurationValuesEqual(actual, want) { + t.Fatal("explicit replacement retained parent payload") + } + } + }) + } +} + +func TestLoadInheritanceReplacementUsesNewYamlKey(t *testing.T) { + t.Parallel() + loaded, err := testLoad[map[string]any](Bytes("replacement", []byte(`configkit: + sequences: + - {destination: [base, items], key: id} + - {destination: [child, items], key: name, mode: replace} +base: {items: [{id: parent}]} +child: + extends: base + items: [{name: child}] +`))) + if err != nil { + t.Fatal(err) + } + value, found := loaded.Explain(Keys("child", "items")) + if !found || value.Effective.YAML != "- name: child\n" { + t.Fatalf("replacement payload = %#v", value) + } +} + func TestInheritanceCombinesIdentifiedSequencesAtDestination(t *testing.T) { root := parseItemPolicyDocument(t, "inheritance-values", `base: items: diff --git a/internal/engine/inheritance_schedule.go b/internal/engine/inheritance_schedule.go index 1ec147a..997e1a5 100644 --- a/internal/engine/inheritance_schedule.go +++ b/internal/engine/inheritance_schedule.go @@ -222,7 +222,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int childPolicyScope := projectCurrentInheritanceCollectionPolicies(templatePolicies, node.path) merged := inheritanceParentPayload(parent, node.identityFields) mergedPolicyScope, policyDiagnostics := mergeInheritanceCollectionPolicies( - parentPolicyScope, childPolicyScope, node.path, parent, adapter.applicationPolicy(), + parentPolicyScope, childPolicyScope, node.path, parent, payload, adapter.applicationPolicy(), ) if len(policyDiagnostics) != 0 { return nil, policyDiagnostics @@ -258,7 +258,7 @@ func applyInheritanceScopeTemplates(adapter inheritanceScopeAdapter, order []int }, merge: func(childPath Path, existing, incoming *document.Node) (*document.Node, bool) { resolution, _, _, active, issues := inheritanceCollectionResolution( - childPath, parentPolicyScope, childPolicyScope, adapter.applicationPolicy(), + childPath, parentPolicyScope, childPolicyScope, adapter.applicationPolicy(), incoming, ) if len(issues) != 0 { combinationDiagnostics = append(combinationDiagnostics, issues...)