fix(runtime)!: a param-less declarative update refuses undeclared wire params at both action doors

.changeset/24268-declarative-update-undeclared-params-refused.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/runtime': minor
+---
+
+fix(runtime)!: an `operation: 'update'` action that declares no `params` refuses every undeclared wire param at both action doors, instead of writing it to the row
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing an author writes, and no stored shape, is added, removed, renamed or converted: the change is which wire params the two action doors accept for one declared action shape. No spec schema changes, so nothing exists for `objectstack migrate meta` to rewrite. A request is not metadata, and whether a value the client sends should become a declared param is a decision only the action's author can make. -->
+
+**BREAKING** (the REST `/actions` door and the MCP `run_action` door now refuse, with a 400 `VALIDATION_ERROR`, a wire param that a declarative update action without `params` does not declare; measured consumers: objectstack-ai/hotcrm's `complete_task` called with such params; any console vendored before objectui `fb501bc894`, which posts the action's `patch` keys as `params`; and page action rows (`action:button` / `action:icon` with `operation: 'update'`) whose static object `params` the console forwards to a param-less declarative update action, none of which exist in this repo, with the row surface tracked in #24394), shipped as `minor` under the launch-window convention for breaking changes.
+
+A declarative update (`operation: 'update'` + `patch`) has no handler: the route merges the request's `params` over `patch` and writes the result to the current record. An action that declared no `params` skipped the ADR-0104 D2 param check altogether, so any key the caller sent was written too. An action declaring `patch: { status: 'completed' }` could rewrite any other field the caller may write, which is not what its author declared.
+
+Now an empty or absent `params` list on a declarative update is an empty accept set. Every key that is neither a declared param nor a dispatcher built-in (`recordId`, `objectName`, `_selectedIds`) is refused before anything is read or written, through the same D2 path a declared-params action already takes: the same `Invalid action params: Unknown action param "KEY"` message, the same 400, and the same `OS_ALLOW_LAX_ACTION_PARAMS=1` escape hatch. The action still writes its `patch` when the request carries no `params` or an empty bag, and a declared param still writes and still wins over `patch`.
+
+Unchanged: an action with a handler (`target`, `body`, or a flow) that declares no `params` keeps its pass-through.
+
+Also at the MCP `run_action` door: every ADR-0104 D2 param refusal, and the declarative update's own refusals (no `recordId`, or nothing to write), now carry `code: 'VALIDATION_ERROR'` and `status: 400`, so the tool error renders the coded envelope the REST door already answers with, rather than the message alone.
+
+What to do: declare the param on the action if the request's value should be written, or stop sending it, because the route applies `patch` from the declaration and never needs it on the wire.

content/docs/automation/flows.mdx

@@ -1193,7 +1193,8 @@ is consumed: nothing is applied, the run stays parked, and the corrected
 submission still lands.
 
 Three shapes declare no contract and so keep the pass-through — the same way an
-action with no `params` is untouched:
+action with no `params` is untouched, unless it is a declarative
+`operation: 'update'`, whose empty list is an empty accept set:
 
 - an **object-form** screen (`config.objectName`), whose flat `fields` list is
   empty by construction; the client persists the record through the normal write

packages/runtime/src/action-declarative-update.test.ts

@@ -87,6 +87,15 @@ const TRIAGE_CASE = {
     ai: { exposed: true, description: 'Triage this case.' },
 };
 
+/** `params` only, no `patch` — the shape whose empty bag is point 7's "nothing to write". */
+const NOTE_CASE = {
+    name: 'note_case',
+    label: 'Note',
+    operation: 'update',
+    params: [{ name: 'note', type: 'text' }],
+    ai: { exposed: true, description: 'Write a note on this case.' },
+};
+
 /** The REVERSE pin's subject: a handler-less `type: 'script'` action with NO `operation`. */
 const HANDLER_ONLY = {
     name: 'nudge_case',
@@ -103,7 +112,7 @@ const OBJECT_DEF = {
         priority: { type: 'text' },
         note: { type: 'text' },
     },
-    actions: [CLOSE_CASE, TRIAGE_CASE, HANDLER_ONLY],
+    actions: [CLOSE_CASE, TRIAGE_CASE, NOTE_CASE, HANDLER_ONLY],
 };
 
 /** The acting principal, as `resolveExecutionContext` builds one. */
@@ -323,6 +332,116 @@ describe('#15079 point 4 — the write is `{ ...patch, ...params }`, a param of
     });
 });
 
