diff --git a/.changeset/check-docs-no-generator.md b/.changeset/check-docs-no-generator.md new file mode 100644 index 0000000000..59a996d54c --- /dev/null +++ b/.changeset/check-docs-no-generator.md @@ -0,0 +1,39 @@ +--- +"@objectstack/spec": patch +--- + +`check:docs` 不再以 `gen:schema` 开头 —— 一条名字叫 check 的脚本不该改工作区 + +`check:docs` 原本是 `pnpm gen:schema && tsx scripts/build-docs.ts --check`。前半截是**生成器**: +`json-schema.manifest.json` 与 `authorable-surface.json` 陈旧时它会把这两个 **tracked** 文件写掉。 +于是跑一次「检查」就改了跑它的人的工作区,而且陈旧本身**从未被报告**。这正是 #4711 从 `--check` +里摘掉的缺陷,只是换了个入口(#4723)。 + +在 `check:generated` 里最难解释:`check:authorable-surface` 排在 `check:docs` 前面,且前者失败**不会** +中止后者。所以 manifest 陈旧时跑一次聚合门禁的结果是 —— 一份红色报告,配一个已经被悄悄修好的文件。 + +修法与 #4711 同形:**检查只检查,生成交给调用方。** + +- `check:docs` = `tsx scripts/build-docs.ts --check`,不再生成任何东西。 +- 调用方本来就在生成:CI 的 `check:authorable-surface` 步骤、`check:generated` 的门禁顺序、 + `pnpm build`、`apps/docs` 的 build。它们跑的是 `build-schemas.ts --check`,该模式写 gitignored 的 + `json-schema/`、拒绝碰任何 tracked 文件(#4711),正是这里唯一合格的「显式先跑一步」。 +- 顺带省掉一次重复生成:`check:generated` 与 lint.yml 的 typecheck job 原本各跑两遍 ~1600 个 schema。 + 实测 `check:docs` 从 8.97s 降到 2.05s。 + +原来的第一步还**顺手保证了新鲜度**,所以这一半必须补上,否则只是把「改工作区」换成更糟的 +「假绿」——对着编辑前生成的树报告「文档已同步」。因此: + +- `build-docs.ts` 在**所有模式**下先断言 `packages/spec/json-schema/` 存在且不旧于 `src/`,否则红着退出 + 并给出 `gen:schema` 命令。写模式尤其要拒绝:陈旧树上的 `gen:docs` 不会失败,它会**写出**陈旧页面, + 即 `readsDist` 那个坑挪一个产物(AGENTS.md 记着它的代价)。 +- 新鲜度规则 `schemaTreeIsStale()` 与 `distIsStale()` 同住 `scripts/check-regen-pending.mjs`:同一个问题、 + 三个消费方(生成器、pre-commit 钩子、merge driver 的提示),两份拷贝会朝「拿没人重建过的树渲染出 + 一个自信页面」的方向漂移(#4675)。与 `distIsStale` 唯一的有意差别是排除 `.test.ts` —— 测试文件不是 + `build-schemas.ts` 的输入,算进去会让每个纯测试 PR 都被要求跑一次没有意义的 `gen:schema`。 +- `check:generated` 的 GATED 表把这条依赖**声明**出来(`readsSchemaTree`),reconciliation 在生产者缺失或 + 排在消费者后面时失败 —— 数组字面量的顺序是一条真实依赖,不该靠巧合表达。 + +对使用者的影响:**`check:docs` 不再自足**。先跑 `pnpm --filter @objectstack/spec build` +(AGENTS.md 里本来就因 `dist` 那条要求先跑),或让 `check:generated` 按顺序跑。忘了也不会得到错的结论 —— +`build-docs.ts` 会指名道姓地拒绝。 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c853ca36e3..e9cc49b8f1 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -555,11 +555,20 @@ jobs: # this job one at a time, each with a comment saying the filter had failed # it; these four finish the migration and retire the duplicate ledger. # - # Affordable because the work was already being done here: `check:docs` - # below runs `gen:schema` — the same scripts/build-schemas.ts that backs - # `check:authorable-surface` — and that whole step measures 4s in CI. All - # four read source via tsx and need no build, so they run before the - # workspace build, as the gates below already do. + # Affordable because the work was already being done here: one run of + # scripts/build-schemas.ts serves both `check:authorable-surface` and the + # `check:docs` gate below, and that whole step measures 4s in CI. All four + # read source via tsx and need no build, so they run before the workspace + # build, as the gates below already do. + # + # It used to be `check:docs` that ran `gen:schema`, and that is the half + # #4723 removed: `gen:schema` is a generator, so a step named `check:` was + # rewriting two TRACKED files (json-schema.manifest.json, + # authorable-surface.json) whenever they were behind — silently repairing + # locally what it was supposed to report, and running the same ~1600-schema + # generation twice per job. The generation now happens once, in the + # `check:authorable-surface` step below, whose `--check` mode writes the + # gitignored json-schema/ tree and refuses to touch a tracked file (#4711). - name: Check skill docs are generated from SKILL.md frontmatter run: pnpm --filter @objectstack/spec check:skill-docs @@ -576,6 +585,14 @@ jobs: # key added to or removed from a schema. #3883 removed three authorable keys # with every witness green; #3733 did it by accident. ADR-0059 §5 deferred # this gate until a narrowing actually slipped both — it has. + # + # ⚠ ORDER: this step must stay ABOVE the `check:docs` step below. Its + # `--check` run of scripts/build-schemas.ts writes the gitignored + # packages/spec/json-schema/ tree that `check:docs` renders from — the + # explicit generation that replaced the `gen:schema` `check:docs` used to run + # itself (#4723). Nothing here silently depends on that: `build-docs.ts` + # refuses on a missing or stale tree, so a reordering fails loudly with the + # prescription rather than reporting on last run's schemas. - name: Check the authorable key surface is recorded and nothing vanished run: pnpm --filter @objectstack/spec check:authorable-surface @@ -591,6 +608,11 @@ jobs: # job has no paths filter and is a required status check, so the gate cannot go # dormant. It reads src/ + json-schema/ via tsx and needs no build, so it runs # before the workspace build and fails in ~2s. + # + # It CONSUMES the json-schema/ tree the `check:authorable-surface` step above + # generated; it no longer generates one itself (#4723). Two things came off + # this step with that: the duplicate ~1600-schema generation, and the tracked + # files a `check:` script had no business writing. - name: Check generated reference docs are in sync with the spec run: pnpm --filter @objectstack/spec check:docs diff --git a/AGENTS.md b/AGENTS.md index 4977de5b5c..48c170e9a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -399,6 +399,20 @@ signal: it rewrites artifacts whose staleness you never saw, so a real semantic lands silently inside a mechanical diff. Let the check tell you which are stale, then regenerate those. +**No `check:` script regenerates anything — that is the point of the split, not an +oversight.** `check:docs` used to begin with `pnpm gen:schema`, which rewrites two +*tracked* files (`json-schema.manifest.json`, `authorable-surface.json`) whenever they +are behind: running the gate edited your working tree and reported nothing, so a +`check:generated` run on a stale manifest printed a red `check:authorable-surface` +over a file the gate two lines below had already quietly fixed (#4711, #4723). The +generation belongs to the **caller** now — `pnpm build`, or the +`check:authorable-surface` gate that runs before `check:docs` in both CI and +`check:generated`, whose `--check` mode writes the gitignored `json-schema/` tree and +refuses to touch a tracked one. Consequence for you: **`check:docs` is not +self-sufficient**. Run the `build` line above first (it is already required for the +`dist` caveat below) — `build-docs.ts` refuses on a missing or stale tree and names +the command, so the failure is loud, never a wrong verdict. + The script carries its own ledger of gate → generator and **reconciles it against `package.json` on every run**, in both directions. A new `check:`/`gen:` script that nobody classified fails the run rather than quietly dropping out of coverage — the diff --git a/packages/spec/package.json b/packages/spec/package.json index 4d76b3fb09..a49bf1ace5 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -189,7 +189,7 @@ "gen:authorable-surface-base": "OS_EAGER_SCHEMAS=1 tsx scripts/build-schemas.ts --update-base", "gen:openapi": "tsx scripts/build-openapi.ts", "gen:docs": "tsx scripts/build-docs.ts", - "check:docs": "pnpm gen:schema && tsx scripts/build-docs.ts --check", + "check:docs": "tsx scripts/build-docs.ts --check", "check:generated": "tsx scripts/check-generated.ts", "gen:skill-refs": "tsx scripts/build-skill-references.ts", "check:skill-refs": "tsx scripts/build-skill-references.ts --check", diff --git a/packages/spec/scripts/build-docs.ts b/packages/spec/scripts/build-docs.ts index a73cfdeadc..7114fb1ddf 100644 --- a/packages/spec/scripts/build-docs.ts +++ b/packages/spec/scripts/build-docs.ts @@ -20,6 +20,12 @@ import fs from 'fs'; import path from 'path'; +// One staleness rule, shared with the merge driver's pre-commit half and with +// `check:generated`'s `--fix` refusal — a second copy of "is this artifact older +// than src" would drift, and the direction it drifts in is the one that renders +// a confident page from a tree nobody rebuilt (#4675, #4723). +import { schemaTreeIsStale } from '../../../scripts/check-regen-pending.mjs'; + import { evaluateBaseline, loadEntrySurfaces, @@ -57,6 +63,50 @@ const IMPORT_BASELINE_PATH = path.resolve(__dirname, '../docs-import-surface.bas const CHECK = process.argv.includes('--check'); const UPDATE_IMPORT_BASELINE = process.argv.includes('--update-import-baseline'); +// ── The input tree is a BUILD ARTIFACT, and it must be current (#4723) ──────── +// +// Every mode below renders from `SCHEMA_DIR` — `packages/spec/json-schema/`, +// which is gitignored, so no checkout carries it and nothing in git can tell you +// how old it is. +// +// Until #4723 the question could not come up: `check:docs` was +// `pnpm gen:schema && tsx scripts/build-docs.ts --check`, so the tree was rebuilt +// on every run. That first step is what made a script called `check:` WRITE two +// TRACKED files — `json-schema.manifest.json` and `authorable-surface.json` are +// projections `gen:schema` repairs whenever they are behind — so running the gate +// silently edited the tree of whoever ran it and left the staleness unreported. +// #4711 removed exactly that from `--check`; this was the same defect at a +// different entry, and the fix is the same shape: the check checks, and the +// CALLER generates (lint.yml's `check:authorable-surface` step, `check:generated`'s +// declared gate order, `pnpm build`, `apps/docs`' build). +// +// What the old first step also provided, silently, was FRESHNESS. Dropping it +// without asserting freshness would trade a tracked-file write for something +// worse: a green `check:docs` computed against a tree that predates the edit +// under test — a false green on precisely the change (`.describe()` added, a key +// renamed) this gate exists to catch. So the prerequisite is stated, in every +// mode, and it is fatal rather than a warning: `gen:docs` on a stale tree does +// not fail, it WRITES stale pages, which is the `readsDist` trap one artifact +// over (AGENTS.md records what that one cost). +if (schemaTreeIsStale(path.resolve(__dirname, '..'))) { + const missing = !fs.existsSync(SCHEMA_DIR); + console.error( + `\n❌ ${path.relative(REPO_ROOT, SCHEMA_DIR)} is ${missing ? 'missing' : 'older than packages/spec/src'}.\n\n` + + ` The reference docs are rendered from that tree, and it is a gitignored build\n` + + ` artifact — nothing in a checkout carries it, and a merge never brings it along.\n` + + ` Rendering ${CHECK ? 'a verdict' : 'pages'} from a stale tree would ${ + CHECK ? 'report the docs in sync with sources this run never read' : 'WRITE pages describing sources this run never read' + }.\n\n` + + ` Generate it first:\n\n` + + ` pnpm --filter @objectstack/spec gen:schema\n\n` + + ` (\`pnpm --filter @objectstack/spec build\` does this as its first step, and so does\n` + + ` \`check:authorable-surface\`, which runs before this gate in CI and in check:generated.\n` + + ` This script no longer runs it for you: a check that regenerates is a check that\n` + + ` repairs the two tracked projections instead of reporting them — #4711, #4723.)`, + ); + process.exit(1); +} + // ── Output sink ────────────────────────────────────────────────────────────── // Shared with the spec's other generators — see lib/generated-output.ts for why // the write and --check paths must be the same code. @@ -661,7 +711,12 @@ Object.keys(CATEGORIES).forEach(category => { && fs.readdirSync(schemaDir).some(f => f.endsWith('.json')); if (!hasSchemas) { if (fs.existsSync(dir)) { - console.warn(`⚠ Skipping clean of ${category}/ — no JSON schemas found in ${schemaDir}. Run \`pnpm gen:schema\` first.`); + // NOT "run gen:schema first" any more: the freshness guard at the top of + // this file has already proved the tree is newer than src, so this is the + // steady state for a category whose schemas are all unrepresentable in JSON + // Schema (`contracts/` is the standing example) — the old line sent readers + // after a regeneration that would change nothing (#4723). + console.warn(`⚠ Skipping clean of ${category}/ — this build published no JSON Schema under ${schemaDir}; leaving its pages as they are.`); } return; } @@ -938,13 +993,19 @@ flush({ regenerate: ' pnpm --filter @objectstack/spec gen:schema && pnpm --filter @objectstack/spec gen:docs\n' + ' git add content/docs/references', - // json-schema/ is gitignored, so a fresh checkout that forgot gen:schema has no - // input at all: every category is skipped, nothing is managed, and "nothing - // differs" would read as success — green while checking no pages. Fail loudly. + // Backstop to the freshness guard at the top of this file. That one catches the + // common shape — an absent or stale tree — before a single page is rendered. + // This one catches what mtimes cannot see: a tree that is NEWER than src and + // still has no category with schemas in it (a truncated or half-written + // generation). Either way "nothing differs" must never read as success — green + // while checking no pages is the silent shape this whole file guards against. + // `check:docs` no longer regenerates for you, deliberately: that first step is + // what made a check repair two tracked projections (#4711, #4723). guard: () => managedCount === 0 ? `No JSON schemas found under ${path.relative(REPO_ROOT, SCHEMA_DIR)} — nothing to check against.\n` + - ' Run `pnpm --filter @objectstack/spec gen:schema` first (`check:docs` does this for you).' + ' The tree is newer than packages/spec/src but published no category, which means a\n' + + ' partial generation. Run `pnpm --filter @objectstack/spec gen:schema` again.' : null, }); diff --git a/packages/spec/scripts/check-generated-ledger.test.ts b/packages/spec/scripts/check-generated-ledger.test.ts index 833a15ca7a..37b6b73099 100644 --- a/packages/spec/scripts/check-generated-ledger.test.ts +++ b/packages/spec/scripts/check-generated-ledger.test.ts @@ -112,4 +112,81 @@ describe('check:generated --reconcile-only', () => { expect(scripts['gen:test-typecheck-debt']).toBeDefined(); expect(runReconcile().status).toBe(0); }); + + // ─────────────────────────────────────────────────────────────────────────── + // #4723 — no `check:` script may run a `gen:` script. + // + // The defect this pins is a COMPOSITION, one line of package.json, and that is + // why it is pinned here rather than by an end-to-end run: nothing has to be + // executed to see it, and a source-level assertion cannot go quiet the way a + // spawned run can when its gitignored input is absent (which is the state + // `turbo run test` leaves this package in — see root-index.test.ts). + // + // `check:docs` was `pnpm gen:schema && tsx scripts/build-docs.ts --check`. The + // first half is a GENERATOR: on a stale tree it rewrites `json-schema.manifest.json` + // and `authorable-surface.json`, both TRACKED. So running the gate edited the + // working tree of whoever ran it, and — because `check:generated` runs + // `check:authorable-surface` first and does not stop on failure — a single + // aggregate run produced a red report about a manifest that the gate two lines + // below had already quietly fixed. #4711 removed exactly this from `--check`; + // this was the same defect at a different entry. + // + // Stated as the CLASS rather than the one instance, because the class is what + // came back: a check that repairs what it detects can never report it. + describe('no check: script composes a gen: script (#4711, #4723)', () => { + const generatorNames = gens; + + it('is true of every check: script in this package', () => { + const offenders = checks + .map((name) => ({ + name, + runs: generatorNames.filter((g) => + // The composition spellings pnpm accepts. Matched with the boundary + // included so `gen:schema` does not also match `gen:schema-foo`. + new RegExp(`pnpm(?: run)? ${g.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}(?![\\w:-])`).test(scripts[name]), + ), + })) + .filter((o) => o.runs.length > 0); + + expect( + offenders, + offenders + .map( + (o) => + `\`${o.name}\` runs the generator(s) ${o.runs.map((r) => `\`${r}\``).join(', ')}:\n` + + ` ${scripts[o.name]}\n` + + ' A gate that regenerates repairs the tracked artifact it is supposed to report,\n' + + " and silently edits the tree of whoever ran it. Move the generation to the CALLER\n" + + ' (the CI step / the check:generated gate order), or make the gate read the build\n' + + ' artifact and refuse when it is stale, as build-docs.ts does (#4711, #4723).', + ) + .join('\n'), + ).toEqual([]); + }); + + it('leaves check:docs as the read-only half it is named for', () => { + // The specimen, named so a re-composition has to come back through here + // even if the regex above is ever loosened. + expect(scripts['check:docs']).toBe('tsx scripts/build-docs.ts --check'); + }); + }); + + it('declares which gate generates the tree check:docs renders from (#4723)', () => { + // With the generation gone from the composition, `check:docs` depends on a + // gitignored build artifact somebody else produced. Inside this aggregate that + // somebody is the gate ORDER — a dependency an array literal expresses by + // accident, so the ledger declares it and the reconciliation enforces it. The + // narration is asserted because an ordering nobody can see is one the next + // tidy-up breaks silently. + const { status, output } = runReconcile(); + expect(status, output).toBe(0); + expect(output).toContain( + 'check:docs renders from json-schema/, generated by check:authorable-surface above it.', + ); + // And the enforcement itself is real, not just printed: the two failure + // sentences exist in the script that would emit them. + const ledger = fs.readFileSync(path.join(HERE, 'check-generated.ts'), 'utf8'); + expect(ledger).toContain('BEFORE its declared producer'); + expect(ledger).toContain('which this ledger does not run'); + }); }); diff --git a/packages/spec/scripts/check-generated.ts b/packages/spec/scripts/check-generated.ts index 9adc6c67c3..bcaab52d2e 100644 --- a/packages/spec/scripts/check-generated.ts +++ b/packages/spec/scripts/check-generated.ts @@ -52,6 +52,28 @@ const GATED: ReadonlyArray<{ gen: string; artifact: string; readsDist?: true; + /** + * This gate renders from `packages/spec/json-schema/`, and the value names the + * gate in this very list that PRODUCES that tree (#4723). + * + * The tree is gitignored, so no checkout carries it — someone has to generate + * it, and until #4723 that someone was `check:docs` itself: its first step was + * `gen:schema`, which also repairs the two TRACKED projections whenever they + * are behind. So running this aggregate on a stale manifest produced a report + * that was red at `check:authorable-surface` and a working tree that had been + * quietly fixed by the gate two lines below it — a red verdict over a file + * already repaired, which is harder to explain than the staleness was. + * + * The generation moved to the caller, and the caller here is this list's ORDER. + * That is a real dependency, so it is declared rather than left to the array + * literal's shape: `reconcileLedger` fails if the named producer is absent or + * runs after its consumer. It is not a second `gen:` step — the producer is a + * `--check` run, which writes the gitignored tree and refuses to touch a + * tracked file (#4711). Anything that DID regenerate here would repair the + * projections before `check:authorable-surface` could report them, which is the + * defect wearing a fix's clothes. + */ + readsSchemaTree?: string; ratchet?: true; }> = [ { check: 'check:spec-changes', gen: 'gen:spec-changes', artifact: 'spec-changes.json' }, @@ -69,7 +91,12 @@ const GATED: ReadonlyArray<{ // that has cost real triage time (AGENTS.md records the trap). Flagged so the // failure explains itself instead of sending the next reader after a ghost. { check: 'check:api-surface', gen: 'gen:api-surface', artifact: 'api-surface.json', readsDist: true }, - { check: 'check:docs', gen: 'gen:docs', artifact: 'content/docs/references/**' }, + { + check: 'check:docs', + gen: 'gen:docs', + artifact: 'content/docs/references/**', + readsSchemaTree: 'check:authorable-surface', + }, // Moved out of NO_GENERATOR at #5107: the strictness ledger's numbers became a // generated artifact, so this gate now has something to regenerate. It still // audits source too (a hand-written row must name a live sited file), which is @@ -306,6 +333,38 @@ function reconcileLedger(scripts: Record): void { } } + // ── A declared input dependency must actually be satisfiable HERE (#4723) ── + // + // `readsSchemaTree` says "this gate renders from packages/spec/json-schema/, + // which that gate generates". Both halves have to hold, and the second one is + // an ORDER, which an array literal expresses by accident. Left unchecked, a + // later reader tidying this list alphabetically would move `check:docs` above + // its producer and turn it into a gate reporting on the previous run's tree — + // green or red for reasons unrelated to the commit, with nothing saying so. + // + // Cheap to state, so it is stated: the producer must be in this list, and it + // must run first. (Belt and braces, not belt alone: `build-docs.ts` refuses on a + // stale tree no matter who invoked it. This is the half that keeps the ORDER + // honest so the refusal never has to fire.) + for (const [i, entry] of GATED.entries()) { + if (!entry.readsSchemaTree) continue; + const producer = GATED.findIndex((g) => g.check === entry.readsSchemaTree); + if (producer < 0) { + problems.push( + ` GATED says \`${entry.check}\` reads the json-schema/ tree produced by \`${entry.readsSchemaTree}\`,\n` + + ` which this ledger does not run. Name a gate that IS run here, or the tree is\n` + + ` whatever the last unrelated command left on disk (#4723).`, + ); + } else if (producer > i) { + problems.push( + ` GATED runs \`${entry.check}\` (position ${i + 1}) BEFORE its declared producer\n` + + ` \`${entry.readsSchemaTree}\` (position ${producer + 1}). packages/spec/json-schema/ is\n` + + ` gitignored, so on that order this gate reads whatever tree happened to be on disk.\n` + + ` Move the producer above it.`, + ); + } + } + if (problems.length) { console.error(`✗ check:generated ledger is out of sync with package.json:\n\n${problems.join('\n')}\n`); process.exit(1); @@ -351,6 +410,11 @@ if (reconcileOnly) { EXTERNAL_INPUT_REQUIRED.map( (e) => ` ⚠ cannot run here: ${e.check} — needs ${e.input}; runs in ${e.runBy}.\n`, ).join('') + + // Named for the same reason as the bucket above: an ordering constraint + // nobody can see is one a later tidy-up silently breaks (#4723). + GATED.filter((g) => g.readsSchemaTree) + .map((g) => ` ↳ ${g.check} renders from json-schema/, generated by ${g.readsSchemaTree} above it.\n`) + .join('') + ` --reconcile-only: no gates were run — this verifies coverage, not artifacts.`, ); process.exit(0); @@ -372,6 +436,11 @@ for (const entry of GATED) { console.log(` \`pnpm --filter @objectstack/spec build\` since your last pull, the removals`); console.log(` above are phantoms. Build first, then re-run, before regenerating.`); } + if (entry.readsSchemaTree) { + console.log(` ℹ this gate renders from packages/spec/json-schema/, generated by`); + console.log(` \`${entry.readsSchemaTree}\` above. It no longer regenerates that tree itself —`); + console.log(` the first step that did also repaired two TRACKED projections (#4711, #4723).`); + } } } @@ -460,6 +529,11 @@ for (const s of stale) { // generated files against `main`. --fix is the one path that WRITES, so it is // the one place the trap is unsurvivable: a visible conflict is recoverable, // a confidently wrong artifact is not (#4675). + // `readsSchemaTree` gets no refusal of its own here, deliberately. Its + // generator (`build-docs.ts`) carries the guard itself, so EVERY caller is + // covered rather than this one — and by the time --fix runs, the producer gate + // above has already rebuilt the tree from the sources under test, so the guard + // is a backstop rather than the mechanism (#4723). if (s.readsDist && distIsStale()) { failed++; console.log(` ✗ ${s.gen} — REFUSED`); diff --git a/packages/spec/scripts/schema-tree-freshness.test.ts b/packages/spec/scripts/schema-tree-freshness.test.ts new file mode 100644 index 0000000000..453a67d9cf --- /dev/null +++ b/packages/spec/scripts/schema-tree-freshness.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Pins the freshness rule that `check:docs` now leans on (#4723). +// +// ## Why this rule had to exist before the composition could change +// +// `check:docs` used to be `pnpm gen:schema && tsx scripts/build-docs.ts --check`. +// That first step is a GENERATOR, and on a stale tree it rewrites two TRACKED +// files (`json-schema.manifest.json`, `authorable-surface.json`), so a script +// called `check:` edited the working tree of whoever ran it and left the +// staleness unreported — #4711's defect at a different entry point. +// +// Deleting the step fixes that. What the step ALSO did, silently, was guarantee +// that `packages/spec/json-schema/` — the gitignored tree the reference docs are +// rendered from — described the sources under test. Drop it without replacing +// that guarantee and the trade is bad: instead of a gate that repairs a tracked +// file, you get a gate that reports the docs "in sync" against a tree generated +// before the edit. A false GREEN on exactly the change (`.describe()` added, a +// key renamed) the gate exists to catch, with nothing to notice. +// +// So the guarantee became an assertion, and this is that assertion's test. The +// dangerous direction is the only one worth arguing about: **saying fresh when +// stale**. Every case below is written so that a rule which answered `false` +// unconditionally would fail it. +// +// The rule lives in `scripts/check-regen-pending.mjs` beside `distIsStale`, +// deliberately: it is the same question about a different artifact, three +// consumers read it (`build-docs.ts`, the pre-commit hook, the merge driver's +// prescription), and two copies of "is this older than src" drift in the +// direction that renders a confident page from a tree nobody rebuilt (#4675). + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { schemaTreeIsStale } from '../../../scripts/check-regen-pending.mjs'; + +/** A throwaway `packages/spec`-shaped directory: `src/` plus `json-schema/`. */ +let sandbox: string; + +/** Write `rel` with `content`, creating parents, and stamp its mtime. */ +function write(rel: string, content: string, mtimeEpochSeconds?: number): string { + const p = path.join(sandbox, rel); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, content); + if (mtimeEpochSeconds !== undefined) fs.utimesSync(p, mtimeEpochSeconds, mtimeEpochSeconds); + return p; +} + +// Explicit stamps rather than sleeps: mtime resolution and scheduling are not +// what this rule is about, and a test that races them is a test that gets +// `.skip`ped later. OLD/NEW are far enough apart that no filesystem's timestamp +// granularity can collapse them. +const OLD = Math.floor(Date.now() / 1000) - 3600; +const NEW = Math.floor(Date.now() / 1000) - 60; + +beforeEach(() => { + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'os-schema-tree-')); +}); + +afterEach(() => { + fs.rmSync(sandbox, { recursive: true, force: true }); +}); + +describe('schemaTreeIsStale — the json-schema/ freshness rule (#4723)', () => { + it('reads a MISSING tree as stale', () => { + // The state every fresh checkout and every worktree is in: json-schema/ is + // gitignored, so nothing delivers it. Conservative by construction — this is + // also the shape `--self-test` asserts at the root, on a directory that has + // neither half. + write('src/data/object.zod.ts', 'export const x = 1;', OLD); + expect(schemaTreeIsStale(sandbox)).toBe(true); + }); + + it('reads an EMPTY tree as stale', () => { + // A directory that exists but holds no `.json` is not a generation, it is a + // leftover. `newestMtime` returns 0 for it, which must not read as "old but + // present" — that is the silent-green shape build-docs.ts's second guard + // exists to catch downstream. + write('src/data/object.zod.ts', 'export const x = 1;', OLD); + fs.mkdirSync(path.join(sandbox, 'json-schema', 'data'), { recursive: true }); + expect(schemaTreeIsStale(sandbox)).toBe(true); + }); + + it('reads a tree OLDER than src as stale — the false-green case', () => { + // THE case. Before #4723 it could not arise, because `check:docs` regenerated + // on every run; after it, this is what an unrebuilt tree looks like, and + // answering `false` here is what would let `check:docs` report the docs in + // sync with a `.describe()` it never read. + write('json-schema/data/Object.json', '{}', OLD); + write('src/data/object.zod.ts', 'export const x = 1;', NEW); + expect(schemaTreeIsStale(sandbox)).toBe(true); + }); + + it('reads a tree NEWER than src as fresh', () => { + // The state right after `gen:schema`, `pnpm build`, or the + // `check:authorable-surface` gate that precedes `check:docs` in CI and in + // `check:generated`. If this direction were wrong the guard would be a + // permanent red and would get deleted rather than obeyed. + write('src/data/object.zod.ts', 'export const x = 1;', OLD); + write('json-schema/data/Object.json', '{}', NEW); + expect(schemaTreeIsStale(sandbox)).toBe(false); + }); + + it('finds the newest source at ANY depth, not just the top level', () => { + // `src/data/driver/postgres.zod.ts` is real (#4410 is the story of a walk + // that stopped one level down and produced confident output about surface it + // never saw). A rule that only looked at `src/*` would call this fresh. + write('src/data/object.zod.ts', 'export const x = 1;', OLD); + write('json-schema/data/Object.json', '{}', OLD + 60); + write('src/data/driver/postgres.zod.ts', 'export const y = 2;', NEW); + expect(schemaTreeIsStale(sandbox)).toBe(true); + }); + + it('ignores a newer .test.ts — the one deliberate difference from distIsStale', () => { + // Test files are not inputs to `build-schemas.ts`: it imports the namespace + // barrels, and no `.test.ts` is re-exported from one. Counting them would send + // every test-only spec PR to a `gen:schema` that changes nothing, and a guard + // that cries wolf is a guard the next person deletes. Documented in the rule, + // asserted here so the exclusion cannot be "tidied away" as an oversight. + write('json-schema/data/Object.json', '{}', OLD + 60); + write('src/data/object.zod.ts', 'export const x = 1;', OLD); + write('src/data/object.test.ts', 'it("x", () => {});', NEW); + expect(schemaTreeIsStale(sandbox)).toBe(false); + + // …and the exclusion is by SUFFIX, not by "contains test": a source file + // whose name merely starts with the word is still a source file. + write('src/data/test-helpers.zod.ts', 'export const z = 3;', NEW); + expect(schemaTreeIsStale(sandbox)).toBe(true); + }); +}); diff --git a/scripts/check-regen-pending.mjs b/scripts/check-regen-pending.mjs index 3a28c2f6dd..f1ca6113f9 100755 --- a/scripts/check-regen-pending.mjs +++ b/scripts/check-regen-pending.mjs @@ -66,6 +66,35 @@ export function distIsStale(specDir = SPEC_DIR) { return newestMtime(join(specDir, 'src'), (n) => n.endsWith('.ts')) > dist; } +/** + * The same question for the OTHER build artifact a gate reads: is + * `packages/spec/json-schema` older than the sources it was generated from? + * + * `build-docs.ts` reads that tree — it is the input the reference docs are + * rendered from — and the tree is gitignored, so nothing in a checkout carries + * it. Until #4723 the question could not arise: `check:docs` ran `gen:schema` as + * its first step, so the tree was regenerated on every run. That is also what + * made `check:docs` a "check" that WROTE two tracked files (json-schema.manifest.json + * and authorable-surface.json, whenever they were behind), which is the defect + * #4711 removed from `--check` and #4723 removed from this composition. With the + * generation gone, the freshness it silently guaranteed has to be ASSERTED, or + * `check:docs` reports a verdict about a tree that predates the edit under test — + * a FALSE GREEN on exactly the change (`.describe()` added, key renamed) the gate + * exists to catch. Same trap, same conservative direction, as `readsDist` above. + * + * Two deliberate differences from `distIsStale`: + * - `.test.ts` is excluded from the source side. Test files are not inputs to + * `build-schemas.ts` (it imports the namespace barrels), so counting them + * would send every test-only spec PR to a `gen:schema` it does not need — + * and a guard that cries wolf is a guard someone deletes. + * - the artifact side matches `.json`, the tree's only content. + */ +export function schemaTreeIsStale(specDir = SPEC_DIR) { + const tree = newestMtime(join(specDir, 'json-schema'), (n) => n.endsWith('.json')); + if (!tree) return true; + return newestMtime(join(specDir, 'src'), (n) => n.endsWith('.ts') && !n.endsWith('.test.ts')) > tree; +} + function markerPath() { const gitDir = execFileSync('git', ['rev-parse', '--absolute-git-dir'], { encoding: 'utf8' }).trim(); return join(gitDir, PENDING_MARKER); @@ -118,6 +147,20 @@ function main() { ); continue; } + // Same shape one artifact over (#4723). The gate would refuse on its own — + // `build-docs.ts` carries the guard, so every caller is covered, not just + // this one — but running it here would spend the spawn only to reprint a + // message this hook can state with the merge context already in hand. + if (entry.readsSchemaTree && schemaTreeIsStale()) { + blocked++; + console.error( + ` ✗ ${paths.join(', ')}\n` + + ` ${check} reads packages/spec/json-schema/, which is missing or older than src —\n` + + ` NOT running it. That tree is gitignored, so a merge never brings it with them.\n` + + ` pnpm --filter @objectstack/spec gen:schema && pnpm --filter @objectstack/spec ${entry.gen}`, + ); + continue; + } const { ok, output } = runCheck(check); if (ok) { console.error(` ✓ ${paths.join(', ')} — current`); @@ -155,10 +198,13 @@ const invokedDirectly = process.argv[1] && resolve(process.argv[1]) === fileURLT if (invokedDirectly) { if (process.argv.includes('--self-test')) { - // Touches no repo state: the interesting logic is the staleness rule, and its - // dangerous direction is "says fresh when stale". - const ok = distIsStale(join(REPO_ROOT, 'scripts')) === true; - console.log(`${ok ? '✓' : '✗'} a directory with no dist/ reads as STALE (conservative default)`); + // Touches no repo state: the interesting logic is the staleness rules, and + // their dangerous direction is "says fresh when stale". + const noDist = distIsStale(join(REPO_ROOT, 'scripts')) === true; + console.log(`${noDist ? '✓' : '✗'} a directory with no dist/ reads as STALE (conservative default)`); + const noTree = schemaTreeIsStale(join(REPO_ROOT, 'scripts')) === true; + console.log(`${noTree ? '✓' : '✗'} a directory with no json-schema/ reads as STALE (conservative default)`); + const ok = noDist && noTree; console.log(ok ? '\n✓ check-regen-pending self-test passed.' : '\n✗ self-test failed.'); process.exit(ok ? 0 : 1); } diff --git a/scripts/git-merge-regen.mjs b/scripts/git-merge-regen.mjs index 2b456a62bd..fd3bf4408c 100755 --- a/scripts/git-merge-regen.mjs +++ b/scripts/git-merge-regen.mjs @@ -112,10 +112,17 @@ function drive(argv) { const dist = entry.readsDist ? '\n (this one is built from dist/*.d.ts — the regeneration will refuse on a stale build)' : ''; + // The json-schema/ tree is gitignored, so a merge never carries it: whatever is + // on disk describes one side of the merge. Name the prerequisite here rather + // than let the reader discover it from the generator's refusal (#4723). + const tree = entry.readsSchemaTree + ? '\n (this one renders from the gitignored packages/spec/json-schema/ tree —' + + '\n run `pnpm --filter @objectstack/spec gen:schema` first, or it will refuse)' + : ''; console.error( ` ⟳ ${path}\n` + ` not text-merged — it is generated. Regenerate from the merged tree:\n` - + ` pnpm --filter @objectstack/spec ${entry.gen}${dist}\n` + + ` pnpm --filter @objectstack/spec ${entry.gen}${dist}${tree}\n` + ` The pre-commit hook will not let this commit through until you do.`, ); return 0; diff --git a/scripts/regen-artifacts.mjs b/scripts/regen-artifacts.mjs index 4e073c5daa..89eec5b0b1 100644 --- a/scripts/regen-artifacts.mjs +++ b/scripts/regen-artifacts.mjs @@ -59,7 +59,15 @@ export const REGEN_ARTIFACTS = Object.freeze([ check: 'check:api-surface', readsDist: true, }, - { path: 'content/docs/references/**', gen: 'gen:docs', check: 'check:docs' }, + // `readsSchemaTree` is the `readsDist` above, one artifact over (#4723). Both + // `gen:docs` and `check:docs` render from `packages/spec/json-schema/`, which is + // GITIGNORED — a merge therefore never delivers it, and a leftover tree from + // before the merge describes the sources as they were on one side of it. Until + // #4723 `check:docs` began with `gen:schema`, so the tree was always rebuilt (at + // the cost of a "check" that wrote two tracked files whenever they were behind). + // With that step gone, the freshness is asserted instead: every path that would + // run either script refuses unless the tree is newer than the sources. + { path: 'content/docs/references/**', gen: 'gen:docs', check: 'check:docs', readsSchemaTree: true }, // #5107. Unlike its neighbours this one is derived from the AST *plus* a // hand-written column (the ledger's `Class` verdicts feed the per-class // subtotals), which is exactly why it belongs here rather than in the ledger: