From 5e142130d1a74d251f064a79b4bbf20e762e34b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:21:47 +0000 Subject: [PATCH] =?UTF-8?q?feat(spec):=20`check:generated`=20=E2=80=94=20r?= =?UTF-8?q?eport=20every=20stale=20artifact=20in=20one=20run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `packages/spec` has eight checked-in generated artifacts, each with its own gate, split across two CI jobs that run their gates SEQUENTIALLY. The first stale artifact therefore masks every one behind it: you fix it, push, and learn about the next on the following run. Two pushes on #4040 (`check:docs`, then `check:api-surface`), two more on #4161 (`check:spec-changes`, then `check:upgrade-guide`) — four round trips spent discovering something one local run could have said at once. Every gate runs; a failure does not stop the rest. The summary lists all stale artifacts with the exact `gen:` command for each. `--fix` regenerates ONLY what this run proved stale. Deliberately not a regenerate-everything button: blanket regeneration rewrites artifacts whose staleness you never saw, which is how a real semantic change lands silently inside a mechanical diff. The gate -> generator ledger reconciles against `package.json` on EVERY run, in both directions, rather than behind a `--self-test` flag. An unclassified `check:`/`gen:` script fails the run instead of quietly dropping out of coverage — otherwise the summary would still say "all artifacts up to date" while checking fewer, the exact class of lie this exists to remove. It proved itself by rejecting its own `package.json` entry on the first run. Two things it refuses to be silent about: the `check:api-surface` stale-`dist` trap is printed inline when that gate is the one failing (it reads the built `.d.ts`, so an unbuilt tree reports newly-added exports as breaking removals), and the four source audits it does NOT run are named, so "all up to date" never reads as "everything passed". It also reports that `gen:openapi` and `gen:sbom` have no gate at all. Verified both directions: clean built tree -> all 8 pass; a change injected into a migration step's `rationale` -> `check:upgrade-guide` fails and `--fix` regenerates that artifact and no other. That run also corrected an earlier reading of mine: the two ADR-0087 gates have different inputs — the conversion registry drives `spec-changes.json`, the rationale prose drives `protocol-upgrade-guide.md` — they had only appeared to fail together because #4161 changed both. AGENTS.md's hand-rolled loop over eight hardcoded gate names is replaced by the command; that list could not survive a ninth artifact, and the ledger can. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq --- .changeset/spec-check-generated-aggregate.md | 44 +++++ AGENTS.md | 34 +++- packages/spec/package.json | 1 + packages/spec/scripts/check-generated.ts | 183 +++++++++++++++++++ 4 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 .changeset/spec-check-generated-aggregate.md create mode 100644 packages/spec/scripts/check-generated.ts diff --git a/.changeset/spec-check-generated-aggregate.md b/.changeset/spec-check-generated-aggregate.md new file mode 100644 index 0000000000..8b8bd552ba --- /dev/null +++ b/.changeset/spec-check-generated-aggregate.md @@ -0,0 +1,44 @@ +--- +'@objectstack/spec': patch +--- + +`check:generated` — one command that reports every stale generated artifact, instead of one red build per artifact. + +`packages/spec` has eight checked-in generated artifacts, each with its own gate, split +across two CI jobs that run their gates **sequentially**. So the first stale artifact +masks every one behind it: you fix it, push, and learn about the next one on the +following run. That cost two pushes on #4040 (`check:docs`, then `check:api-surface`) and +two more on #4161 (`check:spec-changes`, then `check:upgrade-guide`) — four round trips +spent discovering something one local run could have said at once. + +- Every gate runs; a failure does not stop the rest. The summary lists all stale + artifacts with the exact `gen:` command for each. +- `--fix` regenerates **only** the artifacts this run proved stale. Deliberately not a + "regenerate everything" button: blanket regeneration rewrites artifacts whose staleness + you never saw, which is how a real semantic change lands silently inside a mechanical + diff. +- The gate → generator ledger is **reconciled against `package.json` on every run**, both + directions, rather than behind a `--self-test` flag. A new `check:`/`gen:` script that + nobody classified fails the run instead of quietly dropping out of coverage — otherwise + the summary would still say "all artifacts up to date" while silently checking fewer, + which is the exact class of lie this script exists to remove. It proved itself + immediately by rejecting its own `package.json` entry on the first run. +- The `check:api-surface` stale-`dist` trap (it reads the built `.d.ts`, so an unbuilt + tree reports every newly-added export as a "breaking removal") is flagged inline when + that gate is the one failing, instead of leaving the next reader to chase a phantom. +- What it deliberately does **not** run is named in the output: the four source audits + with no artifact (`check:liveness`, `check:empty-state`, `check:react-conformance`, + `check:skill-examples`), so "all up to date" never reads as "everything passed". + +It also surfaces a standing gap rather than staying quiet about it: `gen:openapi` and +`gen:sbom` produce artifacts that **no gate verifies**. + +Verified both directions: on a clean built tree all 8 pass; injecting a change into a +migration step's `rationale` makes `check:upgrade-guide` fail and `--fix` regenerates +that one artifact and no other. The same run also showed the two gates have different +inputs — the conversion registry drives `spec-changes.json`, the rationale prose drives +`protocol-upgrade-guide.md` — which the earlier "both failed together" reading had +conflated. + +AGENTS.md's hand-rolled loop over eight hardcoded gate names is replaced by the command; +that list could not survive a ninth artifact, and the script's ledger can. diff --git a/AGENTS.md b/AGENTS.md index ce5686a7bd..8574771346 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -247,27 +247,41 @@ A `.describe()` string counts — it is not "just a comment", it lands in `content/docs/references/`. Adding one export counts — it lands in `api-surface.json`. Both were learned the hard way in #4040: two separate red builds, neither a logic error. -Cheapest way to be sure, since a gate is just its generator in `--check` mode — run the -whole set and let it tell you which artifact is stale: +Don't match by hand — one command runs **every** gate and reports **all** stale +artifacts at once, which is precisely what CI cannot do: ```bash -cd packages/spec -pnpm build # REQUIRED first — see the dist caveat below -for c in check:docs check:api-surface check:authorable-surface check:spec-changes \ - check:upgrade-guide check:skill-refs check:skill-docs check:react-blocks; do - printf '%-28s ' "$c"; pnpm -s $c >/dev/null 2>&1 && echo PASS || echo FAIL -done +pnpm --filter @objectstack/spec build # REQUIRED first — see the dist caveat +pnpm --filter @objectstack/spec check:generated # every gate; the first failure does not stop the rest +pnpm --filter @objectstack/spec check:generated --fix # regenerate ONLY the ones it proved stale ``` +`--fix` is deliberately narrow. Regenerating the whole set on principle destroys the +signal: it rewrites artifacts whose staleness you never saw, so a real semantic change +lands silently inside a mechanical diff. Let the check tell you which are stale, then +regenerate those. + +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 +failure mode a hardcoded list here would have had. (It caught its own `package.json` +entry on the very first run.) + ⚠️ **`check:api-surface` reads the built `dist/*.d.ts`, not `src/`.** A stale `dist` makes it report exports as **removed** — "N breaking (removed/narrowed)" — when nothing was removed at all: the snapshot is simply newer than your build. Rebuild before you believe it, and before you file a bug about `main` being red. (Two phantom "breaking -removals" this way while writing this section.) +removals" this way while writing this section; `check:generated` now prints this caveat +inline when that gate is the one failing.) `check:liveness`, `check:empty-state`, `check:skill-examples` and `check:react-conformance` are pure checks with no generator — a failure there is a real -finding to fix, not an artifact to regenerate. +finding to fix, not an artifact to regenerate. `check:generated` names them as +deliberately not run, so its "all up to date" never reads as "everything passed". + +Two generators have **no** gate at all — `gen:openapi` and `gen:sbom`. Nothing verifies +their output is current; the script reports that each run rather than staying silent +about it. --- diff --git a/packages/spec/package.json b/packages/spec/package.json index 01c4507899..c8a76fa551 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -189,6 +189,7 @@ "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: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", "gen:skill-docs": "tsx scripts/build-skill-docs.ts", diff --git a/packages/spec/scripts/check-generated.ts b/packages/spec/scripts/check-generated.ts new file mode 100644 index 0000000000..ff944e0d22 --- /dev/null +++ b/packages/spec/scripts/check-generated.ts @@ -0,0 +1,183 @@ +#!/usr/bin/env tsx +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Run **every** generated-artifact gate and report **all** stale artifacts in one + * pass. + * + * CI runs these gates as separate sequential steps across two jobs, so the first + * stale artifact masks the rest: you fix it, push, and discover the next one on + * the following run. That happened twice on #4040 (`check:docs`, then + * `check:api-surface`) and twice again on #4161 (`check:spec-changes`, then + * `check:upgrade-guide`) — four pushes spent learning something one local run + * could have told you. + * + * This is deliberately **not** a "regenerate everything" script. Blanket + * regeneration destroys the signal: it rewrites artifacts whose staleness you + * never saw, so a real semantic change lands silently inside a mechanical diff. + * What is worth automating is the *diagnosis* — which artifacts are stale, and + * the exact command for each. `--fix` then regenerates **only** the ones this run + * proved stale, and says so. + * + * Usage: + * pnpm --filter @objectstack/spec check:generated # report every stale artifact + * pnpm --filter @objectstack/spec check:generated --fix # + regenerate exactly those + */ + +import { execSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const pkgRoot = join(dirname(fileURLToPath(import.meta.url)), '..'); + +/** + * The gates that verify a checked-in artifact against its source, with the + * generator that rewrites each. Order is the cheapest-first order a human would + * want the answers in, not CI's. + */ +const GATED: ReadonlyArray<{ check: string; gen: string; artifact: string; readsDist?: true }> = [ + { check: 'check:spec-changes', gen: 'gen:spec-changes', artifact: 'spec-changes.json' }, + { check: 'check:upgrade-guide', gen: 'gen:upgrade-guide', artifact: 'docs/protocol-upgrade-guide.md' }, + { check: 'check:skill-docs', gen: 'gen:skill-docs', artifact: 'skill docs (from SKILL.md frontmatter)' }, + { check: 'check:skill-refs', gen: 'gen:skill-refs', artifact: 'skill references' }, + { check: 'check:react-blocks', gen: 'gen:react-blocks', artifact: 'react-blocks contract' }, + { check: 'check:authorable-surface', gen: 'gen:schema', artifact: 'authorable-surface.json + JSON schemas' }, + // Reads the BUILT `dist/*.d.ts`, not the source. On a stale dist it reports + // every export added since the last build as a "breaking removal" — a phantom + // 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/**' }, +]; + +/** + * Gates this script deliberately does NOT run. They audit the source for a + * property (liveness, conformance, example validity) rather than compare a + * checked-in artifact against its generator — there is nothing to regenerate, + * so a failure is a code change, not a `gen:` command. + */ +const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [ + { check: 'check:liveness', why: 'audits whether declared spec properties have a reader — no artifact' }, + { check: 'check:empty-state', why: 'audits empty-state coverage — no artifact' }, + { check: 'check:react-conformance', why: 'audits react blocks against their contract — no artifact' }, + { check: 'check:skill-examples', why: 'validates skill examples parse — no artifact' }, +]; + +/** + * Generators whose output NOTHING verifies. Recorded rather than ignored: each + * one is an artifact that can silently drift from its source, which is the class + * every gate above exists to prevent. Adding a gate for either is a real + * follow-up, not a formality. + */ +const UNGATED_GENERATORS: ReadonlyArray<{ gen: string; why: string }> = [ + { gen: 'gen:openapi', why: 'the OpenAPI document is generated but no check gate compares it to the routes' }, + { gen: 'gen:sbom', why: 'the SBOM is a release artifact, regenerated at publish time rather than checked in' }, +]; + +/** This aggregate itself — a `check:` script that gates nothing of its own. */ +const SELF = 'check:generated'; + +/** + * Reconcile the ledgers above against `package.json` — in BOTH directions, on + * every run rather than behind a `--self-test` flag, because the failure this + * prevents is a gate quietly dropping out of coverage. A gate absent from both + * lists would simply never run here, and the summary would still say "all + * artifacts up to date" — the exact shape of lie this script exists to remove. + * + * It works: the very first run rejected this script's own `package.json` entry + * as unclassified, before it had checked a single artifact. + */ +function reconcileLedger(scripts: Record): void { + const problems: string[] = []; + const declaredChecks = new Set([...GATED.map((g) => g.check), ...NO_GENERATOR.map((n) => n.check)]); + const declaredGens = new Set([...GATED.map((g) => g.gen), ...UNGATED_GENERATORS.map((u) => u.gen)]); + + for (const name of Object.keys(scripts)) { + if (name === SELF) continue; + if (name.startsWith('check:') && !declaredChecks.has(name)) { + problems.push(` \`${name}\` exists in package.json but is in neither GATED nor NO_GENERATOR.\n` + + ` Classify it: does it compare a checked-in artifact against a generator, or audit source?`); + } + if (name.startsWith('gen:') && !declaredGens.has(name)) { + problems.push(` \`${name}\` exists in package.json but no GATED entry names it and it is not in UNGATED_GENERATORS.\n` + + ` Either wire its gate in, or record why its output is unverified.`); + } + } + for (const { check } of GATED) if (!scripts[check]) problems.push(` GATED names \`${check}\`, which package.json no longer has.`); + for (const { gen } of GATED) if (!scripts[gen]) problems.push(` GATED names \`${gen}\`, which package.json no longer has.`); + for (const { check } of NO_GENERATOR) if (!scripts[check]) problems.push(` NO_GENERATOR names \`${check}\`, which package.json no longer has.`); + for (const { gen } of UNGATED_GENERATORS) if (!scripts[gen]) problems.push(` UNGATED_GENERATORS names \`${gen}\`, which package.json no longer has.`); + + if (problems.length) { + console.error(`✗ check:generated ledger is out of sync with package.json:\n\n${problems.join('\n')}\n`); + process.exit(1); + } +} + +function run(script: string): { ok: boolean; output: string } { + try { + const output = execSync(`pnpm -s ${script}`, { cwd: pkgRoot, stdio: ['ignore', 'pipe', 'pipe'] }).toString(); + return { ok: true, output }; + } catch (err: any) { + return { ok: false, output: `${err?.stdout?.toString() ?? ''}${err?.stderr?.toString() ?? ''}`.trim() }; + } +} + +const fix = process.argv.includes('--fix'); +const scripts = JSON.parse(readFileSync(join(pkgRoot, 'package.json'), 'utf8')).scripts ?? {}; +reconcileLedger(scripts); + +console.log(`Checking ${GATED.length} generated artifacts (every gate runs — the first failure does not stop the rest).\n`); + +const stale: typeof GATED[number][] = []; +for (const entry of GATED) { + const { ok, output } = run(entry.check); + console.log(` ${ok ? '✓' : '✗'} ${entry.check.padEnd(26)} ${entry.artifact}`); + if (!ok) { + stale.push(entry); + // The gates print their own prescription; surface it rather than paraphrasing. + const detail = output.split('\n').filter(Boolean).slice(0, 3).map((l) => ` ${l}`).join('\n'); + if (detail) console.log(detail); + if (entry.readsDist) { + console.log(` ⚠ this gate reads the BUILT dist, not the source — if you have not run`); + 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.`); + } + } +} + +// Narrowing is never silent: say what was deliberately not run. +console.log(`\nNot run here (${NO_GENERATOR.length} source audits with no artifact to regenerate): ` + + NO_GENERATOR.map((n) => n.check).join(', ')); +if (UNGATED_GENERATORS.length) { + console.log(`Generated but ungated (${UNGATED_GENERATORS.length}): ` + + UNGATED_GENERATORS.map((u) => u.gen).join(', ') + ' — nothing verifies these are current.'); +} + +if (!stale.length) { + console.log(`\n✓ All ${GATED.length} generated artifacts are up to date.`); + process.exit(0); +} + +console.log(`\n✗ ${stale.length} of ${GATED.length} artifact(s) stale:\n`); +for (const s of stale) console.log(` ${s.artifact}\n pnpm --filter @objectstack/spec ${s.gen}`); + +if (!fix) { + console.log(`\nRegenerate exactly these:\n ` + + stale.map((s) => `pnpm --filter @objectstack/spec ${s.gen}`).join(' && ') + + `\n\nOr re-run with --fix to do it now (only the ${stale.length} proved stale — never the whole set).`); + process.exit(1); +} + +console.log(`\n--fix: regenerating the ${stale.length} stale artifact(s). Review the diff before committing.\n`); +let failed = 0; +for (const s of stale) { + const { ok, output } = run(s.gen); + console.log(` ${ok ? '✓' : '✗'} ${s.gen}`); + if (!ok) { + failed++; + console.error(output.split('\n').filter(Boolean).slice(0, 5).map((l) => ` ${l}`).join('\n')); + } +} +process.exit(failed ? 1 : 0);