Skip to content

Commit 62311b6

Browse files
authored
ci(spec,type-check): spec-named symbols must be derived, 12 packages type-check their tests, ActionDef's keys get an inventory (#3032)
Three issues, one theme: a rule that isn't a check isn't a rule. objectstack#4115 — scripts/check-spec-symbol-derivation.mjs enumerates all 4231 @objectstack/spec export names through the compiler's view of each subpath's .d.ts, then flags an exported declaration whose name the spec owns unless it is derived (re-export or z.infer in a structural position). Ledger names 156 pre-existing collisions by symbol, not by count, so it is shrink-only. Runs in the type-check job, 5.5s. objectstack#4118 — 12 packages (not 8) gain a tsconfig.test.json following objectui#3009's template. Wiring sdui-parser up surfaced two phantom dependencies (@object-ui/core, @object-ui/react) resolved only through the root paths alias; both are devDependencies now. check-type-check-coverage.mjs grows a second half: a package with test files either compiles them or carries a measured TEST_DEBT entry (240 code-tier errors across 15 packages), and a tsconfig.test.json nothing runs is itself a failure. objectstack#4075 step 1 — ActionDef's [key: string]: any means a typo or a tombstoned key reaches the runner and silently does nothing. Inventory the key set as data, warn in dev on anything outside it, change no types. execute gets a louder rename prescription since it is a live tombstone. actionKeys.pin.test.ts re-derives each list from its real source so the hand-maintained lists cannot drift.
1 parent 4874117 commit 62311b6

34 files changed

