From 2ea951a3476149ca0fa3b8e7ef5cf92c76fb66da Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:05:21 -0400 Subject: [PATCH 1/2] feat(pages)!: refuse an empty pages mapping when the file is parsed The update endpoint rejects an empty body, so pages: {} configured nothing; the planner noted it and the run went on. The shape now refuses it naming the three intents it could have meant, the published schema mirrors the refusal with minProperties, and the plan-time branch goes. The upgrade guide records it under the Pages refusal section. --- docs/reference/coverage.md | 2 +- docs/sections/pages.docs.yml | 5 +++-- docs/upgrading/v2-to-v3.md | 2 +- docs/upgrading/v2-to-v3/refusals-repository.md | 2 ++ src/sections/pages/index.ts | 6 ------ src/sections/pages/schema.ts | 14 +++++++++++++- test/published-schema.test.ts | 3 +++ test/sections/pages/pages.test.ts | 11 ----------- test/sections/pages/schema.test.ts | 7 +++++++ 9 files changed, 30 insertions(+), 22 deletions(-) diff --git a/docs/reference/coverage.md b/docs/reference/coverage.md index a7376e5e3..c0e78f754 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 42dcd3851..83ec474ae 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 04a894a97..d6c06bfd5 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 69d012e53..1761452f8 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 002b31312..eecb89682 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 5d00862db..39d0c639f 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,13 @@ 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. + if (Object.keys(site).length === 0) { + 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 +71,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 3fdaa55ac..84a53ad99 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 402f0ee0c..ee816f98b 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 b17b22803..e3bc33786 100644 --- a/test/sections/pages/schema.test.ts +++ b/test/sections/pages/schema.test.ts @@ -56,6 +56,13 @@ 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. + expect(issues({})).toEqual([ + "pages: an empty mapping configures nothing; declare at least one field, declare pages: null to disable the site, or remove the section", + ]); + }); + test("the fields the update takes parse, and null turns the site off", () => { expect( issues({ From 7b932a18ae3249918dcb847a13022e19b544f91f Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:10:45 -0400 Subject: [PATCH 2/2] fix(pages): count an undefined field as no field in the empty-mapping refusal --- src/sections/pages/schema.ts | 5 +++-- test/sections/pages/schema.test.ts | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/sections/pages/schema.ts b/src/sections/pages/schema.ts index 39d0c639f..ede5b3d8d 100644 --- a/src/sections/pages/schema.ts +++ b/src/sections/pages/schema.ts @@ -48,8 +48,9 @@ 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. - if (Object.keys(site).length === 0) { + // 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, diff --git a/test/sections/pages/schema.test.ts b/test/sections/pages/schema.test.ts index e3bc33786..7b7ddfef7 100644 --- a/test/sections/pages/schema.test.ts +++ b/test/sections/pages/schema.test.ts @@ -58,9 +58,11 @@ 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. - expect(issues({})).toEqual([ - "pages: an empty mapping configures nothing; declare at least one field, declare pages: null to disable the site, or remove the section", - ]); + 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", () => {