diff --git a/.github/workflows/pre-install-import-graph.yml b/.github/workflows/pre-install-import-graph.yml new file mode 100644 index 0000000000..5987f3cef2 --- /dev/null +++ b/.github/workflows/pre-install-import-graph.yml @@ -0,0 +1,78 @@ +name: Pre-Install Import Graph + +# Why this is its own workflow rather than a step in `lint.yml` or `ci.yml`: +# what it judges is the ARRANGEMENT of the workflows themselves — which steps +# run a `scripts/` gate before their job installs anything. Its whole input is +# `.github/workflows/**` plus the `scripts/` files those steps name, so a run +# lives or dies on files that both `ci.yml` and `lint.yml` keep OUT of their +# expensive lanes. Putting it inside either one would make it skip on exactly +# the change most likely to break it: a workflow edit that moves a step across +# `pnpm install`. +# +# Same shape and the same reasoning as `docs-links.yml`, `control-bytes.yml`, +# `skills-paths.yml` and `changeset-presence.yml`, whose headers record the +# conclusion this repository reached four times already — a gate that cannot see +# the PR shape most likely to trip it "rebuilds the hole it exists to close". +# One gate, one home. +# +# Hence: no `paths` and no `paths-ignore` here, deliberately. Reporting on every +# pull request is also what makes the check requirable, and +# `scripts/dependabot-merge-gate.mjs` classifies it as a required context — +# an unclassified blocking check is one a Dependabot merge would be let past +# (objectui#6135). +# +# It needs no install and no build — a checkout plus one `node` call over 24 +# workflow files and a dozen scripts, well under a second. Keep it that way: the +# gate exists to protect install-free gates, so an install here would be the +# joke telling itself. + +on: + pull_request: + branches: [main, develop] + push: + branches: [main, develop] + # Merge queue (objectui#3523 — see `ci.yml`'s trigger block for the full note + # and the measurements behind it). A required check that does not report on a + # queue build stalls the queue until the ruleset's 60-minute timeout fails it, + # so an unfiltered gate that can become required subscribes here from the + # start. `types:` is named although `checks_requested` is currently the only + # activity type GitHub defines for `merge_group`. + merge_group: + types: [checks_requested] + workflow_dispatch: + +concurrency: + group: pre-install-import-graph-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + pre-install-import-graph: + name: Pre-Install Import Graph Check + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22.x' + + # Two legs, in this order and in one step, for the reason `lint.yml` runs + # the entry-guard gate the same way: a scan whose recogniser is broken + # reports a clean tree. The self-test pins the workflow parser and the + # graph walk against fixtures, so a green scan below means something. + # + # This step is itself in the population the gate derives — it runs a + # `scripts/` file before any install, in a job that never installs — so + # the gate walks its own import graph on every run. That is deliberate: a + # floor that exempted its own enforcer would be the first thing to rot. + - name: Check the pre-install gates' import graphs + run: | + node scripts/check-pre-install-import-graph.mjs --self-test + node scripts/check-pre-install-import-graph.mjs diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index 0f5ce042d4..0c57798c45 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -33,6 +33,7 @@ one has its own section below. | `doc-component-types.yml` | Doc Component Type Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a `content/docs/**.mdx` snippet teaches a `type` nothing registers | | `doc-snippet-types.yml` | Doc Snippet Type Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a covered documentation snippet no longer compiles against the packages' built types | | `doc-fence-languages.yml` | Doc Fence Language Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a TypeScript block sits under a fence the snippet gate does not read | +| `pre-install-import-graph.yml` | Pre-Install Import Graph Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a gate a workflow runs *before* `pnpm install` reaches a package anywhere in its import graph | | `performance-budget.yml` | Bundle Analysis | Push / PR touching `packages/**`, `apps/console/**`, `pnpm-lock.yaml` | **Yes** — the console entry gzip budget | | `live-e2e.yml` | Live E2E (informational) | PR to `main`, `develop` (code paths); nightly cron `30 6 * * *`; manual | No — informational lane, `continue-on-error` | | `labeler.yml` | Auto Label PRs | PR `opened`, `synchronize`, `reopened` | No | @@ -722,6 +723,60 @@ which is why the probe runs before the verdict. block ```ts (or ```tsx) and fix whatever `check-doc-snippets` then reports, then lower the file's number. Run it locally with `pnpm check:doc-fences`; it needs no install and no build. +## Pre-Install Import Graphs (`pre-install-import-graph.yml`) + +**Triggers:** Push and PR to `main`/`develop`, merge-queue builds, plus manual dispatch — with **no +path filter at all**. What this gate judges is the arrangement of the workflows themselves, so its +input is `.github/workflows/**` plus the `scripts/` files those workflows name, and the change most +likely to break it is a workflow edit. It appears in the checks list as **Pre-Install Import Graph +Check**. + +Runs `scripts/check-pre-install-import-graph.mjs`. Several gates in this repository deliberately run +**before any `pnpm install`** — that is what lets them run unfiltered on every pull request shape for +the price of a checkout plus one `node` call. The property that arrangement silently depends on is +that each of those scripts' *whole static import graph* is node builtins plus repo-relative modules, +with nothing in it needing `node_modules`. + +**Why it needed a gate.** A violation is invisible everywhere it could be caught cheaply: it is not a +type error (`tsc` is happy with a package import), not a lint error (the package is a real dependency +of the repo), not a local failure (locally `node_modules` exists), and — until +[#6148](https://github.com/objectstack-ai/objectui/issues/6148) — not a test failure, because exactly +one of the pre-install scripts had a test asserting it. It surfaces only as `ERR_MODULE_NOT_FOUND` +inside one CI job, on whichever pull request happens to touch the file; and for the gates that carry +no path filter *precisely so they see every PR shape*, that is a gate which **stops running** rather +than one that fails loudly. + +**The population is derived, never listed.** On every run the gate parses every workflow and, per +job, compares each step's index against the index of the first `pnpm install` step **in that same +job**. Move a step above an install and the population grows on the next run; move one below and it +shrinks. A hard-coded list would break silently the first time someone moved a step across an +install, which is exactly the edit that needs catching. Two anchoring decisions the derivation +depends on, each with a case in this repository: `pnpm exec playwright install chromium` installs a +browser rather than the workspace, and `git config merge.pnpm-merge.driver "pnpm install …"` in +`dependabot-auto-merge.yml` *configures* a driver in a job that never installs — reading either as an +install would move a boundary and silently drop a script out of the population. + +**It walks the graph, not the entry file.** Requiring each of the entry's own imports to start with +`node:` is too narrow in one direction (a relative import of a builtins-only local module is fine, +and two of these scripts spell their builtins bare as `from "fs"`, which is equally install-free) and +too weak in the other, because it cannot see a package pulled in **one hop away**. Since +[#6092](https://github.com/objectstack-ai/objectui/issues/6092) every one of these scripts imports +`scripts/invoked-as.mjs`, so one hop away is exactly where the next breach comes from. The check is +static rather than a runtime resolver hook because a hook *executes* module top level, and these +files are CI gates that spawn `git`, read the whole tree and call `process.exit`. + +**It is in its own population.** The step above runs a `scripts/` file before any install, in a job +that never installs, so the gate walks its own import graph on every run. A floor that exempted its +own enforcer would be the first thing to rot. + +**If it fails:** it prints the offending chain — `scripts/some-gate.mjs -> scripts/invoked-as.mjs -> +typescript` — rather than a bare verdict, so the hop that introduced the package is named. Repairing +the import is deliberately *not* this gate's job: either drop the package, or move the step below +`pnpm install` in its workflow and accept the install cost. Run it locally with +`pnpm check:pre-install-import-graph`, `node scripts/check-pre-install-import-graph.mjs --list` to see +the derived population and every module walked, or `--self-test` to exercise the parser and the walk +against fixtures. + ## Link Checking (`check-links.yml`) **Trigger:** Weekly cron (`17 4 * * 0` — Sundays, off the top of the hour, when the scheduled-run diff --git a/package.json b/package.json index ec7439ee77..55f2bdf24b 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "check:doc-fences": "node scripts/check-doc-fence-languages.mjs", "check:eager-closure": "node scripts/check-eager-closure-budget.mjs", "check:entry-guard": "node scripts/check-entry-guard.mjs", + "check:pre-install-import-graph": "node scripts/check-pre-install-import-graph.mjs", "cli": "node packages/cli/dist/cli.js", "objectui": "node packages/cli/dist/cli.js", "create-plugin": "node packages/create-plugin/dist/index.js", diff --git a/scripts/__tests__/check-pre-install-import-graph.test.ts b/scripts/__tests__/check-pre-install-import-graph.test.ts new file mode 100644 index 0000000000..af331b6cea --- /dev/null +++ b/scripts/__tests__/check-pre-install-import-graph.test.ts @@ -0,0 +1,344 @@ +import { describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Plain-JS CI helper. Its types are INFERRED from the .mjs source by +// `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here — +// re-adding one is now itself an error (TS2578). See objectui#3494. +import { + derivePreInstallSteps, + isBuiltinSpecifier, + parseWorkflowJobs, + scan, + staticSpecifiers, + walkImportGraph, +} from '../check-pre-install-import-graph.mjs'; +import { REQUIRED_CONTEXTS } from '../dependabot-merge-gate.mjs'; + +/** + * objectui#6148 — the gate for the property that lets a gate run pre-install. + * + * Eight workflow steps ran a `scripts/` gate before any `pnpm install`, and + * exactly one of them had a test pinning its import graph to node builtins. The + * other seven held the property by accident of what they happened to import, + * and a violation is invisible to `tsc`, to ESLint, to a local run and to the + * suite — it surfaces only as `ERR_MODULE_NOT_FOUND` in one CI job. For the + * gates that carry no path filter *precisely so* they see every PR shape, that + * is a gate which stops running rather than one that fails loudly. + * + * What this file pins, in the order the gate can go wrong: + * + * 1. **the derivation is a derivation** — the population comes from the + * workflows, so moving a step across `pnpm install` moves the population. A + * gate reporting "13 steps" while reading a constant is the defect it was + * written to prevent, and the only way to tell the two apart is to move a + * step in a fixture and watch the number follow; + * 2. **the walk follows the graph** — a package ONE HOP away must be caught + * and its chain named, because the assertion this generalises could not see + * one; + * 3. **the floor over the real tree** — every step objectui#6148 measured is + * still derived, by workflow and job rather than by count; + * 4. **the wiring** — a gate nobody runs is indistinguishable from a gate that + * passes, so the workflow, the alias and the Dependabot classification are + * asserted here rather than trusted by reading. + * + * Deliberately NOT asserted: the total number of pre-install steps. That number + * is the gate's own output and moves whenever a workflow does; a hand-copied + * count here would drift by construction, which is the lesson + * `lint-workflow.test.ts` records at length. + */ +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const workflowDir = path.join(repoRoot, '.github/workflows'); +const SCRIPT = 'scripts/check-pre-install-import-graph.mjs'; +const WORKFLOW = 'pre-install-import-graph.yml'; +const CHECK_NAME = 'Pre-Install Import Graph Check'; + +/** A workflow's YAML with whole-line comments removed — every file here discusses the shapes in prose. */ +const withoutComments = (yaml: string): string => + yaml + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .join('\n'); + +const readWorkflow = (file: string): string => fs.readFileSync(path.join(workflowDir, file), 'utf8'); + +// ── 1. the derivation is a derivation ──────────────────────────────────────── + +describe('the population follows the workflows', () => { + const fixture = (installStepIndex: number): string => { + const gate = ' - name: Gate\n run: node scripts/probe.mjs\n'; + const install = ' - name: Install dependencies\n run: pnpm install --frozen-lockfile\n'; + const filler = ' - name: Checkout code\n uses: actions/checkout@v7\n'; + const steps = [filler, gate, filler]; + steps.splice(installStepIndex, 0, install); + return `on:\n pull_request:\njobs:\n probe:\n steps:\n${steps.join('')}`; + }; + + const derive = (text: string): string[] => + derivePreInstallSteps([{ file: 'probe.yml', text }]).map((s: { script: string }) => s.script); + + it('SHRINKS when the install moves above the gate', () => { + // Install first, gate second: the gate is no longer pre-install. + expect(derive(fixture(0))).toEqual([]); + }); + + it('GROWS when the install moves below the gate', () => { + // The same file with the install one place later — nothing else changed. + expect(derive(fixture(2))).toEqual(['scripts/probe.mjs']); + }); + + it('counts a job that never installs entirely', () => { + const noInstall = 'jobs:\n j:\n steps:\n - name: Gate\n run: node scripts/probe.mjs\n'; + expect(derive(noInstall)).toEqual(['scripts/probe.mjs']); + }); + + it('measures the boundary PER JOB, not per file', () => { + const twoJobs = `jobs: + installs: + steps: + - name: Install dependencies + run: pnpm install --frozen-lockfile + - name: Late + run: node scripts/late.mjs + does-not: + steps: + - name: Early + run: node scripts/early.mjs +`; + expect(derive(twoJobs)).toEqual(['scripts/early.mjs']); + }); + + it('does not read an install out of a shell comment or a quoted argument', () => { + // Both shapes are in this repository: `dependabot-auto-merge.yml` configures + // a merge driver whose VALUE is `"pnpm install --no-frozen-lockfile"` in a + // job that never installs, and block scalars carry `#` lines that are shell + // comments rather than YAML ones. Reading either as an install moves the + // boundary earlier and silently drops a script out of the population — the + // shrinking direction, which costs coverage rather than raising a false red. + const tricky = `jobs: + j: + steps: + - name: Configure Git merge driver for pnpm-lock.yaml + run: | + # pnpm install --frozen-lockfile + git config merge.pnpm-merge.driver "pnpm install --no-frozen-lockfile" + - name: Browsers are not node_modules + run: pnpm exec playwright install chromium + - name: Gate + run: node scripts/probe.mjs +`; + expect(derive(tricky)).toEqual(['scripts/probe.mjs']); + }); + + it('reads block scalars, and ignores an invocation commented out inside one', () => { + const block = `jobs: + j: + steps: + - name: Two legs + run: | + node scripts/probe.mjs --self-test + node scripts/probe.mjs + - name: Commented out + run: | + # node scripts/ghost.mjs + echo done +`; + // One row per (step, script) — two invocations of one script in one step is + // one step, not two. + expect(derive(block)).toEqual(['scripts/probe.mjs']); + }); + + it('parses jobs and steps in order — the index is what "before" means', () => { + const jobs = parseWorkflowJobs(fixture(2)) as Array<{ id: string; steps: Array<{ name: string }> }>; + expect(jobs.map((j) => j.id)).toEqual(['probe']); + expect(jobs[0].steps.map((s) => s.name)).toEqual([ + 'Checkout code', + 'Gate', + 'Install dependencies', + 'Checkout code', + ]); + }); +}); + +// ── 2. the walk follows the graph ──────────────────────────────────────────── + +describe('the import-graph walk sees past the entry file', () => { + const graphOf = (files: Record, entry: string) => + walkImportGraph(entry, { read: (p: string) => (Object.hasOwn(files, p) ? files[p] : null) }) as { + modules: string[]; + violations: Array<{ chain: string[]; specifier: string }>; + unresolved: Array<{ chain: string[]; specifier: string }>; + }; + + it('catches a package ONE HOP away and names the chain', () => { + // The control objectui#6148 hands this gate: `import ts from 'typescript'` + // in `scripts/invoked-as.mjs` is invisible to any check that reads only the + // entry's own import lines, and every pre-install script imports it. + const graph = graphOf( + { + 'scripts/gate.mjs': "import { isEntrypoint } from './invoked-as.mjs';\n", + 'scripts/invoked-as.mjs': "import ts from 'typescript';\nexport const isEntrypoint = ts;\n", + }, + 'scripts/gate.mjs', + ); + expect(graph.violations).toHaveLength(1); + expect(graph.violations[0].specifier).toBe('typescript'); + expect(graph.violations[0].chain.join(' -> ')).toBe( + 'scripts/gate.mjs -> scripts/invoked-as.mjs -> typescript', + ); + }); + + it('accepts a bare builtin, which the narrower predicate called a violation', () => { + // `check-changeset-fixed.mjs` and `check-type-check-coverage.mjs` really do + // spell theirs `from "fs"`. That is install-free, so requiring the `node:` + // prefix would be a style rule wearing a gate's clothes. + expect(isBuiltinSpecifier('fs')).toBe(true); + expect(isBuiltinSpecifier('node:fs')).toBe(true); + expect(isBuiltinSpecifier('typescript')).toBe(false); + expect(graphOf({ 'scripts/a.mjs': 'import { readFileSync } from "fs";\n' }, 'scripts/a.mjs').violations).toEqual( + [], + ); + }); + + it('reads an import in a comment or a string literal as prose, not as code', () => { + // `check-entry-guard.mjs` carries `'require("fs").writeFileSync(…)'` inside + // a corpus string, and the gate's own self-test spells a `typescript` import + // inside a fixture. A scan that counted either would invent findings. + const source = [ + "import { readFileSync } from 'node:fs';", + "// import ts from 'typescript';", + 'const FIXTURE = "import ts from \'typescript\';";', + 'export const x = [readFileSync, FIXTURE];', + ].join('\n'); + expect(staticSpecifiers(source)).toEqual(['node:fs']); + }); + + it('reports a relative import that resolves to nothing instead of walking past it', () => { + const graph = graphOf({ 'scripts/a.mjs': "import './gone.mjs';\n" }, 'scripts/a.mjs'); + expect(graph.unresolved.map((u) => u.specifier)).toEqual(['./gone.mjs']); + }); + + it('terminates on a cycle', () => { + const graph = graphOf( + { 'scripts/a.mjs': "import './b.mjs';\n", 'scripts/b.mjs': "import './a.mjs';\n" }, + 'scripts/a.mjs', + ); + expect(graph.modules.sort()).toEqual(['scripts/a.mjs', 'scripts/b.mjs']); + }); +}); + +// ── 3. the floor over the real tree ────────────────────────────────────────── + +describe('the real tree — every step objectui#6148 measured is still derived', () => { + const result = scan(repoRoot) as { + steps: Array<{ workflow: string; job: string; script: string }>; + scripts: string[]; + modules: string[]; + findings: Array<{ script: string; chain: string[]; kind: string }>; + }; + const rows = new Set(result.steps.map((s) => `${s.workflow} : ${s.job} -> ${s.script}`)); + + /** + * The table objectui#6148 measured, plus `lint.yml`'s entry-guard step, which + * the card named separately. A FLOOR, not an inventory: the assertion is that + * none of these silently leaves the population. The gate's own `--list` is + * where the current total lives, and pinning a total here would drift. + */ + const MEASURED = [ + 'changeset-guard.yml : no-major -> scripts/check-changeset-no-major.mjs', + 'changeset-presence.yml : changeset-presence -> scripts/check-changeset-presence.mjs', + 'ci.yml : changeset-check -> scripts/check-changeset-fixed.mjs', + 'ci.yml : type-check -> scripts/check-type-check-coverage.mjs', + 'control-bytes.yml : control-bytes -> scripts/check-control-bytes.mjs', + 'doc-component-types.yml : doc-component-types -> scripts/check-doc-component-types.mjs', + 'docs-links.yml : docs-links -> scripts/check-doc-links.mjs', + 'skills-paths.yml : skills-paths -> scripts/check-skills-paths.mjs', + 'lint.yml : lint -> scripts/check-entry-guard.mjs', + ]; + + it.each(MEASURED)('still derives %s', (row) => { + expect([...rows], `the population no longer contains this step — was it moved below an install?`).toContain(row); + }); + + it('walks every pre-install script it derived, and reaches past the entries', () => { + // The walk is only worth anything if it follows relative edges: since + // objectui#6092 these scripts share `scripts/invoked-as.mjs`, which is + // reached by no workflow step directly. + expect(result.modules).toContain('scripts/invoked-as.mjs'); + expect(result.modules.length).toBeGreaterThan(result.scripts.length); + }); + + it('is in its own population — the gate walks its own import graph', () => { + // A floor that exempted its own enforcer would be the first thing to rot. + expect(result.scripts).toContain(SCRIPT); + }); + + it('finds no pre-install gate reaching a package', () => { + expect( + result.findings.map((f) => `${f.kind}: ${f.chain.join(' -> ')}`), + 'a script this repository runs before `pnpm install` needs `node_modules` to load', + ).toEqual([]); + }); +}); + +// ── 4. the wiring ──────────────────────────────────────────────────────────── + +describe('the gate is wired, not merely present', () => { + const yaml = withoutComments(readWorkflow(WORKFLOW)); + + it('exists, with a package.json alias that names the same file', () => { + expect(fs.existsSync(path.join(repoRoot, SCRIPT))).toBe(true); + const pkg = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect(pkg.scripts['check:pre-install-import-graph']).toContain(SCRIPT); + }); + + it('runs both legs — the self-test first, then the scan', () => { + expect(yaml).toContain(`node ${SCRIPT} --self-test`); + expect(yaml.split('\n').some((l) => l.trim() === `node ${SCRIPT}`)).toBe(true); + }); + + it('needs no install, which is the property it exists to protect', () => { + expect(yaml).not.toMatch(/pnpm install/); + expect(yaml).not.toMatch(/corepack/); + }); + + it('carries NO path filter of any kind', () => { + // Deliberate: its input is the workflows themselves, and a workflow edit is + // the change most likely to break it. A filter would also make the check + // unrequirable (objectui#3523). + expect(yaml).not.toMatch(/paths-ignore:/); + expect(yaml).not.toMatch(/^\s+paths:/m); + }); + + it('subscribes merge_group, so a queue build is not stalled by a silent context', () => { + expect(yaml).toMatch(/^\s{2}merge_group:/m); + }); + + it('is the only workflow that runs it — one gate, one home', () => { + const runners = fs + .readdirSync(workflowDir) + .filter((f) => f.endsWith('.yml')) + .filter((f) => withoutComments(readWorkflow(f)).includes(SCRIPT)); + expect(runners).toEqual([WORKFLOW]); + }); + + it('is classified by the Dependabot merge gate as a blocking check', () => { + // objectui#6135: an UNCLASSIFIED blocking check is one a Dependabot merge + // would be let past. `dependabot-merge-gate.test.ts` asserts the partition + // itself; this asserts the direction that matters for this check. + expect(REQUIRED_CONTEXTS).toContain(CHECK_NAME); + }); + + it('names the same check in the workflow as the gate requires', () => { + expect(yaml).toContain(`name: ${CHECK_NAME}`); + }); + + it('passes its own self-test', () => { + // A scan whose recogniser is broken reports a clean tree. + const out = execFileSync('node', [SCRIPT, '--self-test'], { cwd: repoRoot, encoding: 'utf8' }); + expect(out).toContain('self-test:'); + expect(out).toMatch(/^✓/); + }); +}); diff --git a/scripts/__tests__/merge-queue-reporting.test.ts b/scripts/__tests__/merge-queue-reporting.test.ts index 007b25b245..c2c9c081f2 100644 --- a/scripts/__tests__/merge-queue-reporting.test.ts +++ b/scripts/__tests__/merge-queue-reporting.test.ts @@ -74,6 +74,13 @@ const MUST_SUBSCRIBE_MERGE_GROUP = new Map([ 'entire scan surface is markdown, so it carries no path filter, reports on every pull ' + 'request, and is therefore requirable', ], + [ + 'pre-install-import-graph.yml', + 'produces Pre-Install Import Graph Check — added by objectui#6148. What it judges is the ' + + 'arrangement of the workflows themselves, so it carries no path filter, reports on every ' + + 'pull request, and is requirable; `scripts/dependabot-merge-gate.mjs` already classifies ' + + 'it as a required context', + ], ]); /** Workflows whose path filtering had to move from the trigger into the jobs. */ diff --git a/scripts/check-pre-install-import-graph.mjs b/scripts/check-pre-install-import-graph.mjs new file mode 100644 index 0000000000..10a7368d6b --- /dev/null +++ b/scripts/check-pre-install-import-graph.mjs @@ -0,0 +1,757 @@ +#!/usr/bin/env node +/** + * check-pre-install-import-graph -- every `scripts/` gate a workflow step runs + * BEFORE `pnpm install` must load without `node_modules`. + * + * node scripts/check-pre-install-import-graph.mjs + * node scripts/check-pre-install-import-graph.mjs --list + * node scripts/check-pre-install-import-graph.mjs --self-test + * + * ## The property, and why it is worth a gate (objectui#6148) + * + * A handful of this repository's gates deliberately run before any install: + * they read the checkout and nothing else, which is what lets them run + * UNFILTERED on every pull request shape for the price of a checkout plus one + * `node` call. `docs-links.yml`, `skills-paths.yml`, `control-bytes.yml` and + * `changeset-presence.yml` exist in that shape on purpose -- their headers each + * say a gate a markdown-only PR cannot start "rebuilds the hole it exists to + * close". + * + * The property that arrangement depends on is narrow and unwritten: each of + * those scripts' WHOLE STATIC IMPORT GRAPH has to be node builtins plus + * repo-relative modules, with nothing anywhere in it needing `node_modules`. + * + * A violation is invisible everywhere it could be caught cheaply: + * + * - not a type error -- `tsc -p tsconfig.scripts.json` is happy with a + * package import; + * - not a lint error -- the package is a real dependency of the repo; + * - not a local failure -- locally `node_modules` exists, so it runs fine; + * - not a test failure -- until this gate, exactly ONE of the pre-install + * scripts had a test asserting it. + * + * It surfaces only as `ERR_MODULE_NOT_FOUND` in a CI job, on whichever pull + * request happens to touch the file. And for the four gates above, whose whole + * point is running on shapes that skip installs, that is a gate that STOPS + * RUNNING rather than a gate that fails loudly -- the failure direction + * AGENTS.md names as worse than no verifier at all. + * + * The class is live rather than theoretical: objectui#6092's PR 2 changed + * `check-doc-component-types.mjs` -- the single pinned one -- and its test went + * red immediately, on a change that was in fact still install-free. The same + * change to any of the others would have produced nothing. + * + * ## Two design decisions, and the defect each one prevents + * + * **The population is DERIVED from `.github/workflows/`, never hard-coded.** A + * hard-coded list of scripts rots the first time someone moves a step above + * `pnpm install` -- which is exactly the edit that needs catching. So this gate + * parses every workflow, and for every job compares each step's index against + * the index of the first `pnpm install` step IN THE SAME JOB. Move a step + * above an install and the population grows on the next run; move one below and + * it shrinks. objectui#6135 landed the same lesson one layer over: the + * enumeration is the thing that rots. + * + * **The check walks the whole graph, not the entry file's own import lines.** + * The assertion this gate generalises read one file's imports and required each + * to start with `node:`. That is too narrow in one direction -- a relative + * import of a builtins-only local module is fine, and two gates here spell + * their builtins bare (`from "fs"`), which is equally install-free -- and too + * weak in the other, because it cannot see a package pulled in ONE HOP AWAY. + * Measured: adding `import ts from 'typescript'` to `scripts/invoked-as.mjs` + * reddens the graph walk and is invisible to any own-imports-only form. Since + * objectui#6092 every one of these scripts imports `./invoked-as.mjs`, so one + * hop away is where the next breach will come from. + * + * ## Why static, and not a resolver hook + * + * The runtime alternative -- load each script under a `module.register` hook + * that throws on any specifier needing `node_modules` -- was the right ad-hoc + * instrument for the one-off measurement in objectui#6092, and is the wrong one + * for a gate that runs on every pull request: it EXECUTES module top level. + * These files are CI gates; several of them spawn `git`, read the whole tree, + * or `process.exit`. A gate that runs nine other gates to decide whether they + * could run is a much larger blast radius than the question deserves. + * + * ## This gate satisfies its own rule, by construction + * + * `pre-install-import-graph.yml` runs this file before any install, so this + * file is IN its own derived population and walks itself. Its graph is + * `node:fs`, `node:module`, `node:path`, `node:url`, plus `./invoked-as.mjs` + * and `./js-comment-mask.mjs`, whose own graphs are builtins only. Nothing here + * may grow a package import without the gate reporting itself -- which is the + * intended arrangement, not a coincidence to be preserved by hand. + * + * ## What is deliberately NOT judged + * + * Non-`node` pre-install steps (a `bash scripts/*.sh`, a `uses:` action) have + * no JavaScript import graph and are not in the population. And this gate never + * REPAIRS anything: it installs the floor. A script that needs a package must + * move below the install in its workflow, or lose the package. + */ + +import { readFileSync, readdirSync } from 'node:fs'; +import { isBuiltin } from 'node:module'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { isEntrypoint } from './invoked-as.mjs'; +import { scanSource } from './js-comment-mask.mjs'; + +const HERE = dirname(fileURLToPath(import.meta.url)); + +/** The repository root -- this file lives at `scripts/` depth 0. */ +export function repoRoot() { + return resolve(HERE, '..'); +} + +const WORKFLOW_DIR = '.github/workflows'; + +// --------------------------------------------------------------------------- +// A workflow parser that needs no YAML package +// --------------------------------------------------------------------------- + +/** + * The one constraint that shapes everything below: this gate runs BEFORE + * `pnpm install`, so it cannot `import { parse } from 'yaml'` the way + * `scripts/__tests__/entry-guard-wiring.test.ts` legitimately does. What it + * needs from YAML is small and structural -- jobs, their steps in order, and + * each step's `run:` scalar -- so it reads the indentation directly. + * + * The one part that genuinely cannot be skipped is BLOCK SCALARS: `run: |` + * bodies are shell, they contain `#` comment lines that are CONTENT rather than + * YAML comments, and several of them are long. Reading them as ordinary lines + * would both mis-nest the parse and let a `# pnpm install` in a shell comment + * move the install boundary -- shrinking the population, which is the unsafe + * direction. + * + * @typedef {{ indent: number, text: string, line: number, block: string | null }} Line + * @param {string} text + * @returns {Line[]} + */ +export function lexYaml(text) { + const raw = text.split('\n'); + /** @type {Line[]} */ + const out = []; + + for (let i = 0; i < raw.length; i++) { + const line = raw[i]; + if (line.trim() === '') continue; + const indent = line.length - line.trimStart().length; + const text_ = line.trim(); + if (text_.startsWith('#')) continue; + + /** @type {Line} */ + const entry = { indent, text: text_, line: i + 1, block: null }; + out.push(entry); + + // `key: |`, `key: >-`, `key: |2` ... everything more-indented is content. + if (!/:\s*[|>][+-]?\d*$/.test(text_)) continue; + + /** @type {string[]} */ + const body = []; + let j = i + 1; + for (; j < raw.length; j++) { + const l = raw[j]; + if (l.trim() === '') { + body.push(''); + continue; + } + if (l.length - l.trimStart().length <= indent) break; + body.push(l); + } + const widths = body.filter((l) => l.trim() !== '').map((l) => l.length - l.trimStart().length); + const dedent = widths.length > 0 ? Math.min(...widths) : 0; + entry.block = body.map((l) => l.slice(dedent)).join('\n'); + i = j - 1; + } + + return out; +} + +/** `key: value` split for one mapping line, `value` empty for a block scalar. */ +function keyValue(text) { + const at = text.indexOf(':'); + if (at === -1) return null; + const key = text.slice(0, at).trim(); + if (!/^[A-Za-z0-9_.-]+$/.test(key)) return null; + return { key, value: text.slice(at + 1).trim() }; +} + +/** + * The jobs of one workflow, each with its steps IN ORDER. + * + * Step order is the whole point -- "before `pnpm install`" is a statement about + * indices, and a parser that returned a set rather than a sequence could not + * answer the question at all. + * + * @param {string} text A workflow file's YAML. + * @returns {Array<{ id: string, steps: Array<{ index: number, name: string, run: string, uses: string }> }>} + */ +export function parseWorkflowJobs(text) { + const lines = lexYaml(text); + const jobsAt = lines.findIndex((l) => l.indent === 0 && l.text === 'jobs:'); + if (jobsAt === -1) return []; + + const after = lines.slice(jobsAt + 1); + const endsAt = after.findIndex((l) => l.indent === 0); + const scoped = endsAt === -1 ? after : after.slice(0, endsAt); + if (scoped.length === 0) return []; + + const jobIndent = scoped[0].indent; + /** @type {number[]} */ + const starts = []; + scoped.forEach((l, i) => { + if (l.indent === jobIndent && /^[A-Za-z0-9_-]+:$/.test(l.text)) starts.push(i); + }); + + return starts.map((start, k) => { + const block = scoped.slice(start, starts[k + 1] ?? scoped.length); + return { id: block[0].text.slice(0, -1), steps: parseSteps(block) }; + }); +} + +/** The `steps:` sequence of one job block, in order. */ +function parseSteps(block) { + const at = block.findIndex((l) => l.text === 'steps:'); + if (at === -1) return []; + const stepsIndent = block[at].indent; + + const steps = []; + let itemIndent = -1; + /** @type {{ index: number, name: string, run: string, uses: string } | null} */ + let current = null; + + const take = (line, text) => { + const kv = keyValue(text); + if (!current || !kv) return; + const value = line.block ?? kv.value; + if (kv.key === 'run') current.run = value; + else if (kv.key === 'name') current.name = value; + else if (kv.key === 'uses') current.uses = value; + }; + + for (const line of block.slice(at + 1)) { + if (line.indent <= stepsIndent) break; + + if (line.text.startsWith('- ') || line.text === '-') { + if (itemIndent === -1) itemIndent = line.indent; + if (line.indent !== itemIndent) continue; // a nested sequence, not a step + current = { index: steps.length, name: '', run: '', uses: '' }; + steps.push(current); + if (line.text.length > 1) take(line, line.text.slice(2)); + continue; + } + + if (itemIndent !== -1 && line.indent === itemIndent + 2) take(line, line.text); + } + + return steps; +} + +// --------------------------------------------------------------------------- +// The derivation: which steps run a `scripts/` gate before an install +// --------------------------------------------------------------------------- + +/** A `run:` scalar with its SHELL comment lines dropped. */ +function shellCode(run) { + return run + .split('\n') + .filter((l) => !l.trim().startsWith('#')) + .join('\n'); +} + +/** + * Does this step install `node_modules`? + * + * Anchored at a COMMAND POSITION -- start of line, or just after a `;`, `&&`, + * `||`, `|` or `(` -- rather than anywhere whitespace precedes the word. Both + * halves of that are load-bearing, and this repository supplies a case for + * each: + * + * - `pnpm exec playwright install chromium` (`ci.yml`'s `e2e` job) installs a + * BROWSER, not the workspace. The package-manager anchor is what excludes + * it, since `playwright` is not `pnpm`. + * - `git config merge.pnpm-merge.driver "pnpm install --no-frozen-lockfile"` + * (`dependabot-auto-merge.yml`) CONFIGURES a merge driver; it installs + * nothing in that job, which explicitly never installs. The command-position + * anchor is what excludes it -- the `pnpm` there is inside an argument. + * + * Getting that second one wrong would move the install boundary to step 4 and + * silently drop `scripts/dependabot-merge-gate.mjs` out of the population -- + * the SHRINKING direction, which is the one that costs coverage rather than + * producing a false red. + */ +const INSTALL_RE = /(?:^|[;&|(])\s*(?:pnpm|npm|yarn|bun)\s+(?:install|ci|i)(?:\s|$)/m; + +/** + * `node scripts/whatever.mjs`, allowing `node --flag script.mjs`. + * + * Deliberately looser than `INSTALL_RE`: any whitespace will do before `node`, + * so a wrapped invocation (`xargs node scripts/x.mjs`, a `bash -c "..."`) is + * still seen. Over-inclusion here costs a graph walk over a script that may not + * really run pre-install; under-inclusion costs the coverage this gate exists + * to install. Those are not symmetric, so the loose form is the right one. + */ +const NODE_SCRIPT_RE = /(?:^|[\s;&|(])node\s+(?:--[^\s]+\s+)*(scripts\/[A-Za-z0-9._/-]+\.(?:mjs|cjs|js))/g; + +/** + * Every step that runs a `scripts/` JavaScript file before the first + * `pnpm install` in its own job. + * + * A job with NO install at all counts entirely: `skills-paths.yml` and + * `docs-links.yml` never install, so every `node` step in them is a + * pre-install step. That is not an edge case, it is four of the nine. + * + * @param {Array<{ file: string, text: string }>} workflows + * @returns {Array<{ workflow: string, job: string, step: number, stepName: string, script: string }>} + */ +export function derivePreInstallSteps(workflows) { + const found = []; + + for (const { file, text } of workflows) { + for (const job of parseWorkflowJobs(text)) { + const code = job.steps.map((s) => shellCode(s.run)); + const installAt = code.findIndex((run) => INSTALL_RE.test(run)); + const limit = installAt === -1 ? job.steps.length : installAt; + + for (let i = 0; i < limit; i++) { + // One row per (step, script), not per invocation: `lint.yml`'s + // entry-guard step runs `--self-test` and then the gate itself, and two + // identical rows would read as two steps. + const seen = new Set(); + for (const m of code[i].matchAll(NODE_SCRIPT_RE)) { + if (seen.has(m[1])) continue; + seen.add(m[1]); + found.push({ + workflow: file, + job: job.id, + step: i, + stepName: job.steps[i].name, + script: m[1], + }); + } + } + } + } + + return found; +} + +/** Read `.github/workflows/` from disk, sorted so output is stable. */ +export function readWorkflows(root) { + const dir = join(root, WORKFLOW_DIR); + return readdirSync(dir) + .filter((f) => f.endsWith('.yml') || f.endsWith('.yaml')) + .sort() + .map((file) => ({ file, text: readFileSync(join(dir, file), 'utf8') })); +} + +// --------------------------------------------------------------------------- +// The static import graph +// --------------------------------------------------------------------------- + +/** + * Every specifier this source statically imports. + * + * Comment- and literal-aware via `scripts/js-comment-mask.mjs`, and that is not + * belt-and-braces: `check-entry-guard.mjs` really does carry + * `'require("fs").writeFileSync(...)'` inside a string literal as one of its + * corpus cases, and this file's own self-test below spells a `typescript` + * import inside a fixture string. A scan that read either as code would report + * a finding it invented out of prose. + * + * The KEYWORD's offset is what gets tested against the masks, and the specifier + * is then read from the untouched source -- so the quotes and their contents + * survive, which blanking literals outright would not allow. + * + * `import()` with a literal argument counts. A dynamic import of a package does + * not break module load, it breaks whenever the branch is reached -- still a + * pre-install script that cannot do its job, and still statically visible. + * + * @param {string} source + * @returns {string[]} + */ +export function staticSpecifiers(source) { + const { comment, literal } = scanSource(source); + const out = []; + + const patterns = [ + // `import x from 'a'` / `import 'a'` / `export { x } from 'a'` + /\b(import|export)\b(?:[^'"();]*?\bfrom\s*)?\s*['"]([^'"]+)['"]/g, + // `import('a')` and `require('a')` + /\b(import|require)\s*\(\s*['"]([^'"]+)['"]\s*\)/g, + ]; + + for (const re of patterns) { + for (const m of source.matchAll(re)) { + const at = m.index ?? 0; + if (comment[at] || literal[at]) continue; + if (!out.includes(m[2])) out.push(m[2]); + } + } + + return out; +} + +/** Is this specifier satisfied by node itself, with or without the `node:` prefix? */ +export function isBuiltinSpecifier(spec) { + return isBuiltin(spec) || isBuiltin(spec.replace(/^node:/, '')); +} + +/** + * Walk one entry's static import graph. + * + * `read(relativePath)` returns the file's source, or `null` when it does not + * exist. Injected rather than hard-wired to `readFileSync` so the self-test's + * fixtures are in-memory graphs rather than temp directories -- the shapes are + * the contract here, not this tree's current contents. + * + * @param {string} entry Repo-relative path, e.g. `scripts/check-doc-links.mjs`. + * @param {{ read: (path: string) => string | null }} io + * @returns {{ modules: string[], violations: Array<{ chain: string[], specifier: string }>, unresolved: Array<{ chain: string[], specifier: string }> }} + */ +export function walkImportGraph(entry, { read }) { + /** @type {Map} module -> the module that first reached it */ + const parents = new Map([[entry, null]]); + const queue = [entry]; + const modules = []; + const violations = []; + const unresolved = []; + + /** The path from the entry down to `module`, as it was first reached. */ + const chainTo = (module) => { + const chain = []; + for (let at = module; at != null; at = parents.get(at) ?? null) chain.unshift(at); + return chain; + }; + + while (queue.length > 0) { + const module = queue.shift(); + const source = read(module); + if (source == null) { + // Only reachable for the ENTRY itself: a relative import that resolves to + // nothing is recorded as unresolved by its importer, below. + unresolved.push({ chain: chainTo(module), specifier: module }); + continue; + } + modules.push(module); + if (module.endsWith('.json')) continue; + + for (const spec of staticSpecifiers(source)) { + if (spec.startsWith('.')) { + const target = join(dirname(module), spec).split('\\').join('/'); + if (read(target) == null) { + unresolved.push({ chain: [...chainTo(module), spec], specifier: spec }); + continue; + } + if (!parents.has(target)) { + parents.set(target, module); + queue.push(target); + } + continue; + } + if (isBuiltinSpecifier(spec)) continue; + violations.push({ chain: [...chainTo(module), spec], specifier: spec }); + } + } + + return { modules, violations, unresolved }; +} + +// --------------------------------------------------------------------------- +// The scan +// --------------------------------------------------------------------------- + +/** + * @param {string} root + * @returns {{ steps: ReturnType, scripts: string[], modules: string[], findings: Array<{ script: string, chain: string[], specifier: string, kind: 'package' | 'unresolved' }> }} + */ +export function scan(root) { + const steps = derivePreInstallSteps(readWorkflows(root)); + const scripts = [...new Set(steps.map((s) => s.script))].sort(); + + const read = (rel) => { + try { + return readFileSync(join(root, rel), 'utf8'); + } catch { + return null; + } + }; + + const modules = new Set(); + const findings = []; + for (const script of scripts) { + const graph = walkImportGraph(script, { read }); + for (const m of graph.modules) modules.add(m); + for (const v of graph.violations) findings.push({ script, ...v, kind: 'package' }); + for (const u of graph.unresolved) findings.push({ script, ...u, kind: 'unresolved' }); + } + + return { steps, scripts, modules: [...modules].sort(), findings }; +} + +const describe = (s) => `${s.workflow} : ${s.job} (step ${s.step}) -> ${s.script}`; + +function main() { + const result = scan(repoRoot()); + const jobs = new Set(result.steps.map((s) => `${s.workflow}:${s.job}`)).size; + + if (result.findings.length === 0) { + console.log( + `✅ check-pre-install-import-graph: OK — ${result.steps.length} pre-install step(s) in ` + + `${jobs} job(s) run ${result.scripts.length} scripts/ gate(s); ${result.modules.length} ` + + `module(s) walked, every non-relative leaf a node builtin.`, + ); + return; + } + + const packages = result.findings.filter((f) => f.kind === 'package'); + const missing = result.findings.filter((f) => f.kind === 'unresolved'); + + console.error( + `❌ check-pre-install-import-graph: ${result.findings.length} pre-install import-graph ` + + `finding(s) across ${result.scripts.length} gate(s)\n`, + ); + + for (const f of packages) { + console.error(` • ${f.script} reaches the package \`${f.specifier}\`:`); + console.error(` ${f.chain.join(' -> ')}`); + } + for (const f of missing) { + console.error(` • ${f.script} imports \`${f.specifier}\`, which does not exist:`); + console.error(` ${f.chain.join(' -> ')}`); + } + + console.error(` +These scripts run BEFORE \`pnpm install\` in their workflow job, so at the moment +they run there is no \`node_modules\` for a package specifier to resolve against. +This does not fail as a type error, a lint error, a local run or (until now) a +test — it fails as ERR_MODULE_NOT_FOUND inside one CI job, on whichever pull +request happens to touch the file. + +For the gates that carry NO path filter precisely so they see every PR shape, +that is a gate which stops running rather than one that fails loudly. + +Two ways out, and repairing the import is not this gate's job: + + • drop the package — most of these graphs need only node builtins and each + other; or + • move the step BELOW \`pnpm install\` in its workflow job, accepting the + install cost and, for an unfiltered gate, deciding deliberately that it is + worth paying. + +\`node scripts/check-pre-install-import-graph.mjs --list\` prints the derived +population and every module walked. The population is DERIVED from +${WORKFLOW_DIR}/ on every run, so moving a step across an install +changes it — nothing here is a hard-coded list to keep in sync.`); + + process.exit(1); +} + +function list() { + const result = scan(repoRoot()); + console.log(`Derived from ${WORKFLOW_DIR}/ — steps running a scripts/ gate before any pnpm install:\n`); + for (const s of result.steps) console.log(` ${describe(s)}`); + console.log(`\n${result.steps.length} step(s), ${result.scripts.length} distinct script(s).\n`); + console.log('Modules reached by the walk:\n'); + for (const m of result.modules) console.log(` ${m}`); + console.log(`\n${result.modules.length} module(s).`); + for (const f of result.findings) console.log(`\nFINDING ${f.kind} ${f.chain.join(' -> ')}`); +} + +// --------------------------------------------------------------------------- +// Self-test — the shapes, not this tree's contents +// --------------------------------------------------------------------------- + +/** + * A green run over today's workflows proves only that today's workflows are + * clean. These cases are the contract: each one is a shape that has to be read + * correctly for the derivation to be a derivation and for the walk to see one + * hop away. + */ +export function selfTest() { + const cases = []; + const t = (name, ok, detail) => cases.push({ name, ok: Boolean(ok), detail }); + + // -- the derivation ------------------------------------------------------ + const WF = `name: Probe +on: + pull_request: +jobs: + before: + steps: + - name: Checkout code + uses: actions/checkout@v7 + - name: Gate + run: node scripts/probe-before.mjs + - name: Install dependencies + run: pnpm install --frozen-lockfile + - name: Late gate + run: node scripts/probe-after.mjs + never-installs: + steps: + - name: Checkout code + uses: actions/checkout@v7 + - name: Gate + run: node scripts/probe-no-install.mjs +`; + const derived = derivePreInstallSteps([{ file: 'probe.yml', text: WF }]); + const scripts = derived.map((s) => s.script); + + t( + 'a step above `pnpm install` is in the population', + scripts.includes('scripts/probe-before.mjs'), + scripts.join(', '), + ); + t( + 'a step BELOW `pnpm install` is not — the boundary is per job, by index', + !scripts.includes('scripts/probe-after.mjs'), + scripts.join(', '), + ); + t( + 'a job that never installs counts entirely (four of this repo`s nine are that shape)', + scripts.includes('scripts/probe-no-install.mjs'), + scripts.join(', '), + ); + t( + 'the step INDEX is reported, so a moved step is visible in --list', + derived[0]?.step === 1 && derived[0]?.job === 'before', + JSON.stringify(derived[0] ?? null), + ); + + const BLOCK = `jobs: + j: + steps: + - name: Multi-line + run: | + # pnpm install --frozen-lockfile <- a SHELL comment, not an install + echo "about to run the gate" + node scripts/probe-block.mjs + - name: Browsers are not node_modules + run: pnpm exec playwright install chromium + - name: Still pre-install + run: node scripts/probe-after-playwright.mjs + - name: Commented out + run: | + # node scripts/probe-commented.mjs + echo done +`; + const block = derivePreInstallSteps([{ file: 'block.yml', text: BLOCK }]).map((s) => s.script); + t('reads a `run: |` block scalar', block.includes('scripts/probe-block.mjs'), block.join(', ')); + t( + 'a `#` line inside a block scalar cannot move the install boundary', + block.includes('scripts/probe-after-playwright.mjs'), + block.join(', '), + ); + t( + '`pnpm exec playwright install` is not an install of node_modules', + block.includes('scripts/probe-after-playwright.mjs'), + block.join(', '), + ); + t( + 'a commented-out invocation is not a step that runs a gate', + !block.includes('scripts/probe-commented.mjs'), + block.join(', '), + ); + + // -- the walk ------------------------------------------------------------ + const graphOf = (files, entry) => + walkImportGraph(entry, { read: (p) => (Object.hasOwn(files, p) ? files[p] : null) }); + + const clean = graphOf( + { + 'scripts/a.mjs': "import { readFileSync } from 'node:fs';\nimport { helper } from './b.mjs';\n", + 'scripts/b.mjs': 'import { resolve } from "path";\nexport const helper = resolve;\n', + }, + 'scripts/a.mjs', + ); + t('a builtins-only graph is clean', clean.violations.length === 0, JSON.stringify(clean.violations)); + t( + 'a bare builtin (`from "path"`) is install-free too — the old form called it a violation', + clean.modules.length === 2 && clean.violations.length === 0, + clean.modules.join(', '), + ); + + const oneHop = graphOf( + { + 'scripts/a.mjs': "import { helper } from './b.mjs';\n", + 'scripts/b.mjs': "import ts from 'typescript';\nexport const helper = ts;\n", + }, + 'scripts/a.mjs', + ); + t( + 'a package ONE HOP away is a violation — the whole reason this walks', + oneHop.violations.length === 1 && oneHop.violations[0].specifier === 'typescript', + JSON.stringify(oneHop.violations), + ); + t( + 'and the finding names the PATH, not just a verdict', + oneHop.violations[0]?.chain.join(' -> ') === 'scripts/a.mjs -> scripts/b.mjs -> typescript', + oneHop.violations[0]?.chain.join(' -> '), + ); + + const quoted = graphOf( + { + // A gate carrying an import inside a fixture string, and one inside a + // comment. Both are prose about code, not code. + 'scripts/a.mjs': [ + "import { readFileSync } from 'node:fs';", + "// import ts from 'typescript';", + "const FIXTURE = \"import ts from 'typescript';\";", + 'export const x = [readFileSync, FIXTURE];', + ].join('\n'), + }, + 'scripts/a.mjs', + ); + t( + 'an import inside a comment or a string literal is not an import', + quoted.violations.length === 0, + JSON.stringify(quoted.violations), + ); + + const dynamic = graphOf({ 'scripts/a.mjs': "await import('typescript');\n" }, 'scripts/a.mjs'); + t( + 'a literal `import()` of a package counts — it fails when reached, not at load', + dynamic.violations.length === 1, + JSON.stringify(dynamic.violations), + ); + + const broken = graphOf({ 'scripts/a.mjs': "import './nope.mjs';\n" }, 'scripts/a.mjs'); + t( + 'a relative import resolving to nothing is reported, never walked past', + broken.unresolved.length === 1 && broken.violations.length === 0, + JSON.stringify(broken.unresolved), + ); + + const cyclic = graphOf( + { + 'scripts/a.mjs': "import './b.mjs';\n", + 'scripts/b.mjs': "import './a.mjs';\n", + }, + 'scripts/a.mjs', + ); + t('a cycle terminates', cyclic.modules.length === 2, cyclic.modules.join(', ')); + + return cases; +} + +function runSelfTest() { + const cases = selfTest(); + const failed = cases.filter((c) => !c.ok); + for (const c of failed) console.error(` ✗ ${c.name}\n got: ${c.detail}`); + if (failed.length > 0) { + console.error(`\n❌ check-pre-install-import-graph self-test: ${failed.length}/${cases.length} case(s) FAILED.`); + process.exit(1); + } + console.log( + `✓ check-pre-install-import-graph self-test: ${cases.length} cases pass ` + + `(derivation boundary, block scalars, one-hop package, masked prose, cycles).`, + ); +} + +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--self-test')) runSelfTest(); + else if (process.argv.includes('--list')) list(); + else main(); +} diff --git a/scripts/dependabot-merge-gate.mjs b/scripts/dependabot-merge-gate.mjs index 05134bcd51..02bd41835a 100644 --- a/scripts/dependabot-merge-gate.mjs +++ b/scripts/dependabot-merge-gate.mjs @@ -126,6 +126,7 @@ import { isEntrypoint } from './invoked-as.mjs'; * doc-component-types.yml Doc Component Type Check * doc-snippet-types.yml Doc Snippet Type Check * doc-fence-languages.yml Doc Fence Language Check + * pre-install-import-graph.yml Pre-Install Import Graph Check * * The four shards are spelled out individually on purpose. A single `Test` * entry, or any pattern match, would be satisfied by whichever shard happened @@ -148,6 +149,7 @@ export const REQUIRED_CONTEXTS = Object.freeze([ 'Doc Component Type Check', 'Doc Snippet Type Check', 'Doc Fence Language Check', + 'Pre-Install Import Graph Check', ]); /**