From b1c8114977297ee4ff56fcefe6d03c7f87f1feb2 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:25:21 -0500 Subject: [PATCH 1/2] test(e2e): check GraphQL variables against GitHub's schema in the mock The e2e mock answered a GraphQL request by operation name and handed its variables to the handler unread, so a mutation carrying an input field GitHub's schema lacks, or a null for a `Boolean!` variable, passed every scenario. The mock now checks each request the way GitHub does before it runs any of it: graphql-js validates the document against the schema and coerces the variables to the types the operation declares. A request that fails either is answered with HTTP 200, `errors`, and no `data`. No handler runs, and the pipeline records a mock violation, so the scenario fails whatever outcome it expected. The check sits ahead of the check-mode, fault, and permission barriers. A request GitHub accepts reaches its handler with the variables exactly as sent. The schema is the SDL of @octokit/graphql-schema plus the upstream-gap SDL, built once per process. Its builder moves from test/sections/graphql-queries.test.ts to test/e2e/mock/graphql-schema.ts, so the declared-query test and the mock read one schema and one gap tripwire. The section unit fake runs the same check. The fixture operations of graphql-pipeline.test.ts were off GitHub's schema (`hasWiki` is not an `UpdateRepositoryInput` field, and two reads carried another operation's document), so they are now valid documents. --- test/e2e/mock/graphql-pipeline.test.ts | 105 ++++++++++++++++++++++--- test/e2e/mock/graphql-request.test.ts | 98 +++++++++++++++++++++++ test/e2e/mock/graphql-request.ts | 57 ++++++++++++++ test/e2e/mock/graphql-schema.ts | 44 +++++++++++ test/e2e/mock/routes.ts | 38 ++++++++- test/sections/fragment-fake.ts | 30 +++++-- test/sections/graphql-queries.test.ts | 42 +--------- 7 files changed, 353 insertions(+), 61 deletions(-) create mode 100644 test/e2e/mock/graphql-request.test.ts create mode 100644 test/e2e/mock/graphql-request.ts create mode 100644 test/e2e/mock/graphql-schema.ts diff --git a/test/e2e/mock/graphql-pipeline.test.ts b/test/e2e/mock/graphql-pipeline.test.ts index e3a8da03..f87b0957 100644 --- a/test/e2e/mock/graphql-pipeline.test.ts +++ b/test/e2e/mock/graphql-pipeline.test.ts @@ -32,12 +32,16 @@ function scenario(overrides: Partial = {}): Scenario { } // Fixture operations, tagged onto the real repository section so the permission gate resolves its real requirement (administration). +// Each is a document GitHub's schema accepts, named as its declaration names it: the pipeline refuses any other before a barrier runs. + +function repoIdQuery(name: string) { + return `query ${name}($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { id } }` as const; +} const G_READ: TaggedGraphqlOp = { name: "RepoToggles", kind: "read", - query: - "query RepoToggles($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { id } }", + query: repoIdQuery("RepoToggles"), outcomes: { ok: "the toggles" }, section: "repository", role: "gToggles", @@ -45,12 +49,14 @@ const G_READ: TaggedGraphqlOp = { const G_READ_TOLERANT: TaggedGraphqlOp = { ...G_READ, name: "RepoTogglesProbe", + query: repoIdQuery("RepoTogglesProbe"), role: "gProbe", outcomes: { ok: "the toggles", NOT_FOUND: "the feature is off" }, }; const G_READ_EXECUTION: TaggedGraphqlOp = { ...G_READ, name: "RepoNodeId", + query: repoIdQuery("RepoNodeId"), role: "gNodeId", phase: "execution", }; @@ -58,17 +64,25 @@ const G_WRITE: TaggedGraphqlOp = { name: "UpdateToggles", kind: "write", query: - "mutation UpdateToggles($repositoryId: ID!, $hasWiki: Boolean!) { updateRepository(input: {repositoryId: $repositoryId, hasWiki: $hasWiki}) { clientMutationId } }", + "mutation UpdateToggles($repositoryId: ID!, $hasWikiEnabled: Boolean) { updateRepository(input: {repositoryId: $repositoryId, hasWikiEnabled: $hasWikiEnabled}) { clientMutationId } }", outcomes: { ok: "toggles updated" }, section: "repository", role: "gUpdate", }; +const G_WRITE_NESTED: TaggedGraphqlOp = { + ...G_WRITE, + name: "UpdateTogglesInput", + query: + "mutation UpdateTogglesInput($input: UpdateRepositoryInput!) { updateRepository(input: $input) { clientMutationId } }", + role: "gUpdateInput", +}; const OPS: Readonly> = { "repository.gToggles": G_READ, "repository.gProbe": G_READ_TOLERANT, "repository.gNodeId": G_READ_EXECUTION, "repository.gUpdate": G_WRITE, + "repository.gUpdateInput": G_WRITE_NESTED, }; const HANDLERS: Record = { @@ -80,7 +94,11 @@ const HANDLERS: Record = { data: { repository: { id: String(state.repo.node_id) } }, }), "repository.gUpdate": ({ state, variables }) => { - state.repo.has_wiki = variables.hasWiki === true; + state.repo.has_wiki = variables.hasWikiEnabled === true; + return { data: { updateRepository: { clientMutationId: null } } }; + }, + "repository.gUpdateInput": ({ state, variables }) => { + state.repo.has_wiki = (variables.input as Json).hasWikiEnabled === true; return { data: { updateRepository: { clientMutationId: null } } }; }, }; @@ -171,7 +189,7 @@ describe("GraphQL dispatch and logging", () => { const opts = options(scenario()); const state = opts.working.mode === "single" ? opts.working.state : undefined; const id = mintNodeId("repo", ADMIN_SLUG, ""); - const result = dispatch(G_WRITE, { repositoryId: id, hasWiki: true }, opts); + const result = dispatch(G_WRITE, { repositoryId: id, hasWikiEnabled: true }, opts); expect(result.violation).toBeUndefined(); expect(state?.repo.has_wiki).toBe(true); expect(isWriteRequest(result.log)).toBe(true); @@ -191,12 +209,34 @@ describe("GraphQL dispatch and logging", () => { violation: ['node ids of "acme/other"', `serves only "${ADMIN_SLUG}"`], }, ])("a single-mode mutation $name is a violation", ({ repositoryId, violation }) => { - const result = dispatch(G_WRITE, { repositoryId, hasWiki: true }, options(scenario())); + const result = dispatch(G_WRITE, { repositoryId, hasWikiEnabled: true }, options(scenario())); for (const fragment of violation) { expect(result.violation).toContain(fragment); } }); + test("control: the handler receives the variables exactly as sent, an undeclared one included", () => { + // The schema check coerces to judge, never to rewrite: the coerced variables would lack `stray`. + let received: unknown; + const handlers: Record = { + ...HANDLERS, + "repository.gToggles": ({ state, variables }) => { + received = variables; + return { data: { repository: { id: String(state.repo.node_id) } } }; + }, + }; + const body = gqlBody(G_READ, { owner: OWNER, repo: REPO, stray: 1 }); + const result = handleGraphqlRequest( + { method: "POST", body }, + options(scenario()), + baseLog(body), + OPS, + handlers, + ); + expect(result.violation).toBeUndefined(); + expect(received).toEqual({ owner: OWNER, repo: REPO, stray: 1 }); + }); + test("sectionForRequest attributes a /graphql request through its body", () => { // The live registry declares no "RepoToggles", so attribution resolves null and the REST // fallback stays intact. @@ -205,6 +245,34 @@ describe("GraphQL dispatch and logging", () => { }); }); +describe("GraphQL schema check", () => { + test("a request GitHub's schema refuses is answered ahead of the check-mode, fault, and permission barriers", () => { + // Behind any of the three, a check-mode, faulted, or denied request would hide variables GitHub refuses. + const opts = options( + scenario({ inputs: { mode: "check" }, token_permissions: { administration: "none" } }), + { faults: [{ key: "repository.gUpdate", kind: "server_error" }] }, + ); + const result = dispatch( + G_WRITE, + { repositoryId: mintNodeId("repo", ADMIN_SLUG, ""), hasWikiEnabled: "yes" }, + opts, + ); + expect(result.response).toEqual({ + status: 200, + body: { + errors: [ + expect.objectContaining({ + message: expect.stringContaining("Boolean cannot represent a non boolean value"), + }), + ], + }, + }); + expect(result.violation).toStartWith("GraphQL UpdateToggles is refused by GitHub's schema: "); + expect(opts.faultCounts.size).toBe(0); + expect(result.log.deniedBy).toBeUndefined(); + }); +}); + describe("GraphQL check-mode barrier", () => { test("a write in check mode is a violation, before the permission gate", () => { const s = scenario({ @@ -316,7 +384,7 @@ describe("GraphQL denial barrier (shared with REST)", () => { const wikiBefore = state.repo.has_wiki; const write = dispatch( G_WRITE, - { repositoryId: mintNodeId("repo", ADMIN_SLUG, ""), hasWiki: !wikiBefore }, + { repositoryId: mintNodeId("repo", ADMIN_SLUG, ""), hasWikiEnabled: !wikiBefore }, opts, ); expect(write.violation).toBeUndefined(); @@ -343,7 +411,7 @@ describe("GraphQL denial barrier (shared with REST)", () => { const wikiBefore = state.repo.has_wiki; const write = dispatch( G_WRITE, - { repositoryId: mintNodeId("repo", ADMIN_SLUG, ""), hasWiki: !wikiBefore }, + { repositoryId: mintNodeId("repo", ADMIN_SLUG, ""), hasWikiEnabled: !wikiBefore }, opts, ); expect(write.violation).toBeUndefined(); @@ -377,8 +445,22 @@ describe("GraphQL multi-repo slug resolution", () => { expect(denied.log.deniedBy).toBe("administration"); }); - test("a read without $owner/$repo variables is a violation", () => { - const result = dispatch(G_READ, { owner: "acme" }, multiOptions(scenario())); + test("a read addressing no repository is a violation", () => { + // A read that omits a $repo its document declares is GitHub's own refusal; this one declares no address at all. + const unaddressed: TaggedGraphqlOp = { + ...G_READ, + name: "ViewerLogin", + query: "query ViewerLogin { viewer { login } }", + role: "gViewer", + }; + const body = gqlBody(unaddressed, {}); + const result = handleGraphqlRequest( + { method: "POST", body }, + multiOptions(scenario()), + baseLog(body), + { ...OPS, "repository.gViewer": unaddressed }, + HANDLERS, + ); expect(result.violation).toContain("carries no $owner/$repo variables"); }); @@ -388,7 +470,8 @@ describe("GraphQL multi-repo slug resolution", () => { // The fixture starts every repo at has_wiki: true, so flipping alpha to // false is what proves the write landed on alpha and ONLY alpha. const id = mintNodeId("repo", "acme/alpha", ""); - const result = dispatch(G_WRITE, { input: { repositoryId: id }, hasWiki: false }, opts); + const input = { repositoryId: id, hasWikiEnabled: false }; + const result = dispatch(G_WRITE_NESTED, { input }, opts); expect(result.violation).toBeUndefined(); expect(multi?.repos.get("acme/alpha")?.repo.has_wiki).toBe(false); expect(multi?.repos.get("acme/beta")?.repo.has_wiki).toBe(true); diff --git a/test/e2e/mock/graphql-request.test.ts b/test/e2e/mock/graphql-request.test.ts new file mode 100644 index 00000000..868614f6 --- /dev/null +++ b/test/e2e/mock/graphql-request.test.ts @@ -0,0 +1,98 @@ +/** + * The pipeline answers a GraphQL request GitHub's schema refuses the way GitHub does, with errors and no data, ahead + * of every handler, and fails the run. Without it the mock stores what GitHub would have refused and every scenario + * passes on it. Pinned over the wire, since a handler unit test bypasses the pipeline. + */ + +import { describe, expect, test } from "bun:test"; +import { allGraphqlOps } from "../../../src/sections/registry.js"; +import { ADMIN_SLUG, ADMIN_OWNER as OWNER, ADMIN_REPO as REPO } from "../constants.js"; +import { sharedValidator } from "../openapi/validate.js"; +import { mintNodeId } from "./node-id.js"; +import { call, json, mockServerLifecycle, scenario, singleState } from "./server-test-support.js"; + +const start = mockServerLifecycle(); +const OPS = allGraphqlOps(); +const RULE_ID = mintNodeId("rule", ADMIN_SLUG, "release/*"); +const ENVIRONMENT_ID = mintNodeId("environment", ADMIN_SLUG, "prod"); + +/** One wildcard rule and one pinned environment, so a handler that ran would have something to change. */ +function seeded() { + return scenario({ + live_state: { + branch_protection_rules: [{ pattern: "release/*", isAdminEnforced: false }], + environments: { prod: { name: "prod", protection_rules: [] } }, + pinned_environments: ["prod"], + }, + }); +} + +describe("a request GitHub's schema refuses is answered with errors and no data, and fails the run", () => { + test.each([ + { + name: "an input field the schema lacks (the REST key on a rule mutation)", + op: OPS["branches.updateRule"], + variables: { input: { branchProtectionRuleId: RULE_ID, enforce_admins: true } }, + refusal: 'not to include unknown field "enforce_admins"', + }, + { + // Unrefused, the pin handler reads a null as "unpin" and drops the pin. + name: "null for a variable the operation declares Boolean!", + op: OPS["environments.pin"], + variables: { environmentId: ENVIRONMENT_ID, pinned: null }, + refusal: 'Variable "$pinned" has invalid value: Expected value of non-null type "Boolean!"', + }, + { + name: "a document selecting a field the schema lacks", + op: { + ...OPS["branches.repoLookup"], + query: + "query BranchProtectionRepository($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { id nodeId } }", + }, + variables: { owner: OWNER, repo: REPO }, + refusal: 'Cannot query field "nodeId" on type "Repository"', + }, + { + name: "a document that does not parse", + op: { ...OPS["branches.repoLookup"], query: "query BranchProtectionRepository {" }, + variables: { owner: OWNER, repo: REPO }, + refusal: "Syntax Error: Expected Name, found ", + }, + ])("$name stores nothing", async ({ op, variables, refusal }) => { + const h = await start(seeded()); + const before = structuredClone(singleState(h)); + const res = await call(h, "POST", "/graphql", { + body: { query: op.query, operationName: op.name, variables }, + }); + expect(res.status).toBe(200); + // The whole body: GitHub sends no `data` key beside a request error. + expect(await json(res)).toEqual({ + errors: [expect.objectContaining({ message: expect.stringContaining(refusal) })], + }); + expect(singleState(h)).toEqual(before); + expect(h.violations).toEqual([ + expect.stringContaining(`GraphQL ${op.name} is refused by GitHub's schema: `), + ]); + expect(h.violations[0]).toContain(refusal); + // The violation above is the one report: the contract validator must not add its own for the untyped errors. + expect(sharedValidator().validateLog(h.requests)).toEqual([]); + }); +}); + +test("control: a request on the schema is answered by its handler, with data and no violation", async () => { + const h = await start(seeded()); + const op = OPS["environments.pin"]; + const res = await call(h, "POST", "/graphql", { + body: { + query: op.query, + operationName: op.name, + variables: { environmentId: ENVIRONMENT_ID, pinned: false }, + }, + }); + expect(res.status).toBe(200); + expect(await json(res)).toEqual({ + data: { pinEnvironment: { environment: { name: "prod", isPinned: false } } }, + }); + expect(singleState(h).pinned_environments).toEqual([]); + expect(h.violations).toEqual([]); +}); diff --git a/test/e2e/mock/graphql-request.ts b/test/e2e/mock/graphql-request.ts new file mode 100644 index 00000000..ae2a8ef0 --- /dev/null +++ b/test/e2e/mock/graphql-request.ts @@ -0,0 +1,57 @@ +/** + * What GitHub refuses of a GraphQL request before it runs any of it: a document that does not validate against its + * schema, and variables that do not coerce to the types the operation declares. A handler handed such a request + * would store an input field GitHub's schema does not have, and every scenario would pass on it. + */ + +import { + type DocumentNode, + GraphQLError, + type GraphQLFormattedError, + parse, + validate, + validateExecutionArgs, +} from "graphql"; +import { sharedGraphqlSchema } from "./graphql-schema.js"; + +/** The document, or the syntax error graphql-js reports by throwing it. */ +function parsedDocument(query: string): DocumentNode | GraphQLError { + try { + return parse(query); + } catch (error) { + if (error instanceof GraphQLError) { + return error; + } + throw error; + } +} + +/** + * The errors GitHub answers `operationName` of `query` with under `variables`, or null when it accepts the request. + * graphql-js words them; api.github.com words the same refusals its own way. Coercion only judges: a caller keeps + * handing on the variables as sent. + */ +export function graphqlRequestErrors( + query: string, + operationName: string, + variables: Readonly>, +): readonly GraphQLFormattedError[] | null { + const document = parsedDocument(query); + if (document instanceof GraphQLError) { + return [document.toJSON()]; + } + + const schema = sharedGraphqlSchema(); + const documentErrors = validate(schema, document); + if (documentErrors.length > 0) { + return documentErrors.map((error) => error.toJSON()); + } + + const prepared = validateExecutionArgs({ + schema, + document, + operationName, + variableValues: variables, + }); + return "operation" in prepared ? null : prepared.map((error) => error.toJSON()); +} diff --git a/test/e2e/mock/graphql-schema.ts b/test/e2e/mock/graphql-schema.ts new file mode 100644 index 00000000..845f0cd6 --- /dev/null +++ b/test/e2e/mock/graphql-schema.ts @@ -0,0 +1,44 @@ +/** + * GitHub's GraphQL schema as the e2e mock and the declared-query tests read it: the SDL the @octokit/graphql-schema + * package ships, extended by the SDL of the upstream gaps the package lags. + */ + +import { schema as published } from "@octokit/graphql-schema"; +import { buildSchema, extendSchema, GraphQLSchema, parse } from "graphql"; +import { UNSHIPPED_GRAPHQL_SDL } from "../../../src/generated/upstream-gaps.js"; +import type { UnshippedGraphqlSdl } from "../../../src/upstream-gaps/gap.js"; + +/** Built once per process: parsing GitHub's SDL is the slow part, and run.ts and fuzz.ts check every GraphQL request against it. */ +let shared: GraphQLSchema | undefined; + +/** + * The published schema plus every graphql-schema gap's SDL. assumeValid skips graphql-js's schema-level validation, + * which rejects GitHub's SDL as-is (it deprecates implementation fields whose interface fields are not deprecated). + * Each extension is validated without the flag, so a type or field the package now ships is refused (the gap's + * tripwire); the result is rebuilt as assumeValid for the query validation. + */ +export function schemaWithGaps( + gaps: readonly UnshippedGraphqlSdl[] = UNSHIPPED_GRAPHQL_SDL, +): GraphQLSchema { + let schema = buildSchema(published.idl, { assumeValid: true }); + for (const { file, sdl } of gaps) { + try { + schema = new GraphQLSchema({ + ...extendSchema(schema, parse(sdl)).toConfig(), + assumeValid: true, + }); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error( + `the pinned @octokit/graphql-schema refuses the SDL of ${file}, so it now ships what that gap declares: ` + + `delete the file and regenerate the index (bun .github/scripts/gen-gaps-index.ts). ${reason}`, + ); + } + } + return schema; +} + +export function sharedGraphqlSchema(): GraphQLSchema { + shared ??= schemaWithGaps(); + return shared; +} diff --git a/test/e2e/mock/routes.ts b/test/e2e/mock/routes.ts index b45f97af..73ca925b 100644 --- a/test/e2e/mock/routes.ts +++ b/test/e2e/mock/routes.ts @@ -6,6 +6,7 @@ * The stage order is the contract, the same on both wires: * wire checks -> route match -> check-mode barrier -> target resolution -> fault barrier -> permission gate * -> denial barrier -> body allowlist -> handler -> response guard -> chaos hook + * GraphQL has no body allowlist: its schema check follows the route match, as GitHub validates before it runs anything. */ import { isPlainObject } from "../../../src/plain-data.js"; @@ -57,6 +58,7 @@ import { type Requirement, SECTION_BY_KEY, } from "./grading.js"; +import { graphqlRequestErrors } from "./graphql-request.js"; import { GRAPHQL_HANDLERS, HANDLERS } from "./handlers.js"; import { decodeNodeId, type NodeFamily } from "./node-id.js"; import { acceptedBody } from "./request-body.js"; @@ -248,7 +250,7 @@ function parseGraphqlRequest( request: { method: string; body: unknown }, ops: Readonly>, violation: (message: string) => PipelineResult, -): { key: string; op: TaggedGraphqlOp; variables: Json } | PipelineResult { +): { key: string; op: TaggedGraphqlOp; query: string; variables: Json } | PipelineResult { if (request.method !== "POST") { return violation(`GraphQL requests must be POST, got ${request.method}`); } @@ -268,7 +270,31 @@ function parseGraphqlRequest( `no GraphQL operation named "${String(body.operationName)}" is declared by any section`, ); } - return { key: dispatched.key, op: dispatched.op, variables: body.variables }; + return { key: dispatched.key, op: dispatched.op, query: body.query, variables: body.variables }; +} + +/** + * The schema check. GitHub answers a document off its schema, or variables off the operation's declared types, with + * HTTP 200 carrying errors and no data. The action shapes every document and variable itself, so the refusal is a + * violation too: the scenario fails whatever outcome it expected. + */ +function graphqlSchemaRefusal( + op: TaggedGraphqlOp, + query: string, + variables: Json, + log: LoggedRequest, +): PipelineResult | null { + const errors = graphqlRequestErrors(query, op.name, variables); + if (errors === null) { + return null; + } + return { + response: { status: 200, body: { errors } }, + log: { ...log, status: 200 }, + violation: `GraphQL ${op.name} is refused by GitHub's schema: ${errors.map((error) => error.message).join("; ")}`, + // A request error carries no type and no data beside it, which the validator's response contract would report a second time. + offSpecBody: true, + }; } /** @@ -340,12 +366,18 @@ export function handleGraphqlRequest( if (!("op" in parsed)) { return parsed; } - const { key, op, variables } = parsed; + const { key, op, query, variables } = parsed; const graphqlLog: LoggedRequest = { ...baseLog, graphql: { operationName: op.name, kind: op.kind }, }; + // Ahead of every barrier: no fault, denial, or handler answers a request GitHub refuses before running it. + const refused = graphqlSchemaRefusal(op, query, variables, graphqlLog); + if (refused) { + return refused; + } + // Check-mode barrier before the fault barrier: a synthetic fault must not mask the one bug this barrier exists to catch. if (options.checkMode && op.kind !== "read") { return violationFor(graphqlLog)(`GraphQL write in check mode (${op.name})`); diff --git a/test/sections/fragment-fake.ts b/test/sections/fragment-fake.ts index f29c6a0d..53c1581c 100644 --- a/test/sections/fragment-fake.ts +++ b/test/sections/fragment-fake.ts @@ -7,8 +7,13 @@ * GraphQL -> dispatched by operation name onto the mock's GraphQL handlers */ -import { type ClientAnswer, type GitHubClient, PAGE_SIZE } from "../../src/github/api.js"; -import type { ApiError } from "../../src/github/api-error.js"; +import { + type ClientAnswer, + type GitHubClient, + type GraphqlOp, + PAGE_SIZE, +} from "../../src/github/api.js"; +import { type ApiError, apiErrorFromGraphqlErrors } from "../../src/github/api-error.js"; import type { SectionKey } from "../../src/schema.js"; import type { SectionMeta } from "../../src/sections/contract/module.js"; import { endpointPermission } from "../../src/sections/contract/operations.js"; @@ -22,6 +27,7 @@ import { type Requirement, SECTION_BY_KEY, } from "../e2e/mock/grading.js"; +import { graphqlRequestErrors } from "../e2e/mock/graphql-request.js"; import { GRAPHQL_HANDLERS, HANDLERS } from "../e2e/mock/handlers.js"; import { acceptedBody } from "../e2e/mock/request-body.js"; import { buildStateForSlug, type LiveState, type MockState } from "../e2e/mock/state.js"; @@ -100,19 +106,29 @@ function routeOf( return { url, matched, handler }; } -/** The GraphQL operation the body names and its handler, or the 404 the fake answers outside the section's (or the registry's) handlers. */ +/** + * The GraphQL operation the request names and its handler; else the 404 the fake answers outside the section's (or the registry's) handlers, or + * GitHub's refusal of a request off its schema, the stage the wire pipeline runs ahead of every barrier (routes.ts). + */ function graphqlRouteOf( only: SectionKey | null, - name: string, + op: GraphqlOp, + variables: Readonly>, ): GraphqlRouted | { error: ApiError } { - const dispatched = graphqlOpForBody({ operationName: name }, allGraphqlOps()); + const dispatched = graphqlOpForBody({ operationName: op.name }, allGraphqlOps()); const handler = dispatched === null ? undefined : GRAPHQL_HANDLERS[dispatched.key]; if ( dispatched === null || (only !== null && dispatched.op.section !== only) || handler === undefined ) { - return { error: { status: 404, message: `unexpected GRAPHQL ${name}`, body: "" } }; + return { error: { status: 404, message: `unexpected GRAPHQL ${op.name}`, body: "" } }; + } + + // Refused before the handler, so a section proof cannot converge on variables GitHub would not accept. + const refused = graphqlRequestErrors(op.query, op.name, variables); + if (refused !== null) { + return { error: apiErrorFromGraphqlErrors([...refused], false) }; } return { dispatched, handler }; } @@ -233,7 +249,7 @@ function handlerFake( } }, async tryGraphql(op, variables) { - const routed = graphqlRouteOf(only, op.name); + const routed = graphqlRouteOf(only, op, variables); if ("error" in routed) { return routed; } diff --git a/test/sections/graphql-queries.test.ts b/test/sections/graphql-queries.test.ts index c631b220..d9cecde8 100644 --- a/test/sections/graphql-queries.test.ts +++ b/test/sections/graphql-queries.test.ts @@ -6,52 +6,14 @@ */ import { describe, expect, test } from "bun:test"; -import { schema as published } from "@octokit/graphql-schema"; -import { - buildSchema, - extendSchema, - type FieldNode, - GraphQLSchema, - type OperationDefinitionNode, - parse, - validate, - visit, -} from "graphql"; -import { UNSHIPPED_GRAPHQL_SDL } from "../../src/generated/upstream-gaps.js"; +import { type FieldNode, type OperationDefinitionNode, parse, validate, visit } from "graphql"; import { GRAPHQL_BOOLEAN_TWINS, GRAPHQL_REVIEW_TWINS, GRAPHQL_STATUS_CHECK_TWINS, } from "../../src/sections/branches/graphql-vocabulary.js"; import { allGraphqlOps } from "../../src/sections/registry.js"; -import type { UnshippedGraphqlSdl } from "../../src/upstream-gaps/gap.js"; - -/** - * The published schema plus every graphql-schema gap's SDL. assumeValid skips graphql-js's schema-level validation, - * which rejects GitHub's SDL as-is (it deprecates implementation fields whose interface fields are not deprecated). - * Each extension is validated without the flag, so a type or field the package now ships is refused (the gap's - * tripwire); the result is rebuilt as assumeValid for the query validation. - */ -function schemaWithGaps( - gaps: readonly UnshippedGraphqlSdl[] = UNSHIPPED_GRAPHQL_SDL, -): GraphQLSchema { - let schema = buildSchema(published.idl, { assumeValid: true }); - for (const { file, sdl } of gaps) { - try { - schema = new GraphQLSchema({ - ...extendSchema(schema, parse(sdl)).toConfig(), - assumeValid: true, - }); - } catch (error) { - const reason = error instanceof Error ? error.message : String(error); - throw new Error( - `the pinned @octokit/graphql-schema refuses the SDL of ${file}, so it now ships what that gap declares: ` + - `delete the file and regenerate the index (bun .github/scripts/gen-gaps-index.ts). ${reason}`, - ); - } - } - return schema; -} +import { schemaWithGaps } from "../e2e/mock/graphql-schema.js"; /** The single operation definition of a declared query, asserted to exist. */ function operationOf(key: string, query: string): OperationDefinitionNode { From 887b7dd2af5c872b6e599954850c4c580eccebd8 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:50:31 -0500 Subject: [PATCH 2/2] test(e2e): pin the unit fake's GraphQL refusal and name the gap SDL's consumer The unit fake's refusal of a request off GitHub's schema had no test, because the section suites send only requests GitHub accepts. test/sections/fragment-fake.test.ts sends `pinned: null` through the fake and expects GitHub's refusal with nothing stored, then the same request with a boolean, which unpins. The generated gap index said UNSHIPPED_GRAPHQL_SDL is consumed by test/sections/graphql-queries.test.ts. The schema builder in test/e2e/mock/graphql-schema.ts consumes it now, so the generator's sentence names the builder and src/generated/upstream-gaps.ts is regenerated: one comment line. --- .github/scripts/gen-gaps-index.ts | 2 +- src/generated/upstream-gaps.ts | 2 +- test/sections/fragment-fake.test.ts | 39 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 test/sections/fragment-fake.test.ts diff --git a/.github/scripts/gen-gaps-index.ts b/.github/scripts/gen-gaps-index.ts index 3ecd56ba..0218da13 100644 --- a/.github/scripts/gen-gaps-index.ts +++ b/.github/scripts/gen-gaps-index.ts @@ -107,7 +107,7 @@ export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] /** * The SDL GitHub's GraphQL API serves but the pinned @octokit/graphql-schema * release lacks, each with the gap file that carries it. Consumed by - * test/sections/graphql-queries.test.ts, which extends the published schema + * test/e2e/mock/graphql-schema.ts, where the published schema is extended * with it before validating the declared queries. */ export const UNSHIPPED_GRAPHQL_SDL: readonly UnshippedGraphqlSdl[] = unshippedGraphqlSdl(GAPS); diff --git a/src/generated/upstream-gaps.ts b/src/generated/upstream-gaps.ts index 96d7bfd1..8b52796d 100644 --- a/src/generated/upstream-gaps.ts +++ b/src/generated/upstream-gaps.ts @@ -42,7 +42,7 @@ export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] /** * The SDL GitHub's GraphQL API serves but the pinned @octokit/graphql-schema * release lacks, each with the gap file that carries it. Consumed by - * test/sections/graphql-queries.test.ts, which extends the published schema + * test/e2e/mock/graphql-schema.ts, where the published schema is extended * with it before validating the declared queries. */ export const UNSHIPPED_GRAPHQL_SDL: readonly UnshippedGraphqlSdl[] = unshippedGraphqlSdl(GAPS); diff --git a/test/sections/fragment-fake.test.ts b/test/sections/fragment-fake.test.ts new file mode 100644 index 00000000..0cf0b0ed --- /dev/null +++ b/test/sections/fragment-fake.test.ts @@ -0,0 +1,39 @@ +/** + * The fake answers GraphQL through the stages the wire pipeline runs ahead of its handlers. The section suites send + * only requests GitHub accepts, so the refusal path is pinned here: without it a section proof could converge on + * variables GitHub would refuse. + */ + +import { expect, test } from "bun:test"; +import { allGraphqlOps } from "../../src/sections/registry.js"; +import { mintNodeId } from "../e2e/mock/node-id.js"; +import { registryFake } from "./fragment-fake.js"; +import { REPO } from "./section-run.js"; + +test("control: pinned: null through the fake is refused as GitHub refuses it and stores nothing, while pinned: false unpins", async () => { + const fake = registryFake({ + environments: { prod: { name: "prod", protection_rules: [] } }, + pinned_environments: ["prod"], + }); + const op = allGraphqlOps()["environments.pin"]; + const environmentId = mintNodeId("environment", REPO.slug, "prod"); + const pinned = () => fake.state.pinned_environments.map((pin) => pin.name); + + const refused = await fake.tryGraphql(op, { environmentId, pinned: null }, REPO.slug); + expect(refused).toEqual({ + error: expect.objectContaining({ + status: 422, + message: expect.stringContaining('Variable "$pinned" has invalid value'), + }), + }); + expect(fake.writes).toEqual([]); + expect(pinned()).toEqual(["prod"]); + + // The same request with a boolean: the refusal above came from the null, not from the fake. + const accepted = await fake.tryGraphql(op, { environmentId, pinned: false }, REPO.slug); + expect(accepted).toEqual({ + data: { pinEnvironment: { environment: { name: "prod", isPinned: false } } }, + }); + expect(fake.writes).toEqual(["GRAPHQL PinEnvironment"]); + expect(pinned()).toEqual([]); +});