diff --git a/.changeset/action-key-inventory-and-unknown-key-warning.md b/.changeset/action-key-inventory-and-unknown-key-warning.md new file mode 100644 index 000000000..3cc7afd24 --- /dev/null +++ b/.changeset/action-key-inventory-and-unknown-key-warning.md @@ -0,0 +1,37 @@ +--- +"@object-ui/core": minor +--- + +feat(core): inventory `ActionDef`'s keys and warn on the ones nothing reads — 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` — which has no index signature — correctly +produced `TS2353` at the authoring site. One of the two readers can catch a +retired key; the other is structurally incapable, which is how a typo (`targt`) +and a tombstoned key both reach a runner that then silently does nothing. + +This is the non-breaking first step of the staged narrowing: it makes the key set +**visible** and warns on anything outside it, without changing a single type. + +New exports from `@object-ui/core`: + +- `ACTION_DEF_KEYS`, `SPEC_ACTION_KEYS`, `NAVIGATION_ALIAS_KEYS`, + `RETIRED_ACTION_KEYS`, `KNOWN_ACTION_KEYS` — the inventory. +- `classifyActionKeys(action)` — splits an action's own keys into `unknown` and + `retired`. +- `warnOnUnknownActionKeys(action)` — dev-mode only, warn-once. Called by + `ActionRunner.execute`, so no consumer wiring is needed. + +A retired key gets a louder, more specific warning than an unknown one: an +unknown key is probably a typo, a retired key is metadata that used to work. +`execute` is not simply gone from the spec — it is a live **tombstone**, still +present in `ActionSchema` so the parser can reject it by name with the rename +prescription. + +Nothing is rejected and no types changed, so existing metadata behaves exactly as +before. Promoting the legitimate keys to explicit optional fields, then removing +the index signature so `tsc` catches both typos and retired keys, are steps 2 and +3 of objectstack#4075. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13a2f85f8..af9ca5b16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,6 +75,14 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # A local type/const declared under a `@objectstack/spec` export's NAME reads + # to the next agent as the spec's own definition — four such symbols had + # already drifted from the spec they claimed to be (objectstack#4115). Needs + # the install (it reads the spec's own `.d.ts`) but not the build, so it runs + # before the expensive steps. + - name: Verify spec-named symbols are derived, not hand-written + run: pnpm check:spec-symbols + - name: Turbo Cache uses: actions/cache@v6 with: diff --git a/package.json b/package.json index 05d37caff..3c43cb910 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "lint:coverage": "node scripts/check-lint-coverage.mjs", "type-check": "turbo run type-check", "type-check:coverage": "node scripts/check-type-check-coverage.mjs", + "check:spec-symbols": "node scripts/check-spec-symbol-derivation.mjs", "cli": "node packages/cli/dist/cli.js", "objectui": "node packages/cli/dist/cli.js", "create-plugin": "node packages/create-plugin/dist/index.js", diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 3e1eaa20e..0a075f489 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -24,6 +24,7 @@ import type { RunnableActionType } from '@object-ui/types'; import { ExpressionEvaluator } from '../evaluator/ExpressionEvaluator'; import { globalUndoManager, type UndoableOperation } from './UndoManager'; +import { warnOnUnknownActionKeys } from './actionKeys'; export interface ActionResult { success: boolean; @@ -475,6 +476,12 @@ export class ActionRunner { async execute(action: ActionDef): Promise { try { + // `ActionDef` accepts any key of any type, so a typo (`targt`) and a + // retired key (`execute`) both reach here having type-checked. Neither + // binds a handler, and binding no handler silently is the #2169 "Mark Done + // does nothing" shape. Dev-only, warn-once, changes nothing (#4075 step 1). + warnOnUnknownActionKeys(action); + // Resolve the action type const actionType = action.type || action.actionType || action.name || ''; diff --git a/packages/core/src/actions/__tests__/actionKeys.pin.test.ts b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts new file mode 100644 index 000000000..c9d695d02 --- /dev/null +++ b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts @@ -0,0 +1,181 @@ +/** + * Pins the key inventory in `actionKeys.ts` to the two things it mirrors — + * `ActionDef`'s own declarations and `@objectstack/spec`'s `ActionSchema` + * (objectstack#4075 step 1). + * + * Why a test rather than a comment: a hand-maintained list that silently drifts + * from the interface it claims to mirror is the exact "declared ≠ enforced" + * failure this work is about. `ActionDef` cannot be enumerated at runtime — its + * `[key: string]: any` widens `keyof` to `string | number` — so the list is data, + * and this file is what makes the data true. Adding a field to `ActionDef` + * without adding it here fails, by name. + * + * Discrimination proof for the guard below: with `ACTION_DEF_KEYS` complete these + * pass; dropping any single entry (e.g. `target`) fails with + * "ActionDef declares keys the inventory is missing: target". + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; +import ts from 'typescript'; +import * as SpecUI from '@objectstack/spec/ui'; +import { + ACTION_DEF_KEYS, + SPEC_ACTION_KEYS, + NAVIGATION_ALIAS_KEYS, + RETIRED_ACTION_KEYS, + KNOWN_ACTION_KEYS, + classifyActionKeys, + warnOnUnknownActionKeys, + resetActionKeyWarnings, +} from '../actionKeys'; + +const RUNNER = join(dirname(fileURLToPath(import.meta.url)), '..', 'ActionRunner.ts'); + +/** `ActionDef`'s declared property names, read off the interface itself. */ +function declaredActionDefKeys(): string[] { + const sf = ts.createSourceFile(RUNNER, readFileSync(RUNNER, 'utf8'), ts.ScriptTarget.Latest, true); + for (const stmt of sf.statements) { + if (!ts.isInterfaceDeclaration(stmt) || stmt.name.text !== 'ActionDef') continue; + return stmt.members + .filter(ts.isPropertySignature) + .map((m) => (m.name && (ts.isIdentifier(m.name) || ts.isStringLiteral(m.name)) ? m.name.text : null)) + .filter((n): n is string => n !== null); + } + throw new Error('ActionDef interface not found in ActionRunner.ts'); +} + +/** + * The spec's `ActionSchema` keys. `ActionSchema` is a lazy proxy that does not + * forward `.shape`, so this walks zod internals to reach the object shape — + * acceptable HERE (a test pins a fact) and deliberately not done in shipped code. + */ +function specActionKeys(): string[] { + const seen = new Set(); + const walk = (schema: unknown, depth = 0): string[] | null => { + if (!schema || depth > 8 || seen.has(schema)) return null; + seen.add(schema); + const s = schema as Record; + const shapeOf = (v: unknown): string[] | null => + v && typeof v === 'object' ? Object.keys(v as object) : null; + if (s.shape) return shapeOf(s.shape); + const def = (s._def ?? s.def) as Record | undefined; + if (!def) return null; + if (def.shape) return shapeOf(def.shape); + for (const key of ['in', 'out', 'innerType', 'schema', 'left', 'right']) { + const found = def[key] ? walk(def[key], depth + 1) : null; + if (found) return found; + } + return null; + }; + const keys = walk(SpecUI.ActionSchema); + if (!keys) throw new Error('could not resolve @objectstack/spec/ui ActionSchema shape'); + return keys; +} + +describe('action key inventory (objectstack#4075 step 1)', () => { + it('ActionDef still has the index signature this inventory compensates for', () => { + // The day this fails, step 3 has landed: `tsc` catches unknown keys itself + // and the dev-mode warning (plus `executeScript`'s rename branch) can retire. + expect(readFileSync(RUNNER, 'utf8')).toContain('[key: string]: any'); + }); + + it('lists every key ActionDef declares', () => { + const declared = declaredActionDefKeys(); + const missing = declared.filter((k) => !(ACTION_DEF_KEYS as readonly string[]).includes(k)); + const stale = (ACTION_DEF_KEYS as readonly string[]).filter((k) => !declared.includes(k)); + expect({ missing, stale }).toEqual({ missing: [], stale: [] }); + }); + + it('lists every key the spec ActionSchema declares', () => { + const spec = specActionKeys(); + const missing = spec.filter((k) => !(SPEC_ACTION_KEYS as readonly string[]).includes(k)); + const stale = (SPEC_ACTION_KEYS as readonly string[]).filter((k) => !spec.includes(k)); + // `missing` means the spec grew a key objectui does not know about; `stale` + // means it dropped one. Either way the inventory has to be re-derived, and + // the diff names exactly which key moved. + expect({ missing, stale }).toEqual({ missing: [], stale: [] }); + }); + + it('`execute` is still a live spec tombstone, so it must not count as known', () => { + const parsed = SpecUI.ActionSchema.safeParse({ + name: 'mark_done', + label: 'Mark Done', + type: 'script', + execute: 'markDone', + }); + expect(parsed.success).toBe(false); + expect(RETIRED_ACTION_KEYS).toHaveProperty('execute'); + expect(KNOWN_ACTION_KEYS.has('execute')).toBe(false); + }); + + it('keeps the navigation alias out of the spec vocabulary it is not part of', () => { + // If the spec ever adopts one of these, it stops being objectui dialect and + // this fails — naming the alias to retire, the same tripwire shape as + // `ObjectUiLocalActionType`. + const spec = specActionKeys(); + expect(NAVIGATION_ALIAS_KEYS.filter((k) => spec.includes(k))).toEqual([]); + }); +}); + +describe('unknown-key warning', () => { + let warn: ReturnType; + + beforeEach(() => { + resetActionKeyWarnings(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => warn.mockRestore()); + + it('says nothing about an action built only from recognized keys', () => { + warnOnUnknownActionKeys({ name: 'save', type: 'api', target: '/api/v1/save', locations: ['record_header'] }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('names a typo that the compiler cannot see', () => { + // `targt` type-checks today: ActionDef's index signature accepts it. Nothing + // reads it, so the action runs and does nothing — #2169's shape. + warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'saveRecord' }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('`targt`'); + }); + + it('gives a retired key its rename prescription, not a bare "unknown"', () => { + warnOnUnknownActionKeys({ name: 'mark_done', type: 'script', execute: 'markDone' }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('rename the key to `target`'); + }); + + it('warns once per action, not once per click', () => { + for (let i = 0; i < 5; i++) warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('still names a SECOND action carrying the same typo', () => { + // Keying the warn-once memo on the keys alone would report `save` and stay + // silent about `remove` — sending the author to fix the first symptom only. + warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' }); + warnOnUnknownActionKeys({ name: 'remove', type: 'script', targt: 'y' }); + expect(warn).toHaveBeenCalledTimes(2); + expect(warn.mock.calls[1][0]).toContain('"remove"'); + }); + + it('is silent in production', () => { + const prev = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + try { + warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' }); + expect(warn).not.toHaveBeenCalled(); + } finally { + process.env.NODE_ENV = prev; + } + }); + + it('classifies unknown and retired keys separately', () => { + expect(classifyActionKeys({ type: 'script', execute: 'a', targt: 'b' })).toEqual({ + unknown: ['targt'], + retired: ['execute'], + }); + }); +}); diff --git a/packages/core/src/actions/actionKeys.ts b/packages/core/src/actions/actionKeys.ts new file mode 100644 index 000000000..0d52ec650 --- /dev/null +++ b/packages/core/src/actions/actionKeys.ts @@ -0,0 +1,255 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * What an action is allowed to carry — objectstack#4075 step 1. + * + * `ActionDef` ends with `[key: string]: any`, so it accepts any key of any type. + * Concretely (objectui#2990): deleting `ActionDef.execute` produced ZERO compile + * errors even though the field had just been removed, and stale metadata still + * authoring `execute: 'markDone'` type-checks today. The same deletion against + * `@object-ui/types`' `ActionSchema` — which has no index signature — correctly + * produced `TS2353` at the authoring site. One of the two readers can catch a + * retired key; the other is structurally incapable. + * + * That asymmetry is the issue. An open key set on a DECLARED METADATA CONTRACT + * is what lets a typo (`targt`, `exectue`) and a tombstoned key (`execute`) both + * sail through to a runner that then silently does nothing — the #2169 "Mark + * Done does nothing" shape. + * + * This module is step 1 of the staged narrowing: it makes the key set VISIBLE + * and warns on anything outside it, without changing a single type. Nothing + * breaks, and an invisible failure becomes an audible one. Steps 2 and 3 — + * promoting the legitimate keys to explicit optional fields, then removing the + * index signature — are what finally let `tsc` catch typos and retired keys. + * + * The lists are pinned by `__tests__/actionKeys.pin.test.ts`, which re-derives + * each one from its actual source. A hand-maintained list that drifts from the + * interface it mirrors would be the same "declared ≠ enforced" trap this whole + * thread is about. + * + * ── What the inventory found, i.e. step 2's worklist ───────────────────────── + * `ActionDef` declares 34 keys, the spec's `ActionSchema` declares 36, and they + * overlap on 17. The two halves of the difference are different problems: + * + * 18 keys the SPEC owns that `ActionDef` never declared — `ai`, `aria`, + * `bodyExtra`, `bodyShape`, `bulkEnabled`, `component`, `icon`, `locations`, + * `mode`, `objectName`, `order`, `recordIdField`, `recordIdParam`, + * `requiredPermissions`, `requiresFeature`, `shortcut`, `variant`, `visible`. + * These are authored today and reach readers through the index signature. + * `ActionEngine` reads two of them (`visible` at the location filter, `locations` + * at registration) through an `as any` cast — a cast that exists ONLY because the + * field is undeclared, and that goes away when step 2 promotes them. + * + * 17 keys `ActionDef` declares that the spec does not own — `actionType`, `api`, + * `chain`, `chainMode`, `close`, `condition`, `confirm`, `endpoint`, `modal`, + * `navigate`, `onClick`, `onFailure`, `onSuccess`, `redirect`, `reload`, `toast`, + * `actionParams`. Several are already marked legacy at their declaration. These + * are objectui's own dialect and need the #4115 treatment: kept and documented as + * deliberate, or retired — but not silently carried. + */ + +/** + * Every property `ActionDef` declares, in declaration order. + * + * Kept as data because TypeScript types do not survive to runtime — and + * `keyof ActionDef` cannot substitute for this list: the index signature widens + * it to `string | number`, which is precisely the problem being worked around. + */ +export const ACTION_DEF_KEYS = [ + 'type', + 'actionType', + 'name', + 'label', + 'confirmText', + 'confirm', + 'condition', + 'disabled', + 'api', + 'endpoint', + 'method', + 'navigate', + 'onClick', + 'reload', + 'close', + 'redirect', + 'toast', + 'successMessage', + 'errorMessage', + 'refreshAfter', + 'undoable', + 'params', + 'actionParams', + 'resultDialog', + 'target', + 'body', + 'openIn', + 'modal', + 'chain', + 'chainMode', + 'onSuccess', + 'onFailure', + 'opensInNewTab', + 'newTabUrl', +] as const; + +/** + * Every property `@objectstack/spec`'s `ActionSchema` declares. + * + * `ActionDef` mirrors that schema, so a key the spec owns is legitimate even + * when `ActionDef` has not declared it yet — several are read today only through + * an `as any` cast (`visible` and `locations` in `ActionEngine`), which is the + * cast this list exists to eventually retire. + * + * Pinned rather than read off the schema at runtime: the spec exports + * `ActionSchema` as a lazy proxy that does not forward `.shape`, so resolving it + * means walking zod internals (`_def.in` / `_def.innerType`). That is fine in a + * test and wrong in shipped code — `@object-ui/types` made the same call for + * `ActionComponent`. The pin test does the walk and fails the day this drifts. + */ +export const SPEC_ACTION_KEYS = [ + 'ai', + 'aria', + 'body', + 'bodyExtra', + 'bodyShape', + 'bulkEnabled', + 'component', + 'confirmText', + 'disabled', + 'errorMessage', + 'execute', + 'icon', + 'label', + 'locations', + 'method', + 'mode', + 'name', + 'newTabUrl', + 'objectName', + 'openIn', + 'opensInNewTab', + 'order', + 'params', + 'recordIdField', + 'recordIdParam', + 'refreshAfter', + 'requiredPermissions', + 'requiresFeature', + 'resultDialog', + 'shortcut', + 'successMessage', + 'target', + 'type', + 'undoable', + 'variant', + 'visible', +] as const; + +/** + * The `navigation` alias's own spelling of a target, read off the action itself + * when no nested `navigate` object is present (`ActionRunner.executeNavigation`). + * + * A declared objectui-side dialect, not spec vocabulary: `replace` is documented + * at its read site as "the one thing the `navigation` shape carries that + * `ActionSchema` has no field for". Listing it here is what keeps it from being + * silent dialect — the alternative is a warning that cries wolf on a shape the + * runner itself supports. + */ +export const NAVIGATION_ALIAS_KEYS = ['to', 'external', 'newTab', 'replace'] as const; + +/** + * Keys the spec has TOMBSTONED: still present in `ActionSchema` so the parser can + * reject them BY NAME with a rename prescription, rather than fail with a bare + * "unrecognized key". + * + * Warned about separately from unknown keys, and more loudly: an unknown key is + * probably a typo, while a retired key is metadata that used to work. That + * distinction is why `executeScript` carries a runtime branch returning the + * rename prescription — a branch that exists solely to compensate for the index + * signature, and that can retire with it in step 3. + */ +export const RETIRED_ACTION_KEYS: Readonly> = { + execute: '`execute` was removed in @objectstack/spec 17 (#3855) — rename the key to `target`. ' + + 'The value (a script name or expression) is unchanged. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', +}; + +/** Every key an action may legitimately carry today. */ +export const KNOWN_ACTION_KEYS: ReadonlySet = new Set([ + ...ACTION_DEF_KEYS, + ...SPEC_ACTION_KEYS, + ...NAVIGATION_ALIAS_KEYS, +].filter((key) => !(key in RETIRED_ACTION_KEYS))); + +/** Split an action's own keys into the two things worth saying out loud. */ +export function classifyActionKeys(action: object | null | undefined): { + unknown: string[]; + retired: string[]; +} { + const unknown: string[] = []; + const retired: string[] = []; + if (!action || typeof action !== 'object') return { unknown, retired }; + for (const key of Object.keys(action)) { + if (key in RETIRED_ACTION_KEYS) retired.push(key); + else if (!KNOWN_ACTION_KEYS.has(key)) unknown.push(key); + } + return { unknown, retired }; +} + +// Warn once per (action, problem), not once per execution: an unrecognized key +// usually sits in metadata driving a button that gets clicked repeatedly, and a +// warning that floods the console is a warning that gets muted. +// +// Keyed by action name as well as by the keys, deliberately. Keying on the keys +// alone would report the FIRST action carrying `targt` and stay silent about +// every other one — sending the author to fix a button that was only the first +// symptom. The memo is bounded by the number of authored actions either way. +const warned = new Set(); + +/** Reset the warn-once memo. Exported for tests. */ +export function resetActionKeyWarnings(): void { + warned.clear(); +} + +const isDev = (): boolean => + (globalThis as { process?: { env?: Record } }).process?.env?.NODE_ENV !== + 'production'; + +/** + * Dev-mode only: name the keys an action carries that no reader will ever look + * at. Non-breaking by construction — it changes no types and rejects nothing. + * + * Silently binding no handler is the #2169 "Mark Done does nothing" shape, and + * the compiler cannot help while the index signature stands. Until it comes + * down, this is the audible half. + */ +export function warnOnUnknownActionKeys(action: object | null | undefined, where = 'ActionRunner'): void { + if (!isDev()) return; + const { unknown, retired } = classifyActionKeys(action); + const name = (action as { name?: unknown; type?: unknown })?.name ?? (action as { type?: unknown })?.type ?? '(unnamed)'; + + for (const key of retired) { + const memo = `retired:${String(name)}:${key}`; + if (warned.has(memo)) continue; + warned.add(memo); + console.warn(`[${where}] action "${String(name)}" carries the retired key \`${key}\`. ${RETIRED_ACTION_KEYS[key]}`); + } + + if (unknown.length === 0) return; + const memo = `unknown:${String(name)}:${unknown.slice().sort().join(',')}`; + if (warned.has(memo)) return; + warned.add(memo); + console.warn( + `[${where}] action "${String(name)}" carries ${unknown.length === 1 ? 'a key' : 'keys'} no reader ` + + `recognizes: \`${unknown.join('`, `')}\`. Nothing will read ${unknown.length === 1 ? 'it' : 'them'} — ` + + 'check for a typo, or promote the key to an explicit field on `ActionDef` ' + + '(packages/core/src/actions/actionKeys.ts). Warned rather than rejected because `ActionDef` ' + + 'still carries `[key: string]: any`, so the compiler cannot see this (objectstack#4075).', + ); +} diff --git a/packages/core/src/actions/index.ts b/packages/core/src/actions/index.ts index c3c845cb1..12088f266 100644 --- a/packages/core/src/actions/index.ts +++ b/packages/core/src/actions/index.ts @@ -8,6 +8,7 @@ export * from './ActionRunner.js'; export * from './ActionEngine.js'; +export * from './actionKeys.js'; export * from './TransactionManager.js'; export * from './UndoManager.js'; export * from './bulkFastPath.js'; diff --git a/packages/mobile/package.json b/packages/mobile/package.json index 293749b81..2f2cd3a72 100644 --- a/packages/mobile/package.json +++ b/packages/mobile/package.json @@ -30,7 +30,7 @@ "build": "tsc", "clean": "rm -rf dist", "test": "vitest run", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "peerDependencies": { diff --git a/packages/mobile/tsconfig.test.json b/packages/mobile/tsconfig.test.json new file mode 100644 index 000000000..5af001b05 --- /dev/null +++ b/packages/mobile/tsconfig.test.json @@ -0,0 +1,22 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build is `composite` + `declaration`; this project emits + // nothing, so it must not inherit those. + "composite": false, + // `gesture-spec-parity.test.tsx` reads `mock.calls.at(-1)`. ES2022 is the + // floor for `Array.prototype.at` — the root config's ES2020 is what the + // SOURCE targets, and a test is free to need more than the shipped bundle. + "lib": ["ES2022", "DOM", "DOM.Iterable"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-calendar/package.json b/packages/plugin-calendar/package.json index 5db907fa3..8fc97832c 100644 --- a/packages/plugin-calendar/package.json +++ b/packages/plugin-calendar/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-calendar/tsconfig.test.json b/packages/plugin-calendar/tsconfig.test.json new file mode 100644 index 000000000..a98ee894b --- /dev/null +++ b/packages/plugin-calendar/tsconfig.test.json @@ -0,0 +1,24 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // The `toBeInTheDocument` / `toHaveTextContent` matchers these tests use are + // a global augmentation, not an import, and `@testing-library/jest-dom` does + // not live under `@types/` — so it is never picked up automatically and has + // to be named here. Naming `types` at all switches off automatic `@types/*` + // inclusion, which is fine: nothing in these tests touches Node globals. + "types": ["@testing-library/jest-dom"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-designer/package.json b/packages/plugin-designer/package.json index 045bd865c..23a95a451 100644 --- a/packages/plugin-designer/package.json +++ b/packages/plugin-designer/package.json @@ -24,7 +24,7 @@ "build": "vite build", "clean": "rm -rf dist", "test": "vitest run", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "peerDependencies": { diff --git a/packages/plugin-designer/tsconfig.test.json b/packages/plugin-designer/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/plugin-designer/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-kanban/package.json b/packages/plugin-kanban/package.json index 92bdbbd55..778e2a8a8 100644 --- a/packages/plugin-kanban/package.json +++ b/packages/plugin-kanban/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-kanban/tsconfig.test.json b/packages/plugin-kanban/tsconfig.test.json new file mode 100644 index 000000000..a98ee894b --- /dev/null +++ b/packages/plugin-kanban/tsconfig.test.json @@ -0,0 +1,24 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // The `toBeInTheDocument` / `toHaveTextContent` matchers these tests use are + // a global augmentation, not an import, and `@testing-library/jest-dom` does + // not live under `@types/` — so it is never picked up automatically and has + // to be named here. Naming `types` at all switches off automatic `@types/*` + // inclusion, which is fine: nothing in these tests touches Node globals. + "types": ["@testing-library/jest-dom"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-markdown/package.json b/packages/plugin-markdown/package.json index df168f237..d944dd54d 100644 --- a/packages/plugin-markdown/package.json +++ b/packages/plugin-markdown/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-markdown/tsconfig.test.json b/packages/plugin-markdown/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/plugin-markdown/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-report/package.json b/packages/plugin-report/package.json index 32388b525..79afe34be 100644 --- a/packages/plugin-report/package.json +++ b/packages/plugin-report/package.json @@ -20,7 +20,7 @@ ], "scripts": { "build": "vite build", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "clean": "rm -rf dist", "test": "vitest run", "lint": "eslint ." diff --git a/packages/plugin-report/tsconfig.test.json b/packages/plugin-report/tsconfig.test.json new file mode 100644 index 000000000..be3d460a9 --- /dev/null +++ b/packages/plugin-report/tsconfig.test.json @@ -0,0 +1,25 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing. + "composite": false, + // Naming `types` at all switches off automatic `@types/*` inclusion, so + // BOTH entries are load-bearing: + // - jest-dom: the `toBeInTheDocument` / `toHaveClass` matchers these tests + // use are a global augmentation, not an import, and it does not live + // under `@types/` so it is never picked up automatically. + // - node: the test program pulls in `LegacyReportRenderer.tsx`, which + // reads `process.env`. + "types": ["@testing-library/jest-dom", "node"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-timeline/package.json b/packages/plugin-timeline/package.json index 9c023888a..a00d50fbb 100644 --- a/packages/plugin-timeline/package.json +++ b/packages/plugin-timeline/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-timeline/tsconfig.test.json b/packages/plugin-timeline/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/plugin-timeline/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/plugin-tree/package.json b/packages/plugin-tree/package.json index ba8719a77..0ffeeac1b 100644 --- a/packages/plugin-tree/package.json +++ b/packages/plugin-tree/package.json @@ -27,7 +27,7 @@ "build": "vite build", "test": "vitest run", "test:watch": "vitest", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-tree/tsconfig.test.json b/packages/plugin-tree/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/plugin-tree/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/providers/package.json b/packages/providers/package.json index cdd96d35b..6585fb5ab 100644 --- a/packages/providers/package.json +++ b/packages/providers/package.json @@ -26,7 +26,7 @@ "scripts": { "build": "tsc", "test": "vitest run", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/providers/tsconfig.test.json b/packages/providers/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/providers/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/react-runtime/package.json b/packages/react-runtime/package.json index b834f49e5..1ed85fbac 100644 --- a/packages/react-runtime/package.json +++ b/packages/react-runtime/package.json @@ -21,7 +21,7 @@ "scripts": { "build": "tsc", "test": "vitest run", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/react-runtime/tsconfig.test.json b/packages/react-runtime/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/react-runtime/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/runner/package.json b/packages/runner/package.json index bb17631f8..d2a05842d 100644 --- a/packages/runner/package.json +++ b/packages/runner/package.json @@ -16,7 +16,7 @@ "scripts": { "dev": "vite", "build": "vite build", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint .", "preview": "vite preview", "test": "vitest run" diff --git a/packages/runner/tsconfig.test.json b/packages/runner/tsconfig.test.json new file mode 100644 index 000000000..1edf1cd83 --- /dev/null +++ b/packages/runner/tsconfig.test.json @@ -0,0 +1,18 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing, so it must + // not inherit `composite` / `declaration` from the build config. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/packages/sdui-parser/package.json b/packages/sdui-parser/package.json index 2fad2afc4..2a0320c33 100644 --- a/packages/sdui-parser/package.json +++ b/packages/sdui-parser/package.json @@ -27,8 +27,11 @@ "scripts": { "build": "tsc", "test": "vitest run", - "type-check": "tsc --noEmit", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, - "devDependencies": {} + "devDependencies": { + "@object-ui/core": "workspace:*", + "@object-ui/react": "workspace:*" + } } diff --git a/packages/sdui-parser/tsconfig.test.json b/packages/sdui-parser/tsconfig.test.json new file mode 100644 index 000000000..49dbdf50b --- /dev/null +++ b/packages/sdui-parser/tsconfig.test.json @@ -0,0 +1,25 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + // + // Wiring this up is what revealed that `render.test.tsx` imported + // `@object-ui/core` and `@object-ui/react` with neither declared in + // package.json — phantom dependencies that only resolved through the ROOT + // tsconfig's `paths` alias and vitest. Both are devDependencies now, so the + // test resolves them the way a consumer would. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing. + "composite": false, + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + // This is also what makes the phantom-dependency check above meaningful: + // with the alias in place, an undeclared workspace import still compiles. + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2a48e9df1..073234d9e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2638,7 +2638,14 @@ importers: specifier: ^8.1.5 version: 8.1.5(@types/node@26.1.1)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.1)(yaml@2.9.0) - packages/sdui-parser: {} + packages/sdui-parser: + devDependencies: + '@object-ui/core': + specifier: workspace:* + version: link:../core + '@object-ui/react': + specifier: workspace:* + version: link:../react packages/types: dependencies: diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs new file mode 100644 index 000000000..a7b497212 --- /dev/null +++ b/scripts/check-spec-symbol-derivation.mjs @@ -0,0 +1,617 @@ +#!/usr/bin/env node +/** + * A spec-named symbol must be DERIVED from `@objectstack/spec`, not hand-written. + * + * The failure class (objectstack#4115): objectui declares a local type or const + * under the SAME NAME as a `@objectstack/spec` export, with a doc comment + * claiming spec canonicity — and the declaration has drifted, or is one spec + * release away from drifting. Every audited instance had: + * + * ActionType "canonical definition from @objectstack/spec" → hand union, missing `form` (#2231/#2901) + * ChartType (sibling schema re-exported under spec's name) → 7 of 19 members (objectui#2944) + * ActionLocation + 2 "single source of truth… re-export here" → re-DECLARED union + z.enum (#4074) + * ActionParamFieldType "aligned with the field types available in spec" → 16 of 49 members (#4074) + * + * What makes this class expensive 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 was re-exported under the + * spec's own symbol name. A wrong canonical-claim is not stale documentation — + * it is a planted premise for the next session. + * + * This guard lands the rule AS THE CHECK, not as an AGENTS.md paragraph. A + * prose-only rule is precisely the "declared ≠ enforced" landmine the whole + * thread is about (#4074, objectui#3009 — where a guard file's own header + * falsely claimed its checks were "the real enforcement" for the entire interval + * during which nothing compiled them). + * + * ── What counts as derived ─────────────────────────────────────────────────── + * Both of the repo's sanctioned forms pass, because both bind the local name to + * the spec at compile time: + * + * export type { ActionLocation } from '@objectstack/spec/ui'; // re-export + * export type ActionType = z.infer; // derivation + * + * A declaration is derived only when the spec reference sits 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 hand-written fork looks like: + * + * export interface ActionSchema { locations?: ActionLocation[] } // NOT derived + * + * Consts are stricter still: object and array literals are not descended into. + * `export const ACTION_LOCATIONS = [...SpecLocations]` is a COPY, and a faithful + * copy passes every value comparison — reference identity is the only check that + * distinguishes a re-export from a fork (objectui#3003, and the insight already + * written down in `spec-subschema-parity.test.ts`). + * + * Run: node scripts/check-spec-symbol-derivation.mjs + * Exit: 0 = OK, 1 = a spec-named symbol is hand-written, or the allowlist is stale + */ + +import ts from "typescript"; +import { createRequire } from "module"; +import { readFileSync, readdirSync, statSync } from "fs"; +import { resolve, dirname, join, relative } from "path"; +import { fileURLToPath } from "url"; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + +// ── Allowlist ──────────────────────────────────────────────────────────────── +// Same governance as `check-type-check-coverage.mjs`'s DEBT map: declared, +// reasoned, shrink-only. An entry that no longer matches a real violation is +// stale and fails this guard, so the list cannot outlive the code it excuses. +// +// A same-name symbol that is narrower or wider ON PURPOSE belongs here with that +// purpose written down — at which point the comment is load-bearing instead of +// decorative. A symbol that is *supposed* to be the spec's should be imported +// instead; that is the whole point of the check. +// +// Key format: ":". +const ALLOW = { + "@object-ui/types:ActionSchema": { + reason: + "Two deliberate objectui-side shapes, both documented at their declaration. " + + "`ui-action.ts` is a renderer VIEW over the spec's action — it carries renderer-only " + + "fields the spec does not model, while importing the spec-owned parts it does share " + + "(`ActionLocation`, `ActionType`). `crud.ts` is the explicitly @deprecated legacy " + + "shape kept for backward compatibility and slated for removal in a future major.", + issue: 4115, + }, +}; + +// ── Untriaged collisions (the ledger) ──────────────────────────────────────── +// Same governance as `check-type-check-coverage.mjs`'s DEBT map: declared, +// shrink-only. These are the same-name symbols that predate this guard and have +// not been triaged one-by-one yet. This check exists to stop the BLEEDING — a +// new fork fails on the PR that writes it — not to retro-fix 166 symbols at once. +// +// Named symbols rather than a per-package COUNT, deliberately: a 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 collisions +// happen to sort first instead of at the one just written. The list is longer; +// it is also the thing that names the new symbol at the moment it appears. +// +// To burn one down: import/derive it from the spec, rename it to a declared +// local dialect (`ObjectUiLocal…`, with a tripwire test asserting the spec does +// not own the name), or move it to ALLOW with the reason it deliberately +// differs. Then delete it from this list — leaving it here fails the ratchet. +const DEBT_ISSUE = 4115; +const DEBT = { + "@object-ui/types": [ + "ActionParam", + "AppContextSelectorSchema", + "AppSchema", + "BatchOperationResult", + "BreakpointName", + "CacheStrategy", + "ChartSeries", + "ChartSeriesSchema", + "ConditionalValidation", + "CreateExportJobRequest", + "CreateExportJobResult", + "CrossFieldValidation", + "DashboardSchema", + "DashboardWidgetSchema", + "DatasourceSchema", + "DriverInterface", + "EventHandler", + "ExportJobStatus", + "ExpressionSchema", + "FeedItemType", + "FileMetadata", + "FilterCondition", + "FilterConditionSchema", + "FilterOperator", + "FormField", + "FormFieldSchema", + "FormatValidation", + "GestureConfig", + "GestureType", + "GlobalFilterSchema", + "GroupByNode", + "ImportJobStatus", + "ImportRowResult", + "ImportWriteMode", + "JoinNode", + "JoinStrategy", + "JoinedReportBlock", + "ListViewSchema", + "MutationEvent", + "NavigationArea", + "NavigationAreaSchema", + "NavigationItem", + "NavigationItemSchema", + "ObjectIndex", + "ObjectPermission", + "OfflineConfig", + "PageRegion", + "PageRegionSchema", + "PageSchema", + "PageVariable", + "PageVariableSchema", + "PermissionAction", + "QueryAST", + "QuerySchema", + "ReportSchedule", + "ReportSchema", + "ResponsiveConfig", + "ScriptValidation", + "SelectOption", + "SelectOptionSchema", + "SharingRule", + "SpanSchema", + "StateMachineValidation", + "Theme", + "ThemeSchema", + "ValidationError", + "ValidationRule", + "ValidationRuleSchema", + "WidgetManifest", + "WidgetSource", + "WindowFunction", + ], + "@object-ui/app-shell": [ + "ConversationSummary", + "DecisionOutputDef", + "ExplainDecision", + "ExplainLayer", + "ExplainMatchedRule", + "ExplainRecordAttribution", + "ExternalCatalog", + "ExternalColumn", + "ExternalTable", + "FieldGroup", + "FieldInput", + "FilterCondition", + "FlowEdge", + "FlowNode", + "GenerateDraftOpts", + "InstalledPackage", + "ObjectDraft", + "PackageManifest", + "PageHeaderProps", + "PluginPermissions", + "RemoteTable", + "RuntimeConfig", + "SchemaDiffEntry", + "SchemaDiffEntryKind", + "SchemaValidationResult", + "ScreenFieldSpec", + "ScreenSpec", + "isAggregatedViewContainer", + ], + "@object-ui/core": [ + "ActionHandler", + "CONTEXT_TOKENS", + "ChartSeries", + "CrudAffordances", + "PluginDefinition", + "ResponsiveStyles", + "RowCrudPredicates", + "RowHeight", + "StyleMap", + "ValidationError", + "ValidationResult", + "defineView", + "resolveCrudAffordances", + ], + "@object-ui/auth": [ + "AuthProvider", + "AuthProviderConfig", + "AuthSession", + "AuthUser", + "DelegableScope", + "TenancyPosture", + ], + "@object-ui/components": [ + "Field", + "FilterCondition", + "ShareLink", + "ShareLinkAudience", + "ShareLinkPermission", + "SortItem", + ], + "@object-ui/react": [ + "ConflictResolutionStrategy", + "NavigationConfig", + "OfflineCacheConfig", + "OfflineConfig", + "OfflineStrategy", + "PerformanceConfig", + ], + "@object-ui/data-objectstack": [ + "CacheStats", + "DroppedFieldsEvent", + "MetadataSaveOptions", + "SecurityPolicy", + "ValidationError", + ], + "@object-ui/plugin-chatbot": [ + "MessageContent", + "PendingActionRow", + "PendingActionStatus", + "Tool", + ], + "@object-ui/plugin-list": [ + "ListView", + "UserFilters", + "ViewTab", + ], + "@object-ui/fields": [ + "FieldWidgetProps", + "isFileIdToken", + ], + "@object-ui/layout": [ + "Page", + "PageHeaderProps", + ], + "@object-ui/plugin-detail": [ + "FeedFilterMode", + "ObjectFieldLike", + ], + "@object-ui/plugin-grid": [ + "ColumnSummaryConfig", + "isMultiValueField", + ], + "@object-ui/collaboration": [ + "RealtimeConfig", + ], + "@object-ui/plugin-charts": [ + "ChartConfig", + ], + "@object-ui/plugin-form": [ + "FormSection", + ], + "@object-ui/providers": [ + "Theme", + ], + "@object-ui/runner": [ + "App", + ], + "@object-ui/sdui-parser": [ + "ValidationResult", + ], +}; + +// Files under these paths are not objectui's own authored surface. +// - `ui/` is the Shadcn no-touch zone (AGENTS.md #7): upstream 3rd-party files +// overwritten by sync scripts, so a collision there is not ours to fix. +const SKIP_PATH_SEGMENTS = ["components/src/ui/"]; + +const isSpecModule = (m) => m === "@objectstack/spec" || m.startsWith("@objectstack/spec/"); + +// ── 1. Enumerate every `@objectstack/spec` export name, per subpath ────────── +// Types AND values: the drifted symbols in the table above are mostly types, and +// a runtime `import()` only sees values. The compiler's own view of each +// subpath's `.d.ts` is the only source that covers both. +function specExportNames() { + const require = createRequire(import.meta.url); + let pkgPath; + try { + pkgPath = require.resolve("@objectstack/spec/package.json"); + } catch { + console.error( + "❌ cannot resolve @objectstack/spec — run `pnpm install` first.\n" + + " This guard reads the spec's own type declarations; it cannot fall back to a\n" + + " hardcoded name list without becoming the stale copy it exists to prevent." + ); + process.exit(1); + } + const pkgDir = dirname(pkgPath); + const pkg = JSON.parse(readFileSync(pkgPath, "utf8")); + + const entries = []; + for (const [sub, cond] of Object.entries(pkg.exports ?? {})) { + if (typeof cond !== "object" || cond === null) continue; + const dts = cond?.import?.types ?? cond?.require?.types; + if (!dts) continue; + entries.push({ sub: sub === "." ? "@objectstack/spec" : `@objectstack/spec${sub.slice(1)}`, file: resolve(pkgDir, dts) }); + } + + const program = ts.createProgram( + entries.map((e) => e.file), + { + noEmit: true, + skipLibCheck: true, + strict: false, + target: ts.ScriptTarget.ESNext, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + } + ); + const checker = program.getTypeChecker(); + + const names = new Map(); // name -> Set + for (const entry of entries) { + const sf = program.getSourceFile(entry.file); + if (!sf) continue; + const moduleSymbol = checker.getSymbolAtLocation(sf); + if (!moduleSymbol) continue; + for (const exported of checker.getExportsOfModule(moduleSymbol)) { + const name = exported.getName(); + if (!names.has(name)) names.set(name, new Set()); + names.get(name).add(entry.sub); + } + } + return names; +} + +// ── 2. Scan objectui's authored source for exported declarations ───────────── +function sourceFiles() { + const out = []; + const pkgsDir = resolve(root, "packages"); + for (const pkg of readdirSync(pkgsDir)) { + const srcDir = join(pkgsDir, pkg, "src"); + try { + if (!statSync(srcDir).isDirectory()) continue; + } catch { + continue; + } + walk(srcDir, out); + } + return out; +} + +function walk(dir, out) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + // Tests do not publish a surface, so a local type there cannot be mistaken + // for the spec's by an importer. Keeps the signal on the public API. + if (entry.name === "node_modules" || entry.name === "dist" || entry.name === "__tests__") continue; + walk(full, out); + continue; + } + if (!/\.tsx?$/.test(entry.name) || /\.d\.ts$/.test(entry.name)) continue; + if (/\.test\.tsx?$/.test(entry.name) || /\.stories\.tsx?$/.test(entry.name)) continue; + out.push(full); + } +} + +const packageNameCache = new Map(); +function packageNameFor(file) { + const rel = relative(root, file); + const pkgDir = rel.split("/").slice(0, 2).join("/"); + if (!packageNameCache.has(pkgDir)) { + let name = pkgDir; + try { + name = JSON.parse(readFileSync(resolve(root, pkgDir, "package.json"), "utf8")).name ?? pkgDir; + } catch { + /* keep the directory name */ + } + packageNameCache.set(pkgDir, name); + } + return packageNameCache.get(pkgDir); +} + +const hasExportModifier = (node) => + node.modifiers?.some((m) => m.kind === ts.SyntaxKind.ExportKeyword) ?? false; + +/** + * Does this subtree reference a name bound to a spec import? + * + * `skipLiterals` stops the walk at object/array literals and type-literal member + * blocks: a spec name mentioned inside a members block is a hand-written shape + * that merely USES a spec type, which is exactly what a fork looks like. + */ +function referencesSpec(node, bindings, skipLiterals) { + let found = false; + const visit = (n) => { + if (found || !n) return; + if (skipLiterals) { + if (ts.isTypeLiteralNode(n) || ts.isObjectLiteralExpression(n) || ts.isArrayLiteralExpression(n)) return; + } + if (ts.isIdentifier(n) && bindings.has(n.text)) { + found = true; + return; + } + ts.forEachChild(n, visit); + }; + ts.forEachChild(node, visit); + return found; +} + +function scanFile(file, specNames) { + const text = readFileSync(file, "utf8"); + const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, /\.tsx$/.test(file) ? ts.ScriptKind.TSX : ts.ScriptKind.TS); + + // Local names bound to a `@objectstack/spec` import in THIS file. + const specBindings = new Set(); + for (const stmt of sf.statements) { + if (!ts.isImportDeclaration(stmt) || !ts.isStringLiteral(stmt.moduleSpecifier)) continue; + if (!isSpecModule(stmt.moduleSpecifier.text)) continue; + const clause = stmt.importClause; + if (!clause) continue; + if (clause.name) specBindings.add(clause.name.text); + const named = clause.namedBindings; + if (named && ts.isNamespaceImport(named)) specBindings.add(named.name.text); + if (named && ts.isNamedImports(named)) for (const el of named.elements) specBindings.add(el.name.text); + } + + const findings = []; + const record = (name, kind, derived, node) => { + if (!specNames.has(name) || derived) return; + const { line } = sf.getLineAndCharacterOfPosition(node.getStart(sf)); + findings.push({ name, kind, file, line: line + 1, subpaths: [...specNames.get(name)] }); + }; + + for (const stmt of sf.statements) { + // `export { X } from '…'` / `export { X }` / `export type { X } from '…'` + if (ts.isExportDeclaration(stmt)) { + const fromModule = stmt.moduleSpecifier && ts.isStringLiteral(stmt.moduleSpecifier) ? stmt.moduleSpecifier.text : null; + // `export * from '…'` exports names this AST pass cannot attribute. When it + // re-exports the spec it is derivation by definition; when it re-exports a + // sibling, that sibling's own declaration is scanned on its own turn. + if (!stmt.exportClause || !ts.isNamedExports(stmt.exportClause)) continue; + // A barrel re-exporting a module this scan already covers — a relative path + // or a `@object-ui/*` sibling — is not a second finding. Whatever it points + // at gets judged at its own declaration site; reporting the barrel too would + // just make one fork look like four, and would make the fix land in the + // barrel rather than at the declaration. + if (fromModule && (fromModule.startsWith(".") || fromModule.startsWith("@object-ui/"))) continue; + for (const el of stmt.exportClause.elements) { + const exportedName = el.name.text; + const localName = (el.propertyName ?? el.name).text; + const derived = fromModule + ? isSpecModule(fromModule) // re-export straight from the spec + : specBindings.has(localName); // `import { X } from spec; export { X }` + record(exportedName, "re-export", derived, el); + } + continue; + } + + if (!hasExportModifier(stmt)) continue; + + if (ts.isTypeAliasDeclaration(stmt)) { + record(stmt.name.text, "type", referencesSpec(stmt, specBindings, true), stmt); + } else if (ts.isInterfaceDeclaration(stmt)) { + // Only `extends` counts — see the header. + const extendsSpec = (stmt.heritageClauses ?? []).some((h) => referencesSpec(h, specBindings, false)); + record(stmt.name.text, "interface", extendsSpec, stmt); + } else if (ts.isClassDeclaration(stmt) && stmt.name) { + const extendsSpec = (stmt.heritageClauses ?? []).some((h) => referencesSpec(h, specBindings, false)); + record(stmt.name.text, "class", extendsSpec, stmt); + } else if (ts.isVariableStatement(stmt)) { + for (const decl of stmt.declarationList.declarations) { + if (!ts.isIdentifier(decl.name)) continue; + record(decl.name.text, "const", decl.initializer ? referencesSpec(decl, specBindings, true) : false, decl); + } + } else if (ts.isEnumDeclaration(stmt)) { + // An enum cannot be derived from anything — a spec-named one is a fork. + record(stmt.name.text, "enum", false, stmt); + } else if (ts.isFunctionDeclaration(stmt) && stmt.name) { + record(stmt.name.text, "function", false, stmt); + } + } + return findings; +} + +// ── 3. Report ──────────────────────────────────────────────────────────────── +const specNames = specExportNames(); +const files = sourceFiles().filter((f) => !SKIP_PATH_SEGMENTS.some((seg) => f.includes(seg))); + +const violations = []; +for (const file of files) violations.push(...scanFile(file, specNames)); + +const matchedAllowKeys = new Set(); +const unallowed = []; +for (const v of violations) { + const pkg = packageNameFor(v.file); + const key = `${pkg}:${v.name}`; + if (ALLOW[key]) { + matchedAllowKeys.add(key); + continue; + } + unallowed.push({ ...v, pkg, key }); +} + +const byPackage = new Map(); +for (const v of unallowed) { + if (!byPackage.has(v.pkg)) byPackage.set(v.pkg, []); + byPackage.get(v.pkg).push(v); +} + +// `--ledger` regenerates the DEBT block from the working tree, so the list is +// never hand-maintained (and so burning symbols down is a mechanical edit). +if (process.argv.includes("--ledger")) { + const lines = [...byPackage.entries()] + .sort((a, b) => b[1].length - a[1].length || a[0].localeCompare(b[0])) + .map(([pkg, found]) => { + const names = [...new Set(found.map((v) => v.name))].sort(); + return ` ${JSON.stringify(pkg)}: [\n` + names.map((n) => ` ${JSON.stringify(n)},`).join("\n") + `\n ],`; + }); + console.log("const DEBT = {\n" + lines.join("\n") + "\n};"); + process.exit(0); +} + +const errors = []; + +// 1. A collision that is not in the ledger is NEW. This is the half that stops +// the bleeding: a fresh fork fails on the PR that writes it, by name. +for (const [pkg, found] of byPackage) { + const declared = new Set(DEBT[pkg] ?? []); + const fresh = found.filter((v) => !declared.has(v.name)); + if (fresh.length === 0) continue; + errors.push( + `${pkg} declares ${fresh.length} spec-named symbol${fresh.length === 1 ? "" : "s"} the spec already owns:\n` + + fresh + .map( + (v) => + ` ${v.kind} \`${v.name}\` ${relative(root, v.file)}:${v.line} ` + + `(exported by \`${v.subpaths.join("`, `")}\`)` + ) + .join("\n") + + `\n Import it (\`export type { X } from '@objectstack/spec/…'\`), derive it\n` + + ` (\`export type X = z.infer\`), or — if it deliberately differs — rename it\n` + + ` to a declared dialect (\`ObjectUiLocalX\`, with a tripwire test asserting the spec does\n` + + ` not own the name) or add an ALLOW entry with the reason.` + ); +} + +// 2. Ratchet — a ledger entry whose symbol is fixed (or gone) must be deleted. +// Left in, it reserves the name: the next fork under it would land silently. +for (const [pkg, names] of Object.entries(DEBT)) { + const live = new Set((byPackage.get(pkg) ?? []).map((v) => v.name)); + const stale = names.filter((n) => !live.has(n)); + if (stale.length === 0) continue; + errors.push( + `${pkg} lists ${stale.length} symbol${stale.length === 1 ? "" : "s"} in DEBT that no longer collide` + + ` — \`${stale.join("`, `")}\`.\n` + + ` Delete them from scripts/check-spec-symbol-derivation.mjs (\`--ledger\` regenerates the\n` + + ` block) so the names cannot be re-forked silently` + + `${DEBT_ISSUE ? ` (and close #${DEBT_ISSUE} once the ledger is empty)` : ""}.` + ); +} + +// 3. Ratchet — an ALLOW entry that excuses nothing is stale and must go, or it +// silently keeps a name reserved for a future fork. +for (const key of Object.keys(ALLOW)) { + if (!matchedAllowKeys.has(key)) { + errors.push( + `${key} is in ALLOW but no longer collides with a spec export name — the symbol was\n` + + ` renamed, removed, or is now imported from the spec. Delete the entry so the\n` + + ` exemption cannot be inherited by a future fork under the same name.` + ); + } +} + +const outstanding = Object.values(DEBT).reduce((sum, names) => sum + names.length, 0); + +if (errors.length === 0) { + console.log( + `✅ spec symbol derivation: ${files.length} files scanned against ${specNames.size} spec export names; ` + + `${Object.keys(ALLOW).length} declared dialect${Object.keys(ALLOW).length === 1 ? "" : "s"}, ` + + `${outstanding} untriaged collision${outstanding === 1 ? "" : "s"} in ${Object.keys(DEBT).length} packages.` + ); + process.exit(0); +} + +console.error("❌ a spec-named symbol is hand-written, not derived:\n"); +for (const message of errors) console.error(` • ${message}\n`); +console.error( + "A local declaration under a spec export's name is read by the next agent as the spec's own\n" + + "definition — that is how #2901 was filed with a backwards premise. See\n" + + "https://github.com/objectstack-ai/objectstack/issues/4115 for the four symbols that had\n" + + "already drifted when this guard was written." +); +process.exit(1); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index c34d81bfe..cf58755df 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -12,6 +12,17 @@ * only shrink: once a package gains the script, its entry has to be deleted or * this guard fails. * + * The same question applies one level down, to TESTS (objectstack#4118): a + * package tsconfig excludes test files — correctly, it is the BUILD config and + * they would emit into `dist` — and for most packages no other `tsc` invocation + * read them either. Tests are where agents encode their understanding of a + * contract, so unchecked tests let a wrong understanding accumulate silently and + * then READ AS EVIDENCE: objectui#3009 found `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. So the second half of this guard: a package with test files either + * type-checks them or carries a TEST_DEBT entry with a measured error count. + * * Run: node scripts/check-type-check-coverage.mjs * Exit: 0 = OK, 1 = coverage regressed or the lists are stale */ @@ -58,9 +69,65 @@ const CHECKED_BY_OWN_BUILD = { }, }; +// ── Known gaps: tests that nothing type-checks ─────────────────────────────── +// Packages whose tests do not compile yet, so they cannot chain a +// `tsconfig.test.json` from `type-check`. Every entry is real debt: fix the +// errors, add the config, then delete the entry. +// +// Counts are MEASURED, not estimated — a temp tsconfig per package lifting only +// the test exclusion, against `main` at the time of the sweep, with the same +// template the wired-up packages use (`noEmit`, `composite: false`, `paths: {}`, +// plus `lib`/`types` where the tests need them). Errors resolvable in that +// config are excluded, so these are code-tier only. +// +// The two dominant codes are not random, and each has a playbook: +// - TS2741/TS2739 "missing required properties" — authoring fixtures typed as +// PARSED OUTPUT, whose `.default()` fields are required. Type them as the +// input surface (`z.input`), the fix objectstack#4074 applied in `types`. +// - TS2339/TS2353 "property does not exist" — implementation wider than the +// type, the dialect problem. Declare the dialect next to the vocabulary it +// extends (see scripts/check-spec-symbol-derivation.mjs), don't widen the +// type to silence it. +const TEST_DEBT = { + "@object-ui/core": { errors: 72, issue: 4118, note: "TS2741x32, TS2322x17 — mostly the input-vs-output fixture confusion" }, + "@object-ui/app-shell": { errors: 53, issue: 4118, note: "TS2339x24 — implementation wider than the type" }, + "@object-ui/components": { errors: 31, issue: 4118, note: "TS7006x12, TS7031x12 — untyped test callback params" }, + "@object-ui/react": { errors: 27, issue: 4118, note: "TS2769x9 — overload mismatch on render helpers" }, + "@object-ui/i18n": { errors: 13, issue: 4118, note: "TS2769x9" }, + "@object-ui/plugin-form": { errors: 12, issue: 4118, note: "TS2322x10" }, + "@object-ui/plugin-dashboard": { errors: 6, issue: 4118 }, + "@object-ui/plugin-list": { errors: 6, issue: 4118, note: "TS2353x3 — dialect keys" }, + "@object-ui/permissions": { errors: 5, issue: 4118, note: "TS2741x5" }, + "@object-ui/plugin-detail": { errors: 5, issue: 4118, note: "TS2353x3 — dialect keys" }, + "@object-ui/plugin-gantt": { errors: 3, issue: 4118 }, + "@object-ui/auth": { errors: 2, issue: 4118 }, + "@object-ui/plugin-chatbot": { errors: 2, issue: 4118 }, + "@object-ui/plugin-grid": { errors: 2, issue: 4118 }, + "@object-ui/plugin-map": { errors: 1, issue: 4118 }, +}; + // ── Collect workspace packages ─────────────────────────────────────────────── const GROUPS = ["packages", "apps", "examples"]; +function countTestFiles(dir) { + let n = 0; + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return 0; + } + for (const entry of entries) { + if (entry.isDirectory()) { + if (entry.name === "node_modules" || entry.name === "dist") continue; + n += countTestFiles(join(dir, entry.name)); + } else if (/\.test\.tsx?$/.test(entry.name)) { + n++; + } + } + return n; +} + function collect() { const out = []; for (const group of GROUPS) { @@ -80,19 +147,36 @@ function collect() { } const pkg = JSON.parse(readFileSync(manifest, "utf8")); if (!pkg.name) continue; - let hasTsconfig = true; + let tsconfig = null; try { - statSync(resolve(root, dir, "tsconfig.json")); + tsconfig = readFileSync(resolve(root, dir, "tsconfig.json"), "utf8"); } catch { - hasTsconfig = false; + /* no tsconfig */ } + let testConfig = null; + try { + testConfig = readFileSync(resolve(root, dir, "tsconfig.test.json"), "utf8"); + } catch { + /* no test config */ + } + const typeCheck = pkg.scripts?.["type-check"] ?? ""; out.push({ name: pkg.name, dir, hasScript: Boolean(pkg.scripts?.["type-check"]), + typeCheck, build: pkg.scripts?.build, hasBuild: Boolean(pkg.scripts?.build), - hasTsconfig, + hasTsconfig: tsconfig !== null, + testFiles: countTestFiles(resolve(root, dir, "src")), + // A package tsconfig that never excludes a test glob compiles its tests + // as part of the build program, so they are already covered. + buildExcludesTests: tsconfig !== null && /\*\.test\./.test(tsconfig), + hasTestConfig: testConfig !== null, + testConfig, + // The config existing is not the same as anything running it — that gap + // IS objectui#3009. `type-check` has to chain it. + chainsTestConfig: /-p\s+tsconfig\.test\.json/.test(typeCheck), }); } } @@ -190,12 +274,97 @@ for (const [name, spec] of Object.entries(CHECKED_BY_OWN_BUILD)) { } } +// ── 5. Tests are code too (objectstack#4118) ───────────────────────────────── +// Only asked of packages that are type-checked at all: one whose whole package +// is unchecked is already covered by DEBT above, and stacking a second entry on +// it would just make the same gap look like two. +const testsCovered = (pkg) => !pkg.buildExcludesTests || (pkg.hasTestConfig && pkg.chainsTestConfig); + +for (const pkg of packages) { + if (!pkg.hasScript || pkg.testFiles === 0) continue; + + // 5a. A `tsconfig.test.json` nothing runs is the objectui#3009 shape exactly: + // a file that looks like enforcement while no `tsc` invocation reads it. + if (pkg.hasTestConfig && !pkg.chainsTestConfig) { + errors.push( + `${pkg.name} (${pkg.dir}) has a tsconfig.test.json that its "type-check" script never runs,\n` + + ` so the file looks like enforcement while nothing compiles it — the exact shape of\n` + + ` objectui#3009. Chain it: "type-check": "tsc --noEmit && tsc -p tsconfig.test.json"` + ); + continue; + } + + // 5b. A config that emits, or that does not actually include the tests, would + // pass 5a while checking nothing. + if (pkg.hasTestConfig && pkg.chainsTestConfig) { + if (!/"noEmit"\s*:\s*true/.test(pkg.testConfig)) { + errors.push( + `${pkg.name} (${pkg.dir}): tsconfig.test.json must set "noEmit": true — it is a checking\n` + + ` project, and emitting would put test output in the published dist.` + ); + } + if (!/\*\.test\./.test(pkg.testConfig)) { + errors.push( + `${pkg.name} (${pkg.dir}): tsconfig.test.json does not "include" any test glob, so it\n` + + ` compiles nothing. Add "include": ["src/**/*.test.ts", "src/**/*.test.tsx"]` + ); + } + continue; + } + + // 5c. Undeclared gap — tests exist and nothing reads them. + if (!testsCovered(pkg) && !TEST_DEBT[pkg.name]) { + errors.push( + `${pkg.name} (${pkg.dir}) has ${pkg.testFiles} test file${pkg.testFiles === 1 ? "" : "s"} that no \`tsc\`\n` + + ` invocation reads: its tsconfig.json excludes them (correctly — it is the build config)\n` + + ` and no tsconfig.test.json chains off "type-check". An unchecked test can assert a\n` + + ` contract the compiler never checked, and then read as evidence that the contract holds.\n` + + ` Add a tsconfig.test.json (see packages/types/tsconfig.test.json) and chain it, or add a\n` + + ` TEST_DEBT entry with the measured error count.` + ); + } +} + +// 6. Ratchet — a declared test gap that has been closed must leave the list. +for (const [name, spec] of Object.entries(TEST_DEBT)) { + const pkg = byName.get(name); + if (!pkg) { + errors.push(`${name} is listed in TEST_DEBT but is not a workspace package any more — delete the entry.`); + continue; + } + if (pkg.testFiles === 0) { + errors.push(`${name} is listed in TEST_DEBT but has no test files any more — delete the entry.`); + continue; + } + // A package that is not type-checked AT ALL is already declared in DEBT. A + // second entry here would make one gap read as two, and would survive the day + // DEBT is paid off, so the test gap has to be re-measured then anyway. + if (!pkg.hasScript) { + errors.push( + `${name} is listed in TEST_DEBT but has no "type-check" script, so its whole package is\n` + + ` unchecked and DEBT already declares that. Delete the TEST_DEBT entry; add it back with\n` + + ` a fresh measurement once the package type-checks at all.` + ); + continue; + } + if (testsCovered(pkg)) { + errors.push( + `${name} type-checks its tests now — delete its TEST_DEBT entry so the gap cannot reopen` + + `${spec.issue ? ` (and close #${spec.issue} if the list is empty)` : ""}.` + ); + } +} + // ── Report ─────────────────────────────────────────────────────────────────── const checked = packages.filter((p) => p.hasScript).length; const debtCount = Object.keys(DEBT).length; const debtErrors = Object.values(DEBT).reduce((sum, d) => sum + d.errors, 0); const byBuild = Object.keys(CHECKED_BY_OWN_BUILD).length; +const withTests = packages.filter((p) => p.hasScript && p.testFiles > 0); +const testsChecked = withTests.filter(testsCovered).length; +const testDebtErrors = Object.values(TEST_DEBT).reduce((sum, d) => sum + d.errors, 0); + if (errors.length === 0) { console.log( `✅ type-check coverage: ${checked}/${packages.length} via \`type-check\`, ` + @@ -203,6 +372,10 @@ if (errors.length === 0) { `${debtCount} known-broken (${debtErrors} errors outstanding), ` + `${NOT_COMPILED.length} not compiled.` ); + console.log( + `✅ test type-check coverage: ${testsChecked}/${withTests.length} packages compile their tests, ` + + `${Object.keys(TEST_DEBT).length} declared debt (${testDebtErrors} errors outstanding).` + ); process.exit(0); } @@ -212,6 +385,8 @@ for (const message of errors) { } console.error( "\nA package with no `type-check` script is not passing — turbo skips it and CI sees nothing.\n" + + "An excluded TEST file is not passing either — nothing compiles it, so what it asserts about\n" + + "a contract was never checked (objectstack#4118).\n" + "See https://github.com/objectstack-ai/objectui/issues/2911 for why this guard exists." ); process.exit(1);