From d00d1e877cb0faff7ee2c4178edd6386ff539e09 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 10:45:22 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(core,app-shell):=20close=20ActionDef=20?= =?UTF-8?q?=E2=80=94=20delete=20the=20index=20signature,=20converge=20visi?= =?UTF-8?q?ble/disabled=20on=20the=20spec=20(#4046)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of objectstack#4075, executing the maintainer's 2026-08-06 ruling now that its upstream half shipped in @objectstack/spec 17.0.0-rc.6 (objectstack#5970, PR objectstack#6450). - `visible` / `disabled` derive from the spec's unified three-arm shape (`boolean | string(CEL) | { dialect, source }`); `visible` loses its local `| boolean` restatement, `disabled` gains the envelope arm it never had. - `[key: string]: any` deleted from `ActionDef`. `ActionContext` keeps its own — a runtime data bag is legitimately open, a declared contract is not — and the asymmetry is now pinned in both directions. - Five keys the deletion surfaced promoted to real fields: the `navigation` alias spelling (`to` / `external` / `newTab` / `replace`) and `description`. - `DeclaredActionsBar` drops its `(action as any)` predicate casts. - The #4097 ruling-B documentation block lands at `interpolateTarget`'s non-array `params` branch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../actiondef-close-the-index-signature.md | 51 ++++ ...clared-actions-bar-drop-predicate-casts.md | 18 ++ .../src/views/DeclaredActionsBar.tsx | 24 +- packages/core/src/actions/ActionRunner.ts | 183 ++++++++++++-- .../actionDef-closed-surface.test.ts | 237 ++++++++++++++++++ .../actions/__tests__/actionKeys.pin.test.ts | 50 +++- .../__tests__/actionKeys.types.test.ts | 46 +++- packages/core/src/actions/actionKeys.ts | 89 +++++-- packages/core/tsconfig.typetests.json | 9 +- 9 files changed, 638 insertions(+), 69 deletions(-) create mode 100644 .changeset/actiondef-close-the-index-signature.md create mode 100644 .changeset/declared-actions-bar-drop-predicate-casts.md create mode 100644 packages/core/src/actions/__tests__/actionDef-closed-surface.test.ts diff --git a/.changeset/actiondef-close-the-index-signature.md b/.changeset/actiondef-close-the-index-signature.md new file mode 100644 index 0000000000..95ffedda4b --- /dev/null +++ b/.changeset/actiondef-close-the-index-signature.md @@ -0,0 +1,51 @@ +--- +"@object-ui/core": minor +--- + +Close `ActionDef` — delete the `[key: string]: any` index signature and converge `visible` / `disabled` on the spec's unified shape. + +`ActionDef` accepted any key of any type, so a typo (`targt`) and a retired spec +key (`execute`) both type-checked and the runner then silently bound no handler +— the objectstack#2169 "Mark Done does nothing" shape. Step 1 +(objectstack#4075) made that audible with a dev-mode warning; step 2 promoted +the 18 spec-owned keys to real fields. This is **step 3**, executing the +maintainer's 2026-08-06 ruling now that its upstream half shipped in +`@objectstack/spec` 17.0.0-rc.6 (objectstack#5970). + +- **`visible` and `disabled` now have ONE shape, derived from the spec** — + `boolean | string(CEL) | { dialect, source }`. The ruling was "统一形状,spec + 采纳": boolean is the degenerate literal verdict, the string is CEL shorthand, + the envelope is the full form. `visible` loses its hand-written `| boolean` + (the spec adopted that arm, so restating it locally would be a second + contract), and `disabled` gains the envelope arm it never had — it was + `string | boolean`, which is why the envelope the spec emits could only be + read through a cast. +- **The index signature is gone.** `tsc` now rejects an unknown or retired key + at any site that authors an action literal in code. +- **Five keys the deletion surfaced, promoted to real fields.** `to`, + `external`, `newTab`, `replace` — the `navigation` alias's own spelling, ruled + legitimate by step 1 and listed in `NAVIGATION_ALIAS_KEYS` ever since, but + declared only as data; and `description`, which every action renderer forwards + (`check:action-forward-parity` requires it) and the param-collection dialog + reads for its subtitle (objectui#4192). These were the only two `TS2353`s the + deletion produced across the whole workspace. +- **`ActionContext` keeps its index signature**, deliberately. It is a runtime + data bag whose keys are genuinely open; `ActionDef` is a declared metadata + contract. That asymmetry is the point, and it is now pinned in both + directions. + +**Breaking edge, deliberate — same class as step 2's, one step further.** An +`ActionDef` literal carrying a key this interface does not declare is now a +compile error where it previously compiled and did nothing at runtime. That +includes the retired `execute` (rename it to `target`; `os migrate meta --from +16` rewrites it) and plain typos. Values that were only ever absorbed silently +are the ones that stop compiling, so the failure moves to where it can be fixed +rather than appearing as a button that does nothing. + +**What did NOT retire with the index signature**, contrary to step 1's +expectation: the dev-mode `warnOnUnknownActionKeys` shim and `executeScript`'s +`execute` rename prescription both stay. `tsc` only ever sees actions authored +as TypeScript, while stored `sys_metadata` rows are rehydrated UNPARSED +(objectstack#3903) — which is the population `execute: 'markDone'` actually +lives in. The two mechanisms cover disjoint populations; retiring the runtime +half would have re-opened the gap it was written for. diff --git a/.changeset/declared-actions-bar-drop-predicate-casts.md b/.changeset/declared-actions-bar-drop-predicate-casts.md new file mode 100644 index 0000000000..48ead4a244 --- /dev/null +++ b/.changeset/declared-actions-bar-drop-predicate-casts.md @@ -0,0 +1,18 @@ +--- +"@object-ui/app-shell": patch +--- + +`DeclaredActionsBar` reads `visible` / `disabled` off the typed action def instead of through `(action as any)`. + +The `disabled` cast was the one the maintainer's 2026-08-06 ruling on +objectstack#4075 named: `ActionDef.disabled` was hand-written as +`string | boolean` and could not describe the `{ dialect, source }` envelope the +spec emits, so the bar had to reach around the type to evaluate it. With both +keys now derived from the spec's unified three-arm shape (`@object-ui/core`, step +3) there is nothing left to reach around. The adjacent `visible` casts go with +them for the same reason — step 2 declared `visible` and deleted `ActionEngine`'s +equivalent casts, but missed this file's. + +No behaviour change: `toPredicateInput` and `hasDeclaredPredicate` both take +`unknown`, so the casts only ever affected whether the property access compiled, +never which verdict it produced. diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index 3919fa1908..e5ec0b4366 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -150,16 +150,22 @@ const DeclaredActionButton: React.FC<{ // `visible` fails CLOSED on a throwing predicate — mirrors action:button and // ActionEngine.getActionsForLocation: a guard that can't be evaluated hides // the action rather than exposing one whose precondition is broken. - const isVisible = useCondition(toPredicateInput((action as any).visible), predicateRecord, { + const isVisible = useCondition(toPredicateInput(action.visible), predicateRecord, { throwOnError: true, label: `declared action "${action.name ?? action.label ?? 'action'}" (visible)`, }); - // Spec `disabled` (boolean | CEL — disabled when TRUE), evaluated against the - // same record context as `visible`. #1885 wired it in action-button only; - // this bar ignored it, so a spec-authored `disabled` guard on a declared - // action did nothing here. (No legacy `enabled` fallback: server-declared - // actions are spec-shaped and never carried the non-spec key.) - const isDisabledPred = useCondition(toPredicateInput((action as any).disabled), predicateRecord); + // Spec `disabled` — the same three arms as `visible` (`boolean | CEL string | + // { dialect, source }`, disabled when TRUE), evaluated against the same record + // context. #1885 wired it in action-button only; this bar ignored it, so a + // spec-authored `disabled` guard on a declared action did nothing here. (No + // legacy `enabled` fallback: server-declared actions are spec-shaped and never + // carried the non-spec key.) + // + // Read straight off the typed def since objectstack#4075 step 3: both keys are + // now derived from the spec's unified shape, so the `(action as any)` casts + // these two lines carried — which existed only because `ActionDef.disabled` + // could not describe the envelope arm — have nothing left to reach around. + const isDisabledPred = useCondition(toPredicateInput(action.disabled), predicateRecord); const handleClick = useCallback(async () => { if (loading) return; @@ -239,7 +245,7 @@ const DeclaredActionButton: React.FC<{ // The verdict stays with the evaluation entry above: `toPredicateInput` passes // a boolean through untouched and `useCondition` short-circuits it instead of // calling the expression engine, so a declared `false` is `false`. - if (hasDeclaredVisibilityGate((action as any).visible) && !isVisible) return null; + if (hasDeclaredVisibilityGate(action.visible) && !isVisible) return null; const iconName = typeof (action as any).icon === 'string' ? (action as any).icon as string : undefined; // Map the spec's action `variant` enum (primary|secondary|danger|ghost|link) @@ -277,7 +283,7 @@ const DeclaredActionButton: React.FC<{ // a permanently greyed-out Approve / Reject — the mirror image of // objectui#3835 on the same surface, and equally impossible to tell from // deliberate metadata by looking at it. - disabled={(hasDeclaredVisibilityGate((action as any).disabled) ? isDisabledPred : false) || loading} + disabled={(hasDeclaredVisibilityGate(action.disabled) ? isDisabledPred : false) || loading} onClick={handleClick} data-testid={`declared-action-${action.name}`} > diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index f8f8f8d1ad..8646366605 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -21,7 +21,7 @@ * redirect handling, action chaining, custom handler registration. */ -import type { RunnableActionType } from '@object-ui/types'; +import type { RunnableActionType, UIActionSchema } from '@object-ui/types'; import type { Action as SpecActionInput } from '@objectstack/spec/ui'; import { ExpressionEvaluator } from '../evaluator/ExpressionEvaluator'; import { hasDeclaredPredicate } from '../evaluator/declaredPredicate'; @@ -84,6 +84,30 @@ export interface ApiConfig { /** * Action definition accepted by the runner. * Compatible with both UIActionSchema (spec v2.0.1) and legacy crud.ts ActionSchema. + * + * ── CLOSED since objectstack#4075 step 3 ────────────────────────────────────── + * + * This interface used to end with `[key: string]: any`, so it accepted any key + * of any type: deleting `execute` from it produced ZERO compile errors, a typo + * (`targt`, `exectue`) type-checked, and a tombstoned key sailed through to a + * runner that then silently did nothing — the objectstack#2169 "Mark Done does + * nothing" shape. The index signature is gone. Every key this type accepts is + * now written down below, and `tsc` rejects the rest at any site that AUTHORS an + * action literal in code. + * + * What it does NOT reach, and what still covers that: stored `sys_metadata` rows + * are rehydrated UNPARSED (objectstack#3903), so authored metadata arrives at + * `runner.execute()` as a plain object the compiler never saw. The dev-mode + * `warnOnUnknownActionKeys` shim from step 1 therefore STAYS — it is not + * redundant with the closed type, it is the other half of the same surface, and + * the two halves catch disjoint populations (code-authored literals vs. stored + * rows). + * + * {@link ActionContext} keeps ITS index signature, deliberately. It is a runtime + * data bag whose keys are genuinely open (`data`, `record`, `pageVariables`, + * `user`, plus whatever a host passes); this is a declared metadata contract + * mirroring `@objectstack/spec`'s `ActionSchema`. Open key set on a data bag is + * correct; open key set on a contract is the bug above. */ export interface ActionDef { /** Action type identifier — a `RunnableActionType` (the spec's six plus the @@ -100,6 +124,24 @@ export interface ActionDef { name?: string; /** Display label */ label?: string; + /** + * Human description of the action, shown as the param-collection dialog's + * subtitle (`useConsoleActionRuntime` reads it beside `label`). + * + * objectui vocabulary, not spec: `@objectstack/spec`'s `ActionSchema` has no + * `description`, so this derives from `@object-ui/types`' renderer view of an + * action (`UIActionSchema`, `ui-action.ts`) — the same "authorable for a + * declared surface" source `check:action-forward-parity` derives its owed set + * from, which is why that gate already requires all four action renderers to + * forward this key (objectui#4192: `action:menu`'s dialog titled itself + * "Action parameters" instead of naming the action). + * + * Promoted by step 3 (objectstack#4075): the key was authorable, forwarded + * and read, but `ActionDef` never declared it, so it reached its reader only + * through `[key: string]: any`. Deleting the index signature is what surfaced + * it — the two TS2353s the deletion produced repo-wide were both this key. + */ + description?: UIActionSchema['description']; /** Confirmation text — shows a confirm dialog before executing */ confirmText?: string; /** Structured confirmation (from crud.ts) */ @@ -115,8 +157,20 @@ export interface ActionDef { * the same widening `disabled` already carries below. */ condition?: string | boolean; - /** Disabled expression — if truthy, skip action */ - disabled?: string | boolean; + /** + * Disabled predicate — the control renders inert while it holds. + * + * Same three arms as {@link ActionDef.visible}, derived from the same spec + * field, because the 2026-08-06 maintainer ruling on objectstack#4075 gave + * both keys ONE shape: `boolean | string(CEL) | { dialect, source }` — + * "boolean = 条件的退化形字面量,string = CEL 简写,信封 = 完整形". The + * envelope arm arrived in `@objectstack/spec` 17.0.0-rc.6 (objectstack#5970, + * PR objectstack#6450); until then this was a hand-written `string | boolean` + * that could not describe the envelope, which is why `DeclaredActionsBar` + * read it through an `(action as any).disabled` cast. Derived, not restated, + * so the two keys cannot drift apart again. + */ + disabled?: SpecActionInput['disabled']; /** * API endpoint (string URL or complex config). * @@ -148,6 +202,43 @@ export interface ActionDef { * today (objectstack#4075 step 2). */ navigate?: any; + + // ── The `navigation` alias's own spelling, promoted by step 3 ────────────── + // + // objectstack#4075 step 3. Step 1's inventory established these four as a + // REAL objectui dialect rather than undeclared drift — `executeNavigation` + // reads them off the action itself when no nested `navigate` envelope is + // present, and `NAVIGATION_ALIAS_KEYS` in `actionKeys.ts` has listed them as + // legitimate since then, with a tripwire that fires if the spec ever adopts + // one of the names. But step 2 promoted only the 18 SPEC-owned keys, so these + // four kept reaching their own reader through `[key: string]: any`. Deleting + // the index signature is what surfaced that: `nav.to`, `source.external`, + // `source.newTab` and `source.replace` were the four TS2339s the deletion + // produced inside this file. + // + // Declared, deliberately NOT `@deprecated`: step 2's acceptance ruled the + // dispatch wrong on exactly this point — "navigation 别名四键不标弃用(第 1 步 + // 盘点已判定其合法,派发指令有误)". They are objectui vocabulary the runner + // itself implements, not an alias of a spec spelling an author should prefer. + // Hand-written rather than derived because there is no spec field to derive + // FROM — that is what makes them dialect. `replace` is the load-bearing case: + // it is documented at its read site as "the one thing the `navigation` shape + // carries that `ActionSchema` has no field for". + + /** `navigation` alias: the target URL. `executeNavigation` resolves + * `to || target || redirect`, so a flat spec-style `target` also works. */ + to?: string; + /** `navigation` alias: force the external-URL treatment that the scheme + * heuristic would otherwise decide. */ + external?: boolean; + /** `navigation` alias: open in a new tab. The first-class spec switch is + * `openIn: 'new-tab'`, which WINS over this one — prefer it in new metadata. + * Unrelated to the retired `params.newTab` read (objectui#4097). */ + newTab?: boolean; + /** `navigation` alias: replace the current history entry instead of pushing + * a new one. No `ActionSchema` counterpart in any spelling. */ + replace?: boolean; + /** onClick callback (legacy) */ onClick?: () => void | Promise; /** Whether to reload data after success */ @@ -231,7 +322,7 @@ export interface ActionDef { * which re-runs every check the POST half would have done). */ newTabUrl?: string; - // ── Spec-owned keys, promoted from the index signature ───────────────────── + // ── Spec-owned keys, promoted out of the index signature ─────────────────── // // objectstack#4075 step 2. Step 1's inventory found 18 keys that // `@objectstack/spec`'s `ActionSchema` owns and that authored metadata @@ -272,16 +363,23 @@ export interface ActionDef { /** * Visibility predicate, evaluated against the runner's context. * - * The `boolean` arm is objectui's own, deliberately wider than the spec: - * `ActionSchema.visible` admits only a CEL string or a `{ dialect, source }` - * envelope, while `ActionEngine.getActionsForLocation` also honours a literal - * `visible: true` / `false` (pinned by `ActionEngine.visibility.test.ts`). - * Declared rather than left to the index signature so the tolerance is - * auditable instead of invisible — but it IS a dialect, and objectstack#4075 - * step 3 has to decide whether the spec adopts the boolean or objectui drops - * it. Do not widen this further. + * Three arms — `boolean | string(CEL) | { dialect, source }` — and all three + * now come from the spec. Step 2 had to write `SpecActionInput['visible'] | + * boolean` because the literal `visible: true` / `false` that + * `ActionEngine.getActionsForLocation` honours (pinned by + * `ActionEngine.visibility.test.ts`) was objectui tolerance the spec did not + * share, and it left the open question step 3 was blocked on: does the spec + * adopt the boolean, or does objectui drop it? + * + * The maintainer ruled ADOPT on 2026-08-06 (objectstack#4075): "统一形状, + * spec 采纳 —— `visible` / `disabled` 两键在 spec 侧统一收敛为 + * `boolean | string(CEL) | {dialect, source}`(boolean = 条件的退化形字面量, + * string = CEL 简写,信封 = 完整形)". `@objectstack/spec` 17.0.0-rc.6 carries + * it (objectstack#5970, PR objectstack#6450), so the local `| boolean` is no + * longer a tolerance to declare — it is part of the derived type, and adding + * it back would restate a spec arm rather than widen anything. */ - visible?: SpecActionInput['visible'] | boolean; + visible?: SpecActionInput['visible']; /** System permissions the caller must ALL hold for the action to be offered * (ADR-0066 D4 UI half — the server enforces the source of truth). */ requiredPermissions?: SpecActionInput['requiredPermissions']; @@ -327,9 +425,6 @@ export interface ActionDef { * remains available to a HOST passing it explicitly. */ bulkEnabled?: SpecActionInput['bulkEnabled']; - - /** Any additional properties */ - [key: string]: any; } /** @@ -1052,12 +1147,25 @@ export class ActionRunner { 'build one with createServerActionHandler from @object-ui/core.', }; } - // ActionDef is open-ended (`[key: string]: any`), so hand-authored - // metadata that never passed through the spec parser still compiles with - // the retired key. Carry the same prescription the spec's tombstone does, - // for the same reason: a bare "no script provided" reads as "you forgot a - // field" to an author who did write one. - if (typeof action.execute === 'string') { + // Read off an UNTYPED view of the action on purpose, and this branch + // survives step 3 on purpose (objectstack#4075). + // + // `ActionDef` no longer declares `execute` — nor any open index signature + // — so `tsc` now catches the retired key at every site that AUTHORS an + // action literal in code. That is the half of the problem the type can + // reach. It is not this half: #3903 established that stored + // `sys_metadata` rows are rehydrated UNPARSED, so metadata written before + // spec 17 arrives here as a plain object the compiler never saw, still + // carrying `execute`. Declaring the key to make this read compile would + // re-legitimize a tombstone (#3855 removed it; the spec keeps it only to + // reject it BY NAME), and deleting the branch would send those rows back + // to a bare "no script provided" — which reads as "you forgot a field" to + // an author who did write one, the objectstack#2169 shape. + // + // So: the cast is the honest spelling of "a key this type deliberately + // does not have, read from a row this type never validated". + const retiredExecute = (action as Record).execute; + if (typeof retiredExecute === 'string') { return { success: false, error: @@ -1527,6 +1635,37 @@ export class ActionRunner { */ private interpolateTarget(target: string, action: ActionDef): string { if (typeof target !== 'string' || target.indexOf('${') === -1) return target; + // ── The `${param.X}` scope is an INTERNAL runtime value bag, not an + // authoring surface (objectui#4097 ruling B, 2026-08-11) ────────────── + // + // A non-array `params` reaches this line from ONE direction only: a host + // synthesizing a value bag on its way into `runner.execute()` — the + // collected values a param dialog just gathered, `_selectedIds` from an + // aggregate bulk dispatch, `_rowRecord` from a row surface. Eight internal + // producers, measured. No AUTHOR can put a value here: `ActionSchema.params` + // is `z.array(ActionParamSchema)`, so the object form is refused at parse, + // and the refusal message names the sanctioned spellings. + // + // That refusal is why this branch survived objectui#4097 while its two + // siblings did not. The objectstack#6828 maintainer ruling targeted the + // AUTHORING vocabulary — "a key with three meanings and no authorized + // spelling for the third" — and both authored meanings are now handled + // upstream at parse; PR objectui#4262 removed the third runtime read + // (`params.newTab`). What is left here is a channel no author can reach or + // observe, so removing it would spend a four-package runtime-contract + // migration on an already-unreachable shape. Option A was declined for + // exactly that reason: "plumbing expansion with zero pull". + // + // ⚠️ The hazard the ruling flagged, recorded here so the next reader sees + // it at the code rather than in a closed thread: this line's safety rests + // ENTIRELY on the producer refusing an authored object-form `params`. If a + // future spec loosening ever admits one, an authored scope silently becomes + // reachable again — and it would arrive as an interpolation source for URLs + // and api targets, which is the shape a params dialog field named for a + // navigation directive could steer. The guards are the upstream refusal pin + // and the two local pins (`ActionRunner.resultDialog.test.ts`, + // `ActionRunner.test.ts`); a spec change that turns them red is the signal, + // not a nuisance. const params = (action.params && typeof action.params === 'object' && !Array.isArray(action.params)) ? (action.params as Record) : {}; diff --git a/packages/core/src/actions/__tests__/actionDef-closed-surface.test.ts b/packages/core/src/actions/__tests__/actionDef-closed-surface.test.ts new file mode 100644 index 0000000000..f2855f2fc8 --- /dev/null +++ b/packages/core/src/actions/__tests__/actionDef-closed-surface.test.ts @@ -0,0 +1,237 @@ +/** + * 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. + */ + +/** + * `ActionDef` is a CLOSED surface — objectstack#4075 step 3, pinned at compile + * time. + * + * The card's whole complaint was that the compiler could not help: with + * `[key: string]: any` on the interface, deleting `ActionDef.execute` produced + * ZERO compile errors, a typo (`targt`) type-checked, and both sailed through to + * a runner that then did nothing (objectui#2990, objectstack#2169). Step 3 + * deleted the index signature. These assertions are what makes that deletion a + * pin rather than a diff. + * + * ── Read the `@ts-expect-error` direction carefully ────────────────────────── + * Each one asserts THE NEXT LINE DOES NOT COMPILE. So a suppression here goes + * red in the direction most people do not expect: if the key becomes ACCEPTED + * again — say the index signature is restored — the assignment compiles, the + * suppression has nothing to suppress, and `tsc` fails the build with + * "Unused '@ts-expect-error' directive". There is no way to end up quietly + * green. That inversion is the reverse-verification mechanism for this change + * and is stated here because it is not the intuitive reading. + * + * ── Why a second type-test file, next to `actionKeys.types.test.ts` ────────── + * They pin different things and fail differently, so neither subsumes the other. + * That file drives the compiler API over a virtual module and compares the + * diagnostics against a control interface carrying nothing but an index + * signature — which is what proves the REJECTIONS come from the promotion and + * not from some unrelated error. This file states the same contract as ordinary + * TypeScript that CI compiles directly (via `tsconfig.typetests.json`), so the + * pin survives even if the harness's compiler-host plumbing ever breaks, and a + * reader can see the rejected spellings without decoding a test harness. + * + * These assertions are TYPES: this package's own `tsconfig.json` is the BUILD + * config and excludes `src/**` test files, so without the explicit entry in + * `tsconfig.typetests.json` (chained off `type-check`, enforced by + * `scripts/check-type-check-coverage.mjs`) nothing would compile this file and + * every pin below would be decoration — the "declared != enforced" landmine + * objectstack#4115 exists to remove. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ActionRunner, type ActionDef } from '../ActionRunner'; +import { ActionEngine } from '../ActionEngine'; + +// ── Retired and misspelled keys are now compile errors ─────────────────────── + +// The tombstone. `execute` was removed from `@objectstack/spec` in 17 +// (objectstack#3855) and the spec keeps it only so the parser can reject it BY +// NAME with a rename prescription. Before step 3 this line compiled — which is +// the sentence the card was filed over. +// @ts-expect-error - `execute` is retired; the spec's prescription is `target` +const _retiredExecute: ActionDef = { type: 'script', execute: 'markDone' }; + +// The typo. `targt` reaches no reader, so the action ran and did nothing — +// objectstack#2169's shape, and the case `warnOnUnknownActionKeys` was written +// to make audible while the compiler still could not see it. +// @ts-expect-error - `targt` is a typo for `target` +const _typoTarget: ActionDef = { type: 'script', targt: 'saveRecord' }; + +// A retired SPEC key that is not a tombstone, to show the closure is general +// rather than a list of two special cases. +// @ts-expect-error - `bulkEnabled` is a retiredKey() tombstone in spec 17 +const _retiredBulk: ActionDef = { type: 'api', bulkEnabled: true }; + +// ── The unified `visible` / `disabled` shape: three arms each ──────────────── +// +// The 2026-08-06 maintainer ruling on objectstack#4075 — "统一形状,spec 采纳" +// — gave both keys one shape, `boolean | string(CEL) | { dialect, source }`, +// and `@objectstack/spec` 17.0.0-rc.6 carries it (objectstack#5970). All six +// combinations must type-check; the runtime half is pinned in +// `ActionRunner.disabledGate.test.ts` / `ActionEngine.visibility.test.ts`. + +const _visibleBool: ActionDef = { type: 'script', visible: true }; +const _visibleCel: ActionDef = { type: 'script', visible: 'record.status == "open"' }; +const _visibleEnvelope: ActionDef = { + type: 'script', + visible: { dialect: 'cel', source: 'record.status == "open"' }, +}; + +const _disabledBool: ActionDef = { type: 'script', disabled: true }; +const _disabledCel: ActionDef = { type: 'script', disabled: 'record.locked' }; +const _disabledEnvelope: ActionDef = { + type: 'script', + disabled: { dialect: 'cel', source: 'record.locked' }, +}; + +// And the shape is narrow in both directions — a number is neither a literal +// verdict, a CEL string, nor an envelope. Before step 3 the index signature +// swallowed this too, on both keys. +// @ts-expect-error - a number is none of the three arms +const _visibleNumber: ActionDef = { type: 'script', visible: 42 }; +// @ts-expect-error - a number is none of the three arms +const _disabledNumber: ActionDef = { type: 'script', disabled: 42 }; + +// ── The five keys the deletion itself surfaced ─────────────────────────────── +// +// Step 3 promoted these; before it they reached their own readers through the +// index signature. Each accepts its declared type and rejects a wrong one, which +// is what separates "promoted to a real field" from "promoted to `any`". + +const _description: ActionDef = { type: 'api', description: 'Creates an environment' }; +// @ts-expect-error - `description` is a string, not a number +const _descriptionNumber: ActionDef = { type: 'api', description: 42 }; + +const _navigationAlias: ActionDef = { + type: 'navigation', + to: '/records/1', + external: false, + newTab: true, + replace: false, +}; +// @ts-expect-error - `newTab` is the boolean switch, not the `openIn` string +const _navNewTabString: ActionDef = { type: 'navigation', to: '/x', newTab: 'new-tab' }; + +describe('ActionDef is a closed surface (objectstack#4075 step 3)', () => { + it('states its contract as types, compiled by tsconfig.typetests.json', () => { + // The assertions above are the test. This body exists so the file is a + // legitimate vitest suite as well as a tsc input, and so a reader running + // the suite sees the pin acknowledged rather than an empty file. + expect([ + _retiredExecute, _typoTarget, _retiredBulk, + _visibleBool, _visibleCel, _visibleEnvelope, + _disabledBool, _disabledCel, _disabledEnvelope, + _visibleNumber, _disabledNumber, + _description, _descriptionNumber, _navigationAlias, _navNewTabString, + ]).toHaveLength(15); + }); +}); + +/** + * The JOIN between the two faces — deliberately not a duplicate of either. + * + * Both keys already have thorough RUNTIME coverage of all three arms, and it + * stays where it is: `ActionRunner.disabledGate.test.ts` drives a 12-row shape + * table through the execution gate (boolean / bare CEL / envelope / `${…}` / + * junk, both polarities) and `ActionEngine.visibility.test.ts` does the same for + * `visible` through the location filter. Re-asserting those verdicts here would + * be coverage theatre. + * + * What neither of them can assert is the join, because both reach their runtime + * through a cast — `runShape` builds a `Record` and casts + * `as unknown as ActionDef`, and the visibility suite writes `as ActionDef`. + * Those casts are correct there (the tables deliberately carry junk shapes like + * `disabled: 0` that the closed type now rejects), but they mean the existing + * pins would keep passing even if the declared type stopped admitting the arm + * being exercised. Step 3's claim is that the SAME literal both type-checks and + * evaluates, so that is what this asserts: the six consts above — already + * type-checked by virtue of being `ActionDef` with no cast — reaching a real + * verdict. + */ +describe('the six unified arms type-check AND evaluate (the same literal, both faces)', () => { + const CONTEXT = { record: { status: 'open', locked: true }, user: { role: 'admin' } }; + + /** + * Does the runner's `disabled` GATE block this action? + * + * Two details, both put here by the negative control below catching a first + * draft that lacked them: + * + * `type` is dropped and an `onClick` handler attached, so the action has + * something to actually DO. The literals above say `type: 'script'` because + * that is the natural spelling for a type pin, but a script action with no + * registered script fails on its own terms — and a first draft that just + * asked `result.success === false` scored those failures as "blocked" and + * reported all six arms blocking, including the three that must release. + * + * The verdict is then read off the gate's OWN error rather than off + * `success`, so "the gate blocked it" cannot be confused with "it failed for + * some other reason" — the same string `ActionRunner.disabledGate.test.ts` + * asserts. + */ + async function blocked(action: ActionDef): Promise { + const onClick = vi.fn(); + const result = await new ActionRunner(CONTEXT).execute({ ...action, type: undefined, onClick }); + const isBlocked = result.error === 'Action is disabled'; + expect(onClick.mock.calls.length, 'a blocked action must not run its handler').toBe(isBlocked ? 0 : 1); + return isBlocked; + } + + /** Does the engine's location filter keep this action? */ + function shown(action: ActionDef): boolean { + const engine = new ActionEngine(new ActionRunner(CONTEXT)); + engine.updateContext(CONTEXT); + engine.registerAction({ ...action, name: 'a' }, { locations: ['record_section'] }); + return engine.getActionsForLocation('record_section').length === 1; + } + + // Each arm is asserted on BOTH polarities. A positive-only version of these + // two cases would be green if the gate were ignored altogether — every action + // runs, every action shows — which is the "passes because nothing is + // produced" shape. The negative half is what makes the verdict a verdict: + // these literals are `ActionDef` with no cast, so they carry the type claim + // too, and each pair moves in opposite directions on the same key. + const _disabledBoolOff: ActionDef = { type: 'script', disabled: false }; + const _disabledCelOff: ActionDef = { type: 'script', disabled: 'record.status == "closed"' }; + const _disabledEnvelopeOff: ActionDef = { + type: 'script', + disabled: { dialect: 'cel', source: 'record.status == "closed"' }, + }; + const _visibleBoolOff: ActionDef = { type: 'api', visible: false }; + const _visibleCelOff: ActionDef = { type: 'api', visible: 'record.status == "closed"' }; + const _visibleEnvelopeOff: ActionDef = { + type: 'api', + visible: { dialect: 'cel', source: 'record.status == "closed"' }, + }; + + it('`disabled` blocks on all three arms, uncast — and releases on all three', async () => { + // `record.locked` is true in CONTEXT, so every "on" arm means block; the + // "off" arms ask about `record.status == "closed"`, which is false. + expect({ + blocks: [ + await blocked(_disabledBool), + await blocked(_disabledCel), + await blocked(_disabledEnvelope), + ], + releases: [ + await blocked(_disabledBoolOff), + await blocked(_disabledCelOff), + await blocked(_disabledEnvelopeOff), + ], + }).toEqual({ blocks: [true, true, true], releases: [false, false, false] }); + }); + + it('`visible` shows on all three arms, uncast — and hides on all three', () => { + // `record.status == "open"` holds in CONTEXT, so every "on" arm means show. + expect({ + shows: [shown(_visibleBool), shown(_visibleCel), shown(_visibleEnvelope)], + hides: [shown(_visibleBoolOff), shown(_visibleCelOff), shown(_visibleEnvelopeOff)], + }).toEqual({ shows: [true, true, true], hides: [false, false, false] }); + }); +}); diff --git a/packages/core/src/actions/__tests__/actionKeys.pin.test.ts b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts index c8e3e7b14a..97f5ba77b2 100644 --- a/packages/core/src/actions/__tests__/actionKeys.pin.test.ts +++ b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts @@ -33,17 +33,26 @@ import { const RUNNER = join(dirname(fileURLToPath(import.meta.url)), '..', 'ActionRunner.ts'); -/** `ActionDef`'s declared property names, read off the interface itself. */ -function declaredActionDefKeys(): string[] { +/** The named interface's members, read off `ActionRunner.ts` itself. */ +function interfaceMembers(name: string): readonly ts.TypeElement[] { 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); + if (ts.isInterfaceDeclaration(stmt) && stmt.name.text === name) return stmt.members; } - throw new Error('ActionDef interface not found in ActionRunner.ts'); + throw new Error(`${name} interface not found in ActionRunner.ts`); +} + +/** `ActionDef`'s declared property names, read off the interface itself. */ +function declaredActionDefKeys(): string[] { + return interfaceMembers('ActionDef') + .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); +} + +/** Does the named interface end with an `[key: string]: any`-style catch-all? */ +function hasIndexSignature(name: string): boolean { + return interfaceMembers(name).some(ts.isIndexSignatureDeclaration); } /** @@ -75,10 +84,27 @@ function specActionKeys(): string[] { } 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('ActionDef has NO index signature, and ActionContext still does', () => { + // The successor to step 1's inverted pin. That one asserted the index + // signature was STILL THERE and named its own retirement condition: "the day + // this fails, step 3 has landed". Step 3 landed, so the assertion inverts + // rather than disappears — the property is still worth pinning, in the + // opposite direction, and silently deleting the pin would have left the + // deletion unguarded against a well-meaning re-add. + // + // Asserted through the AST, not `toContain('[key: string]: any')` as the + // original did. A whole-file text match cannot tell the DECLARATION from + // prose ABOUT it, and both files now discuss the index signature at length; + // a text-negative pin would go red on a comment. It would also have been + // blind to the half of this assertion that carries the real discrimination: + // `ActionContext` KEEPING its own index signature. The card turns on that + // asymmetry — a runtime data bag is legitimately open, a declared metadata + // contract is not — so a pin that only checked `ActionDef` would stay green + // through a change that "tidied up" `ActionContext` too. + expect({ + ActionDef: hasIndexSignature('ActionDef'), + ActionContext: hasIndexSignature('ActionContext'), + }).toEqual({ ActionDef: false, ActionContext: true }); }); it('lists every key ActionDef declares', () => { diff --git a/packages/core/src/actions/__tests__/actionKeys.types.test.ts b/packages/core/src/actions/__tests__/actionKeys.types.test.ts index 6fa251602d..4403641cf4 100644 --- a/packages/core/src/actions/__tests__/actionKeys.types.test.ts +++ b/packages/core/src/actions/__tests__/actionKeys.types.test.ts @@ -7,8 +7,9 @@ */ /** - * Type-level pins for objectstack#4075 step 2 — the 18 spec-owned keys that - * `ActionDef` now declares explicitly. + * Type-level pins for objectstack#4075 — the keys `ActionDef` declares + * explicitly (step 2's 18 spec-owned ones, plus the five step 3's deletion + * surfaced) and, since step 3, the keys it now REFUSES. * * Why this file has to drive `tsc` itself: the property under test is that the * COMPILER rejects a wrong-typed value. A normal runtime test cannot observe @@ -26,6 +27,13 @@ * assertions go green-to-red, because the index signature swallows them again, * which is the entire bug objectstack#4075 describes. * + * Since step 3 that control also covers the deletion itself, not just the + * promotions. `{ targt: … }` and `{ execute: … }` are now `rejected: true`, and + * the only thing standing between them and silence is the absence of the index + * signature — so restoring it makes those two rows leak into the control's + * "leaked" list as well as flipping them here. The control is no longer a + * historical foil; it is the exact shape of the thing that was removed. + * * Cost note (AGENTS.md 测试纪律): the program is built once at MODULE SCOPE, not * in `beforeAll`. Module-scope work runs in the import phase and is bound by no * test or hook timeout; a `beforeAll` would put ~3s of compiler work under the @@ -94,11 +102,35 @@ const CASES: readonly Case[] = [ { what: 'visible accepts a { dialect, source } envelope', rejected: false, code: `{ visible: { dialect: 'cel', source: 'record.done == false' } }` }, { what: 'visible accepts a literal boolean (objectui tolerance, pinned by ActionEngine.visibility.test.ts)', rejected: false, code: `{ visible: true }` }, - // Step 3's completion check, from the type side. While `[key: string]: any` - // stands, a typo is still absorbed silently — which is precisely why the - // dev-mode warning in `actionKeys.ts` exists. When this case starts FAILING, - // step 3 has landed and the warning can retire. - { what: 'an unrecognized key is still absorbed — the index signature stands', rejected: false, code: `{ targt: 'form_1' }` }, + // `disabled`'s three arms, converged onto `visible`'s by the 2026-08-06 + // maintainer ruling and delivered by `@objectstack/spec` 17.0.0-rc.6 + // (objectstack#5970). Step 2 could only declare `string | boolean` by hand, + // which is why `DeclaredActionsBar` still read the key through a cast: the + // envelope arm the spec emits had no declaration to land in. + { what: 'disabled accepts a literal boolean', rejected: false, code: `{ disabled: true }` }, + { what: 'disabled accepts a raw CEL string', rejected: false, code: `{ disabled: 'record.locked' }` }, + { what: 'disabled accepts a { dialect, source } envelope', rejected: false, code: `{ disabled: { dialect: 'cel', source: 'record.locked' } }` }, + { what: 'disabled rejects a number — neither predicate, envelope, nor boolean', rejected: true, code: `{ disabled: 42 }` }, + + // The five keys step 3's deletion surfaced: four `navigation`-alias spellings + // that step 1 had ruled legitimate but step 2 left as data, and `description`, + // which every action renderer forwards and the param dialog reads. + { what: 'the navigation alias spelling is declared', rejected: false, code: `{ to: '/records/1', external: false, newTab: true, replace: false }` }, + { what: 'newTab is the boolean switch, not the openIn string', rejected: true, code: `{ newTab: 'new-tab' }` }, + { what: 'description accepts the human string the param dialog titles itself with', rejected: false, code: `{ description: 'Creates an environment' }` }, + { what: 'description rejects a number', rejected: true, code: `{ description: 42 }` }, + + // Step 3's completion check, from the type side, INVERTED by step 3 landing. + // While `[key: string]: any` stood, a typo was absorbed silently and this case + // read `rejected: false` under a comment promising it would flip. It flipped. + // + // The dev-mode warning it pointed at did NOT retire with it, and the reason is + // the one thing this file cannot pin: `tsc` only ever sees actions authored as + // TypeScript, while `execute: 'markDone'` lives in stored `sys_metadata` rows + // that reach the runner UNPARSED (objectstack#3903). Closing the type is half + // the surface; the warning is the other half. + { what: 'an unrecognized key is REJECTED — the index signature is gone', rejected: true, code: `{ targt: 'form_1' }` }, + { what: 'the retired `execute` tombstone is rejected by name', rejected: true, code: `{ execute: 'markDone' }` }, ]; /** `ActionDef` as it was BEFORE step 2: nothing but the open index signature. */ diff --git a/packages/core/src/actions/actionKeys.ts b/packages/core/src/actions/actionKeys.ts index 2358e7af4d..ad7906dc0a 100644 --- a/packages/core/src/actions/actionKeys.ts +++ b/packages/core/src/actions/actionKeys.ts @@ -7,26 +7,46 @@ */ /** - * What an action is allowed to carry — objectstack#4075 step 1. + * What an action is allowed to carry — objectstack#4075, all three steps. * - * `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. + * `ActionDef` used to end with `[key: string]: any`, so it accepted 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 authoring `execute: 'markDone'` type-checked. 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 + * could catch a retired key; the other was structurally incapable. * - * That asymmetry is the issue. An open key set on a DECLARED METADATA CONTRACT + * That asymmetry was 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. + * This module was step 1 of the staged narrowing: make the key set VISIBLE and + * warn on anything outside it, without changing a single type. Step 2 promoted + * the legitimate keys to explicit optional fields; **step 3 removed the index + * signature**, so `tsc` now rejects a typo or a retired key outright. + * + * ── Why this module SURVIVES step 3 ────────────────────────────────────────── + * Step 1 wrote that the dev-mode warning "can retire" once the index signature + * came down. Step 3 measured that and kept it, because the two mechanisms cover + * DISJOINT populations rather than the same one twice: + * + * - `tsc` sees action literals AUTHORED IN CODE. It rejects an unknown key at + * the construction site, which is the strongest possible signal — but only + * for actions that exist as TypeScript. + * - the warning below sees actions that arrive as DATA. objectstack#3903 + * established that stored `sys_metadata` rows are rehydrated UNPARSED, so + * authored metadata reaches `runner.execute()` as a plain object no + * compiler ever looked at. That is the population `execute: 'markDone'` + * actually lives in, and no type can reach it. + * + * Retiring the warning would therefore have re-opened the runtime half of the + * gap while closing the compile-time half — which is why `executeScript`'s + * rename-prescription branch also survives, for the same rows and the same + * reason. What DID retire with the index signature is the inverted pin in + * `actionKeys.pin.test.ts` that asserted the signature was still there; it now + * asserts the opposite, which was always its stated completion condition. * * 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 @@ -67,8 +87,22 @@ * deprecating them toward a spelling that does not exist would be worse than * leaving them declared. * - * Step 3 remains: remove the index signature, at which point `tsc` catches both - * typos and retired keys and the dev-mode warning below can retire with it. + * ── What step 3 found the first two steps had missed ───────────────────────── + * Deleting the index signature is the only thing that can name the keys still + * hiding behind it, and it named five, in two groups: + * + * `to` / `external` / `newTab` / `replace` — the `navigation` alias's own + * spelling. Step 1 had already ruled these legitimate and listed them below in + * {@link NAVIGATION_ALIAS_KEYS}, but step 2's scope was the SPEC-owned keys, so + * they were declared as data and never as fields; `executeNavigation` read them + * through the index signature. Promoted, and deliberately not `@deprecated` — + * step 2's acceptance ruled on exactly that point. + * + * `description` — objectui vocabulary from `@object-ui/types`' renderer view of + * an action, forwarded by all four action renderers (`check:action-forward-parity` + * requires it) and read by the param-collection dialog for its subtitle + * (objectui#4192). Authorable, forwarded, read — and undeclared. These were the + * only two `TS2353`s the deletion produced across the whole workspace. */ /** @@ -83,6 +117,10 @@ export const ACTION_DEF_KEYS = [ 'actionType', 'name', 'label', + // Promoted by step 3 — objectui vocabulary (`@object-ui/types`' renderer view + // of an action), forwarded by all four action renderers and read by the + // param-collection dialog, but never declared on `ActionDef`. + 'description', 'confirmText', 'confirm', 'condition', @@ -91,6 +129,14 @@ export const ACTION_DEF_KEYS = [ 'endpoint', 'method', 'navigate', + // The `navigation` alias's own spelling, promoted to real fields by step 3. + // Listed a second time in `NAVIGATION_ALIAS_KEYS` below, which keeps the + // separate fact that these four are objectui dialect with a spec-adoption + // tripwire; this list is only "what the interface declares". + 'to', + 'external', + 'newTab', + 'replace', 'onClick', 'reload', 'close', @@ -231,8 +277,15 @@ export const NAVIGATION_ALIAS_KEYS = ['to', 'external', 'newTab', 'replace'] as * 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. + * rename prescription. + * + * Step 1 expected that branch to retire with the index signature. It does not, + * and the reason is the one in this module's header: `tsc` closed the + * code-authored half of the gap, while `execute: 'markDone'` lives in stored + * rows that reach the runner UNPARSED (objectstack#3903). The branch reads the + * key off an untyped view of the action precisely so that `ActionDef` can go on + * NOT declaring it — declaring it to make the read compile would re-legitimize + * a tombstone the spec keeps only in order to reject it by name. */ export const RETIRED_ACTION_KEYS: Readonly> = { execute: '`execute` was removed in @objectstack/spec 17 (#3855) — rename the key to `target`. ' + diff --git a/packages/core/tsconfig.typetests.json b/packages/core/tsconfig.typetests.json index 0910c080ed..556620c266 100644 --- a/packages/core/tsconfig.typetests.json +++ b/packages/core/tsconfig.typetests.json @@ -27,5 +27,12 @@ // graph rather than through sibling `src/`. "paths": {} }, - "include": ["src/utils/__tests__/dataset-result-field-spec-parity.test.ts"] + "include": [ + "src/utils/__tests__/dataset-result-field-spec-parity.test.ts", + // objectstack#4075 step 3 — `ActionDef` is a closed surface. Every + // assertion in this file is an `@ts-expect-error` or a bare assignment, so + // it is a compile error or it is nothing; the package's build config + // excludes `src/**` test files, which is why it has to be listed here. + "src/actions/__tests__/actionDef-closed-surface.test.ts" + ] } From 796c21b30c406bf7ccc8bce2bc0eb5e8a15e72b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 11:18:05 +0000 Subject: [PATCH 2/2] fix(core): move the `description` type pin out of the compiler-API harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI shard 3/4 went red on `description rejects a number` while the same case was green locally, and the difference is the harness's own resolution model rather than anything about `ActionDef`. `erroringLines` builds its program with a default compiler host and no `paths`, so it reads whatever is on disk. Every other derivation it checks resolves through `@objectstack/spec`, an installed package; `description` derives from `UIActionSchema['description']` in `@object-ui/types`, a WORKSPACE package that resolves through its built `dist/index.d.ts`. The unit-test job does not build workspace packages first, so the indexed access degraded to `any`, the case was accepted, and the row went red for an unrelated reason. `tsconfig.typetests.json` is a real project in the dependency graph, so CI's Type Check job builds `@object-ui/types` before compiling it and the `@ts-expect-error` on `{ description: 42 }` there is enforced for real — which is why Type Check was green on the same commit. The pin therefore moves rather than weakens; an assertion whose colour depends on whether someone ran a build is not a pin. Reproduced locally by moving `packages/types/dist` aside: 37 passed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../__tests__/actionKeys.types.test.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/core/src/actions/__tests__/actionKeys.types.test.ts b/packages/core/src/actions/__tests__/actionKeys.types.test.ts index 4403641cf4..c33af59c65 100644 --- a/packages/core/src/actions/__tests__/actionKeys.types.test.ts +++ b/packages/core/src/actions/__tests__/actionKeys.types.test.ts @@ -117,8 +117,28 @@ const CASES: readonly Case[] = [ // which every action renderer forwards and the param dialog reads. { what: 'the navigation alias spelling is declared', rejected: false, code: `{ to: '/records/1', external: false, newTab: true, replace: false }` }, { what: 'newTab is the boolean switch, not the openIn string', rejected: true, code: `{ newTab: 'new-tab' }` }, - { what: 'description accepts the human string the param dialog titles itself with', rejected: false, code: `{ description: 'Creates an environment' }` }, - { what: 'description rejects a number', rejected: true, code: `{ description: 42 }` }, + // + // `description` is deliberately NOT pinned here — it is pinned in + // `actionDef-closed-surface.test.ts` instead, and the reason is a real limit + // of this harness rather than an oversight. + // + // Every other derivation above resolves through `@objectstack/spec`, an + // ordinary installed package. `description` derives from + // `UIActionSchema['description']` in `@object-ui/types`, a WORKSPACE package + // whose types resolve through its built `dist/index.d.ts`. This harness + // builds its own program with a default compiler host and no `paths` (see + // `erroringLines`), so it reads whatever is on disk — and the unit-test job + // does not build workspace packages first. When `dist` is stale or absent the + // indexed access degrades to `any`, `{ description: 42 }` is accepted, and a + // `rejected: true` row here goes red for a reason that has nothing to do with + // `ActionDef`. Measured: green locally after a build, red in CI without one. + // + // `tsconfig.typetests.json` has no such problem — it is a real project in the + // dependency graph, so CI's Type Check job builds `@object-ui/types` before + // compiling it, and the `@ts-expect-error` on `{ description: 42 }` there is + // enforced for real. Splitting the two `description` pins out is therefore + // the honest placement, not a weakening: an assertion whose colour depends on + // whether someone ran a build is not a pin. // Step 3's completion check, from the type side, INVERTED by step 3 landing. // While `[key: string]: any` stood, a typo was absorbed silently and this case