From eb5ed447a904d76f2e1a7c4d616f6f77edede1f5 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Tue, 22 Sep 2026 00:18:14 -0400 Subject: [PATCH] chore(test): take the OpenAPI and GraphQL schemas from the Octokit packages instead of fetching them The e2e validator and the docs generator read GitHub's dereferenced OpenAPI descriptor from @octokit/openapi, cut in memory to USED_PATHS. The GraphQL lockstep tests read GitHub's schema from @octokit/graphql-schema. trim-openapi.ts, fetch-graphql-schema.ts, their fetch and staleness helpers, the fetch-test-artifacts composite, and every cache step are deleted. No script fetches from the network any more, and Dependabot moves both pins. A used path the descriptor lacks, an upstream gap it now documents, or a $ref left in the slice fails the load by name. The pinned @octokit/graphql-schema predates Repository.issueCreationPolicy, so a graphql-schema gap kind carries the SDL the package lags and the lockstep test extends the schema with it. The nightly probe installs both packages at latest and runs the lockstep tests instead of re-cutting the spec from upstream HEAD. --- .../actions/fetch-test-artifacts/action.yml | 40 ---- .github/scripts/check-compat-markers.ts | 5 +- .github/scripts/endpoint-docs.ts | 10 +- .github/scripts/endpoint-docs.yml | 2 +- .github/scripts/fetch-graphql-schema.ts | 73 ------- .github/scripts/gen-gaps-index.ts | 43 ++-- .github/scripts/graduate-upstream-gaps.ts | 2 +- .github/scripts/lib/fetch-retry.ts | 79 ------- .github/scripts/lib/fetched-artifact.ts | 71 ------- .github/scripts/trim-openapi.ts | 184 ----------------- .github/workflows/auto-fix.yml | 2 - .github/workflows/checks.yml | 14 +- .github/workflows/nightly-fuzz.yml | 6 - .github/workflows/nightly.yml | 85 +++----- .github/workflows/update-release.yml | 2 - .gitignore | 8 - CONTRIBUTING.md | 2 +- bun.lock | 10 + package.json | 13 +- src/sections/shared/roles.ts | 2 +- src/upstream-gaps/gap.ts | 60 ++++-- src/upstream-gaps/index.ts | 24 ++- src/upstream-gaps/issue-creation-policy.ts | 17 ++ test/docs/checks-workflow.test.ts | 194 +----------------- test/docs/workflow-loader.ts | 12 +- test/e2e/mock/request-body.test.ts | 2 +- test/e2e/mock/request-body.ts | 2 +- test/e2e/mock/support.ts | 2 +- test/e2e/openapi/paths.ts | 10 +- test/e2e/openapi/validate.test.ts | 95 ++++++--- test/e2e/openapi/validate.ts | 112 +++++++--- test/scripts/auto-fix-allowlist.test.ts | 8 +- test/scripts/changed-sections.test.ts | 2 +- test/scripts/check-compat-markers.test.ts | 3 +- test/scripts/endpoint-docs.test.ts | 75 ++----- test/scripts/fetch-retry.test.ts | 111 ---------- test/scripts/fetched-artifact.test.ts | 102 --------- test/scripts/generated.test.ts | 3 - test/scripts/graduate-upstream-gaps.test.ts | 19 +- test/sections/graphql-queries.test.ts | 75 ++++--- 40 files changed, 419 insertions(+), 1162 deletions(-) delete mode 100644 .github/actions/fetch-test-artifacts/action.yml delete mode 100644 .github/scripts/fetch-graphql-schema.ts delete mode 100644 .github/scripts/lib/fetch-retry.ts delete mode 100644 .github/scripts/lib/fetched-artifact.ts delete mode 100644 .github/scripts/trim-openapi.ts create mode 100644 src/upstream-gaps/issue-creation-policy.ts delete mode 100644 test/scripts/fetch-retry.test.ts delete mode 100644 test/scripts/fetched-artifact.test.ts diff --git a/.github/actions/fetch-test-artifacts/action.yml b/.github/actions/fetch-test-artifacts/action.yml deleted file mode 100644 index 60177fd6..00000000 --- a/.github/actions/fetch-test-artifacts/action.yml +++ /dev/null @@ -1,40 +0,0 @@ -# Restores the two fetched, gitignored test artifacts (trimmed OpenAPI spec, GraphQL schema) -# from cache, fetching each on a miss: the ONE place an artifact is cached (the drift-tripwire -# nightlies fetch uncached by design). Needs ./.github/actions/setup (bun and the install) earlier in the job. -name: Fetch the test artifacts -description: Restore the trimmed OpenAPI spec and the GraphQL schema from cache, fetching each on a miss - -runs: - using: composite - steps: - # Hashes every input that changes the output: the trim script (UPSTREAM_REF), its fetch helper - # and the staleness helper (the recorded source-URL key), USED_PATHS and its route-data imports, and - # the DEFAULT_API_VERSION source. - - name: Cache the trimmed OpenAPI spec - id: openapi-cache - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: test/e2e/openapi/github-openapi.trimmed.json - key: >- - openapi-trimmed-${{ hashFiles('.github/scripts/trim-openapi.ts', - '.github/scripts/lib/fetch-retry.ts', '.github/scripts/lib/fetched-artifact.ts', - 'test/e2e/openapi/paths.ts', 'src/report/issue-report.ts', 'src/sections/**', - 'src/upstream-gaps/**', 'src/schema.ts', 'src/github/api.ts') }} - - name: Fetch the trimmed OpenAPI spec (cache miss) - if: steps.openapi-cache.outputs.cache-hit != 'true' - shell: bash - run: bun .github/scripts/trim-openapi.ts - # The key hashes the fetch script (it carries the pinned UPSTREAM_REF and the source-URL marker line), - # the fetch helper the bytes come through, and the staleness helper. - - name: Cache the GraphQL schema - id: graphql-schema-cache - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: test/e2e/graphql/schema.docs.graphql - key: >- - graphql-schema-${{ hashFiles('.github/scripts/fetch-graphql-schema.ts', - '.github/scripts/lib/fetch-retry.ts', '.github/scripts/lib/fetched-artifact.ts') }} - - name: Fetch the GraphQL schema (cache miss) - if: steps.graphql-schema-cache.outputs.cache-hit != 'true' - shell: bash - run: bun .github/scripts/fetch-graphql-schema.ts diff --git a/.github/scripts/check-compat-markers.ts b/.github/scripts/check-compat-markers.ts index cfcd2b83..03f491ef 100644 --- a/.github/scripts/check-compat-markers.ts +++ b/.github/scripts/check-compat-markers.ts @@ -16,13 +16,12 @@ import { execFileSync } from "node:child_process"; import { lstatSync, readFileSync } from "node:fs"; import { join } from "node:path"; -/** Built output, dependencies, the fetched spec, release-please's changelog (it quotes PR titles, so a removal PR's - * title would outlive the marker it deleted), and the two files that spell the syntax to define and test it. +/** Built output, dependencies, release-please's changelog (it quotes PR titles, so a removal PR's title would + * outlive the marker it deleted), and the two files that spell the syntax to define and test it. * A trailing slash skips a directory; anything else is one exact path. */ const SKIPPED_PATHS = [ "lib/", "node_modules/", - "test/e2e/openapi/github-openapi.trimmed.json", "CHANGELOG.md", ".github/scripts/check-compat-markers.ts", "test/scripts/check-compat-markers.test.ts", diff --git a/.github/scripts/endpoint-docs.ts b/.github/scripts/endpoint-docs.ts index 3bcc4bfb..5ff36a99 100644 --- a/.github/scripts/endpoint-docs.ts +++ b/.github/scripts/endpoint-docs.ts @@ -1,6 +1,6 @@ /** * The docs.github.com page behind every REST route and GraphQL operation the sections declare, for the coverage - * page's Endpoints cells (gen-docs.ts). REST pages are the trimmed OpenAPI descriptor's own externalDocs links; + * page's Endpoints cells (gen-docs.ts). REST pages are the OpenAPI descriptor's own externalDocs links; * endpoint-docs.yml beside this file holds only what the descriptor lacks: the routes GitHub does not document and * every GraphQL operation. resolveAnchors() fails the docs build by name on a declared call with no page, a hand * entry the descriptor already covers, and a hand entry no section declares. @@ -10,7 +10,7 @@ import { join } from "node:path"; import { z } from "zod"; import { readDocsYaml } from "../../src/sections/contract/docs.js"; import { allEndpoints, allGraphqlOps } from "../../src/sections/registry.js"; -import { readSpecText } from "../../test/e2e/openapi/validate.js"; +import { loadSpec } from "../../test/e2e/openapi/validate.js"; const DOCS_URL = z.string().url().startsWith("https://docs.github.com/en/"); @@ -118,10 +118,6 @@ export function resolveAnchors( return { rest, graphql }; } -function specOperations(): SpecOperations { - return (JSON.parse(readSpecText()) as { paths: SpecOperations }).paths; -} - /** The distinct routes and operation names the sections declare, in registry order. */ export function declaredCalls(): { routes: string[]; operations: string[] } { return { @@ -132,5 +128,5 @@ export function declaredCalls(): { routes: string[]; operations: string[] } { export const ENDPOINT_ANCHORS: EndpointAnchors = (() => { const { routes, operations } = declaredCalls(); - return resolveAnchors(specOperations(), ENDPOINT_DOCS, routes, operations); + return resolveAnchors(loadSpec().paths, ENDPOINT_DOCS, routes, operations); })(); diff --git a/.github/scripts/endpoint-docs.yml b/.github/scripts/endpoint-docs.yml index e1dda09c..678a38e5 100644 --- a/.github/scripts/endpoint-docs.yml +++ b/.github/scripts/endpoint-docs.yml @@ -1,4 +1,4 @@ -# The docs.github.com page for every call the trimmed OpenAPI descriptor cannot supply: the routes GitHub does not +# The docs.github.com page for every call the OpenAPI descriptor cannot supply: the routes GitHub does not # document (src/upstream-gaps/) and every GraphQL operation, keyed as the declaration spells them. Every other REST # route takes its page from the descriptor's own externalDocs link (endpoint-docs.ts). The lockstep test in # test/scripts/endpoint-docs.test.ts fails on a declared call without a page, an entry the descriptor already diff --git a/.github/scripts/fetch-graphql-schema.ts b/.github/scripts/fetch-graphql-schema.ts deleted file mode 100644 index 7c954aa1..00000000 --- a/.github/scripts/fetch-graphql-schema.ts +++ /dev/null @@ -1,73 +0,0 @@ -/** - * Fetches GitHub's public GraphQL schema to disk, the GraphQL sibling of trim-openapi.ts. This script is the ONLY - * thing that touches the network; the output is a fetched, gitignored artifact. - * test/sections/graphql-queries.test.ts -> loads it from disk - * test, test:e2e, fuzz scripts -> run this with --when-stale first: a fetch only when the file is absent - * or fetched from another ref, so a fresh checkout fetches once - * CI -> restores it from cache, re-fetches on a miss, then the same bun run test - * UPSTREAM_REF -> PINNED to a github/docs commit, so two runs months apart write - * byte-identical text; the first line records SCHEMA_URL as a comment - */ - -import { mkdirSync, renameSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; -import { buildSchema } from "graphql"; -import { fetchTextWithRetry } from "./lib/fetch-retry.js"; -import { readArtifact, schemaStaleness, whenStale } from "./lib/fetched-artifact.js"; - -const UPSTREAM_REF = "01f2174e1ab5d15d4946cfe96ef7dfb5c9a8b889"; - -/** fpt is the free-tier (github.com) flavor, the one the action targets. */ -const SCHEMA_URL = - `https://raw.githubusercontent.com/github/docs/${UPSTREAM_REF}` + - "/src/graphql/data/fpt/schema.docs.graphql"; - -const OUT_PATH = join(import.meta.dir, "..", "..", "test", "e2e", "graphql", "schema.docs.graphql"); - -/** The first line of the written file: a GraphQL comment naming the source URL, so --when-stale can tell a stale file. */ -const MARKER = `# ${SCHEMA_URL}`; - -const FETCH_TIMEOUT_MS = 60_000; - -async function main(): Promise { - if (whenStale(process.argv)) { - const reason = schemaStaleness(readArtifact(OUT_PATH), MARKER); - if (reason === null) { - console.log(`${OUT_PATH} is current (fetched from ${SCHEMA_URL}); not fetching`); - return 0; - } - console.log(`regenerating ${OUT_PATH}: ${reason}`); - } - console.log(`fetching ${SCHEMA_URL}`); - const fetched = await fetchTextWithRetry("GraphQL schema", SCHEMA_URL, FETCH_TIMEOUT_MS); - if (!fetched.ok) { - throw new Error( - `failed to fetch the GraphQL schema: ${fetched.status} ${fetched.statusText} for ${SCHEMA_URL}. Check UPSTREAM_REF and the schema path`, - ); - } - const text = fetched.text; - // A truncated download or a moved upstream file must fail HERE, not as an opaque parse error in the disk-only consumer. - try { - buildSchema(text); - } catch (error) { - throw new Error( - `the fetched GraphQL schema from ${SCHEMA_URL} failed to parse: ${error instanceof Error ? error.message : String(error)}. The download may be truncated (re-run), or the upstream file changed shape (check UPSTREAM_REF)`, - ); - } - // Temp file then rename, so an aborted run leaves the previously written schema intact. The directory holds no - // tracked file, so a fresh checkout must create it first. - mkdirSync(dirname(OUT_PATH), { recursive: true }); - const tmpPath = `${OUT_PATH}.tmp`; - writeFileSync(tmpPath, `${MARKER}\n${text}`); - renameSync(tmpPath, OUT_PATH); - const sizeKb = Math.round(Buffer.byteLength(text) / 1024); - console.log(`wrote ${OUT_PATH} (${sizeKb} KB)`); - return 0; -} - -try { - process.exit(await main()); -} catch (error) { - console.error(error instanceof Error ? error.message : String(error)); - process.exit(1); -} diff --git a/.github/scripts/gen-gaps-index.ts b/.github/scripts/gen-gaps-index.ts index 78be15b9..65ac04c0 100644 --- a/.github/scripts/gen-gaps-index.ts +++ b/.github/scripts/gen-gaps-index.ts @@ -1,5 +1,5 @@ /** - * Regenerates src/upstream-gaps/index.ts WHOLESALE from the directory listing: the derivations split the two gap + * Regenerates src/upstream-gaps/index.ts WHOLESALE from the directory listing: the derivations split the gap * kinds by their `kind` field, so nothing but the file names is needed. * a gap file added by hand -> `bun .github/scripts/gen-gaps-index.ts` * graduate-upstream-gaps.ts -> calls regenerateIndex() itself @@ -31,6 +31,12 @@ export function camelCaseGapName(base: string): string { return base.replace(/-([a-z0-9])/g, (_, ch: string) => ch.toUpperCase()); } +/** The record entry for a gap file: shorthand when the alias is the base name, a quoted key otherwise. */ +export function gapEntry(base: string): string { + const alias = camelCaseGapName(base); + return alias === base ? `${alias},` : `"${base}": ${alias},`; +} + export function gapFileBases(listing: readonly string[]): string[] { return listing .filter(isGapFileName) @@ -43,7 +49,10 @@ export function gapFileBases(listing: readonly string[]): string[] { export function generateIndex(bases: readonly string[]): string { const sorted = [...bases].sort(); const imports = [ - { specifier: "./gap.js", line: `import { undocumentedRoutes } from "./gap.js";` }, + { + specifier: "./gap.js", + line: `import { undocumentedRoutes, type UnshippedGraphqlSdl, unshippedGraphqlSdl } from "./gap.js";`, + }, ...sorted.map((base) => ({ specifier: `./${base}.js`, line: `import { GAP as ${camelCaseGapName(base)} } from "./${base}.js";`, @@ -52,17 +61,15 @@ export function generateIndex(bases: readonly string[]): string { .sort((a, b) => (a.specifier < b.specifier ? -1 : 1)) .map((entry) => entry.line) .join("\n"); - const gapsArray = + const gapsRecord = sorted.length === 0 - ? "const GAPS = [] as const;" - : [ - "const GAPS = [", - ...sorted.map((base) => ` ${camelCaseGapName(base)},`), - "] as const;", - ].join("\n"); + ? "const GAPS = {} as const;" + : ["const GAPS = {", ...sorted.map((base) => ` ${gapEntry(base)}`), "} as const;"].join( + "\n", + ); return `/** * GENERATED by gen-gaps-index.ts - do not edit. Every pending upstream gap, - * aggregated in sorted file order; regenerate with + * keyed by file base name in sorted 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. @@ -70,9 +77,9 @@ export function generateIndex(bases: readonly string[]): string { ${imports} -${gapsArray} +${gapsRecord} -type GapUnion = (typeof GAPS)[number]; +type GapUnion = (typeof GAPS)[keyof typeof GAPS]; /** * Routes GitHub documents but the pinned @octokit/types release does not @@ -90,11 +97,19 @@ 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. + * excludes their paths from the descriptor slice the e2e validator loads + * and exempts exactly these METHOD+path pairs from its unknown-route check. */ export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] = undocumentedRoutes(GAPS); + +/** + * 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 + * with it before validating the declared queries. + */ +export const UNSHIPPED_GRAPHQL_SDL: readonly UnshippedGraphqlSdl[] = unshippedGraphqlSdl(GAPS); `; } diff --git a/.github/scripts/graduate-upstream-gaps.ts b/.github/scripts/graduate-upstream-gaps.ts index bf04a485..ea0fda49 100644 --- a/.github/scripts/graduate-upstream-gaps.ts +++ b/.github/scripts/graduate-upstream-gaps.ts @@ -98,7 +98,7 @@ export function isSpecOnly(gapSource: string): boolean { } /** A documentedInSpec: false gap whose tripwire fired means octokit caught up but the pinned descriptor did not: it - * is rewritten rather than deleted, so its UNDOCUMENTED_ROUTES exemption survives until a bumped UPSTREAM_REF documents the paths. */ + * is rewritten rather than deleted, so its UNDOCUMENTED_ROUTES exemption survives until an @octokit/openapi bump documents the paths. */ export function isSpecPinned(gapSource: string): boolean { return /documentedInSpec:\s*false/.test(gapSource); } diff --git a/.github/scripts/lib/fetch-retry.ts b/.github/scripts/lib/fetch-retry.ts deleted file mode 100644 index e2fa84fa..00000000 --- a/.github/scripts/lib/fetch-retry.ts +++ /dev/null @@ -1,79 +0,0 @@ -/** - * Bounded fetch retry with backoff for trim-openapi.ts and fetch-graphql-schema.ts, which run on a cache miss inside - * the CI gate, so a single network blip must not fail all-green; a deterministic 4xx still surfaces immediately. - * Under lib/ so knip treats it as project code: if every caller stops importing it, knip flags it as unused. - */ - -const FETCH_ATTEMPTS = 3; -export const BACKOFF_BASE_MS = 2_000; - -/** Statuses below 500 that are still transient, not deterministic. */ -const TRANSIENT_STATUSES = new Set([408, 429]); - -export interface FetchedText { - ok: boolean; - status: number; - statusText: string; - /** Empty on a non-ok response; callers report the status. */ - text: string; -} - -/** Injectable seams for tests; production callers pass nothing. */ -export interface FetchRetryDeps { - /** Only the call shape the retry loop uses, so a test stub types plainly. */ - fetchImpl?: (url: string, init?: RequestInit) => Promise; - sleep?: (ms: number) => Promise; - warn?: (line: string) => void; -} - -/** Timeouts include mid-body, so a dropped connection while a multi-MB artifact downloads retries too. A deterministic - * non-ok response (a plain 4xx) comes back as-is with an empty body rather than retrying. */ -export async function fetchTextWithRetry( - label: string, - url: string, - timeoutMs: number, - deps: FetchRetryDeps = {}, -): Promise { - const fetchImpl = deps.fetchImpl ?? fetch; - const sleep = deps.sleep ?? Bun.sleep; - const warn = deps.warn ?? console.warn; - // Hoisted so a malformed URL fails here, not from inside the error path. - const host = new URL(url).host; - for (let attempt = 1; ; attempt++) { - let failure: string; - try { - const response = await fetchImpl(url, { signal: AbortSignal.timeout(timeoutMs) }); - if (!response.ok && !TRANSIENT_STATUSES.has(response.status) && response.status < 500) { - return { - ok: false, - status: response.status, - statusText: response.statusText, - text: "", - }; - } - if (response.ok) { - // The body read shares the attempt's AbortSignal, so a stall here also times out and is retried. - const text = await response.text(); - return { ok: true, status: response.status, statusText: response.statusText, text }; - } - failure = `HTTP ${response.status} ${response.statusText}`; - } catch (error) { - failure = - error instanceof Error && error.name === "TimeoutError" - ? `timed out after ${timeoutMs}ms` - : error instanceof Error - ? error.message - : String(error); - } - if (attempt >= FETCH_ATTEMPTS) { - throw new Error( - `fetching the ${label} failed after ${FETCH_ATTEMPTS} attempts for ${url}: ${failure}. Check network access to ${host} and re-run`, - ); - } - const delayMs = BACKOFF_BASE_MS * 2 ** (attempt - 1); - warn( - `fetching the ${label}: attempt ${attempt}/${FETCH_ATTEMPTS} for ${url} failed (${failure}); retrying in ${delayMs}ms`, - ); - await sleep(delayMs); - } -} diff --git a/.github/scripts/lib/fetched-artifact.ts b/.github/scripts/lib/fetched-artifact.ts deleted file mode 100644 index 1fb9feeb..00000000 --- a/.github/scripts/lib/fetched-artifact.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** - * The "is the fetched artifact on disk current?" decision behind `--when-stale` in trim-openapi.ts and - * fetch-graphql-schema.ts: the test, test:e2e, and fuzz scripts run both in that mode first, so a fresh checkout fetches the - * gitignored artifacts once and a current file costs no network. Each artifact records the URL it was fetched from, - * which carries the pinned ref (and, for the spec, the API version), so a bumped pin regenerates it on the next run. - * Mtimes cannot carry this decision: actions/cache restores yesterday's mtime under today's checkout, so every CI - * run would look stale and refetch. - */ - -import { readFileSync } from "node:fs"; - -export function whenStale(argv: readonly string[]): boolean { - return argv.includes("--when-stale"); -} - -export function readArtifact(path: string): string | null { - try { - return readFileSync(path, "utf8"); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - return null; - } - throw error; - } -} - -/** The root key the trimmed OpenAPI spec records its source URL under (an OpenAPI `x-` extension). */ -export const SOURCE_KEY = "x-source-url"; - -/** Why the trimmed OpenAPI spec `raw` must be regenerated from `sourceUrl` with `usedPaths`, or null when current. */ -export function specStaleness( - raw: string | null, - sourceUrl: string, - usedPaths: readonly string[], -): string | null { - if (raw === null) { - return "the file is absent"; - } - let doc: unknown; - try { - doc = JSON.parse(raw); - } catch { - return "the file is not valid JSON"; - } - if (typeof doc !== "object" || doc === null) { - return "the file is not a JSON object"; - } - const { [SOURCE_KEY]: recorded, paths } = doc as Record; - if (recorded !== sourceUrl) { - const from = typeof recorded === "string" ? recorded : "an unrecorded URL"; - return `it was trimmed from ${from}, the script fetches ${sourceUrl}`; - } - const have = typeof paths === "object" && paths !== null ? Object.keys(paths).sort() : []; - const want = [...usedPaths].sort(); - if (have.length !== want.length || have.some((path, i) => path !== want[i])) { - return "its paths differ from USED_PATHS"; - } - return null; -} - -/** The GraphQL sibling of specStaleness(): the source URL lives in the file's first line. */ -export function schemaStaleness(raw: string | null, marker: string): string | null { - if (raw === null) { - return "the file is absent"; - } - const firstLine = raw.split("\n", 1)[0] ?? ""; - if (firstLine !== marker) { - return `its first line is ${JSON.stringify(firstLine)}, the script writes ${JSON.stringify(marker)}`; - } - return null; -} diff --git a/.github/scripts/trim-openapi.ts b/.github/scripts/trim-openapi.ts deleted file mode 100644 index b44e9c65..00000000 --- a/.github/scripts/trim-openapi.ts +++ /dev/null @@ -1,184 +0,0 @@ -/** - * Trims the published GitHub OpenAPI description to exactly the paths the action can reach and writes it to disk. - * This script is the ONLY thing that touches the network; the output is a fetched, gitignored artifact (~4MB). - * test/e2e/openapi/validate.ts -> loads it from disk - * test, test:e2e, fuzz scripts -> run this with --when-stale first: a fetch only when the file is absent, cut - * from another ref, or holding other paths, so a fresh checkout fetches once - * CI -> restores it from cache, re-fetches on a miss, then runs the same bun run test - * UPSTREAM_REF -> PINNED to a commit SHA, so two runs months apart produce byte-identical output - * from the same USED_PATHS; the output records SPEC_URL under "x-source-url" - */ - -import { renameSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { DEFAULT_API_VERSION } from "../../src/github/api.js"; -import { UNDOCUMENTED_PATHS, USED_PATHS } from "../../test/e2e/openapi/paths.js"; -import { fetchTextWithRetry } from "./lib/fetch-retry.js"; -import { readArtifact, SOURCE_KEY, specStaleness, whenStale } from "./lib/fetched-artifact.js"; - -const UPSTREAM_REF = "16bc535ad66fac59d585b1516d1d52f58f787962"; - -/** TRIM_UPSTREAM_REF overrides the pin for one run (the nightly upstream probe points it at the latest descriptor); - * unset or blank means the pinned SHA, so default runs stay byte-identical. */ -const REF = resolveRef(process.env.TRIM_UPSTREAM_REF); - -function resolveRef(override: string | undefined): string { - const ref = override?.trim(); - if (!ref) { - return UPSTREAM_REF; - } - // The ref lands in a URL path: refuse anything that could reshape the URL (query, fragment, traversal). - if (!/^[A-Za-z0-9._/-]+$/.test(ref) || ref.includes("..")) { - throw new Error( - `TRIM_UPSTREAM_REF "${ref}" is not a plain git ref (letters, digits, ".", "_", "/", "-"; no "..")`, - ); - } - return ref; -} - -/** The dereferenced (no $ref) descriptor, so the trimmed slice is self-contained: keeping a path drags its inlined - * schemas along, with no components/schemas graph to also carry. */ -const SPEC_URL = - `https://raw.githubusercontent.com/github/rest-api-description/${REF}` + - `/descriptions/api.github.com/dereferenced/api.github.com.${DEFAULT_API_VERSION}.deref.json`; - -const OUT_PATH = join( - import.meta.dir, - "..", - "..", - "test", - "e2e", - "openapi", - "github-openapi.trimmed.json", -); - -interface OpenApiDoc { - openapi: string; - info: unknown; - paths: Record; - [key: string]: unknown; -} - -const FETCH_TIMEOUT_MS = 60_000; - -async function fetchSpec(url: string): Promise { - const fetched = await fetchTextWithRetry("OpenAPI descriptor", url, FETCH_TIMEOUT_MS); - if (!fetched.ok) { - throw new Error( - `failed to fetch the OpenAPI descriptor: ${fetched.status} ${fetched.statusText} for ${url}. Check UPSTREAM_REF and the DEFAULT_API_VERSION file name`, - ); - } - let doc: OpenApiDoc; - try { - doc = JSON.parse(fetched.text) as OpenApiDoc; - } catch (error) { - throw new Error( - `the OpenAPI descriptor from ${url} is not valid JSON: ${error instanceof Error ? error.message : String(error)}. The download may be truncated; re-run`, - ); - } - if (!doc.paths || typeof doc.paths !== "object") { - throw new Error( - `the fetched descriptor has no "paths" object; got keys: ${Object.keys(doc).join(", ")}. Confirm SPEC_URL points at the dereferenced OpenAPI descriptor (.deref.json) for ${DEFAULT_API_VERSION}`, - ); - } - return doc; -} - -/** A partial deref upstream or a wrong file name would leave dangling $refs the disk-only validator cannot resolve: - * ajv would throw at compile time or silently skip a subschema. Caught here, at generation. */ -function assertRefFree(trimmed: OpenApiDoc): void { - const serialized = JSON.stringify(trimmed); - if (serialized.includes('"$ref"')) { - const matches = [...serialized.matchAll(/"\$ref":\s*"([^"]+)"/g)].slice(0, 5); - const sample = matches.map((m) => m[1]).join(", "); - throw new Error( - `the trimmed slice still contains $ref pointers (e.g. ${sample}); the descriptor was not fully dereferenced. Confirm SPEC_URL points at the .deref.json file, not the source spec`, - ); - } -} - -/** USED_PATHS spells templates as OpenAPI keys them ("/repos/{owner}/{repo}/labels"), so the match is exact string - * equality. An entry absent upstream is a hard error: the action calls a path GitHub does not document at this - * version, which the validator could never check. */ -function trimPaths(doc: OpenApiDoc): { trimmed: OpenApiDoc; kept: string[]; missing: string[] } { - const kept: string[] = []; - const missing: string[] = []; - const paths: Record = {}; - for (const path of USED_PATHS) { - const entry = doc.paths[path]; - if (entry === undefined) { - missing.push(path); - continue; - } - paths[path] = entry; - kept.push(path); - } - const trimmed: OpenApiDoc = { - openapi: doc.openapi, - info: doc.info, - ...(doc.servers ? { servers: doc.servers } : {}), - [SOURCE_KEY]: SPEC_URL, - paths, - }; - return { trimmed, kept, missing }; -} - -async function main(): Promise { - if (whenStale(process.argv)) { - const reason = specStaleness(readArtifact(OUT_PATH), SPEC_URL, USED_PATHS); - if (reason === null) { - console.log(`${OUT_PATH} is current (trimmed from ${SPEC_URL}); not fetching`); - return 0; - } - console.log(`regenerating ${OUT_PATH}: ${reason}`); - } - console.log(`fetching ${SPEC_URL}`); - const doc = await fetchSpec(SPEC_URL); - // An UNDOCUMENTED_PATHS entry exists precisely BECAUSE the descriptor lacks it, so the moment upstream documents - // one, the carve-out must go (and validation switch on). - const nowDocumented = UNDOCUMENTED_PATHS.filter((path) => doc.paths[path] !== undefined); - if (nowDocumented.length > 0) { - // On a probe run (overridden ref) the pinned descriptor may still lack the paths, so retiring the gap right away - // would break pinned runs: the pin must move first. - const remedy = - REF === UPSTREAM_REF - ? "Retire the owning gap in src/upstream-gaps/ (delete the spec-only file, " + - "or flip documentedInSpec to true on an octokit-kind one), " + - "regenerate the index (bun .github/scripts/gen-gaps-index.ts), and re-run, " + - "so the validator covers them" - : `The probe ref documents them but the pinned ${UPSTREAM_REF} may not: ` + - "bump UPSTREAM_REF in this script first, then retire the gap and regenerate the index"; - throw new Error( - `the upstream descriptor at ${REF} now documents: ${nowDocumented.join(", ")}. ${remedy}`, - ); - } - const { trimmed, kept, missing } = trimPaths(doc); - if (missing.length > 0) { - throw new Error( - `these USED_PATHS are not in the upstream descriptor at ${REF} for ${DEFAULT_API_VERSION}:\n ${missing.join("\n ")}\nEither the path is wrong in test/e2e/openapi/paths.ts, or UPSTREAM_REF/api-version needs updating`, - ); - } - assertRefFree(trimmed); - // Stable key order and a trailing newline, so re-runs are byte-identical. - const json = `${JSON.stringify(trimmed, null, 2)}\n`; - // Temp file then rename, so an aborted run leaves the previously written spec intact rather than a half-written - // file the validator would fail to parse. - const tmpPath = `${OUT_PATH}.tmp`; - writeFileSync(tmpPath, json); - renameSync(tmpPath, OUT_PATH); - const sizeKb = Math.round(Buffer.byteLength(json) / 1024); - console.log(`wrote ${OUT_PATH} (${kept.length} paths, ${sizeKb} KB)`); - if (REF !== UPSTREAM_REF) { - console.log( - `note: trimmed from TRIM_UPSTREAM_REF override "${REF}", not the pinned UPSTREAM_REF - do not cache this artifact as the pinned slice`, - ); - } - return 0; -} - -try { - process.exit(await main()); -} catch (error) { - console.error(error instanceof Error ? error.message : String(error)); - process.exit(1); -} diff --git a/.github/workflows/auto-fix.yml b/.github/workflows/auto-fix.yml index fb58fba1..cbf107dc 100644 --- a/.github/workflows/auto-fix.yml +++ b/.github/workflows/auto-fix.yml @@ -77,8 +77,6 @@ jobs: ref: ${{ github.event.pull_request.head.ref }} persist-credentials: false - uses: ./.github/actions/setup - # build:docs reads the fetched OpenAPI spec for the coverage page's endpoint links. - - uses: ./.github/actions/fetch-test-artifacts - name: Graduate upstream gaps octokit now ships shell: bash run: bun .github/scripts/graduate-upstream-gaps.ts diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index e787b637..72656720 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -16,9 +16,6 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup - # bun test loads the fetched, gitignored OpenAPI spec and GraphQL - # schema; the composite restores each from cache or fetches on a miss. - - uses: ./.github/actions/fetch-test-artifacts - name: Lint (biome) run: bun run lint - name: Lint (architecture) @@ -70,8 +67,6 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup - # The docs generator reads the fetched OpenAPI spec for the coverage page's endpoint links. - - uses: ./.github/actions/fetch-test-artifacts - name: Regenerate the schema and compare run: bun run build:check @@ -151,10 +146,6 @@ jobs: fi echo "sections=$SECTIONS" >> "$GITHUB_OUTPUT" echo "selected sections: $SECTIONS" - # The harness validates mock responses against the fetched, gitignored spec; the composite restores or fetches - # it, skipped with the run when the selector picks nothing. - - uses: ./.github/actions/fetch-test-artifacts - if: steps.select.outputs.sections != 'none' - name: Run scenarios and fuzz if: steps.select.outputs.sections != 'none' env: @@ -182,8 +173,7 @@ jobs: - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: 24 - # Only the selector's "none" answer matters here. It runs BEFORE the spec cache/fetch so a PR that selects - # nothing skips the download too. + # Only the selector's "none" answer matters here. - name: Check whether the diff can affect route coverage id: select env: @@ -196,8 +186,6 @@ jobs: fi echo "sections=$SECTIONS" >> "$GITHUB_OUTPUT" echo "selected sections: $SECTIONS" - - uses: ./.github/actions/fetch-test-artifacts - if: steps.select.outputs.sections != 'none' - name: Endpoint-coverage tripwire if: steps.select.outputs.sections != 'none' run: bun .github/scripts/check-endpoint-coverage.ts diff --git a/.github/workflows/nightly-fuzz.yml b/.github/workflows/nightly-fuzz.yml index 8c66103b..c3808de5 100644 --- a/.github/workflows/nightly-fuzz.yml +++ b/.github/workflows/nightly-fuzz.yml @@ -57,12 +57,6 @@ jobs: with: node-version: 24 - # The trimmed OpenAPI spec is a fetched, gitignored artifact the mock - # validates responses against. Always fetched fresh (no cache), same as - # nightly.yml's e2e job: the fetch doubles as an upstream-drift tripwire. - - name: Fetch the trimmed OpenAPI spec - run: bun .github/scripts/trim-openapi.ts - - name: Fuzz env: SEED: ${{ inputs.seed }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4583b583..47660015 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -1,15 +1,13 @@ # Nightly upstream-freshness probes and the whole-corpus e2e run. Generated # from the fleet sync's nightly starter and never overwritten by a later -# sync. The checks job re-cuts the trimmed OpenAPI spec from upstream HEAD -# (a path-level staleness check) and typechecks against the latest -# @octokit/types, so pinned-dependency staleness fails a night instead of -# surprising the next manual bump; the float-canary job re-resolves the -# lockfile from scratch the way the Dependabot lockfile workflow does, so a -# breaking upstream release fails a night instead of reddening every open -# Dependabot PR; the e2e job runs the curated corpus and the endpoint-coverage -# tripwire against a freshly fetched spec; the report job below files or -# updates one label-deduplicated tracking issue on a red night and closes it -# on a green one (see the platform repository's docs/nightly.md). +# sync. +# +# Each job fails a night instead of surprising a later PR: checks runs the +# schema lockstep tests and the typecheck against the latest Octokit +# packages, float-canary re-resolves the lockfile from scratch like the +# Dependabot lockfile workflow, and e2e runs the curated corpus with the +# endpoint-coverage tripwire. The report job files one tracking issue on a +# red night and closes it on a green one (the platform's docs/nightly.md). name: Nightly on: @@ -38,44 +36,31 @@ jobs: - uses: ./.github/actions/setup id: setup - # PR CI proves USED_PATHS against the pinned descriptor; this probe - # re-cuts against upstream HEAD (deliberately bypassing the cached - # spec), so trim-openapi's own errors become the freshness signal: a - # newly documented UNDOCUMENTED_PATHS entry or a removed/renamed used - # path fails the night. - - name: Probe the latest OpenAPI descriptor - env: - GH_TOKEN: ${{ github.token }} + # The lockstep tests PR CI runs against the pinned @octokit/openapi and + # @octokit/graphql-schema, here against each package's latest release: + # a used path removed, an upstream gap now documented, or a selected + # field retired fails the night before Dependabot's bump PR does. + # --no-save leaves package.json and the lockfile untouched; the upgraded + # node_modules stays ambient for the rest of this job. + - name: Probe the latest OpenAPI descriptor and GraphQL schema run: | - # Assign and export separately: `export X="$(cmd)"` takes export's - # exit status, so a failed gh would run the trim with an empty ref. - TRIM_UPSTREAM_REF="$(gh api repos/github/rest-api-description/commits/main --jq .sha)" - export TRIM_UPSTREAM_REF - if ! printf '%s' "$TRIM_UPSTREAM_REF" | grep -Eq '^[0-9a-f]{40}$'; then - echo "::error::could not resolve the latest rest-api-description commit; got '$TRIM_UPSTREAM_REF'" - exit 1 - fi - log="$(mktemp)" - if ! bun .github/scripts/trim-openapi.ts >"$log" 2>&1; then - cat "$log" - exit 1 - fi - cat "$log" - # The script's "fetching " line embeds the ref it actually - # used; anything else in the log (like the override note) also - # naming the sha must not satisfy this. A pinned ref here means - # the override was ignored - which would make this probe pass - # green forever without probing anything. - if ! grep "^fetching " "$log" | grep -qF "$TRIM_UPSTREAM_REF"; then - echo "::error::trim-openapi.ts did not fetch $TRIM_UPSTREAM_REF; its TRIM_UPSTREAM_REF override is gone and this probe is testing nothing" - exit 1 - fi + bun add --no-save --ignore-scripts @octokit/openapi@latest @octokit/graphql-schema@latest + # A pinned version left in place would make this probe pass green + # forever without probing anything. + for pkg in @octokit/openapi @octokit/graphql-schema; do + installed="$(bun -e "console.log(require('$pkg/package.json').version)")" + latest="$(bun info "$pkg" version)" + if [ "$installed" != "$latest" ]; then + echo "::error::$pkg@$installed is installed but $latest is the latest; the probe is testing the pin" + exit 1 + fi + echo "$pkg@$installed" + done + bun test test/e2e/openapi/validate.test.ts test/sections/graphql-queries.test.ts - # always() so a red spec probe cannot mask this one (the job still + # always() so a red schema probe cannot mask this one (the job still # fails if either step failed); gated on the setup step because there - # is nothing to probe when setup itself broke. --no-save keeps the - # lockfile and package.json untouched; the upgraded node_modules stays - # ambient for the rest of this job, so keep later steps out of it. + # is nothing to probe when setup itself broke. - name: Probe the latest @octokit/types if: ${{ !cancelled() && steps.setup.outcome == 'success' }} run: | @@ -125,15 +110,11 @@ jobs: bun install --ignore-scripts git --no-pager diff --stat -- bun.lock git checkout -- bun.lock - # The gate's suite loads two fetched artifacts; the composite restores or fetches - # each. It runs AFTER the floated install so a miss runs the fetch on the floated graph. - - uses: ./.github/actions/fetch-test-artifacts - name: Run the gate on the floated resolution run: bun run check - # The whole curated corpus (PR CI runs only the sections a PR changed) plus the endpoint-coverage tripwire, - # against the pinned spec fetched fresh: PR CI restores a cached spec for speed, so this fetch is where a used - # path GitHub stopped documenting surfaces. The bundle runs under node24, so node is installed beside bun. + # The whole curated corpus (PR CI runs only the sections a PR changed) plus the endpoint-coverage tripwire. + # The bundle runs under node24, so node is installed beside bun. e2e: runs-on: ubuntu-latest timeout-minutes: 15 @@ -143,8 +124,6 @@ jobs: - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: 24 - - name: Fetch the trimmed OpenAPI spec (drift tripwire) - run: bun .github/scripts/trim-openapi.ts - name: Curated corpus run: bun test/e2e/run.ts - name: Endpoint-coverage tripwire diff --git a/.github/workflows/update-release.yml b/.github/workflows/update-release.yml index dc216cca..de8de14d 100644 --- a/.github/workflows/update-release.yml +++ b/.github/workflows/update-release.yml @@ -68,8 +68,6 @@ jobs: # new version tag names the build tag by the commit's position; a shallow checkout can do none of that. fetch-depth: 0 - uses: ./.github/actions/setup - # build:docs reads the fetched OpenAPI spec for the coverage page's endpoint links. - - uses: ./.github/actions/fetch-test-artifacts - name: Build the bundle and the schema run: | bun run build diff --git a/.gitignore b/.gitignore index 1c96b73d..9fe94bc1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,4 @@ test/e2e/.artifacts/ -# The trimmed GitHub OpenAPI spec is a FETCHED artifact, not committed: it is a -# ~2MB generated blob that would bloat history on every ref bump. Generate it -# with `bun .github/scripts/trim-openapi.ts` (reproducible at the pinned sha); -# CI restores it from actions/cache or re-fetches on a miss. -test/e2e/openapi/github-openapi.trimmed.json -# GitHub's public GraphQL schema, the same fetched-artifact arrangement: -# generate it with `bun .github/scripts/fetch-graphql-schema.ts`. -test/e2e/graphql/schema.docs.graphql # Agent session state, the whole directory (the managed region below ignores # only named paths inside it); stray biome.json copies inside its worktrees # otherwise abort `biome ci .` as nested roots. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 35485ec8..18a3ef49 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ The fleet-wide conventions - Conventional Commit titles, squash merges, the `all ## Toolchain - `src/` is TypeScript built with [bun](https://bun.com). The scripts in `package.json` are the commands; `bun run check` is the whole local gate. -- `bun run test`, `bun run test:e2e`, and `bun run fuzz` start with `bun run test:artifacts`, which fetches the two gitignored test artifacts (the trimmed OpenAPI spec, the GraphQL schema) when one is absent or was fetched from a URL other than the one its script builds (the pinned ref, and the API version for the spec). A fresh checkout fetches once (a few seconds); a current file costs no network. CI restores the same files from cache and then runs the same command. +- GitHub's OpenAPI descriptor and GraphQL schema come from the `@octokit/openapi` and `@octokit/graphql-schema` devDependencies, so no test or generator touches the network. Dependabot moves the pins; a bump that stops documenting a path the action calls, starts documenting an upstream gap, or retires a field a query selects fails the schema tests on that PR by name. - Committed generated output is the table in `.github/scripts/generated.ts`: `lib/settings.schema.json`, `src/upstream-gaps/index.ts`, and the generated regions of `action.yml` and the docs pages. - `bun run build:check` regenerates every table entry and fails on drift. - `lib/index.js` (the action bundle) and `lib/pkg/` (the npm library) are built where they are needed and never committed on `main`. Every runtime dependency is compiled into them. diff --git a/bun.lock b/bun.lock index 279aacdf..14973795 100644 --- a/bun.lock +++ b/bun.lock @@ -28,6 +28,8 @@ "@actions/core": "3.0.1", "@arethetypeswrong/cli": "0.18.5", "@biomejs/biome": "2.5.11", + "@octokit/graphql-schema": "15.26.1", + "@octokit/openapi": "23.0.2", "@octokit/openapi-webhooks": "12.1.0", "@types/bun": "1.4.0", "@types/node": "26.4.1", @@ -146,6 +148,10 @@ "@octokit/graphql": ["@octokit/graphql@9.0.5", "", { "dependencies": { "@octokit/request": "^10.0.16", "@octokit/types": "^18.0.0", "universal-user-agent": "^7.0.0" } }, "sha512-bt/hm03LeU6Vy7FwTrkkC9p3XGT/lBwClglMqxBSe5/q0E5CdJTXeAqEI0vlw89/LF/G6tryTIH8HirZ3prMVg=="], + "@octokit/graphql-schema": ["@octokit/graphql-schema@15.26.1", "", { "dependencies": { "graphql": "^16.0.0", "graphql-tag": "^2.10.3" } }, "sha512-RFDC2MpRBd4AxSRvUeBIVeBU7ojN/SxDfALUd7iVYOSeEK3gZaqR2MGOysj4Zh2xj2RY5fQAUT+Oqq7hWTraMA=="], + + "@octokit/openapi": ["@octokit/openapi@23.0.2", "", {}, "sha512-pV8M7L9GY23AybNvTmo2nyjmpmnt6+2sRE/tqr0ZLQcPS4lnw7u5eZGNmwRNkBC3D7gZXbFx5AHzLUVRBXGDhg=="], + "@octokit/openapi-types": ["@octokit/openapi-types@29.0.1", "", {}, "sha512-9qWOMFNxxLokERcms42rU0PTLqQmVs7g5E41TI4mCOxmpFayD1rfC7XxOL55cG9MBZLFlC31BrR37myMKardwg=="], "@octokit/openapi-webhooks": ["@octokit/openapi-webhooks@12.1.0", "", {}, "sha512-Z5eDsLCUNbx1zHpZb1LNQirADqdMTvWJQ/deahi0ws+tWWnwHXB+Pr4jfmM9OrETS9Y5iW8ZRu1fdbudObhLrg=="], @@ -546,6 +552,8 @@ "graphql": ["graphql@17.0.2", "", {}, "sha512-FRWbddMxfkjiB7z+aQDWIR+E34xo9I8c9mtK2RPv8PmMzKRvrdsreHL/Ui/TmwHJfhHChEtsFPyMHKI+xuarQQ=="], + "graphql-tag": ["graphql-tag@2.12.7", "", { "dependencies": { "tslib": "^2.1.0" }, "peerDependencies": { "graphql": "^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0 || ^16.0.0 || ^17.0.0" } }, "sha512-xnE/NFzy+0eIesvAsREJZ284zTl/wYuBAvpsFSDhRGRdRHdnE90M21Q3xAWyYInb0J756c6x0pIQ62+vtvOs1Q=="], + "has-flag": ["has-flag@4.0.0", "", {}, "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ=="], "highlight.js": ["highlight.js@10.7.3", "", {}, "sha512-tzcUFauisWKNHaRkN4Wjl/ZA07gENAjFl3J/c480dprkGTg5EQstgaNFqBfUqCq54kZRIEcreTsAgF/m2quD7A=="], @@ -816,6 +824,8 @@ "@noble/post-quantum/@noble/hashes": ["@noble/hashes@2.0.1", "", {}, "sha512-XlOlEbQcE9fmuXxrVTXCTlG2nlRXa9Rj3rr5Ue/+tX+nmkgbX720YHh0VR3hBF9xDvwnb8D2shVGOwNx+ulArw=="], + "@octokit/graphql-schema/graphql": ["graphql@16.14.2", "", {}, "sha512-Chq1s4CY7jmh8gO2qvLIJyfCDIN+EHLFW/9iShnp1z8FjBQMoodWP1kDC36VAMXXIvAjj4ARa7ntfAV2BrjsbA=="], + "@octokit/plugin-paginate-rest/@octokit/types": ["@octokit/types@16.0.0", "", { "dependencies": { "@octokit/openapi-types": "^27.0.0" } }, "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg=="], "@octokit/plugin-rest-endpoint-methods/@octokit/types": ["@octokit/types@16.0.0", "", { "dependencies": { "@octokit/openapi-types": "^27.0.0" } }, "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg=="], diff --git a/package.json b/package.json index d45cb49a..5694a104 100644 --- a/package.json +++ b/package.json @@ -47,17 +47,16 @@ "lint:package": "bun x publint --strict && bun x attw --pack . --profile esm-only", "typecheck": "bun x tsc -p .", "knip": "knip", - "test": "bun run test:artifacts && bun test", - "test:artifacts": "bun .github/scripts/trim-openapi.ts --when-stale && bun .github/scripts/fetch-graphql-schema.ts --when-stale", - "test:e2e": "bun run test:artifacts && bun test/e2e/run.ts", - "fuzz": "bun run test:artifacts && bun test/e2e/fuzz.ts", + "test": "bun test", + "test:e2e": "bun test/e2e/run.ts", + "fuzz": "bun test/e2e/fuzz.ts", "build": "bun run build:bundle && bun run build:lib && bun run build:schema && bun run build:docs && bun run build:action-docs", "build:bundle": "bun build src/main.ts --target=node --outfile lib/index.js", "build:lib": "bun x tsdown", "build:schema": "bun .github/scripts/gen-settings-schema.ts", - "build:docs": "bun run test:artifacts && bun .github/scripts/gen-docs.ts", + "build:docs": "bun .github/scripts/gen-docs.ts", "build:action-docs": "bun .github/scripts/gen-action-docs.ts", - "build:check": "bun run test:artifacts && bun .github/scripts/generated.ts", + "build:check": "bun .github/scripts/generated.ts", "prepare": "lefthook install || true" }, "dependencies": { @@ -84,6 +83,8 @@ "@actions/core": "3.0.1", "@arethetypeswrong/cli": "0.18.5", "@biomejs/biome": "2.5.11", + "@octokit/graphql-schema": "15.26.1", + "@octokit/openapi": "23.0.2", "@octokit/openapi-webhooks": "12.1.0", "@types/bun": "1.4.0", "@types/node": "26.4.1", diff --git a/src/sections/shared/roles.ts b/src/sections/shared/roles.ts index 16067720..55e6741f 100644 --- a/src/sections/shared/roles.ts +++ b/src/sections/shared/roles.ts @@ -92,7 +92,7 @@ export const PermissionSchema = z.string().regex(PERMISSION_PATTERN, { /** * The GET reports this enum and the PATCH accepts nothing else, so a declared custom org role can never be * verified on (or set on) a pending invitation; the PUT applies it once accepted. The e2e mock's stored - * invitations must stay inside it; a lockstep test pins it to the trimmed OpenAPI spec. + * invitations must stay inside it; a lockstep test pins it to GitHub's OpenAPI descriptor. */ export const INVITATION_ROLES: ReadonlySet = new Set([ "read", diff --git a/src/upstream-gaps/gap.ts b/src/upstream-gaps/gap.ts index 03992ce8..f5c45a43 100644 --- a/src/upstream-gaps/gap.ts +++ b/src/upstream-gaps/gap.ts @@ -11,23 +11,37 @@ export interface OctokitGap { readonly kind: "octokit"; readonly routes: readonly [R, ...R[]]; /** - * False only for features GitHub's OpenAPI descriptor ALSO lags; those are excluded from the trimmed - * spec and exempted from the e2e unknown-route check via UNDOCUMENTED_ROUTES. + * False only for features GitHub's OpenAPI descriptor ALSO lags; those are excluded from the descriptor + * slice the e2e validator loads and exempted from its unknown-route check via UNDOCUMENTED_ROUTES. */ readonly documentedInSpec: boolean; } /** - * Routes octokit HAS typed but the pinned OpenAPI descriptor lacks: no tripwire, only the - * UNDOCUMENTED_ROUTES exemption. Graduates by hand: bump UPSTREAM_REF in trim-openapi.ts, delete the - * file, regenerate the index and the trimmed spec. + * Routes octokit HAS typed but the pinned @octokit/openapi descriptor lacks: no tripwire, only the + * UNDOCUMENTED_ROUTES exemption. Graduates by hand once a Dependabot bump documents the route (the e2e + * validator's load fails naming it): delete the file and regenerate the index. */ export interface SpecOnlyGap { readonly kind: "spec-only"; readonly routes: readonly [R, ...R[]]; } -export type UpstreamGap = OctokitGap | SpecOnlyGap; +/** + * SDL GitHub's GraphQL API serves but the pinned @octokit/graphql-schema release lacks (new types, `extend type`, + * `extend input`). No tripwire type can see a schema: the lockstep test extends the package's schema with each + * gap's SDL, and graphql-js refuses the extension once the package ships a declared type or field. Graduates by + * hand: delete the file and regenerate the index. + */ +export interface GraphqlSchemaGap { + readonly kind: "graphql-schema"; + readonly sdl: string; +} + +export type UpstreamGap = + | OctokitGap + | SpecOnlyGap + | GraphqlSchemaGap; /** `const G` preserves the routes tuple's literals, so index.ts derives SupplementalRoute as a literal union. */ export function defineGap>( @@ -46,14 +60,36 @@ export function defineSpecOnlyGap>( + gap: G, +): G & { readonly kind: "graphql-schema" } { + return { ...gap, kind: "graphql-schema" }; +} + +/** The gaps keyed by file base name ("lfs" for lfs.ts), as the generated index spells them. */ +export type GapsByFile = Readonly>>; + /** * Generic over the caller's route union so the result keeps its literal typing WITHOUT a cast, and total - * on an empty gaps list, where an inline flatMap over the literal tuple would stop compiling. + * on an empty gaps record, where an inline flatMap over the literal values would stop compiling. */ -export function undocumentedRoutes( - gaps: readonly UpstreamGap[], -): readonly R[] { - return gaps.flatMap((gap) => - gap.kind === "spec-only" || !gap.documentedInSpec ? gap.routes : [], +export function undocumentedRoutes(gaps: GapsByFile): readonly R[] { + return Object.values(gaps).flatMap((gap) => { + if (gap.kind === "graphql-schema") { + return []; + } + return gap.kind === "spec-only" || !gap.documentedInSpec ? gap.routes : []; + }); +} + +/** A graphql-schema gap's SDL with the file that carries it, so a refused extension names the file to retire. */ +export interface UnshippedGraphqlSdl { + readonly file: string; + readonly sdl: string; +} + +export function unshippedGraphqlSdl(gaps: GapsByFile): readonly UnshippedGraphqlSdl[] { + return Object.entries(gaps).flatMap(([base, gap]) => + gap.kind === "graphql-schema" ? [{ file: `src/upstream-gaps/${base}.ts`, sdl: gap.sdl }] : [], ); } diff --git a/src/upstream-gaps/index.ts b/src/upstream-gaps/index.ts index 98b16a0e..f646d1e7 100644 --- a/src/upstream-gaps/index.ts +++ b/src/upstream-gaps/index.ts @@ -1,19 +1,21 @@ /** * GENERATED by gen-gaps-index.ts - do not edit. Every pending upstream gap, - * aggregated in sorted file order; regenerate with + * keyed by file base name in sorted 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. */ -import { undocumentedRoutes } from "./gap.js"; +import { undocumentedRoutes, type UnshippedGraphqlSdl, unshippedGraphqlSdl } from "./gap.js"; +import { GAP as issueCreationPolicy } from "./issue-creation-policy.js"; import { GAP as lfs } from "./lfs.js"; -const GAPS = [ +const GAPS = { + "issue-creation-policy": issueCreationPolicy, lfs, -] as const; +} as const; -type GapUnion = (typeof GAPS)[number]; +type GapUnion = (typeof GAPS)[keyof typeof GAPS]; /** * Routes GitHub documents but the pinned @octokit/types release does not @@ -31,8 +33,16 @@ 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. + * excludes their paths from the descriptor slice the e2e validator loads + * and exempts exactly these METHOD+path pairs from its unknown-route check. */ export const UNDOCUMENTED_ROUTES: readonly (SupplementalRoute | SpecOnlyRoute)[] = undocumentedRoutes(GAPS); + +/** + * 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 + * with it before validating the declared queries. + */ +export const UNSHIPPED_GRAPHQL_SDL: readonly UnshippedGraphqlSdl[] = unshippedGraphqlSdl(GAPS); diff --git a/src/upstream-gaps/issue-creation-policy.ts b/src/upstream-gaps/issue-creation-policy.ts new file mode 100644 index 00000000..ea624d7d --- /dev/null +++ b/src/upstream-gaps/issue-creation-policy.ts @@ -0,0 +1,17 @@ +import { defineGraphqlSchemaGap } from "./gap.js"; + +/** GitHub shipped the repository issue-creation policy; the pinned @octokit/graphql-schema release predates it. */ +export const GAP = defineGraphqlSchemaGap({ + sdl: ` + enum IssueCreationPolicy { + ALL + COLLABORATORS_ONLY + } + extend type Repository { + issueCreationPolicy: IssueCreationPolicy + } + extend input UpdateRepositoryInput { + issueCreationPolicy: IssueCreationPolicy + } + `, +}); diff --git a/test/docs/checks-workflow.test.ts b/test/docs/checks-workflow.test.ts index 1e0d3cb5..22c64013 100644 --- a/test/docs/checks-workflow.test.ts +++ b/test/docs/checks-workflow.test.ts @@ -1,201 +1,13 @@ /** - * checks.yml and the fetch-test-artifacts composite against the code they run: a cache key that hashes every input its artifact depends on - * (a stale restore would test against yesterday's spec with no failure anywhere), and head_ref conditions that spell the release PR - * branch prefix the pipeline script owns (a drifted spelling skips the anchor-check on every release PR instead of failing there). - * - * The cache-key test catches ACCIDENTAL omissions: an input the trim script imports that no hashFiles argument names. Deliberately - * hiding an input behind expression syntax is out of scope. + * checks.yml against the code it runs: head_ref conditions that spell the release PR branch prefix the pipeline + * script owns (a drifted spelling skips the anchor-check on every release PR instead of failing there). */ import { describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join, relative, resolve } from "node:path"; import { parse as parseYaml } from "yaml"; import { RELEASE_PR_BRANCH_PREFIX } from "../../.github/scripts/release-pipeline.js"; -import { ROOT } from "../root.js"; import { headRefPrefixes, headRefPrefixesIn } from "./head-ref.js"; -import { readAction, type Step, type Workflow, workflowText } from "./workflow-loader.js"; - -const COMPOSITE_DIR = ".github/actions/fetch-test-artifacts"; -const PATHS_TS = "test/e2e/openapi/paths.ts"; -const TRIM_TS = ".github/scripts/trim-openapi.ts"; -const FETCH_GRAPHQL_TS = ".github/scripts/fetch-graphql-schema.ts"; - -/** A fetched, gitignored test artifact the composite restores from its cache. */ -interface FetchedArtifact { - label: string; - path: string; - /** Every source the fetched output depends on; the cache key must hash each. */ - hashInputs: () => string[]; -} -const FETCHED_ARTIFACTS: readonly FetchedArtifact[] = [ - { - label: "trimmed OpenAPI spec", - path: "test/e2e/openapi/github-openapi.trimmed.json", - // Every import of the scripts: the paths and the API version trimmed, and the fetch helper the bytes come through. - hashInputs: () => [ - TRIM_TS, - PATHS_TS, - ...relativeImportsOf(TRIM_TS), - ...relativeImportsOf(PATHS_TS), - ], - }, - { - label: "GraphQL schema", - path: "test/e2e/graphql/schema.docs.graphql", - hashInputs: () => [FETCH_GRAPHQL_TS, ...relativeImportsOf(FETCH_GRAPHQL_TS)], - }, -]; -const [OPENAPI, GRAPHQL] = FETCHED_ARTIFACTS as [FetchedArtifact, FetchedArtifact]; - -/** The composite's one actions/cache step restoring exactly `path`; zero or several is a broken composite, never a skip. */ -function cacheStepFor(path: string): Step { - const steps = readAction(COMPOSITE_DIR).runs.steps ?? []; - const found = steps.filter( - (step) => (step.uses ?? "").startsWith("actions/cache@") && step.with?.path === path, - ); - expect( - found.length, - `${COMPOSITE_DIR} must cache ${path} in exactly one step, found ${found.length}`, - ).toBe(1); - return found[0] as Step; -} - -/** The key of an artifact cache step; anything but a string key is a broken cache, never a skip. */ -function cacheKeyOf(step: Step, path: string): string { - const key = step.with?.key; - expect( - typeof key, - `the cache step for ${path} has a non-string key: ${JSON.stringify(key)}`, - ).toBe("string"); - return key as string; -} - -/** Every path pattern any hashFiles(...) call in the key names, in order; a key with no call is a constant and fails here. */ -function hashFilesPatterns(key: string): string[] { - const calls = [...key.matchAll(/hashFiles\(([^)]*)\)/g)]; - expect(calls.length, `cache key has no hashFiles call: ${key}`).toBeGreaterThan(0); - return calls.flatMap((call) => - (call[1] ?? "") - .split(",") - .map((arg) => arg.trim().replace(/^'|'$/g, "")) - .filter(Boolean), - ); -} - -/** - * The repository .ts files `file` imports. Single-line static imports and literal `import()`/`require()` calls are recognized; any other - * import-ish line fails, so an unsupported form extends this parser instead of being skipped. - */ -function relativeImportsOf(file: string): string[] { - const source = readFileSync(join(ROOT, file), "utf8"); - const specifiers: string[] = []; - for (const line of source.split("\n")) { - const openers = [...line.matchAll(/\b(?:import|require)\s*\(/g)].length; - if (openers === 0 && !/^\s*import[\s{"]/.test(line)) { - continue; - } - // Every call on the line is read and must close on it with a quoted literal; a call left open (a multiline argument) or a - // non-literal argument is the unsupported form. - const calls = [...line.matchAll(/\b(?:import|require)\s*\(([^)]*)\)/g)]; - const found = - openers > 0 - ? calls.length === openers - ? calls.map((call) => (call[1] ?? "").trim().match(/^(["'])([^"']+)\1$/)?.[2] ?? null) - : [null] - : [line.match(/^import [^"]*from "([^"]+)";$/)?.[1] ?? null]; - expect( - found.every((spec) => spec !== null), - `unrecognized import form in ${file}: "${line.trim()}" - teach relativeImportsOf() to parse it`, - ).toBe(true); - specifiers.push(...found.map((spec) => spec ?? "")); - } - return specifiers - .filter((spec) => spec.startsWith(".")) - .map((spec) => - relative(ROOT, resolve(ROOT, file, "..", spec)) - .split("\\") - .join("/") - .replace(/\.js$/, ".ts"), - ); -} - -/** True when a file is named by the pattern list, directly or via a ** glob. */ -function covered(patterns: string[], file: string): boolean { - if (patterns.includes(file)) { - return true; - } - return patterns.some( - (pattern) => pattern.endsWith("/**") && file.startsWith(pattern.slice(0, -2)), - ); -} - -/** The key's hashFiles list names at least one pattern and covers every input the artifact depends on. */ -function expectKeyHashesInputs(key: string, artifact: FetchedArtifact): void { - const patterns = hashFilesPatterns(key); - expect(patterns.length, `the ${artifact.label} cache key hashes nothing: ${key}`).toBeGreaterThan( - 0, - ); - for (const file of artifact.hashInputs()) { - expect( - covered(patterns, file), - `${file} changes the ${artifact.label} but its cache key does not hash it`, - ).toBe(true); - } -} - -describe("the fetch-test-artifacts cache keys", () => { - const keyOf = (artifact: FetchedArtifact) => - cacheKeyOf(cacheStepFor(artifact.path), artifact.path); - - test("each key hashes every input its artifact depends on", () => { - // The import walk found the scripts' own imports, so the coverage below is not vacuous. - expect(OPENAPI.hashInputs().length).toBeGreaterThan(2); - expect(OPENAPI.hashInputs()).toEqual( - expect.arrayContaining(["src/github/api.ts", ".github/scripts/lib/fetch-retry.ts"]), - ); - expect(GRAPHQL.hashInputs()).toContain(".github/scripts/lib/fetch-retry.ts"); - // Every call in the key contributes, wherever the expression puts it. - expect( - hashFilesPatterns( - `k-\${{ format('{0}', hashFiles('a.ts', 'b/**')) }}-\${{ hashFiles('c.ts') }}`, - ), - ).toEqual(["a.ts", "b/**", "c.ts"]); - for (const artifact of FETCHED_ARTIFACTS) { - expectKeyHashesInputs(keyOf(artifact), artifact); - } - }); - - /** A GraphQL schema key whose expression is `call`. */ - const keyed = (call: string) => `graphql-schema-\${{ ${call} }}`; - - test.each<[string, string, RegExp]>([ - ["a key hashing nothing", keyed("hashFiles()"), /GraphQL schema cache key hashes nothing/], - [ - "a key hashing an unrelated file", - keyed("hashFiles('package.json')"), - /fetch-graphql-schema\.ts changes the GraphQL schema but its cache key does not hash it/, - ], - ["a key without hashFiles", "graphql-schema-v1", /cache key has no hashFiles call/], - ])("%s fails the guard (negative control)", (_, key, message) => { - expect(() => expectKeyHashesInputs(key, GRAPHQL)).toThrow(message); - }); - - test("every hashFiles pattern of every key matches at least one file on disk", () => { - // hashFiles() silently skips a pattern that matches nothing (a moved input), so the key would stop changing with it while the coverage test still - // sees the stale pattern string. - for (const artifact of FETCHED_ARTIFACTS) { - for (const pattern of hashFilesPatterns(keyOf(artifact))) { - // dot: true because the scripts live under .github/, which the glob scanner skips by default (hashFiles itself does not). - const matches = [...new Bun.Glob(pattern).scanSync({ cwd: ROOT, dot: true })]; - expect( - matches.length, - `hashFiles pattern '${pattern}' matches no file on disk, so it contributes nothing to the cache key`, - ).toBeGreaterThan(0); - } - } - }); -}); +import { type Workflow, workflowText } from "./workflow-loader.js"; /** The guard: one anchor-check step, its job gated on the constant, and no job or step condition spelling it otherwise. */ function expectReleasePrefixes(wf: Workflow): void { diff --git a/test/docs/workflow-loader.ts b/test/docs/workflow-loader.ts index 41fb8833..bbd7cee9 100644 --- a/test/docs/workflow-loader.ts +++ b/test/docs/workflow-loader.ts @@ -1,6 +1,5 @@ /** - * One reader for the workflow and composite-action YAML the docs tests pin: the parsed shapes and the setup composite - * every repo-owned job goes through. + * One reader for the workflow YAML the docs tests pin, and the parsed shapes. */ import { readFileSync } from "node:fs"; @@ -52,10 +51,6 @@ export interface Workflow { env?: Record; jobs: Record; } -interface CompositeAction { - runs: { using?: string; steps?: Step[] }; -} - export function workflowText(file: string): string { return readFileSync(join(WORKFLOWS_DIR, file), "utf8"); } @@ -63,8 +58,3 @@ export function workflowText(file: string): string { export function readWorkflow(file: string): Workflow { return parseYaml(workflowText(file)) as Workflow; } - -/** `dir` is relative to the repository root, e.g. `.github/actions/setup`. */ -export function readAction(dir: string): CompositeAction { - return parseYaml(readFileSync(join(ROOT, dir, "action.yml"), "utf8")) as CompositeAction; -} diff --git a/test/e2e/mock/request-body.test.ts b/test/e2e/mock/request-body.test.ts index 4c22655a..309891c4 100644 --- a/test/e2e/mock/request-body.test.ts +++ b/test/e2e/mock/request-body.test.ts @@ -1,5 +1,5 @@ /** - * The pipeline hands a handler only what GitHub keeps of a body: the fields the trimmed spec documents. + * The pipeline hands a handler only what GitHub keeps of a body: the fields the descriptor documents. * An undocumented key on an open body is dropped (GitHub ignores it; the GET never echoes it), on a * closed body it is GitHub's 422. Pinned over the wire, since a handler unit test bypasses the pipeline. */ diff --git a/test/e2e/mock/request-body.ts b/test/e2e/mock/request-body.ts index 1cc76d0e..a5d23215 100644 --- a/test/e2e/mock/request-body.ts +++ b/test/e2e/mock/request-body.ts @@ -1,6 +1,6 @@ /** * What GitHub keeps of a request body, decided once for every section route before its handler runs: - * the fields the trimmed spec documents. api.github.com ignores an unknown key on an open body (the + * the fields the descriptor documents. api.github.com ignores an unknown key on an open body (the * GET never echoes it, so a misspelled setting never converges) and answers a 422 on a closed one; a * handler that stored the body verbatim would hide both from every scenario. */ diff --git a/test/e2e/mock/support.ts b/test/e2e/mock/support.ts index 95473048..8167db13 100644 --- a/test/e2e/mock/support.ts +++ b/test/e2e/mock/support.ts @@ -558,7 +558,7 @@ export function storedKeyMaterial(key: string): string { /** * Mock-only realism: the runtime never consults this list (an unknown rules[].type passes through verbatim); * it exists so a typo'd type answers GitHub's real 422 shape instead of being stored silently. A lockstep test - * (openapi/validate.test.ts) pins it to the trimmed spec's rules[].type enums, and rulesets-schema.test.ts + * (openapi/validate.test.ts) pins it to the descriptor's rules[].type enums, and rulesets-schema.test.ts * pins the schema's KNOWN_RULE_TYPES to it. */ export const RULESET_RULE_TYPES = new Set([ diff --git a/test/e2e/openapi/paths.ts b/test/e2e/openapi/paths.ts index c9e5a64f..9de95a29 100644 --- a/test/e2e/openapi/paths.ts +++ b/test/e2e/openapi/paths.ts @@ -1,7 +1,7 @@ /** - * Every REST path template the action can reach, derived from the endpoint declarations. The trim - * script (.github/scripts/trim-openapi.ts) imports USED_PATHS to slice the published spec down to - * what the mock must model, so this file stays dependency-light and re-derives nothing. + * Every REST path template the action can reach, derived from the endpoint declarations. The + * validator (validate.ts) cuts the published descriptor down to USED_PATHS, what the mock must model, + * so this file stays dependency-light and re-derives nothing. */ import { ISSUE_REPORT_ENDPOINTS } from "../../../src/report/issue-report.js"; @@ -27,9 +27,9 @@ const CORE_PATHS: readonly string[] = [ /** * Real endpoints GitHub's descriptor does not document (src/upstream-gaps/ holds them), kept out of USED_PATHS so - * trim-openapi does not hard-error while the e2e validator exempts the exact METHOD+path pairs. Staleness fails in both directions: + * the validator's load does not hard-error while it exempts the exact METHOD+path pairs. Staleness fails in both directions: * an entry is no longer a declared endpoint path -> excludeUndocumented() throws - * upstream starts documenting one -> trim-openapi.ts errors; retire the gap file + * a package bump documents one -> loadSpec() errors; retire the gap file */ export const UNDOCUMENTED_PATHS: readonly string[] = [ ...new Set(UNDOCUMENTED_ROUTES.map(endpointPath)), diff --git a/test/e2e/openapi/validate.test.ts b/test/e2e/openapi/validate.test.ts index d726035e..f2e2e71a 100644 --- a/test/e2e/openapi/validate.test.ts +++ b/test/e2e/openapi/validate.test.ts @@ -4,12 +4,13 @@ import { allEndpoints } from "../../../src/sections/registry.js"; import type { LoggedRequest } from "../mock/contract.js"; import { excludeUndocumented, USED_PATHS } from "./paths.js"; import { + loadSpec, OpenApiValidator, type OpenApiViolation, pathMatches, - readSpecText, sharedValidator, toJsonSchema, + trimDescriptor, validateExchange, } from "./validate.js"; @@ -17,6 +18,15 @@ function req(overrides: Partial): LoggedRequest { return { method: "GET", pathname: "/", query: "", status: 200, ...overrides }; } +/** The descriptor node at `keys`, for the lockstep tests, which walk into schemas the validator does not type. */ +function at(root: unknown, ...keys: string[]): Record { + let node: unknown = root; + for (const key of keys) { + node = (node as Record)[key]; + } + return node as Record; +} + const line = (violation: OpenApiViolation): string => `${violation.kind}: ${violation.detail}`; /** An empty `expected` pins a clean exchange; otherwise every pattern must match one finding line. */ @@ -325,13 +335,13 @@ describe("OpenApiValidator against the fetched spec", () => { // The per_page cap is not machine-readable: it lives in the description prose. GitHub CLAMPS an // oversized per_page and the page loop stops on a short page, so an undeclared sub-100 cap // silently truncates after page one (the variables family is capped at 30). - const spec = JSON.parse(readSpecText()) as { + const spec = loadSpec() as { paths?: Record< string, Record & { parameters?: unknown[] } >; }; - // trim-openapi.ts rejects any surviving $ref, so parameters are inline objects. + // loadSpec() rejects any surviving $ref, so parameters are inline objects. const asParam = (param: unknown): { name?: string; description?: string } => param as { name?: string; description?: string }; let cappedEndpoints = 0; @@ -577,18 +587,51 @@ describe("OpenApiValidator against the fetched spec", () => { }); }); -describe("the fetched trimmed spec", () => { +describe("the descriptor slice", () => { test("contains exactly the USED_PATHS paths (no more, no fewer)", () => { - // Read through the loaded validator, not a static JSON import, so a missing spec surfaces the - // actionable fetch error from load() rather than a module-resolution failure. + // Read through the loaded validator, so the pinned @octokit/openapi descriptor is what is cut. const specPaths = [...sharedValidator().paths()].sort(); expect(specPaths).toEqual([...USED_PATHS].sort()); }); - test("a missing spec throws a loud, actionable fetch error naming the script", () => { - expect(() => OpenApiValidator.loadFrom("/nonexistent/github-openapi.trimmed.json")).toThrow( - /bun \.github\/scripts\/trim-openapi\.ts/, + const doc = { + paths: { + "/repos/{owner}/{repo}": { get: {} }, + "/repos/{owner}/{repo}/labels": { get: {}, post: {} }, + "/repos/{owner}/{repo}/lfs": { put: {} }, + }, + }; + + test.each<[string, string[], string[], RegExp]>([ + [ + "a used path the descriptor lacks", + ["/repos/{owner}/{repo}", "/repos/{owner}/{repo}/topics"], + [], + /not in the @octokit\/openapi descriptor:\n {2}\/repos\/\{owner\}\/\{repo\}\/topics\n/, + ], + [ + "an undocumented path the descriptor now documents", + ["/repos/{owner}/{repo}"], + ["/repos/{owner}/{repo}/lfs"], + /now documents: \/repos\/\{owner\}\/\{repo\}\/lfs\. Retire the owning gap/, + ], + ])("%s fails the load by name", (_, used, undocumented, message) => { + expect(() => trimDescriptor(doc, used, undocumented)).toThrow(message); + }); + + test("a $ref left in the kept slice fails the load; one on a path outside the slice is cut away with it", () => { + const withRef = { + paths: { + ...doc.paths, + "/user/repos": { get: { parameters: [{ $ref: "#/components/parameters/per-page" }] } }, + }, + }; + expect(() => trimDescriptor(withRef, ["/user/repos"], [])).toThrow( + /still contains \$ref pointers \(e\.g\. #\/components\/parameters\/per-page\)/, ); + expect(Object.keys(trimDescriptor(withRef, ["/repos/{owner}/{repo}"], []).paths)).toEqual([ + "/repos/{owner}/{repo}", + ]); }); }); @@ -654,18 +697,17 @@ describe("mock rule-type catalog lockstep", () => { // accepted bodies against the SPEC's enums. Drift either falsely 422s a real new type or lets // the mock accept a type the validator flags; pinned equal, a spec refresh is the one update point. const { RULESET_RULE_TYPES } = await import("../mock/support.js"); - const spec = JSON.parse(readSpecText()); + const { paths } = loadSpec(); const operations = [ - spec.paths["/repos/{owner}/{repo}/rulesets"].post, - spec.paths["/repos/{owner}/{repo}/rulesets/{ruleset_id}"].put, + at(paths, "/repos/{owner}/{repo}/rulesets", "post"), + at(paths, "/repos/{owner}/{repo}/rulesets/{ruleset_id}", "put"), ]; for (const operation of operations) { - const rules = operation.requestBody.content["application/json"].schema.properties.rules; + const body = at(operation, "requestBody", "content", "application/json", "schema"); + const items = at(body, "properties", "rules", "items"); // Only TOP-LEVEL variants count: rule parameters nest their own `type` enums (actor kinds and // the like) that a deep walk would wrongly collect. - const variants = (rules.items.oneOf ?? rules.items.anyOf ?? []) as Array< - Record - >; + const variants = (items.oneOf ?? items.anyOf ?? []) as Array>; const specTypes = new Set(); for (const variant of variants) { const type = (variant.properties as Record | undefined)?.type as @@ -688,13 +730,20 @@ describe("invitation role vocabulary lockstep", () => { // The collaborators handler gates PATCH-vs-note on this set and the mock clamps stored invitation // permissions into it, so a spec refresh that moves the enum must land here too. const { INVITATION_ROLES } = await import("../../../src/sections/shared/roles.js"); - const spec = JSON.parse(readSpecText()); - const getEnum = spec.paths["/repos/{owner}/{repo}/invitations"].get.responses["200"].content[ - "application/json" - ].schema.items.properties.permissions.enum as string[]; - const patchEnum = spec.paths["/repos/{owner}/{repo}/invitations/{invitation_id}"].patch - .requestBody.content["application/json"].schema.properties.permissions.enum as string[]; - for (const specEnum of [getEnum, patchEnum]) { + const { paths } = loadSpec(); + const listed = at(paths, "/repos/{owner}/{repo}/invitations", "get", "responses", "200"); + const getEnum = at(listed, "content", "application/json", "schema", "items", "properties") + .permissions as { enum: string[] }; + const patch = at(paths, "/repos/{owner}/{repo}/invitations/{invitation_id}", "patch"); + const patchEnum = at( + patch, + "requestBody", + "content", + "application/json", + "schema", + "properties", + ).permissions as { enum: string[] }; + for (const specEnum of [getEnum.enum, patchEnum.enum]) { expect(specEnum.length).toBeGreaterThan(0); expect([...INVITATION_ROLES].sort()).toEqual([...specEnum].sort()); } diff --git a/test/e2e/openapi/validate.ts b/test/e2e/openapi/validate.ts index 4fbfa5d4..fcafb828 100644 --- a/test/e2e/openapi/validate.ts +++ b/test/e2e/openapi/validate.ts @@ -1,12 +1,13 @@ /** * Validates the mock's traffic against GitHub's published OpenAPI contract: the mock stands in for - * GitHub, so drift between what it serves and what GitHub documents is a mock bug (or a stale spec). - * The trimmed spec is a fetched, gitignored artifact read from disk, never the network, so the runner - * keeps validation always on; a missing spec fails with the fetch command (see readSpecText()). + * GitHub, so drift between what it serves and what GitHub documents is a mock bug (or a stale descriptor). + * The descriptor is @octokit/openapi's dereferenced api.github.com document, cut in memory to USED_PATHS + * by loadSpec(); a Dependabot bump that stops documenting a used path, or starts documenting an upstream + * gap, fails the load by name. */ import { readFileSync } from "node:fs"; -import { join } from "node:path"; +import { fileURLToPath } from "node:url"; import { Ajv, type ValidateFunction } from "ajv"; import addFormats from "ajv-formats"; import { @@ -20,29 +21,18 @@ import { allGraphqlOps } from "../../../src/sections/registry.js"; import { UNDOCUMENTED_ROUTES } from "../../../src/upstream-gaps/index.js"; import { VIOLATION_PREFIX } from "../constants.js"; import type { LoggedRequest } from "../mock/contract.js"; +import { UNDOCUMENTED_PATHS, USED_PATHS } from "./paths.js"; type Json = Record; -const SPEC_PATH = join(import.meta.dir, "github-openapi.trimmed.json"); - /** - * The trimmed spec's text from disk: the one read every consumer goes through, so a missing file fails once, - * naming the command that fetches it, instead of as a bare ENOENT from whichever test read it first. + * The dereferenced (no $ref) api.github.com descriptor, resolved by file so the package's index, which loads every + * GHES and GHEC variant too, never runs. Dereferenced, so a kept path carries its inlined schemas and no + * components graph has to come along. */ -export function readSpecText(specPath = SPEC_PATH): string { - try { - return readFileSync(specPath, "utf8"); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - throw new Error( - `the trimmed OpenAPI spec is missing at ${specPath}. It is a fetched, gitignored artifact: ` + - "bun run test:artifacts fetches it (bun .github/scripts/trim-openapi.ts --when-stale), and the test, " + - "test:e2e, and fuzz scripts run it first; the docs generator (bun run build:docs) reads it too.", - ); - } - throw error; - } -} +const DESCRIPTOR_PATH = fileURLToPath( + import.meta.resolve("@octokit/openapi/generated/api.github.com.deref.json"), +); function segments(path: string): string[] { return path.split("/").filter((s) => s.length > 0); @@ -137,21 +127,80 @@ export function toJsonSchema(node: unknown, keepRequired = false): unknown { return out; } -/** The subset of an OpenAPI operation the validator reads. */ +/** The subset of an OpenAPI operation the validator and the docs generator read. */ interface Operation { requestBody?: { required?: boolean; content?: Record; }; responses?: Record }>; + externalDocs?: { url: string }; + [key: string]: unknown; } type PathItem = Record; -interface OpenApiSpec { +export interface OpenApiSpec { paths: Record; } +/** + * `doc` cut to exactly `usedPaths`, the slice the validator and the docs generator read. USED_PATHS spells + * templates as OpenAPI keys them ("/repos/{owner}/{repo}/labels"), so the match is exact string equality. + * Three descriptor states fail here, at load, by name, instead of as a wrong verdict downstream: + * a used path the descriptor lacks -> the action calls a path GitHub does not document at this version + * an undocumented path it now documents -> the owning gap in src/upstream-gaps/ is due for retirement + * a $ref left in the kept slice -> a partial deref ajv would compile wrong or skip silently + */ +export function trimDescriptor( + doc: { paths: Record }, + usedPaths: readonly string[], + undocumentedPaths: readonly string[], +): OpenApiSpec { + const missing = usedPaths.filter((path) => doc.paths[path] === undefined); + if (missing.length > 0) { + throw new Error( + `these USED_PATHS are not in the @octokit/openapi descriptor:\n ${missing.join("\n ")}\n` + + "Either the path is wrong in test/e2e/openapi/paths.ts, or GitHub stopped documenting it", + ); + } + // An UNDOCUMENTED_PATHS entry exists precisely BECAUSE the descriptor lacks it, so the moment a bump documents + // one, the carve-out must go (and validation switch on). + const nowDocumented = undocumentedPaths.filter((path) => doc.paths[path] !== undefined); + if (nowDocumented.length > 0) { + throw new Error( + `the @octokit/openapi descriptor now documents: ${nowDocumented.join(", ")}. Retire the owning gap in ` + + "src/upstream-gaps/ (delete the spec-only file, or flip documentedInSpec to true on an octokit-kind one) " + + "and regenerate the index (bun .github/scripts/gen-gaps-index.ts), so the validator covers them", + ); + } + const paths: Record = {}; + for (const path of usedPaths) { + paths[path] = doc.paths[path] as PathItem; + } + const serialized = JSON.stringify(paths); + if (serialized.includes('"$ref"')) { + const sample = [...serialized.matchAll(/"\$ref":\s*"([^"]+)"/g)].slice(0, 5).map((m) => m[1]); + throw new Error( + `the descriptor slice still contains $ref pointers (e.g. ${sample.join(", ")}); ` + + "@octokit/openapi's .deref.json is no longer fully dereferenced", + ); + } + return { paths }; +} + +/** Parsed and cut once per process: the runner, the mock's body pipeline, the docs generator, and the tests all read it. */ +let sharedSpec: OpenApiSpec | undefined; +export function loadSpec(): OpenApiSpec { + if (!sharedSpec) { + const doc = JSON.parse(readFileSync(DESCRIPTOR_PATH, "utf8")) as { + paths: Record; + }; + sharedSpec = trimDescriptor(doc, USED_PATHS, UNDOCUMENTED_PATHS); + } + return sharedSpec; +} + /** * What GitHub keeps of a request body: the properties the operation's schema documents, across every * oneOf/anyOf/allOf branch, and whether the schema closes over them (additionalProperties: false, where @@ -203,7 +252,7 @@ export class OpenApiValidator { // Injectable so tests can check the known-name rule with fixture names. graphqlOpNames?: ReadonlySet, ) { - // strict: false because the trimmed doc still carries vocabulary ajv treats as unknown; + // strict: false because the descriptor carries vocabulary ajv treats as unknown; // validateFormats: false because structure is checked, not string formats. this.ajv = new Ajv({ strict: false, validateFormats: false, allErrors: true }); addFormats(this.ajv); @@ -212,14 +261,9 @@ export class OpenApiValidator { graphqlOpNames ?? new Set(Object.values(allGraphqlOps()).map((op) => op.name)); } - /** A fresh clone lacks the fetched spec; a missing file fails naming the fetch command, never skips. */ + /** Over the shared descriptor slice; a descriptor the action outgrew fails in loadSpec() by name, never skips. */ static load(): OpenApiValidator { - return OpenApiValidator.loadFrom(SPEC_PATH); - } - - /** load() against an explicit path; the missing-file branch is testable this way. */ - static loadFrom(specPath: string): OpenApiValidator { - return new OpenApiValidator(JSON.parse(readSpecText(specPath)) as OpenApiSpec); + return new OpenApiValidator(loadSpec()); } private matchTemplate(pathname: string): string | null { @@ -231,7 +275,7 @@ export class OpenApiValidator { return null; } - /** The path templates the loaded spec documents; validate.test.ts pins them equal to USED_PATHS. */ + /** The path templates the loaded slice documents; validate.test.ts pins them equal to USED_PATHS. */ paths(): readonly string[] { return this.templates; } @@ -319,7 +363,7 @@ export class OpenApiValidator { { request: label, kind: "unknown-route", - detail: "path matches no template in the trimmed spec", + detail: "path matches no template in the descriptor slice", }, ]; } diff --git a/test/scripts/auto-fix-allowlist.test.ts b/test/scripts/auto-fix-allowlist.test.ts index ea5a8f65..ad4e90bf 100644 --- a/test/scripts/auto-fix-allowlist.test.ts +++ b/test/scripts/auto-fix-allowlist.test.ts @@ -169,12 +169,10 @@ describe("auto-fix.yml tracks the generated-output table", () => { test("the rebuild step runs exactly the generators, in table order", () => { // The graduation step regenerates the gaps index only when a gap graduates, so the index generator runs here - // too. `bun run build:x` resolves through package.json to the one generator it runs, after at most the - // artifact fetch build:docs opens with; any other shape resolves to nothing and fails the comparison. + // too. `bun run build:x` resolves through package.json to the one generator it runs; any other shape + // resolves to nothing and fails the comparison. const run = [...rebuildRun.matchAll(/^\s*bun (run )?(\S+)$/gm)].map(([, viaScript, name]) => - viaScript === undefined - ? name - : /^(?:bun run test:artifacts && )?bun (\S+)$/.exec(scripts[name ?? ""] ?? "")?.[1], + viaScript === undefined ? name : /^bun (\S+)$/.exec(scripts[name ?? ""] ?? "")?.[1], ); expect(run).toEqual(generators); }); diff --git a/test/scripts/changed-sections.test.ts b/test/scripts/changed-sections.test.ts index 64a6a775..7f770e59 100644 --- a/test/scripts/changed-sections.test.ts +++ b/test/scripts/changed-sections.test.ts @@ -348,7 +348,7 @@ describe("changed-sections selection", () => { ["the e2e runner", "all", ["test/e2e/runner.ts"]], ["the selector itself", "all", [".github/scripts/changed-sections.ts"]], ["a workflow", "all", [".github/workflows/checks.yml"]], - ["a composite action", "all", [".github/actions/fetch-test-artifacts/action.yml"]], + ["a composite action", "all", [".github/actions/setup/action.yml"]], // lib/settings.schema.json regenerates alongside schema-affecting src changes; forcing "all" would kill diff-awareness. [ "a section change plus a regenerated schema, which scopes to the section", diff --git a/test/scripts/check-compat-markers.test.ts b/test/scripts/check-compat-markers.test.ts index a1c8ec90..9e48f615 100644 --- a/test/scripts/check-compat-markers.test.ts +++ b/test/scripts/check-compat-markers.test.ts @@ -160,14 +160,13 @@ describe("checkCompatMarkers", () => { }), ); - test("skips built output, dependencies, the fetched spec, the changelog, ignored files, symlinks, deleted tracked files, and its own two files", () => + test("skips built output, dependencies, the changelog, ignored files, symlinks, deleted tracked files, and its own two files", () => withTempDir("compat-markers-", (dir) => { const marker = "// COMPAT(v3): kept; delete it\n"; const cwd = repo(dir, "2.0.0", { ".gitignore": "scratch/\n", "lib/index.js": marker, "node_modules/dep/index.js": marker, - "test/e2e/openapi/github-openapi.trimmed.json": marker, "CHANGELOG.md": "* remove the COMPAT(v3) legacy reader (#12)\n", "scratch/out.ts": marker, "gone.ts": marker, diff --git a/test/scripts/endpoint-docs.test.ts b/test/scripts/endpoint-docs.test.ts index e906f318..5ce1d249 100644 --- a/test/scripts/endpoint-docs.test.ts +++ b/test/scripts/endpoint-docs.test.ts @@ -1,14 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; -import { - buildSchema, - type ConstDirectiveNode, - type GraphQLObjectType, - getNamedType, - isObjectType, - Kind, -} from "graphql"; +import { schema as published } from "@octokit/graphql-schema"; +import { buildSchema, type GraphQLObjectType, isObjectType } from "graphql"; import { ENDPOINT_DOCS, ENDPOINT_DOCS_PATH, @@ -17,9 +9,6 @@ import { type SpecOperations, } from "../../.github/scripts/endpoint-docs.js"; import { UNDOCUMENTED_ROUTES } from "../../src/upstream-gaps/index.js"; -import { ROOT } from "../root.js"; - -const SCHEMA_PATH = join(ROOT, "test", "e2e", "graphql", "schema.docs.graphql"); describe("resolveAnchors", () => { const spec: SpecOperations = { @@ -130,66 +119,40 @@ describe("endpoint-docs.yml against the registry and the descriptor", () => { ); }); - test("a GraphQL page is the category page the schema assigns, anchored on an entry it declares", () => { - // docs.github.com groups the GraphQL reference by the schema's own @docsCategory and anchors each entry as - // -. A mutation or query field carries the directive itself or inherits its return type's; - // an object type carries its own. A name the schema does not declare, or a category it does not assign, is a - // link that 404s or lands on the wrong page. - const schema = buildSchema(readFileSync(SCHEMA_PATH, "utf8"), { assumeValid: true }); - const categoryOf = (node: { - astNode?: { directives?: readonly ConstDirectiveNode[] } | null; - }): string | undefined => { - const directive = node.astNode?.directives?.find((d) => d.name.value === "docsCategory"); - const value = directive?.arguments?.[0]?.value; - return value?.kind === Kind.STRING ? value.value : undefined; - }; - const objects = new Map( - Object.values(schema.getTypeMap()) - .filter(isObjectType) - .map((type) => [type.name.toLowerCase(), categoryOf(type)] as const), - ); + test("a GraphQL page is a category-page anchor on an entry the schema declares", () => { + // docs.github.com anchors each reference entry as - on a category page. A name the schema + // does not declare is a link that 404s. The category itself comes from a @docsCategory directive the introspected + // schema @octokit/graphql-schema ships cannot carry, so a wrong category page is not caught here. + const schema = buildSchema(published.idl, { assumeValid: true }); const fields = (type: GraphQLObjectType | null | undefined) => - new Map( - Object.values(type?.getFields() ?? {}).map((field) => { - const returned = getNamedType(field.type); - const inherited = isObjectType(returned) ? categoryOf(returned) : undefined; - return [field.name.toLowerCase(), categoryOf(field) ?? inherited] as const; - }), - ); - const declared: Record> = { + new Set(Object.keys(type?.getFields() ?? {}).map((name) => name.toLowerCase())); + const declared: Record> = { mutation: fields(schema.getMutationType()), query: fields(schema.getQueryType()), - object: objects, + object: new Set( + Object.values(schema.getTypeMap()) + .filter(isObjectType) + .map((type) => type.name.toLowerCase()), + ), }; const anchor = - /^https:\/\/docs\.github\.com\/en\/graphql\/reference\/([a-z-]+)#(mutation|query|object)-([a-z0-9]+)$/; + /^https:\/\/docs\.github\.com\/en\/graphql\/reference\/[a-z-]+#(mutation|query|object)-([a-z0-9]+)$/; /** "ok", or why the URL is wrong. */ const verdict = (url: string): string => { const match = anchor.exec(url); if (match === null) { return "not a category-page anchor"; } - const [, category, kind, name] = match; - const entries = declared[kind ?? ""]; - if (!entries?.has(name ?? "")) { - return `the schema declares no ${kind} named "${name}"`; - } - const assigned = entries.get(name ?? ""); - if (assigned === undefined) { - return `the schema assigns "${name}" no category`; - } - return assigned === category + const [, kind, name] = match; + return declared[kind ?? ""]?.has(name ?? "") ? "ok" - : `the schema files "${name}" under ${assigned}, not ${category}`; + : `the schema declares no ${kind} named "${name}"`; }; - // Controls: the right page passes; the wrong category, an undeclared name, and the retired flat page each fail. + // Controls: the right page passes; an undeclared name and the retired flat page each fail. const base = "https://docs.github.com/en/graphql/reference/"; expect(verdict(`${base}deployments#mutation-pinenvironment`)).toBe("ok"); expect(verdict(`${base}users#query-user`)).toBe("ok"); expect(verdict(`${base}repos#object-repository`)).toBe("ok"); - expect(verdict(`${base}does-not-exist#mutation-pinenvironment`)).toBe( - 'the schema files "pinenvironment" under deployments, not does-not-exist', - ); expect(verdict(`${base}deployments#mutation-pinenvironments`)).toBe( 'the schema declares no mutation named "pinenvironments"', ); diff --git a/test/scripts/fetch-retry.test.ts b/test/scripts/fetch-retry.test.ts deleted file mode 100644 index 06900dcd..00000000 --- a/test/scripts/fetch-retry.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -/** - * The fetch and sleep seams are injected, so no test touches the network or waits out a real backoff. - */ - -import { describe, expect, test } from "bun:test"; -import { BACKOFF_BASE_MS, fetchTextWithRetry } from "../../.github/scripts/lib/fetch-retry.js"; - -const URL_UNDER_TEST = "https://raw.githubusercontent.com/owner/repo/ref/artifact.json"; - -function fetchScript(outcomes: Array): { - fetchImpl: (url: string, init?: RequestInit) => Promise; - calls: () => number; - sleeps: number[]; - warnings: string[]; -} { - let call = 0; - const fetchImpl = () => { - const outcome = outcomes[call]; - call++; - if (outcome === undefined) { - throw new Error(`fetch stub called ${call} times but scripted for ${outcomes.length}`); - } - return outcome instanceof Error ? Promise.reject(outcome) : Promise.resolve(outcome); - }; - return { fetchImpl, calls: () => call, sleeps: [], warnings: [] }; -} - -function deps(script: ReturnType) { - return { - fetchImpl: script.fetchImpl, - sleep: (ms: number) => { - script.sleeps.push(ms); - return Promise.resolve(); - }, - warn: (line: string) => { - script.warnings.push(line); - }, - }; -} - -/** A 200 whose body stream errors mid-read, like a dropped connection. */ -function bodyDropResponse(): Response { - return new Response( - new ReadableStream({ - start(controller) { - controller.error(new Error("connection reset mid-body")); - }, - }), - ); -} - -describe("fetchTextWithRetry", () => { - test("returns the body on first success without sleeping", async () => { - const script = fetchScript([new Response("payload")]); - const fetched = await fetchTextWithRetry("artifact", URL_UNDER_TEST, 1000, deps(script)); - expect(fetched).toEqual({ ok: true, status: 200, statusText: "", text: "payload" }); - expect(script.sleeps).toEqual([]); - }); - - test("retries a network error with exponential backoff, then succeeds", async () => { - const script = fetchScript([new Error("ECONNRESET"), new Response("payload")]); - const fetched = await fetchTextWithRetry("artifact", URL_UNDER_TEST, 1000, deps(script)); - expect(fetched.text).toBe("payload"); - expect(script.sleeps).toEqual([BACKOFF_BASE_MS]); - expect(script.warnings).toEqual([ - `fetching the artifact: attempt 1/3 for ${URL_UNDER_TEST} failed (ECONNRESET); retrying in ${BACKOFF_BASE_MS}ms`, - ]); - }); - - for (const status of [500, 408, 429]) { - test(`retries a transient ${status}`, async () => { - const script = fetchScript([ - new Response("nope", { status, statusText: "transient" }), - new Response("payload"), - ]); - const fetched = await fetchTextWithRetry("artifact", URL_UNDER_TEST, 1000, deps(script)); - expect(fetched.text).toBe("payload"); - expect(script.sleeps).toEqual([BACKOFF_BASE_MS]); - }); - } - - test("retries a mid-body drop (the failure a headers-only retry misses)", async () => { - const script = fetchScript([bodyDropResponse(), new Response("payload")]); - const fetched = await fetchTextWithRetry("artifact", URL_UNDER_TEST, 1000, deps(script)); - expect(fetched.text).toBe("payload"); - expect(script.calls()).toBe(2); - }); - - test("returns a plain 4xx immediately with an empty body, no retry", async () => { - const script = fetchScript([new Response("missing", { status: 404, statusText: "Not Found" })]); - const fetched = await fetchTextWithRetry("artifact", URL_UNDER_TEST, 1000, deps(script)); - expect(fetched).toEqual({ ok: false, status: 404, statusText: "Not Found", text: "" }); - expect(script.calls()).toBe(1); - expect(script.sleeps).toEqual([]); - }); - - test("exhaustion names the artifact, the attempt count, the last failure, and the host", async () => { - const script = fetchScript([ - new Error("ECONNRESET"), - new Response("nope", { status: 503, statusText: "Service Unavailable" }), - new Error("socket hang up"), - ]); - const promise = fetchTextWithRetry("OpenAPI descriptor", URL_UNDER_TEST, 1000, deps(script)); - await expect(promise).rejects.toThrow( - /fetching the OpenAPI descriptor failed after 3 attempts .*socket hang up.*raw\.githubusercontent\.com/, - ); - expect(script.calls()).toBe(3); - // Doubling per attempt; with three attempts the two sleeps are also what a linear ramp would give. - expect(script.sleeps).toEqual([BACKOFF_BASE_MS, BACKOFF_BASE_MS * 2]); - }); -}); diff --git a/test/scripts/fetched-artifact.test.ts b/test/scripts/fetched-artifact.test.ts deleted file mode 100644 index d93a6821..00000000 --- a/test/scripts/fetched-artifact.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -/** - * The --when-stale decision both fetch scripts share: pure over the file's text, so no test touches the network or - * the real artifacts. The absent/current pairs are the positive cases; each stale reason is its own negative control. - */ - -import { describe, expect, test } from "bun:test"; -import { writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { - readArtifact, - SOURCE_KEY, - schemaStaleness, - specStaleness, - whenStale, -} from "../../.github/scripts/lib/fetched-artifact.js"; -import { withTempDir } from "../temp-dir.js"; - -const URL = - "https://raw.githubusercontent.com/github/rest-api-description/16bc535a/api.2022-11-28.deref.json"; -const OTHER_URL = URL.replace("2022-11-28", "2025-01-01"); -const PATHS = ["/repos/{owner}/{repo}", "/repos/{owner}/{repo}/labels"]; - -function spec(sourceUrl: unknown, paths: readonly string[]): string { - const doc: Record = { - openapi: "3.0.3", - paths: Object.fromEntries(paths.map((p) => [p, {}])), - }; - if (sourceUrl !== undefined) { - doc[SOURCE_KEY] = sourceUrl; - } - return JSON.stringify(doc); -} - -describe("whenStale", () => { - test("is the --when-stale flag anywhere in argv", () => { - expect(whenStale(["bun", "script.ts", "--when-stale"])).toBe(true); - expect(whenStale(["bun", "script.ts"])).toBe(false); - }); -}); - -describe("readArtifact", () => { - test("returns the text of a present file and null for an absent one", () => - withTempDir("fetched-artifact-", (dir) => { - const path = join(dir, "artifact.txt"); - expect(readArtifact(path)).toBeNull(); - writeFileSync(path, "hello\n"); - expect(readArtifact(path)).toBe("hello\n"); - })); -}); - -describe("specStaleness", () => { - test("a spec trimmed from the source URL with exactly USED_PATHS is current, in any path order", () => { - expect(specStaleness(spec(URL, PATHS), URL, PATHS)).toBeNull(); - expect(specStaleness(spec(URL, [...PATHS].reverse()), URL, PATHS)).toBeNull(); - }); - - test.each<[string, string | null, string]>([ - ["an absent file", null, "the file is absent"], - ["invalid JSON", "{", "the file is not valid JSON"], - ["a JSON null root", "null", "the file is not a JSON object"], - ["a JSON string root", '"spec"', "the file is not a JSON object"], - [ - "another URL (a different API version)", - spec(OTHER_URL, PATHS), - `it was trimmed from ${OTHER_URL}, the script fetches ${URL}`, - ], - [ - "no recorded URL", - spec(undefined, PATHS), - `it was trimmed from an unrecorded URL, the script fetches ${URL}`, - ], - ["a missing path", spec(URL, PATHS.slice(1)), "its paths differ from USED_PATHS"], - ["an extra path", spec(URL, [...PATHS, "/user"]), "its paths differ from USED_PATHS"], - ["no paths object", JSON.stringify({ [SOURCE_KEY]: URL }), "its paths differ from USED_PATHS"], - ])("%s is stale (negative control)", (_, raw, reason) => { - expect(specStaleness(raw, URL, PATHS)).toBe(reason); - }); -}); - -describe("schemaStaleness", () => { - const marker = "# https://raw.githubusercontent.com/github/docs/01f2174e/schema.docs.graphql"; - - test("a schema whose first line is the marker is current", () => { - expect(schemaStaleness(`${marker}\ntype Query { a: Int }\n`, marker)).toBeNull(); - }); - - test.each<[string, string | null, string]>([ - ["an absent file", null, "the file is absent"], - [ - "a file without the marker", - "type Query { a: Int }\n", - `its first line is "type Query { a: Int }", the script writes ${JSON.stringify(marker)}`, - ], - [ - "another URL", - "# https://raw.githubusercontent.com/github/docs/00000000/schema.docs.graphql\ntype Query { a: Int }\n", - `its first line is "# https://raw.githubusercontent.com/github/docs/00000000/schema.docs.graphql", the script writes ${JSON.stringify(marker)}`, - ], - ])("%s is stale (negative control)", (_, raw, reason) => { - expect(schemaStaleness(raw, marker)).toBe(reason); - }); -}); diff --git a/test/scripts/generated.test.ts b/test/scripts/generated.test.ts index 35df0f9a..7d79504f 100644 --- a/test/scripts/generated.test.ts +++ b/test/scripts/generated.test.ts @@ -81,9 +81,6 @@ describe("the build:check runner", () => { // generators are copied from the working tree, so the code under test is the code being edited. git(ROOT, "clone", "--quiet", "--shared", ROOT, dir); symlinkSync(join(ROOT, "node_modules"), join(dir, "node_modules")); - // The docs generator reads the fetched, gitignored OpenAPI spec, which the clone lacks; it borrows the tree's. - const spec = join("test", "e2e", "openapi", "github-openapi.trimmed.json"); - symlinkSync(join(ROOT, spec), join(dir, spec)); cpSync(join(ROOT, ".github", "scripts"), join(dir, ".github", "scripts"), { recursive: true, }); diff --git a/test/scripts/graduate-upstream-gaps.test.ts b/test/scripts/graduate-upstream-gaps.test.ts index da00fb8c..745ba5d3 100644 --- a/test/scripts/graduate-upstream-gaps.test.ts +++ b/test/scripts/graduate-upstream-gaps.test.ts @@ -195,20 +195,23 @@ describe("gapFileBases", () => { }); describe("generateIndex", () => { - test("one import and one GAPS element per gap file, aliased and sorted; an empty directory keeps the same template around an empty GAPS", () => { - // The varying parts of the file: the gap imports (gap.js's is the template's) and the GAPS array, whole. + test("one import and one GAPS entry per gap file, keyed by base name and sorted; an empty directory keeps the template", () => { + // The varying parts of the file: the gap imports (gap.js's is the template's) and the GAPS record, whole. const gapImports = (text: string): string[] => text.match(/^import \{ GAP as .*$/gm) ?? []; const gapsArray = (text: string): string => - text.match(/const GAPS = [\s\S]*?\] as const;/)?.[0] ?? ""; - const two = generateIndex(["pages-https", "merge-queue"]); + text.match(/const GAPS = [\s\S]*?\} as const;/)?.[0] ?? ""; + const two = generateIndex(["pages-https", "lfs"]); expect(gapImports(two)).toEqual([ - 'import { GAP as mergeQueue } from "./merge-queue.js";', + 'import { GAP as lfs } from "./lfs.js";', 'import { GAP as pagesHttps } from "./pages-https.js";', ]); - expect(gapsArray(two)).toBe("const GAPS = [\n mergeQueue,\n pagesHttps,\n] as const;"); + // A key that is its own alias is shorthand; a hyphenated one is quoted, so the file base name survives as the key. + expect(gapsArray(two)).toBe( + 'const GAPS = {\n lfs,\n "pages-https": pagesHttps,\n} as const;', + ); const none = generateIndex([]); expect(gapImports(none)).toEqual([]); - expect(gapsArray(none)).toBe("const GAPS = [] as const;"); + expect(gapsArray(none)).toBe("const GAPS = {} as const;"); // Everything but the imports and the GAPS elements is one template, so the derivations the consumers import // (SupplementalRoute, UNDOCUMENTED_ROUTES) are the same text whatever the directory holds. const template = (text: string): string => @@ -221,7 +224,7 @@ describe("generateIndex", () => { test("the empty index type-checks beside gap.ts: the derivations must not index into an empty tuple", () => withTempDir("gaps-index-empty-", (dir) => { // The committed index compiles under the project typecheck only while a gap file exists; a derivation - // written for a populated GAPS (say `(typeof GAPS)[0]`) would first break the day the last gap graduates. + // written for a populated GAPS (say `(typeof GAPS)["lfs"]`) would first break the day the last gap graduates. symlinkSync(join(ROOT, "node_modules"), join(dir, "node_modules"), "dir"); copyFileSync(join(ROOT, "src", "upstream-gaps", "gap.ts"), join(dir, "gap.ts")); writeFileSync(join(dir, "index.ts"), generateIndex([])); diff --git a/test/sections/graphql-queries.test.ts b/test/sections/graphql-queries.test.ts index 602f6953..51d85b86 100644 --- a/test/sections/graphql-queries.test.ts +++ b/test/sections/graphql-queries.test.ts @@ -1,34 +1,55 @@ /** - * Structural checks need only the query TEXT and run always; full schema validation runs when the fetched, gitignored schema artifact is present. - * Locally its absence skips with the fetch command; in CI the artifact is cache-restored or re-fetched before `bun test`, so absence there is a - * broken pipeline and FAILS. + * Structural checks need only the query TEXT; full validation runs against GitHub's published schema as the + * @octokit/graphql-schema package ships it, extended by the SDL of the upstream gaps the package lags. A + * Dependabot bump that retires a field a query selects fails here, and one that ships a gap's field fails the + * extension, naming the gap file to retire. */ import { describe, expect, test } from "bun:test"; -import { existsSync, readFileSync } from "node:fs"; -import { join } from "node:path"; -import { buildSchema, type OperationDefinitionNode, parse, validate, visit } from "graphql"; +import { schema as published } from "@octokit/graphql-schema"; +import { + buildSchema, + extendSchema, + GraphQLSchema, + type OperationDefinitionNode, + parse, + validate, + visit, +} from "graphql"; import { GRAPHQL_BOOLEAN_TWINS, GRAPHQL_REVIEW_TWINS, GRAPHQL_STATUS_CHECK_TWINS, } from "../../src/sections/branches/graphql-rules.js"; import { allGraphqlOps } from "../../src/sections/registry.js"; -import { ROOT } from "../root.js"; - -const SCHEMA_PATH = join(ROOT, "test", "e2e", "graphql", "schema.docs.graphql"); -const FETCH_COMMAND = "bun .github/scripts/fetch-graphql-schema.ts"; +import type { UnshippedGraphqlSdl } from "../../src/upstream-gaps/gap.js"; +import { UNSHIPPED_GRAPHQL_SDL } from "../../src/upstream-gaps/index.js"; -const schemaAvailable = existsSync(SCHEMA_PATH); -if (!schemaAvailable) { - if (process.env.CI) { - throw new Error( - `the GraphQL schema is missing at ${SCHEMA_PATH} in CI. The checks workflow must restore it from cache or fetch it (${FETCH_COMMAND}) before running tests`, - ); +/** + * 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}`, + ); + } } - console.warn( - `graphql-queries: schema validation skipped - the fetched artifact is missing at ${SCHEMA_PATH}. Generate it with: ${FETCH_COMMAND}`, - ); + return schema; } /** The single operation definition of a declared query, asserted to exist. */ @@ -63,10 +84,8 @@ describe("declared GraphQL queries", () => { } }); - test.skipIf(!schemaAvailable)("every query validates against GitHub's published schema", () => { - // assumeValid skips graphql-js's SCHEMA-level validation, which rejects GitHub's published SDL as-is (it deprecates implementation fields whose - // interface fields are not deprecated); each QUERY is still validated. - const schema = buildSchema(readFileSync(SCHEMA_PATH, "utf8"), { assumeValid: true }); + test("every query validates against GitHub's published schema, extended by the upstream gaps", () => { + const schema = schemaWithGaps(); for (const [key, op] of Object.entries(allGraphqlOps())) { const errors = validate(schema, parse(op.query)); expect( @@ -76,6 +95,16 @@ describe("declared GraphQL queries", () => { } }); + test("a gap whose SDL the package already ships fails the extension naming the gap file (negative control)", () => { + const shipped = { + file: "src/upstream-gaps/example.ts", + sdl: "extend type Repository { id: ID! }", + }; + expect(() => schemaWithGaps([shipped])).toThrow( + /refuses the SDL of src\/upstream-gaps\/example\.ts[\s\S]*Field "Repository\.id" already exists/, + ); + }); + test.each(["branches.rulesQuery", "branches.rulesSnapshot"] as const)( "%s selects every translation-table twin", (key) => {