ci(spec,type-check): spec-named symbols must be derived, 12 packages type-check their tests, ActionDef's keys get an inventory

.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.

.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:

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",

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<ActionResult> {
     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 || '';
 

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<unknown>();
+  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<string, unknown>;
+    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<string, unknown> | 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<typeof vi.spyOn>;
+
+  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'],
+    });
+  });
+});