From 1ea59d0c1146758a4ffa6cce551654a004155edd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 17:26:44 +0000 Subject: [PATCH 1/3] test(runtime): pin the object-less action-key agreement and the ADR-0104 D2 param gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both functions were ablated repo-wide first. `seedFlowActionParams` turned out to be pinned already — indirectly, through the REST route — but only on its object-BOUND leg; `enforceActionParams` had no pin at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../action-object-less-key-agreement.test.ts | 119 ++++++++++++++++++ .../src/action-params-enforcement.test.ts | 102 +++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 packages/runtime/src/action-object-less-key-agreement.test.ts create mode 100644 packages/runtime/src/action-params-enforcement.test.ts diff --git a/packages/runtime/src/action-object-less-key-agreement.test.ts b/packages/runtime/src/action-object-less-key-agreement.test.ts new file mode 100644 index 0000000000..865c95c48f --- /dev/null +++ b/packages/runtime/src/action-object-less-key-agreement.test.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * "Object-less" has ONE answer inside `action-execution.ts` (#14864). + * + * `isObjectLessActionKey` (`@objectstack/objectql`) is the canonical predicate: + * the routed object is object-less when it is the canonical + * `GLOBAL_ACTION_OBJECT_KEY`, the legacy `'*'`, or nothing at all. + * `dispatchFlowAction` asks it directly when it decides whether to hand the + * automation service an `object` at all — and then, on the very next line, + * hands the same `objectName` to `seedFlowActionParams`, which used to answer + * the same question with a second, narrower comparison of its own. + * + * The two parted on exactly one input, `'*'`: the automation envelope treated a + * `'*'` route as object-less and omitted `object`, while the params bag treated + * it as a real object and seeded a nonsense `'*Id'` alias key beside + * `recordId`. One dispatch, two answers, three lines apart. + * + * ## Why the pin sits HERE and not only on the route + * + * `seedFlowActionParams` was NOT unpinned — `http-dispatcher.actions-type- + * dispatch.test.ts` covers its whole seeding ladder, indirectly, through the + * REST route, without ever naming it. What that file never does is route at an + * object-LESS key: every case there is `/crm_lead/...`. So the ladder was + * pinned and the object-less leg of it was not, which is why the divergence + * survived. This file pins the leg, at the level the two predicates actually + * meet: one function, the whole `isObjectLessActionKey` domain, one bag. + * + * ## The arms, and which one is the control + * + * The `OBJECT_FUL` case is an ANTI-VACUITY CONTROL, not a pin: it asserts the + * alias key IS seeded for a real object. If a future edit makes + * `seedFlowActionParams` seed nothing at all, the negative assertions below + * would all pass for the wrong reason, and this control is what fails instead. + * ⛔ A red here is not this file's finding — read the object-less arm first. + */ + +import { describe, it, expect } from 'vitest'; +import { GLOBAL_ACTION_OBJECT_KEY, isObjectLessActionKey } from '@objectstack/objectql'; +import { seedFlowActionParams, type ActionExecutionDeps } from './action-execution.js'; + +/** `seedFlowActionParams` ignores its first parameter — see its signature. */ +const NO_DEPS = undefined as unknown as ActionExecutionDeps; + +const ROW_ID = 'row_1'; + +/** + * The `Id` camelCase alias `seedFlowActionParams` seeds for an + * object-bound route, derived the way the function derives it rather than + * hard-coded — a hard-coded copy would go stale in silence the day the + * spelling changes, which is the same failure this whole card is about. + */ +const aliasKeyFor = (objectName: string): string => + `${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`; + +const seedFor = (objectName: string): Record => + seedFlowActionParams(NO_DEPS, { name: 'convert_lead', type: 'flow' }, { + objectName, + record: {}, + params: {}, + recordId: ROW_ID, + }); + +/** + * Every string spelling `isObjectLessActionKey` accepts. The table is asserted + * against the predicate itself below, so narrowing the predicate (retiring + * `'*'`, say) fails HERE with a readable message instead of quietly leaving a + * row that no longer describes anything. + */ +const OBJECT_LESS_KEYS: readonly string[] = [GLOBAL_ACTION_OBJECT_KEY, '*', '']; + +/** A real object — the control's route, and the one the REST pin already uses. */ +const OBJECT_FUL = 'crm_lead'; + +describe('object-less action key — one predicate, one answer (#14864)', () => { + it('anti-vacuity control: an object-BOUND route still seeds its alias key', () => { + // Positive control. Every negative below is a claim that a key is + // absent; without this, deleting the seeding branch outright would + // turn them all green. + expect(isObjectLessActionKey(OBJECT_FUL)).toBe(false); + const bag = seedFor(OBJECT_FUL); + expect(bag[aliasKeyFor(OBJECT_FUL)]).toBe(ROW_ID); + expect(bag.recordId).toBe(ROW_ID); + }); + + it('the table below describes exactly what the predicate accepts', () => { + // Guards the table, not the code: a narrowed predicate must come here + // and say so rather than leaving an inert row behind. + for (const key of OBJECT_LESS_KEYS) { + expect(isObjectLessActionKey(key), `${JSON.stringify(key)} is no longer object-less`).toBe(true); + } + }); + + it.each(OBJECT_LESS_KEYS.map((key) => ({ key, label: JSON.stringify(key) })))( + 'seeds no object alias for the object-less key $label', + ({ key }) => { + const bag = seedFor(key); + // The row id still reaches the flow — this is about the ALIAS only. + expect(bag.recordId).toBe(ROW_ID); + expect( + Object.keys(bag), + `seedFlowActionParams seeded the alias key ${JSON.stringify(aliasKeyFor(key))} for the ` + + `object-less route ${JSON.stringify(key)}. isObjectLessActionKey() calls that route ` + + `object-less and dispatchFlowAction omits \`object\` from the automation envelope for ` + + `it, so the params bag must not invent an object alias either (#14864).`, + ).not.toContain(aliasKeyFor(key)); + }, + ); + + it('every object-less spelling lands the SAME bag as the canonical key', () => { + // The agreement stated as one assertion: which object-less spelling a + // caller routed at must not be observable in the flow's params. + const canonical = seedFor(GLOBAL_ACTION_OBJECT_KEY); + for (const key of OBJECT_LESS_KEYS) { + expect(seedFor(key), `routing at ${JSON.stringify(key)} produced a different params bag`) + .toEqual(canonical); + } + }); +}); diff --git a/packages/runtime/src/action-params-enforcement.test.ts b/packages/runtime/src/action-params-enforcement.test.ts new file mode 100644 index 0000000000..041bacdc95 --- /dev/null +++ b/packages/runtime/src/action-params-enforcement.test.ts @@ -0,0 +1,102 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `enforceActionParams` — the ADR-0104 D2 gate itself, not its validator + * (#14864). + * + * ## What was measured, and why this file exists + * + * The card that produced this file claimed neither `seedFlowActionParams` nor + * `enforceActionParams` was named by any test. Grep cannot settle that — a pin + * can live in a file that never names the function — so both were ABLATED + * instead, repo-wide against `packages/runtime`'s 217 files / 3143 tests: + * + * - `seedFlowActionParams`, gutted → **5 tests red** in + * `http-dispatcher.actions-type-dispatch.test.ts`. Pinned all along, + * indirectly, through the REST route. The claim was wrong about it. + * - `enforceActionParams`, replaced with an unconditional `return null` (the + * gate accepting every bag) → **3143 passed, 0 failed**. Nothing in the repo + * noticed the param contract had stopped existing. + * + * The VALIDATOR is thoroughly pinned — `@objectstack/spec`'s + * `action-params.test.ts` covers `validateActionParams` case by case. What had + * no pin is the runtime GATE wrapped around it, and the gate is where the + * decisions live that the validator never makes: the param-less pass-through, + * the strict-by-default rejection, and the `OS_ALLOW_LAX_ACTION_PARAMS` escape + * hatch. A green validator says nothing about whether anything still calls it. + * + * That gap matters more than a missing unit test usually does: this gate is + * what stops an AI/MCP caller's plausible-but-wrong bag from reaching an + * action body (#3438), and its only other mention outside the source is a + * MANUALLY-run platform-checklist clause. So it is pinned here at the level + * the ablation showed to be empty. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { enforceActionParams, type ActionExecutionDeps } from './action-execution.js'; + +/** `enforceActionParams` reaches nothing on `deps` — it only forwards it. */ +const NO_DEPS = undefined as unknown as ActionExecutionDeps; + +/** No parent object schema: an object-less action carrying inline params only. */ +const NO_OBJECT = undefined; + +const WHERE = { objectName: 'crm_lead', actionName: 'convert_lead' }; + +const REQUIRES_TITLE = { + name: 'convert_lead', + params: [{ name: 'title', type: 'text', required: true }], +}; + +describe('enforceActionParams — the ADR-0104 D2 gate (#14864)', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + it('anti-vacuity control: a CONFORMING bag against declared params is accepted', () => { + // Positive control for the rejection below. Without it, a gate that + // rejected everything, or one that had stopped resolving params at + // all, would still satisfy "rejects a bad bag". + expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, { title: 'Hi' }, WHERE)).toBeNull(); + }); + + it('rejects a bag that violates the declared contract, naming the param', () => { + const error = enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, WHERE); + expect(error).toContain('Invalid action params'); + expect(error).toContain('title'); + }); + + it('passes an action that declares NO params straight through', () => { + // The documented compatibility leg: nothing to validate against, so a + // param-less action is untouched however odd its bag looks. + expect(enforceActionParams(NO_DEPS, { name: 'ping' }, NO_OBJECT, { anything: 1 }, WHERE)).toBeNull(); + expect(enforceActionParams(NO_DEPS, { name: 'ping', params: [] }, NO_OBJECT, { anything: 1 }, WHERE)).toBeNull(); + }); + + it('is STRICT by default — no environment variable needed to reject (#3438)', () => { + vi.stubEnv('OS_ALLOW_LAX_ACTION_PARAMS', ''); + expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, WHERE)).toContain('Invalid action params'); + }); + + it('`OS_ALLOW_LAX_ACTION_PARAMS=1` accepts the same bag instead, and warns', () => { + // The opt-OUT of a check that ships ON (Prime Directive #9): the flag + // must change the ANSWER, not merely the log line — a flag that only + // logs would leave the rejection in place and strand the caller it was + // added to unblock. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.stubEnv('OS_ALLOW_LAX_ACTION_PARAMS', '1'); + + // A dedup key this suite has not warned on yet — `warnActionParamsOnce` + // keys on `objectName/actionName` and its Set is module-global, so a + // reused key would make the warn assertion pass or fail on test order. + const where = { objectName: 'crm_lead', actionName: 'lax_probe' }; + expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, where)).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain('OS_ALLOW_LAX_ACTION_PARAMS=1'); + }); +}); From 02b68dd95c89a7762bd00ae1d1acd11cc36e98ce Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 17:28:35 +0000 Subject: [PATCH 2/3] fix(runtime): route the flow param seeder through the single object-less predicate Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/runtime/src/action-execution.ts | 11 ++++++++++- .../src/action-owner-key-single-source.test.ts | 14 +++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 122fbe6bed..13d66a752c 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -638,7 +638,16 @@ export function seedFlowActionParams(_deps: ActionExecutionDeps, if (rowId != null) { const keys = new Set(['recordId']); - if (objectName && objectName !== GLOBAL_ACTION_OBJECT_KEY) { + // [#14864] ONE predicate for "object-less", the same one + // `dispatchFlowAction` asks three lines from here before it decides + // whether to hand the automation service an `object` at all. This used + // to be a second, narrower comparison (`objectName !== + // GLOBAL_ACTION_OBJECT_KEY`), and the two parted on exactly one input: + // a route resolved at the legacy `'*'` was object-less to the envelope + // and object-BOUND here, so the bag grew a nonsense `'*Id'` alias. The + // empty-string leg was never the divergence — the `objectName &&` + // truthiness test this replaces already covered it. + if (!isObjectLessActionKey(objectName)) { keys.add(`${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`); } if (typeof action?.recordIdParam === 'string' && action.recordIdParam) { diff --git a/packages/runtime/src/action-owner-key-single-source.test.ts b/packages/runtime/src/action-owner-key-single-source.test.ts index 53aab30cc5..78ec06f5fb 100644 --- a/packages/runtime/src/action-owner-key-single-source.test.ts +++ b/packages/runtime/src/action-owner-key-single-source.test.ts @@ -150,8 +150,20 @@ describe('standalone-action owner key — half C: no bare literal (#14678)', () // exactly the wrong reason — the can-never-fail property this whole // file was written to replace. Both controls are positive assertions // against text the converged file must carry. + // + // [#14864] The second control used to be the `seedFlowActionParams` + // comparison `objectName !== GLOBAL_ACTION_OBJECT_KEY`. That guard is + // gone — it was one of the two rival answers to "is this route + // object-less", and it now delegates to `isObjectLessActionKey` like + // its neighbours. Re-anchored rather than deleted, and deliberately + // onto a site this file's own subject does not move: the warn-once log + // key in `enforceActionParams`, which is the SECOND of the three bare + // literals #14678 converged and is untouched by the predicate work. + // ⛔ Do not re-anchor a control onto the thing the next change is most + // likely to edit — a control that moves with its subject stops being a + // control. expect(src).toContain('GLOBAL_ACTION_OBJECT_KEY'); - expect(src).toContain('objectName !== GLOBAL_ACTION_OBJECT_KEY'); + expect(src).toContain('where.objectName ?? GLOBAL_ACTION_OBJECT_KEY'); for (const literal of BARE_LITERALS) { expect( From ae512c7e73df6f32d45393355e0fea50048fbb7e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 17:30:02 +0000 Subject: [PATCH 3/3] chore(changeset): patch for the object-less action-key predicate convergence Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../object-less-action-key-one-predicate.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .changeset/object-less-action-key-one-predicate.md diff --git a/.changeset/object-less-action-key-one-predicate.md b/.changeset/object-less-action-key-one-predicate.md new file mode 100644 index 0000000000..fe350af79c --- /dev/null +++ b/.changeset/object-less-action-key-one-predicate.md @@ -0,0 +1,47 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): route the flow param seeder through the single object-less predicate (#14864) + +`isObjectLessActionKey` (`@objectstack/objectql`) is the canonical answer to +"is this routed object the object-less placeholder": the canonical +`GLOBAL_ACTION_OBJECT_KEY`, the legacy `'*'`, or nothing at all. +`dispatchFlowAction` asks it directly when it decides whether to hand the +automation service an `object` at all — and then, three lines later, handed the +same `objectName` to `seedFlowActionParams`, which answered the same question +with a second, narrower comparison of its own (`objectName !== +GLOBAL_ACTION_OBJECT_KEY`). + +The two parted on exactly one input, `'*'`. A request routed at the legacy +wildcard — `POST /actions/*//`, which resolves today because +`actionHandlerObjectKeys` deliberately probes `'*'` last so a handler user code +registered against it still resolves — was object-less to the automation +envelope (no `object` sent) and object-BOUND to the params bag, which seeded a +nonsense `'*Id'` key beside `recordId`. Same dispatch, two answers. + +The empty-string half was never part of the divergence: the `objectName &&` +truthiness leg of the old guard already covered it, and `undefined` with it. +`'*'` was the whole of it. + +**Direction.** The guard is widened onto the shared predicate rather than +`isObjectLessActionKey` being narrowed. `'*'` is *unused today*, not *dead*: +nothing first-party registers under it, but it is a deliberately-honoured +legacy read path with its own docblock, reachable through the public +`engine.registerAction(objectName, …)` surface that user code calls. Retiring +it is a compatibility decision about someone else's package, not a tidy-up this +fix is entitled to make. + +**Coverage.** Both functions this touches were ablated repo-wide first rather +than grepped, because a grep scoped to the file you expect a pin in cannot see +a pin living elsewhere: + +- `seedFlowActionParams` gutted → 5 tests red. It was pinned all along, + indirectly, through the REST route — but every case there routes at a real + object, so the object-LESS leg, where the two predicates actually disagreed, + was the unpinned part. Now pinned, over the whole predicate domain. +- `enforceActionParams` replaced with an unconditional `return null` → 3143 + passed, 0 failed. The ADR-0104 D2 gate could stop existing with nothing in + the repo noticing. Its validator is well pinned in `@objectstack/spec`; the + runtime gate around it was not, and that gate is what keeps an AI/MCP + caller's plausible-but-wrong bag out of an action body. Now pinned.