Skip to content

Commit 2a92e5b

Browse files
author
settings-as-code-release
committed
fix(snapshot): a null body stays a shape violation; service hooks are left out
A whole-section read answering a null 200 body kept reading as "nothing exists" and the section vanished from the document; the projection now leaves a rejected root value in place so the engine's validation names the body as outside the shape, and pages proves its body is a mapping first, since its nullable slice would otherwise read a null body as the declaration that disables the site. A legacy service hook (name other than "web") projected a name the schema refuses and failed the whole webhooks snapshot; it is now noted and left out, after the duplicate check that still counts it, because the planner matches a declared url against every live hook.
1 parent d5002c0 commit 2a92e5b

6 files changed

Lines changed: 103 additions & 14 deletions

File tree

‎src/sections/pages/index.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
* declares Pages OFF (mirroring branches' `protection: null`).
44
*/
55

6+
import { z } from "zod";
67
import { subsetDiff } from "../../engine/diff.js";
78
import type { EndpointDecl } from "../contract/endpoints.js";
9+
import { parseLive } from "../contract/live.js";
810
import { loosen, type SectionModule } from "../contract/module.js";
911
import type { SectionPermission } from "../contract/permissions.js";
1012
import { hasDrift, type PlannedOp, plainData, type SectionPlan } from "../contract/plan.js";
@@ -154,6 +156,9 @@ export const pagesSection = {
154156
if ("missing" in probe) {
155157
return { value: undefined, notes: [] };
156158
}
157-
return { value: projectOntoSchema(PagesConfig, probe.data), notes: [] };
159+
// A site body must be a mapping: PagesConfig accepts null (the declared "Pages off"), so a
160+
// null 200 would otherwise read back as a declaration that DISABLES the site.
161+
const site = parseLive(this, ENDPOINTS.get, z.looseObject({}), probe.data);
162+
return { value: projectOntoSchema(PagesConfig, site), notes: [] };
158163
},
159164
} satisfies SectionModule<"pages", typeof ENDPOINTS>;

‎src/sections/shared/snapshot-helpers.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {
4343
}
4444

4545
/**
46-
* A live value projected onto a schema slice: an object keeps exactly the keys the slice's shape
47-
* names (a loose object keeps its passthrough keys too), each child projected in turn; a list
48-
* projects its items; a union projects through its first option accepting the value.
49-
* A `null` the slice cannot hold is GitHub's "no value" (a not-configured setup's null
50-
* runner_type), so the key is omitted rather than emitted as an invalid declaration; every other
51-
* value the slice rejects is left in place for the document validation to name.
52-
* A passthrough slice (a catchall other than never) keeps every live key by design, so server
53-
* fields cannot fall away there: a section on such a slice names the keys it reads back itself.
54-
* The one cast is the boundary: the engine validates the assembled document before returning it.
46+
* A live value projected onto a schema slice, so server-assigned fields fall away without a hand
47+
* list per section. A nested `null` the slice cannot hold is GitHub's "no value" (a not-configured
48+
* setup's runner_type) and its key is omitted; a value the slice rejects at the root stays, so the
49+
* engine's validation names a body outside the shape instead of the section vanishing. A
50+
* passthrough slice (a catchall other than never) keeps every live key by design, so a section on
51+
* one names the keys it reads back itself. The casts are the boundary the engine validates behind.
5552
*/
5653
export function projectOntoSchema<T>(schema: z.ZodType<T>, live: unknown): T {
54+
if (live === null && !schema.safeParse(null).success) {
55+
return live as T;
56+
}
5757
return project(schema, live) as T;
5858
}
5959