Lines changed: 1556 additions & 18 deletions
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@object-ui/core": minor
3+
---
4+
5+
feat(core): inventory `ActionDef`'s keys and warn on the ones nothing reads — objectstack#4075 step 1
6+
7+
`ActionDef` ends with `[key: string]: any`, so it accepts any key of any type.
8+
Deleting `ActionDef.execute` produced **zero** compile errors even though the
9+
field had just been removed (objectui#2990), and stale metadata still authoring
10+
`execute: 'markDone'` type-checks today. The same deletion against
11+
`@object-ui/types`' `ActionSchema` — which has no index signature — correctly
12+
produced `TS2353` at the authoring site. One of the two readers can catch a
13+
retired key; the other is structurally incapable, which is how a typo (`targt`)
14+
and a tombstoned key both reach a runner that then silently does nothing.
15+
16+
This is the non-breaking first step of the staged narrowing: it makes the key set
17+
**visible** and warns on anything outside it, without changing a single type.
18+
19+
New exports from `@object-ui/core`:
20+
21+
- `ACTION_DEF_KEYS`, `SPEC_ACTION_KEYS`, `NAVIGATION_ALIAS_KEYS`,
22+
`RETIRED_ACTION_KEYS`, `KNOWN_ACTION_KEYS` — the inventory.
23+
- `classifyActionKeys(action)` — splits an action's own keys into `unknown` and
24+
`retired`.
25+
- `warnOnUnknownActionKeys(action)` — dev-mode only, warn-once. Called by
26+
`ActionRunner.execute`, so no consumer wiring is needed.
27+
28+
A retired key gets a louder, more specific warning than an unknown one: an
29+
unknown key is probably a typo, a retired key is metadata that used to work.
30+
`execute` is not simply gone from the spec — it is a live **tombstone**, still
31+
present in `ActionSchema` so the parser can reject it by name with the rename
32+
prescription.
33+
34+
Nothing is rejected and no types changed, so existing metadata behaves exactly as
35+
before. Promoting the legitimate keys to explicit optional fields, then removing
36+
the index signature so `tsc` catches both typos and retired keys, are steps 2 and
37+
3 of objectstack#4075.

.github/workflows/ci.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ jobs:
7575
- name: Install dependencies
7676
run: pnpm install --frozen-lockfile
7777

78+
# A local type/const declared under a `@objectstack/spec` export's NAME reads
79+
# to the next agent as the spec's own definition — four such symbols had
80+
# already drifted from the spec they claimed to be (objectstack#4115). Needs
81+
# the install (it reads the spec's own `.d.ts`) but not the build, so it runs
82+
# before the expensive steps.
83+
- name: Verify spec-named symbols are derived, not hand-written
84+
run: pnpm check:spec-symbols
85+
7886
- name: Turbo Cache
7987
uses: actions/cache@v6
8088
with:

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
"lint:coverage": "node scripts/check-lint-coverage.mjs",
3434
"type-check": "turbo run type-check",
3535
"type-check:coverage": "node scripts/check-type-check-coverage.mjs",
36+
"check:spec-symbols": "node scripts/check-spec-symbol-derivation.mjs",
3637
"cli": "node packages/cli/dist/cli.js",
3738
"objectui": "node packages/cli/dist/cli.js",
3839
"create-plugin": "node packages/create-plugin/dist/index.js",

packages/core/src/actions/ActionRunner.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import type { RunnableActionType } from '@object-ui/types';
2525
import { ExpressionEvaluator } from '../evaluator/ExpressionEvaluator';
2626
import { globalUndoManager, type UndoableOperation } from './UndoManager';
27+
import { warnOnUnknownActionKeys } from './actionKeys';
2728

2829
export interface ActionResult {
2930
success: boolean;
@@ -475,6 +476,12 @@ export class ActionRunner {
475476

476477
async execute(action: ActionDef): Promise<ActionResult> {
477478
try {
479+
// `ActionDef` accepts any key of any type, so a typo (`targt`) and a
480+
// retired key (`execute`) both reach here having type-checked. Neither
481+
// binds a handler, and binding no handler silently is the #2169 "Mark Done
482+
// does nothing" shape. Dev-only, warn-once, changes nothing (#4075 step 1).
483+
warnOnUnknownActionKeys(action);
484+
478485
// Resolve the action type
479486
const actionType = action.type || action.actionType || action.name || '';
480487

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/**
2+
* Pins the key inventory in `actionKeys.ts` to the two things it mirrors —
3+
* `ActionDef`'s own declarations and `@objectstack/spec`'s `ActionSchema`
4+
* (objectstack#4075 step 1).
5+
*
6+
* Why a test rather than a comment: a hand-maintained list that silently drifts
7+
* from the interface it claims to mirror is the exact "declared ≠ enforced"
8+
* failure this work is about. `ActionDef` cannot be enumerated at runtime — its
9+
* `[key: string]: any` widens `keyof` to `string | number` — so the list is data,
10+
* and this file is what makes the data true. Adding a field to `ActionDef`
11+
* without adding it here fails, by name.
12+
*
13+
* Discrimination proof for the guard below: with `ACTION_DEF_KEYS` complete these
14+
* pass; dropping any single entry (e.g. `target`) fails with
15+
* "ActionDef declares keys the inventory is missing: target".
16+
*/
17+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
18+
import { readFileSync } from 'node:fs';
19+
import { fileURLToPath } from 'node:url';
20+
import { dirname, join } from 'node:path';
21+
import ts from 'typescript';
22+
import * as SpecUI from '@objectstack/spec/ui';
23+
import {
24+
ACTION_DEF_KEYS,
25+
SPEC_ACTION_KEYS,
26+
NAVIGATION_ALIAS_KEYS,
27+
RETIRED_ACTION_KEYS,
28+
KNOWN_ACTION_KEYS,
29+
classifyActionKeys,
30+
warnOnUnknownActionKeys,
31+
resetActionKeyWarnings,
32+
} from '../actionKeys';
33+
34+
const RUNNER = join(dirname(fileURLToPath(import.meta.url)), '..', 'ActionRunner.ts');
35+
36+
/** `ActionDef`'s declared property names, read off the interface itself. */
37+
function declaredActionDefKeys(): string[] {
38+
const sf = ts.createSourceFile(RUNNER, readFileSync(RUNNER, 'utf8'), ts.ScriptTarget.Latest, true);
39+
for (const stmt of sf.statements) {
40+
if (!ts.isInterfaceDeclaration(stmt) || stmt.name.text !== 'ActionDef') continue;
41+
return stmt.members
42+
.filter(ts.isPropertySignature)
43+
.map((m) => (m.name && (ts.isIdentifier(m.name) || ts.isStringLiteral(m.name)) ? m.name.text : null))
44+
.filter((n): n is string => n !== null);
45+
}
46+
throw new Error('ActionDef interface not found in ActionRunner.ts');
47+
}
48+
49+
/**
50+
* The spec's `ActionSchema` keys. `ActionSchema` is a lazy proxy that does not
51+
* forward `.shape`, so this walks zod internals to reach the object shape —
52+
* acceptable HERE (a test pins a fact) and deliberately not done in shipped code.
53+
*/
54+
function specActionKeys(): string[] {
55+
const seen = new Set<unknown>();
56+
const walk = (schema: unknown, depth = 0): string[] | null => {
57+
if (!schema || depth > 8 || seen.has(schema)) return null;
58+
seen.add(schema);
59+
const s = schema as Record<string, unknown>;
60+
const shapeOf = (v: unknown): string[] | null =>
61+
v && typeof v === 'object' ? Object.keys(v as object) : null;
62+
if (s.shape) return shapeOf(s.shape);
63+
const def = (s._def ?? s.def) as Record<string, unknown> | undefined;
64+
if (!def) return null;
65+
if (def.shape) return shapeOf(def.shape);
66+
for (const key of ['in', 'out', 'innerType', 'schema', 'left', 'right']) {
67+
const found = def[key] ? walk(def[key], depth + 1) : null;
68+
if (found) return found;
69+
}
70+
return null;
71+
};
72+
const keys = walk(SpecUI.ActionSchema);
73+
if (!keys) throw new Error('could not resolve @objectstack/spec/ui ActionSchema shape');
74+
return keys;
75+
}
76+
77+
describe('action key inventory (objectstack#4075 step 1)', () => {
78+
it('ActionDef still has the index signature this inventory compensates for', () => {
79+
// The day this fails, step 3 has landed: `tsc` catches unknown keys itself
80+
// and the dev-mode warning (plus `executeScript`'s rename branch) can retire.
81+
expect(readFileSync(RUNNER, 'utf8')).toContain('[key: string]: any');
82+
});
83+
84+
it('lists every key ActionDef declares', () => {
85+
const declared = declaredActionDefKeys();
86+
const missing = declared.filter((k) => !(ACTION_DEF_KEYS as readonly string[]).includes(k));
87+
const stale = (ACTION_DEF_KEYS as readonly string[]).filter((k) => !declared.includes(k));
88+
expect({ missing, stale }).toEqual({ missing: [], stale: [] });
89+
});
90+
91+
it('lists every key the spec ActionSchema declares', () => {
92+
const spec = specActionKeys();
93+
const missing = spec.filter((k) => !(SPEC_ACTION_KEYS as readonly string[]).includes(k));
94+
const stale = (SPEC_ACTION_KEYS as readonly string[]).filter((k) => !spec.includes(k));
95+
// `missing` means the spec grew a key objectui does not know about; `stale`
96+
// means it dropped one. Either way the inventory has to be re-derived, and
97+
// the diff names exactly which key moved.
98+
expect({ missing, stale }).toEqual({ missing: [], stale: [] });
99+
});
100+
101+
it('`execute` is still a live spec tombstone, so it must not count as known', () => {
102+
const parsed = SpecUI.ActionSchema.safeParse({
103+
name: 'mark_done',
104+
label: 'Mark Done',
105+
type: 'script',
106+
execute: 'markDone',
107+
});
108+
expect(parsed.success).toBe(false);
109+
expect(RETIRED_ACTION_KEYS).toHaveProperty('execute');
110+
expect(KNOWN_ACTION_KEYS.has('execute')).toBe(false);
111+
});
112+
113+
it('keeps the navigation alias out of the spec vocabulary it is not part of', () => {
114+
// If the spec ever adopts one of these, it stops being objectui dialect and
115+
// this fails — naming the alias to retire, the same tripwire shape as
116+
// `ObjectUiLocalActionType`.
117+
const spec = specActionKeys();
118+
expect(NAVIGATION_ALIAS_KEYS.filter((k) => spec.includes(k))).toEqual([]);
119+
});
120+
});
121+
122+
describe('unknown-key warning', () => {
123+
let warn: ReturnType<typeof vi.spyOn>;
124+
125+
beforeEach(() => {
126+
resetActionKeyWarnings();
127+
warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
128+
});
129+
afterEach(() => warn.mockRestore());
130+
131+
it('says nothing about an action built only from recognized keys', () => {
132+
warnOnUnknownActionKeys({ name: 'save', type: 'api', target: '/api/v1/save', locations: ['record_header'] });
133+
expect(warn).not.toHaveBeenCalled();
134+
});
135+
136+
it('names a typo that the compiler cannot see', () => {
137+
// `targt` type-checks today: ActionDef's index signature accepts it. Nothing
138+
// reads it, so the action runs and does nothing — #2169's shape.
139+
warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'saveRecord' });
140+
expect(warn).toHaveBeenCalledTimes(1);
141+
expect(warn.mock.calls[0][0]).toContain('`targt`');
142+
});
143+
144+
it('gives a retired key its rename prescription, not a bare "unknown"', () => {
145+
warnOnUnknownActionKeys({ name: 'mark_done', type: 'script', execute: 'markDone' });
146+
expect(warn).toHaveBeenCalledTimes(1);
147+
expect(warn.mock.calls[0][0]).toContain('rename the key to `target`');
148+
});
149+
150+
it('warns once per action, not once per click', () => {
151+
for (let i = 0; i < 5; i++) warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' });
152+
expect(warn).toHaveBeenCalledTimes(1);
153+
});
154+
155+
it('still names a SECOND action carrying the same typo', () => {
156+
// Keying the warn-once memo on the keys alone would report `save` and stay
157+
// silent about `remove` — sending the author to fix the first symptom only.
158+
warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' });
159+
warnOnUnknownActionKeys({ name: 'remove', type: 'script', targt: 'y' });
160+
expect(warn).toHaveBeenCalledTimes(2);
161+
expect(warn.mock.calls[1][0]).toContain('"remove"');
162+
});
163+
164+
it('is silent in production', () => {
165+
const prev = process.env.NODE_ENV;
166+
process.env.NODE_ENV = 'production';
167+
try {
168+
warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' });
169+
expect(warn).not.toHaveBeenCalled();
170+
} finally {
171+
process.env.NODE_ENV = prev;
172+
}
173+
});
174+
175+
it('classifies unknown and retired keys separately', () => {
176+
expect(classifyActionKeys({ type: 'script', execute: 'a', targt: 'b' })).toEqual({
177+
unknown: ['targt'],
178+
retired: ['execute'],
179+
});
180+
});
181+
});

0 commit comments

Comments
 (0)