From 0314e587b60f7abcb26478de7cd477085d1df1e9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 23 Oct 2026 14:07:07 +0000 Subject: [PATCH] Reconcile collection declarations and modes --- docs/roadmap/implementation.md | 3 + internal/engine/collection_policy.go | 183 +++++++++++++++++++++ internal/engine/collection_policy_test.go | 192 ++++++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 internal/engine/collection_policy.go create mode 100644 internal/engine/collection_policy_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index e987568..2cc2681 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -260,6 +260,9 @@ and of the ownership transformations introduced next. Validation: focused policy-agreement/selection tests and the everyday check. Do not activate collection combination or implement the ownership layer here. +**Completion:** Implemented and verified with focused collection-policy tests and +`./scripts/check.sh`. + ## Stage 5 — Track Policy Lifetime And Record Ownership Scope: private scoped-state transformations built on Stage 4's resolution helpers. diff --git a/internal/engine/collection_policy.go b/internal/engine/collection_policy.go new file mode 100644 index 0000000..0a372d0 --- /dev/null +++ b/internal/engine/collection_policy.go @@ -0,0 +1,183 @@ +package engine + +type collectionPolicy struct { + identityFields []string + identityPresent bool + identityLocation *Location + explicitMode sequenceMode + explicitModePresent bool + modeLocation *Location +} + +type collectionPolicyResolution struct { + policy collectionPolicy + operation sequenceMode + declarations []sequenceDeclarationSpec + declarationsPresent bool + identityDeclarationPresent bool + modeDeclarationPresent bool + applicationIdentityFields []string + applicationIdentityPresent bool + effectiveIdentityFields []string +} + +// resolveCollectionPolicy reconciles declarations for one destination before +// peers are composed. The caller supplies only lower policy that is applicable +// to this destination; record survival and ownership are resolved elsewhere. +func resolveCollectionPolicy( + destination Path, + declarations []sequenceDeclarationSpec, + applicationIdentityFields []string, + lower *collectionPolicy, +) (collectionPolicyResolution, []Diagnostic) { + normalizedApplicationIdentityFields := cloneStrings(applicationIdentityFields) + if len(applicationIdentityFields) != 0 { + var err error + normalizedApplicationIdentityFields, err = normalizeSequenceIdentityFields(applicationIdentityFields) + if err != nil { + return collectionPolicyResolution{}, []Diagnostic{{ + Code: CodeOptionsInvalid, Severity: SeverityError, Path: destination.clone(), + Message: "application collection identity fields are invalid", + }} + } + } + result := collectionPolicyResolution{ + declarations: cloneSequenceDeclarations(declarations), + declarationsPresent: len(declarations) != 0, + applicationIdentityFields: normalizedApplicationIdentityFields, + applicationIdentityPresent: len(normalizedApplicationIdentityFields) != 0, + } + + var diagnostics []Diagnostic + var declaredIdentityFields []string + var declaredIdentityLocation *Location + var declaredMode sequenceMode + var declaredModeLocation *Location + for _, declaration := range declarations { + if declaration.identityPresent { + result.identityDeclarationPresent = true + if declaredIdentityFields == nil { + declaredIdentityFields = cloneStrings(declaration.identityFields) + declaredIdentityLocation = collectionDeclarationLocation(declaration.identityLocation, declaration.location) + } else if !equalSequenceIdentityFields(declaredIdentityFields, declaration.identityFields) { + diagnostics = append(diagnostics, collectionPolicyConflictDiagnostic( + destination, declaredIdentityLocation, + collectionDeclarationLocation(declaration.identityLocation, declaration.location), + "collection identity declarations conflict", + )) + } + } + if declaration.modePresent { + result.modeDeclarationPresent = true + if declaredMode == 0 { + declaredMode = declaration.mode + declaredModeLocation = collectionDeclarationLocation(declaration.modeLocation, declaration.location) + } else if declaredMode != declaration.mode { + diagnostics = append(diagnostics, collectionPolicyConflictDiagnostic( + destination, declaredModeLocation, + collectionDeclarationLocation(declaration.modeLocation, declaration.location), + "collection operation declarations conflict", + )) + } + } + } + if len(diagnostics) != 0 { + return collectionPolicyResolution{}, canonicalDiagnostics(diagnostics) + } + + if lower != nil && lower.identityPresent && declaredIdentityFields != nil && !equalSequenceIdentityFields(lower.identityFields, declaredIdentityFields) { + diagnostics = append(diagnostics, collectionPolicyConflictDiagnostic( + destination, declaredIdentityLocation, + collectionDeclarationLocation(lower.identityLocation, Location{}), + "collection identity declaration conflicts with retained identity", + )) + } + if result.applicationIdentityPresent { + if declaredIdentityFields != nil && !equalSequenceIdentityFields(normalizedApplicationIdentityFields, declaredIdentityFields) { + diagnostics = append(diagnostics, collectionPolicyConflictDiagnostic( + destination, declaredIdentityLocation, nil, + "collection identity declaration conflicts with application constraint", + )) + } + if lower != nil && lower.identityPresent && !equalSequenceIdentityFields(normalizedApplicationIdentityFields, lower.identityFields) { + diagnostics = append(diagnostics, collectionPolicyConflictDiagnostic( + destination, declaredIdentityLocation, + collectionDeclarationLocation(lower.identityLocation, Location{}), + "retained collection identity conflicts with application constraint", + )) + } + } + if len(diagnostics) != 0 { + return collectionPolicyResolution{}, canonicalDiagnostics(diagnostics) + } + + if declaredIdentityFields != nil { + result.policy.identityFields = declaredIdentityFields + result.policy.identityPresent = true + result.policy.identityLocation = cloneLocation(declaredIdentityLocation) + } else if lower != nil && lower.identityPresent { + result.policy.identityFields = cloneStrings(lower.identityFields) + result.policy.identityPresent = true + result.policy.identityLocation = cloneLocation(lower.identityLocation) + } + if result.applicationIdentityPresent { + result.effectiveIdentityFields = cloneStrings(normalizedApplicationIdentityFields) + } else if result.policy.identityPresent { + result.effectiveIdentityFields = cloneStrings(result.policy.identityFields) + } + + if result.modeDeclarationPresent { + result.policy.explicitMode = declaredMode + result.policy.explicitModePresent = true + result.policy.modeLocation = cloneLocation(declaredModeLocation) + result.operation = declaredMode + } else if lower != nil && lower.explicitModePresent { + result.policy.explicitMode = lower.explicitMode + result.policy.explicitModePresent = true + result.policy.modeLocation = cloneLocation(lower.modeLocation) + result.operation = lower.explicitMode + } else if len(result.effectiveIdentityFields) != 0 { + result.operation = sequenceModeUpsert + } else { + result.operation = sequenceModeReplace + } + + return result, nil +} + +func cloneSequenceDeclarations(declarations []sequenceDeclarationSpec) []sequenceDeclarationSpec { + if declarations == nil { + return nil + } + cloned := make([]sequenceDeclarationSpec, len(declarations)) + for index, declaration := range declarations { + cloned[index] = declaration + cloned[index].path = declaration.path.clone() + cloned[index].destination = declaration.destination.clone() + cloned[index].identityFields = cloneStrings(declaration.identityFields) + cloned[index].identityLocation = cloneLocation(declaration.identityLocation) + cloned[index].modeLocation = cloneLocation(declaration.modeLocation) + } + return cloned +} + +func collectionDeclarationLocation(specific *Location, fallback Location) *Location { + if specific != nil { + return cloneLocation(specific) + } + if fallback != (Location{}) { + return cloneLocation(&fallback) + } + return nil +} + +func collectionPolicyConflictDiagnostic(path Path, primary, related *Location, message string) Diagnostic { + diagnostic := Diagnostic{ + Code: CodeSequenceConflict, Severity: SeverityError, + Path: path.clone(), Message: message, Primary: cloneLocation(primary), + } + if related != nil { + diagnostic.Related = []Location{*cloneLocation(related)} + } + return diagnostic +} diff --git a/internal/engine/collection_policy_test.go b/internal/engine/collection_policy_test.go new file mode 100644 index 0000000..2496d6d --- /dev/null +++ b/internal/engine/collection_policy_test.go @@ -0,0 +1,192 @@ +package engine + +import ( + "reflect" + "testing" +) + +func TestResolveCollectionPolicyReconcilesDeclarationsBeforeComposition(t *testing.T) { + declarations := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + key: [namespace, id] + mode: append + - destination: [prompts] + key: [id, namespace, id] + - destination: [prompts] + mode: append +`, rootDirectiveContextSource).sequences + + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), declarations, nil, nil) + if len(diagnostics) != 0 { + t.Fatalf("policy diagnostics = %#v", diagnostics) + } + if !resolved.declarationsPresent || !resolved.identityDeclarationPresent || !resolved.modeDeclarationPresent { + t.Fatalf("declaration presence was lost: %#v", resolved) + } + wantFields := []string{"id", "namespace"} + if !reflect.DeepEqual(resolved.policy.identityFields, wantFields) || !reflect.DeepEqual(resolved.effectiveIdentityFields, wantFields) { + t.Fatalf("effective identity = %#v / %#v, want %#v", resolved.policy.identityFields, resolved.effectiveIdentityFields, wantFields) + } + if !resolved.policy.identityPresent || !resolved.policy.explicitModePresent || resolved.policy.explicitMode != sequenceModeAppend || resolved.operation != sequenceModeAppend { + t.Fatalf("resolved explicit policy = %#v, operation %d", resolved.policy, resolved.operation) + } + if len(resolved.declarations) != 3 || resolved.declarations[0].identityLocation == nil || resolved.declarations[2].location.Line == 0 { + t.Fatalf("declaration locations were not retained: %#v", resolved.declarations) + } +} + +func TestResolveCollectionPolicyRejectsConflictingPeersAtBothSites(t *testing.T) { + tests := []struct { + name string + body string + }{ + { + name: "identity fields", + body: `configkit: + sequences: + - destination: [prompts] + key: id + - destination: [prompts] + key: name +`, + }, + { + name: "explicit modes", + body: `configkit: + sequences: + - destination: [prompts] + mode: append + - destination: [prompts] + mode: replace +`, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + declarations := extractSequenceDeclarationsYAML(t, test.body, rootDirectiveContextSource).sequences + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), declarations, nil, nil) + if !reflect.DeepEqual(resolved, collectionPolicyResolution{}) || len(diagnostics) != 1 || diagnostics[0].Code != CodeSequenceConflict { + t.Fatalf("resolution/diagnostics = %#v / %#v", resolved, diagnostics) + } + diagnostic := diagnostics[0] + if diagnostic.Path.String() != "/prompts" || diagnostic.Primary == nil || len(diagnostic.Related) != 1 { + t.Fatalf("conflict does not identify both declaration sites: %#v", diagnostic) + } + if diagnostic.Primary.SourceID != "sequence-declarations" || diagnostic.Related[0].SourceID != "sequence-declarations" || diagnostic.Primary.Line == diagnostic.Related[0].Line { + t.Fatalf("conflict locations = %#v and %#v", diagnostic.Primary, diagnostic.Related) + } + }) + } +} + +func TestResolveCollectionPolicySelectsCurrentThenRetainedExplicitMode(t *testing.T) { + lower := &collectionPolicy{ + identityFields: []string{"id"}, + identityPresent: true, + explicitMode: sequenceModeAppend, + explicitModePresent: true, + modeLocation: &Location{SourceID: "lower", Line: 2}, + } + current := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + mode: upsert +`, rootDirectiveContextSource).sequences + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), current, nil, lower) + if len(diagnostics) != 0 || resolved.operation != sequenceModeUpsert || resolved.policy.explicitMode != sequenceModeUpsert || !resolved.policy.explicitModePresent { + t.Fatalf("current mode did not override retained mode: %#v, %#v", resolved, diagnostics) + } + + resolved, diagnostics = resolveCollectionPolicy(Keys("prompts"), nil, nil, lower) + if len(diagnostics) != 0 || resolved.operation != sequenceModeAppend || resolved.policy.explicitMode != sequenceModeAppend || !resolved.policy.explicitModePresent { + t.Fatalf("omitted current mode did not retain lower explicit mode: %#v, %#v", resolved, diagnostics) + } + if !reflect.DeepEqual(resolved.effectiveIdentityFields, []string{"id"}) { + t.Fatalf("retained identity = %#v", resolved.effectiveIdentityFields) + } +} + +func TestResolveCollectionPolicyInfersWithoutPersistingMode(t *testing.T) { + keyOnly := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + key: id +`, rootDirectiveContextSource).sequences + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), keyOnly, nil, nil) + if len(diagnostics) != 0 || resolved.operation != sequenceModeUpsert || resolved.policy.explicitModePresent { + t.Fatalf("identity-based mode inference = %#v, %#v", resolved, diagnostics) + } + + resolved, diagnostics = resolveCollectionPolicy(Keys("prompts"), nil, []string{"id"}, nil) + if len(diagnostics) != 0 || resolved.operation != sequenceModeUpsert || resolved.policy.identityPresent || !resolved.applicationIdentityPresent { + t.Fatalf("application identity was not kept distinct from YAML policy: %#v, %#v", resolved, diagnostics) + } + + noIdentity := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] +`, rootDirectiveContextSource).sequences + resolved, diagnostics = resolveCollectionPolicy(Keys("prompts"), noIdentity, nil, nil) + if len(diagnostics) != 0 || resolved.operation != sequenceModeReplace || resolved.policy.explicitModePresent { + t.Fatalf("unkeyed mode inference = %#v, %#v", resolved, diagnostics) + } + + explicitDefault := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + mode: replace +`, rootDirectiveContextSource).sequences + resolved, diagnostics = resolveCollectionPolicy(Keys("prompts"), explicitDefault, nil, nil) + if len(diagnostics) != 0 || resolved.operation != sequenceModeReplace || !resolved.policy.explicitModePresent { + t.Fatalf("explicit default mode lost its presence: %#v, %#v", resolved, diagnostics) + } +} + +func TestResolveCollectionPolicyChecksKeyOnlyAgainstApplicationConstraint(t *testing.T) { + declarations := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + key: [namespace, id] +`, rootDirectiveContextSource).sequences + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), declarations, []string{"id", "namespace", "id"}, nil) + if len(diagnostics) != 0 || resolved.operation != sequenceModeUpsert || !reflect.DeepEqual(resolved.effectiveIdentityFields, []string{"id", "namespace"}) { + t.Fatalf("equivalent key-only declaration was rejected: %#v, %#v", resolved, diagnostics) + } + + resolved, diagnostics = resolveCollectionPolicy(Keys("prompts"), declarations, []string{"slug"}, nil) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeSequenceConflict { + t.Fatalf("conflicting key-only declaration = %#v, %#v", resolved, diagnostics) + } + if diagnostics[0].Primary == nil || diagnostics[0].Path.String() != "/prompts" || len(diagnostics[0].Related) != 0 { + t.Fatalf("application conflict diagnostic = %#v", diagnostics[0]) + } +} + +func TestResolveCollectionPolicyChecksRetainedIdentityAgainstApplicationConstraint(t *testing.T) { + lower := &collectionPolicy{identityFields: []string{"id"}, identityPresent: true} + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), nil, []string{"name"}, lower) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeSequenceConflict || diagnostics[0].Path.String() != "/prompts" { + t.Fatalf("retained/application conflict = %#v, %#v", resolved, diagnostics) + } +} + +func TestResolveCollectionPolicyRequiresCurrentIdentityToAgreeWithRetainedIdentity(t *testing.T) { + declarations := extractSequenceDeclarationsYAML(t, `configkit: + sequences: + - destination: [prompts] + key: name +`, rootDirectiveContextSource).sequences + lower := &collectionPolicy{ + identityFields: []string{"id"}, + identityPresent: true, + identityLocation: &Location{SourceID: "lower", Line: 4}, + } + resolved, diagnostics := resolveCollectionPolicy(Keys("prompts"), declarations, nil, lower) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeSequenceConflict || diagnostics[0].Path.String() != "/prompts" { + t.Fatalf("current/retained identity conflict = %#v, %#v", resolved, diagnostics) + } + if diagnostics[0].Primary == nil || len(diagnostics[0].Related) != 1 || diagnostics[0].Related[0].SourceID != "lower" { + t.Fatalf("current/retained conflict locations = %#v", diagnostics[0]) + } +}