‎src/sections/webhooks/index.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,13 @@ export const webhooksSection = {
317317
return plan;
318318
},
319319
// A live hook reports a set secret as "********": the entry carries a `$WEBHOOK_SECRET_<id>`
320-
// reference in its place, and a note asks for the value. A hook without a config.url has no
321-
// identity this section can declare, so it is noted and left out.
320+
// reference in its place, and a note asks for the value. A legacy service hook (name other
321+
// than "web") and a hook without a config.url are outside what this section manages, so each is
322+
// noted and left out.
322323
async snapshot(ctx) {
323324
const live = parseLive(this, ENDPOINTS.list, z.array(LiveHook), await ctx.read.list.listAll());
324325
const notes: string[] = [];
325-
const addressable = live.filter((hook) => {
326+
const withUrl = live.filter((hook) => {
326327
if (typeof hook.config?.url === "string" && hook.config.url !== "") {
327328
return true;
328329
}
@@ -331,13 +332,24 @@ export const webhooksSection = {
331332
);
332333
return false;
333334
});
335+
// Over every url-bearing hook, service hooks included: the planner matches a declared url
336+
// against ALL live hooks, so a web hook sharing a url with a service hook cannot converge.
334337
rejectLiveDuplicates(
335338
this,
336339
"webhook",
337-
addressable,
340+
withUrl,
338341
(hook) => String(hook.config?.url),
339342
(hook) => `${String(hook.config?.url)} (id ${hook.id})`,
340343
);
344+
const addressable = withUrl.filter((hook) => {
345+
if (hook.name === undefined || hook.name === "web") {
346+
return true;
347+
}
348+
notes.push(
349+
`webhooks[${describeHook(hook)}]: a "${hook.name}" service hook is not a web hook this section manages, so it is left out of the snapshot`,
350+
);
351+
return false;
352+
});
341353
if (addressable.length === 0) {
342354
return { value: undefined, notes };
343355
}

‎test/engine/snapshot.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,66 @@ describe("snapshotRepository", () => {
275275
});
276276
});
277277

278+
describe("snapshotRepository shape guard", () => {
279+
test("a null 200 body on a whole-section read fails the section as a body outside the shape, never an omitted section", async () => {
280+
const fake = registryFake(LIVE);
281+
const nulling: GithubClient = {
282+
tryRequest: (method, path, payload, options) =>
283+
method === "GET" && path === "/repos/o/r/code-scanning/default-setup"
284+
? Promise.resolve({ data: null })
285+
: fake.tryRequest(method, path, payload, options),
286+
tryGraphql: (op, variables, slug) => fake.tryGraphql(op, variables, slug),
287+
};
288+
const result = await snapshotRepository(
289+
nulling,
290+
{ ...opts(), onlySections: new Set(["code_scanning_default_setup", "labels"]) },
291+
captureIo().io,
292+
);
293+
expect(result.result).toBe("failed");
294+
expect(result.settings).toBeUndefined();
295+
expect(result.outcomes.map((o) => [o.key, o.status])).toEqual([
296+
["labels", "snapshot"],
297+
["code_scanning_default_setup", "failed"],
298+
]);
299+
expect(result.outcomes[1]?.detail).toEqual([
300+
expect.stringMatching(
301+
/^BUG: code_scanning_default_setup produced a snapshot its own schema rejects - .*code_scanning_default_setup: Invalid input: expected object, received null/,
302+
),
303+
]);
304+
});
305+
});
306+
307+
describe("pages null body", () => {
308+
test("a null 200 on the Pages GET fails the section as a body outside the shape, never `pages: null`", async () => {
309+
const fake = registryFake(LIVE);
310+
const nulling: GithubClient = {
311+
tryRequest: (method, path, payload, options) =>
312+
method === "GET" && path === "/repos/o/r/pages"
313+
? Promise.resolve({ data: null })
314+
: fake.tryRequest(method, path, payload, options),
315+
tryGraphql: (op, variables, slug) => fake.tryGraphql(op, variables, slug),
316+
};
317+
const result = await snapshotRepository(
318+
nulling,
319+
{ ...opts(), onlySections: new Set(["pages"]) },
320+
captureIo().io,
321+
);
322+
expect(result.result).toBe("partial");
323+
expect(Object.keys(result.settings ?? {})).toEqual([]);
324+
expect(result.outcomes).toEqual([
325+
{
326+
key: "pages",
327+
status: "failed",
328+
detail: [
329+
expect.stringMatching(
330+
/^pages: GET \/repos\/\{owner\}\/\{repo\}\/pages returned a body outside the documented shape - \(body\): Invalid input: expected object, received null/,
331+
),
332+
],
333+
},
334+
]);
335+
});
336+
});
337+
278338
describe("renderSnapshotYaml", () => {
279339
test("a message spanning several lines is commented line by line, so the file still parses", async () => {
280340
const stubbed = spyOn(labelsSection, "snapshot").mockRejectedValue(

‎test/sections/snapshot-roundtrip.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ describe("snapshot round trip", () => {
105105
{ id: 2, config: { url: "https://ci.example.com/hook" }, events: ["release"] },
106106
],
107107
});
108+
// A service hook on the same url counts too: the planner matches against every live hook.
109+
const mixed = registryFake({
110+
hooks: [
111+
{ id: 1, config: { url: "https://ci.example.com/hook" } },
112+
{ id: 2, name: "slack", config: { url: "https://ci.example.com/hook" } },
113+
],
114+
});
115+
await expect(
116+
webhooksSection.snapshot(planContext(webhooksSection, mixed, REPO)),
117+
).rejects.toThrow(/webhooks: GitHub holds webhooks that resolve to one identity/);
108118
await expect(
109119
webhooksSection.snapshot(planContext(webhooksSection, hooks, REPO)),
110120
).rejects.toThrow(

‎test/sections/snapshot-rows/webhooks.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export const row: Row = {
1616
},
1717
},
1818
{ id: 602, active: false, config: { url: "https://deploy.example.com/hook" } },
19+
{ id: 603, name: "slack", config: { url: "https://hooks.slack.example/T0/B0" } },
1920
],
2021
},
2122
expected: {
@@ -42,6 +43,7 @@ export const row: Row = {
4243
],
4344
},
4445
notes: [
46+
'webhooks["https://hooks.slack.example/T0/B0"]: a "slack" service hook is not a web hook this section manages, so it is left out of the snapshot',
4547
'webhooks["https://ci.example.com/hook"].config.secret: the webhook secret is not readable; export a value as WEBHOOK_SECRET_601 into the environment before apply',
4648
],
4749
},

0 commit comments

Comments
 (0)