Skip to content

fix(spec): gen:api-surface / check:api-surface refuse a stale dist - #7183

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7122-api-surface-stale-dist
Aug 10, 2026
Merged

fix(spec): gen:api-surface / check:api-surface refuse a stale dist#7183
os-help merged 1 commit into
mainfrom
claude/issue-7122-api-surface-stale-dist

Conversation

@os-help

Copy link
Copy Markdown
Collaborator

Fixes#7122

build-api-surface.ts reads the built dist/** declarations and has always documented that as a precondition — "Reads the built dist — run after pnpm --filter @objectstack/spec build" — while enforcing nothing. Both modes now refuse before a single declaration file is read.

Premise check first: half of the card's premise is false

The card names two generators. Measured on origin/main @ 5087ac6:

  • build-api-surface.ts reads the dist. Confirmed — the exports map points its 16 entry points at ./dist/index.d.ts, ./dist/data/index.d.ts, and so on.
  • build-export-origins.ts does NOT read the dist. Its docblock says so in as many words at :75-77: "Reads src/, NOT the built dist — deliberately. … Nothing here needs pnpm build to have run." collectSourceEntries resolves src/index.ts and src/NAME/index.ts, never dist.

Triage's positive control on that file was itself a false positive. The "5 dist hits in build-export-origins.ts" that made the scan look trustworthy are a substring match:

75:dist ← the docblock saying it does NOT read the dist
77:dist ← same sentence
245:distinguishable
327:distinguishable
335:distinct

Word-boundary search returns exactly the two docblock lines. So the stale-dist laundering mechanism cannot apply to gen:export-origins, and this PR scopes to the one generator where it does. The gen:export-origins half of #7122 is disproved rather than deferred.

What lands

A guard in the generator, refusing in both modes per the ruling:

  • packages/spec/scripts/lib/dist-freshness.tsinspectDistFreshness(pkgDir, mode) returns a verdict plus the message, rather than exiting, so both directions can be driven from a test.
  • packages/spec/scripts/build-api-surface.ts — calls it immediately after CHECK is resolved and beforets.createProgram. Once the program exists over a stale dist, every answer below it is already wrong.
  • missing and stale are distinguished, as the ruling asked. They are one refusal with two causes and two fixes worth suggesting, and a never-built tree is the one state the old code already failed honestly in (the type checker could not resolve a module symbol) — that must not regress into something quieter.

No bootstrap path is broken.packages/spec's build script runs gen:schema, gen:openapi and tsup; it never runs gen:api-surface. CI runs check:api-surface after the build step (lint.yml comment: "Runs after the build step above"). Nothing invokes this generator before a dist exists.

Why the mtime rule and NOT dist/.build-input-hash — the suggested route, falsified

The card and the dispatch both suggested reusing the content stamp scripts/check-dev-prereqs.mjs writes. Measured, that stamp is the wrong primitive for this consumer, and wrong in the dangerous direction:

  1. It is blind to exactly this generator's artifact.packages/spec's build script is
    ... && tsup && if [ -z "$OS_SKIP_DTS" ]; then BUILD_DTS=true tsup; fi && node ../../scripts/check-dev-prereqs.mjs --stamp
    — the declaration pass is conditional, the stamp is not. After OS_SKIP_DTS=1 pnpm build the stamp matches the sources exactly while the declarations on disk are arbitrarily old. check-dev-prereqs.mjs:103-107 lists this as a known FALSE GREEN and names this gate as the one it breaks; AGENTS.md says the same from the other side ("skips exactly the artifact the gate inspects, and the check passes locally while failing in CI"). A stamp-based guard would be green on the one local build flag that guarantees a stale input.
  2. It is not importable.check-dev-prereqs.mjs ends in an unguarded top-level process.exit(report(inspect(ROOT))), so importing it runs the whole gate. Using it needs either an extraction from that script or a second copy of the hash definition.
  3. A freshness rule for this exact artifact already exists, is already imported by three spec-side consumers, and already ships a maintained .d.mts: distIsStale in scripts/check-regen-pending.mjs, whose own docblock names this trap — "gen:api-surface reads the BUILT dist/*.d.ts. On a stale dist it does not fail." It keys on dist/** declaration mtimes against src/** sources, so it catches the OS_SKIP_DTS=1 shape as well as the card's rebase-behind-the-dist shape.

So this adds no second notion of "is packages/spec/dist current". The stamp's own strength — content, not mtime (#5864) — is real and untouched; it still guards pnpm dev. It is simply blind to the half of the dist this generator is made of. Closing that hole in the stamp is a separate change to a script this card was told to treat as read-only, and scripts/check-dev-prereqs.mjs is unmodified here.

Why the guard is in the generator, not in another caller

It already lived in two callers and neither covers the path the reporter took: check:generated --fix refuses readsDist generators on a stale dist, and scripts/check-regen-pending.mjs refuses them at pre-commit. Running pnpm --filter @objectstack/spec gen:api-surface directly — what AGENTS.md tells you to do after changing an export, and what lint.yml runs — reaches neither. build-docs.ts made this exact move one artifact over in #4723, carrying schemaTreeIsStale itself "so EVERY caller is covered rather than this one".

Reverse verification — direction predicted before running

Predicted: on one stale tree, the pre-fix script renders a verdict and the post-fix script refuses. Both halves measured on the real package, with the guard taken out via git checkout origin/main -- FILE (never git stash).

The tree: packages/spec built, then packages/spec/src/contracts/job-service.ts — the file the card names — aged by mtime only, bytes untouched. distIsStale() flips false to true.

Pre-fix, --check, stale dist:

@objectstack/spec public API surface + factory signatures unchanged ✓
EXIT=0

Post-fix, same tree, --check:

❌ packages/spec/dist/**/*.d.ts is OLDER than packages/spec/src — the declarations on disk
predate the sources. If you built with OS_SKIP_DTS=1, that build did not rebuild them.
A verdict now would be computed against a build that no longer matches src, so this
check would report the public API "unchanged" without ever reading the exports under
test — a FALSE GREEN on exactly the change it exists to catch (#7122).
EXIT=1

Post-fix, same tree, generate mode: exit 1, damage stated as WRITE a baseline, and git status empty afterwards — the refused run wrote nothing.

Honest scope of that control: this reproduction ages the dist by mtime while its contents still match src, so the pre-fix green above is materially correct — what it demonstrates is that the gate renders a verdict on a dist it never vouched for. The wrong-baseline half is demonstrated instead by the end-to-end positive control below, and was measured in the field by the card itself (JobRunOutcome (interface) deleted from api-surface/contracts.json while the export was live).

Tests

packages/spec/scripts/dist-freshness.test.ts, 12 cases, 4.6s.

Eight unit cases drive the rule on real temp fixtures with explicit mtimes (no sleeps), including the two that a rule answering "fresh" unconditionally would pass and the two that a rule answering "stale" unconditionally would fail. One pins the OS_SKIP_DTS=1-on-an-already-built-tree shape — fresh JS, fresh .build-input-hash, stale declarations — which is green under a stamp-based guard and red under this one.

Four end-to-end cases spawn the real script in a repo-shaped sandbox that copies scripts/ and symlinks the read-only inputs, following build-schemas-check-mode.test.ts and its argument that a test-only seam is itself a place where the gate can differ from what CI runs. The first is a positive control: with a fresh fixture dist the script runs to completion and rewrites the seeded baseline. That is what makes the other three non-vacuous — the same tree, aged, leaves the baseline byte-identical, so the refusal is stopping a write that would otherwise have happened.

✓ POSITIVE CONTROL: a fresh dist runs to completion and rewrites the baseline 1271ms
✓ gen: refuses on a stale dist and writes NOTHING 784ms
✓ --check refuses on the same stale dist, so CI cannot pass against it either 761ms
✓ refuses a MISSING dist in both modes, naming the build rather than a phantom removal 1508ms
Test Files 1 passed (1) Tests 12 passed (12)

Gates run locally

gateresult
pnpm --filter @objectstack/spec test360 files / 9394 tests passed
pnpm --filter @objectstack/spec typecheckpass (tsc, check:scripts-typecheck, check:test-typecheck)
check:api-surfacepass
check:export-originspass (4970 exports / 16 entry points)
check:generatedpass, 11 of 11 artifacts current
check:generated --reconcile-onlypass, 20 check: + 14 gen: classified (no script added or renamed)
node scripts/check-nul-bytes.mjspass, 6595 files
eslint on the three changed filesclean

No changeset: packages/spec's published files list does not include scripts/, so nothing here ships. skip-changeset applied.

packages/spec/scripts/lib/sharded-artifacts.ts is not touched — it stays whole for #7076.

Out of scope, filed

#7181check:dual-source-exports, check:exported-any and check:skill-examples read the built declarations with the same unenforced precondition. All three are check-only, so none can launder a baseline into a commit, and CI runs them after the build; filed as an observation with the cheap adoption path rather than fixed here.


Generated by Claude Code

…7122)
`build-api-surface.ts` reads the built `dist/*.d.ts` and has always
documented that as a precondition without enforcing it. On a stale dist it
does not fail: it writes a baseline missing every export added since the
build, each of which reads as a BREAKING removal by this generator's own
rule -- and `--check` then compares that baseline against the same stale
dist and agrees. Green at every step.
Both modes now refuse before a single `.d.ts` is read, using the freshness
rule `check:generated --fix` and the pre-commit hook already share.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KJATVrh6V2ysutYUJigh3B
@vercel

vercelBot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 10, 2026 1:58am

Request Review

@os-helpos-help added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/l labels Aug 10, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gateteststooling

Projects

None yet

2 participants

@os-help@claude