From 94cfa453864ac22e357d655a04e50f6b815b1704 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 12:56:42 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(runtime,objectql,metadata)!:=20ADR-011?= =?UTF-8?q?0=20=E2=80=94=20action=20identity=20is=20`name`;=20refuse=20und?= =?UTF-8?q?eclared=20executables=20(#3935)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/v1/actions/:object/:action resolved the DECLARATION from the URL segment as a name but dispatched the HANDLER using that same segment as a registry key. For a target-bound action those differ, so the two documented callers each worked on exactly the half the other broke: the documented curl resolved the declaration then 404ed, while the Console's target-addressed call dispatched fine and resolved no declaration — silently skipping the ADR-0066 D4 capability gate and the ADR-0104 param contract. D1/D2 — identity is the declarative name on every surface; handler-key candidates are derived from the resolved declaration via a rotation now shared with MCP run_action (resolveActionHandlerKeys, executeRegisteredAction). REST previously rotated only the object key. D3 (BREAKING) — declaration resolution is a trichotomy. A genuinely undeclared handler is refused (404) naming the defineAction to add, rather than executed ungated with system privileges; an unreachable metadata plane is a 503 rather than a silent ungating, which MetadataManager.loadDiagnosed makes detectable by telling a clean miss from an outage. OS_ALLOW_UNDECLARED_ACTIONS=1 is the migration valve, warning per invocation, removed in 18. D5 — reconcileActionRegistrations + ObjectQLEngine.listRegisteredActions power a kernel:ready inventory listing registered-but-undeclared handlers and declared script actions bound to no handler: the ADR-0078 converse. D6 — security-gate strictness is opt-out (OS_ALLOW_*), never opt-in. Two superseded tests updated rather than worked around: both asserted that an undeclared action still runs, which is the behaviour D3 rules out. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01DTTKgYriDB6RVrkYjxxnG1 --- .../adr-0110-action-declaration-admission.md | 36 +++ content/docs/releases/v17.mdx | 38 +++ content/docs/ui/actions.mdx | 48 ++++ ...tion-identity-and-declaration-admission.md | 3 +- packages/metadata/src/metadata-manager.ts | 34 ++- packages/objectql/src/engine.ts | 25 ++ packages/runtime/src/action-execution.ts | 200 ++++++++++++++-- .../runtime/src/action-reconciliation.test.ts | 125 ++++++++++ packages/runtime/src/app-plugin.ts | 63 ++++- packages/runtime/src/domains/actions.ts | 94 ++++++-- ...http-dispatcher.actions-global-key.test.ts | 35 ++- ...atcher.actions-identity-addressing.test.ts | 226 ++++++++++++++++++ ...p-dispatcher.actions-type-dispatch.test.ts | 71 +++++- 13 files changed, 942 insertions(+), 56 deletions(-) create mode 100644 .changeset/adr-0110-action-declaration-admission.md create mode 100644 packages/runtime/src/action-reconciliation.test.ts create mode 100644 packages/runtime/src/http-dispatcher.actions-identity-addressing.test.ts diff --git a/.changeset/adr-0110-action-declaration-admission.md b/.changeset/adr-0110-action-declaration-admission.md new file mode 100644 index 0000000000..d68e623202 --- /dev/null +++ b/.changeset/adr-0110-action-declaration-admission.md @@ -0,0 +1,36 @@ +--- +'@objectstack/runtime': major +'@objectstack/objectql': minor +'@objectstack/metadata': minor +--- + +**ADR-0110 — an action's identity is its `name`, and anything executable over a +governed surface must have a declaration.** + +`POST /api/v1/actions/:object/:action` resolved the DECLARATION from the URL +segment as a `name` but dispatched the HANDLER using that same segment as a +registry key. For a target-bound action (`{ name: 'complete_task', target: +'completeTask' }`) those are different strings, so the two documented callers +each worked on exactly the half the other broke: the documented curl resolved +the declaration then 404ed, while the Console's `target`-addressed call +dispatched fine and resolved no declaration — silently skipping the ADR-0066 D4 +capability gate and the ADR-0104 param contract (#3935). + +- **D1/D2** — identity is always the declarative `name`; the handler key is + derived from the resolved declaration through a rotation now shared with the + MCP `run_action` bridge (`resolveActionHandlerKeys`, `executeRegisteredAction`). + The REST route previously rotated only the object key, never the handler key. +- **D3 (breaking)** — declaration resolution is a trichotomy. A genuinely + undeclared handler is **refused (404)** with the `defineAction` to add, rather + than executed ungated with system privileges; an unreachable metadata plane is + a **503** rather than a silent ungating (`MetadataManager.loadDiagnosed` tells + a clean miss from an outage). `OS_ALLOW_UNDECLARED_ACTIONS=1` is the migration + valve — it warns on every invocation and is removed in 18. +- **D5** — `reconcileActionRegistrations` plus `ObjectQLEngine.listRegisteredActions` + power a `kernel:ready` inventory logging every registered-but-undeclared + handler (refused at dispatch) and every declared script action bound to no + handler — the ADR-0078 converse, mechanised. +- **D6** — security-gate strictness is opt-**out** (`OS_ALLOW_*`), never opt-in. + +Apps whose actions are all declared need no changes beyond gaining enforcement +of the `requiredPermissions` they already declared. diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 87e690d803..9a1042a245 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -156,6 +156,44 @@ and flags whitelists the mapping would *widen* so every edit stays reviewable. `['upsert']`) strips to `[]`, which is **deny-all** — the object's API closes rather than widening. +### An action must be declared to be invocable, and is identified by `name` (ADR-0110, #3935) + +Two halves of the `/api/v1/actions/:object/:action` contract were broken in +complementary ways, which is why neither surfaced: the route resolved the +**declaration** from the URL segment as a `name`, but dispatched the +**handler** using that same segment as a registry key. For a target-bound +action (`{ name: 'complete_task', target: 'completeTask' }`) those differ — so +the documented `curl .../todo_task/complete_task` resolved the declaration and +then 404ed, while the Console's `target`-addressed call dispatched fine and +resolved **no** declaration, silently skipping the ADR-0066 D4 capability gate +and the ADR-0104 param contract. + +Three changes land together: + +- **Identity is `name`.** The URL, MCP `run_action`, and every future surface + identify an action by its declarative `name`. `target` is a binding + expression — polymorphic per type, `${param.X}`-interpolatable, and legally + non-unique — so it never identifies anything. The server derives the handler + key from the declaration it resolved, using the rotation the MCP bridge + already used. The Console posts `name` (objectui ships in lockstep). +- **An undeclared handler is refused.** `engine.registerAction` with no + matching declaration has no `requiredPermissions` to enforce and no param + contract to check, yet executes with system privileges. It now returns 404 + naming the `defineAction` to add. Deleting a declaration used to *remove* an + action's gate while leaving it callable; removal now narrows. +- **An unreachable metadata plane refuses instead of degrading.** A loader + failure used to be indistinguishable from "no declaration", so an outage + silently ungated every action it could not see. That is now a **503** — the + same posture as the datasource entry below. + +**Migration:** boot logs list every registered-but-undeclared handler under +`[action-governance]`, alongside declared script actions bound to no handler. +Declare each one. `OS_ALLOW_UNDECLARED_ACTIONS=1` runs them meanwhile, warning +on every invocation; **it is removed in 18**. Apps whose actions are all +declared — anything with working Console buttons — need no changes, other than +gaining enforcement of the `requiredPermissions` they already declared. Callers +that hard-coded a `target` in an action URL switch to the action's `name`. + ### A flow run with no trigger user may not touch data (#3760) An effective `runAs: 'user'` run that resolved **no trigger user** used to diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 6a12b4a3fe..a8ef54bdab 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -232,6 +232,13 @@ curl -b cookies.txt -X POST \ # → 200 { "success": true, "data": { "success": true, "data": ... } } ``` +The URL names the action by its **`name`**, never by `target`. `target` binds +the action to whatever runs it — a handler key here, a flow id for +`type: 'flow'`, a URL for `type: 'url'` — so it is an implementation detail: +the server resolves your declaration by name and derives the handler key from +it. Rename the underlying function freely; as long as the declaration's +`target` follows, the public URL is unchanged. + Failures split by **whether a handler ran**: - **It ran and rejected** — HTTP **200**, `data: { "success": false, "error", @@ -255,6 +262,47 @@ The endpoint dispatches on the **declared `type`**, exactly like the MCP | `api` | **400** — it dispatches on `target`; call that endpoint directly. | | `url` / `modal` / `form` | **400** — client-side navigation; there is nothing for the server to run. | +## Headless actions: declare it, then hide it + +An action that should be callable but not appear in the UI is still a +**declared** action. Hiding is a property you set; it is not the absence of a +declaration: + +```typescript +defineAction({ + name: 'recalculate_commissions', + type: 'script', + target: 'recalcCommissions', + locations: [], // no UI surface + requiredPermissions: ['finance.admin'], // still gated + // `ai.exposed` is false by default — no MCP tool either +}) +``` + +You keep the capability gate, the param contract, the audit trail, and Setup +visibility for admins. A declaration that no surface renders costs you nothing. + + +Registering a handler **without** a declaration is not a way to hide an action — +it is refused. An undeclared handler has no `requiredPermissions` to enforce and +no param contract to check, yet it would execute with system privileges, so the +server declines it: + +``` +Action 'recalc' on 'crm_account' has no declaration — add +`defineAction({ name: 'recalc', … })`, or register the handler under a declared +action's `target`. +``` + +If server-side logic should never be reachable over HTTP at all, do not register +it as an action — export a plain function and call it from your own code. +`engine.registerAction` means "publish this on the HTTP and MCP surfaces". + +Migrating an app that has undeclared handlers? Boot logs list every one of them +under `[action-governance]`, and `OS_ALLOW_UNDECLARED_ACTIONS=1` runs them +(warning each time) while you add the declarations. That valve is removed in 18. + + ## Expose it to AI (MCP) Actions are **not** AI-visible by default. Opting in takes two fields — and diff --git a/docs/adr/0110-action-identity-and-declaration-admission.md b/docs/adr/0110-action-identity-and-declaration-admission.md index 1abcd6aa9c..26bb9350d5 100644 --- a/docs/adr/0110-action-identity-and-declaration-admission.md +++ b/docs/adr/0110-action-identity-and-declaration-admission.md @@ -1,6 +1,7 @@ # ADR-0110: An action's identity is its `name`; anything executable over a governed surface must have a declaration (the declaration-admission gate) -**Status**: Proposed (2026-07-29) — **targeted at protocol 17** (`@objectstack/spec@17.0.0-rc.0`, in RC now). The fail-closed inversion lands in that major rather than staging across two, joining its two siblings already in the v17 breaking set: *"A flow run with no trigger user may not touch data"* (#3760 — missing identity fails closed) and *"A datasource that cannot connect fails the boot"* (#3741/#3758/#3826 — an unreachable dependency refuses rather than degrades). +**Status**: **Accepted — implemented** (2026-07-29) in protocol 17 (`@objectstack/spec@17.0.0-rc.0`). The fail-closed inversion landed in that major rather than staging across two, joining its two siblings in the v17 breaking set: *"A flow run with no trigger user may not touch data"* (#3760 — missing identity fails closed) and *"A datasource that cannot connect fails the boot"* (#3741/#3758/#3826 — an unreachable dependency refuses rather than degrades). +Evidence: D1/D2 — `action-execution.ts` (`resolveActionHandlerKeys`, `executeRegisteredAction`) shared by `domains/actions.ts` and `invokeBusinessAction`, pinned by `http-dispatcher.actions-identity-addressing.test.ts` (the documented curl now gates AND dispatches). D3 — the trichotomy in `domains/actions.ts`, reading the `degraded` signal `MetadataManager.loadDiagnosed` supplies (`metadata-manager.ts`). D5 — `reconcileActionRegistrations` + `ObjectQLEngine.listRegisteredActions`, wired to the `kernel:ready` inventory in `app-plugin.ts`, covered by `action-reconciliation.test.ts`. D1 client — `../objectui` `useConsoleActionRuntime.tsx`. D4 + migration — `content/docs/ui/actions.mdx`, `content/docs/releases/v17.mdx`. **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove gate; a security property that parses but does nothing is worse than absent), [ADR-0066](./0066-unified-authorization-model.md) (D4 — action `requiredPermissions`, dual-surface, server is source of truth), [ADR-0078](./0078-no-silently-inert-metadata.md) (no silently inert metadata — this ADR is its **converse**: no silently ungoverned executable), [ADR-0096](./0096-execution-surface-identity-admission.md) (missing **identity** must not fail open; this ADR extends the same posture to missing **declaration**), [ADR-0104](./0104-field-runtime-value-shape-contract.md) (D2 — declared action param contract), [ADR-0109](./0109-ai-tool-authoring-model.md) (every declarative action materialises `action_`; the declaration IS the AI-facing capability) **Consumers**: `@objectstack/runtime` (`domains/actions.ts`, `action-execution.ts`), `@objectstack/objectql` (`registerAction` contract), `@objectstack/lint` (new reconciliation rule), `../objectui` (`useConsoleActionRuntime` invocation URL), `content/docs/ui/actions.mdx` (public REST contract) diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index 54a81354f0..3ec9ae9564 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -1395,23 +1395,53 @@ export class MetadataManager implements IMetadataService { /** * Load a single metadata item from loaders. * Iterates through registered loaders until found. + * + * Returns `null` both when no loader HAS the item and when every loader + * FAILED — see {@link loadDiagnosed} when the caller must tell those apart. */ async load( type: string, name: string, options?: MetadataLoadOptions ): Promise { + return (await this.loadDiagnosed(type, name, options)).data; + } + + /** + * `load`, plus whether the answer can be trusted as complete. + * + * [ADR-0110 D3] A miss and an outage are different facts with opposite + * security meanings, and plain `load` cannot express the difference: a + * loader that throws is warn-logged and skipped, so a database the metadata + * plane cannot reach returns the same `null` as a name that was never + * declared. Callers that gate on a declaration MUST NOT read that `null` as + * "the author declared no gate" — an availability failure would silently + * widen access (the REST `/actions` route's fail-open branch, #3935). + * + * `degraded` is true when at least one loader threw AND no loader answered + * with the item. The posture is deliberately conservative: with a loader + * down we cannot prove the item is absent, so we decline to claim it is. + * A clean miss (every loader answered, none had it) is NOT degraded. + */ + async loadDiagnosed( + type: string, + name: string, + options?: MetadataLoadOptions + ): Promise<{ data: T | null; degraded: boolean; errors: string[] }> { + const errors: string[] = []; for (const loader of this.loaders.values()) { try { const result = await loader.load(type, name, options); if (result.data) { - return result.data as T; + return { data: result.data as T, degraded: false, errors }; } } catch (e) { + const message = e instanceof Error ? e.message : String(e); + errors.push(`${loader.contract.name}: ${message}`); this.logger.warn(`Loader ${loader.contract.name} failed to load ${type}:${name}`, { error: e }); } } - return null; + return { data: null, degraded: errors.length > 0, errors }; } /** diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 5ec3fbe48b..f7a0544252 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -745,6 +745,31 @@ export class ObjectQL implements IDataEngine { return entry.handler(ctx); } + /** + * Every action handler currently registered, as `{ objectName, actionName }` + * pairs (plus the owning package when one was given). + * + * [ADR-0110 D5] The handler registry is one half of the + * declaration↔executable bijection; with no way to enumerate it, the other + * half could only be checked when someone happened to invoke a route. The + * boot reconciliation reads this to list handlers no declaration covers — + * which since D3 are refused at dispatch, so having the inventory is what + * makes that refusal a checklist instead of a support ticket. + */ + listRegisteredActions(): Array<{ objectName: string; actionName: string; package?: string }> { + const out: Array<{ objectName: string; actionName: string; package?: string }> = []; + for (const [key, entry] of this.actions.entries()) { + const sep = key.indexOf(':'); + if (sep < 0) continue; + out.push({ + objectName: key.slice(0, sep), + actionName: key.slice(sep + 1), + ...(entry.package ? { package: entry.package } : {}), + }); + } + return out; + } + /** * Remove all actions registered by a specific package. */ diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 4e51e7838e..2c3eaebe8a 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -29,6 +29,19 @@ function actionParamsStrict(): boolean { return typeof process !== 'undefined' && process.env?.OS_ACTION_PARAMS_STRICT_ENABLED === '1'; } +/** + * [ADR-0110 D3/D6] Migration valve for executing an action that has no + * declaration. Enforcement is the DEFAULT and this opts OUT of it — the + * direction matters: a security gate whose strictness is opt-in (as + * `OS_ACTION_PARAMS_STRICT_ENABLED` is, acceptably, for a DX contract) ships + * open for everyone who never read the release notes. `OS_ALLOW_*` is the + * sanctioned shape for a security escape hatch; it warns on every invocation + * and is slated for removal in 18. + */ +export function undeclaredActionsAllowed(deps: ActionExecutionDeps): boolean { + return typeof process !== 'undefined' && process.env?.OS_ALLOW_UNDECLARED_ACTIONS === '1'; +} + const _warnedActionParams = new Set(); function warnActionParamsOnce(key: string, message: string): void { if (_warnedActionParams.has(key)) return; @@ -813,25 +826,15 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps, executionContext: buildActionExecutionContext(ec), params: { ...params, recordId, objectName }, }; - // Handler key: body-based actions register under `name` (AppPlugin); - // target-bound script actions register under `target` (user code). - // Probe both, then the object-less keys (#3913), distinguishing the - // engine's "action not registered" miss from a genuine handler error. - const primary = action.body ? action.name : (action.target || action.name); - const candidates = [primary, action.target, action.name].filter( - (k: unknown, i: number, a: unknown[]): k is string => typeof k === 'string' && a.indexOf(k) === i, + // [ADR-0110 D2] Handler-key derivation + the probe rotation are shared with + // the REST `/actions` route — one addressing algorithm, not two. + const dispatch = await executeRegisteredAction( + deps, ql, objectName, resolveActionHandlerKeys(action), actionContext, ); - for (const obj of actionHandlerObjectKeys(objectName)) { - for (const key of candidates) { - try { - const result = await ql.executeAction(obj, key, actionContext); - return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result: result ?? null }; - } catch (err: any) { - if (!isActionNotRegisteredError(err)) throw err; // real handler failure → surface - } - } + if (!dispatch.dispatched) { + throw new Error(`No handler registered for action '${name}' on '${objectName}'`); } - throw new Error(`No handler registered for action '${name}' on '${objectName}'`); + return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result: dispatch.result ?? null }; } /** @@ -914,6 +917,72 @@ export async function collectActionDeclarations(deps: ActionExecutionDeps, return out; } +/** + * [ADR-0110 D5] Reconcile the two halves of the declaration↔executable + * bijection and report the orphans on both sides. + * + * ADR-0078 outlaws a declaration nothing executes (silently *inert*); D3 + * outlaws an executable nothing declares (silently *ungoverned*). Together + * they are one invariant — everything declared runs, everything that runs is + * declared — and this is the mechanism that makes a violation visible instead + * of waiting for someone to invoke the route. + * + * Two findings: + * - `undeclared_handler` — a registered key that reconciles to no + * declaration. Since D3 those are REFUSED at dispatch, so this list is the + * upgrade checklist: everything on it is an endpoint that stopped working + * and the exact `defineAction` that fixes it. + * - `unbound_declaration` — a declared `script` action with no `body` and no + * handler under any candidate key: a button wired to nothing. + * + * A handler reconciles when some declaration for its object (or an object-less + * one) yields it among {@link resolveActionHandlerKeys} — the SAME derivation + * dispatch uses, so the inventory cannot disagree with the router. + */ +export function reconcileActionRegistrations(deps: ActionExecutionDeps, + registered: Array<{ objectName: string; actionName: string; package?: string }>, + declarations: Array<{ action: any; objectName: string }>, +): { + undeclaredHandlers: Array<{ objectName: string; actionName: string; package?: string }>; + unboundDeclarations: Array<{ objectName: string; actionName: string }>; +} { + // Every key any declaration can address, per owning object key. + const addressable = new Map>(); + const addKey = (objectKey: string, handlerKey: string) => { + let set = addressable.get(objectKey); + if (!set) addressable.set(objectKey, (set = new Set())); + set.add(handlerKey); + }; + for (const { action, objectName } of declarations) { + for (const key of resolveActionHandlerKeys(action)) addKey(objectName, key); + } + + const covers = (objectName: string, actionName: string): boolean => { + if (addressable.get(objectName)?.has(actionName)) return true; + // A handler registered under an object-less key is addressable by any + // object-less declaration, mirroring `actionHandlerObjectKeys`. + if (!isObjectLessActionKey(objectName)) return false; + for (const [objectKey, keys] of addressable) { + if (isObjectLessActionKey(objectKey) && keys.has(actionName)) return true; + } + return false; + }; + + const undeclaredHandlers = registered.filter((r) => !covers(r.objectName, r.actionName)); + + const registeredKeys = new Set(registered.map((r) => `${r.objectName}:${r.actionName}`)); + const unboundDeclarations: Array<{ objectName: string; actionName: string }> = []; + for (const { action, objectName } of declarations) { + if ((action?.type ?? 'script') !== 'script') continue; // only script needs a handler + if (action?.body) continue; // its handler is synthesized + const bound = resolveActionHandlerKeys(action).some((key) => + actionHandlerObjectKeys(objectName).some((obj) => registeredKeys.has(`${obj}:${key}`))); + if (!bound) unboundDeclarations.push({ objectName, actionName: action?.name }); + } + + return { undeclaredHandlers, unboundDeclarations }; +} + /** * Owning object of a standalone `action` item — must stay in lockstep with * the ObjectQL plugin's `actionObjectKey` (the engine registration key), so @@ -968,6 +1037,69 @@ export function isActionNotRegisteredError(err: any): boolean { return /Action '.+' on object '.+' not found/i.test(String(err?.message ?? err)); } +/** + * [ADR-0110 D2] Handler-key candidates for an action, most-specific first — + * the *addressing* half of "resolve, then address". + * + * A registration key is NOT an action's identity. `app-plugin.ts` + * auto-registers **body** actions under `name`, while user code registers a + * **target-bound** script action under `target` + * (`engine.registerAction('todo_task', 'completeTask', …)`). Identity is + * always the declarative `name` (D1); which key the handler happens to live + * under is derived HERE, from the already-resolved declaration, so no caller + * ever has to know it. + * + * `fallbackKey` (the routed URL segment) is the last candidate and exists for + * the UNDECLARED case, where there is no declaration to derive anything from. + * It is deduped away whenever the declaration already yields it, so it never + * widens what a declared action can reach. + */ +export function resolveActionHandlerKeys(action: any, fallbackKey?: string): string[] { + const primary = action ? (action.body ? action.name : (action.target || action.name)) : undefined; + return [primary, action?.target, action?.name, fallbackKey].filter( + (k: unknown, i: number, a: unknown[]): k is string => + typeof k === 'string' && k.length > 0 && a.indexOf(k) === i, + ); +} + +/** + * [ADR-0110 D2] Run a script/body action through the engine's handler + * registry: rotate the derived key candidates across the object-key rotation + * (`actionHandlerObjectKeys`), telling an "unregistered key" miss apart from + * a genuine handler failure. + * + * Shared by the REST `/actions` route and the MCP `run_action` bridge so both + * surfaces address handlers identically. Before it was shared, REST rotated + * only the OBJECT and used the URL segment verbatim as the key — strictly + * weaker than MCP, and the reason the documented + * `POST /api/v1/actions/todo_task/complete_task` curl 404ed for every + * target-bound action while the Console's `target`-addressed call worked + * (and skipped the D4 gate on the way past). + * + * Reports a total miss as `{ dispatched: false }` rather than throwing, so + * neither surface has to pattern-match this function's own error message to + * tell "no handler anywhere" (a routing miss — 404) from "the handler ran and + * failed" (a business outcome, which propagates). Each surface words its own + * miss: REST 404s naming the routed object, MCP throws naming the action. + */ +export async function executeRegisteredAction(deps: ActionExecutionDeps, + ql: any, + objectName: string, + candidates: string[], + actionContext: any, +): Promise<{ dispatched: boolean; result?: any }> { + for (const obj of actionHandlerObjectKeys(objectName)) { + for (const key of candidates) { + try { + return { dispatched: true, result: await ql.executeAction(obj, key, actionContext) }; + } catch (err: any) { + if (!isActionNotRegisteredError(err)) throw err; // real handler failure → surface + } + } + } + return { dispatched: false }; +} + /** * True when the routed "object" is the object-less placeholder rather than a * real object — the canonical `'global'`, the legacy `'*'`, or nothing at all @@ -1004,7 +1136,7 @@ export function isObjectLessActionKey(objectName: string | undefined | null): bo */ export async function resolveRouteActionDeclaration(deps: ActionExecutionDeps, args: { ql: any; objectName: string; actionName: string; envId?: string }, -): Promise<{ action: any; obj: any } | undefined> { +): Promise<{ action: any; obj: any; degraded?: boolean; reason?: string }> { const { ql, objectName, actionName, envId } = args; let obj: any; @@ -1032,13 +1164,35 @@ export async function resolveRouteActionDeclaration(deps: ActionExecutionDeps, /* registry without an item lookup → fall through to the metadata service */ } + // [ADR-0110 D3] A miss and an OUTAGE are different facts. `load` answers + // `null` for both — a loader that throws is warn-logged and skipped — so + // reading its `null` as "no declaration, hence no gate to enforce" lets an + // unreachable metadata plane silently ungate every action it can't see. + // `loadDiagnosed` reports whether the answer is trustworthy; a service + // that predates it (or a test double) simply reports nothing degraded. + let degraded = false; + let reason: string | undefined; try { const meta: any = await deps.resolveService('metadata', envId); - const fromMeta: any = await meta?.load?.('action', actionName); - if (fromMeta && ownsRoute(fromMeta)) return { action: fromMeta, obj }; - } catch { - /* no metadata service on this kernel → no declaration to resolve */ + if (meta && typeof meta.loadDiagnosed === 'function') { + const diag: any = await meta.loadDiagnosed('action', actionName); + if (diag?.data && ownsRoute(diag.data)) return { action: diag.data, obj }; + if (diag?.degraded) { + degraded = true; + reason = Array.isArray(diag.errors) && diag.errors.length > 0 + ? diag.errors.join('; ') + : 'the metadata plane reported a loader failure'; + } + } else { + const fromMeta: any = await meta?.load?.('action', actionName); + if (fromMeta && ownsRoute(fromMeta)) return { action: fromMeta, obj }; + } + } catch (err: any) { + // `resolveService` swallows its own resolution failures, so reaching + // here means the metadata service itself threw while answering. + degraded = true; + reason = err?.message ?? String(err); } - return obj ? { action: undefined, obj } : undefined; + return { action: undefined, obj, degraded, reason }; } diff --git a/packages/runtime/src/action-reconciliation.test.ts b/packages/runtime/src/action-reconciliation.test.ts new file mode 100644 index 0000000000..8043ed5356 --- /dev/null +++ b/packages/runtime/src/action-reconciliation.test.ts @@ -0,0 +1,125 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0110 D5] Declaration ↔ executable reconciliation. + * + * ADR-0078 outlaws a declaration nothing executes (silently *inert*); ADR-0110 + * D3 outlaws an executable nothing declares (silently *ungoverned*). Together + * they are one invariant — everything declared runs, everything that runs is + * declared — and this reconciliation is what makes a violation visible at boot + * instead of when a caller happens to hit the route. + * + * The load-bearing property: reconciliation derives handler keys through the + * SAME {@link resolveActionHandlerKeys} the router dispatches through, so the + * inventory can never disagree with what the route actually does. A test that + * hard-coded its own key derivation would drift silently. + */ + +import { describe, it, expect } from 'vitest'; +import { reconcileActionRegistrations } from './action-execution.js'; + +const deps: any = { resolveService: () => undefined, getObjectQL: async () => undefined }; + +const reconcile = ( + registered: Array<{ objectName: string; actionName: string; package?: string }>, + declarations: Array<{ action: any; objectName: string }>, +) => reconcileActionRegistrations(deps, registered, declarations); + +describe('reconcileActionRegistrations — undeclared handlers (ADR-0110 D5)', () => { + it('reconciles a target-bound handler to its declaration', async () => { + // app-todo's real shape: registered under `completeTask`, declared as + // `complete_task`. The names differ, so a naive name-equality check + // would report a false orphan on every well-formed app. + const { undeclaredHandlers } = reconcile( + [{ objectName: 'todo_task', actionName: 'completeTask' }], + [{ objectName: 'todo_task', action: { name: 'complete_task', type: 'script', target: 'completeTask' } }], + ); + + expect(undeclaredHandlers).toEqual([]); + }); + + it('reports a handler no declaration covers', async () => { + const { undeclaredHandlers } = reconcile( + [ + { objectName: 'todo_task', actionName: 'completeTask' }, + { objectName: 'todo_task', actionName: 'secretRecalc', package: 'app:todo' }, + ], + [{ objectName: 'todo_task', action: { name: 'complete_task', type: 'script', target: 'completeTask' } }], + ); + + expect(undeclaredHandlers).toEqual([ + { objectName: 'todo_task', actionName: 'secretRecalc', package: 'app:todo' }, + ]); + }); + + it('reconciles a body action registered under its NAME', async () => { + const { undeclaredHandlers } = reconcile( + [{ objectName: 'todo_task', actionName: 'archive_task' }], + [{ objectName: 'todo_task', action: { name: 'archive_task', type: 'script', body: 'return 1;' } }], + ); + + expect(undeclaredHandlers).toEqual([]); + }); + + it('does not let another OBJECT\'s declaration cover a handler', async () => { + const { undeclaredHandlers } = reconcile( + [{ objectName: 'crm_contact', actionName: 'completeTask' }], + [{ objectName: 'todo_task', action: { name: 'complete_task', type: 'script', target: 'completeTask' } }], + ); + + expect(undeclaredHandlers).toHaveLength(1); + }); + + it('lets an object-less declaration cover an object-less handler', async () => { + const { undeclaredHandlers } = reconcile( + [{ objectName: 'global', actionName: 'logCall' }], + [{ objectName: '*', action: { name: 'log_call', type: 'script', target: 'logCall' } }], + ); + + expect(undeclaredHandlers).toEqual([]); + }); +}); + +describe('reconcileActionRegistrations — unbound declarations (ADR-0078 side)', () => { + it('reports a declared script action with no handler anywhere', async () => { + const { unboundDeclarations } = reconcile( + [], + [{ objectName: 'todo_task', action: { name: 'complete_task', type: 'script', target: 'completeTask' } }], + ); + + expect(unboundDeclarations).toEqual([{ objectName: 'todo_task', actionName: 'complete_task' }]); + }); + + it('does not report a body action — its handler is synthesized', async () => { + const { unboundDeclarations } = reconcile( + [], + [{ objectName: 'todo_task', action: { name: 'archive_task', type: 'script', body: 'return 1;' } }], + ); + + expect(unboundDeclarations).toEqual([]); + }); + + it('does not report non-script types — they dispatch elsewhere', async () => { + const { unboundDeclarations } = reconcile( + [], + [ + { objectName: 'crm_lead', action: { name: 'convert_lead', type: 'flow', target: 'wizard' } }, + { objectName: 'crm_lead', action: { name: 'open_docs', type: 'url', target: 'https://x.test' } }, + { objectName: 'todo_task', action: { name: 'defer_task', type: 'modal', target: 'defer_modal' } }, + ], + ); + + expect(unboundDeclarations).toEqual([]); + }); + + it('accepts a handler registered under the object-less key for an object-scoped declaration', async () => { + // `actionHandlerObjectKeys` rotates object → 'global' → '*' at + // dispatch, so a declaration bound to a global handler IS reachable. + const { unboundDeclarations } = reconcile( + [{ objectName: 'global', actionName: 'completeTask' }], + [{ objectName: 'todo_task', action: { name: 'complete_task', type: 'script', target: 'completeTask' } }], + ); + + expect(unboundDeclarations).toEqual([]); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 37c952c304..28a7d75498 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -10,7 +10,12 @@ import { loadDisabledPackageIds } from './package-state-store.js'; import type { IMetadataService, II18nService } from '@objectstack/spec/contracts'; import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js'; import { hookBodyRunnerFactory, actionBodyRunnerFactory } from './sandbox/body-runner.js'; -import { GLOBAL_ACTION_OBJECT_KEY } from './action-execution.js'; +import { + GLOBAL_ACTION_OBJECT_KEY, + collectActionDeclarations, + reconcileActionRegistrations, + type ActionExecutionDeps, +} from './action-execution.js'; import { countServerTiming } from '@objectstack/observability'; /** @@ -685,6 +690,62 @@ export class AppPlugin implements Plugin { }); } + // ── [ADR-0110 D5] Action governance inventory ──────────────────── + // Reconcile the handler registry against the declarations, on + // `kernel:ready` so imperative `engine.registerAction(...)` calls in + // user code (which run after this bind) are counted. + // + // Since D3 an undeclared handler is REFUSED at dispatch. A hard + // failure with no inventory is a support ticket; with one it is a + // checklist, so this list is the whole point of shipping the refusal + // and the inventory in the same release. Best-effort and warn-only — + // a diagnostic must never be the reason a kernel fails to boot. + ctx.hook('kernel:ready', async () => { + try { + const engine: any = ctx.getService('objectql'); + if (!engine || typeof engine.listRegisteredActions !== 'function') return; + let meta: any; + try { meta = ctx.getService('metadata'); } catch { /* optional */ } + // The reconciliation reads no services of its own — it is pure + // over the two lists — so a thin deps shim is enough here. + const actionDeps: ActionExecutionDeps = { + resolveService: (name: string) => { try { return ctx.getService(name); } catch { return undefined; } }, + getObjectQL: async () => engine, + }; + const declarations = await collectActionDeclarations(actionDeps, meta); + const { undeclaredHandlers, unboundDeclarations } = reconcileActionRegistrations( + actionDeps, engine.listRegisteredActions(), declarations, + ); + if (undeclaredHandlers.length > 0) { + ctx.logger.warn( + '[action-governance] registered handlers with NO declaration — these are refused ' + + 'at dispatch (ADR-0110 D3); declare each one with `defineAction`, or set ' + + 'OS_ALLOW_UNDECLARED_ACTIONS=1 to run them during migration (removed in 18)', + { + appId, + count: undeclaredHandlers.length, + handlers: undeclaredHandlers.map((h) => `${h.objectName}:${h.actionName}`), + }, + ); + } + if (unboundDeclarations.length > 0) { + ctx.logger.warn( + '[action-governance] declared script actions with NO handler — a button wired to ' + + 'nothing (ADR-0078); add a `body`, or register a handler under the declared `target`', + { + appId, + count: unboundDeclarations.length, + actions: unboundDeclarations.map((d) => `${d.objectName}:${d.actionName}`), + }, + ); + } + } catch (err: any) { + ctx.logger.debug('[action-governance] inventory skipped', { + appId, error: err?.message ?? String(err), + }); + } + }); + // ── Auto-register declarative Background Jobs ──────────────────── // Jobs declared via `defineStack({ jobs })` are scheduled against the // running `IJobService` on `kernel:ready` (so the service plugin and diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index f483b7b04e..5dccbd3a07 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -129,7 +129,7 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string // read it. let actionSchema: any; let actionDef: any; - try { + { // Standalone declarations (ObjectQL registry artifacts / Studio- // authored `action` rows) resolve here too, so a route whose action // never appears inside an object definition is gated and dispatched @@ -142,12 +142,64 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string }); actionSchema = declaration?.obj; actionDef = declaration?.action; + + // [ADR-0110 D3] Resolution is a TRICHOTOMY, and only one branch may + // dispatch. This block used to be a `try { … } catch { /* no gate to + // enforce */ }`, which collapsed three states with opposite meanings + // into one fail-open path: a declaration that resolved, a metadata + // plane that could not answer, and an action that genuinely has no + // declaration all arrived as "ungated, run it". + // + // ── 2/3: the metadata plane could not answer ── + // An availability failure is not an authorization decision. Refusing + // is the same posture v17 already takes for a datasource that cannot + // connect (#3741) and a flow run with no trigger user (#3760): decline + // rather than degrade, because the alternative is that an outage + // quietly removes the gate an author declared. + if (declaration.degraded) { + return { + handled: true, + response: deps.error( + `Cannot verify the declaration for action '${actionName}' on '${objectName}' — ` + + `the metadata plane is unavailable (${declaration.reason ?? 'unknown failure'}). ` + + `Refusing rather than running it ungated.`, + 503, + ), + }; + } + + // ── 3/3: genuinely undeclared ── + // A handler with no declaration is invisible to every governance + // surface — ADR-0066 D4 has no `requiredPermissions` to read, ADR-0104 + // no param contract, ADR-0109 materialises no `action_` tool — + // yet it executes TRUSTED. Refuse, and say what to add. The valve is + // for an upgrade that cannot stop to declare one at 3am; it warns on + // every invocation and is slated for removal in 18. + if (!actionDef) { + if (!actionExec.undeclaredActionsAllowed(deps)) { + return { + handled: true, + response: deps.error( + `Action '${actionName}' on '${objectName}' has no declaration — ` + + `add \`defineAction({ name: '${actionName}', … })\`, or register the handler under a ` + + `declared action's \`target\`. Undeclared handlers cannot be permission-gated ` + + `(ADR-0110 D3); set OS_ALLOW_UNDECLARED_ACTIONS=1 to run it during migration.`, + 404, + ), + }; + } + console.warn( + `[action-governance] UNDECLARED action '${objectName}/${actionName}' executed under ` + + `OS_ALLOW_UNDECLARED_ACTIONS — it is ungated (no requiredPermissions, no param contract) ` + + `and invisible to the AI surface. Declare it; the valve is removed in 18.`, + ); + } + + // ── 1/3: declared → gate against it ── const gateError = actionExec.actionPermissionError(deps, actionDef, _context?.executionContext, objectName); if (gateError) { return { handled: true, response: deps.error(gateError, 403) }; } - } catch { - /* schema unresolved → no declared gate to enforce (handler-only action) */ } // [#3915] Action-TYPE dispatch. Per spec every non-`script` type @@ -264,24 +316,24 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string `(system-elevated context, RLS/FLS-bypassing) for user '${userFromAuth.id}'`, ); - // ── script/body dispatch ── - // Probe the routed object first, then the object-less keys (#3913): - // the canonical `'global'` both writers register under, then the legacy - // `'*'`. `executeAction` is an exact-string Map lookup with no wildcard - // semantics, so every candidate key has to be tried for real; only its - // "not registered" miss rotates, a genuine handler error propagates. - let dispatched = false; - let result: any; - for (const obj of actionExec.actionHandlerObjectKeys(objectName)) { - try { - result = await ql.executeAction(obj, actionName, actionContext); - dispatched = true; - break; - } catch (err: any) { - if (!actionExec.isActionNotRegisteredError(err)) throw err; - } - } - if (!dispatched) { + // ── script/body dispatch ── [ADR-0110 D2] "resolve, then address": + // the handler KEY is derived from the resolved declaration, not read + // off the URL. `app-plugin.ts` registers a body action under `name` + // while user code registers a target-bound one under `target`, so the + // URL segment matches the registration key only by luck — which is why + // the documented `/actions/todo_task/complete_task` curl used to 404 + // for every target-bound action. The candidate keys rotate across the + // object-key rotation (#3913) inside the shared helper the MCP + // `run_action` bridge also calls, so both surfaces address handlers + // identically. The routed URL segment stays as the last candidate for + // an UNDECLARED action, which has no declaration to derive from. + const dispatch = await actionExec.executeRegisteredAction( + deps, ql, objectName, + actionExec.resolveActionHandlerKeys(actionDef, actionName), + actionContext, + ); + const result = dispatch.result; + if (!dispatch.dispatched) { // No key carried a handler. That is a routing miss, not a server // fault — 404, and named after the ROUTED object rather than // whichever probe happened to run last (the old fallback reported diff --git a/packages/runtime/src/http-dispatcher.actions-global-key.test.ts b/packages/runtime/src/http-dispatcher.actions-global-key.test.ts index 7254b3b053..f0a36f9343 100644 --- a/packages/runtime/src/http-dispatcher.actions-global-key.test.ts +++ b/packages/runtime/src/http-dispatcher.actions-global-key.test.ts @@ -47,6 +47,15 @@ import { HttpDispatcher } from './http-dispatcher.js'; function makeDispatcher(opts: { registered?: Record any>; objectDef?: any; + /** + * [ADR-0110 D3] This file's axis is ADDRESSING, not governance: every case + * below asks "which key did the route probe, and what envelope came back". + * An undeclared action is refused since D3, so the double declares + * whatever name is asked for — a plain script action bound to its own name, + * which is the very key the URL segment used to be taken as, so the probe + * order these tests pin is unchanged. Pass `false` to exercise the refusal. + */ + declared?: boolean; } = {}) { const registered = opts.registered ?? {}; const executeAction = vi.fn(async (object: string, action: string, ctx: any) => { @@ -65,8 +74,16 @@ function makeDispatcher(opts: { find: vi.fn(async () => []), insert: vi.fn(), update: vi.fn(), delete: vi.fn(), }; + const declareAny = opts.declared !== false; + const synthesize = (type: string, name: string) => + (declareAny && type === 'action') + ? { name, type: 'script', target: name, objectName: 'global' } + : null; const metadata: any = { - load: vi.fn(async () => null), + load: vi.fn(async (type: string, name: string) => synthesize(type, name)), + loadDiagnosed: vi.fn(async (type: string, name: string) => ({ + data: synthesize(type, name), degraded: false, errors: [], + })), listObjects: vi.fn(async () => (objectDef ? [objectDef] : [])), getObject: vi.fn(async () => objectDef), }; @@ -145,6 +162,22 @@ describe('REST /actions — object-less ("global") action key (#3913)', () => { expect(probed).toEqual(['global', '*']); }); + // [ADR-0110 D3] The fixture above declares whatever it is asked for so the + // addressing cases stay on their own axis. That must not be read as "the + // object-less route is exempt from governance" — it is not. + it('refuses an object-less action that has no declaration, before any probe', async () => { + const { dispatcher, executeAction } = makeDispatcher({ + registered: { 'global:log_call': () => ({ logged: true }) }, + declared: false, + }); + + const res = await dispatcher.handleActions('/global/log_call', 'POST', {}, ctx()); + + expect(res.response.status).toBe(404); + expect(res.response.body.error.message).toMatch(/has no declaration/i); + expect(executeAction).not.toHaveBeenCalled(); + }); + it('prefers the object-specific handler over the global one', async () => { const { dispatcher } = makeDispatcher({ registered: { diff --git a/packages/runtime/src/http-dispatcher.actions-identity-addressing.test.ts b/packages/runtime/src/http-dispatcher.actions-identity-addressing.test.ts new file mode 100644 index 0000000000..07dc4731fd --- /dev/null +++ b/packages/runtime/src/http-dispatcher.actions-identity-addressing.test.ts @@ -0,0 +1,226 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * REST `/actions/:object/:action` — identity vs addressing (ADR-0110 D1/D2, + * #3935). + * + * The spec separates two fields that the route used to let bleed into each + * other: `name` is the machine identity (`SnakeCaseIdentifierSchema`), while + * `target` is a BINDING EXPRESSION — polymorphic per type (URL / script key / + * flow id / FormView name), `${param.X}`-interpolatable, and legitimately + * non-unique. The route resolved the DECLARATION from the URL segment as a + * name, but dispatched the HANDLER using the same segment as a registry key. + * + * For a target-bound script action those are different strings, so the two + * documented callers each worked on exactly the half the other broke: + * + * - the documented curl (`.../todo_task/complete_task`, the NAME) resolved + * the declaration — gate + params enforced — then 404ed, because the + * handler is registered under `completeTask`; + * - the Console (`target || name`, the KEY) dispatched fine, but resolved + * NO declaration, so ADR-0066 D4 `requiredPermissions` and the ADR-0104 + * param contract were silently skipped. + * + * These tests pin the D2 fix: identity is always `name`, and the handler key + * is DERIVED from the resolved declaration (the rotation the MCP bridge has + * always used), so the documented URL both gates and dispatches. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { HttpDispatcher } from './http-dispatcher.js'; + +/** app-todo's real shape: declarative name ≠ handler registration key. */ +const targetBoundAction = { + name: 'complete_task', + label: 'Complete', + type: 'script', + target: 'completeTask', +}; + +/** + * A dispatcher whose engine registers handlers under `registeredKeys` only — + * `executeAction` throws the engine's exact "not registered" miss for any + * other key, so the key rotation is exercised for real rather than mocked. + */ +function makeDispatcher(opts: { + objectDef?: any; + registeredKeys?: Record; // objectKey -> handler keys + record?: any; +} = {}) { + const objectDef = opts.objectDef ?? { name: 'todo_task', actions: [targetBoundAction] }; + const registered = opts.registeredKeys ?? { todo_task: ['completeTask'] }; + const calls: Array<{ object: string; key: string }> = []; + + const executeAction = vi.fn(async (object: string, key: string, _ctx: any) => { + calls.push({ object, key }); + if (!registered[object]?.includes(key)) { + throw new Error(`Action '${key}' on object '${object}' not found`); + } + return { ran: key }; + }); + + const ql: any = { + executeAction, + getSchema: (name: string) => (name === objectDef.name ? objectDef : undefined), + registry: { + getObject: (name: string) => (name === objectDef.name ? objectDef : undefined), + getItem: () => undefined, + }, + find: vi.fn(async () => (opts.record ? [opts.record] : [])), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const metadata: any = { + load: vi.fn(async () => null), + listObjects: vi.fn(async () => [objectDef]), + getObject: vi.fn(async () => objectDef), + }; + const kernel: any = { + context: { + getService: (n: string) => + n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null, + }, + }; + return { dispatcher: new HttpDispatcher(kernel), executeAction, calls }; +} + +const ctxFor = (executionContext: any = { userId: 'u1', systemPermissions: [] }): any => ({ + request: {}, + environmentId: 'platform', + executionContext, +}); + +describe('REST /actions — identity is `name`, the handler key is derived (ADR-0110 D2, #3935)', () => { + it('dispatches the DOCUMENTED name-addressed curl to a target-registered handler', async () => { + // `content/docs/ui/actions.mdx` teaches exactly this URL. Before D2 it + // 404ed: the route used `complete_task` as the registry key, but the + // handler lives under `completeTask`. + const { dispatcher, calls } = makeDispatcher(); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', { recordId: 'task_1' }, ctxFor(), + ); + + expect(res.response.status).toBe(200); + expect(res.response.body.data).toMatchObject({ success: true, data: { ran: 'completeTask' } }); + expect(calls).toContainEqual({ object: 'todo_task', key: 'completeTask' }); + }); + + it('enforces the D4 capability gate on the name-addressed URL', async () => { + const gated = { ...targetBoundAction, requiredPermissions: ['task.manage'] }; + const { dispatcher, executeAction } = makeDispatcher({ + objectDef: { name: 'todo_task', actions: [gated] }, + }); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', { recordId: 'task_1' }, + ctxFor({ userId: 'u1', systemPermissions: [] }), + ); + + expect(res.response.status).toBe(403); + expect(res.response.body.error.message).toMatch(/task\.manage/); + expect(executeAction).not.toHaveBeenCalled(); + }); + + it('runs the gated action for a caller who holds the capability', async () => { + const gated = { ...targetBoundAction, requiredPermissions: ['task.manage'] }; + const { dispatcher } = makeDispatcher({ + objectDef: { name: 'todo_task', actions: [gated] }, + }); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', { recordId: 'task_1' }, + ctxFor({ userId: 'u1', systemPermissions: ['task.manage'] }), + ); + + expect(res.response.status).toBe(200); + expect(res.response.body.data).toMatchObject({ success: true, data: { ran: 'completeTask' } }); + }); + + it('prefers `name` when a BODY action declares a target (the AppPlugin registration key)', async () => { + // A body action is auto-registered under `name` even when it also + // carries a target, so the derivation must not blindly prefer target. + const bodyAction = { name: 'archive_task', type: 'script', target: 'someOtherThing', body: 'return 1;' }; + const { dispatcher, calls } = makeDispatcher({ + objectDef: { name: 'todo_task', actions: [bodyAction] }, + registeredKeys: { todo_task: ['archive_task'] }, + }); + + const res = await dispatcher.handleActions( + '/todo_task/archive_task', 'POST', {}, ctxFor(), + ); + + expect(res.response.status).toBe(200); + expect(calls[0]).toEqual({ object: 'todo_task', key: 'archive_task' }); + }); + + it('still resolves a handler registered under the declarative name', async () => { + const { dispatcher } = makeDispatcher({ + registeredKeys: { todo_task: ['complete_task'] }, + }); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', {}, ctxFor(), + ); + + expect(res.response.status).toBe(200); + expect(res.response.body.data).toMatchObject({ success: true, data: { ran: 'complete_task' } }); + }); + + it('rotates to the object-less registration keys for a global handler', async () => { + const { dispatcher, calls } = makeDispatcher({ + registeredKeys: { global: ['completeTask'] }, + }); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', {}, ctxFor(), + ); + + expect(res.response.status).toBe(200); + expect(calls).toContainEqual({ object: 'global', key: 'completeTask' }); + }); + + it('404s naming the ROUTED object when no candidate key carries a handler', async () => { + const { dispatcher } = makeDispatcher({ registeredKeys: {} }); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', {}, ctxFor(), + ); + + expect(res.response.status).toBe(404); + expect(res.response.body.error.message).toContain("'todo_task'"); + }); + + it('propagates a genuine handler failure instead of rotating past it', async () => { + // The rotation must only advance on the engine's "not registered" + // miss; a handler that throws its own error is a business outcome. + const objectDef = { name: 'todo_task', actions: [targetBoundAction] }; + const executeAction = vi.fn(async (_o: string, key: string) => { + if (key === 'completeTask') throw new Error('task already closed'); + throw new Error(`Action '${key}' on object 'todo_task' not found`); + }); + const ql: any = { + executeAction, + getSchema: () => objectDef, + registry: { getObject: () => objectDef, getItem: () => undefined }, + find: vi.fn(async () => []), insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const kernel: any = { + context: { + getService: (n: string) => + n === 'objectql' || n === 'data' ? ql + : n === 'metadata' ? { load: async () => null, listObjects: async () => [objectDef] } + : null, + }, + }; + const dispatcher = new HttpDispatcher(kernel); + + const res = await dispatcher.handleActions( + '/todo_task/complete_task', 'POST', {}, ctxFor(), + ); + + // Business failure reports in the payload (the route's long-standing + // wire contract), NOT as a 404 routing miss. + expect(res.response.status).toBe(200); + expect(res.response.body.data).toMatchObject({ success: false, error: 'task already closed' }); + }); +}); diff --git a/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts b/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts index 08405e3d03..b0ee5aa9f8 100644 --- a/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts +++ b/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts @@ -40,6 +40,8 @@ function makeDispatcher(opts: { standaloneAction?: any; registryAction?: any; record?: any; + /** Simulate a metadata plane that cannot answer (ADR-0110 D3). */ + metadataDegraded?: string; } = {}) { const executeAction = vi.fn(async () => ({ ran: 'script' })); const objectDef = opts.objectDef ?? { name: 'crm_lead', actions: [] }; @@ -57,6 +59,14 @@ function makeDispatcher(opts: { const metadata: any = { load: vi.fn(async (type: string, name: string) => type === 'action' && opts.standaloneAction?.name === name ? opts.standaloneAction : null), + // The real MetadataManager reports whether a `null` is a clean miss or + // an unanswerable one (every loader threw) — the D3 trichotomy needs + // that distinction, so the double carries it too. + loadDiagnosed: vi.fn(async (type: string, name: string) => ({ + data: type === 'action' && opts.standaloneAction?.name === name ? opts.standaloneAction : null, + degraded: Boolean(opts.metadataDegraded), + errors: opts.metadataDegraded ? [opts.metadataDegraded] : [], + })), listObjects: vi.fn(async () => [objectDef]), getObject: vi.fn(async () => objectDef), }; @@ -340,15 +350,57 @@ describe('REST /actions — script dispatch is unchanged (#3915 regression guard expect(res.response.body.data).toEqual({ success: true, data: { ran: 'script' } }); }); - it('still runs an UNDECLARED action through the handler registry (handler-only actions)', async () => { - // `engine.registerAction(...)` with no metadata declaration anywhere — - // the type dispatch must not turn these into a 400. + // [ADR-0110 D3] An UNDECLARED action used to run here, ungated — this test + // asserted exactly that. A handler with no declaration has no + // `requiredPermissions` to enforce, no param contract, and materialises no + // `action_` tool, yet it executes TRUSTED; it now refuses. The valve + // test below is the only path that still runs it. + it('refuses an UNDECLARED action with a prescriptive error instead of running it ungated', async () => { const { dispatcher, executeAction } = makeDispatcher({ objectDef: { name: 'crm_lead', actions: [] } }); const res = await dispatcher.handleActions('/crm_lead/handler_only', 'POST', {}, ctxFor()); - expect(executeAction).toHaveBeenCalledTimes(1); - expect(res.response.body.data.success).toBe(true); + expect(res.response.status).toBe(404); + expect(res.response.body.error.message).toMatch(/has no declaration/i); + expect(res.response.body.error.message).toMatch(/defineAction\(\{ name: 'handler_only'/); + expect(res.response.body.error.message).toMatch(/OS_ALLOW_UNDECLARED_ACTIONS=1/); + expect(executeAction).not.toHaveBeenCalled(); + }); + + it('runs an UNDECLARED action when the migration valve is set, warning every time', async () => { + const prev = process.env.OS_ALLOW_UNDECLARED_ACTIONS; + process.env.OS_ALLOW_UNDECLARED_ACTIONS = '1'; + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { dispatcher, executeAction } = makeDispatcher({ objectDef: { name: 'crm_lead', actions: [] } }); + + const res = await dispatcher.handleActions('/crm_lead/handler_only', 'POST', {}, ctxFor()); + + expect(executeAction).toHaveBeenCalledTimes(1); + expect(res.response.body.data.success).toBe(true); + expect(warn).toHaveBeenCalledWith(expect.stringMatching(/UNDECLARED action 'crm_lead\/handler_only'/)); + } finally { + warn.mockRestore(); + if (prev === undefined) delete process.env.OS_ALLOW_UNDECLARED_ACTIONS; + else process.env.OS_ALLOW_UNDECLARED_ACTIONS = prev; + } + }); + + // [ADR-0110 D3] An unreachable metadata plane must not read as "no + // declaration, hence no gate" — an availability failure would silently + // widen access. Same posture as v17's datasource-that-cannot-connect. + it('503s rather than running ungated when the metadata plane cannot answer', async () => { + const { dispatcher, executeAction } = makeDispatcher({ + objectDef: { name: 'crm_lead', actions: [] }, + metadataDegraded: 'database-loader: ECONNREFUSED', + }); + + const res = await dispatcher.handleActions('/crm_lead/mark_done', 'POST', {}, ctxFor()); + + expect(res.response.status).toBe(503); + expect(res.response.body.error.message).toMatch(/metadata plane is unavailable/i); + expect(res.response.body.error.message).toContain('ECONNREFUSED'); + expect(executeAction).not.toHaveBeenCalled(); }); }); @@ -394,10 +446,15 @@ describe('REST /actions — standalone declarations (#3915)', () => { automation: { execute }, }); - await dispatcher.handleActions('/crm_contact/convert_lead', 'POST', {}, ctxFor()); + const res = await dispatcher.handleActions('/crm_contact/convert_lead', 'POST', {}, ctxFor()); + // Another object's declaration must not gate or dispatch this route. expect(execute).not.toHaveBeenCalled(); - expect(executeAction).toHaveBeenCalledTimes(1); // falls through to the registry, as before + // [ADR-0110 D3] It used to fall through to the registry and run + // ungated; with no declaration OWNED BY THIS ROUTE there is nothing to + // enforce, so it refuses. + expect(res.response.status).toBe(404); + expect(executeAction).not.toHaveBeenCalled(); }); it('enforces the ADR-0066 D4 capability gate on a standalone declaration too', async () => { From cf7036b713b9912a16e6e235157c10a8dd2429be Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 23:59:37 +0000 Subject: [PATCH 2/2] fix(runtime): the D5 governance inventory must not require ctx.hook to exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test Core caught 10 failures in app-plugin.test.ts — `TypeError: ctx.hook is not a function`. The inventory registered its kernel:ready hook unconditionally, while every pre-existing ctx.hook call site was conditional (jobs register one only when the bundle declares jobs), so the shared mock context had never needed the member. Both halves fixed, because both were wrong: - app-plugin.ts — the block promises it is best-effort and 'must never be the reason a kernel fails to boot', then threw when the member was absent. Optional-call so it keeps its own contract for a host with a partial context. - app-plugin.test.ts — hook/trigger are REQUIRED members of PluginContext, so the double was modelling something that cannot exist. Added rather than shrinking the code to fit the double. objectql 1163, metadata 276, runtime 857 — each package run separately and verified by EXIT CODE (the earlier combined run was filtered through grep, which silently dropped the runtime block that carried this failure). --- packages/runtime/src/app-plugin.test.ts | 11 ++++++++++- packages/runtime/src/app-plugin.ts | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/app-plugin.test.ts b/packages/runtime/src/app-plugin.test.ts index 321dad2836..93f12c5b62 100644 --- a/packages/runtime/src/app-plugin.test.ts +++ b/packages/runtime/src/app-plugin.test.ts @@ -16,7 +16,16 @@ describe('AppPlugin', () => { }, registerService: vi.fn(), getService: vi.fn(), - getServices: vi.fn() + getServices: vi.fn(), + // `hook` / `trigger` are REQUIRED members of PluginContext. This + // double omitted them and got away with it only because every + // `ctx.hook` call site happened to be conditional (jobs register + // one only when the bundle declares jobs). ADR-0110 D5's governance + // inventory registers one unconditionally, which surfaced the gap — + // model the real interface rather than shrinking the code to fit + // an incomplete double. + hook: vi.fn(), + trigger: vi.fn() } as unknown as PluginContext; }); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 28a7d75498..b12dae7eba 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -700,7 +700,10 @@ export class AppPlugin implements Plugin { // checklist, so this list is the whole point of shipping the refusal // and the inventory in the same release. Best-effort and warn-only — // a diagnostic must never be the reason a kernel fails to boot. - ctx.hook('kernel:ready', async () => { + // A host whose context predates `hook` (or a partial test double) must + // not lose its kernel to a DIAGNOSTIC — the guard keeps this block's + // own promise, which registering unconditionally did not. + ctx.hook?.('kernel:ready', async () => { try { const engine: any = ctx.getService('objectql'); if (!engine || typeof engine.listRegisteredActions !== 'function') return;