diff --git a/docs/reference/coverage.md b/docs/reference/coverage.md index a7376e5e..c0e78f75 100644 --- a/docs/reference/coverage.md +++ b/docs/reference/coverage.md @@ -376,7 +376,7 @@ The side-by-side comparison with the Probot Settings app lives in the migration **Fields the Pages update ignores** (`pages`) -- Parsing the file refuses the fields only the site GET reports (url, html_url, status, custom_404, protected_domain_state, pending_domain_unverified_at, https_certificate) and a source.path other than / or /docs, the only directories Pages publishes from. +- Parsing the file refuses the fields only the site GET reports (url, html_url, status, custom_404, protected_domain_state, pending_domain_unverified_at, https_certificate), a source.path other than / or /docs, the only directories Pages publishes from, and an empty mapping, which the update endpoint rejects; the published schema carries the same three. - Sent, GitHub would drop the fields and report them as drift on every run, and the path would 422 at apply. - A workflow-built site still reports its source, so build_type: workflow beside a source converges. diff --git a/docs/sections/pages.docs.yml b/docs/sections/pages.docs.yml index 42dcd385..83ec474a 100644 --- a/docs/sections/pages.docs.yml +++ b/docs/sections/pages.docs.yml @@ -28,8 +28,9 @@ coverage: notes: - >- Parsing the file refuses the fields only the site GET reports (url, html_url, status, - custom_404, protected_domain_state, pending_domain_unverified_at, https_certificate) and a - source.path other than / or /docs, the only directories Pages publishes from. + custom_404, protected_domain_state, pending_domain_unverified_at, https_certificate), a + source.path other than / or /docs, the only directories Pages publishes from, and an empty + mapping, which the update endpoint rejects; the published schema carries the same three. - >- Sent, GitHub would drop the fields and report them as drift on every run, and the path would 422 at apply. diff --git a/docs/upgrading/v2-to-v3.md b/docs/upgrading/v2-to-v3.md index 04a894a9..d6c06bfd 100644 --- a/docs/upgrading/v2-to-v3.md +++ b/docs/upgrading/v2-to-v3.md @@ -81,7 +81,7 @@ The detail is in [v2-to-v3/refusals-repository.md](v2-to-v3/refusals-repository. | Break | v2 | v3 | What the old form does now | |---|---|---|---| | Repository: GET-only keys, commit-message pairs, topics, and security sub-keys | `has_downloads: false` drifted on every run; an illegal squash pair or a bad topic 422ed at apply | Refused at parse: a GET-only key (`has_downloads`, `has_pages`, `custom_properties`, ids, urls, counts), an unknown `security_and_analysis` sub-key, a commit message without its title or in an illegal squash pair, a malformed or empty topic, more than 20 topics | Validation fails naming the key and the fix; [section 37](v2-to-v3/refusals-repository.md#37-repository-get-only-keys-commit-pairs-topics-and-security-sub-keys) | -| Pages: GET-only site fields and the source path | `custom_404: true` rode the PUT, was dropped, and drifted forever; `source.path: /src` 422ed | `url`, `html_url`, `status`, `custom_404`, `protected_domain_state`, `pending_domain_unverified_at`, and `https_certificate` are refused; `source.path` is `/` or `/docs` | Validation fails naming the key; [section 38](v2-to-v3/refusals-repository.md#38-pages-get-only-fields-and-the-source-path) | +| Pages: GET-only site fields, the source path, and the empty mapping | `custom_404: true` rode the PUT, was dropped, and drifted forever; `source.path: /src` 422ed; `pages: {}` ran and configured nothing | `url`, `html_url`, `status`, `custom_404`, `protected_domain_state`, `pending_domain_unverified_at`, and `https_certificate` are refused; `source.path` is `/` or `/docs`; an empty mapping is refused naming its three fixes | Validation fails naming the key; [section 38](v2-to-v3/refusals-repository.md#38-pages-get-only-fields-and-the-source-path) | | Actions: reported-only fields, a closed `selected_actions`, enums and bounds | `maximum_allowed_days: 400` was re-PUT on every run; a misspelled `selected_actions` key was re-PUT silently forever | `selected_actions_url`, `artifact_and_log_retention.maximum_allowed_days`, and `oidc_customization_sub.sub_claim_prefix` are refused; `selected_actions` is closed to its three keys and typed; `approval_policy` is an enum; claim keys match `^[A-Za-z0-9_]+$` and are unique; retention days and cache limits are positive integers; `include_claim_keys` beside `use_default: true` is refused; `sha_pinning_required` is a known boolean | Validation fails naming the key and the fix; [section 39](v2-to-v3/refusals-repository.md#39-actions-fields-that-could-never-converge) | | Code scanning and code quality setup: GET-only keys, languages, the runner pair | `schedule: weekly` and a `runner_label` under `runner_type: standard` each drifted forever; `languages: [javascript]` never matched the pair GitHub reports | `schedule` and `updated_at` are refused; languages are spelled as the PATCH takes them (`javascript-typescript`, no `rust`); a string `runner_label` goes only with `runner_type: labeled`, which requires one | Validation fails naming the key and the spelling to write; a key the GET never echoes earns the never-converges note; [section 40](v2-to-v3/refusals-repository.md#40-setup-sections-get-only-keys-languages-and-the-runner-pair) | | Check-suite preferences: `app_id` | `app_id: 0` and a negative id were PATCHed on every run; a repeated id was collapsed by GitHub with nothing reading it back | `app_id` is a positive integer and appears once | Validation fails naming the entries; [section 44](v2-to-v3/refusals-repository.md#44-check-suite-preferences-app_id) | diff --git a/docs/upgrading/v2-to-v3/refusals-repository.md b/docs/upgrading/v2-to-v3/refusals-repository.md index 69d012e5..1761452f 100644 --- a/docs/upgrading/v2-to-v3/refusals-repository.md +++ b/docs/upgrading/v2-to-v3/refusals-repository.md @@ -47,6 +47,8 @@ v3 pages.custom_404: GitHub reports this field on the Pages site and the updat `url`, `html_url`, `status`, `custom_404`, `protected_domain_state`, `pending_domain_unverified_at`, and `https_certificate` are refused. `public: false` stays declarable: Enterprise Cloud shares the same host, so the check run's note beside the drift says why it cannot converge elsewhere. +`pages: {}` is refused as well: the update endpoint rejects an empty body, so the mapping configured nothing, and the pre-release v3 builds only noted it at plan time. The message names the three intents it could have meant: declare a field, declare `pages: null` to disable the site, or remove the section. + For `@vivswan/github-settings-as-code` consumers, `source.path` under `SectionInput<"pages">` (and so `SettingsFile`) is `"/" | "/docs"`, where the pre-release v3 builds typed `string`; another path there stops compiling. Fix: delete the reported field from the file, and publish from `/` or `/docs`. diff --git a/src/sections/pages/index.ts b/src/sections/pages/index.ts index 002b3131..eecb8968 100644 --- a/src/sections/pages/index.ts +++ b/src/sections/pages/index.ts @@ -91,12 +91,6 @@ export const pagesSection = { }); return plan; } - if (Object.keys(desired).length === 0) { - plan.notes.push( - "pages: declared as an empty mapping, which configures nothing (the update endpoint rejects an empty body). Declare at least one field, use pages: null to disable the site, or remove the section", - ); - return plan; - } // The source is split off so the no-source form never carries a source key at all: an own // `source: undefined` would count as a remainder below. const { source, ...restConfig } = desired; diff --git a/src/sections/pages/schema.ts b/src/sections/pages/schema.ts index 5d00862d..ede5b3d8 100644 --- a/src/sections/pages/schema.ts +++ b/src/sections/pages/schema.ts @@ -33,6 +33,10 @@ const READ_ONLY_SITE_FIELDS = { "GitHub provisions the certificate for `cname`; declare `cname` and `https_enforced`", } satisfies Record; +/** The refusal for `pages: {}`: each of the three fixes is a different intent, so all three are named. */ +const EMPTY_SITE_FIX = + "an empty mapping configures nothing; declare at least one field, declare pages: null to disable the site, or remove the section"; + // Not exported: consumers spell it NonNullable. The definition id stays "PagesConfig"; // moving it onto the nullable wrapper would change the published schema. const PagesSite = open({ @@ -44,6 +48,14 @@ const PagesSite = open({ }) .check( rule((site, refineCtx) => { + // The update endpoint rejects an empty body, so a mapping with no field can only ever be a mistake. A + // library caller's `{ field: undefined }` serializes to the same empty body, so an undefined field is no field. + if (Object.values(site).every((value) => value === undefined)) { + refineCtx.addIssue({ + code: "custom", + message: EMPTY_SITE_FIX, + }); + } // The type hides these keys; the open() parse lets them reach here. for (const [key, fix] of Object.entries(READ_ONLY_SITE_FIELDS)) { if ((site as Record)[key] !== undefined) { @@ -60,7 +72,8 @@ const PagesSite = open({ ) .meta({ id: "PagesConfig", - // The published schema carries the same refusal, so an editor flags the key as the file is written. + // The published schema carries the same refusals, so an editor flags the key or the empty mapping as the file is written. + minProperties: 1, not: { anyOf: Object.keys(READ_ONLY_SITE_FIELDS).map((key) => ({ required: [key] })) }, }); diff --git a/test/published-schema.test.ts b/test/published-schema.test.ts index 3fdaa55a..84a53ad9 100644 --- a/test/published-schema.test.ts +++ b/test/published-schema.test.ts @@ -68,6 +68,9 @@ describe("the published schema and the runtime agree on the shapes the corpus ne false, ], ["a bad policy value", { rulesets: { _undeclared: "remove", entries: [] } }, false], + // The update endpoint rejects an empty body; minProperties mirrors the parse refusal so an editor flags it too. + ["an empty pages mapping", { pages: {} }, false], + ["a pages mapping with one field", { pages: { https_enforced: true } }, true], [ "the nested variables knob, wrapped", prod({ variables: { _undeclared: "keep", entries: [{ name: "A", value: "1" }] } }), diff --git a/test/sections/pages/pages.test.ts b/test/sections/pages/pages.test.ts index 402f0ee0..ee816f98 100644 --- a/test/sections/pages/pages.test.ts +++ b/test/sections/pages/pages.test.ts @@ -197,17 +197,6 @@ describe("pages", () => { expect(result.ops[0]?.payload).toEqual({ source: { branch: "main", path: "/" } }); }); - test("an empty pages mapping is a note, not an empty PUT", async () => { - const api = new MockApi({ [GET]: { data: {} } }); - expect(await plan(api, {})).toEqual({ - ops: [], - notes: [ - "pages: declared as an empty mapping, which configures nothing (the update endpoint rejects an empty body). Declare at least one field, use pages: null to disable the site, or remove the section", - ], - drift: [], - }); - }); - test("a passthrough key named like a prototype member still reaches the second PUT", async () => { // The create body is a plain object, so an `in` check would see Object.prototype's members and drop such a key from the remainder. const api = new MockApi({}); diff --git a/test/sections/pages/schema.test.ts b/test/sections/pages/schema.test.ts index b17b2280..7b7ddfef 100644 --- a/test/sections/pages/schema.test.ts +++ b/test/sections/pages/schema.test.ts @@ -56,6 +56,15 @@ describe("a Pages field the update cannot set is refused at parse, naming the ke ]); }); + test("an empty mapping is refused at parse, naming the three intents it could have meant", () => { + // The update endpoint rejects an empty body, so the old plan-time note left a file that ran and configured nothing. + const refusal = + "pages: an empty mapping configures nothing; declare at least one field, declare pages: null to disable the site, or remove the section"; + expect(issues({})).toEqual([refusal]); + // A library caller's undefined field is dropped by JSON, so the body GitHub would see is the same empty one. + expect(issues({ https_enforced: undefined })).toEqual([refusal]); + }); + test("the fields the update takes parse, and null turns the site off", () => { expect( issues({