diff --git a/.github/workflows/ci-cli.yml b/.github/workflows/ci-cli.yml index c957bb4..03133f2 100644 --- a/.github/workflows/ci-cli.yml +++ b/.github/workflows/ci-cli.yml @@ -64,15 +64,18 @@ jobs: - name: Lint & format check (biome check) run: bun run lint - # Belt and braces over `postinstall`. `src/api-types.d.ts` is generated from - # `contract/openapi.json` and is not committed, so Checks A and C have - # nothing to compare against until it exists. `postinstall` already writes - # it during `bun install`; running it explicitly means a cache hit, a - # changed install flag or a future `--ignore-scripts` cannot quietly turn - # the two compile-time checks into a no-op. - - name: Generate API types from the vendored contract - run: bun run api-types:generate - + # No separate generate step. `bun run typecheck` regenerates + # `src/api-types.d.ts` itself, so this lane, the release lane and a + # developer's terminal all compile against the same freshly generated + # types - one source of truth, in package.json. + # + # This matters because the file is generated from `contract/openapi.json` + # and is not committed: without it, Checks A and C in `src/api-contract.ts` + # have nothing to compare against, and every comparison resolves to `never` + # rather than failing loudly. `postinstall` writes it during `bun install`, + # but a cache hit, a changed install flag or a future `--ignore-scripts` + # can skip that - which is a local-only mystery when the script does not + # own the generation. `scripts/ci-contract-drift.test.ts` asserts it does. - name: Typecheck run: bun run typecheck diff --git a/.husky/pre-commit b/.husky/pre-commit index 1911220..5a4699e 100644 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -132,9 +132,14 @@ fi # --- the expensive one ----------------------------------------------------- -# ~0.32s warm, and the most common `ci-cli` failure after lint. tsc is whole-program, +# ~0.7s warm, and the most common `ci-cli` failure after lint. tsc is whole-program, # so there is no staged-only version of it: it runs when any TypeScript is staged. # +# `bun run typecheck` regenerates src/api-types.d.ts first (~0.36s of that 0.7s). +# That is deliberate and not worth trimming here: it is the single place the +# generation lives, shared with ci-cli and the release lane, and it is what makes +# the `contract` case below compile against the contract as just staged. +# # `contract` is in the condition for a case that stages no TypeScript at all. A # refresh commit is `contract/openapi.json` alone - CONTRIBUTING says to commit # that diff on its own, and the regenerated src/api-types.d.ts is gitignored, so diff --git a/package.json b/package.json index 580e970..c347cef 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "lint": "biome check", "lint:staged": "biome check --staged --no-errors-on-unmatched", "format": "biome check --write", - "typecheck": "bunx --package @typescript/native tsc --noEmit", + "typecheck": "bun run api-types:generate && bunx --package @typescript/native tsc --noEmit", "test": "bun --env-file=.env.local.example test ./src ./scripts", "check": "bun run lint && bun run typecheck && bun run test", "prepare": "husky" diff --git a/scripts/ci-contract-drift.test.ts b/scripts/ci-contract-drift.test.ts index 38babef..1f448a6 100644 --- a/scripts/ci-contract-drift.test.ts +++ b/scripts/ci-contract-drift.test.ts @@ -14,14 +14,20 @@ * carries a `servers` block naming its host, so a staging URL would rewrite * that line on every refresh and compare the committed contract against one * nothing ships against. - * - `src/api-types.d.ts` is generated, not committed. Checks A and C are - * compile-time comparisons against it; a committed copy is a second contract - * to keep in step, and a stale one silently checks the wrong shapes. + * - `src/api-types.d.ts` is generated, not committed, and the `typecheck` + * script is what generates it. Checks A and C are compile-time comparisons + * against it; a committed copy is a second contract to keep in step, and a + * stale one silently checks the wrong shapes. Move the generation back out + * into a workflow step and only the lane carrying that step is covered - + * the release lane and every developer's terminal compare against whatever + * `postinstall` last left behind, or against nothing at all. */ import { describe, expect, it } from "bun:test"; import { readFileSync } from "node:fs"; const CI = ".github/workflows/ci-cli.yml"; +/** The other lane that typechecks, and the reason generation belongs to the script. */ +const RELEASE = ".github/workflows/release-cli.yml"; const DRIFT_STEP = "Contract drift (warning only)"; @@ -130,14 +136,59 @@ describe("ci-cli: the contract drift step cannot veto a merge", () => { }); }); -describe("ci-cli: the generated types exist before anything compares them", () => { - it("generates them before the typecheck", () => { - const generate = steps.findIndex((step) => - step.run?.includes("api-types:generate"), +describe("the generated types exist before anything compares them", () => { + // The guarantee used to be a `ci-cli` step that ran `api-types:generate` + // ahead of the typecheck. That covered this one lane and nothing else: the + // release lane typechecks without it, and a developer whose install skipped + // `postinstall` got 168 errors about code that is fine, because Checks A and + // C resolve to `never` when the file is absent. The generation now lives in + // the `typecheck` script, so every caller inherits it. + it("are regenerated by the typecheck script itself", () => { + const typecheck = scripts.typecheck ?? ""; + // Anchored and POSITIVE, matching the whole generate-to-bunx boundary, + // because a blacklist only refuses the separator someone already thought + // of. `; bunx` alone let three worse spellings through: `;bunx` (no + // space), a bare `&` (generation backgrounded, tsc racing it) and a + // newline - and `||`, which is the inversion of the guarantee, running + // tsc only when the generation FAILED. A new separator now has to be + // written into this line to pass, rather than merely dodge it. + expect(typecheck).toMatch( + /^\s*bun run api-types:generate\s+&&\s+bunx\b.*\btsc\b/, ); - const typecheck = indexOfStep("Typecheck"); - expect(generate).toBeGreaterThan(-1); - expect(typecheck).toBeGreaterThan(generate); + expect(typecheck).toContain("--noEmit"); + }); + + it("are not regenerated by a second, drifting copy in ci-cli", () => { + // A standalone step here would be a second place to keep in step with + // package.json, and the lane that has one would silently diverge from the + // lane that does not. + const standalone = steps.filter( + (step) => + step.run?.includes("api-types:generate") && + !step.run?.includes("typecheck"), + ); + expect(standalone).toEqual([]); + expect(indexOfStep("Typecheck")).toBeGreaterThan(-1); + expect(stepNamed("Typecheck")?.run).toBe("bun run typecheck"); + }); + + it("cover the release lane too, which has no generate step of its own", () => { + const release = Bun.YAML.parse(readFileSync(RELEASE, "utf8")) as Workflow; + const releaseSteps = Object.values(release.jobs).flatMap( + (releaseJob) => releaseJob.steps ?? [], + ); + const typechecks = releaseSteps.filter((step) => + step.run?.includes("typecheck"), + ); + expect(typechecks.length).toBeGreaterThan(0); + for (const step of typechecks) expect(step.run).toBe("bun run typecheck"); + expect( + releaseSteps.filter( + (step) => + step.run?.includes("api-types:generate") && + !step.run?.includes("typecheck"), + ), + ).toEqual([]); }); it("runs the unit tests over ./src, where Check B lives", () => {