diff --git a/docs/operate/layering.md b/docs/operate/layering.md index 40e84399..9647e6a5 100644 --- a/docs/operate/layering.md +++ b/docs/operate/layering.md @@ -82,16 +82,16 @@ Inside a keyed entry, an underscore key is a field like any other: a closed entr Layers fold low to high. At each step the higher layer's value meets whatever the lower layers built so far. -| Higher layer | Over | Result | -| ---------------------------------------------------------------------------------------------------------- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| A mapping | A mapping | Merged key by key. Lower keys keep their order, higher-only keys follow | -| A scalar, a list, or a YAML-tagged value | Anything | Replaces | -| `null` (at any depth) | Anything, or nothing | Wins, like any value, and is written as such: `null` is the EMPTY or OFF state on GitHub (`pages: null` turns Pages off, `protection: null` removes a branch's protection), never a marker that deletes a key. Where a key or a whole section admits `null` is the file's own rule ([null is the empty state](../reference/semantics.md#null-is-the-empty-state)); a `null` a key does not admit is refused at validation ([refusals](#refusals)) | -| `_remove: true` on a keyed entry | A lower entry under the same key | Drops that lower entry, with a notice naming the layer and the removal by its index; the marker never reaches the rendered file. Refused where nothing meets it: under `replace`, inside an entry copied whole, or when no lower layer declares the key (see [Refusals](#refusals)). A removal names its key and nothing else | -| A list section's entries, layering `deep` | Its entries | Union by the section's key (the table below). A same-key pair merges field by field, the higher fields winning, in the lower entry's place; a nested keyed list (a ruleset's `rules` by `type`, an environment's `variables` by name) unions the same way under `deep`, its same-key pairs merging field by field too, in its bare or `{_undeclared, entries}` form alike (a lower wrapper's policy is inherited by a higher bare list); new keys are appended in the higher order | -| A list section's entries, layering `shallow` | Its entries | Union by the section's key. A same-key entry is swapped for the higher one in place, new keys are appended | -| A list section's entries, layering `replace` | Its entries | The higher list wins. An omitted `_undeclared` still inherits the lower layer's | -| Any other list (`topics`, a ruleset's `bypass_actors`, `check_suite_preferences.auto_trigger_checks`, ...) | A list | Replaces, whatever the run's layering | +| Higher layer | Over | Result | +| ---------------------------------------------------------------------------------------------------------- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| A mapping | A mapping | Merged key by key. Lower keys keep their order, higher-only keys follow | +| A scalar, a list, or a YAML-tagged value | Anything | Replaces (a label's `name` aside: the rename rule below) | +| `null` (at any depth) | Anything, or nothing | Wins, like any value, and is written as such: `null` is the EMPTY or OFF state on GitHub (`pages: null` turns Pages off, `protection: null` removes a branch's protection), never a marker that deletes a key. Where a key or a whole section admits `null` is the file's own rule ([null is the empty state](../reference/semantics.md#null-is-the-empty-state)); a `null` a key does not admit is refused at validation ([refusals](#refusals)) | +| `_remove: true` on a keyed entry | A lower entry under the same key | Drops that lower entry, with a notice naming the layer and the removal by its index; the marker never reaches the rendered file. Refused where nothing meets it: under `replace`, inside an entry copied whole, or when no lower layer declares the key (see [Refusals](#refusals)). A removal names its key and nothing else | +| A list section's entries, layering `deep` | Its entries | Union by the section's key (the table below). A same-key pair merges field by field, the higher fields winning (a label's `name` aside: the rename rule below), in the lower entry's place; a nested keyed list (a ruleset's `rules` by `type`, an environment's `variables` by name) unions the same way under `deep`, its same-key pairs merging field by field too, in its bare or `{_undeclared, entries}` form alike (a lower wrapper's policy is inherited by a higher bare list); new keys are appended in the higher order | +| A list section's entries, layering `shallow` | Its entries | Union by the section's key. A same-key entry is swapped for the higher one in place, new keys are appended | +| A list section's entries, layering `replace` | Its entries | The higher list wins. An omitted `_undeclared` still inherits the lower layer's | +| Any other list (`topics`, a ruleset's `bypass_actors`, `check_suite_preferences.auto_trigger_checks`, ...) | A list | Replaces, whatever the run's layering | Under `shallow` and `deep` an empty higher list adds nothing. To clear a list, write `_layering: replace` with an empty list. @@ -115,7 +115,7 @@ Every list section layers by the key its planner matches entries by, folded the | `deploy_keys` | `title` | verbatim | | `secret_scanning_custom_patterns` | `name` | verbatim | -So a fleet `Bug` and a repository `bug` are one label, a fleet `MY_SECRET` and a repository `my_secret` are one secret, and a fleet `Prod` environment and a repository `prod` are one environment, their variables unioned by name. When the spellings differ, the higher layer's spelling is the one written, under every directive. +So a fleet `Bug` and a repository `bug` are one label, a fleet `MY_SECRET` and a repository `my_secret` are one secret, and a fleet `Prod` environment and a repository `prod` are one environment, their variables unioned by name. When the spellings differ, the higher layer's spelling is the one written, under every directive, except a higher label's `name` where the rename rule below writes the lower one. Within one layer two entries may not share a key, at any depth; the fold refuses that layer (see [Refusals](#refusals)). @@ -123,17 +123,32 @@ Under `shallow` a same-key entry is swapped whole, its nested lists with it: onl Under `deep` a lower entry that two higher entries both claim (a lower label renaming into a name one higher entry declares while another declares its old name) is superseded by both as written: only a one-to-one pair merges field by field, so the rendered document never carries two entries claiming one key. +Under `deep` a higher entry may name a label by the `new_name` a lower layer gives it, and the two merge as one pair. The pair takes the higher entry's `name`, like any field, unless that leaves a name the pair claimed unclaimed and the lower entry's `name` leaves none. Then the lower `name` is written, so apply still renames the live label: + +```text +lower layer {name: bug, new_name: defect, color: d73a4a} +higher layer {name: defect, description: a defect} + +deep renders {name: bug, new_name: defect, color: d73a4a, description: a defect} +shallow renders {name: defect, description: a defect} +``` + +Under `shallow` the swap drops the rename with the lower entry: against a repository that still holds `bug`, apply creates `defect` and, under `_undeclared: delete`, deletes `bug`. + +Names count as the section folds them, so over that lower layer a higher `{name: defect, new_name: Defect}` claims one name and renders `{name: bug, new_name: Defect}` under `deep`. A higher entry that renames the label on to a third name (`{name: defect, new_name: flaw}`) is written under its own two names under both directives, since one entry cannot claim all three. + The rules above decide the content. The written file's order is the canonical one every rendered document has, the same order `mode: snapshot` writes: - sections in the order the action applies them - keys as the schema declares them -- the entries of every keyed list sorted by their identity +- the entries of every keyed list sorted by their identity, except the lists kept as written below -Reordering keys, or the entries of a keyed list, in a layer changes nothing in the rendered file. The lists kept as written do change it: +Reordering keys, or the entries of a sorted list, in a layer changes nothing in the rendered file. Reordering a list kept as written does change it: - `branches`: GitHub applies overlapping wildcard rules in creation order, so the union keeps the lower order and appends - the `pinned: true` environments: their order is the pin rank -- `bypass_actors`, the mapping list with no layering identity, so a higher list replaces the lower one and never unions with it (the planner still pairs actors by `actor_type` plus `actor_id` for its drift lines); `reviewers` is not kept as written, since the fold unions it by type and id +- `bypass_actors`, the mapping list with no layering identity, so a higher list replaces the lower one and never unions with it (the planner still pairs actors by `actor_type` plus `actor_id` for its drift lines) +- an environment's `reviewers`: the fold unions them by type and id, keeping the lower order and appending, and a reviewer has no one identity field to sort by - every scalar list: `topics`, `include` patterns The `_undeclared` knob across layers: @@ -239,7 +254,9 @@ Reading it back: ## Null wins, and _remove drops an entry -The fold is a cascade: the higher layer's value wins, and `null` is a value like any other. Our `null` is the EMPTY or OFF state on GitHub, not the CSS `unset` that falls back to a default; an undeclared key keeps GitHub's current value, which CSS has no word for; and a keyed list unions by key, where `_remove` drops one entry. One example per rule, lowest layer first: +The fold is a cascade: the higher layer's value wins (a label's `name` aside: [the rename rule](#the-rules)), and `null` is a value like any other. Our `null` is the EMPTY or OFF state on GitHub, not the CSS `unset` that falls back to a default; an undeclared key keeps GitHub's current value, which CSS has no word for; and a keyed list unions by key, where `_remove` drops one entry. + +One example per rule, lowest layer first: ```yaml layer # .github/settings/fleet.yml @@ -401,6 +418,7 @@ The per-layer validation catches what a standalone settings file could not say, | Two entries under one key in one list (`labels: [{name: bug}, {name: docs}, {name: Bug}]`) | Validation: `labels[2].name: "Bug" names the same label as "bug" declared earlier; keep exactly one entry per label` | | Two milestones under one title in one list (`milestones: [{title: v1}, {title: v1, state: closed}]`) | Validation: `milestones[1].title: "v1" names the same milestone as "v1" declared earlier; keep exactly one entry per milestone` | | Two rules of one type in one ruleset (`rulesets: [{name: main, rules: [{type: deletion}, {type: deletion}]}]`) | Validation: `rulesets[0].rules: the ruleset "main" lists the rule type "deletion" more than once, and GitHub keeps one rule per type - declare each type once` | +| One bypass actor listed twice in one ruleset (`rulesets: [{name: main, bypass_actors: [{actor_type: Team, actor_id: 1}, {actor_type: Team, actor_id: 1}]}]`) | Validation: `rulesets[0].bypass_actors[1]: "Team 1 (bypass_actors[1])" names the same bypass actor of the "main" ruleset as "Team 1 (bypass_actors[0])" declared earlier; keep exactly one entry per bypass actor of the "main" ruleset` | | Two variables of one environment under one uppercased name, in either form (`environments: [{name: prod, variables: {entries: [{name: region, value: eu}, {name: REGION, value: us}]}}]`) | Validation: `environments[0].variables.entries[1].name: "REGION" names the same variable of the "prod" environment as "region" declared earlier; keep exactly one entry per variable of the "prod" environment` | | A workflow named by its file and its path in one list (`workflows: [{path: ci.yml, state: active}, {path: .github/workflows/ci.yml, state: disabled}]`) | Validation: `workflows[1].path: ".github/workflows/ci.yml" names the same workflow as "ci.yml" declared earlier; keep exactly one entry per workflow` | diff --git a/src/engine/layers.ts b/src/engine/layers.ts index 0ec83600..ebfd2561 100644 --- a/src/engine/layers.ts +++ b/src/engine/layers.ts @@ -301,6 +301,39 @@ function placementsOf( }); } +/** + * A one-to-one pair under deep, merged field by field. Under the higher key field alone, `{name: defect}` over + * `{name: bug, new_name: defect}` would rename `defect` to itself and stop claiming `bug`: the planner then deletes a + * live `bug` as undeclared and creates `defect`, which takes the label off every issue that carries it. + */ +function mergedPair( + union: Union, + below: unknown, + lowerKeys: readonly string[], + placement: Placed, + site: string, +): unknown { + const { keys, keyField } = union.keyed; + const merged = mergeValue(below, placement.item, site, union.step, entryScope(union.keyed)); + const lowerKey = isPlainObject(below) ? own(below, keyField) : undefined; + if (!isPlainObject(merged) || lowerKey === undefined) { + return merged; + } + + const claimed = [...lowerKeys, ...placement.keys]; + const keepsEveryClaim = (entry: Readonly>): boolean => { + const held = keys(entry) ?? []; + return claimed.every((key) => held.includes(key)); + }; + if (keepsEveryClaim(merged)) { + return merged; + } + + const underLowerKey = { ...merged }; + put(underLowerKey, keyField, lowerKey); + return keepsEveryClaim(underLowerKey) ? underLowerKey : merged; +} + /** * A lower entry under the higher entries placed on it: kept when none claims it; else, per claimant, dropped by a * removal (noticed), merged field by field as a one-to-one pair under deep, or the claimant as written. @@ -330,7 +363,7 @@ function placeOver(union: Union, below: unknown, index: number): unknown[] { ) === 1 && claims(placement.keys, union.lowerKeys) === 1; out.push( union.directive === "deep" && paired - ? mergeValue(below, placement.item, site, union.step, entryScope(union.keyed)) + ? mergedPair(union, below, keys, placement, site) : copiedAsWritten(placement.item, union.keyed, site, "swapped", union.step), ); } diff --git a/test/e2e/generators/oracle.test.ts b/test/e2e/generators/oracle.test.ts index bfd74cbb..44150968 100644 --- a/test/e2e/generators/oracle.test.ts +++ b/test/e2e/generators/oracle.test.ts @@ -1485,7 +1485,7 @@ describe("foldMergeLayers (the oracle's own dialect)", () => { }, }, { - name: "a label renaming into a name a higher entry declares is one resource: the higher entry stands in its slot, merged under deep", + name: "a label renaming into a name a higher entry declares is one resource: under deep the higher entry's fields merge into the rename, which keeps the name it renames from", layering: "deep", layers: stack( { labels: [{ name: "bug", new_name: "defect", color: "111111" }, { name: "docs" }] }, @@ -1495,7 +1495,7 @@ describe("foldMergeLayers (the oracle's own dialect)", () => { merged: { labels: { [UNDECLARED_KEY]: "delete", - entries: [{ name: "Defect", new_name: "defect", color: "222222" }, { name: "docs" }], + entries: [{ name: "bug", new_name: "defect", color: "222222" }, { name: "docs" }], }, }, notices: [], diff --git a/test/e2e/generators/oracle.ts b/test/e2e/generators/oracle.ts index 110ecdf8..57bb1691 100644 --- a/test/e2e/generators/oracle.ts +++ b/test/e2e/generators/oracle.ts @@ -1128,6 +1128,29 @@ function sameResource(a: readonly string[], b: readonly string[]): boolean { return a.some((key) => b.includes(key)); } +/** + * A deep pair names one resource, so its merged entry should still answer to every name either entry answered to: a + * label declared above a lower rename by the rename's target must leave the old name declared, or apply deletes the + * live label under it. Where no key field answers to all (a rename continued under a third name), the higher layer's + * stands. + */ +function pairKeepingNamesWhenItCan( + merged: Json, + below: Json, + names: readonly string[], + keyed: KeyedList, +): Json { + const field = keyed.keyField; + const candidates = Object.hasOwn(below, field) + ? [merged, { ...merged, [field]: below[field] }] + : [merged]; + const answersToAll = (entry: Json): boolean => { + const answers = keyed.keysOf(entry) ?? []; + return names.every((name) => answers.includes(name)); + }; + return candidates.find(answersToAll) ?? merged; +} + /** * The higher entries claiming the lower entry at `index`, in their order: a removal drops it and is a notice naming * the removal's INDEX in the higher list, never a key value; a one-to-one pair merges field by field under deep; any @@ -1150,12 +1173,12 @@ function placeClaimants(union: KeyedUnion, index: number): Json[] { ).length; const paired = claimedBy === 1 && claimsLower === 1; if (union.directive === "deep" && paired) { - return [ - mergeTrees(below, entry, `${union.path}[${h}]`, union.site, { - keyed: union.keyed, - prefix: "", - }), - ]; + const merged = mergeTrees(below, entry, `${union.path}[${h}]`, union.site, { + keyed: union.keyed, + prefix: "", + }); + const names = [...keys, ...(union.higherKeys[h] ?? [])]; + return [pairKeepingNamesWhenItCan(merged, below, names, union.keyed)]; } if (carriesNestedRemoval(entry, union.keyed)) { throw new FoldRefused(union.site.layer); diff --git a/test/engine/layers.test.ts b/test/engine/layers.test.ts index 4c70516b..5944b9de 100644 --- a/test/engine/layers.test.ts +++ b/test/engine/layers.test.ts @@ -219,9 +219,12 @@ describe("mergeLayers: keyed sections", () => { }, ); - /** The labels planner over an empty repository rejects two entries claiming one label, so a merged document it plans is one apply accepts. */ - async function planLabels(entries: readonly Record[]) { - const api = new MockApi({ "GET /repos/o/r/labels?per_page=100&page=1": { data: [] } }); + /** The labels planner rejects two entries claiming one label, so a merged document it plans is one apply accepts; `live` is the repository's labels, none unless given. */ + async function planLabels( + entries: readonly Record[], + live: readonly Record[] = [], + ) { + const api = new MockApi({ "GET /repos/o/r/labels?per_page=100&page=1": { data: live } }); const plan = unwrap( await labelsSection.plan( planContext(labelsSection, api, REPO), @@ -267,13 +270,22 @@ describe("mergeLayers: keyed sections", () => { [{ name: "Bug", new_name: "Defect" }], ], [ - "the plain label above the rename, merged: the lower rename target rides along as a self-rename", + "the plain label above the rename, merged: the rename keeps the name it renames from and takes the higher color", "deep", { lower: [{ name: "bug", new_name: "defect" }], higher: [{ name: "defect", color: "ffffff" }], }, - [{ name: "defect", new_name: "defect", color: "ffffff" }], + [{ name: "bug", new_name: "defect", color: "ffffff" }], + ], + [ + "a rename above the rename it continues, merged: the higher layer's two names stand as written, since one entry cannot claim all three names", + "deep", + { + lower: [{ name: "bug", new_name: "defect" }], + higher: [{ name: "defect", new_name: "flaw" }], + }, + [{ name: "defect", new_name: "flaw" }], ], [ "the rename above the plain label, merged: the lower color rides along", @@ -302,6 +314,32 @@ describe("mergeLayers: keyed sections", () => { }, ); + // The folded shape cannot show what is lost: only a plan against the label under its old name tells a rename from a + // delete and a create, which under the section's default delete policy takes the label off every issue carrying it. + test.each<[string, Record[] | null, Record]>([ + ["alone (the control)", null, { name: "bug", new_name: "defect", color: "d73a4a" }], + [ + "under a higher layer adding a description by the new name", + [{ name: "defect", description: "a defect" }], + { name: "bug", new_name: "defect", color: "d73a4a", description: "a defect" }, + ], + ])( + "a lower layer's rename, %s, still renames the live label: one update, never a delete and a create", + async (_case, higher, entry) => { + const fleet = layer("fleet", { + labels: [{ name: "bug", new_name: "defect", color: "d73a4a" }], + }); + const layers = higher === null ? [fleet] : [fleet, layer("repo", { labels: higher })]; + const folded = mergeLayers(layers, { layering: "deep" })._unsafeUnwrap(); + const { entries } = (folded.settings as { labels: { entries: Record[] } }) + .labels; + expect( + await planLabels(entries, [{ name: "bug", color: "d73a4a", description: null }]), + ).toEqual(['updating label "defect"']); + expect(entries).toEqual([entry]); + }, + ); + test.each([ ["rename first", [{ name: "bug", new_name: "defect" }, { name: "docs" }]], ["rename last", [{ name: "docs" }, { name: "bug", new_name: "defect" }]],