diff --git a/.changeset/lucky-doors-tickle.md b/.changeset/lucky-doors-tickle.md new file mode 100644 index 0000000000..20e2591bb4 --- /dev/null +++ b/.changeset/lucky-doors-tickle.md @@ -0,0 +1,20 @@ +--- +'@objectstack/objectql': patch +'@objectstack/lint': patch +--- + +Stop reporting a declarative `operation: 'update'` action as "a button wired to nothing" + +The boot action-governance inventory (ADR-0110 D5) built its `unboundDeclarations` +finding from a `type`-only test. The declarative single-record field write +(`operation: 'update'` + `patch`, #14092) is exactly the shape that test mistakes +for a dead button: `ActionSchema` refuses `target` and `body` beside it and keeps +`type` at its default `script`, because the platform action route is where the +write is performed. Every such action was named at every boot and every +`metadata:reloaded` — with a prescription ("add a `body`, or register a handler +under the declared `target`") that parse itself refuses. + +Both readers now read `operation` before `type`, the precedence the runtime doors +already use: the engine inventory, and the authoring-time AI tool-reference rule, +which had diverged from the runtime's listing door and reported a resolvable +`action_` reference as fictional. diff --git a/packages/lint/src/validate-ai-tool-references.test.ts b/packages/lint/src/validate-ai-tool-references.test.ts index 45dd2bf1e4..995d50f1cc 100644 --- a/packages/lint/src/validate-ai-tool-references.test.ts +++ b/packages/lint/src/validate-ai-tool-references.test.ts @@ -172,4 +172,43 @@ describe('validate-ai-tool-references', () => { validateAiToolReferences({ skills: [null, { tools: [7, null, 'query_records'] }] } as never), ).toEqual([]); }); + + /** + * [#15444] The declarative `operation: 'update'` action materialises a tool. + * + * `materialisesAsTool` mirrors the runtime's headless-invocability door, and + * that door learned `operation` before `type` in #15079 — the declarative + * single-record field write (#14092) is invokable with NEITHER a `target` + * nor a `body`, because `ActionSchema` refuses both beside + * `operation: 'update'` and the platform action route performs the write. + * While this mirror still read `type` only, its `script` arm answered + * `false`: `action_mark_done` was reported as a fictional tool reference and + * the action was offered in the near-miss hint as one that never + * materialises — for metadata the runtime lists and runs. + */ + it('materialises a tool for a declarative `operation: \'update\'` action (no target, no body)', () => { + const markDone = { + name: 'mark_done', + operation: 'update', + patch: { status: 'done' }, + ai: { exposed: true, description: 'Mark the current task done.' }, + }; + const stack = { + objects: [{ name: 'todo_task', actions: [markDone] }], + skills: [{ name: 's', tools: ['action_mark_done'] }], + }; + expect(validateAiToolReferences(stack)).toEqual([]); + + // Positive control, same rule and same shape: an AI-exposed `script` + // action with no `operation` and nothing to dispatch on still does NOT + // materialise, so a reference to it is still reported. Without this a + // predicate hard-wired to `true` would pass the assertion above. + const wiredToNothing = { ...markDone, operation: undefined, patch: undefined }; + const control = validateAiToolReferences({ + objects: [{ name: 'todo_task', actions: [wiredToNothing] }], + skills: [{ name: 's', tools: ['action_mark_done'] }], + }); + expect(control).toHaveLength(1); + expect(control[0].path).toBe('skills[0].tools[0]'); + }); }); diff --git a/packages/lint/src/validate-ai-tool-references.ts b/packages/lint/src/validate-ai-tool-references.ts index f9f41ff9de..4509e751ff 100644 --- a/packages/lint/src/validate-ai-tool-references.ts +++ b/packages/lint/src/validate-ai-tool-references.ts @@ -89,6 +89,20 @@ function suggest(target: string, known: Set): string { */ const HEADLESS_ACTION_TYPES = new Set(['script', 'api', 'flow']); +/** + * [#15444] The one row-level `operation` the spec admits (`ActionSchema. + * operation`, `z.enum(['update'])` — #14092). Deliberately NOT a member of + * {@link HEADLESS_ACTION_TYPES}: `operation` is not a type, and adding + * `'update'` there would be the member spelling the ruling rejected. What + * learns `operation` is the READER below. Spelled locally rather than imported + * from `@objectstack/runtime`'s `DECLARATIVE_UPDATE_OPERATION`: this package is + * authoring-time (`spec`, `formula`, `sdui-parser`) and does not depend on the + * runtime at all — taking that dependency to reach one string would invert the + * layering. The coupling that matters is behavioural, and it is pinned: the + * predicate below must keep answering what the runtime's listing door answers. + */ +const DECLARATIVE_UPDATE_OPERATION = 'update'; + /** * Would the runtime materialise an `action_` tool for this action? * @@ -113,6 +127,16 @@ function materialisesAsTool(action: AnyRec): boolean { if (aiRec.exposed !== true) return false; if (!strName(aiRec.description)) return false; + // [#15444] `operation` before `type`, the precedence the runtime door reads + // (`isDeclarativeUpdateAction` — #15079, ruling #14092). The declarative + // single-record field write materialises a tool with NEITHER a `target` nor + // a `body`: `ActionSchema` refuses both beside `operation: 'update'` because + // the platform action route performs the write. The `script` arm below + // therefore answers `false` for it — the exact divergence #15079 closed on + // the runtime side, which lists it — and this rule would then report a + // resolvable `action_` reference as fictional, and name the action in + // the near-miss hint as one that "never materialises". + if (action.operation === DECLARATIVE_UPDATE_OPERATION) return true; const type = strName(action.type); if (!type || !HEADLESS_ACTION_TYPES.has(type)) return false; // `script` can carry either a named handler or an inline body; `api` and diff --git a/packages/objectql/src/action-governance.test.ts b/packages/objectql/src/action-governance.test.ts index 6b0e9fd3d9..9de2535d2f 100644 --- a/packages/objectql/src/action-governance.test.ts +++ b/packages/objectql/src/action-governance.test.ts @@ -298,3 +298,64 @@ describe('runActionGovernanceInventory — the router registry rung (#14123)', ( ); }); }); + +/** + * [#15444] The declarative single-record field write is not a dead button. + * + * `unboundDeclarations` was built from a `type`-only test, and the declarative + * `operation: 'update'` action (#14092, maintainer ruling 2026-09-01) is + * exactly the shape that test mistakes for a button wired to nothing: the spec + * refuses `target` and `body` beside `operation: 'update'` and keeps `type` at + * its materialized default `'script'`, because the platform action route IS + * where the write is performed. So it fell through both `continue`s and was + * named, at every boot and every `metadata:reloaded`, in the ADR-0110 D5 + * surface an operator reads to find REAL dead buttons — with a prescription + * ("add a `body`, or register a handler under the declared `target`") that + * `ActionSchema` refuses at parse time. + * + * The positive control shares the object and the run deliberately: a pin that + * only asserted the update action's absence would pass just as well against a + * reader that reported nothing at all. + */ +describe('[#15444] a declarative `operation: \'update\'` action is not an unbound declaration', () => { + const objects = [{ + name: 'todo_task', + actions: [ + // The platform performs this write; `type` stays at its default. + { name: 'mark_done', operation: 'update', patch: { status: 'done' } }, + // Positive control: a real dead button, still named. + { name: 'ghost_button', type: 'script' }, + ], + }]; + + it('omits it from the finding while still naming a real dead button', async () => { + const logger = makeLogger(); + await runActionGovernanceInventory({ registered: [], objects, logger }); + + const unbound = logger.warn.mock.calls.filter( + (call: any[]) => /declared script actions with NO handler/.test(String(call[0])), + ); + expect(unbound).toHaveLength(1); + expect(unbound[0][1]).toEqual(expect.objectContaining({ + count: 1, + actions: ['todo_task:ghost_button'], + })); + }); + + it('holds when the row carries an explicit `type: \'script\'` (the materialized default)', async () => { + const logger = makeLogger(); + await runActionGovernanceInventory({ + registered: [], + objects: [{ + name: 'todo_task', + actions: [{ name: 'mark_done', type: 'script', operation: 'update', patch: { status: 'done' } }], + }], + logger, + }); + + expect(logger.warn).not.toHaveBeenCalledWith( + expect.stringMatching(/declared script actions with NO handler/), + expect.anything(), + ); + }); +}); diff --git a/packages/objectql/src/action-governance.ts b/packages/objectql/src/action-governance.ts index a38081fc7d..752ed5414d 100644 --- a/packages/objectql/src/action-governance.ts +++ b/packages/objectql/src/action-governance.ts @@ -183,7 +183,11 @@ export function resolveActionHandlerKeys(action: any, fallbackKey?: string): str * it. A caller that skips that step is asserting a dispatch outcome from * two of the router's three sources. * - `unboundDeclarations` — a declared `script` action with no `body` and no - * handler under any candidate key: a button wired to nothing. + * handler under any candidate key: a button wired to nothing. [#15444] The + * declarative `operation: 'update'` action is excluded: it is bound to + * nothing by construction and correct, because the platform action route + * performs its write. `operation` is read before `type` here for the same + * reason the runtime doors read it first. */ export function reconcileActionRegistrations( registered: Array<{ objectName: string; actionName: string; package?: string }>, @@ -219,6 +223,26 @@ export function reconcileActionRegistrations( const registeredKeys = new Set(registered.map((r) => `${r.objectName}:${r.actionName}`)); const unboundDeclarations: Array<{ objectName: string; actionName: string }> = []; for (const { action, objectName, storeKey } of declarations) { + // [#15444] `operation` before `type` — the precedence the runtime doors + // read (`isDeclarativeUpdateAction`, #15079; ruling #14092). The + // declarative single-record field write carries NO handler BY + // CONSTRUCTION: `ActionSchema` refuses `target` and `body` beside + // `operation: 'update'`, because the platform action route is where the + // write is performed. So it is the one declared `script` action that is + // bound to nothing and entirely correct, and the `type`-keyed test below + // would give the right answer to the wrong question — naming it in the + // ADR-0110 D5 surface an operator reads to find REAL dead buttons, with + // a prescription ("add a `body`, or register a handler under the + // declared `target`") that parse REFUSES. A false population that grows + // with every declarative update action an app author writes is how a + // diagnostic stops being read. + // + // A bare equality on the declared key, with no `type` clause, is the + // ruled spelling: an action carrying `operation: 'update'` IS the + // declarative write, whatever `type` says. Data at rest that never went + // through `ActionSchema` (a Studio row, a `strict: false` bundle) is + // exactly the population where the two keys can contradict. + if (action?.operation === 'update') continue; if ((action?.type ?? 'script') !== 'script') continue; // only script needs a handler if (action?.body) continue; // its handler is synthesized // A row with neither an own `name` nor a store key cannot be addressed