Skip to content

Commit 8fe64ef

Browse files
committed
fix(snapshot): parse the creation-cap body and refuse duplicate milestone titles
The creation-cap 200 body was projected and tested with an equality on `enabled`, so a malformed body (a quoted flag, a null) read as "no cap" and the section vanished; the body is now parsed at the boundary, so an off-shape answer fails the section with the documented-shape error. Two live milestones under one title mapped to two entries the planner would refuse on the next run; the snapshot now refuses them at read time, naming each by title and number, like the list factory and webhooks do.
1 parent c01d950 commit 8fe64ef

3 files changed

Lines changed: 68 additions & 7 deletions

File tree

‎src/sections/interaction_limits/index.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ function noLiveLimit(live: unknown): boolean {
115115
);
116116
}
117117

118+
const LiveCreationCap = z.looseObject({
119+
enabled: z.boolean(),
120+
max_open_pull_requests: z.number().optional(),
121+
});
122+
118123
const LiveInteractionLimit = z.looseObject({
119124
limit: z.string(),
120125
origin: z.string().optional(),
@@ -384,12 +389,14 @@ export const interactionLimitsSection = {
384389
`interaction_limits: ${CAP_UNAVAILABLE} (405), so pull_request_creation_cap and pull_request_creation_bypass are omitted`,
385390
);
386391
} else {
387-
const liveCap = projectOntoSchema(
388-
InteractionLimitsConfig.unwrap().shape.pull_request_creation_cap,
389-
cap.data,
390-
);
391-
if (liveCap?.enabled === true) {
392-
value.pull_request_creation_cap = liveCap;
392+
// Parsed at the boundary: a body off the shape (a null, a quoted flag) fails the section
393+
// instead of reading as "no cap".
394+
const liveCap = parseLive(this, ENDPOINTS.capGet, LiveCreationCap, cap.data);
395+
if (liveCap.enabled) {
396+
value.pull_request_creation_cap = projectOntoSchema(
397+
InteractionLimitsConfig.unwrap().shape.pull_request_creation_cap,
398+
liveCap,
399+
);
393400
}
394401
const bypass = await liveBypassLogins(ctx, this);
395402
if (bypass.length > 0) {

‎src/sections/milestones/index.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ import type { SectionPermission } from "../contract/permissions.js";
1919
import { hasDrift, type PlannedOp, plainData, type SectionPlan } from "../contract/plan.js";
2020
import { rejectDuplicates } from "../contract/requests.js";
2121
import { knobbed } from "../shared/schema-helpers.js";
22-
import { knobbedSnapshot, projectOntoSchema } from "../shared/snapshot-helpers.js";
22+
import {
23+
knobbedSnapshot,
24+
projectOntoSchema,
25+
rejectLiveDuplicates,
26+
} from "../shared/snapshot-helpers.js";
2327
import { MilestoneConfig } from "./schema.js";
2428

2529
const LiveMilestone = z.looseObject({ number: z.number(), title: z.string() });
@@ -150,6 +154,13 @@ export const milestonesSection = {
150154
if (live.length === 0) {
151155
return { value: undefined, notes: [] };
152156
}
157+
rejectLiveDuplicates(
158+
this,
159+
"milestone",
160+
live,
161+
(milestone) => milestone.title,
162+
(milestone) => `${milestone.title} (number ${milestone.number})`,
163+
);
153164
const entries = live.map((milestone) => projectOntoSchema(MilestoneConfig, milestone));
154165
return { value: knobbedSnapshot(this, entries), notes: [] };
155166
},

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@
99
import { describe, expect, test } from "bun:test";
1010
import { readdirSync } from "node:fs";
1111
import { join } from "node:path";
12+
import type { GithubClient } from "../../src/github/api.js";
1213
import type { SectionKey } from "../../src/schema.js";
1314
import { actionsSecretsSection } from "../../src/sections/actions_secrets/index.js";
1415
import { planContext } from "../../src/sections/contract/plan.js";
1516
import { deployKeysSection } from "../../src/sections/deploy_keys/index.js";
17+
import { interactionLimitsSection } from "../../src/sections/interaction_limits/index.js";
1618
import { labelsSection } from "../../src/sections/labels/index.js";
19+
import { milestonesSection } from "../../src/sections/milestones/index.js";
1720
import { SECTIONS } from "../../src/sections/registry.js";
1821
import { webhooksSection } from "../../src/sections/webhooks/index.js";
1922
import { registryFake } from "./fragment-fake.js";
@@ -139,6 +142,46 @@ describe("snapshot round trip", () => {
139142
});
140143
});
141144

145+
test("two live milestones under one title fail the snapshot, naming both", async () => {
146+
const api = registryFake({
147+
milestones: [
148+
{ id: 1, number: 1, title: "v1", state: "open" },
149+
{ id: 2, number: 2, title: "v1", state: "closed" },
150+
],
151+
});
152+
await expect(
153+
milestonesSection.snapshot(planContext(milestonesSection, api, REPO)),
154+
).rejects.toThrow(
155+
'milestones: GitHub holds milestones that resolve to one identity: "v1 (number 1)" and ' +
156+
'"v1 (number 2)". This section manages one milestone per identity, so the snapshot ' +
157+
"cannot declare them; delete all but one of each on GitHub, then snapshot again",
158+
);
159+
});
160+
161+
test.each([
162+
[{ enabled: "yes" }, "enabled: Invalid input: expected boolean, received string"],
163+
[null, "(body): Invalid input: expected object, received null"],
164+
])(
165+
"a creation-cap body off the shape (%j) fails the snapshot instead of reading as no cap",
166+
async (body, issue) => {
167+
const fake = registryFake({});
168+
const api: GithubClient = {
169+
tryRequest: (method, path, payload, options) =>
170+
method === "GET" && path === "/repos/o/r/interaction-limits/pulls/creation-cap"
171+
? Promise.resolve({ data: body })
172+
: fake.tryRequest(method, path, payload, options),
173+
tryGraphql: (op, variables, slug) => fake.tryGraphql(op, variables, slug),
174+
};
175+
await expect(
176+
interactionLimitsSection.snapshot(planContext(interactionLimitsSection, api, REPO)),
177+
).rejects.toThrow(
178+
"interaction_limits: GET /repos/{owner}/{repo}/interaction-limits/pulls/creation-cap " +
179+
`returned a body outside the documented shape - ${issue}. Check the "api-version" ` +
180+
"input against the GitHub REST docs for this endpoint",
181+
);
182+
},
183+
);
184+
142185
test("a hook without a config.url is noted and left out; alone, it leaves nothing to declare", async () => {
143186
const api = registryFake({ hooks: [{ id: 7, config: {} }] });
144187
const snapshot = await webhooksSection.snapshot(planContext(webhooksSection, api, REPO));

0 commit comments

Comments
 (0)