diff --git a/src/sections/actions_secrets/actions_secrets.test.ts b/src/sections/actions_secrets/actions_secrets.test.ts index 853bd23b..67d904a5 100644 --- a/src/sections/actions_secrets/actions_secrets.test.ts +++ b/src/sections/actions_secrets/actions_secrets.test.ts @@ -453,26 +453,6 @@ describe("actions_secrets sealing key", () => { /no usable \{key_id, key\} pair \(key_id is empty\)/, ); }); - - test("a key that is not base64 or not X25519-sized fails with the endpoint named", async () => { - // The key parser rejects both malformations with the endpoint named, so a - // seal never receives invalid key material. - const notB64 = new MockApi({ - [LIST]: listOf(), - [PUBLIC_KEY]: { data: { key_id: "k1", key: "!!not-base64!!" } }, - }); - await expect(plan(notB64, [{ name: "X", value: "$V" }])).rejects.toThrow( - /^actions_secrets: GET \/repos\/\{owner\}\/\{repo\}\/actions\/secrets\/public-key \(the actions_secrets sealing key\) returned a key that is not valid base64/, - ); - - const wrongLength = new MockApi({ - [LIST]: listOf(), - [PUBLIC_KEY]: { data: { key_id: "k1", key: "AAAA" } }, - }); - await expect(plan(wrongLength, [{ name: "X", value: "$V" }])).rejects.toThrow( - /decodes to 3 bytes where an X25519 public key has 32/, - ); - }); }); describe("actions_secrets contract", () => { diff --git a/src/sections/branches/branches.test.ts b/src/sections/branches/branches.test.ts index 7c31ba84..e9b87c26 100644 --- a/src/sections/branches/branches.test.ts +++ b/src/sections/branches/branches.test.ts @@ -143,6 +143,9 @@ const NULL_FILLED = { required_pull_request_reviews: null, restrictions: null, }; +/** The whole failure for a user actor whose GraphQL lookup answers `user: null`. */ +const GHOST_ACTOR_ERROR = + 'branches: force_push_bypassers actor "ghost": the GraphQL lookup succeeded but returned no node id, so the allowance cannot be applied; re-run the workflow, and report this if it persists'; describe("branches", () => { const declared: Desired = [{ name: "main", protection: { enforce_admins: true } }]; @@ -866,9 +869,7 @@ describe("branches GraphQL-routed keys", () => { expect(api.calls.filter((c) => c.path.startsWith("BranchProtectionActor"))).toHaveLength(0); const execution = await executePlan(result, branchesSection, api, REPO, NO_SECRETS); expect(execution.status).toBe("failed"); - expect(String((execution as { error: Error }).error.message)).toMatch( - /GraphQL lookup succeeded but returned no node id/, - ); + expect(String((execution as { error: Error }).error.message)).toBe(GHOST_ACTOR_ERROR); expect(execution.landed).toBe(0); expect(api.mutations()).toHaveLength(0); }); @@ -901,11 +902,8 @@ describe("branches GraphQL-routed keys", () => { changes: [], notes: [], landed: 0, - error: expect.any(Error), + error: new Error(GHOST_ACTOR_ERROR), }); - expect(String((execution as { error: Error }).error.message)).toMatch( - /force_push_bypassers actor "ghost".*returned no node id/, - ); expect(api.mutations()).toHaveLength(0); }); diff --git a/src/sections/check_suite_preferences/check_suite_preferences.test.ts b/src/sections/check_suite_preferences/check_suite_preferences.test.ts index cba77ec4..7574c059 100644 --- a/src/sections/check_suite_preferences/check_suite_preferences.test.ts +++ b/src/sections/check_suite_preferences/check_suite_preferences.test.ts @@ -84,7 +84,8 @@ describe("check_suite_preferences", () => { expect(changes).toEqual(["applied check suite preferences (2 auto_trigger_checks entries)"]); expect(notes).toEqual([]); expect(second.ops.map((op) => op.role)).toEqual(["update"]); - expect(second.notes).toEqual(first.notes); + expect(first.notes).toEqual([note]); + expect(second.notes).toEqual([note]); // Both executions (the proof also runs the converged plan) sent the // declaration verbatim; nothing was ever read. expect(api.calls).toEqual([ diff --git a/src/sections/environments/pins.test.ts b/src/sections/environments/pins.test.ts index 573052e5..37591e59 100644 --- a/src/sections/environments/pins.test.ts +++ b/src/sections/environments/pins.test.ts @@ -350,9 +350,11 @@ describe("environments pinned check mode", () => { 'environments.pinned: apply will also move "b" to position 2 in that reordering', ], ]); - expect(planDrift(planned).some((line) => /environments\[[abc]\]\.pinned/.test(line))).toBe( - false, - ); + // No per-environment pin drift joins the two order lines. + expect(planDrift(planned)).toEqual([ + "environments.pinned: the declared pin order is [a, b, c] but the live pinned order is [c, b, a]; apply will reorder the pins so the declared ones lead in declaration order", + 'environments.pinned: apply will also move "b" to position 2 in that reordering', + ]); }); test("clean when the declared pins lead in declaration order; trailing undeclared pins earn nothing", async () => { diff --git a/src/sections/interaction_limits/interaction_limits.test.ts b/src/sections/interaction_limits/interaction_limits.test.ts index de711b8c..e563b31b 100644 --- a/src/sections/interaction_limits/interaction_limits.test.ts +++ b/src/sections/interaction_limits/interaction_limits.test.ts @@ -6,7 +6,6 @@ import { MockApi } from "../../../test/mock-api.js"; import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js"; import { REPO } from "../../../test/sections/section-run.js"; import { PermissionDenied } from "../contract/errors.js"; -import { grantFor } from "../contract/permissions.js"; import { interactionLimitsSection } from "./index.js"; import type { InteractionLimitsConfig } from "./schema.js"; @@ -211,7 +210,9 @@ describe("interaction_limits", () => { const denied = thrown as PermissionDenied; expect(denied.section).toBe("interaction_limits"); expect(denied.status).toBe(404); - expect(denied.detail).toContain(grantFor({ repo: ["administration"] })); + expect(denied.detail).toContain( + 'grant "Administration" (read and write) under the PAT\'s Repository permissions', + ); }); test("executing the plan converges: the base limit re-arms on every apply, nothing else recurs", async () => { @@ -276,8 +277,8 @@ describe("interaction_limits", () => { const silent = { role: "remove", drift: [], change: "" } as const; // @ts-expect-error the DELETE is not alwaysRewrite, so it must carry drift const _silent: Op = silent; - const rearm: Op = { role: "put", drift: [], change: "re-armed" }; - expect(rearm.drift).toEqual([]); + // The PUT is alwaysRewrite, so a driftless re-arm is a valid plan. + const _rearm: Op = { role: "put", drift: [], change: "re-armed" }; const undeclared = { role: "remove", drift: ["x"], diff --git a/src/sections/labels/labels.test.ts b/src/sections/labels/labels.test.ts index 522e5c4c..1c62ca89 100644 --- a/src/sections/labels/labels.test.ts +++ b/src/sections/labels/labels.test.ts @@ -163,7 +163,11 @@ describe("labels", () => { { name: "bug", new_name: "triage" }, { name: "enhancement", new_name: "Triage" }, ]), - ).rejects.toThrow(/name the same labels entry: "triage" and "Triage"/); + ).rejects.toThrow( + new Error( + 'labels: the settings file declares entries that name the same labels entry: "triage" and "Triage". Keep exactly one entry per resource', + ), + ); expect(api.calls).toHaveLength(0); }); diff --git a/src/sections/labels/mock.test.ts b/src/sections/labels/mock.test.ts index a7fe70ff..b53feedf 100644 --- a/src/sections/labels/mock.test.ts +++ b/src/sections/labels/mock.test.ts @@ -35,13 +35,18 @@ describe("labels.create identity minting", () => { ); create(state, { name: "bug", color: "d73a4a" }); create(state, { name: "docs", color: "0075ca" }); - for (const label of state.labels) { + // The pool starts at 90_000_000 and each label, seeded or created, takes + // the next id; its node_id encodes that same id. + const identity = (label: unknown) => { const body = label as Record; - expect(body.node_id).toBe(`MDU6TGFiZWw${body.id}`); - } - // No two labels (seeded or created) share a node_id. - const nodeIds = state.labels.map((label) => (label as Record).node_id); - expect(new Set(nodeIds).size).toBe(nodeIds.length); + return [body.name, body.id, body.node_id]; + }; + expect(state.labels.map(identity)).toEqual([ + ["area-1", 90_000_000, "MDU6TGFiZWw90000000"], + ["area-2", 90_000_001, "MDU6TGFiZWw90000001"], + ["bug", 90_000_002, "MDU6TGFiZWw90000002"], + ["docs", 90_000_003, "MDU6TGFiZWw90000003"], + ]); }); test("the created label's url names the state slug", () => { diff --git a/src/sections/repository/repository.test.ts b/src/sections/repository/repository.test.ts index 203b260e..dc273519 100644 --- a/src/sections/repository/repository.test.ts +++ b/src/sections/repository/repository.test.ts @@ -7,7 +7,6 @@ import { provePlanIdempotent } from "../../../test/sections/plan-idempotence.js" import { REPO } from "../../../test/sections/section-run.js"; import { validateSectionShapes } from "../../engine/validate.js"; import { PermissionDenied } from "../contract/errors.js"; -import { grantFor } from "../contract/permissions.js"; import { FEATURE_TOGGLES, normalizeTopics, repositorySection } from "./index.js"; /** The verdict's error prose, or null when the document validated. */ @@ -32,7 +31,9 @@ async function apply(api: GithubClient, desired: Desired) { /** The rejection must be a PermissionDenied CARRYING the section's grant advice. */ function expectAdministrationDenied(thrown: unknown): void { expect(thrown).toBeInstanceOf(PermissionDenied); - expect((thrown as PermissionDenied).detail).toContain(grantFor({ repo: ["administration"] })); + expect((thrown as PermissionDenied).detail).toContain( + 'grant "Administration" (read and write) under the PAT\'s Repository permissions', + ); } async function rejection(promise: Promise): Promise { diff --git a/src/sections/rulesets/rulesets.test.ts b/src/sections/rulesets/rulesets.test.ts index 02bd932b..00f127ac 100644 --- a/src/sections/rulesets/rulesets.test.ts +++ b/src/sections/rulesets/rulesets.test.ts @@ -445,8 +445,7 @@ describe("rulesets", () => { // is built first and assigned on one line, so the directive anchors to // the assignment whichever property the compiler blames. type Op = PlannedOp; - const create: Op = { role: "create", payload: { name: "x" }, drift: ["missing"], change: "" }; - expect(create.role).toBe("create"); + const _create: Op = { role: "create", payload: { name: "x" }, drift: ["missing"], change: "" }; const read = { role: "get", params: { ruleset_id: "1" }, drift: ["x"], change: "" } as const; // @ts-expect-error the get role is a read, not a plannable write const _read: Op = read; diff --git a/src/sections/workflows/workflows.test.ts b/src/sections/workflows/workflows.test.ts index baae7d9b..70f867c7 100644 --- a/src/sections/workflows/workflows.test.ts +++ b/src/sections/workflows/workflows.test.ts @@ -163,9 +163,11 @@ describe("workflows", () => { "PUT /repos/o/r/actions/workflows/1/disable", "PUT /repos/o/r/actions/workflows/2/enable", ]); - // The unfixable finding survives both plans unchanged. - expect(first.drift).toHaveLength(1); - expect(second).toEqual({ ops: [], notes: [], drift: first.drift }); + // The unfixable finding (a "deleted" live workflow is absent) survives both plans unchanged. + const gone = + "workflows[gone.yml]: declared in the settings file but no workflow with that path exists on the repo, so apply skips it - create the workflow file, or remove it from the workflows section"; + expect(first.drift).toEqual([gone]); + expect(second).toEqual({ ops: [], notes: [], drift: [gone] }); }); test("the read port exposes exactly the list role, narrowed to its denied posture", () => { @@ -188,13 +190,12 @@ describe("workflows", () => { // is built first and assigned on one line, so the directive anchors to // the assignment whichever property the compiler blames. type Op = PlannedOp; - const enable: Op = { + const _enable: Op = { role: "enable", params: { workflow_id: "1" }, drift: ["enabling"], change: "", }; - expect(enable.role).toBe("enable"); const read = { role: "list", drift: ["x"], change: "" } as const; // @ts-expect-error the list role is a read, not a plannable write const _read: Op = read; diff --git a/test/discovery/central.test.ts b/test/discovery/central.test.ts index 958b4a2a..f18d49ad 100644 --- a/test/discovery/central.test.ts +++ b/test/discovery/central.test.ts @@ -3,27 +3,55 @@ import { resolveCentralTargets } from "../../src/discovery/central.js"; describe("resolveCentralTargets", () => { test("reads owner-shorthand and owner/name files, warns on strays", () => { - const resolved = resolveCentralTargets("test/fixtures/repos", "viv"); - if ("error" in resolved) { - throw new Error(resolved.error); - } - expect(resolved.targets.map((t) => t.slug).sort()).toEqual(["octo/web", "viv/api"]); - expect(resolved.warnings.some((w) => w.includes("README.md"))).toBe(true); - expect(resolved.warnings.some((w) => w.includes("deep"))).toBe(true); + // The walk sorts directory entries, so the target and warning order is + // fixed: top-level files first, then each owner directory. + expect(resolveCentralTargets("test/fixtures/repos", "viv")).toEqual({ + targets: [ + { + slug: "viv/api", + source: "central", + origin: "test/fixtures/repos/api.yml", + filePath: "test/fixtures/repos/api.yml", + }, + { + slug: "octo/web", + source: "central", + origin: "test/fixtures/repos/octo/web.yml", + filePath: "test/fixtures/repos/octo/web.yml", + }, + ], + warnings: [ + "ignoring test/fixtures/repos/README.md: not a .yml/.yaml file, so it defines no target repository", + "ignoring test/fixtures/repos/octo/deep: repos-dir supports only .yml and /.yml, nothing deeper. Move the files up or remove the directory", + ], + }); }); test("shorthand without a known admin owner is an error", () => { - const resolved = resolveCentralTargets("test/fixtures/repos", ""); - expect("error" in resolved && resolved.error).toContain("/.yml"); + expect(resolveCentralTargets("test/fixtures/repos", "")).toEqual({ + error: + 'repos-dir "test/fixtures/repos" has 1 invalid settings file(s):\n' + + "- cannot resolve test/fixtures/repos/api.yml: top-level repos-dir files use the current " + + "repository's owner, which is unknown outside GitHub Actions. Use the /.yml " + + "layout instead", + }); }); test("the same repo defined twice is an error", () => { - const resolved = resolveCentralTargets("test/fixtures/repos-dup", "viv"); - expect("error" in resolved && resolved.error).toContain("duplicate target viv/x"); + expect(resolveCentralTargets("test/fixtures/repos-dup", "viv")).toEqual({ + error: + 'repos-dir "test/fixtures/repos-dup" has 1 invalid settings file(s):\n' + + "- duplicate target viv/x: defined by both test/fixtures/repos-dup/viv/x.yml and " + + "test/fixtures/repos-dup/x.yml. Keep exactly one settings file per repository", + }); }); test("missing dir errors with a checkout hint", () => { - const resolved = resolveCentralTargets("test/fixtures/nope", "viv"); - expect("error" in resolved && resolved.error).toContain("actions/checkout"); + expect(resolveCentralTargets("test/fixtures/nope", "viv")).toEqual({ + error: + 'repos-dir "test/fixtures/nope" does not exist in the workspace, so there are no central ' + + "settings files to read. Add an actions/checkout step before this action, or fix the " + + "repos-dir path", + }); }); }); diff --git a/test/discovery/discover.test.ts b/test/discovery/discover.test.ts index 3d9115ee..474e6604 100644 --- a/test/discovery/discover.test.ts +++ b/test/discovery/discover.test.ts @@ -228,7 +228,14 @@ describe("discoverRepos", () => { }, }); const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect("error" in discovered && discovered.error).toContain("Discovery needs a user PAT"); + expect(discovered).toEqual({ + error: + 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + + "403 Resource not accessible. " + + "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + + "cannot enumerate a user's repositories. List the target repositories explicitly in the " + + '"repos" input', + }); }); test("a rate-limit 403 gets re-run advice, not PAT advice", async () => { @@ -241,8 +248,12 @@ describe("discoverRepos", () => { }, }); const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect("error" in discovered && discovered.error).toContain("re-run the workflow"); - expect("error" in discovered && discovered.error).not.toContain("Discovery needs a user PAT"); + expect(discovered).toEqual({ + error: + 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + + "403 API rate limit exceeded for user. " + + "This is not a permission problem; re-run the workflow, and retry later if it persists", + }); }); test("an expired-token 401 explains the PAT requirement", async () => { @@ -252,7 +263,14 @@ describe("discoverRepos", () => { }, }); const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect("error" in discovered && discovered.error).toContain("Discovery needs a user PAT"); + expect(discovered).toEqual({ + error: + 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + + "401 Bad credentials. " + + "Discovery needs a user PAT; the workflow GITHUB_TOKEN and GitHub App installation tokens " + + "cannot enumerate a user's repositories. List the target repositories explicitly in the " + + '"repos" input', + }); }); test("a server error gets re-run advice, not PAT advice", async () => { @@ -262,8 +280,12 @@ describe("discoverRepos", () => { }, }); const discovered = await discoverRepos(api, DEFAULT_DISCOVERY_FILTERS); - expect("error" in discovered && discovered.error).toContain("re-run the workflow"); - expect("error" in discovered && discovered.error).not.toContain("Discovery needs a user PAT"); + expect(discovered).toEqual({ + error: + 'cannot discover repositories for repos: "*": GET /user/repos?affiliation=owner failed: ' + + "500 boom. " + + "This is not a permission problem; re-run the workflow, and retry later if it persists", + }); }); }); @@ -286,9 +308,9 @@ describe("formatSkipNotice", () => { test("without redaction, only the first 20 slugs are listed", () => { const repos = Array.from({ length: 23 }, (_, i) => ref(`o/r${i}`)); - const notice = formatSkipNotice({ reason: "forks=exclude", repos }, false); - expect(notice).toContain("o/r19, and 3 more"); - expect(notice).not.toContain("o/r20"); + expect(formatSkipNotice({ reason: "forks=exclude", repos }, false)).toBe( + 'repos: "*" discovery skipped 23 repositories by forks=exclude: o/r0, o/r1, o/r2, o/r3, o/r4, o/r5, o/r6, o/r7, o/r8, o/r9, o/r10, o/r11, o/r12, o/r13, o/r14, o/r15, o/r16, o/r17, o/r18, o/r19, and 3 more', + ); }); test("redaction lists public slugs and counts the rest", () => { @@ -344,6 +366,5 @@ describe("formatSkipNotice", () => { (_, i) => `o/pub${i}`, ).join(", ")}, and 2 more, and 1 private or internal repository`, ); - expect(notice).not.toContain("o/secret"); }); }); diff --git a/test/discovery/repos-input.test.ts b/test/discovery/repos-input.test.ts index ace18527..2c6637bc 100644 --- a/test/discovery/repos-input.test.ts +++ b/test/discovery/repos-input.test.ts @@ -15,7 +15,9 @@ describe("parseReposInput", () => { test("* mixed with slugs is an error", () => { expect(parseReposInput("*, o/a")).toEqual({ - error: expect.stringContaining('Use "*" alone to discover every repository'), + error: + 'the "repos" input mixes "*" with explicit repositories. Use "*" alone to discover every ' + + "repository the token owns, or list the repositories without it", }); }); diff --git a/test/discovery/targets.test.ts b/test/discovery/targets.test.ts index 4d38ece2..c1f62c3c 100644 --- a/test/discovery/targets.test.ts +++ b/test/discovery/targets.test.ts @@ -22,8 +22,13 @@ describe("dedupeTargets", () => { (m) => notices.push(m), (slug) => slug, ); - expect(merged.map((t) => t.slug)).toEqual(["o/x", "o/z"]); - expect(notices[0]).toContain("using the central file repos/x.yml"); + expect(merged).toEqual([ + { slug: "o/x", source: "central", origin: "repos/x.yml", filePath: "repos/x.yml" }, + { slug: "o/z", source: "remote", origin: 'the "repos" input' }, + ]); + expect(notices).toEqual([ + 'O/X: using the central file repos/x.yml; the entry for the same repository from the "repos" input is ignored', + ]); }); test("the notice renders the slug through display; a non-redacted origin stays verbatim", () => { @@ -34,16 +39,15 @@ describe("dedupeTargets", () => { (m) => notices.push(m), () => "private repository #1", ); - expect(notices).toHaveLength(1); - expect(notices[0]).toStartWith("private repository #1: using the central file repos/x.yml"); - expect(notices[0]).toContain('the entry for the same repository from the "repos" input'); - // no doubled article from wrapping the origin noun phrase - expect(notices[0]).not.toContain("the the "); - expect(notices[0]?.toLowerCase()).not.toContain("o/x"); + // The whole notice: the placeholder replaces the slug everywhere, the + // origin noun phrase is wrapped without a doubled article. + expect(notices).toEqual([ + 'private repository #1: using the central file repos/x.yml; the entry for the same repository from the "repos" input is ignored', + ]); }); test("a redacted target's central origin is rendered generically, never the file path", () => { - // The central file path (repos/x.yml) can embed the real repo name, so for + // The central file path (repos/secret.yml) can embed the real repo name, so for // a redacted target it must not appear next to the placeholder. const notices: string[] = []; dedupeTargets( @@ -60,11 +64,8 @@ describe("dedupeTargets", () => { () => "private repository #1", () => true, ); - expect(notices).toHaveLength(1); - expect(notices[0]).toStartWith( - "private repository #1: using the central file a repos-dir file", - ); - expect(notices[0]).not.toContain("repos/secret.yml"); - expect(notices[0]).not.toContain("secret"); + expect(notices).toEqual([ + 'private repository #1: using the central file a repos-dir file; the entry for the same repository from the "repos" input is ignored', + ]); }); }); diff --git a/test/docs/checks-workflow.test.ts b/test/docs/checks-workflow.test.ts index 860d570e..b3c91c57 100644 --- a/test/docs/checks-workflow.test.ts +++ b/test/docs/checks-workflow.test.ts @@ -602,7 +602,6 @@ describe("the fetch-test-artifacts composite", () => { test("each cache key is the pinned literal and hashes every input its artifact depends on", () => { expect(OPENAPI.hashInputs()).toContain("src/report/issue-report.ts"); - expect(hashFilesPatterns(keyOf(OPENAPI))).toContain("src/sections/**"); for (const artifact of FETCHED_ARTIFACTS) { expectKeyPinned(keyOf(artifact), artifact); expectKeyHashesInputs(keyOf(artifact), artifact); diff --git a/test/e2e/mock/graphql-pipeline.test.ts b/test/e2e/mock/graphql-pipeline.test.ts index 168e0749..7738d979 100644 --- a/test/e2e/mock/graphql-pipeline.test.ts +++ b/test/e2e/mock/graphql-pipeline.test.ts @@ -409,15 +409,6 @@ describe("GraphQL multi-repo slug resolution", () => { expect(multi?.repos.get("acme/beta")?.repo.has_wiki).toBe(true); }); - test("a mutation without a decodable node id is a violation", () => { - const result = dispatch( - G_WRITE, - { repositoryId: "R_kgDOnotOurs", hasWiki: true }, - multiOptions(scenario()), - ); - expect(result.violation).toContain("carries no decodable mock node id"); - }); - test("a mutation whose ids span two repositories is a violation", () => { const result = dispatch( G_WRITE, diff --git a/test/e2e/mock/server.test.ts b/test/e2e/mock/server.test.ts index ad37b244..0b06e5e1 100644 --- a/test/e2e/mock/server.test.ts +++ b/test/e2e/mock/server.test.ts @@ -102,8 +102,8 @@ describe("pagination slicing", () => { const second = await jsonArray(await call(h, "GET", `${labelsPath}?per_page=100&page=2`)); expect(first).toHaveLength(100); expect(second).toHaveLength(0); - // The page=2 read is present in the log, proving the client can advance. - expect(h.requests.some((r) => r.query.includes("page=2"))).toBe(true); + // The log holds exactly the two page reads, query strings intact. + expect(h.requests.map((r) => r.query)).toEqual(["per_page=100&page=1", "per_page=100&page=2"]); }); }); diff --git a/test/e2e/mock/state.test.ts b/test/e2e/mock/state.test.ts index 81b91832..d1062770 100644 --- a/test/e2e/mock/state.test.ts +++ b/test/e2e/mock/state.test.ts @@ -41,7 +41,6 @@ import { ruleWireNode, teamRepoFromPut, } from "./state.js"; -import { storedKeyMaterial } from "./support.js"; describe("buildState overlay semantics", () => { test("undefined LiveState uses fixture defaults and empty lists", () => { @@ -67,12 +66,6 @@ describe("buildState overlay semantics", () => { expect(state.repo.default_branch).toBe("main"); }); - test("explicit labels list replaces the (empty) baseline", () => { - const state = buildState({ labels: [{ name: "bug", color: "d73a4a" }] }, "org"); - expect(state.labels).toHaveLength(1); - expect(state.labels[0]).toMatchObject({ name: "bug", color: "d73a4a" }); - }); - test("labels.generate sugar produces count labels with the prefix and color", () => { const state = buildState( { labels: { generate: { count: 3, prefix: "area", color: "abcdef" } } }, @@ -101,32 +94,39 @@ describe("buildState overlay semantics", () => { }, "org", ); - expect(state.labels[0]).toEqual({ - name: "bug", - color: "d73a4a", - description: null, - default: false, - id: expect.any(Number), - node_id: expect.any(String), - url: "https://api.github.com/repos/e2e-owner/e2e-repo/labels/bug", - }); - expect(state.autolinks[0]).toEqual({ - key_prefix: "JIRA-", - url_template: "https://j.example.com/", - is_alphanumeric: true, - id: expect.any(Number), - }); + // Each explicit list replaces the empty baseline outright (one seed, one item), and the ids + // are minted in seed order from the 90_000_000 pool. + expect(state.labels).toEqual([ + { + name: "bug", + color: "d73a4a", + description: null, + default: false, + id: 90_000_000, + node_id: "MDU6TGFiZWw90000000", + url: "https://api.github.com/repos/e2e-owner/e2e-repo/labels/bug", + }, + ]); + expect(state.autolinks).toEqual([ + { + key_prefix: "JIRA-", + url_template: "https://j.example.com/", + is_alphanumeric: true, + id: 90_000_001, + }, + ]); // The seed's comment is stripped the way a created key is stored, so a converging apply over a - // seeded key still proves the section compares algorithm + blob, not the raw string. - expect(state.deploy_keys[0]).toMatchObject({ + // seeded key still proves the section compares algorithm + blob, not the raw string. The id is + // the third mint from the 90_000_000 pool (the label and the autolink took the first two). + expect(state.deploy_keys[0]).toEqual({ title: "bot", - key: storedKeyMaterial("ssh-ed25519 AAAAC3seedseedseed deploy@bot"), + key: "ssh-ed25519 AAAAC3seedseedseed", read_only: false, verified: true, + id: 90_000_002, + url: "https://api.github.com/repos/e2e-owner/e2e-repo/keys/90000002", + created_at: "2026-07-01T00:00:00Z", }); - expect(String((state.deploy_keys[0] as Record).key)).not.toContain( - "deploy@bot", - ); }); test("a pinned seed id anywhere in the overlay is reserved before any family mints", () => { diff --git a/test/e2e/openapi/validate.test.ts b/test/e2e/openapi/validate.test.ts index 4c2e71d8..91e24b38 100644 --- a/test/e2e/openapi/validate.test.ts +++ b/test/e2e/openapi/validate.test.ts @@ -11,7 +11,7 @@ import { readFileSync } from "node:fs"; import { endpointMethod, endpointPath } from "../../../src/sections/contract/endpoints.js"; import { allEndpoints } from "../../../src/sections/registry.js"; import type { LoggedRequest } from "../mock/contract.js"; -import { excludeUndocumented, UNDOCUMENTED_PATHS, USED_PATHS } from "./paths.js"; +import { excludeUndocumented, USED_PATHS } from "./paths.js"; import { OpenApiValidator, pathMatches, @@ -273,12 +273,6 @@ describe("undocumented-route exemption", () => { /fix or delete that gap file/, ); }); - - test("USED_PATHS carries no undocumented path (empty set included)", () => { - for (const path of UNDOCUMENTED_PATHS) { - expect(USED_PATHS).not.toContain(path); - } - }); }); describe("pathMatches greedy trailing params", () => { @@ -313,7 +307,6 @@ describe("OpenApiValidator against the fetched spec", () => { const v = sharedValidator(); test("the fetched spec loads and shares one instance", () => { - expect(v).toBeInstanceOf(OpenApiValidator); expect(sharedValidator()).toBe(v); // process-wide singleton }); diff --git a/test/engine/execute.test.ts b/test/engine/execute.test.ts index 8c51a7ea..bff60df0 100644 --- a/test/engine/execute.test.ts +++ b/test/engine/execute.test.ts @@ -188,7 +188,9 @@ describe("executePlan", () => { const execution = await executePlan(plan, SECTION, api, REPO, TOOLS); expect(execution.status).toBe("failed"); expect(execution.changes).toEqual(["flipped the toggle"]); - expect(errorOf(execution)).toContain("422"); + expect(errorOf(execution)).toBe( + 'Error: labels: POST /repos/o/r/labels: 422 Validation Failed. The API rejected the request; fix the "labels" values in the settings file to satisfy the message above', + ); }); test("an operation's describe label leads the failure message, REST and GraphQL alike", async () => { @@ -203,8 +205,8 @@ describe("executePlan", () => { REPO, TOOLS, ); - expect(errorOf(rest)).toContain( - 'creating label "bug" failed - POST /repos/o/r/labels: 422 Validation Failed', + expect(errorOf(rest)).toBe( + 'Error: labels: creating label "bug" failed - POST /repos/o/r/labels: 422 Validation Failed. The API rejected the request; fix the "labels" values in the settings file to satisfy the message above', ); const graphql = await executePlan( planOf({ role: "write", variables: {}, describe: "flipping the toggle" }), @@ -213,7 +215,9 @@ describe("executePlan", () => { REPO, TOOLS, ); - expect(errorOf(graphql)).toContain("flipping the toggle failed - GRAPHQL ExecutorWrite: 502"); + expect(errorOf(graphql)).toBe( + "Error: labels: flipping the toggle failed - GRAPHQL ExecutorWrite: 502 Bad Gateway. GitHub returned a server error; re-run the workflow, and retry later if it persists", + ); }); test("a change thunk renders from the response once the request landed, one line or several", async () => { @@ -299,7 +303,7 @@ describe("executePlan", () => { landed: 1, error: new Error("the write did not take"), }); - expect(api.mutations()).toHaveLength(1); + expect(api.mutations().map((m) => `${m.method} ${m.path}`)).toEqual(["POST /repos/o/r/labels"]); // The render failed, so the hook after it never ran. expect(captured).toBe(false); }); @@ -345,7 +349,10 @@ describe("executePlan", () => { landed: 2, error: new Error("the response carried no node id"), }); - expect(api.mutations()).toHaveLength(2); + expect(api.mutations().map((m) => `${m.method} ${m.path}`)).toEqual([ + "POST /repos/o/r/labels", + "GRAPHQL ExecutorWrite", + ]); }); test.each<[hook: string, op: Partial]>([ @@ -455,12 +462,13 @@ describe("executePlan", () => { drift: [], }; const execution = await executePlan(plan, SECTION, api, REPO, TOOLS); - expect(execution).toMatchObject({ + expect(execution).toEqual({ status: "failed", changes: ['created label "bug"'], + notes: [], landed: 1, + error: new Error("the actor does not exist"), }); - expect(errorOf(execution)).toContain("the actor does not exist"); expect(seen).toEqual(['[{"name":"live"}]']); // The read ran before the first write; the second write never left. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ @@ -553,9 +561,15 @@ describe("executePlan", () => { REPO, TOOLS, ); - expect(narrowed.status).toBe("failed"); - expect(narrowed.notes).toEqual([]); - expect(errorOf(narrowed)).toContain("422 Validation Failed"); + expect(narrowed).toEqual({ + status: "failed", + changes: [], + notes: [], + landed: 0, + error: new Error( + 'labels: POST /repos/o/r/labels: 422 Validation Failed. The API rejected the request; fix the "labels" values in the settings file to satisfy the message above', + ), + }); // An undeclared status under the default: throwFor's classification, a // PermissionDenied here, not the operation's outcome. const denied = await executePlan( @@ -565,7 +579,17 @@ describe("executePlan", () => { REPO, TOOLS, ); - expect(denied.status).toBe("failed"); + expect(denied).toEqual({ + status: "failed", + changes: [], + notes: [], + landed: 0, + error: new PermissionDenied( + "labels", + `the token was denied POST /repos/o/r/labels: 403 Forbidden. To fix, grant "Administration" (read and write) under the PAT's Repository permissions`, + 403, + ), + }); expect((denied as { error: unknown }).error).toBeInstanceOf(PermissionDenied); // The control: a tolerated operation that succeeds records its line. const succeeded = await executePlan( @@ -595,8 +619,8 @@ describe("executePlan", () => { TOOLS, ); expect(execution.status).toBe("failed"); - expect(errorOf(execution)).toMatch( - /BUG: POST \/repos\/\{owner\}\/\{repo\}\/labels was asked to tolerate status\(es\) 404, which it does not declare/, + expect(errorOf(execution)).toBe( + "Error: BUG: POST /repos/{owner}/{repo}/labels was asked to tolerate status(es) 404, which it does not declare as a tolerable error status; a tolerance may only name declared 4xx statuses other than 401 and 429", ); expect(api.calls).toEqual([]); }); @@ -637,7 +661,9 @@ describe("executePlan", () => { TOOLS, ); expect(execution.status).toBe("failed"); - expect(errorOf(execution)).toMatch(/rate limit was hit/); + expect(errorOf(execution)).toBe( + "Error: labels: POST /repos/o/r/labels: 403 API rate limit exceeded. The API rate limit was hit; re-run the workflow after the limit resets, or use a token with a higher rate limit", + ); expect(consulted).toBe(false); }); @@ -664,7 +690,9 @@ describe("executePlan", () => { TOOLS, ); expect(execution.status).toBe("failed"); - expect(errorOf(execution)).toContain("502"); + expect(errorOf(execution)).toBe( + "Error: labels: GRAPHQL ExecutorWrite: 502 Bad Gateway. GitHub returned a server error; re-run the workflow, and retry later if it persists", + ); expect(consulted).toBe(false); }); @@ -870,21 +898,52 @@ describe("executePlan", () => { // The refusals: each names a role the erased plan type admits but the // executor must never issue, and each runs against a client that would // otherwise answer, so a missing guard fails the test loudly. - const refused: ReadonlyArray<{ what: string; role: string; message: RegExp }> = [ - { what: "a REST read role", role: "list", message: /is a read endpoint/ }, - { what: "a GraphQL read role", role: "read", message: /is a GraphQL read operation/ }, - { what: "an undeclared role", role: "typo", message: /names no declared endpoint/ }, + const refused: ReadonlyArray<{ what: string; role: string; message: string }> = [ + { + what: "a REST read role", + role: "list", + message: + 'Error: BUG: labels planned an operation under role "list", which is a read endpoint (GET /repos/{owner}/{repo}/labels); only write roles are plannable', + }, + { + what: "a GraphQL read role", + role: "read", + message: + 'Error: BUG: labels planned an operation under role "read", which is a GraphQL read operation; only write roles are plannable', + }, + { + what: "an undeclared role", + role: "typo", + message: + 'Error: BUG: labels planned an operation under role "typo", which names no declared endpoint or GraphQL operation', + }, // Inherited names resolve through a plain `dict[role]` lookup; the // executor must read own properties only. - { what: "an inherited role (constructor)", role: "constructor", message: /names no declared/ }, - { what: "an inherited role (toString)", role: "toString", message: /names no declared/ }, + { + what: "an inherited role (constructor)", + role: "constructor", + message: + 'Error: BUG: labels planned an operation under role "constructor", which names no declared endpoint or GraphQL operation', + }, + { + what: "an inherited role (toString)", + role: "toString", + message: + 'Error: BUG: labels planned an operation under role "toString", which names no declared endpoint or GraphQL operation', + }, // Non-string roles: a number would coerce onto a matching key, a symbol // would enter the property-key path; both are refused before any lookup. - { what: "a numeric role", role: 0 as unknown as string, message: /role is a number/ }, + { + what: "a numeric role", + role: 0 as unknown as string, + message: + "Error: BUG: labels planned an operation whose role is a number, not the name of a declared write", + }, { what: "a symbol role", role: Symbol("create") as unknown as string, - message: /role is a symbol/, + message: + "Error: BUG: labels planned an operation whose role is a symbol, not the name of a declared write", }, ]; for (const { what, role, message } of refused) { @@ -900,7 +959,7 @@ describe("executePlan", () => { expect(execution.status).toBe("failed"); expect(execution.changes).toEqual([]); expect(execution.landed).toBe(0); - expect(errorOf(execution)).toMatch(message); + expect(errorOf(execution)).toBe(message); expect(api.calls).toEqual([]); }); } diff --git a/test/engine/validate.test.ts b/test/engine/validate.test.ts index 061a430a..4905791a 100644 --- a/test/engine/validate.test.ts +++ b/test/engine/validate.test.ts @@ -7,27 +7,50 @@ function errorOf(doc: Record, sourceLabel = "f.yml"): string | return "error" in verdict ? verdict.error : null; } +/** + * Every shape verdict is one line: the source label, the problems joined by + * "; ", and the same passthrough clause. The tables below carry the problem + * list and the assertion pins the whole line. + */ +const PASSTHROUGH = + "Fix these values in the settings file (only the named keys are validated; extra fields pass " + + "through, except in closed sections and strict nested objects like actions.cache, which reject " + + "unrecognized keys)"; + describe("section shape validation", () => { - test("pages: null passes; a bad workflows state fails naming the path", () => { + test("pages: null passes", () => { expect(validateSectionShapes({ pages: null }, "f.yml")).toEqual({ settings: { pages: null } }); - const error = errorOf({ workflows: [{ path: "ci.yml", state: "paused" }] }); - expect(error).toContain("workflows[0].state"); }); - test("the fields handlers dereference are shape-checked, naming the key path", () => { + test.each<[what: string, doc: Record, problem: string]>([ + [ + "a bad workflows state fails naming the path", + { workflows: [{ path: "ci.yml", state: "paused" }] }, + 'workflows[0].state: Invalid option: expected one of "active"|"disabled"', + ], // A missing "-" makes include a string; the handler would call .map on it. - const include = errorOf({ - rulesets: [{ name: "protect-main", conditions: { ref_name: { include: "main" } } }], - }); - expect(include).toContain("rulesets[0].conditions.ref_name.include"); + [ + "a string where the handler maps a list", + { rulesets: [{ name: "protect-main", conditions: { ref_name: { include: "main" } } }] }, + "rulesets[0].conditions.ref_name.include: Invalid input: expected array, received string", + ], // YAML parses new_name: 2.0 as a number; the handler lowercases it. - const rename = errorOf({ labels: [{ name: "v2", new_name: 2 }] }); - expect(rename).toContain("labels[0].new_name"); + [ + "a number where the handler lowercases a string", + { labels: [{ name: "v2", new_name: 2 }] }, + "labels[0].new_name: Invalid input: expected string, received number", + ], // The handler reads source.path, which throws on source: null. - const source = errorOf({ pages: { source: null } }); - expect(source).toContain("pages.source"); - // The happy shapes still pass, and the parsed document carries the - // unknown keys through untouched. + [ + "a null where the handler dereferences a mapping", + { pages: { source: null } }, + "pages.source: Invalid input: expected object, received null", + ], + ])("the fields handlers dereference are shape-checked: %s", (_what, doc, problem) => { + expect(errorOf(doc)).toBe(`f.yml has malformed section entries: ${problem}. ${PASSTHROUGH}`); + }); + + test("the happy shapes pass, and the parsed document carries the unknown keys through untouched", () => { const happy = { rulesets: [{ name: "r", conditions: { ref_name: { include: ["main"] } }, extra: 1 }], labels: [{ name: "v2", new_name: "2.0" }], @@ -46,26 +69,30 @@ describe("section shape validation", () => { }); describe("YAML-tagged values are rejected anywhere in a section", () => { - test("a tagged section VALUE is rejected for mapping sections without a required key", () => { - // zod object schemas accept a Date or Set as an empty mapping, so - // without the plain-data gate these would validate and silently - // configure nothing. - for (const doc of [{ actions: new Date(0) }, { pages: new Date(0) }]) { - const error = errorOf(doc as Record, "settings.yml"); - expect(error).toContain("not plain YAML data"); - expect(error).toContain("!!timestamp"); - } - }); + // zod object schemas accept a Date or Set as an empty mapping, so without + // the plain-data gate these would validate and silently configure nothing. + test.each<[site: string, doc: Record]>([ + ["actions", { actions: new Date(0) }], + ["pages", { pages: new Date(0) }], + ])( + "a tagged section VALUE is rejected for the mapping section %s, which has no required key", + (site, doc) => { + expect(errorOf(doc, "settings.yml")).toBe( + `settings.yml has malformed section entries: ${site} is not plain YAML data (a Date, ` + + `e.g. from a YAML !!timestamp tag); replace it with a plain value. ${PASSTHROUGH}`, + ); + }, + ); test("a tagged NESTED value is rejected with its key path", () => { - const error = errorOf({ actions: { cache: new Date(0) } } as Record); - expect(error).toContain("actions.cache is not plain YAML data"); - const inList = errorOf({ labels: [{ name: "bug", color: new Set(["d73a4a"]) }] } as Record< - string, - unknown - >); - expect(inList).toContain("labels[0].color is not plain YAML data"); - expect(inList).toContain("!!set"); + expect(errorOf({ actions: { cache: new Date(0) } })).toBe( + "f.yml has malformed section entries: actions.cache is not plain YAML data (a Date, e.g. " + + `from a YAML !!timestamp tag); replace it with a plain value. ${PASSTHROUGH}`, + ); + expect(errorOf({ labels: [{ name: "bug", color: new Set(["d73a4a"]) }] })).toBe( + "f.yml has malformed section entries: labels[0].color is not plain YAML data (a set, e.g. " + + `from a YAML !!set tag); replace it with a plain value. ${PASSTHROUGH}`, + ); }); test("a cyclic document (YAML anchors) does not hang the validator", () => { @@ -77,19 +104,29 @@ describe("YAML-tagged values are rejected anywhere in a section", () => { }); describe("closed-surface sections reject unrecognized entry keys upfront", () => { + const ROLE_CONSEQUENCE = + 'a misspelled "permission" key would silently grant the default "push" role instead of the ' + + "intended one"; + test("a misspelled collaborator permission fails validation, before any write", () => { - const error = errorOf({ collaborators: [{ username: "alice", permision: "admin" }] }); - expect(error).toContain('collaborators[alice]: declares "permision"'); - expect(error).toContain("known keys: username, permission"); - expect(error).toContain('default "push" role'); + expect(errorOf({ collaborators: [{ username: "alice", permision: "admin" }] })).toBe( + 'f.yml has malformed section entries: collaborators[alice]: declares "permision", which ' + + `this section does not recognize (known keys: username, permission) - ${ROLE_CONSEQUENCE}. ` + + `Fix the key name, or remove it. ${PASSTHROUGH}`, + ); }); test("teams and workflows are closed too", () => { - const teams = errorOf({ teams: [{ name: "t", permissions: "admin" }] }); - expect(teams).toContain('teams[t]: declares "permissions"'); - const workflows = errorOf({ workflows: [{ path: "ci.yml", state: "active", enabled: true }] }); - expect(workflows).toContain('workflows[ci.yml]: declares "enabled"'); - expect(workflows).toContain("send no payload"); + expect(errorOf({ teams: [{ name: "t", permissions: "admin" }] })).toBe( + 'f.yml has malformed section entries: teams[t]: declares "permissions", which this ' + + `section does not recognize (known keys: name, permission) - ${ROLE_CONSEQUENCE}. ` + + `Fix the key name, or remove it. ${PASSTHROUGH}`, + ); + expect(errorOf({ workflows: [{ path: "ci.yml", state: "active", enabled: true }] })).toBe( + 'f.yml has malformed section entries: workflows[ci.yml]: declares "enabled", which this ' + + "section does not recognize (known keys: path, state) - the enable/disable calls send no " + + `payload, so the key would silently do nothing. Fix the key name, or remove it. ${PASSTHROUGH}`, + ); }); test("open sections still pass extra keys through", () => { @@ -100,6 +137,18 @@ describe("closed-surface sections reject unrecognized entry keys upfront", () => }; expect(validateSectionShapes(doc, "f.yml")).toEqual({ settings: doc }); }); + + test("closed-surface entry checks see through the wrapper (collaborators)", () => { + expect( + errorOf({ + collaborators: { _undeclared: "keep", entries: [{ username: "alice", permision: "x" }] }, + }), + ).toBe( + 'f.yml has malformed section entries: collaborators[alice]: declares "permision", which ' + + `this section does not recognize (known keys: username, permission) - ${ROLE_CONSEQUENCE}. ` + + `Fix the key name, or remove it. ${PASSTHROUGH}`, + ); + }); }); describe("the wrapped undeclared-policy form", () => { @@ -116,18 +165,27 @@ describe("the wrapped undeclared-policy form", () => { test("wrapper typos fail upfront: an unknown wrapper key and a bad policy value", () => { // The wrapper is this action's own strict vocabulary; unlike entry - // passthrough fields, its extra keys have nowhere to go. - const unknownKey = errorOf({ labels: { entires: [{ name: "bug" }] } }); - expect(unknownKey).toContain('"entires"'); - const badPolicy = errorOf({ milestones: { _undeclared: "detele", entries: [] } }); - expect(badPolicy).toContain("milestones._undeclared"); + // passthrough fields, its extra keys have nowhere to go. A misspelled + // "entries" reads as both a missing list and an unrecognized key. + expect(errorOf({ labels: { entires: [{ name: "bug" }] } })).toBe( + "f.yml has malformed section entries: labels.entries: Invalid input: expected array, " + + `received undefined; labels: Unrecognized key: "entires". ${PASSTHROUGH}`, + ); + expect(errorOf({ milestones: { _undeclared: "detele", entries: [] } })).toBe( + "f.yml has malformed section entries: milestones._undeclared: Invalid option: expected " + + `one of "keep"|"delete". ${PASSTHROUGH}`, + ); }); test("entry paths keep their precision inside the wrapper", () => { - const error = errorOf({ - rulesets: { entries: [{ name: "r", conditions: { ref_name: { include: "main" } } }] }, - }); - expect(error).toContain("rulesets.entries[0].conditions.ref_name.include"); + expect( + errorOf({ + rulesets: { entries: [{ name: "r", conditions: { ref_name: { include: "main" } } }] }, + }), + ).toBe( + "f.yml has malformed section entries: rulesets.entries[0].conditions.ref_name.include: " + + `Invalid input: expected array, received string. ${PASSTHROUGH}`, + ); }); test.each([ @@ -154,11 +212,4 @@ describe("the wrapped undeclared-policy form", () => { `nested objects like actions.cache, which reject unrecognized keys)`, ); }); - - test("closed-surface entry checks see through the wrapper (collaborators)", () => { - const error = errorOf({ - collaborators: { _undeclared: "keep", entries: [{ username: "alice", permision: "x" }] }, - }); - expect(error).toContain('collaborators[alice]: declares "permision"'); - }); }); diff --git a/test/github/api.test.ts b/test/github/api.test.ts index 56481901..c4727839 100644 --- a/test/github/api.test.ts +++ b/test/github/api.test.ts @@ -66,7 +66,8 @@ describe("retry and throttling", () => { const result = await api().tryRequest("GET", "/hopeless"); expect(state.calls).toBe(1 + MAX_RETRIES); expect("error" in result && result.error.status).toBe(429); - expect("error" in result && result.error.message).toContain("rate limited"); + // No JSON content-type on the stubbed body, so the raw text is the message. + expect("error" in result && result.error.message).toBe('{"message":"rate limited"}'); }); test("a rate-limit reset beyond the 60s cap fails now instead of stalling", async () => { @@ -286,7 +287,9 @@ describe("error body shaping", () => { "https://docs.github.com/rest/repos/rules", ); // errors[] stays appended to the message, as before. - expect("error" in result && result.error.message).toContain("Invalid rule"); + expect("error" in result && result.error.message).toBe( + 'Validation Failed ([{"field":"rules","message":"Invalid rule"}])', + ); }); test("string and empty bodies leave documentationUrl unset", async () => { @@ -448,47 +451,59 @@ describe("debug-trace hardening for redacted slugs", () => { }); describe("withheld() rebuilds from the allowlist", () => { - const ALLOWLIST = ["status", "message", "body", "rateLimited", "graphqlTypes"]; const base = { status: 422, message: "echo: CANARY", body: '{"echo":"CANARY"}' }; + const rebuilt = { status: 422, message: "withheld reason", body: "withheld reason" }; + const rebuiltRateLimited = { ...rebuilt, rateLimited: true }; // Each shape smuggles CANARY past a different field FILTER (spread, copy - // loop, for..in); only a rebuild that names its fields drops them all. + // loop, for..in); only a rebuild that names its fields drops them all. The + // third column is the exact output, own keys in declaration order: a + // rateLimited: true anywhere on the input (own or inherited) is the one + // field that survives beside the status. test.each([ [ "extra fields", { ...base, documentationUrl: "https://docs/CANARY", extra: "CANARY" } as ApiError, + rebuilt, + ], + [ + "nested objects", + { ...base, rateLimited: true, nested: { deep: "CANARY" } } as ApiError, + rebuiltRateLimited, ], - ["nested objects", { ...base, rateLimited: true, nested: { deep: "CANARY" } } as ApiError], - ["a toJSON hook", { ...base, toJSON: () => ({ leak: "CANARY" }) } as ApiError], + ["a toJSON hook", { ...base, toJSON: () => ({ leak: "CANARY" }) } as ApiError, rebuilt], [ "an enumerable getter", Object.defineProperty({ ...base }, "leak", { get: () => "CANARY", enumerable: true, }) as ApiError, + rebuilt, ], [ "prototype-chain properties", Object.assign(Object.create({ inherited: "CANARY", rateLimited: true }), base) as ApiError, + rebuiltRateLimited, ], - ])("the output carries ONLY allowlisted own data fields against %s", (_shape, error) => { - const out = withheld(error, "withheld reason"); - expect(Object.getPrototypeOf(out)).toBe(Object.prototype); - for (const key of Reflect.ownKeys(out)) { - expect(ALLOWLIST).toContain(String(key)); - const descriptor = Object.getOwnPropertyDescriptor(out, key); - expect(descriptor?.get).toBeUndefined(); - } - expect(out.status).toBe(422); - expect(out.message).toBe("withheld reason"); - expect(out.body).toBe("withheld reason"); - expect(JSON.stringify(out)).not.toContain("CANARY"); - let inherited = 0; - for (const _key in out) { - inherited += 1; - } - expect(inherited).toBe(Object.keys(out).length); - }); + ])( + "the output carries ONLY allowlisted own data fields against %s", + (_shape, error, expected) => { + const out = withheld(error, "withheld reason"); + expect(Object.getPrototypeOf(out)).toBe(Object.prototype); + expect([...Reflect.ownKeys(out)].sort()).toEqual(Object.keys(expected).sort()); + expect(out).toEqual(expected); + for (const key of Reflect.ownKeys(out)) { + const descriptor = Object.getOwnPropertyDescriptor(out, key); + expect(descriptor?.get).toBeUndefined(); + } + expect(JSON.stringify(out)).not.toContain("CANARY"); + let inherited = 0; + for (const _key in out) { + inherited += 1; + } + expect(inherited).toBe(Object.keys(out).length); + }, + ); test("the classification fields survive as fresh values; anything falsy or absent is dropped", () => { const types = Object.freeze(["FORBIDDEN", "NOT_FOUND"]); @@ -527,7 +542,6 @@ describe("withheld() rebuilds from the allowlist", () => { "r", ); expect(disguised).toEqual({ status: 404, message: "r", body: "r" }); - expect(JSON.stringify(disguised)).not.toContain("CANARY"); }); }); @@ -552,6 +566,14 @@ describe("secret-field request redaction and fail-closed error responses", () => // scan for the original string would find the echo. const hostileSecret = 'he said "no" \\ back\nslash'; + // The fixed tail of every unsent-payload abort, after the scan's reason clause. + const NOT_SENT_TAIL = + ", so it could not be safely inspected for secret fields. Replace that value with a plain string in the settings file"; + // The reason when the scan has no typed rejection to name a field with: a + // hostile proxy, a bare bigint, a cycle. + const NOT_PLAIN_FALLBACK = + "its payload is not plain JSON data (a cyclic value, or a value carrying a function or exotic prototype)"; + test("config.secret is masked in the trace; the outgoing request is untouched", async () => { const sent = stubFetchCapturingBodies(() => new Response(null, { status: 204 })); const dbg = traceIo(); @@ -565,8 +587,12 @@ describe("secret-field request redaction and fail-closed error responses", () => // Non-secret payload fields still trace normally. expect(trace).toContain('"url":"https://example.test/hook"'); // The wire carries the real value; only the trace is masked. - expect(sent.bodies.join("")).toContain(JSON.stringify(hostileSecret).slice(1, -1)); - expect(sent.bodies.join("")).not.toContain("***"); + expect(sent.bodies).toEqual([ + JSON.stringify({ + name: "web", + config: { url: "https://example.test/hook", content_type: "json", secret: hostileSecret }, + }), + ]); }); test("encrypted_value is masked in the trace", async () => { @@ -654,7 +680,13 @@ describe("secret-field request redaction and fail-closed error responses", () => const trace = dbg.lines.join(""); expect(trace).toContain('"secret":"***"'); expect(trace).not.toContain("he said"); - expect(sent.bodies.join("")).toContain(JSON.stringify(hostileSecret).slice(1, -1)); + expect(sent.bodies).toEqual([ + JSON.stringify({ + url: "https://example.test/hook", + content_type: "json", + secret: hostileSecret, + }), + ]); if (!("error" in result)) { throw new Error("expected an error result"); } @@ -704,9 +736,10 @@ describe("secret-field request redaction and fail-closed error responses", () => if (thrown === undefined) { throw new Error("expected a thrown transport error"); } - expect(thrown.message).toContain("details withheld"); - expect(thrown.message).not.toContain("he said"); - expect(thrown.message).not.toContain("body was"); + // The withholding clause replaces the transport's own text wholesale. + expect(thrown.message).toBe( + "PATCH /repos/hookco/hookrepo/hooks/1/config failed: the transport failed before an HTTP response arrived (details withheld: the request carried a secret field). Check network connectivity from the runner to https://api.test, then re-run the workflow", + ); }); test("a transport failure on a non-secret request keeps its diagnostic message", async () => { @@ -719,7 +752,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("socket hang up"); + expect(thrown?.message).toBe( + "GET /repos/hookco/hookrepo failed: socket hang up. Check network connectivity from the runner to https://api.test, then re-run the workflow", + ); }); test("a secret-carrying 403 rate limit still classifies as a rate limit", async () => { @@ -933,8 +968,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); - expect(thrown?.message).not.toContain("he said"); + expect(thrown?.message).toBe( + `PATCH /repos/hookco/hookrepo/hooks/1/config was not sent: the value at "toJSON" is not plain JSON data (a function)${NOT_SENT_TAIL}`, + ); expect(calls).toBe(0); expect(sent.bodies).toHaveLength(0); expect(dbg.lines.join("")).not.toContain("he said"); @@ -960,8 +996,10 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); - expect(thrown?.message).not.toContain("he said"); + // The trap's throw is swallowed: no field name, so the fallback reason. + expect(thrown?.message).toBe( + `PATCH /repos/hookco/hookrepo/hooks/1/config was not sent: ${NOT_PLAIN_FALLBACK}${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); }); @@ -984,7 +1022,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); + expect(thrown?.message).toBe( + `POST /repos/hookco/hookrepo/anything was not sent: the value at "note" is not plain JSON data (an accessor property)${NOT_SENT_TAIL}`, + ); expect(getterRan).toBe(false); expect(sent.bodies).toHaveLength(0); }); @@ -1008,7 +1048,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); + expect(thrown?.message).toBe( + `POST /repos/hookco/hookrepo/anything was not sent: the value is not plain JSON data (a non-plain object)${NOT_SENT_TAIL}`, + ); expect(overrideRan).toBe(false); expect(sent.bodies).toHaveLength(0); }); @@ -1027,8 +1069,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); - expect(thrown?.message).toContain("!!timestamp"); + expect(thrown?.message).toBe( + `POST /repos/hookco/hookrepo/anything was not sent: the value at "stamp" is not plain JSON data (a Date, e.g. from a YAML !!timestamp tag)${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); }); @@ -1040,7 +1083,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); + expect(thrown?.message).toBe( + `POST /repos/hookco/hookrepo/anything was not sent: ${NOT_PLAIN_FALLBACK}${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); }); @@ -1060,7 +1105,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); + expect(thrown?.message).toBe( + `POST /repos/hookco/hookrepo/anything was not sent: the value is not plain JSON data (a non-plain object)${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); }); @@ -1106,8 +1153,10 @@ describe("secret-field request redaction and fail-closed error responses", () => if (thrown === undefined) { throw new Error("expected a thrown abort"); } - expect(thrown.message).toContain("was not sent"); - expect(thrown.message).not.toContain("he said"); + // The abort names the field but never the getter's own message. + expect(thrown.message).toBe( + `PATCH /repos/hookco/hookrepo/hooks/1/config was not sent: the value at "secret" is not plain JSON data (an accessor property)${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); expect(dbg.lines.join("")).not.toContain("he said"); }); @@ -1123,8 +1172,9 @@ describe("secret-field request redaction and fail-closed error responses", () => } catch (error) { thrown = error as Error; } - expect(thrown?.message).toContain("was not sent"); - expect(thrown?.message).not.toContain("he said"); + expect(thrown?.message).toBe( + `PATCH /repos/hookco/hookrepo/hooks/1/config was not sent: ${NOT_PLAIN_FALLBACK}${NOT_SENT_TAIL}`, + ); expect(sent.bodies).toHaveLength(0); expect(dbg.lines.join("")).not.toContain("he said"); }); @@ -1219,11 +1269,14 @@ describe("secret-field request redaction and fail-closed error responses", () => if (!("error" in result)) { throw new Error("expected an error result"); } - // The error keeps the message/errors/body shaping exactly as before. + // The error keeps the message/errors/body shaping exactly as before; the + // body is the response JSON re-serialized, key order intact. expect(result.error.message).toBe( 'Validation Failed ([{"field":"name","message":"bad name"}])', ); - expect(result.error.body).toContain('"bad name"'); + expect(result.error.body).toBe( + '{"message":"Validation Failed","errors":[{"field":"name","message":"bad name"}],"documentation_url":"https://docs.github.com/rest"}', + ); expect(result.error.documentationUrl).toBe("https://docs.github.com/rest"); }); }); @@ -1261,12 +1314,12 @@ describe("DELETE request bodies reach the wire", () => { }, ); expect(result).toEqual({ data: null }); - expect(received).toHaveLength(1); - expect(received[0]?.method).toBe("DELETE"); - expect(JSON.parse(received[0]?.body ?? "")).toEqual({ - patterns: [{ pattern_id: 7, custom_pattern_version: "v2" }], - post_delete_action: "resolve_alerts", - }); + expect(received).toEqual([ + { + method: "DELETE", + body: '{"patterns":[{"pattern_id":7,"custom_pattern_version":"v2"}],"post_delete_action":"resolve_alerts"}', + }, + ]); } finally { await server.stop(true); } diff --git a/test/github/graphql.test.ts b/test/github/graphql.test.ts index ec5671c0..6c473610 100644 --- a/test/github/graphql.test.ts +++ b/test/github/graphql.test.ts @@ -81,7 +81,7 @@ describe("tryGraphql errors[] mapping", () => { throw new Error("expected an error result"); } expect(result.error.status).toBe(404); - expect(result.error.message).toContain("Could not resolve"); + expect(result.error.message).toBe("Could not resolve to a Repository"); expect(result.error.graphqlTypes).toEqual(["NOT_FOUND"]); expect(isPermissionError(result.error)).toBe(true); }); @@ -124,7 +124,10 @@ describe("tryGraphql errors[] mapping", () => { } expect(result.error.status).toBe(422); expect(result.error.message).toBe("first problem; second problem"); - expect(result.error.body).toContain("UNPROCESSABLE"); + // The body is the errors[] array alone, never the whole envelope. + expect(result.error.body).toBe( + '[{"type":"UNPROCESSABLE","message":"first problem"},{"message":"second problem"}]', + ); // A partially-typed response carries NO graphqlTypes: the untyped entry // must make the whole response untolerable, not hide behind its typed // sibling. @@ -256,8 +259,11 @@ describe("tryGraphql tracing and redaction", () => { { owner: "o", repo: "secretrepo", pattern: "CANARY-live" }, "o/secretrepo", ); + // The client's line is the constant alone (an exact element, not a fragment): + // no operation name, slug, or variable survives anywhere in the trace. The + // other lines are octokit's own timed chatter, so they are not pinned. + expect(dbg.lines).toContain(""); const trace = dbg.lines.join("\n"); - expect(trace).toContain(""); expect(trace).not.toContain("RepoToggles"); expect(trace).not.toContain("secretrepo"); expect(trace).not.toContain("CANARY-live"); @@ -425,10 +431,9 @@ describe("tryGraphql tracing and redaction", () => { ); // The withholding marker replaces the reason wholesale: neither the raw // transport text nor the slug survives into the thrown message. - expect(thrown).toContain("details withheld: the repository is redacted"); - expect(thrown).not.toContain(rawReason); - expect(thrown).not.toContain("socket hang up"); - expect(thrown).not.toContain("secretrepo"); + expect(thrown).toBe( + "Error: GRAPHQL RepoToggles failed: the transport failed before an HTTP response arrived (details withheld: the repository is redacted). Check network connectivity from the runner to https://api.test, then re-run the workflow", + ); }); test("a secret-named variable is masked in the trace and its error body withheld", async () => { diff --git a/test/github/paginate.test.ts b/test/github/paginate.test.ts index d4a2087b..131e7a6a 100644 --- a/test/github/paginate.test.ts +++ b/test/github/paginate.test.ts @@ -12,7 +12,10 @@ describe("paginate", () => { }); const result = await paginate(api, "/things"); expect(result).toEqual({ items: [...fullPage("a"), "b0", "b1"] }); - expect(api.calls).toHaveLength(2); + expect(api.calls.map((c) => c.path)).toEqual([ + "/things?per_page=100&page=1", + "/things?per_page=100&page=2", + ]); }); test("stop ends the walk early with the items collected so far", async () => { @@ -22,7 +25,7 @@ describe("paginate", () => { }); const result = await paginate(api, "/things", undefined, (items) => items.includes("a5")); expect(result).toEqual({ items: fullPage("a") }); - expect(api.calls).toHaveLength(1); + expect(api.calls.map((c) => c.path)).toEqual(["/things?per_page=100&page=1"]); }); test("a stop that never fires leaves paging behavior unchanged", async () => { @@ -32,7 +35,10 @@ describe("paginate", () => { }); const result = await paginate(api, "/things", undefined, () => false); expect(result).toEqual({ items: [...fullPage("a"), "b0"] }); - expect(api.calls).toHaveLength(2); + expect(api.calls.map((c) => c.path)).toEqual([ + "/things?per_page=100&page=1", + "/things?per_page=100&page=2", + ]); }); test("an existing query string keeps its params and gains the page ones", async () => { diff --git a/test/private.test.ts b/test/private.test.ts index 3c964360..365fbd26 100644 --- a/test/private.test.ts +++ b/test/private.test.ts @@ -114,7 +114,7 @@ describe("Private", () => { expect(isPrivate(plain)).toBe(false); } // The box neither stringifies nor serializes to its content. - expect(`${sealed}`).not.toContain("o/priv"); + expect(`${sealed}`).toBe("[object Object]"); expect(JSON.stringify(sealed)).toBe("{}"); }); diff --git a/test/published-schema.test.ts b/test/published-schema.test.ts index 03212236..b560a02a 100644 --- a/test/published-schema.test.ts +++ b/test/published-schema.test.ts @@ -54,9 +54,27 @@ describe("published schema wrapper strictness", () => { test("one wrapper definition per knobbed section and nested knob, each closed", () => { expect(wrapperNames.length).toBe(UNDECLARED_POLICY_SECTIONS.length + NESTED_WRAPPERS.length); - for (const nested of NESTED_WRAPPERS) { - expect(wrapperNames).toContain(nested); - } + expect([...wrapperNames].sort()).toEqual([ + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + "UndeclaredPolicyList", + ]); for (const name of wrapperNames) { expect( schema.definitions[name]?.additionalProperties, @@ -243,10 +261,18 @@ describe("published schema wrapper strictness", () => { // against the ONE shared fixture set the zod superRefine is also // tested with - and, per fixture, the AJV verdict must agree with // validateSectionShapes (no error = valid), so the schema copy of the - // invariant cannot drift from the runtime copy. The class counts pin - // the SET: a deleted fixture would silently weaken both consumers. - expect(FLAG_PAIRING_FIXTURES.filter((f) => !f.valid)).toHaveLength(4); - expect(FLAG_PAIRING_FIXTURES.filter((f) => f.valid)).toHaveLength(3); + // invariant cannot drift from the runtime copy. The [name, valid] + // pairs pin the SET: a deleted or flipped fixture would silently + // weaken both consumers. + expect(FLAG_PAIRING_FIXTURES.map((f) => [f.name, f.valid])).toEqual([ + ["patterns without the sibling flag object", false], + ["patterns with the flag present but false", false], + ["patterns with the sibling nulled (a clear)", false], + ["the wrapped form takes the same rule", false], + ["the paired form (flag true) passes", true], + ["the wrapped paired form passes", true], + ["an entry without the plural key keeps its freedom (nullable flag)", true], + ]); for (const { name, entry, valid } of FLAG_PAIRING_FIXTURES) { const doc = { environments: [entry] }; expect(validate(doc), `published schema: ${name}`).toBe(valid); diff --git a/test/report/artifact-report.test.ts b/test/report/artifact-report.test.ts index ff70a42e..a7b63725 100644 --- a/test/report/artifact-report.test.ts +++ b/test/report/artifact-report.test.ts @@ -84,21 +84,24 @@ describe("deliverArtifactReport", () => { throw new Error("Unable to get the ACTIONS_RUNTIME_TOKEN env variable"); }, }; - const result = await deliverArtifactReport("doc", recipient, uploader); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("could not upload the private report artifact"); - expect(result.warning).toContain("ACTIONS_RUNTIME_TOKEN"); + expect(await deliverArtifactReport("doc", recipient, uploader)).toEqual({ + warning: + "could not upload the private report artifact: Unable to get the ACTIONS_RUNTIME_TOKEN " + + "env variable. Re-run the workflow, or set private-report: none if it persists", + }); }); test("a malformed recipient is a warning and the uploader is never called", async () => { const { uploader, uploads } = captureUploader(); const result = await deliverArtifactReport("doc", "not-a-key", uploader); + // The middle of the warning is the age library's own wording for a bad recipient, so only + // our prefix and advice are pinned around it. expect(result).toEqual({ - warning: expect.stringContaining("could not upload the private report artifact"), + warning: expect.stringMatching( + /^could not upload the private report artifact: .+\. Re-run the workflow, or set private-report: none if it persists$/, + ), }); - expect(uploads).toHaveLength(0); + expect(uploads).toEqual([]); }); }); @@ -123,15 +126,16 @@ describe("default uploader without a runtime token", () => { const { recipient } = await testKeypair(); const result = await deliverArtifactReport("secret document", recipient); - // exactly one warning, and it is ours (the client's own text never appears) - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("could not upload the private report artifact"); - expect(result.warning).toContain("ACTIONS_RUNTIME_TOKEN"); + // exactly one warning, and it is ours (the client's own text never + // appears, and neither does any report content) + expect(result).toEqual({ + warning: + "could not upload the private report artifact: the artifact service is unavailable: no " + + "ACTIONS_RUNTIME_TOKEN in the environment. Artifact upload needs a GitHub-hosted or " + + "self-hosted Actions runner (it is not available on GitHub Enterprise Server or outside " + + "Actions). Re-run the workflow, or set private-report: none if it persists", + }); expect(uploadSpy).toHaveBeenCalledTimes(0); - // and no report content ever leaves the module - expect(result.warning).not.toContain("secret document"); } finally { uploadSpy.mockRestore(); } diff --git a/test/report/composer.test.ts b/test/report/composer.test.ts index bcceebc8..ecaa7dc9 100644 --- a/test/report/composer.test.ts +++ b/test/report/composer.test.ts @@ -23,17 +23,38 @@ function input(overrides: Partial = {}): ReportInput { describe("composeReport", () => { test("renders the run metadata, the outcome table, and the transcript", () => { - const report = composeReport(input()); - expect(report).toContain("# settings-as-code private report: o/private-repo"); - expect(report).toContain("| Admin repository | o/admin |"); - expect(report).toContain("| Run | https://github.com/o/admin/actions/runs/42 |"); - expect(report).toContain("| Mode | check |"); - expect(report).toContain("| Result | drift |"); - expect(report).toContain("| Generated | 2026-07-22T10:00:00.000Z |"); - expect(report).toContain("| labels | drift | labels[secret-project]: missing |"); - expect(report).toContain("| repository | clean | |"); - expect(report).toContain("labels: comparing 3 labels"); - expect(report).toContain("[warning] labels[secret-project]: missing"); + // The timestamp is an input, so the whole document is deterministic. + expect(composeReport(input())).toBe( + [ + "# settings-as-code private report: o/private-repo", + "", + "Full, unredacted report for this target. The public run redacts it; this document is its private mirror.", + "", + "| | |", + "|---|---|", + "| Target | o/private-repo |", + "| Admin repository | o/admin |", + "| Run | https://github.com/o/admin/actions/runs/42 |", + "| Mode | check |", + "| Result | drift |", + "| Generated | 2026-07-22T10:00:00.000Z |", + "", + "## Sections", + "", + "| Section | Status | Detail |", + "|---|---|---|", + "| labels | drift | labels[secret-project]: missing |", + "| repository | clean | |", + "", + "## Transcript", + "", + "```", + "labels: comparing 3 labels", + "[warning] labels[secret-project]: missing", + "```", + "", + ].join("\n"), + ); }); test("joins multi-line detail with
and escapes table pipes", () => { diff --git a/test/report/delivery.test.ts b/test/report/delivery.test.ts index fb8107a8..27bd46f3 100644 --- a/test/report/delivery.test.ts +++ b/test/report/delivery.test.ts @@ -121,14 +121,11 @@ describe("the issue channel", () => { }); const { io, annotations } = recordingIo(); await open(api, "issue", io)?.deliver(target("o/priv", 1)); - expect(annotations).toHaveLength(1); - const warning = annotations[0] ?? ""; - expect(warning).toStartWith( - "warning: private repository #1: could not deliver the private report", - ); - expect(warning).toContain("HTTP 403"); - expect(warning).not.toContain("o/priv"); - expect(warning).not.toContain("Resource not accessible"); + expect(annotations).toEqual([ + "warning: private repository #1: could not deliver the private report (HTTP 403). To fix, " + + 'grant "Issues" (read and write) under the PAT\'s Repository permissions for the target ' + + "repository, or set private-report: none", + ]); }); test("a target whose slug did not parse gets one safe warning and no API traffic", async () => { @@ -223,11 +220,11 @@ describe("the artifact channel", () => { const channel = openReportChannel(new MockApi({}), "artifact", META, recipient, io, uploader); await channel?.deliver(target("o/priv", 1)); await channel?.flush(); - expect(annotations).toHaveLength(1); - expect(annotations[0]).toStartWith("warning: could not upload the private report artifact"); - expect(annotations[0]).toContain("ACTIONS_RUNTIME_TOKEN"); - expect(annotations[0]).not.toContain("o/priv"); - expect(annotations[0]).not.toContain("CANARY"); + expect(annotations).toEqual([ + "warning: could not upload the private report artifact: Unable to get the " + + "ACTIONS_RUNTIME_TOKEN env variable. Re-run the workflow, or set private-report: none if " + + "it persists", + ]); }); }); diff --git a/test/report/issue-report.test.ts b/test/report/issue-report.test.ts index d65c0363..0d36043a 100644 --- a/test/report/issue-report.test.ts +++ b/test/report/issue-report.test.ts @@ -36,8 +36,11 @@ describe("deliverIssueReport", () => { }); const result = await deliverIssueReport(api, SLUG, "the report body", true, "always"); expect(result).toEqual({ url: "https://github.com/o/private-repo/issues/7" }); - const lookups = api.calls.filter((c) => c.method === "GET"); - expect(lookups).toHaveLength(1); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + LABEL_CREATE, + LABEL_LOOKUP, + "PATCH /repos/o/private-repo/issues/7", + ]); const patch = api.calls.find((c) => c.method === "PATCH"); expect(patch?.payload).toEqual({ body: "the report body", state: "open" }); }); @@ -83,7 +86,13 @@ describe("deliverIssueReport", () => { }); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); expect(result).toEqual({ url: "https://github.com/o/private-repo/issues/3" }); - expect(api.calls.some((c) => `${c.method} ${c.path}` === ISSUE_CREATE)).toBe(false); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + LABEL_CREATE, + LABEL_LOOKUP, + "GET /user", + CREATOR_SCAN, + "PATCH /repos/o/private-repo/issues/3", + ]); }); test("a fallback-scan hit without the marker reattaches it on the upsert PATCH", async () => { @@ -152,7 +161,13 @@ describe("deliverIssueReport", () => { const result = await deliverIssueReport(api, SLUG, "body", true, "always"); expect(result).toEqual({ url: "https://github.com/o/private-repo/issues/150" }); // A full page came back, but the match stops the walk: no page=2 request. - expect(api.calls.filter((c) => c.path.includes("page=2"))).toHaveLength(0); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + LABEL_CREATE, + LABEL_LOOKUP, + "GET /user", + CREATOR_SCAN, + "PATCH /repos/o/private-repo/issues/150", + ]); }); test("nothing anywhere: POST with the marker label, then close when healthy", async () => { @@ -166,10 +181,14 @@ describe("deliverIssueReport", () => { }); const result = await deliverIssueReport(api, SLUG, "body", false, "always"); expect(result).toEqual({ url: "https://github.com/o/private-repo/issues/9" }); - const scanAt = api.calls.findIndex((c) => `${c.method} ${c.path}` === CREATOR_SCAN); - const createAt = api.calls.findIndex((c) => `${c.method} ${c.path}` === ISSUE_CREATE); - expect(scanAt).toBeGreaterThanOrEqual(0); - expect(scanAt).toBeLessThan(createAt); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + LABEL_CREATE, + LABEL_LOOKUP, + "GET /user", + CREATOR_SCAN, + ISSUE_CREATE, + "PATCH /repos/o/private-repo/issues/9", + ]); const close = api.calls.find((c) => c.method === "PATCH"); expect(close?.payload).toEqual({ state: "closed" }); }); @@ -184,7 +203,13 @@ describe("deliverIssueReport", () => { }); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); expect(result).toEqual({ url: "https://github.com/o/private-repo/issues/9" }); - expect(api.calls.some((c) => c.method === "PATCH")).toBe(false); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + LABEL_CREATE, + LABEL_LOOKUP, + "GET /user", + CREATOR_SCAN, + ISSUE_CREATE, + ]); }); test("a denied marker-label create is a safe warning and stops everything", async () => { @@ -194,13 +219,12 @@ describe("deliverIssueReport", () => { }, }); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("HTTP 403"); - expect(result.warning).toContain('"Issues" (read and write)'); - expect(result.warning).not.toContain(SLUG.slug); - expect(api.calls).toHaveLength(1); + expect(result).toEqual({ + warning: + 'could not deliver the private report (HTTP 403). To fix, grant "Issues" (read and write) ' + + "under the PAT's Repository permissions for the target repository, or set private-report: none", + }); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([LABEL_CREATE]); }); test("a non-permission failure gets re-run advice, no grant prose", async () => { @@ -212,13 +236,11 @@ describe("deliverIssueReport", () => { }, }); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("HTTP 500"); - expect(result.warning).toContain("Re-run the workflow"); - expect(result.warning).not.toContain("Issues"); - expect(result.warning).not.toContain(SLUG.slug); + expect(result).toEqual({ + warning: + "could not deliver the private report (HTTP 500). Re-run the workflow, or set " + + "private-report: none if it persists", + }); }); test("a throwing transport never escapes; the warning stays slug-free", async () => { @@ -226,11 +248,11 @@ describe("deliverIssueReport", () => { // failure (GithubApi throws those with the path in the message). const api = new MockApi({}); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("could not deliver the private report"); - expect(result.warning).not.toContain(SLUG.slug); + expect(result).toEqual({ + warning: + "could not deliver the private report: the request failed before an HTTP response " + + "arrived. Re-run the workflow, or set private-report: none if it persists", + }); }); test("a non-list lookup response is a warning, not a crash", async () => { @@ -239,10 +261,11 @@ describe("deliverIssueReport", () => { [LABEL_LOOKUP]: { data: { message: "unexpected" } }, }); const result = await deliverIssueReport(api, SLUG, "body", true, "always"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("the report-issue lookup returned a non-list response"); + expect(result).toEqual({ + warning: + "could not deliver the private report: the report-issue lookup returned a non-list " + + 'response. Check the "api-version" input, or set private-report: none', + }); }); }); @@ -281,11 +304,11 @@ describe("deliverIssueReport under mode: on-failure", () => { [OPEN_LOOKUP]: { error: { status: 500, message: "boom o/private-repo", body: "" } }, }); const result = await deliverIssueReport(api, SLUG, "body", false, "on-failure"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("HTTP 500"); - expect(result.warning).not.toContain(SLUG.slug); + expect(result).toEqual({ + warning: + "could not deliver the private report (HTTP 500). Re-run the workflow, or set " + + "private-report: none if it persists", + }); }); test("a failing close-PATCH is a safe warning too", async () => { @@ -296,20 +319,21 @@ describe("deliverIssueReport under mode: on-failure", () => { }, }); const result = await deliverIssueReport(api, SLUG, "body", false, "on-failure"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("HTTP 403"); - expect(result.warning).not.toContain(SLUG.slug); + expect(result).toEqual({ + warning: + 'could not deliver the private report (HTTP 403). To fix, grant "Issues" (read and write) ' + + "under the PAT's Repository permissions for the target repository, or set private-report: none", + }); }); test("a non-list quiet-path response is a warning, not a crash", async () => { const api = new MockApi({ [OPEN_LOOKUP]: { data: { message: "unexpected" } } }); const result = await deliverIssueReport(api, SLUG, "body", false, "on-failure"); - if (!("warning" in result)) { - throw new Error("expected a warning"); - } - expect(result.warning).toContain("the open-issue lookup returned a non-list response"); + expect(result).toEqual({ + warning: + "could not deliver the private report: the open-issue lookup returned a non-list " + + 'response. Check the "api-version" input, or set private-report: none', + }); }); test("needs-attention requests are identical to always, on both upsert paths", async () => { @@ -347,7 +371,7 @@ describe("injectMarkerLabel", () => { const { settings: injected, outcome } = injectMarkerLabel(settings); expect(outcome).toBe("injected"); expect(injected.labels).toEqual([{ name: "bug", color: "d73a4a" }, MARKER_LABEL_CONFIG]); - expect(settings.labels).toHaveLength(1); + expect(settings.labels).toEqual([{ name: "bug", color: "d73a4a" }]); }); test("a wrapped labels section stays wrapped, keeping its undeclared policy", () => { diff --git a/test/schema-corpus.test.ts b/test/schema-corpus.test.ts index d4afd903..400439c6 100644 --- a/test/schema-corpus.test.ts +++ b/test/schema-corpus.test.ts @@ -74,7 +74,9 @@ function scenarioDocs(): CorpusDoc[] { push(`${name} ${repo} settings`, entry?.settings); } } - expect(docs.length).toBeGreaterThan(150); + // The corpus size is pinned exactly so a loader that silently drops a root, + // a file, or a document kind cannot pass; a new scenario updates the number. + expect(docs.length).toBe(252); return docs; } diff --git a/test/scripts/file-fuzz-issue.test.ts b/test/scripts/file-fuzz-issue.test.ts index d6f4efd7..94948fca 100644 --- a/test/scripts/file-fuzz-issue.test.ts +++ b/test/scripts/file-fuzz-issue.test.ts @@ -219,34 +219,54 @@ describe("fileIssue", () => { return { run, calls }; } - test("create path opens a labeled issue with the body", async () => { + const ensureLabel = [ + "label", + "create", + "e2e-fuzz", + "--force", + "--color", + "B60205", + "--description", + "e2e fuzz failure", + ]; + const findOpen = [ + "issue", + "list", + "--label", + "e2e-fuzz", + "--state", + "open", + "--limit", + "1", + "--json", + "number", + ]; + + // Assignment policy lives in the auto-assign workflow, which the nightly + // dispatches after filing; neither argv sequence below touches assignees. + test("create path ensures the label, finds no open issue, and opens a labeled issue with the body", async () => { const { run, calls } = fakeGh(undefined); await fileIssue(run, "body"); - const create = calls.find((c) => c[0] === "issue" && c[1] === "create"); - expect(create).toBeDefined(); - expect(create).toContain("--label"); - expect(create?.[create.indexOf("--label") + 1]).toBe("e2e-fuzz"); - expect(create?.[create.indexOf("--body") + 1]).toBe("body"); + expect(calls).toEqual([ + ensureLabel, + findOpen, + [ + "issue", + "create", + "--label", + "e2e-fuzz", + "--title", + "e2e fuzz failures (nightly)", + "--body", + "body", + ], + ]); }); - test("comment path comments on the existing issue, does not create a new one", async () => { + test("comment path ensures the label, then comments on the existing issue without creating one", async () => { const { run, calls } = fakeGh(3); await fileIssue(run, "body"); - expect(calls.some((c) => c[0] === "issue" && c[1] === "comment" && c[2] === "3")).toBe(true); - expect(calls.some((c) => c[0] === "issue" && c[1] === "create")).toBe(false); - }); - - test("the filer performs no assignment on either path", async () => { - // Assignment policy lives in the auto-assign workflow, which the nightly - // dispatches after filing; the filer must never touch assignees. - for (const openNumber of [undefined, 3]) { - const { run, calls } = fakeGh(openNumber); - await fileIssue(run, "body"); - const flat = calls.flat(); - expect(flat).not.toContain("--assignee"); - expect(flat).not.toContain("--add-assignee"); - expect(calls.some((c) => c[0] === "issue" && c[1] === "edit")).toBe(false); - } + expect(calls).toEqual([ensureLabel, findOpen, ["issue", "comment", "3", "--body", "body"]]); }); test("returns the created issue number (parsed from gh's create URL)", async () => { diff --git a/test/scripts/graduate-upstream-gaps.test.ts b/test/scripts/graduate-upstream-gaps.test.ts index 6accabb0..fb441ab0 100644 --- a/test/scripts/graduate-upstream-gaps.test.ts +++ b/test/scripts/graduate-upstream-gaps.test.ts @@ -234,48 +234,59 @@ describe("gapFileBases", () => { }); describe("generateIndex", () => { - test("is deterministic and sorts the bases itself", () => { - const sorted = generateIndex(["merge-queue", "pages-https"]); - expect(generateIndex(["pages-https", "merge-queue"])).toBe(sorted); - expect(generateIndex(["merge-queue", "pages-https"])).toBe(sorted); - }); + const header = `/** + * GENERATED by gen-gaps-index.ts - do not edit. Every pending upstream gap, + * aggregated in sorted file order; regenerate with + * \`bun .github/scripts/gen-gaps-index.ts\` after adding, deleting, or + * transforming a gap file. The derivations below degrade gracefully to an + * empty gaps set, so this file survives an empty directory. + */ - test("marks itself generated and un-editable in the header", () => { - expect(generateIndex([])).toContain("GENERATED by gen-gaps-index.ts - do not edit"); - }); +import { undocumentedRoutes } from "./gap.js"; +`; + const derivations = ` +type GapUnion = (typeof GAPS)[number]; - test("emits one import and one GAPS element per gap file", () => { - const index = generateIndex(["merge-queue", "pages-https"]); - expect(index).toContain('import { GAP as mergeQueue } from "./merge-queue.js";'); - expect(index).toContain('import { GAP as pagesHttps } from "./pages-https.js";'); - expect(index).toContain('import { undocumentedRoutes } from "./gap.js";'); - expect(index).toContain("const GAPS = [\n mergeQueue,\n pagesHttps,\n] as const;"); - }); +/** + * Routes GitHub documents but the pinned @octokit/types release does not + * carry yet: the octokit-kind gaps' routes (spec-only gaps are keyof + * Endpoints already). Only the route STRING is consumed (never octokit's + * parameter/response typing), so the literal union defineGap preserves is + * enough; an empty GAPS degrades it to never. + */ +export type SupplementalRoute = Extract["routes"][number]; - test("separates the two gap kinds in the derivations", () => { - const index = generateIndex(["merge-queue"]); - expect(index).toContain( - 'export type SupplementalRoute = Extract["routes"][number];', - ); - expect(index).toContain( - 'type SpecOnlyRoute = Extract["routes"][number];', - ); - expect(index).toContain( - "export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] =", - ); - expect(index).toContain("undocumentedRoutes(GAPS);"); - }); +/** The spec-only gaps' routes: typed by octokit, absent from the descriptor. */ +type SpecOnlyRoute = Extract["routes"][number]; + +/** + * The routes GitHub's api.github.com OpenAPI descriptor does not document: + * every spec-only gap's, plus those of octokit-kind gaps marked + * documentedInSpec: false. Consumed by test/e2e/openapi/paths.ts, which + * excludes their paths from the spec trim and exempts exactly these + * METHOD+path pairs from the e2e unknown-route check. + */ +export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] = + undocumentedRoutes(GAPS); +`; test("an empty directory degrades to an empty GAPS with both exports intact", () => { - const index = generateIndex([]); - expect(index).toContain("const GAPS = [] as const;"); - expect(index).toContain("export type SupplementalRoute"); - expect(index).toContain("export const UNDOCUMENTED_ROUTES"); - expect(index).not.toContain("GAP as"); + expect(generateIndex([])).toBe(`${header} +const GAPS = [] as const; +${derivations}`); }); - test("ends with a trailing newline", () => { - expect(generateIndex(["merge-queue"])).toEndWith("\n"); + test("emits one import and one GAPS element per gap file, in sorted order", () => { + expect( + generateIndex(["pages-https", "merge-queue"]), + ).toBe(`${header}import { GAP as mergeQueue } from "./merge-queue.js"; +import { GAP as pagesHttps } from "./pages-https.js"; + +const GAPS = [ + mergeQueue, + pagesHttps, +] as const; +${derivations}`); }); }); @@ -415,6 +426,9 @@ describe("the real src/upstream-gaps/ satisfies the scripts' contracts", () => { }); test("every real spec-pinned gap is rewritable to the spec-only template", async () => { + // The sweep is legitimately empty once every octokit-kind gap has graduated + // (the auto-fix workflow rewrites them to spec-only), so the corpus is not + // pinned here; the synthetic fixtures above pin the transform itself. for (const gap of realGapFiles) { const abs = join(import.meta.dir, "..", "..", gap); const source = readFileSync(abs, "utf8"); diff --git a/test/sections/contract.test.ts b/test/sections/contract.test.ts index 2fb35401..5e41a6ba 100644 --- a/test/sections/contract.test.ts +++ b/test/sections/contract.test.ts @@ -9,19 +9,13 @@ import { PermissionDenied, throwFor } from "../../src/sections/contract/errors.j import { type GraphqlOpDecl, graphqlOp } from "../../src/sections/contract/graphql.js"; import { denialPosture, - endpointPermission, planningReads, readGating, type SectionMeta, - sectionGrant, sectionOperations, writeGatedReads, } from "../../src/sections/contract/module.js"; -import { - grantFor, - type SectionPermission, - samePermission, -} from "../../src/sections/contract/permissions.js"; +import { type SectionPermission, samePermission } from "../../src/sections/contract/permissions.js"; import { hasDrift, plainData, planContext } from "../../src/sections/contract/plan.js"; import { declaredTolerance, @@ -69,30 +63,40 @@ describe("sectionOperations", () => { test("every REST endpoint and GraphQL operation of a real section appears exactly once", () => { // repositorySection carries BOTH dictionaries, so the GraphQL half of // the flattening binds (a section without `graphql` would prove only the - // REST half). Content equality over the role-keyed dictionaries is the - // exactly-once claim: `role` carries each operation's identity, so a - // duplicated entry canceling an omitted one with the SAME + // REST half). The rows are spelled out: `role` carries each operation's + // identity, so a duplicated entry canceling an omitted one with the SAME // {wire, grade, permission} tuple still fails on content. No repository // endpoint overrides accessGrade, so wire and grade coincide here; the - // override split is pinned by the overrides test below. + // override split is pinned by the overrides test below. Declaration order + // is not part of the contract, so both sides are compared sorted by role. expect(Object.keys(repositorySection.graphql ?? {}).length).toBeGreaterThan(0); - const phaseOf = (op: EndpointDecl | GraphqlOpDecl): "plan" | "execution" => op.phase ?? "plan"; - expect(sectionOperations(repositorySection)).toEqual([ - ...Object.entries(repositorySection.endpoints).map(([role, op]) => ({ - role, - wire: endpointKind(op), - grade: endpointKind(op), - permission: endpointPermission(repositorySection, op), - phase: phaseOf(op), - })), - ...Object.entries(repositorySection.graphql ?? {}).map(([role, op]) => ({ - role, - wire: op.kind, - grade: op.kind, - permission: endpointPermission(repositorySection, op), - phase: phaseOf(op), - })), - ]); + const admin = { repo: ["administration"] } as const; + const row = (role: string, kind: "read" | "write") => + ({ role, wire: kind, grade: kind, permission: admin, phase: "plan" }) as const; + const byRole = (a: { role: string }, b: { role: string }) => a.role.localeCompare(b.role); + expect([...sectionOperations(repositorySection)].sort(byRole)).toEqual( + [ + row("get", "read"), + row("update", "write"), + row("topics", "write"), + row("vulnerabilityAlertsGet", "read"), + row("vulnerabilityAlertsPut", "write"), + row("vulnerabilityAlertsRemove", "write"), + row("automatedSecurityFixesGet", "read"), + row("automatedSecurityFixesPut", "write"), + row("automatedSecurityFixesRemove", "write"), + row("privateVulnerabilityReportingGet", "read"), + row("privateVulnerabilityReportingPut", "write"), + row("privateVulnerabilityReportingRemove", "write"), + row("immutableReleasesGet", "read"), + row("immutableReleasesPut", "write"), + row("immutableReleasesRemove", "write"), + row("lfsPut", "write"), + row("lfsRemove", "write"), + row("featuresQuery", "read"), + row("updateFeatures", "write"), + ].sort(byRole), + ); }); test("resolves per-operation permission overrides and accessGrade write-gating", () => { @@ -172,7 +176,9 @@ describe("sectionOperations", () => { }, }; expect(() => denialPosture(postured)).toThrow( - /BUG: repository declares primaryRead on the execution-phase read GET \/apps\/\{app_slug\}/, + new Error( + "BUG: repository declares primaryRead on the execution-phase read GET /apps/{app_slug}; plan() never issues it, so no denied first read can be classified from it", + ), ); }); }); @@ -297,7 +303,9 @@ describe("throwFor context enrichment", () => { test("generic rejection without context keeps the classic shape", () => { expect(() => throwFor(section, "POST", "/repos/o/r/rulesets", rejection)).toThrow( - /rulesets: POST \/repos\/o\/r\/rulesets: 422 .*fix the "rulesets" values/, + new Error( + 'rulesets: POST /repos/o/r/rulesets: 422 Validation Failed ([{"field":"rules","message":"Invalid rule"}]). The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above', + ), ); }); @@ -306,7 +314,11 @@ describe("throwFor context enrichment", () => { throwFor(section, "POST", "/repos/o/r/rulesets", rejection, { operation: 'creating ruleset "quality"', }), - ).toThrow(/creating ruleset "quality" failed - POST \/repos\/o\/r\/rulesets: 422/); + ).toThrow( + new Error( + 'rulesets: creating ruleset "quality" failed - POST /repos/o/r/rulesets: 422 Validation Failed ([{"field":"rules","message":"Invalid rule"}]). The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above', + ), + ); }); test("a GraphQL rejection appends the declared outcome prose of each observed error type; undeclared types add nothing", () => { @@ -346,8 +358,12 @@ describe("throwFor context enrichment", () => { "settings file to satisfy the message above. The pinned list is full; unpin one in the " + "GitHub UI", ); - expect(message(["FORBIDDEN"])).toMatch(/message above$/); - expect(message([])).toMatch(/message above$/); + const undeclared = + 'rulesets: pinning environment "prod" failed - GRAPHQL PinEnvironment: 422 Repositories ' + + 'may only have 10 pinned. The API rejected the request; fix the "rulesets" values in the ' + + "settings file to satisfy the message above"; + expect(message(["FORBIDDEN"])).toBe(undeclared); + expect(message([])).toBe(undeclared); }); test("the status-matched hint and documentation_url are appended to the generic branch", () => { @@ -360,23 +376,26 @@ describe("throwFor context enrichment", () => { { op: endpoint({ hints: { 422: "Usually this means a typo" } }) }, ), ).toThrow( - /message above\. Usually this means a typo\. The fields and values this endpoint accepts are documented at https:\/\/docs\.github\.com\/rest\/repos\/rules$/, + new Error( + 'rulesets: POST /repos/o/r/rulesets: 422 Validation Failed ([{"field":"rules","message":"Invalid rule"}]). The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above. Usually this means a typo. The fields and values this endpoint accepts are documented at https://docs.github.com/rest/repos/rules', + ), ); }); test("a hint keyed to a different status is not rendered", () => { - try { + expect(() => throwFor( section, "POST", "/repos/o/r/rulesets", { status: 409, message: "Conflict", body: "" }, { op: endpoint({ hints: { 422: "never rendered on a 409" } }) }, - ); - } catch (error) { - expect(String(error)).toContain("409"); - expect(String(error)).not.toContain("never rendered on a 409"); - } + ), + ).toThrow( + new Error( + 'rulesets: POST /repos/o/r/rulesets: 409 Conflict. The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above', + ), + ); }); test("permission errors keep the grant advice and gain the operation label", () => { @@ -396,10 +415,9 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; - expect(denied.detail).toContain('creating ruleset "quality" failed - POST'); - expect(denied.detail).toContain(sectionGrant(section)); - expect(denied.detail).not.toContain("never rendered here"); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied creating ruleset "quality" failed - POST /repos/o/r/rulesets: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + ); }); test("denialHint is appended to the permission branch, and only there", () => { @@ -420,10 +438,8 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; - expect(denied.detail).toContain(sectionGrant(section)); - expect(denied.detail).toContain( - ". Note: a 403 here can also mean LFS is disabled account-wide", + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied PUT /repos/o/r/lfs: 403 Git LFS is globally disabled. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions. Note: a 403 here can also mean LFS is disabled account-wide', ); // The generic branch never renders it. expect(() => @@ -434,32 +450,43 @@ describe("throwFor context enrichment", () => { { status: 422, message: "nope", body: "" }, { op: endpoint({ denialHint: "not for 422s" }) }, ), - ).toThrow(/^(?!.*not for 422s).*fix the "rulesets" values/); + ).toThrow( + new Error( + 'rulesets: PUT /repos/o/r/lfs: 422 nope. The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above', + ), + ); }); - test("rate-limit and 5xx branches do not render the hint", () => { + test("the rate-limit and 5xx branches render their own advice without the hint", () => { // A 5xx-keyed hint is unrepresentable (HintableStatus), so the fixture - // carries a 422 one; the 500 branch must throw its own advice without it. + // carries a 422 one; each branch throws its own advice without it. + const hinted = { op: endpoint({ hints: { 422: "never rendered here" } }) }; expect(() => throwFor( section, "GET", "/repos/o/r/rulesets", { status: 500, message: "Server Error", body: "" }, - { op: endpoint({ hints: { 422: "never rendered here" } }) }, + hinted, ), - ).toThrow(/server error/); - try { + ).toThrow( + new Error( + "rulesets: GET /repos/o/r/rulesets: 500 Server Error. GitHub returned a server error; re-run the workflow, and retry later if it persists", + ), + ); + expect(() => throwFor( section, "GET", "/repos/o/r/rulesets", - { status: 500, message: "Server Error", body: "" }, - { op: endpoint({ hints: { 422: "never rendered here" } }) }, - ); - } catch (error) { - expect(String(error)).not.toContain("never rendered here"); - } + { status: 403, message: "API rate limit exceeded", body: "", rateLimited: true }, + hinted, + ), + ).toThrow( + new Error( + "rulesets: GET /repos/o/r/rulesets: 403 API rate limit exceeded. The API rate limit was hit; re-run the workflow after the limit resets, or use a token with a higher rate limit", + ), + ); }); test("a permission override renders the endpoint's own grant, not the section's", () => { @@ -476,13 +503,13 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; // The synthetic override has no sibling in the rulesets section carrying // the same permission, so the sibling scan finds no write and the advice // asks for read - what matters here is the RESOURCE: the endpoint's own // grant renders, never the section's. - expect(denied.detail).toContain(grantFor({ repo: ["actions"] }, undefined, "read")); - expect(denied.detail).not.toContain(sectionGrant(section)); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied POST /repos/o/r/actions/oidc/customization/sub: 403 Resource not accessible. To fix, grant "Actions" (read) under the PAT\'s Repository permissions', + ); }); test("override advice grades by the section's need: a write sibling on the same permission advises write", () => { @@ -503,7 +530,9 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - expect((thrown as PermissionDenied).detail).toContain(grantFor({ repo: ["actions"] })); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied GET /repos/o/r/actions/oidc/customization/sub: 403 Resource not accessible. To fix, grant "Actions" (read and write) under the PAT\'s Repository permissions', + ); }); test("override advice grades by the section's need: a read-only permission advises read", () => { @@ -523,9 +552,9 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; - expect(denied.detail).toContain(grantFor({ repo: ["actions"] }, undefined, "read")); - expect(denied.detail).not.toContain("read and write"); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied GET /repos/o/r/environments/prod/deployment-branch-policies: 404 Not Found (a 404 here can also mean the resource does not exist). To fix, grant "Actions" (read) under the PAT\'s Repository permissions. Note: a 404 here can also mean the environment does not exist, or that its deployment_branch_policy does not set custom_branch_policies: true', + ); }); test('a public endpoint ("none") cannot be a missing-grant failure', () => { @@ -545,7 +574,9 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).not.toBeInstanceOf(PermissionDenied); - expect(String(thrown)).toContain('fix the "rulesets" values'); + expect((thrown as Error).message).toBe( + 'rulesets: GET /repos/o/r/rulesets: 403 Forbidden. The API rejected the request; fix the "rulesets" values in the settings file to satisfy the message above', + ); }); test("a no-override denial keeps the section grant's caveat", () => { @@ -572,9 +603,8 @@ describe("throwFor context enrichment", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; - expect(denied.detail).toContain( - 'the "oidc_customization_sub" key alone instead needs "Actions"', + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied GET /repos/o/r/actions/permissions: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions; the "oidc_customization_sub" key alone instead needs "Actions" (read and write)', ); }); }); @@ -611,13 +641,16 @@ describe("denialPosture", () => { }); test("a reading section without a posture, or with two, is a BUG rather than a guess", () => { - expect(() => denialPosture(meta({ get: get(), put }))).toThrow(/declares no primaryRead/); - // A GraphQL read is a read the posture must cover too. - expect(() => denialPosture(meta({ put }, { probe: readOp }))).toThrow( - /declares no primaryRead/, + const unpostured = new Error( + "BUG: interaction_limits reads but declares no primaryRead posture, so a denied first read cannot be classified", ); + expect(() => denialPosture(meta({ get: get(), put }))).toThrow(unpostured); + // A GraphQL read is a read the posture must cover too. + expect(() => denialPosture(meta({ put }, { probe: readOp }))).toThrow(unpostured); expect(() => denialPosture(meta({ get: get("denied"), other: get("absent"), put }))).toThrow( - /primaryRead on 2 endpoints/, + new Error( + "BUG: interaction_limits declares primaryRead on 2 endpoints; at most one read carries the 404 posture", + ), ); }); }); @@ -723,7 +756,11 @@ describe("planContext read port", () => { }); // The control: the same status on a plain read classifies through throwFor. expect(typeof ctx.read.plain.call).toBe("function"); - await expect(ctx.read.plain.tryCall()).rejects.toThrow(/500/); + await expect(ctx.read.plain.tryCall()).rejects.toThrow( + new Error( + "branches: GET /repos/o/r/branches: 500 Internal Server Error. GitHub returned a server error; re-run the workflow, and retry later if it persists", + ), + ); }); test("advisory wins over a primaryRead posture on the same declaration", () => { @@ -825,48 +862,65 @@ describe("plainData", () => { expect(plainData([1, "two", null, { three: 3 }])).toEqual([1, "two", null, { three: 3 }]); }); - test.each<[what: string, value: unknown, path: string, reason: RegExp]>([ - ["a function", { rules: [{ check: () => true }] }, "rules[0].check", /a function/], - ["a bigint", { limit: 10n }, "limit", /a bigint/], - ["a class instance", { when: new Date(0) }, "when", /a non-plain object/], - ["a symbol", [Symbol("s")], "[0]", /a symbol/], - ["a non-finite number", { ratio: Number.NaN }, "ratio", /non-finite/], - ["an undefined list item", { list: [undefined] }, "list[0]", /undefined list item/], - ["undefined at the root", undefined, "(root)", /has no JSON form/], + const BUG = "BUG: a planned payload carries a value JSON cannot carry at "; + const PLAIN = "; request data must be plain"; + test.each<[what: string, value: unknown, message: string]>([ + ["a function", { rules: [{ check: () => true }] }, `${BUG}rules[0].check: a function${PLAIN}`], + ["a bigint", { limit: 10n }, `${BUG}limit: a bigint${PLAIN}`], + ["a class instance", { when: new Date(0) }, `${BUG}when: a non-plain object${PLAIN}`], + ["a symbol", [Symbol("s")], `${BUG}[0]: a symbol${PLAIN}`], + [ + "a non-finite number", + { ratio: Number.NaN }, + `${BUG}ratio: a non-finite number, which JSON would turn into null${PLAIN}`, + ], + [ + "an undefined list item", + { list: [undefined] }, + `${BUG}list[0]: an undefined list item, which JSON would turn into null${PLAIN}`, + ], + ["undefined at the root", undefined, `${BUG}(root): undefined, which has no JSON form${PLAIN}`], // Keys that are not bare identifiers render bracketed, so a dotted key // and a nested key cannot read the same. - ["a value under a dotted key", { "a.b": { c: 1n } }, '["a.b"].c', /a bigint/], - ["a value under an empty key", { "": 1n }, '[""]', /a bigint/], - ["a symbol-keyed property", { ok: true, [Symbol("hidden")]: 1n }, "(root)", /symbol-keyed/], + ["a value under a dotted key", { "a.b": { c: 1n } }, `${BUG}["a.b"].c: a bigint${PLAIN}`], + ["a value under an empty key", { "": 1n }, `${BUG}[""]: a bigint${PLAIN}`], + [ + "a symbol-keyed property", + { ok: true, [Symbol("hidden")]: 1n }, + `${BUG}(root): a symbol-keyed property, which JSON drops${PLAIN}`, + ], [ "a symbol-keyed list", { list: Object.assign([1], { [Symbol("hidden")]: 1n }) }, - "list", - /symbol-keyed/, + `${BUG}list: a symbol-keyed property, which JSON drops${PLAIN}`, ], [ "a list with named properties", { list: Object.assign([1], { extra: 2 }) }, - "list", - /named properties/, + `${BUG}list: a list carrying named properties, which JSON drops${PLAIN}`, ], [ "a list with a non-enumerable named property", { list: Object.defineProperty([1], "extra", { value: 2 }) }, - "list", - /named properties/, + `${BUG}list: a list carrying named properties, which JSON drops${PLAIN}`, + ], + [ + "a list of a subclass", + { list: new (class Tagged extends Array {})() }, + `${BUG}list: a list of a subclass, which JSON serializes as a plain list${PLAIN}`, ], - ["a list of a subclass", { list: new (class Tagged extends Array {})() }, "list", /a subclass/], [ "a list with a non-enumerable item", { list: Object.defineProperty([1], "0", { enumerable: false }) }, - "list", - /non-enumerable item/, + `${BUG}list: a list with a hole or a non-enumerable item, which JSON reads as null${PLAIN}`, ], - ["a list with a hole", { list: Object.assign(new Array(3), { 0: 1, 2: 3 }) }, "list", /a hole/], - ])("rejects %s, naming its path", (_what, value, path, reason) => { - expect(() => plainData(value)).toThrow(reason); - expect(() => plainData(value)).toThrow(`at ${path}:`); + [ + "a list with a hole", + { list: Object.assign(new Array(3), { 0: 1, 2: 3 }) }, + `${BUG}list: a list with a hole or a non-enumerable item, which JSON reads as null${PLAIN}`, + ], + ])("rejects %s, naming its path", (_what, value, message) => { + expect(() => plainData(value)).toThrow(new Error(message)); }); test("rejects a cycle, and only a cycle: a shared sibling reference is plain", () => { @@ -875,7 +929,9 @@ describe("plainData", () => { const cyclic: Record = { name: "loop" }; cyclic.self = cyclic; expect(() => plainData(cyclic)).toThrow( - /at self: a reference back to one of its own containers/, + new Error( + "BUG: a planned payload carries a value JSON cannot carry at self: a reference back to one of its own containers (a cycle); request data must be plain", + ), ); }); }); @@ -888,7 +944,7 @@ describe("hasDrift", () => { if (hasDrift(lines)) { // Under the guard the head is a string, not string | undefined. const [head] = lines; - expect(head.startsWith("labels[bug]")).toBe(true); + expect(head).toBe("labels[bug]: color d73a4a != live ffffff"); } }); }); @@ -909,8 +965,8 @@ describe("declaredTolerance", () => { // request could leave, naming the offending status. for (const status of [422, 200, 401, 500]) { expect(() => declaredTolerance(endpoint, [status])).toThrow( - new RegExp( - `BUG: GET .*branches/\\{branch\\} was asked to tolerate status\\(es\\) ${status}, which it does not declare`, + new Error( + `BUG: GET /repos/{owner}/{repo}/branches/{branch} was asked to tolerate status(es) ${status}, which it does not declare as a tolerable error status; a tolerance may only name declared 4xx statuses other than 401 and 429`, ), ); } @@ -980,7 +1036,11 @@ describe("tryCallDeclared", () => { endpoint, { tolerated, describe: "arming the setup" }, ), - ).rejects.toThrow(/arming the setup failed - PATCH .*: 422 Unprocessable/); + ).rejects.toThrow( + new Error( + 'actions: arming the setup failed - PATCH /repos/o/r/code-quality/setup: 422 Unprocessable. The API rejected the request; fix the "actions" values in the settings file to satisfy the message above', + ), + ); }); test("a rate limit is never a tolerated outcome, even under a tolerated 403", async () => { @@ -996,6 +1056,9 @@ describe("tryCallDeclared", () => { error: { status: 403, message: "API rate limit exceeded", body: "", rateLimited: true }, }, }); + const limitHit = new Error( + "actions: GET /repos/o/r/pages: 403 API rate limit exceeded. The API rate limit was hit; re-run the workflow after the limit resets, or use a token with a higher rate limit", + ); await expect( tryCallDeclared( { ...ctx, api: limited, resolveSecret: () => "" }, @@ -1005,10 +1068,10 @@ describe("tryCallDeclared", () => { tolerated: declaredTolerance(declares403), }, ), - ).rejects.toThrow(/rate limit was hit/); + ).rejects.toThrow(limitHit); await expect( probeAbsent({ ...ctx, api: limited, check: true }, actionsSection, declares403), - ).rejects.toThrow(/rate limit was hit/); + ).rejects.toThrow(limitHit); const plain = new MockApi({ "GET /repos/o/r/pages": { error: { status: 403, message: "Forbidden", body: "" } }, }); @@ -1035,7 +1098,11 @@ describe("tryCallDeclared", () => { probeAbsent({ ...ctx, api, check: true }, actionsSection, probe, { tolerate: [422 as unknown as 404], }), - ).rejects.toThrow(/BUG: GET .*pages was asked to tolerate status\(es\) 422/); + ).rejects.toThrow( + new Error( + "BUG: GET /repos/{owner}/{repo}/pages was asked to tolerate status(es) 422, which it does not declare as a tolerable error status; a tolerance may only name declared 4xx statuses other than 401 and 429", + ), + ); expect(api.calls).toEqual([]); }); }); @@ -1094,8 +1161,8 @@ describe("samePermission", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - expect((thrown as PermissionDenied).detail).toContain( - `To fix, ${sectionGrant(actionsSection)}`, + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied GET /repos/o/r/actions/permissions: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions; the "oidc_customization_sub" key alone instead needs "Actions" (read and write)', ); }); }); diff --git a/test/sections/docs-registry.test.ts b/test/sections/docs-registry.test.ts index fcd2ab28..b54f2f11 100644 --- a/test/sections/docs-registry.test.ts +++ b/test/sections/docs-registry.test.ts @@ -194,16 +194,23 @@ describe("section docs completeness", () => { "coverage: []", ].join("\n"), ); - // Zod reports the issues in its own order, so each is pinned on its own. + // The report body is zod's own prettified issue list (its wording, glyphs, and order), so + // only our header and each issue's path are pinned; the missing schema map is reported too. for (const issue of [ `${malformed} is not a valid docs document:`, 'Unrecognized key: "extra"', + "at sections_table", + "at schema", "at sections_table.notes", - "at coverage", + "at coverage[0]", ]) { expect(() => readDocsYaml(malformed, SectionDocs)).toThrow(new RegExp(escapeRe(issue))); } - expect(() => readDocsYaml(join(dir, "absent.yml"), SectionDocs)).toThrow(/absent\.yml/); + // The tail of a missing-file error is the runtime's ENOENT prose, so only our prefix is pinned. + const absent = join(dir, "absent.yml"); + expect(() => readDocsYaml(absent, SectionDocs)).toThrow( + new RegExp(`^${escapeRe(`${absent} is not valid YAML: `)}`), + ); // YAML that does not even parse (a duplicated key, which the loader refuses) names the file too. writeFileSync(malformed, ["sections_table:", " endpoints: a", " endpoints: b"].join("\n")); expect(() => readDocsYaml(malformed, SectionDocs)).toThrow( diff --git a/test/sections/graphql-contract.test.ts b/test/sections/graphql-contract.test.ts index 5b98f444..9d412393 100644 --- a/test/sections/graphql-contract.test.ts +++ b/test/sections/graphql-contract.test.ts @@ -14,12 +14,7 @@ import { graphqlOp, toleratedGraphqlErrors, } from "../../src/sections/contract/graphql.js"; -import { - type SectionContext, - type SectionMeta, - sectionGrant, -} from "../../src/sections/contract/module.js"; -import { grantFor } from "../../src/sections/contract/permissions.js"; +import type { SectionContext, SectionMeta } from "../../src/sections/contract/module.js"; import { callGraphql, listGraphqlConnection, @@ -89,9 +84,9 @@ describe("callGraphql", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - const denied = thrown as PermissionDenied; - expect(denied.detail).toContain("reading repository toggles failed - GRAPHQL RepoToggles: 403"); - expect(denied.detail).toContain(sectionGrant(section)); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied reading repository toggles failed - GRAPHQL RepoToggles: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + ); }); test("a 422 takes the generic rejection branch, naming the operation", async () => { @@ -100,7 +95,11 @@ describe("callGraphql", () => { }); await expect( callGraphql(ctx(api), section, READ_OP, { owner: "o", repo: "r" }), - ).rejects.toThrow(/repository: GRAPHQL RepoToggles: 422 bad value\. The API rejected/); + ).rejects.toThrow( + new Error( + 'repository: GRAPHQL RepoToggles: 422 bad value. The API rejected the request; fix the "repository" values in the settings file to satisfy the message above', + ), + ); }); test("an op-level permission override renders its own grant at the graded level", async () => { @@ -135,7 +134,9 @@ describe("callGraphql", () => { thrown = error; } expect(thrown).toBeInstanceOf(PermissionDenied); - expect((thrown as PermissionDenied).detail).toContain(grantFor({ repo: ["contents"] })); + expect((thrown as PermissionDenied).detail).toBe( + 'the token was denied GRAPHQL RepoToggles: 404 Not Found (a 404 here can also mean the resource does not exist). To fix, grant "Contents" (read and write) under the PAT\'s Repository permissions', + ); expect(overrideAdviceLevel(sectionWithWrite, { repo: ["contents"] })).toBe("write"); }); }); @@ -156,7 +157,9 @@ describe("tryCallGraphql tolerance", () => { }, }); const result = await tryCallGraphql(ctx(api), section, tolerantOp, { owner: "o", repo: "r" }); - expect("error" in result && result.error.status).toBe(404); + expect(result).toEqual({ + error: { status: 404, message: "Not Found", body: "", graphqlTypes: ["NOT_FOUND"] }, + }); }); test("an undeclared observed type still classifies through throwFor", async () => { @@ -224,7 +227,7 @@ describe("tryCallGraphql tolerance", () => { { tolerate: ["UNPROCESSABLE"] }, ); void smuggle; - expect(api.calls).toHaveLength(0); + expect(api.calls).toEqual([]); }); test("a RATE_LIMITED response always classifies as a rate limit", async () => { @@ -244,7 +247,11 @@ describe("tryCallGraphql tolerance", () => { }); await expect( tryCallGraphql(ctx(api), section, tolerantOp, { owner: "o", repo: "r" }), - ).rejects.toThrow(/rate limit was hit/); + ).rejects.toThrow( + new Error( + "repository: GRAPHQL RepoToggles: 403 slow down. The API rate limit was hit; re-run the workflow after the limit resets, or use a token with a higher rate limit", + ), + ); }); test("an INSUFFICIENT_SCOPES response always classifies as a denial", async () => { @@ -378,7 +385,9 @@ describe("listGraphqlConnection", () => { owner: "o", repo: "r", }); - expect("error" in listed && listed.error.status).toBe(404); + expect(listed).toEqual({ + error: { status: 404, message: "Not Found", body: "", graphqlTypes: ["NOT_FOUND"] }, + }); }); test("a tolerated type arriving MID-walk still classifies as an error", async () => { @@ -435,14 +444,22 @@ describe("listGraphqlConnection", () => { const api = pagedApi([{ repository: { rules: { nodes: "not-a-list" } } }]); await expect( listGraphqlConnection(ctx(api), section, pagedOp, { owner: "o", repo: "r" }), - ).rejects.toThrow(/without a "repository.rules" connection/); + ).rejects.toThrow( + new Error( + 'repository: GRAPHQL RepoRules returned a response without a "repository.rules" connection carrying nodes and pageInfo{hasNextPage, endCursor}, so the list cannot be paginated. The operation\'s query must select both under that path', + ), + ); }); test("hasNextPage without a fresh endCursor fails instead of looping", async () => { const api = pagedApi([page(["a"], null, true)]); await expect( listGraphqlConnection(ctx(api), section, pagedOp, { owner: "o", repo: "r" }), - ).rejects.toThrow(/hasNextPage without a new endCursor/); + ).rejects.toThrow( + new Error( + 'repository: GRAPHQL RepoRules reported hasNextPage without a new endCursor at "repository.rules", so the pagination cannot advance. The operation\'s query must select pageInfo{hasNextPage, endCursor}', + ), + ); }); test("errors inside the loop classify through throwFor", async () => { diff --git a/test/sections/list-section.test.ts b/test/sections/list-section.test.ts index 00cdcbe8..7a6cd063 100644 --- a/test/sections/list-section.test.ts +++ b/test/sections/list-section.test.ts @@ -91,7 +91,11 @@ describe("listSection", () => { }); await expect( labelsSection.plan(planContext(labelsSection, api, REPO), [{ name: "bug" }]), - ).rejects.toThrow(/"bug" matches 2 separate live labels \("bug", "BUG"\)/); + ).rejects.toThrow( + new Error( + 'labels: the entry "bug" matches 2 separate live labels ("bug", "BUG"), so it cannot converge; delete all but one of them on GitHub, or declare each as its own entry', + ), + ); // Unclaimed, both are undeclared and both are removed. const unclaimed = await labelsSection.plan(planContext(labelsSection, api, REPO), []); expect(unclaimed.ops.map((op) => [op.role, op.params?.name])).toEqual([ @@ -170,7 +174,11 @@ describe("listSection", () => { { name: "a", new_name: "b" }, { name: "B" }, ]), - ).rejects.toThrow(/name the same labels entry: "b" and "B"/); + ).rejects.toThrow( + new Error( + 'labels: the settings file declares entries that name the same labels entry: "b" and "B". Keep exactly one entry per resource', + ), + ); expect(api.calls).toEqual([]); }); @@ -287,10 +295,15 @@ describe("listSection without an update role", () => { test("the derived mock fragment serves no update handler, and the record type has no such key", () => { const fragment = mockFragmentFor(immutable, LABELS_MOCK); - expect(Object.keys(fragment)).toEqual(["labels.list", "labels.create", "labels.remove"]); + expect(Object.keys(fragment).sort()).toEqual(["labels.create", "labels.list", "labels.remove"]); // @ts-expect-error no update role, no update handler key fragment["labels.update"]; - expect(Object.keys(mockFragmentFor(labelsSection, LABELS_MOCK))).toContain("labels.update"); + expect(Object.keys(mockFragmentFor(labelsSection, LABELS_MOCK)).sort()).toEqual([ + "labels.create", + "labels.list", + "labels.remove", + "labels.update", + ]); }); test("the derived create rejects what the spec declares unique: the folded identity, or the declared key", async () => { diff --git a/test/sections/loosen.test.ts b/test/sections/loosen.test.ts index 71bb40a7..ac950f90 100644 --- a/test/sections/loosen.test.ts +++ b/test/sections/loosen.test.ts @@ -37,10 +37,18 @@ describe("loosen", () => { }); test("array elements and record values are loosened (the def-surgery tripwire)", () => { + // The parsed DATA carries the unknown key: a strip object would parse the + // same input successfully while dropping it, so success alone proves nothing. const viaArray = loosen(z.array(z.object({ name: z.string() }))); - expect(viaArray.safeParse([{ name: "a", extra: 1 }]).success).toBe(true); + expect(viaArray.safeParse([{ name: "a", extra: 1 }])).toEqual({ + success: true, + data: [{ name: "a", extra: 1 }], + }); const viaRecord = loosen(z.record(z.string(), z.object({ name: z.string() }))); - expect(viaRecord.safeParse({ key: { name: "a", extra: 1 } }).success).toBe(true); + expect(viaRecord.safeParse({ key: { name: "a", extra: 1 } })).toEqual({ + success: true, + data: { key: { name: "a", extra: 1 } }, + }); }); test("the knobbed union is rewrapped with per-container issue paths", () => { @@ -69,10 +77,18 @@ describe("loosen", () => { z.strictObject({ entries: z.array(z.object({ name: z.string() })) }), ]) .superRefine(() => {}); - expect(() => loosen(knob)).toThrow(/routed rewrap would silently drop/); + expect(() => loosen(knob)).toThrow( + new Error( + "loosen(): a knobbed-section union carries its own refinements, which the routed rewrap would silently drop - attach them to the entry array or the wrapper", + ), + ); }); test("an unrecognized container type fails loudly instead of skipping the derivation", () => { - expect(() => loosen(z.tuple([z.string()]))).toThrow(/unhandled schema type "tuple"/); + expect(() => loosen(z.tuple([z.string()]))).toThrow( + new Error( + 'loosen(): unhandled schema type "tuple" - teach loosen() its runtime derivation before authoring it in src/schema.ts', + ), + ); }); }); diff --git a/test/sections/plan-idempotence.test.ts b/test/sections/plan-idempotence.test.ts index e0f3ff70..bfd6bc46 100644 --- a/test/sections/plan-idempotence.test.ts +++ b/test/sections/plan-idempotence.test.ts @@ -146,8 +146,15 @@ describe("provePlanIdempotent", () => { DESIRED, TOOLS, ); - expect(first.ops).toHaveLength(1); - expect(second.ops).toHaveLength(1); + // The same write on both passes: the list already holds the secret, so the + // change line reads "updated" each time. + const facets = (ops: typeof first.ops) => + ops.map((op) => [op.role, op.params, op.drift, op.change]); + const recurring = [ + ["put", { secret_name: "DEPLOY_TOKEN" }, [], 'updated secret "DEPLOY_TOKEN"'], + ]; + expect(facets(first.ops)).toEqual(recurring); + expect(facets(second.ops)).toEqual(recurring); // Each pass built its own thunk; the proof passes because it compares // operation identity, not function references. expect(typeof first.ops[0]?.payload).toBe("function"); diff --git a/test/sections/registry.test.ts b/test/sections/registry.test.ts index c4013ac9..2b5081dd 100644 --- a/test/sections/registry.test.ts +++ b/test/sections/registry.test.ts @@ -46,8 +46,8 @@ interface ListDeclView { }; } -// The caveat code-scanning appends to its derived grant. Kept here so the -// snapshot below and the derivation check agree on one source of truth. +// The caveat code-scanning appends to its derived grant, spelled once for +// the snapshot below. const CODE_SCANNING_CAVEAT = "a 403 on this endpoint can also mean GitHub Advanced Security (code security) is not enabled on the repository, or the repository is archived"; @@ -76,16 +76,6 @@ const ACTIONS_OIDC_CAVEAT = const ENVIRONMENTS_POLICIES_CAVEAT = 'declared "deployment_branch_policies" and "deployment_protection_rules" keys additionally need "Actions" (read) and "Administration" (read and write)'; -// The per-section caveats grantFor appends; the derivation test and the -// literal snapshot both read this one map. -const GRANT_CAVEATS: Record = { - code_scanning_default_setup: CODE_SCANNING_CAVEAT, - code_quality_setup: CODE_QUALITY_CAVEAT, - check_suite_preferences: CHECK_SUITE_PREFERENCES_CAVEAT, - actions: ACTIONS_OIDC_CAVEAT, - environments: ENVIRONMENTS_POLICIES_CAVEAT, -}; - // The exact grant prose each section shows in permission errors, captured // against the pre-refactor literals. grantFor derives these now, so any // character-level change is a conscious edit here - not a silent drift. @@ -119,14 +109,14 @@ const EXPECTED_GRANT: Record = { }; describe("section permissions", () => { - test("every knobbed section's shape parses both forms and yields a default policy", () => { + test("every knobbed section's shape parses both forms, and the default policies are these", () => { // The knob invariant is mostly compile-time: UNDECLARED_POLICY_SECTIONS // is pinned to the SettingsFile types in both directions (schema.ts), // and SectionMeta's conditional undeclaredDefault type forces "delete" // or "keep" exactly for listed sections. The zod shapes are the one // runtime-only piece: a listed section must accept both the plain array // and the wrapped form. Round-tripping both forms here pins the shapes - // to the same list. + // to the same list; the policy map pins which side each section defaults to. const byKey = new Map(SECTIONS.map((module) => [module.key as string, module])); for (const key of UNDECLARED_POLICY_SECTIONS) { const module = byKey.get(key); @@ -142,12 +132,28 @@ describe("section permissions", () => { module.shape.safeParse({ _undeclared: "keep", entries: [] }).success, `${key}: wrapper with a policy must parse`, ).toBe(true); - const policy = defaultUndeclaredPolicy(sectionModule(key)); - expect( - ["keep", "delete"], - `${key}: defaultUndeclaredPolicy returned "${policy}", expected "keep" or "delete"`, - ).toContain(policy); } + expect( + Object.fromEntries( + UNDECLARED_POLICY_SECTIONS.map((key) => [key, defaultUndeclaredPolicy(sectionModule(key))]), + ), + ).toEqual({ + labels: "delete", + rulesets: "keep", + autolinks: "delete", + actions_secrets: "keep", + dependabot_secrets: "keep", + codespaces_secrets: "keep", + agents_secrets: "keep", + collaborators: "delete", + milestones: "keep", + actions_variables: "delete", + agents_variables: "delete", + webhooks: "keep", + custom_properties: "keep", + deploy_keys: "keep", + secret_scanning_custom_patterns: "keep", + }); }); test("_layering is accepted on every top-level knobbed wrapper and rejected on the nested ones", () => { @@ -248,12 +254,6 @@ describe("section permissions", () => { ).toEqual([]); }); - test("each section's grant caveat matches the pinned per-section caveats", () => { - for (const module of SECTIONS) { - expect(sectionGrant(module)).toBe(grantFor(module.permission, GRANT_CAVEATS[module.key])); - } - }); - test("each section's grant equals its exact pre-refactor literal", () => { // A section without an expected literal (a new one) fails here. expect(Object.keys(EXPECTED_GRANT).sort()).toEqual([...SECTION_KEYS].sort()); @@ -528,21 +528,155 @@ describe("section endpoints", () => { describe("allEndpoints", () => { test("flattens every section endpoint under a unique section.role key", () => { const all = allEndpoints(); - const keys = Object.keys(all); - // At least one entry per section, and 55+ overall. - expect(keys.length).toBeGreaterThanOrEqual(55); - // Every key is ${sectionKey}.${role}; keys are unique by construction. - for (const key of keys) { - expect(key).toMatch(/^[a-z_]+\.[a-zA-Z]+$/); - } - expect(new Set(keys).size).toBe(keys.length); - // Each entry is tagged with its owning section and role, and the counts - // reconcile with the per-section dictionaries. - let total = 0; - for (const module of SECTIONS) { - total += Object.keys(module.endpoints).length; - } - expect(keys.length).toBe(total); + // The whole key set: a dropped, duplicated, or renamed declaration fails here. + expect(Object.keys(all).sort()).toEqual([ + "actions.getAccess", + "actions.getCacheRetention", + "actions.getCacheStorage", + "actions.getForkPrApproval", + "actions.getForkPrPrivate", + "actions.getOidcSub", + "actions.getPermissions", + "actions.getRetention", + "actions.getSelected", + "actions.getWorkflow", + "actions.putAccess", + "actions.putCacheRetention", + "actions.putCacheStorage", + "actions.putForkPrApproval", + "actions.putForkPrPrivate", + "actions.putOidcSub", + "actions.putPermissions", + "actions.putRetention", + "actions.putSelected", + "actions.putWorkflow", + "actions_secrets.list", + "actions_secrets.publicKey", + "actions_secrets.put", + "actions_secrets.remove", + "actions_variables.create", + "actions_variables.list", + "actions_variables.remove", + "actions_variables.update", + "agents_secrets.list", + "agents_secrets.publicKey", + "agents_secrets.put", + "agents_secrets.remove", + "agents_variables.create", + "agents_variables.list", + "agents_variables.remove", + "agents_variables.update", + "autolinks.create", + "autolinks.list", + "autolinks.remove", + "branches.appLookup", + "branches.branchProbe", + "branches.getProtection", + "branches.putProtection", + "branches.removeProtection", + "branches.sigDelete", + "branches.sigPost", + "check_suite_preferences.update", + "code_quality_setup.get", + "code_quality_setup.update", + "code_scanning_default_setup.get", + "code_scanning_default_setup.update", + "codespaces_secrets.list", + "codespaces_secrets.publicKey", + "codespaces_secrets.put", + "codespaces_secrets.remove", + "collaborators.cancelInvitation", + "collaborators.list", + "collaborators.listInvitations", + "collaborators.remove", + "collaborators.update", + "collaborators.updateInvitation", + "custom_properties.list", + "custom_properties.org", + "custom_properties.update", + "dependabot_secrets.list", + "dependabot_secrets.publicKey", + "dependabot_secrets.put", + "dependabot_secrets.remove", + "deploy_keys.create", + "deploy_keys.list", + "deploy_keys.remove", + "environments.createPolicy", + "environments.createProtectionRule", + "environments.createVariable", + "environments.listPolicies", + "environments.listProtectionRuleApps", + "environments.listProtectionRules", + "environments.listSecrets", + "environments.listVariables", + "environments.probe", + "environments.putSecret", + "environments.removePolicy", + "environments.removeProtectionRule", + "environments.removeSecret", + "environments.removeVariable", + "environments.secretsPublicKey", + "environments.update", + "environments.updateVariable", + "interaction_limits.bypassAdd", + "interaction_limits.bypassList", + "interaction_limits.bypassRemove", + "interaction_limits.capGet", + "interaction_limits.capPatch", + "interaction_limits.get", + "interaction_limits.put", + "interaction_limits.remove", + "labels.create", + "labels.list", + "labels.remove", + "labels.update", + "milestones.create", + "milestones.list", + "milestones.remove", + "milestones.update", + "pages.create", + "pages.get", + "pages.remove", + "pages.update", + "repository.automatedSecurityFixesGet", + "repository.automatedSecurityFixesPut", + "repository.automatedSecurityFixesRemove", + "repository.get", + "repository.immutableReleasesGet", + "repository.immutableReleasesPut", + "repository.immutableReleasesRemove", + "repository.lfsPut", + "repository.lfsRemove", + "repository.privateVulnerabilityReportingGet", + "repository.privateVulnerabilityReportingPut", + "repository.privateVulnerabilityReportingRemove", + "repository.topics", + "repository.update", + "repository.vulnerabilityAlertsGet", + "repository.vulnerabilityAlertsPut", + "repository.vulnerabilityAlertsRemove", + "rulesets.create", + "rulesets.get", + "rulesets.list", + "rulesets.remove", + "rulesets.update", + "secret_scanning_custom_patterns.create", + "secret_scanning_custom_patterns.list", + "secret_scanning_custom_patterns.remove", + "secret_scanning_custom_patterns.update", + "teams.grant", + "teams.org", + "teams.probe", + "webhooks.create", + "webhooks.list", + "webhooks.remove", + "webhooks.update", + "webhooks.updateConfig", + "workflows.disable", + "workflows.enable", + "workflows.list", + ]); + // Each entry is tagged with its owning section and role. for (const [key, endpoint] of Object.entries(all)) { expect(key).toBe(`${endpoint.section}.${endpoint.role}`); } @@ -551,7 +685,12 @@ describe("allEndpoints", () => { test("the returned view is frozen so a consumer cannot corrupt declarations", () => { const all = allEndpoints(); const entry = all["labels.update"]; - expect(entry).toBeDefined(); + expect(entry).toEqual({ + route: "PATCH /repos/{owner}/{repo}/labels/{name}", + statuses: { 200: "label updated" }, + section: "labels", + role: "update", + }); expect(Object.isFrozen(all)).toBe(true); expect(Object.isFrozen(entry)).toBe(true); expect(Object.isFrozen(entry?.statuses)).toBe(true); @@ -588,10 +727,17 @@ describe("allGraphqlOps", () => { test("flattens, tags, and freezes like allEndpoints", () => { const ops = allGraphqlOps([graphqlSection("repository", { toggles: op("RepoToggles") })]); + expect(ops).toEqual({ + "repository.toggles": { + name: "RepoToggles", + kind: "read", + query: "query RepoToggles { viewer { login } }", + outcomes: { ok: "x" }, + section: "repository", + role: "toggles", + }, + }); const tagged = ops["repository.toggles"]; - expect(tagged).toBeDefined(); - expect(tagged?.section).toBe("repository"); - expect(tagged?.role).toBe("toggles"); expect(Object.isFrozen(ops)).toBe(true); expect(Object.isFrozen(tagged)).toBe(true); expect(Object.isFrozen(tagged?.outcomes)).toBe(true); @@ -604,7 +750,9 @@ describe("allGraphqlOps", () => { graphqlSection("branches", { rules: op("RepoToggles") }), ]), ).toThrow( - /operation name "RepoToggles" is declared by both repository\.toggles and branches\.rules/, + new Error( + 'BUG: GraphQL operation name "RepoToggles" is declared by both repository.toggles and branches.rules; operation names are the wire dispatch key and must be globally unique', + ), ); }); @@ -617,7 +765,11 @@ describe("allGraphqlOps", () => { { get: { route: "GET /repos/{owner}/{repo}", statuses: { 200: "x" } } }, ), ]), - ).toThrow(/declares both a REST endpoint and a GraphQL operation under the role "get"/); + ).toThrow( + new Error( + 'BUG: section "repository" declares both a REST endpoint and a GraphQL operation under the role "get"; fault and corruption directives share the "section.role" key space, so roles must be distinct', + ), + ); }); test("a declared connection whose query takes no $cursor does not compile", () => { @@ -868,17 +1020,20 @@ describe("the section.role key space reserves ':'", () => { test("a role containing ':' fails allEndpoints at construction", () => { expect(() => allEndpoints([endpointSection("labels", { "ring:list": LIST })])).toThrow( - /role "ring:list" contains ":".*reserves.*scope prefix/, + new Error( + 'BUG: role "ring:list" contains ":", which the "section.role" key space reserves for a future scope prefix (":
."); rename it without a colon', + ), ); }); test("a section key containing ':' fails both flatteners", () => { - expect(() => allEndpoints([endpointSection("prod:labels", { list: LIST })])).toThrow( - /section key "prod:labels" contains ":"/, + const colonKey = new Error( + 'BUG: section key "prod:labels" contains ":", which the "section.role" key space reserves for a future scope prefix (":
."); rename it without a colon', ); + expect(() => allEndpoints([endpointSection("prod:labels", { list: LIST })])).toThrow(colonKey); expect(() => allGraphqlOps([{ key: "prod:labels" as (typeof SECTION_KEYS)[number], endpoints: {} }]), - ).toThrow(/section key "prod:labels" contains ":"/); + ).toThrow(colonKey); }); test("the live registry passes every construction assert of both flatteners", () => { @@ -927,7 +1082,7 @@ describe("handler contracts", () => { // The registry's _PlanModulesAreExact pin is only as good as its check: the shipped // workflows module measures exact, and the same module with plan() taking labels' // declared value measures misdeclared, so a passing pin means every module was compared. - type Exact = MustBeNever>; + type _Exact = MustBeNever>; const misdeclared = { ...workflowsSection, async plan( @@ -938,9 +1093,7 @@ describe("handler contracts", () => { }, }; // @ts-expect-error a plan() over labels' value is not exact for workflows - type Wrong = MustBeNever>; - const witness: [Exact, Wrong] | undefined = undefined; - expect(witness).toBeUndefined(); + type _Wrong = MustBeNever>; }); test("every reading section declares exactly one primaryRead, and its 404 posture derives from it", () => { diff --git a/test/sections/secrets-engine.test.ts b/test/sections/secrets-engine.test.ts index a5fa0bc3..c8767346 100644 --- a/test/sections/secrets-engine.test.ts +++ b/test/sections/secrets-engine.test.ts @@ -93,31 +93,43 @@ describe("sealing", () => { expect(unsealSecretValue(sealed)).toBe(hostile); }); + const WHERE = + "actions_secrets: GET /repos/{owner}/{repo}/actions/secrets/public-key (the actions_secrets sealing key) returned "; + const ADVICE = + ', so no value can be sealed. Check the "api-version" input against the GitHub REST docs for this endpoint'; test.each([ - ["a missing key_id", { key: MOCK_SECRETS_PUBLIC_KEY }, /pair \(key_id is missing\)/], - ["a key that is not base64", { key_id: "k", key: "not base64!" }, /is not valid base64/], + [ + "a missing key_id", + { key: MOCK_SECRETS_PUBLIC_KEY }, + `${WHERE}no usable {key_id, key} pair (key_id is missing)${ADVICE}`, + ], + [ + "a key that is not base64", + { key_id: "k", key: "not base64!" }, + `${WHERE}a key that is not valid base64${ADVICE}`, + ], [ "a key of the wrong length", { key_id: "k", key: Buffer.from("short").toString("base64") }, - /decodes to 5 bytes where an X25519 public key has 32/, + `${WHERE}a key that decodes to 5 bytes where an X25519 public key has 32${ADVICE}`, ], [ "a right-sized key that is not a usable point", { key_id: "k", key: Buffer.alloc(32).toString("base64") }, - /is not a usable X25519 public key/, + `${WHERE}a key that is not a usable X25519 public key${ADVICE}`, ], - ])("parseSealingKey rejects %s, naming the scope and the defect", async (_what, body, defect) => { - const attempt = parseSealingKey( - section, - { label: "actions_secrets" }, - PUBLIC_KEY_ENDPOINT, - body, - ); - await expect(attempt).rejects.toThrow( - /^actions_secrets: GET \/repos\/\{owner\}\/\{repo\}\/actions\/secrets\/public-key \(the actions_secrets sealing key\) returned /, - ); - await expect(attempt).rejects.toThrow(defect); - }); + ])( + "parseSealingKey rejects %s, naming the scope and the defect", + async (_what, body, message) => { + const attempt = parseSealingKey( + section, + { label: "actions_secrets" }, + PUBLIC_KEY_ENDPOINT, + body, + ); + await expect(attempt).rejects.toThrow(new Error(message)); + }, + ); test("a parsed sealing key seals synchronously into the {encrypted_value, key_id} body, fresh per seal", async () => { // Sealed boxes use a fresh ephemeral key per seal, so the ciphertexts @@ -150,7 +162,11 @@ describe("secretKey and duplicates", () => { { name: "Deploy_Token", value: "$A" }, { name: "DEPLOY_TOKEN", value: "$B" }, ]), - ).toThrow(/same actions_secrets entry.*"Deploy_Token" and "DEPLOY_TOKEN"/s); + ).toThrow( + new Error( + 'actions_secrets: the settings file declares entries that name the same actions_secrets entry: "Deploy_Token" and "DEPLOY_TOKEN". Keep exactly one entry per resource', + ), + ); }); }); @@ -252,7 +268,7 @@ describe("planSecrets and the execution-time resolver", () => { expect(typeof payload).toBe("function"); if (typeof payload === "function") { expect(() => payload({ resolveSecret: resolver({}) })).toThrow( - /no value for \$NEVER_RESOLVED/, + new Error("test resolver has no value for $NEVER_RESOLVED"), ); } }); diff --git a/test/sections/setup-section.test.ts b/test/sections/setup-section.test.ts index cafe4907..d9085814 100644 --- a/test/sections/setup-section.test.ts +++ b/test/sections/setup-section.test.ts @@ -34,7 +34,9 @@ interface SetupFacts { /** A declared document for the verbatim-PATCH case. */ applyPayload: Declared; changeLine: string; - denied403: RegExp; + /** The whole failure messages a 409 and a 403 on the PATCH produce. */ + conflict409: string; + denied403: string; } /** Exhaustive by type: a setup key without a row fails to compile. */ @@ -51,7 +53,10 @@ const SETUP_FACTS: { readonly [K in SetupKey]: SetupFacts } = { driftLine: 'code_scanning_default_setup.query_suite: "extended" != "default"', applyPayload: { state: "configured", query_suite: "extended" }, changeLine: "applied code scanning default setup", - denied403: /Advanced Security/, + conflict409: + "code_scanning_default_setup: PATCH /repos/o/r/code-scanning/default-setup: 409 Conflict. A code scanning default setup configuration run is already in progress on the repository; re-run the workflow after it finishes", + denied403: + 'code_scanning_default_setup: the token was denied PATCH /repos/o/r/code-scanning/default-setup: 403 Forbidden. To fix, grant "Administration" or "Code scanning alerts" (read and write) under the PAT\'s Repository permissions; a 403 on this endpoint can also mean GitHub Advanced Security (code security) is not enabled on the repository, or the repository is archived', }, code_quality_setup: { section: codeQualitySetupSection, @@ -66,7 +71,10 @@ const SETUP_FACTS: { readonly [K in SetupKey]: SetupFacts } = { 'code_quality_setup.ai_findings_option: declared "on_push" but the API response has no such field (new or write-only field?)', applyPayload: { state: "configured", ai_findings_option: "disabled" }, changeLine: "applied code quality setup", - denied403: /code quality is unavailable/, + conflict409: + "code_quality_setup: PATCH /repos/o/r/code-quality/setup: 409 Conflict. A code quality setup configuration run is already in progress on the repository; re-run the workflow after it finishes", + denied403: + 'code_quality_setup: the token was denied PATCH /repos/o/r/code-quality/setup: 403 Forbidden. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions; a 403 on this endpoint can also mean code quality is unavailable on the repository, or the repository is archived', }, }; @@ -135,7 +143,16 @@ describe.each(Object.values(SETUP_FACTS).map((facts) => [facts.section.key, fact (_key, facts) => { // The erased view: one plan() signature over either section's declared value. const section: SectionModule = facts.section; - const { path, live, driftDeclared, driftLine, applyPayload, changeLine, denied403 } = facts; + const { + path, + live, + driftDeclared, + driftLine, + applyPayload, + changeLine, + conflict409, + denied403, + } = facts; const plan = (api: GithubClient, declared: Declared) => section.plan(planContext(section, api, REPO), declared); @@ -144,10 +161,9 @@ describe.each(Object.values(SETUP_FACTS).map((facts) => [facts.section.key, fact // Planning reads through the GET port alone, and never writes. expect(Object.keys(planContext(section, api, REPO).read)).toEqual(["get"]); const drifted = await plan(api, driftDeclared); - expect(drifted.ops).toHaveLength(1); - expect(drifted.ops[0]?.role).toBe("update"); - expect(drifted.ops[0]?.payload as unknown).toEqual(driftDeclared); - expect(drifted.ops[0]?.drift).toEqual([driftLine]); + expect(drifted.ops.map((op) => [op.role, op.payload, op.drift])).toEqual([ + ["update", driftDeclared, [driftLine]], + ]); expect(drifted.notes).toEqual([]); expect(drifted.drift).toEqual([]); const reordered = await plan(api, { languages: [...live.languages].reverse() }); @@ -181,12 +197,7 @@ describe.each(Object.values(SETUP_FACTS).map((facts) => [facts.section.key, fact }); test.each([ - [ - "409", - 409, - "Conflict", - new RegExp(`${section.key}: PATCH ${path}: 409 Conflict\\. .*already in progress`), - ], + ["409", 409, "Conflict", conflict409], ["403", 403, "Forbidden", denied403], ])( "a %s on the PATCH fails with the section's own advice", @@ -205,7 +216,7 @@ describe.each(Object.values(SETUP_FACTS).map((facts) => [facts.section.key, fact tools, ); expect(execution.status).toBe("failed"); - expect(String((execution as { error: unknown }).error)).toMatch(advice); + expect((execution as { error: Error }).error.message).toBe(advice); }, ); },