+// ───────────────────────────────────────────────────────────────────────────
+// #24268 — the declared write set IS the accept set, also when it is empty
+// ───────────────────────────────────────────────────────────────────────────
+
+/**
+ * [#24268] A declarative update has no handler: its param bag is merged over
+ * `patch` and WRITTEN to the row. So its declared `params` list is its whole
+ * wire write set, and an empty or absent list is an EMPTY accept set — not the
+ * absent contract a param-less HANDLER action has. Before this, `close_case`
+ * (patch `{ status }`, no `params`) wrote any field the caller may write.
+ *
+ * Every refusal here is pinned on the envelope (`code` + `status`) and on "no
+ * write happened" — never on a bare throw, which an unfixed door that failed
+ * for any other reason would satisfy too.
+ */
+describe('#24268 — a param-less declarative update writes EXACTLY its patch, at both doors', () => {
+    /** A key the object declares and the action does not: the widening the card measured. */
+    const WIDENING = { note: 'WIDENED BY AN UNDECLARED PARAM' };
+
+    it('REST: an undeclared key is refused with the ADR-0104 D2 envelope, and the row is unchanged', async () => {
+        const rig = makeRig();
+        const res = await dispatchRest(OWNER, rig, `/${OBJECT}/close_case/${RECORD_ID}`, { params: WIDENING });
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.code).toBe('VALIDATION_ERROR');
+        // The named subject: the refusal says WHICH key, so the caller can fix it.
+        expect(res.body.error.message).toContain('"note"');
+        expect(rig.updates).toHaveLength(0);
+        expect(rig.row).toEqual(freshRow());
+    });
+
+    it('REST: a `patch` key sent as a param is undeclared too — the wire never re-states the patch', async () => {
+        // The shape a console vendored before objectui fb501bc894 posts: the
+        // patch keys as `params`. The route applies `patch` from the
+        // declaration, so the copy on the wire is an undeclared param.
+        const rig = makeRig();
+        const res = await dispatchRest(OWNER, rig, `/${OBJECT}/close_case/${RECORD_ID}`, {
+            params: { status: 'reopened' },
+        });
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.code).toBe('VALIDATION_ERROR');
+        expect(rig.updates).toHaveLength(0);
+        expect(rig.row).toEqual(freshRow());
+    });
+
+    it('MCP `run_action`: the same undeclared key is refused with `code` + `status`, before any write', async () => {
+        const rig = makeRig();
+        const err: any = await dispatchMcp(OWNER, rig, 'close_case', { recordId: RECORD_ID, params: WIDENING })
+            .then(() => undefined, (e: unknown) => e);
+
+        expect(err).toBeInstanceOf(Error);
+        // What the MCP tool renderer reads off a throw (`thrownRefusalEnvelope`,
+        // `@objectstack/mcp`): a string `code` and a numeric `status`. The code
+        // is the one the REST door's `deps.error(…, 400)` derives.
+        expect(err.code).toBe('VALIDATION_ERROR');
+        expect(err.status).toBe(400);
+        expect(String(err.message)).toContain('"note"');
+        expect(rig.updates).toHaveLength(0);
+        expect(rig.row).toEqual(freshRow());
+    });
+
+    it('an ABSENT or EMPTY bag still writes the patch — at both doors', async () => {
+        for (const body of [{}, { params: {} }]) {
+            const rig = makeRig();
+            const res = await dispatchRest(OWNER, rig, `/${OBJECT}/close_case/${RECORD_ID}`, body);
+            expect(res.status, JSON.stringify(body)).toBe(200);
+            expect(rig.updates.map((u) => u.data)).toEqual([{ status: 'closed' }]);
+            expect(rig.row.status).toBe('closed');
+        }
+        for (const input of [{ recordId: RECORD_ID }, { recordId: RECORD_ID, params: {} }]) {
+            const rig = makeRig();
+            const out: any = await dispatchMcp(OWNER, rig, 'close_case', input);
+            expect(out.ok, JSON.stringify(input)).toBe(true);
+            expect(rig.updates.map((u) => u.data)).toEqual([{ status: 'closed' }]);
+            expect(rig.row.status).toBe('closed');
+        }
+    });
+
+    it('a DECLARED param still writes and wins over `patch` — at both doors', async () => {
+        const rest = makeRig();
+        const res = await dispatchRest(OWNER, rest, `/${OBJECT}/triage_case/${RECORD_ID}`, {
+            params: { priority: 'urgent' },
+        });
+        expect(res.status).toBe(200);
+        expect(rest.updates.map((u) => u.data)).toEqual([{ status: 'triaged', priority: 'urgent' }]);
+
+        const mcp = makeRig();
+        const out: any = await dispatchMcp(OWNER, mcp, 'triage_case', {
+            recordId: RECORD_ID,
+            params: { priority: 'urgent' },
+        });
+        expect(out.ok).toBe(true);
+        expect(mcp.updates.map((u) => u.data)).toEqual([{ status: 'triaged', priority: 'urgent' }]);
+        expect(mcp.row).toMatchObject({ status: 'triaged', priority: 'urgent' });
+    });
+
+    it('the control: a param-less HANDLER action keeps the pass-through — the bag reaches the registry', async () => {
+        // Not this card's population: a handler is the author's own code
+        // deciding what its bag means. The registry miss (404) is the proof
+        // the param gate let the undeclared key through, as it always did.
+        const rig = makeRig();
+        const res = await dispatchRest(OWNER, rig, `/${OBJECT}/nudge_case/${RECORD_ID}`, { params: WIDENING });
+
+        expect(res.status).toBe(404);
+        expect(rig.ql.executeAction).toHaveBeenCalled();
+        expect(rig.updates).toHaveLength(0);
+    });
+});
+
 // ───────────────────────────────────────────────────────────────────────────
 // Contract point 3 — permissions, hooks and validations, exactly as a user edit
 // ───────────────────────────────────────────────────────────────────────────
