Skip to content

Commit b761286

Browse files
committed
docs(recipe): correct ADR-020 merge-machinery and profile-precedent claims
Signed-off-by: Brian Lockwood <lockwobr@gmail.com>
1 parent 03f898b commit b761286

1 file changed

Lines changed: 41 additions & 5 deletions

File tree

docs/design/020-recipe-resolution-policy.md

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,28 @@ Note this is deliberately **not** "any non-`nil` policy replaces the parent".
176176
Replacement keys on the *list*, not on the policy pointer. Keying on the
177177
pointer would make `resolution: {}` clear an inherited list.
178178

179+
**This requires a new merge clause; there is no last-wins path to reuse.**
180+
`RecipeMetadataSpec.Merge` has no generic replace behavior. Every field is
181+
bespoke: constraints merge by name, componentRefs merge by name and then
182+
field-by-field, validation merges per phase. `Resolution` must be added
183+
explicitly, inheriting when the child's `StrictDimensions` is `nil` and
184+
replacing when it is non-`nil`.
185+
186+
Getting this wrong has a precedent in this exact file. The `#1000` note on
187+
`Merge` records that per-phase *pointer replace* was "the only list-shaped
188+
field with replace semantics, and it silently dropped inherited checks when a
189+
descendant declared its own phase block," and it was fixed to field-by-field
190+
merge. `StrictDimensions` is list-shaped, so an implementer who keys
191+
replacement on the `*ResolutionPolicy` pointer rather than on the slice
192+
recreates that bug: any overlay adding a `resolution:` block clears inherited
193+
strict dimensions for its whole subtree, and the split-overlay query succeeds
194+
again once `requireOSIfNeeded` is deleted.
195+
196+
The mid-chain case is therefore pinned directly: a test asserts a descendant
197+
declaring `resolution: {}` still resolves to an ancestor's `[os]` through the
198+
merge fold. The decode round-trip and the chain-exhausted default are
199+
separately pinned and do not cover it.
200+
179201
The walk stops at the first non-`nil` list rather than unioning, because
180202
element union makes opting out impossible: a subtree could never drop a
181203
dimension its ancestor declared. Stopping keeps the override total and
@@ -210,14 +232,28 @@ No dimension name appears in any Go type, function, or field. The only place
210232
`os` appears is catalog data.
211233

212234
This follows the `spec.profile` precedent from
213-
[ADR-015](015-recipe-configuration-profiles.md): an overlay-scoped spec field
214-
resolved after composition and never copied into a hydrated `RecipeResult`.
235+
[ADR-015](015-recipe-configuration-profiles.md) for its **lifecycle only**: an
236+
overlay-scoped spec field, resolved after composition, never copied into a
237+
hydrated `RecipeResult`.
238+
239+
The precedent stops there, and the difference matters to an implementer.
240+
`resolveProfileDeclaration` does not use `Merge` at all and *errors* when a
241+
resolved composition carries more than one declaration
242+
(`profile_resolution.go`), because a profile is single-declaration by design.
243+
The resolution policy requires the opposite: divergence is legal, several
244+
overlays may declare different lists, and candidates are evaluated
245+
independently against their own resolved list. Reusing
246+
`resolveProfileDeclaration`-shaped logic would import its
247+
multiple-declarations-is-an-error rule and break exactly the per-candidate
248+
independence this design rests on. Candidate evaluation against a
249+
not-applied overlay is likewise new machinery, not something `spec.profile`
250+
already does.
215251

216252
### Scope and inheritance
217253

218-
The policy inherits through `spec.base` using the existing
219-
`resolveInheritanceChain` + `RecipeMetadataSpec.Merge` machinery (last wins),
220-
so a subtree can override it without touching embedded data:
254+
The policy inherits along the `spec.base` chain that
255+
`resolveInheritanceChain` already produces, so a subtree can override it
256+
without touching embedded data:
221257

222258
```yaml
223259
# a deliberately OS-agnostic subtree

0 commit comments

Comments
 (0)