ci(spec,type-check): spec-named symbols must be derived, 12 packages type-check their tests, ActionDef's keys get an inventory - #3032
Merged
Conversation
…ctstack#4115) objectui declares local types and consts under the same names as `@objectstack/spec` exports, with doc comments claiming spec canonicity. Every audited instance had either drifted or was one spec release away from it: `ActionType` (hand union, missing `form`), `ChartType` (7 of 19 members), `ActionLocation` + 2 (re-declared union + tuple + `z.enum` under a comment that said "re-export here"), `ActionParamFieldType` (16 of 49 members). The cost is specific to agent-driven development: an agent reads the comment, takes it as ground truth, and builds on it — #2901 was filed with a backwards premise because the fork carried the spec's own symbol name. A wrong canonical-claim is not stale documentation, it is a planted premise. Lands as the check rather than an AGENTS.md paragraph, which is the same "declared != enforced" landmine the thread is about. - Enumerates all 4231 `@objectstack/spec` export names (types AND values, via the compiler's view of each subpath's `.d.ts` — a runtime `import()` sees only values, and the drifted symbols are mostly types). - Flags an exported declaration whose name the spec owns unless it is derived: re-exported from the spec, or declared with a spec import in a STRUCTURAL position. A spec name merely mentioned inside a members block does not count — that is what a fork looks like. Consts do not descend into array/object literals, because a faithful copy passes every value comparison and only reference identity distinguishes a re-export from a fork. - Ledger of the 156 pre-existing collisions names each symbol rather than counting per package: a count is a budget that lets the next fork land as long as an unrelated one was fixed, and it makes the failure point at whichever symbol sorts first instead of the one just written. `--ledger` regenerates it. - Shrink-only both ways: a fresh collision fails by name, and a fixed symbol fails until its ledger entry is deleted. Barrel re-exports through a relative path or a `@object-ui/*` sibling are not counted twice; the declaration site they point at is judged on its own turn. Discrimination proof — three ways it fails, run before landing: 1. reverting `ActionType` to the pre-#2231 hand union names it exactly: "type `ActionType` packages/types/src/ui-action.ts:80 (exported by `@objectstack/spec/ui`)" 2. a new fork in a package with an empty ledger fails on the PR that writes it 3. renaming a ledger symbol to a dialect fails until its entry is deleted Refs objectstack#4115, objectstack#4074, #3003, #3009, #2944, #2911 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
…now declared (objectstack#4118)
A package tsconfig excludes test files — correctly, it is the BUILD config and
they would emit into `dist`. The hole is that no other `tsc` invocation read them
either, so for most packages nothing compiled a test at all. objectui#3009 found
what that costs: `spec-derived-unions.test.ts` had built its whole contract on
`satisfies` checks that never ran, under a header calling them "the real
enforcement". Reverting a derived alias produced zero errors.
Tests are where agents encode their understanding of a contract. Unchecked, a
wrong understanding accumulates silently and then reads as evidence.
Wires up every package measured at zero code-tier errors — 12, not the 8 the
issue estimated. The extra four fell out of a better config template:
`plugin-calendar` and `plugin-kanban` needed only the jest-dom matcher types,
`providers` was already clean, and `sdui-parser`'s 50 TS2304s were env globals
exactly as the issue suspected, not real errors.
Wiring `sdui-parser` up surfaced a real bug: `render.test.tsx` imports
`@object-ui/core` and `@object-ui/react` with neither declared in its
package.json. Both were phantom — resolved only through the ROOT tsconfig's
`paths` alias and vitest, never as a dependency a consumer would get. Declared as
devDependencies, which is also what makes the `paths: {}` in the new config
meaningful: with the alias in place an undeclared workspace import still compiles.
Every `tsconfig.test.json` follows objectui#3009's template — `noEmit`,
`composite: false`, `paths: {}` to kill the TS6059 source-leak noise, and `lib` /
`types` only where the tests actually need them (`mobile` reads
`Array.prototype.at`, so ES2022; `plugin-report` pulls in a source file that
reads `process.env`, so `node`).
The guard grows a second half so this cannot silently regress. A package with
test files either compiles them or carries a TEST_DEBT entry with a MEASURED
count — re-measured against current main, not copied from the issue's table:
240 code-tier errors across 15 packages. It also fails on a `tsconfig.test.json`
that nothing runs, which is the objectui#3009 shape itself: a file that looks
like enforcement while no compiler reads it.
Discrimination proof — three ways it fails, run before landing:
1. unchaining plugin-tree's config: "has a tsconfig.test.json that its
type-check script never runs … the exact shape of objectui#3009"
2. deleting the `permissions` TEST_DEBT entry: "has 3 test files that no `tsc`
invocation reads"
3. a package listed in both DEBT and TEST_DEBT is rejected as one gap counted
twice (this is how `layout` was caught during the sweep)
Refs objectstack#4118, objectui#3009, objectstack#4074, #2911, #2901
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
…eads (objectstack#4075 step 1) `ActionDef` ends with `[key: string]: any`, so it accepts any key of any type. Deleting `ActionDef.execute` produced ZERO compile errors even though the field had just been removed (objectui#2990), and stale metadata still authoring `execute: 'markDone'` type-checks today. The same deletion against `@object-ui/types`' `ActionSchema` — no index signature — correctly produced TS2353 at the authoring site. One reader can catch a retired key; the other is structurally incapable. Step 1 is the non-breaking half: make the key set visible, warn on anything outside it, change no types. What the inventory found, which is step 2's worklist: - 18 keys the SPEC owns that `ActionDef` never declared — `visible`, `locations`, `icon`, `variant`, `order`, `component`, `bulkEnabled`, `requiredPermissions` and 10 more. `ActionEngine` reads two of them through `as any` casts (`(ra.action as any).visible`, `(action as any).locations`); those casts exist only because the field is undeclared. - 17 keys `ActionDef` declares that the spec does not own — `actionType`, `api`, `chain`, `onClick`, `toast` and others, several already marked legacy. This is objectui's own dialect and needs the #4115 treatment: documented as deliberate, or retired. - `execute` is not simply gone from the spec — it is a live TOMBSTONE, still in `ActionSchema` so the parser can reject it BY NAME with the rename prescription. So it is warned about separately and more loudly than an unknown key: an unknown key is probably a typo, a retired key is metadata that used to work. - `to` / `external` / `newTab` / `replace` are a real objectui dialect — the `navigation` alias's own spelling, read off the action when no nested `navigate` object is present. Declared here rather than left to trip the warning, with a tripwire test that fails if the spec ever adopts one. The lists are data because TypeScript types do not survive to runtime, and `keyof ActionDef` cannot substitute — the index signature widens it to `string | number`, which is the problem itself. `actionKeys.pin.test.ts` re-derives each list from its real source (the interface via the AST, the spec via its schema) so a hand-maintained list cannot drift from what it mirrors. The spec key list is pinned rather than resolved at runtime: `ActionSchema` is a lazy proxy that does not forward `.shape`, so reading it means walking zod internals — fine in a test, wrong in shipped code. `@object-ui/types` made the same call for `ActionComponent`. Discrimination proof: dropping any entry from `ACTION_DEF_KEYS` fails with "ActionDef declares keys the inventory is missing: <name>"; the warning tests assert it stays silent on a well-formed action, names `targt`, gives `execute` its rename prescription, warns once rather than per click, and says nothing in production. Core type-checks clean and all 207 existing action tests still pass. One pin is deliberately inverted: the suite asserts the index signature is STILL THERE. The day that fails, step 3 has landed, `tsc` catches unknown keys itself, and this warning plus `executeScript`'s rename branch can retire together. Refs objectstack#4075, objectstack#3903, objectui#2990, #3856, #3855, #2169, AGENTS.md PD #12 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
Also drops the last `no-explicit-any` lint warning from the pin test's zod walk — `Record<string, unknown>` with a narrowing helper reads the same internals without the escape hatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
Leftover template-string noise from drafting; `${"components"}/src/ui/` is
just "components/src/ui/".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
Keying the warn-once memo on the unknown keys alone reported the FIRST action carrying `targt` and stayed silent about every other one — sending the author to fix a button that was only the first symptom. Keyed by action name too; the memo is bounded by the number of authored actions either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 12:39
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three issues, one theme: a rule that isn't a check isn't a rule. Each lands as CI, and each ships with a run showing the guard failing against pre-fix code.
ActionDef's keys, warn on the ones nothing readsobjectstack#4115 —
scripts/check-spec-symbol-derivation.mjsobjectui declares local types and consts under the same names as
@objectstack/specexports, with doc comments claiming spec canonicity. Every audited instance had drifted or was one release from it:ActionType(missingform),ChartType(7 of 19 members),ActionLocation+ 2 (re-declared union + tuple +z.enumunder a comment that said "re-export here"),ActionParamFieldType(16 of 49).The cost is specific to agent-driven development: an agent reads the comment, takes it as ground truth, and builds on it. #2901 was filed with a backwards premise because the fork carried the spec's own symbol name.
How it works. Enumerates all 4231 spec export names — types and values, through the compiler's view of each subpath's
.d.ts, because a runtimeimport()sees only values and the drifted symbols are mostly types. Then flags an exported declaration whose name the spec owns, unless it is derived.Both sanctioned forms pass, because both bind the local name to the spec at compile time:
Derivation must sit in a structural position — a type alias's own type node, an interface's
extends, a const's initializer. A spec name merely mentioned inside a members block does not count, because that is what a fork looks like. Consts are stricter: array and object literals are not descended into, soexport const ACTION_LOCATIONS = [...SpecLocations]is still flagged — a faithful copy passes every value comparison, and only reference identity distinguishes a re-export from a fork (#3003).The ledger names symbols, not counts. 156 pre-existing collisions across 19 packages, listed by name. A per-package count is a budget: it lets the next fork land as long as an unrelated one was fixed in the same PR, and it makes the failure message point at whichever symbol sorts first rather than the one just written.
--ledgerregenerates the block, so burning them down is mechanical.Barrel re-exports through a relative path or an
@object-ui/*sibling aren't counted twice — the declaration they point at is judged on its own turn.Discrimination proof — three ways it fails
1. Revert
ActionTypeto the pre-#2231 hand-written union:2. A new fork in a package with an empty ledger:
3. Fixing a ledger symbol fails until its entry is deleted (shrink-only):
Runs in the
type-checkjob after install (it reads the spec's.d.ts, not the build). 5.5s.objectstack#4118 — 12 packages type-check their tests
A package tsconfig excludes test files — correctly, it is the BUILD config and they'd emit into
dist. The hole is that no othertscinvocation read them. objectui#3009 found what that costs:spec-derived-unions.test.tshad built its whole contract onsatisfieschecks that never ran, under a header calling them "the real enforcement". Reverting a derived alias produced zero errors.12 free wins, not 8. The extra four fell out of a better config template:
plugin-calendarandplugin-kanbanneeded only the jest-dom matcher types,providerswas already clean, andsdui-parser's 50TS2304s were env globals — exactly as the issue suspected, not real errors.Wiring
sdui-parserup surfaced a real bug.render.test.tsximports@object-ui/coreand@object-ui/reactwith neither declared in itspackage.json— phantom dependencies resolved only through the root tsconfig'spathsalias and vitest, never as a dependency a consumer would get. Both are devDependencies now, which is also what makespaths: {}in the new config meaningful: with the alias in place, an undeclared workspace import still compiles.Every
tsconfig.test.jsonfollows objectui#3009's template —noEmit,composite: false,paths: {}to kill the TS6059 source-leak noise, andlib/typesonly where the tests actually need them (mobilereadsArray.prototype.at, so ES2022;plugin-reportpulls in a source file that readsprocess.env, sonode).The guard grows a second half so this can't silently regress. A package with test files either compiles them or carries a
TEST_DEBTentry with a measured count — re-measured against currentmain, not copied from the issue's table: 240 code-tier errors across 15 packages. It also fails on atsconfig.test.jsonthat nothing runs, which is the objectui#3009 shape itself.Discrimination proof — three ways it fails
1. Unchain
plugin-tree's config:2. Delete the
permissionsTEST_DEBT entry:3. A package in both DEBT and TEST_DEBT is rejected as one gap counted twice — this is how
layoutwas caught during the sweep.objectstack#4075 step 1 —
ActionDef's key inventoryActionDefends with[key: string]: any. DeletingActionDef.executeproduced zero compile errors (objectui#2990); the same deletion against@object-ui/types'ActionSchema— no index signature — correctly producedTS2353. One reader can catch a retired key, the other is structurally incapable. That's how a typo (targt) and a tombstoned key both reach a runner that then silently does nothing — the #2169 shape.Step 1 is the non-breaking half: make the key set visible, warn on anything outside it, change no types.
What the inventory found — step 2's worklist:
ActionDefnever declaredvisible,locations,icon,variant,order,component,bulkEnabled,requiredPermissions,ai,aria,bodyExtra,bodyShape,mode,objectName,recordIdField,recordIdParam,requiresFeature,shortcutActionDefdeclares that the spec doesn't ownactionType,api,chain,chainMode,close,condition,confirm,endpoint,modal,navigate,onClick,onFailure,onSuccess,redirect,reload,toast,actionParamsActionEnginereads two of the first group throughas anycasts —(ra.action as any).visible,(action as any).locations. Those casts exist only because the field is undeclared, and they go away when step 2 promotes them.Two things the inventory corrected:
executeis not simply gone from the spec — it's a live tombstone, still inActionSchemaso the parser can reject it by name with the rename prescription (verified by parsing). So it's warned about separately and more loudly: an unknown key is probably a typo, a retired key is metadata that used to work.to/external/newTab/replaceare a real objectui dialect — thenavigationalias's own spelling, read off the action when no nestednavigateobject is present. Declared rather than left to trip the warning, with a tripwire test that fails if the spec ever adopts one.The lists are data because TS types don't survive to runtime, and
keyof ActionDefcan't substitute — the index signature widens it tostring | number, which is the problem itself.actionKeys.pin.test.tsre-derives each list from its real source (the interface via the AST, the spec via its schema), so a hand-maintained list can't drift from what it mirrors.One pin is deliberately inverted: the suite asserts the index signature is still there. The day that fails, step 3 has landed,
tsccatches unknown keys itself, and this warning plusexecuteScript's rename branch retire together.Discrimination proof
Dropping any entry from
ACTION_DEF_KEYSfails by name:The warning tests assert it stays silent on a well-formed action, names
targt, givesexecuteits rename prescription, warns once per action rather than per click, still names a second action carrying the same typo, and says nothing in production.Verification
pnpm type-check— 76/76 tasks successful (3m51s)node scripts/check-type-check-coverage.mjs— green, both halvesnode scripts/check-spec-symbol-derivation.mjs— green (5.5s)node scripts/check-lint-coverage.mjs— 45/45 packages, 0 errorsnode scripts/check-changeset-fixed.mjs— greeneslinton all new/changed files — 0 errors, 0 warningsChangeset:
@object-ui/coreminor (new public exports). The other 11 packages change only build wiring, so no runtime version bump.Scope note: all three issues are filed on
objectstack-ai/objectstackbut every change is inobjectui; the sibling repo is untouched.🤖 Generated with Claude Code
Generated by Claude Code