@@ -500,6 +619,33 @@ describe('#15079 point 7 — no current record is a located refusal, not a silen
         expect(res.body.error.message).toMatch(/nothing to write/);
         expect(rig.updates).toHaveLength(0);
     });
+
+    // [#24268] The MCP door renders a thrown refusal through `@objectstack/mcp`'s
+    // `thrownRefusalEnvelope`, which reads a string `code` and a numeric
+    // `status` off the throw — a throw with no `code` reaches the agent as a
+    // bare sentence. Measured over the real `/mcp` bridge before the fix: both
+    // refusals below answered message-only. So the pin is on the two members
+    // that renderer reads, and on "nothing was written".
+    it('the MCP door answers the same two refusals with `code` + `status`, and writes nothing', async () => {
+        const noRecord = makeRig();
+        const missing: any = await dispatchMcp(OWNER, noRecord, 'close_case', {})
+            .then(() => undefined, (e: unknown) => e);
+        expect(missing).toBeInstanceOf(Error);
+        expect(missing.code).toBe('VALIDATION_ERROR');
+        expect(missing.status).toBe(400);
+        expect(String(missing.message)).toMatch(/recordId/);
+        expect(noRecord.updates).toHaveLength(0);
+
+        const empty = makeRig();
+        const nothing: any = await dispatchMcp(OWNER, empty, 'note_case', { recordId: RECORD_ID, params: {} })
+            .then(() => undefined, (e: unknown) => e);
+        expect(nothing).toBeInstanceOf(Error);
+        expect(nothing.code).toBe('VALIDATION_ERROR');
+        expect(nothing.status).toBe(400);
+        expect(String(nothing.message)).toMatch(/nothing to write/);
+        expect(empty.updates).toHaveLength(0);
+        expect(empty.row).toEqual(freshRow());
+    });
 });
 
 // ───────────────────────────────────────────────────────────────────────────