Skip to content

fix(api-surface): record the shape behind every exported name - #953

Merged
drewstone merged 1 commit into
mainfrom
fix/api-surface-shapes
Aug 21, 2026
Merged

fix(api-surface): record the shape behind every exported name#953
drewstone merged 1 commit into
mainfrom
fix/api-surface-shapes

Conversation

@drewstone

@drewstone drewstone commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Problem

check:version-bump compares export names. It cannot see a shape, so it answered "consumer surface unchanged" for two merged pull requests that changed one:

PR The change What the gate said Version paid
#946 WorkerView.cwd removed from a public interface (src/tui/top-model.ts:102) package.json: consumer surface unchanged at 0.153.1 none
#949 'cost-unknown' added to the public PromotionVerdict.reason union, plus a new costUnknownTasks field (src/runtime/promotion-gate.ts:48) package.json: consumer surface unchanged at 0.153.2 minor, by human judgement

Both moved zero names, so api-surface.json was byte-identical across both. A gate that reports "safe" when the surface moved is worse than no gate: 0.154.0 was a judgement call that happened to be right, and 0.153.1 shipped a removed public field under a version the registry already held.

lib/api-surface.mjs named this limit in its own header — "never the shape behind a name … Deeper comparison would need a full type checker on both sides". That premise was wrong. The built .d.ts is the shape; no checker is needed to read it.

Change

api-surface.json now records a shape digest beside each kind — "WorkerView": "type 5e430bc3709c" — taken over the built declaration, normalized so only a consumer-visible edit moves it:

  • comments removed, whitespace collapsed;
  • the declaration's own local name blanked, so a bundler rename (AgentProfileAgentProfile$1 when two modules declare one identifier) moves nothing;
  • every type reference rewritten to a stable token: a symbol this package exports contributes its public name, one re-exported from a dependency contributes package:name, and one this package declares without exporting contributes its own digest.

Referencing by public name is what keeps the report readable: editing WorkerView moves WorkerView's line and not the 40 types that reach it. Measured on this tree, 1801 declarations are reachable from the public surface and only 10 are unexported, so inlining is the rare case, not the common one.

check:version-bump gains one bucket, export shape changed, and calls it breaking. Telling an added optional field from a removed required one is a subtyping question and this record states structure; guessing "additive" on a real break is the outcome the gate exists to stop. On 0.x that asks for a minor.

The boundary, and why it is there

The gate must not fire on internal-only work, so I measured it against the eleven pull requests merged into this repo yesterday — nine of which are pure internal refactors (abort cascade, spend fold, JSONL spine, lifecycle vocabulary, three copied primitives). Each pair was built from its own commit and compared with the new record:

PR Subject Shape changes reported
#942 cascade the abort reason through one linker 0
#943 fold conserved spend through one owner 0
#944 read and append every JSONL log through one spine 0
#945 record the answer on every terminal sandbox frame 0
#946 delete the worker cwd nothing ever wrote 1 — ./tui WorkerView
#947 give each lifecycle vocabulary one owner 0
#948 treat a stopped session as terminal 0
#950 accept Eval 0.163.2 and Knowledge 10.7.0 0
#951 give three copied primitives one owner each 0
#952 route the last abort linker through the one cascade 0
#949 refuse a candidate cheaper on unmeasured dollars 1 — ./kernel PromotionVerdict

2 reports across 11 pull requests and 2120 symbols, and both are the real ones. Zero false positives.

Verdicts on the two known misses, with the fixed gate:

PR #946  ab8c3135 -> c16bc4a8   0.153.1 -> 0.153.1
   shape ./tui WorkerView: shape 3c690034e9f9 -> 5e430bc3709c
severity breaking | required minor | paid none          <- now REFUSED

PR #949  1f11e476 -> 7f05cdbd   0.153.2 -> 0.154.0
   shape ./kernel PromotionVerdict: shape 895cf83bdc16 -> a1a5288291cc
severity breaking | required minor | paid minor         <- passes, for the stated reason
  ... with the version reverted to 0.153.2:
severity breaking | required minor | paid none          <- REFUSED

Four things deliberately do not move a digest, each pinned by a test: a doc comment, a reformat, a bundler rename, and an edit to a type this package does not export through the changed symbol.

What this still does not see

A dependency's own type moving under a fixed range. A re-exported external symbol records package:name, never that package's structure, because it moves with the dependency range — which the manifest half of this check already gates. Widening the digest to node_modules would make the record depend on what happens to be installed.

Proof

pnpm run lint                 615 files, no fixes
pnpm run typecheck            clean (tsc --noEmit + examples)
pnpm run build                clean
pnpm run check:api-surface    2120 exports / 17 entry points, record current
                              bench: 223 exports / 41 entry points, record current
pnpm run check:version-bump    consumer surface unchanged at 0.154.0  (scripts/ is not published)
pnpm test                     2804 passed / 170 failed across 20 files
  clean origin/main, same machine, same run:
                              2797 passed / 171 failed across 21 files
  The 20 failing files are a strict SUBSET of the baseline's 21 (`tests/mcp/worktree-harness.test.ts`
  flaked on the baseline and passed here). Zero new failures; +7 passes = the 6 tests
  added here plus that flake. All are macOS git-worktree and process-spawn timeouts;
  CI on Linux is the authority.

The record survives a clean rm -rf dist && pnpm run build byte-identically, and --write is idempotent.

No version bump: scripts/ and api-surface.json are outside files, so nothing a consumer installs changed.

Simplification

Simplification: the shape half of the surface record replaces the human judgement that decided 0.154.0; the two scripts and the shared lib keep one owner for the record format, and check-version-bump.mjs still needs nothing but git.
Net: +621 / -46 lines of script and test across 5 files, plus the two regenerated records; 0 paths removed, 1 blind spot closed.
Not done here: agent-knowledge carries these three files byte-identical and needs the same change plus a regenerated record — a separate pull request in that repository, because its record must be regenerated from its own build. agent-eval does not have these scripts at all.

Tests: +6 (api-surface-record.test.ts — a removed field moves its type's digest and nothing else's, a union member added moves it, a doc comment or reformat does not, a bundler rename does not; version-bump-check.test.ts — a shape move with no bump is refused and names the symbol, and a base record that states no shape is not compared), -0 deleted.

Refs #946, #949

check:version-bump compared export names only, so it reported "consumer
surface unchanged" for #946, which removed WorkerView.cwd from a public
interface, and for #949, which added a member to the public
PromotionVerdict.reason union. Neither moved a name, so api-surface.json was
byte-identical across both and the 0.154.0 bump was a human judgement rather
than a gate result.

The record now states a shape digest beside each kind, taken over the built
declaration with comments removed, whitespace collapsed, the declaration's own
local name blanked, and every type reference rewritten to a stable token. A
reference to a symbol this package exports contributes that symbol's public
name, so an edit is reported once, on the line of the symbol that changed.

A shape move is classified breaking: telling an added optional field from a
removed required one is a subtyping question, and the record states structure.

Refs #946, #949

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — e3bad527

This PR was opened by the trusted drewstone account.

This approval is provisional and was applied by the local stand-in because the pr-reviewer webhook host is unreachable (2026-08-21). CI on this head is fully green. The full PR reviewer audit re-runs via the resweep when the service returns and will publish findings if it detects issues.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants