diff --git a/.changeset/console-action-dispatch-envelope-5611.md b/.changeset/console-action-dispatch-envelope-5611.md new file mode 100644 index 000000000..a7e8555f2 --- /dev/null +++ b/.changeset/console-action-dispatch-envelope-5611.md @@ -0,0 +1,34 @@ +--- +--- + +Internal: the console's admin-override notice now travels on a declared dispatch +type instead of a cast (objectui#5611). No published API and no runtime +behaviour changes, so this changeset declares "no release" rather than a bump — +`@object-ui/app-shell` source moved, nothing it exports did. + +`overrideNotice` is the safety copy shown once, ahead of a privileged admin +override that finalises an approval step over approvers who have not acted. Its +producer (`DeclaredActionsBar`) reached its reader (`useConsoleActionRuntime`'s +param-collection dialog) through a `dispatch as ActionDef` cast on one side and +`action?: any` on the other, so nothing declared the key anywhere and the two +could drift apart in silence — rename it on either side and the notice stops +appearing with every test still green, because each side's suite spells the key +itself. + +Both ends now share one declaration: `ConsoleActionDispatch` +(`ActionDef & { overrideNotice?: string }`), a HOST-composed envelope that lives +at the seam, in the one package where producer and reader both live. The cast is +gone and both param-collection handlers narrow off `any` — which is what puts +those functions under the compiler at all. + +The published `ActionDef` deliberately does NOT declare the key (maintainer +ruling 2026-08-22): it is the authored-metadata mirror, and `overrideNotice` is +the first key no author supplies, so declaring it there would make an unenforced +key legally writable in metadata. That prohibition still holds exactly as +written — `ActionDef` and `ACTION_DEF_KEYS` are unchanged. + +`@object-ui/core` is NOT untouched, and the reason is a separate declaration: +the same ruling's item 4 adds `HOST_DISPATCH_ACTION_KEYS` to the key inventory +so the dev-mode warning stops calling the host-composed key unknown. That change +carries its own changeset (`host-dispatch-action-keys-5611.md`) and its own +patch bump; this one remains the app-shell half, which publishes nothing. diff --git a/.changeset/host-dispatch-action-keys-5611.md b/.changeset/host-dispatch-action-keys-5611.md new file mode 100644 index 000000000..1f43f6e85 --- /dev/null +++ b/.changeset/host-dispatch-action-keys-5611.md @@ -0,0 +1,32 @@ +--- +'@object-ui/core': patch +--- + +The dev-mode unknown-key warning stops flagging `overrideNotice`, the console's +privileged-override safety copy (objectui#5611). + +`ActionRunner.execute` classifies the object it was HANDED, and a console host +hands it a DISPATCH, not a stored metadata row. `DeclaredActionsBar` composes +`overrideNotice` on that dispatch and two param-collection handlers read it — +yet the key inventory only mirrored AUTHORED surfaces, so the runner reported a +key two files read as one "no reader recognizes", and prescribed promoting it to +an explicit field on `ActionDef`. That prescription is the one shape the +2026-08-22 maintainer ruling forbids for this key, so acting on the diagnostic +walked an author into a rejected design. A false warning on the product's own +privileged path — the branch that finalises an approval over approvers who have +not acted — is how a dev console gets muted. + +Adds an exported `HOST_DISPATCH_ACTION_KEYS` (sole member `overrideNotice`) to +`actions/actionKeys.ts` and unions it into `KNOWN_ACTION_KEYS`, which is the +fourth input to that set and the first one that is not an authored-surface +mirror. Measured before and after on the exact dispatch the bar composes: the +warning went from one call naming `overrideNotice` to none, `KNOWN_ACTION_KEYS` +grew by exactly one member, and an action carrying a real typo alongside it +still warns — naming `targt` only. + +The authored surface does not move. `overrideNotice` is still NOT declared on +`ActionDef` and still NOT in `ACTION_DEF_KEYS`; writing it in an action literal +remains a compile error, and the AST-derived pin over the interface is unchanged. +Membership in `KNOWN_ACTION_KEYS` widens what the WARNING tolerates, never what +an author may write — `actionKeys.pin.test.ts` now pins both halves, including +the new list's exact contents so a second member cannot arrive quietly. diff --git a/packages/app-shell/src/__tests__/consoleActionDispatch.pin.test.ts b/packages/app-shell/src/__tests__/consoleActionDispatch.pin.test.ts new file mode 100644 index 000000000..dfb4f038c --- /dev/null +++ b/packages/app-shell/src/__tests__/consoleActionDispatch.pin.test.ts @@ -0,0 +1,256 @@ +/** + * 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. + */ + +/** + * The dispatch seam's contract, pinned in BOTH directions (objectui#5611). + * + * ## What can go wrong here, and why a runtime test cannot see it + * + * `overrideNotice` is the safety copy shown once, in front of a privileged + * admin override that finalises an approval step over approvers who have not + * acted. Its producer (`views/DeclaredActionsBar`) and its reader + * (`hooks/useConsoleActionRuntime`) are two files, and until this card the key + * crossed between them through a `dispatch as ActionDef` cast on one side and + * `action?: any` on the other — nothing declared it anywhere. Rename it on + * either side and the notice simply stops appearing: no test goes red, because + * each side's suite SPELLS THE KEY ITSELF (the producer suite reads + * `dispatch.overrideNotice` off its own spy; the reader suite hands in a + * literal), so neither can observe the other drifting. + * + * What closes that hole is a single declaration both sides import, which makes + * a one-sided rename a COMPILE error. So the gauge has to be the compiler, and + * this file drives `tsc` itself — same harness as + * `packages/core/src/actions/__tests__/actionKeys.types.test.ts`, for the same + * reason (the property under test is which assignments the compiler refuses, + * and that is erased before any assertion could run). + * + * ## The second direction, which is a maintainer ruling + * + * Maintainer ruling 2026-08-22 (narrow B) — the published `ActionDef` stays + * CLOSED and `overrideNotice` is carried at the seam instead: + * + * > the 17 undeclared-in-spec keys already on `ActionDef` are author-writable, + * > runtime-honoured runner mechanics; `overrideNotice` is the first key that + * > is NOT author-supplied at all — declaring it on the authored-metadata + * > mirror would let an author (human or AI) legally write a key whose + * > enforcement on that path is unmeasured, i.e. a declared-but-unenforced + * > surface, which is the platform's red line. + * + * So every case below is compiled TWICE — once against `ConsoleActionDispatch` + * and once against `ActionDef` — and both columns are asserted. The delta + * between them is required to be exactly `overrideNotice`. That makes the file + * revert-proof in both directions: drop the key from the envelope and the + * envelope column flips; declare it on `ActionDef` (the shape that was + * implemented and rejected) and the `ActionDef` column flips. + * + * ## Resolution guard + * + * The harness resolves `@object-ui/core` through the repo's SOURCE `paths`, not + * through `dist`. That is deliberate: with default resolution an unbuilt `dist` + * makes `ActionDef` degrade to `any`, every "rejected" row turns accepted, and + * the pin inverts for a reason that has nothing to do with the seam — the + * failure mode `actionKeys.types.test.ts` documents from measurement. On top of + * that, any diagnostic landing in the virtual module's IMPORT HEADER throws + * loudly here, so a resolution failure can never be read as a verdict about + * `overrideNotice`. + * + * Cost note (AGENTS.md 测试纪律): both programs are built at MODULE SCOPE, so + * the compiler work lands in the import phase, which no test or hook timeout + * bounds. A `beforeAll` would put it under the narrower 10s `hookTimeout`. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { fileURLToPath } from 'node:url'; +import { dirname, join, resolve } from 'node:path'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(HERE, '..', '..', '..', '..'); +/** The envelope under test, by source path — no `dist`, no barrel. */ +const ENVELOPE_IMPORT = join(HERE, '..', 'consoleActionDispatch').replace(/\\/g, '/'); + +/** + * One assignment or read, and whether `tsc` must refuse it against each of the + * two types. `envelope` is `ConsoleActionDispatch`; `actionDef` is the + * published authored-metadata mirror. + */ +interface Case { + readonly what: string; + /** Emits EXACTLY one line — the line index is how a diagnostic is attributed. */ + readonly code: (T: string) => string; + readonly envelope: boolean; + readonly actionDef: boolean; +} + +const CASES: readonly Case[] = [ + // ── The delta. These four rows ARE the ruling. ──────────────────────────── + { + what: 'a host may COMPOSE `overrideNotice` on the dispatch — and only there', + code: (T) => `const c: ${T} = { name: 'approval_reject', overrideNotice: 'n' };`, + envelope: false, + actionDef: true, + }, + { + what: 'a reader may READ `overrideNotice` off the dispatch — and only there', + code: (T) => `const c: string | undefined = (undefined as unknown as ${T}).overrideNotice;`, + envelope: false, + actionDef: true, + }, + // ── The envelope did not re-open the key set it extends ─────────────────── + { + what: 'a typo in the notice key is refused by the envelope too', + code: (T) => `const c: ${T} = { name: 'approval_reject', overrideNotcie: 'n' };`, + envelope: true, + actionDef: true, + }, + { + what: 'a non-string notice is refused — the reader concatenates it verbatim', + code: (T) => `const c: ${T} = { name: 'approval_reject', overrideNotice: 42 };`, + envelope: true, + actionDef: true, + }, + // ── Controls. Not about `overrideNotice`; they prove the two programs are + // really resolving the two real types rather than degrading to `any`. ─── + { + what: 'CONTROL an ordinary declared action compiles against both', + code: (T) => `const c: ${T} = { name: 'approval_reject', label: 'Reject' };`, + envelope: false, + actionDef: false, + }, + { + what: 'CONTROL an unrelated typo stays refused by both (step 3 closed the surface)', + code: (T) => `const c: ${T} = { targt: '/api/v1/x' };`, + envelope: true, + actionDef: true, + }, + { + what: 'CONTROL the dispatch IS an ActionDef, so `execute(dispatch)` needs no cast', + code: (T) => `const c: ConsoleActionDispatch extends ActionDef ? ${T} : never = { name: 'x' };`, + envelope: false, + actionDef: false, + }, +]; + +const IMPORTS = [ + `import type { ActionDef } from '@object-ui/core';`, + `import type { ConsoleActionDispatch } from '${ENVELOPE_IMPORT}';`, + // Keeps both imports "used" so `noUnusedLocals` (were it ever on) and the + // reader of this virtual file both see why they are here. + `type _Used = [ActionDef, ConsoleActionDispatch];`, +].join('\n'); + +/** + * Compile every case as one line against `typeName`, and return the set of case + * indices that produced a diagnostic. + * + * `paths` mirrors the repo root `tsconfig.json`, so `@object-ui/core` resolves + * to `packages/core/src` exactly as the workspace itself resolves it. See the + * file header for why default (`dist`-backed) resolution is not acceptable here. + */ +function erroringCases(typeName: string): Set { + const header = `${IMPORTS}\n`; + // Each case is wrapped in its own BLOCK so seven `const c` declarations do + // not collide — a duplicate-identifier diagnostic would land on every line + // and read as "the compiler refuses everything", which is the one wrong + // answer this file must never produce. Still exactly one line per case. + const body = CASES.map((c) => `{ ${c.code(typeName)} }`).join('\n'); + const source = `${header}${body}\n`; + const headerLines = header.split('\n').length - 1; + + const VIRTUAL = join(HERE, '__consoleActionDispatchPins.virtual.ts').replace(/\\/g, '/'); + const options: ts.CompilerOptions = { + strict: true, + skipLibCheck: true, + noEmit: true, + moduleResolution: ts.ModuleResolutionKind.Bundler, + module: ts.ModuleKind.ESNext, + target: ts.ScriptTarget.ESNext, + baseUrl: REPO_ROOT, + paths: { + '@object-ui/types': ['packages/types/src'], + '@object-ui/types/*': ['packages/types/src/*'], + '@object-ui/core': ['packages/core/src'], + '@object-ui/core/*': ['packages/core/src/*'], + }, + }; + const host = ts.createCompilerHost(options); + const getSourceFile = host.getSourceFile.bind(host); + host.getSourceFile = (fileName, languageVersion, ...rest) => + fileName === VIRTUAL + ? ts.createSourceFile(fileName, source, languageVersion, true) + : getSourceFile(fileName, languageVersion, ...rest); + const fileExists = host.fileExists.bind(host); + host.fileExists = (fileName) => (fileName === VIRTUAL ? true : fileExists(fileName)); + const readFile = host.readFile.bind(host); + host.readFile = (fileName) => (fileName === VIRTUAL ? source : readFile(fileName)); + + const program = ts.createProgram([VIRTUAL], options, host); + const sf = program.getSourceFile(VIRTUAL); + if (!sf) throw new Error('virtual source file was not added to the program'); + + const cases = new Set(); + for (const d of [...program.getSemanticDiagnostics(sf), ...program.getSyntacticDiagnostics(sf)]) { + if (d.start == null) continue; + const index = sf.getLineAndCharacterOfPosition(d.start).line - headerLines; + // A diagnostic ABOVE the first case line is a broken import, not a verdict. + // Fail loudly rather than let it read as "the compiler accepted everything". + if (index < 0) { + throw new Error( + `[${typeName}] the pin harness failed to resolve its own imports — this is a setup ` + + `failure, not a verdict about the seam: ${ts.flattenDiagnosticMessageText(d.messageText, ' ')}`, + ); + } + cases.add(index); + } + return cases; +} + +// Module scope on purpose — see the file header. +const againstEnvelope = erroringCases('ConsoleActionDispatch'); +const againstActionDef = erroringCases('ActionDef'); + +describe('the dispatch envelope carries `overrideNotice` (objectui#5611)', () => { + for (const [i, c] of CASES.entries()) { + it(c.what, () => { + expect({ case: c.what, refused: againstEnvelope.has(i) }) + .toEqual({ case: c.what, refused: c.envelope }); + }); + } +}); + +describe('the published `ActionDef` stays closed (maintainer ruling 2026-08-22)', () => { + for (const [i, c] of CASES.entries()) { + it(c.what, () => { + expect({ case: c.what, refused: againstActionDef.has(i) }) + .toEqual({ case: c.what, refused: c.actionDef }); + }); + } +}); + +describe('discrimination: the delta between the two types is exactly `overrideNotice`', () => { + it('every case the envelope accepts and ActionDef refuses is an overrideNotice case', () => { + const delta = CASES + .map((c, i) => ({ what: c.what, i })) + .filter(({ i }) => !againstEnvelope.has(i) && againstActionDef.has(i)) + .map(({ what }) => what); + expect(delta).toEqual([ + 'a host may COMPOSE `overrideNotice` on the dispatch — and only there', + 'a reader may READ `overrideNotice` off the dispatch — and only there', + ]); + }); + + it('the envelope never accepts something ActionDef would accept but should not', () => { + // The reverse delta must be empty: widening the envelope beyond the one + // declared extra key would show up here rather than in a prose review. + const reverse = CASES + .map((c, i) => ({ what: c.what, i })) + .filter(({ i }) => againstEnvelope.has(i) && !againstActionDef.has(i)) + .map(({ what }) => what); + expect(reverse).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/consoleActionDispatch.ts b/packages/app-shell/src/consoleActionDispatch.ts new file mode 100644 index 000000000..ecfe35eae --- /dev/null +++ b/packages/app-shell/src/consoleActionDispatch.ts @@ -0,0 +1,99 @@ +/** + * 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. + */ + +/** + * The console's action DISPATCH contract — what a host in this package hands + * to the action runtime, as opposed to what an author may write in metadata. + * + * ## Why this type exists at all + * + * `ActionDef` (`@object-ui/core`) is the AUTHORED-METADATA mirror: it describes + * what a metadata document may declare on an action, and objectstack#4075 + * step 3 closed it (the `[key: string]: any` is gone, so `tsc` rejects an + * unknown key at the construction site). Every key on it is therefore a key an + * author may legally write. + * + * A host, however, composes keys of its own at dispatch time — chrome the + * runtime reads once and nothing ever stores. `overrideNotice` is the first of + * those. Declaring it on `ActionDef` was implemented and REJECTED (maintainer + * ruling 2026-08-22, reaffirming the 2026-08-22 morning Option-B ruling with + * its shape made precise): + * + * > the 17 undeclared-in-spec keys already on `ActionDef` are author-writable, + * > runtime-honoured runner mechanics; `overrideNotice` is the first key that + * > is NOT author-supplied at all — declaring it on the authored-metadata + * > mirror would let an author (human or AI) legally write a key whose + * > enforcement on that path is unmeasured, i.e. a declared-but-unenforced + * > surface, which is the platform's red line. + * + * So the key is declared HERE instead: at the seam, in the one package where + * both its producer and its reader live. The authored surface stays exactly as + * strict as it was — writing `overrideNotice` in an `ActionDef` literal is + * still a compile error, and `@object-ui/core`'s published `.d.ts` does not + * move — while the dispatch that really does carry the key finally has a + * declaration to carry it on. + * + * ## What it replaces + * + * A `dispatch as ActionDef` cast in `DeclaredActionsBar`, with `action?: any` + * on both param-collection handlers at the other end. Between those two the key + * crossed the entire seam with nothing declaring it, so producer and reader + * could disagree in silence: rename it on either side and the notice simply + * stops appearing, with every existing test still green (each side's suite + * spells the key itself, so neither can see the other drift). The safety copy + * this carries is shown ONCE, in front of a privileged admin override that + * finalises an approval step over approvers who have not acted — a string that + * must not be able to vanish quietly. + * + * ## Producer / reader + * + * - producer — `views/DeclaredActionsBar.tsx`, which sets `overrideNotice` on + * the privileged-override branch (`can_act:false && can_override:true`), + * naming the approvers about to be bypassed. + * - readers — `hooks/useConsoleActionRuntime.tsx` and + * `views/RecordDetailView.tsx`, the two param-collection handlers the console + * mounts. The first renders the notice ahead of the declared description in + * the param dialog's subtitle; the second is the same seam and takes the same + * envelope, so the two handlers cannot drift apart from each other either. + * + * Deliberately NOT re-exported from this package's barrel (`src/index.ts`): + * it is the contract BETWEEN two modules of this package, not a type any host + * outside it composes. Keeping it off `dist/index.d.ts` means this card adds no + * published surface anywhere. + */ + +import type { ActionDef } from '@object-ui/core'; + +/** + * An action as DISPATCHED by a console host: everything an author may declare, + * plus the host-composed chrome the runtime reads on the way to the dialog. + * + * Anything added here must satisfy all three of: composed by a host in code, + * never read back out of stored metadata, and read by the console runtime. A + * key an AUTHOR is meant to write belongs on `ActionDef` (or, when the spec + * owns it, in `@objectstack/spec` first) — not here. + */ +export type ConsoleActionDispatch = ActionDef & { + /** + * A notice that must reach the user AHEAD of the declared description, shown + * at the top of the param-collection dialog's subtitle. + * + * Composed by `DeclaredActionsBar` for the privileged admin-override branch + * and arriving ALREADY LOCALIZED (bar chrome, resolved through the normal + * locale bundle), so its reader concatenates it verbatim. + * + * Deliberately NOT folded into `description` (objectui#5178): the reader + * resolves `description` through `_actions..description` and PREFERS a + * bundle hit over the passed literal, and `plugin-approvals` ships exactly + * such an entry for `approval_reject` — so a warning routed through + * `description` would be silently replaced by the ordinary "Reject this + * request?" copy in every locale that has the bundle. A safety notice a + * translation can delete is not a safety notice. + */ + overrideNotice?: string; +}; diff --git a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx index 6aeff8f5f..e7b19d5e6 100644 --- a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx +++ b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx @@ -52,6 +52,7 @@ import { resolvePageVarTokens } from '../utils/resolvePageVarTokens.js'; import { interpretFlowResponse } from '../utils/flowResponse.js'; import { createConsoleServerActionHandler } from '../utils/consoleServerAction.js'; import { modalTargetRefusalMessage } from '../utils/modalTargetDiagnostics.js'; +import type { ConsoleActionDispatch } from '../consoleActionDispatch.js'; const FALLBACK_USER = { id: 'current-user', name: 'Demo User', isPlatformAdmin: false }; @@ -193,7 +194,14 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons }); }, []); - const paramCollectionHandler = useCallback((params: ActionParamDef[], action?: any) => { + // `ConsoleActionDispatch`, not bare `ActionDef` (objectui#5611): this handler + // reads `overrideNotice`, which is host-composed dispatch chrome rather than + // authorable metadata, so it is declared at the seam instead of on the + // published authored-metadata mirror. Narrowing off `any` is what puts this + // whole function under the compiler — every other read below is a declared + // `ActionDef` field, and the one that was not is the reason objectui#4282 + // backed the narrowing out. + const paramCollectionHandler = useCallback((params: ActionParamDef[], action?: ConsoleActionDispatch) => { return new Promise | null>((resolve) => { // List_item actions stash the row record under params._rowRecord (see // ObjectGrid → onRowAction). Pull it out so resolveActionParams can diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index e1ad3cfab..e0e46281d 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -41,6 +41,7 @@ import { useActionTextLocalizer, } from '@object-ui/react'; import type { ActionDef } from '@object-ui/core'; +import type { ConsoleActionDispatch } from '../consoleActionDispatch.js'; import { useObjectTranslation } from '@object-ui/i18n'; import { Loader2, ShieldAlert } from 'lucide-react'; import { useConsoleActionRuntime } from '../hooks/useConsoleActionRuntime.js'; @@ -275,7 +276,15 @@ const DeclaredActionButton: React.FC<{ const outputParams = decision ? decisionOutputParams(decisionOutputDefs(recordData), t, { decision }) : []; - const dispatch: any = { + // Typed as the SEAM's contract, not as `any` and not cast to `ActionDef` + // on the way out (objectui#5611). `overrideNotice` below is host-composed + // chrome, not authorable metadata, so it is declared on + // `ConsoleActionDispatch` rather than on the authored-metadata mirror — + // maintainer ruling 2026-08-22, reasoning at the type's own docblock. + // What the annotation buys: the key now passes through ONE declaration + // that its reader also imports, so a rename on either side is a compile + // error instead of a notice that silently stops appearing. + const dispatch: ConsoleActionDispatch = { // Localized copies ride the dispatch: the runner reads `label` for the // param-dialog title, `confirmText` for the confirm prompt and // `successMessage` for the toast. A nameless action has no translation @@ -309,7 +318,7 @@ const DeclaredActionButton: React.FC<{ if (staticParams.length > 0 || outputParams.length > 0) { dispatch.actionParams = [...staticParams, ...outputParams]; } - await execute(dispatch as ActionDef); + await execute(dispatch); } finally { setLoading(false); } diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 18a21294f..b92cc7253 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -43,6 +43,7 @@ import { useRecordBreadcrumbTitle } from '../context/NavigationContext.js'; import { AUDIT_FIELD_NAMES, HIDDEN_SYSTEM_FIELD_NAMES } from './record-detail-system-fields.js'; import type { FeedItem } from '@object-ui/types'; import type { ActionDef, ActionParamDef } from '@object-ui/core'; +import type { ConsoleActionDispatch } from '../consoleActionDispatch.js'; import { useRecordApprovals, recordLockedByApproval } from '../hooks/useRecordApprovals.js'; import { RecordAttachmentsPanel } from './RecordAttachmentsPanel.js'; import { RecordApprovalsPanel } from './RecordApprovalsPanel.js'; @@ -497,7 +498,12 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri }); }, []); - const paramCollectionHandler = useCallback((params: ActionParamDef[], action?: any) => { + // The SECOND param-collection handler the console mounts, narrowed to the + // same seam contract as `useConsoleActionRuntime`'s (objectui#5611). It does + // not read `overrideNotice` itself — it takes the envelope so the two + // handlers on this seam cannot drift apart from each other, and so a reader + // added here later has the key declared rather than reaching for a cast. + const paramCollectionHandler = useCallback((params: ActionParamDef[], action?: ConsoleActionDispatch) => { return new Promise | null>((resolve) => { // Related-list row actions retarget a CHILD object (e.g. sys_member rows // on an org record page) and stash the clicked row under diff --git a/packages/core/src/actions/__tests__/actionKeys.pin.test.ts b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts index 97f5ba77b..060909cc0 100644 --- a/packages/core/src/actions/__tests__/actionKeys.pin.test.ts +++ b/packages/core/src/actions/__tests__/actionKeys.pin.test.ts @@ -24,6 +24,7 @@ import { ACTION_DEF_KEYS, SPEC_ACTION_KEYS, NAVIGATION_ALIAS_KEYS, + HOST_DISPATCH_ACTION_KEYS, RETIRED_ACTION_KEYS, KNOWN_ACTION_KEYS, classifyActionKeys, @@ -136,6 +137,38 @@ describe('action key inventory (objectstack#4075 step 1)', () => { expect(KNOWN_ACTION_KEYS.has('execute')).toBe(false); }); + it('pins the host-dispatch list EXACTLY, because its set is the ruling', () => { + // Exact contents, not `toContain`. The maintainer ruling of 2026-08-22 + // authorized ONE key here, and the danger this list carries is growth: a + // key in it is a key the unknown-key warning stops asking about, so a + // silent addition is a silent hole in the diagnostic. `toEqual` on the whole + // array is what makes adding a second member a red test that names it, + // rather than an edit nobody reads. + expect([...HOST_DISPATCH_ACTION_KEYS]).toEqual(['overrideNotice']); + }); + + it('counts the host-dispatch key as known, so the dev warning stops crying wolf', () => { + // The defect item 4 closes: `DeclaredActionsBar` composes `overrideNotice` + // on the dispatch, `useConsoleActionRuntime` and `RecordDetailView` read + // it, and the runner — which classifies the DISPATCH, not the stored row — + // called it a key "no reader recognizes". + expect(KNOWN_ACTION_KEYS.has('overrideNotice')).toBe(true); + }); + + it('does NOT let the host-dispatch key onto the authored surface', () => { + // The other half of the same ruling, and the half that has to stay true + // while the half above changes: ⛔ not declared on `ActionDef`, ⛔ not in + // `ACTION_DEF_KEYS`. Membership in `KNOWN_ACTION_KEYS` widens what the + // dev-mode WARNING tolerates; it must not widen what an author may WRITE. + // Read off the interface's AST, so re-declaring the key on `ActionDef` to + // "make it consistent" fails here by name. + expect({ + declaredOnActionDef: declaredActionDefKeys().includes('overrideNotice'), + inActionDefKeys: (ACTION_DEF_KEYS as readonly string[]).includes('overrideNotice'), + inSpecActionKeys: (SPEC_ACTION_KEYS as readonly string[]).includes('overrideNotice'), + }).toEqual({ declaredOnActionDef: false, inActionDefKeys: false, inSpecActionKeys: 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 @@ -187,6 +220,38 @@ describe('unknown-key warning', () => { expect(warn.mock.calls[1][0]).toContain('"remove"'); }); + it('says nothing about the dispatch a console host actually composes', () => { + // The exact object literal at `DeclaredActionsBar.tsx`'s override branch, + // reduced to the keys that decide the verdict. Before item 4 this produced + // one warning naming `overrideNotice`, on the one privileged path that + // finalises an approval over approvers who have not acted. + warnOnUnknownActionKeys({ + name: 'approval_reject', + type: 'api', + target: '/api/v1/approvals/{id}/reject', + label: 'Reject (override)', + objectName: 'approval_request', + params: { _rowRecord: { id: 'a1' } }, + overrideNotice: 'You are overriding 2 approvers who have not acted.', + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('still names a typo riding the SAME host dispatch', () => { + // The discrimination half: the fix must silence one key, not the check. + // Without this, replacing `classifyActionKeys` with `() => ({unknown: [], + // retired: []})` would pass the test above. + warnOnUnknownActionKeys({ + name: 'approval_reject', + type: 'api', + overrideNotice: 'You are overriding 2 approvers who have not acted.', + targt: '/api/v1/approvals/{id}/reject', + }); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('`targt`'); + expect(warn.mock.calls[0][0]).not.toContain('overrideNotice'); + }); + it('is silent in production', () => { const prev = process.env.NODE_ENV; process.env.NODE_ENV = 'production'; diff --git a/packages/core/src/actions/actionKeys.ts b/packages/core/src/actions/actionKeys.ts index ad7906dc0..accae6019 100644 --- a/packages/core/src/actions/actionKeys.ts +++ b/packages/core/src/actions/actionKeys.ts @@ -269,6 +269,55 @@ export const SPEC_ACTION_KEYS = [ */ export const NAVIGATION_ALIAS_KEYS = ['to', 'external', 'newTab', 'replace'] as const; +/** + * Keys a HOST composes on the DISPATCH OBJECT at dispatch time — chrome the + * runtime reads once on its way to the dialog, that no author ever writes and + * no store ever holds. + * + * Distinct from every other list in this module, and the distinction is the + * whole point: each of the three lists above restates one AUTHORED surface — + * what `ActionDef` itself declares, what the spec's `ActionSchema` declares, + * and the `navigation` alias's objectui dialect — so a key joining one of them + * is a key an author may legally write. This list claims no such derivation and + * must not acquire one: a key here is the opposite, and writing it in metadata + * is still a compile error that must stay one. Maintainer ruling + * 2026-08-22 fixes both halves of that: ⛔ the key is NOT declared on + * `ActionDef` and ⛔ NOT added to {@link ACTION_DEF_KEYS}; it is declared at the + * seam, on `packages/app-shell/src/consoleActionDispatch.ts`'s + * `ConsoleActionDispatch`, and appears HERE only so the dev-mode warning below + * stops calling it unknown. + * + * Why the warning has to know: `classifyActionKeys` sees the object the runner + * was handed, which is the DISPATCH, not the stored metadata row. With the key + * absent from the inventory, `warnOnUnknownActionKeys` told the author that a + * key TWO files read is one "no reader recognizes", and prescribed promoting it + * to `ActionDef` — the one shape the ruling forbids for it. A diagnostic that + * fires on the product's own privileged path is how a console gets muted. + * + * {@link HOST_STASHED_PARAM_KEYS} below is the same category one level down — + * host-composed keys inside `params` rather than on the action itself — and + * cites `DeclaredActionsBar` by name for the same reason this does. Two lists + * because the two warnings ask different questions of different objects, not + * because the categories differ. + * + * `overrideNotice` — the privileged-override safety copy (objectui#5178), + * naming the approvers about to be bypassed. + * - producer: `packages/app-shell/src/views/DeclaredActionsBar.tsx`, on the + * `can_act:false && can_override:true` branch. + * - readers: `packages/app-shell/src/hooks/useConsoleActionRuntime.tsx` and + * `packages/app-shell/src/views/RecordDetailView.tsx`, the console's two + * param-collection handlers, which render it ahead of the declared + * description in the dialog's subtitle. + * + * Anything added here must satisfy all three of: composed by a host in code, + * never read back out of stored metadata, and read by the runtime. A key an + * AUTHOR is meant to write belongs on `ActionDef` — or, when the spec owns it, + * in `@objectstack/spec` first — and not in this list. `actionKeys.pin.test.ts` + * pins the contents exactly, so growing the set is a deliberate, reviewed edit + * rather than a quiet one. + */ +export const HOST_DISPATCH_ACTION_KEYS = ['overrideNotice'] 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 @@ -293,11 +342,21 @@ export const RETIRED_ACTION_KEYS: Readonly> = { 'Run `os migrate meta --from 16` to rewrite it automatically.', }; -/** Every key an action may legitimately carry today. */ +/** + * Every key an action may legitimately carry today — authored or host-composed. + * + * Four inputs, three of them mirrors of an authored surface and the fourth + * ({@link HOST_DISPATCH_ACTION_KEYS}) the host-composed dispatch chrome. The + * union is what `classifyActionKeys` consults, and it is deliberately wider + * than the authored surface: the runner classifies the object it was HANDED, + * and a host hands it a dispatch. Membership here grants nothing to an author — + * `ActionDef` is what decides that, and it is closed. + */ export const KNOWN_ACTION_KEYS: ReadonlySet = new Set([ ...ACTION_DEF_KEYS, ...SPEC_ACTION_KEYS, ...NAVIGATION_ALIAS_KEYS, + ...HOST_DISPATCH_ACTION_KEYS, ].filter((key) => !(key in RETIRED_ACTION_KEYS))); /** Split an action's own keys into the two things worth saying out loud. */