fix(runtime,mcp): enforce ai.requiresConfirmation at the AI-facing action door, and grow run_action the member that satisfies it

.changeset/action-confirmation-gate-enforced.md

@@ -0,0 +1,28 @@
+---
+"@objectstack/runtime": minor
+"@objectstack/mcp": minor
+---
+
+fix(runtime,mcp): `action.ai.requiresConfirmation` is ENFORCED at the AI-facing action door — an unconfirmed call is refused, and `run_action` grows the `confirm` member that satisfies it (#15942)
+
+**Behaviour change — read this if any of your actions declare `ai.requiresConfirmation: true`.** An AI-facing invocation of such an action (`invokeBusinessAction`, reached from the MCP `run_action` tool) is now REFUSED unless the request carries the confirmation member. A call that succeeded before starts answering `428 ACTION_CONFIRMATION_REQUIRED`, and nothing dispatches: the action body does not run, and the subject record is not even read.
+
+FROM → TO, for a caller of a gated action:
+
+```
+run_action({ actionName: 'archive_lead', recordId: 'lead_1' })                  // was: ran
+run_action({ actionName: 'archive_lead', recordId: 'lead_1', confirm: true })   // now: required
+```
+
+The refusal is machine-readable so the retry is mechanical rather than guessed — `error.details` carries `{ actionName, objectName?, confirmationMember }`, and `confirmationMember` echoes the member's exact spelling (`AI_ACTION_CONFIRMATION_MEMBER`, `@objectstack/spec/contracts`). The `run_action` tool schema advertises `confirm` as an optional boolean, so an agent discovers the retry from the tool definition rather than from prose.
+
+**What is NOT gated**, because this narrows a published accept set and the narrowing is deliberately as small as the author's own declaration:
+
+- Only the DECLARED flag gates. `ai.requiresConfirmation: true`, set by the action's author, and nothing else. The wider `list_actions` heuristic — `mode: 'delete'` / `variant: 'danger'` on an action whose author declared nothing — still reports `requiresConfirmation: true` to advise a client, and still does NOT refuse. An explicit `ai.requiresConfirmation: false` never refuses.
+- Only the boolean `true` confirms. `'true'`, `1` and `false` are not attestations.
+- Only the AI-facing doors. The enforced set is the doors that enforce `ai.exposed` — today `invokeBusinessAction` via MCP `run_action`. REST `/actions` is not `ai.exposed`-gated and sits outside this gate.
+- `list_actions` is unchanged.
+
+**A gate, not a queue.** Nothing is parked, nothing is held for an operator, and there is no resume path: a refused call simply did not run, and the caller confirms with its human and retries. And `confirm: true` is an unverifiable caller claim — an agent that always sends it bypasses the gate. The gate makes FORGETTING loud; it does not prove a human.
+
+Why it is worth the break: the flag was read once and consumed once, to fill a field of the `list_actions` summary. It stopped nothing. That is the failure ADR-0049 retired `tool.requiresConfirmation` for — "a SAFETY flag that is merely accepted is false compliance" — reappearing on the very key the retirement's own ledger entry told authors to move to. The contract this implements landed in `@objectstack/spec` first (#16293).

examples/app-todo/test/mcp-actions.e2e.ts

@@ -180,6 +180,71 @@ function mcpRequest(body: unknown): Request {
   check(unexposedRun.result?.isError === true, 'run_action refuses the unexposed action (fail-closed)');
   check(/not exposed to AI/i.test(unexposedRun.result?.content?.[0]?.text ?? ''), 'refusal names the AI-exposure gate');
 
+  // ── Step 7 — the confirmation gate (#15942), BOTH directions ───────
+  //
+  // The one drive that spans the whole path. A unit test on either side is
+  // blind to the other: `invokeBusinessAction` called directly never sees the
+  // MCP door strip the member (the SDK's shape wrap drops unknown keys, and
+  // the handler forwards a rebuilt object), and a door test with a stubbed
+  // bridge never sees the runtime gate. Here the member travels from a real
+  // JSON-RPC `tools/call`, through both of those layers, into the real gate —
+  // and the record afterwards says whether anything ran.
+  console.log('\n🔒 Step 7 — ai.requiresConfirmation is ENFORCED, and satisfiable');
+  // Same app, but complete_task now declares the author's gate.
+  const gatedConfirmObjects = mergedObjects.map((o) =>
+    o.name !== 'todo_task'
+      ? o
+      : {
+          ...o,
+          actions: o.actions.map((a: any) =>
+            a.name === 'complete_task' ? { ...a, ai: { ...(a.ai ?? {}), requiresConfirmation: true } } : a,
+          ),
+        },
+  );
+  const confirmBridge = bridgeFor(user, gatedConfirmObjects);
+
+  // The listing tells a client the gate is there (unchanged behaviour).
+  const gatedList = JSON.parse((await callMcp(confirmBridge, toolsCall(11, 'list_actions', {}))).result.content[0].text).actions as any[];
+  check(
+    gatedList.find((a) => a.name === 'complete_task')?.requiresConfirmation === true,
+    'list_actions still reports requiresConfirmation:true (unchanged)',
+  );
+
+  const gatedTask: any = await engine.insert('todo_task', { subject: 'Needs a human', status: 'not_started', priority: 'high' });
+  const gatedId = gatedTask?.id ?? gatedTask?.record?.id;
+
+  // 7a — WITHOUT the member: refused, with the declared code, and NOTHING ran.
+  const refused = await callMcp(confirmBridge, toolsCall(12, 'run_action', { actionName: 'complete_task', recordId: gatedId }));
+  check(refused.result?.isError === true, 'run_action WITHOUT confirm is refused');
+  let refusedEnvelope: any = {};
+  try {
+    refusedEnvelope = JSON.parse(refused.result?.content?.[0]?.text ?? '{}');
+  } catch {
+    refusedEnvelope = {};
+  }
+  check(refusedEnvelope?.error?.code === 'ACTION_CONFIRMATION_REQUIRED', `refusal carries code ACTION_CONFIRMATION_REQUIRED (got ${refusedEnvelope?.error?.code})`);
+  check(refusedEnvelope?.error?.status === 428, `refusal carries status 428 (got ${refusedEnvelope?.error?.status})`);
+  check(refusedEnvelope?.error?.details?.actionName === 'complete_task', 'refusal names the action');
+  check(refusedEnvelope?.error?.details?.confirmationMember === 'confirm', 'refusal names the member to set');
+  const afterRefusal: any[] = await engine.find('todo_task', { where: { id: gatedId } });
+  check(afterRefusal?.[0]?.status === 'not_started', `nothing ran — status is still '${afterRefusal?.[0]?.status}'`);
+
+  // 7b — WITH the member: the identical call succeeds and the handler runs.
+  // This is the leg that proves the member is not stripped: before the door
+  // grew it, this call was refused exactly like 7a and the action was
+  // permanently un-invokable.
+  const confirmed = await callMcp(confirmBridge, toolsCall(13, 'run_action', { actionName: 'complete_task', recordId: gatedId, confirm: true }));
+  check(confirmed.result?.isError !== true, 'run_action WITH confirm:true succeeds');
+  const afterConfirm: any[] = await engine.find('todo_task', { where: { id: gatedId } });
+  check(afterConfirm?.[0]?.status === 'completed', `the handler ran — status is now '${afterConfirm?.[0]?.status}'`);
+
+  // 7c — the heuristic must NOT gate: delete_completed is `variant:'danger'`
+  // and the listing calls it requiresConfirmation, but its author declared
+  // nothing, so it stays invokable with no member (this change narrows a
+  // published accept set; refusing here would be the regression).
+  const undeclared = await callMcp(confirmBridge, toolsCall(14, 'run_action', { actionName: 'delete_completed' }));
+  check(undeclared.result?.isError !== true, 'a destructive-LOOKING action with no declared flag is NOT gated');
+
   console.log('\n────────────────────────────────────────────────────────────────────────────────');
   if (failures > 0) {
     console.error(`❌ MCP action E2E FAILED — ${failures} check(s) failed`);

packages/mcp/src/mcp-action-confirmation-member.test.ts

@@ -0,0 +1,219 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#15942 / #16293] The `run_action` door carries the confirmation member —
+ * and the ADR-0112 refusal that member exists to satisfy.
+ *
+ * ## Why this file drives the REAL door
+ *
+ * The gate itself lives in `@objectstack/runtime`
+ * (`actionConfirmationRefusal`), and a test that calls `invokeBusinessAction`
+ * directly is blind to the thing that decides whether the feature works at
+ * all: THE MEMBER NEVER REACHED IT. Measured on this tree before the change,
+ * with a client sending `{ actionName, recordId, confirm: true }` through a
+ * real JSON-RPC `tools/call`, the bridge received `{ recordId: 'r1' }` —
+ * `confirm` was stripped twice over:
+ *
+ *   1. the SDK wraps the raw `inputSchema` shape via `objectFromShape`, and
+ *      under zod a plain object DROPS unknown keys — no error, no reject;
+ *   2. the handler then forwarded only `{ objectName, recordId, params }`.
+ *
+ * `recordId` surviving the same round trip is the lit control on that reading:
+ * the transport works, and only the undeclared member was lost. So enforcing
+ * the gate WITHOUT this door change would have made every action declaring
+ * `ai.requiresConfirmation: true` permanently un-invokable over MCP — refused,
+ * retried with the member, stripped, refused again — which is strictly worse
+ * than the silent no-gate it replaced. That is what this file pins.
+ *
+ * It drives `MCPServerRuntime.handleHttpRequest` over JSON-RPC — the same code
+ * path an external MCP client hits, both strip layers included — rather than
+ * calling `registerActionTools`' handler directly, which would see neither.
+ *
+ * The bridge here is a double: it stands in for the runtime gate so this
+ * package can assert the DOOR's half (schema, forward, envelope) without
+ * depending on `@objectstack/runtime`, which deliberately does not depend back.
+ * The gate's own predicate is pinned in
+ * `packages/runtime/src/action-confirmation-gate.test.ts`, and the two halves
+ * are driven together, against a real engine, in
+ * `examples/app-todo/test/mcp-actions.e2e.ts`.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+
+import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts';
+
+import { MCPServerRuntime } from './mcp-server-runtime.js';
+import type { McpDataBridge, McpActionBridge } from './mcp-http-tools.js';
+
+/** The action the double treats as author-gated. */
+const GATED = 'archive_account';
+
+/**
+ * A bridge that reproduces the runtime gate's OBSERVABLE contract: it refuses
+ * the gated action unless the request carries the member as boolean `true`,
+ * throwing the same `code` / `status` / `details` envelope
+ * `actionConfirmationRefusal` produces.
+ */
+function makeBridge(): McpDataBridge & McpActionBridge & { calls: any[] } {
+  const calls: any[] = [];
+  return {
+    calls,
+    async listObjects() {
+      return [];
+    },
+    async describeObject() {
+      return null;
+    },
+    async query() {
+      return { records: [] };
+    },
+    async get() {
+      return null;
+    },
+    async create() {
+      return {};
+    },
+    async update() {
+      return {};
+    },
+    async remove() {
+      return {};
+    },
+    async listActions() {
+      return [
+        { name: GATED, objectName: 'account', type: 'script', requiresRecord: true, requiresConfirmation: true },
+      ];
+    },
+    async runAction(name: string, input: any) {
+      calls.push([name, input]);
+      if (name === GATED && input?.[AI_ACTION_CONFIRMATION_MEMBER] !== true) {
+        throw Object.assign(
+          new Error(
+            `Action '${GATED}' on 'account' declares ai.requiresConfirmation: true — nothing was run.`,
+          ),
+          {
+            code: 'ACTION_CONFIRMATION_REQUIRED',
+            status: 428,
+            details: {
+              actionName: GATED,
+              objectName: 'account',
+              confirmationMember: AI_ACTION_CONFIRMATION_MEMBER,
+            },
+          },
+        );
+      }
+      return { ok: true, action: name, objectName: 'account', result: { archived: true } };
+    },
+  };
+}
+
+function mcpRequest(body: unknown): Request {
+  return new Request('http://localhost/api/v1/mcp', {
+    method: 'POST',
+    headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
+    body: JSON.stringify(body),
+  });
+}
+
+const toolsCall = (id: number, name: string, args: Record<string, unknown>) => ({
+  jsonrpc: '2.0', id, method: 'tools/call', params: { name, arguments: args },
+});
+
+describe('run_action carries the confirmation member through the real MCP door (#15942)', () => {
+  let runtime: MCPServerRuntime;
+  let bridge: ReturnType<typeof makeBridge>;
+
+  const call = async (body: unknown) => {
+    const res = await runtime.handleHttpRequest(mcpRequest(body), { bridge, parsedBody: body });
+    return (await res.json()) as any;
+  };
+
+  beforeEach(() => {
+    runtime = new MCPServerRuntime({ name: 'objectstack-test', version: '9.9.9' });
+    bridge = makeBridge();
+  });
+
+  it('advertises the member on the tool schema, so a model can DISCOVER the retry', async () => {
+    const json = await call({ jsonrpc: '2.0', id: 1, method: 'tools/list' });
+    const runAction = json.result.tools.find((t: any) => t.name === 'run_action');
+    const props = runAction.inputSchema.properties;
+    // Control: the pre-existing members are still advertised, so a missing
+    // `confirm` below would be a real absence and not an empty read.
+    expect(Object.keys(props)).toEqual(
+      expect.arrayContaining(['actionName', 'objectName', 'recordId', 'params']),
+    );
+    expect(Object.keys(props)).toContain(AI_ACTION_CONFIRMATION_MEMBER);
+    expect(props[AI_ACTION_CONFIRMATION_MEMBER].type).toBe('boolean');
+  });
+
+  it('REFUSES a gated action with no member — code, status and the retry details', async () => {
+    const json = await call(toolsCall(2, 'run_action', { actionName: GATED, recordId: 'a1' }));
+    expect(json.result.isError).toBe(true);
+    const envelope = JSON.parse(json.result.content[0].text);
+    expect(envelope.error.code).toBe('ACTION_CONFIRMATION_REQUIRED');
+    expect(envelope.error.status).toBe(428);
+    // The machine-readable half: a refused agent rebuilds the retry from this
+    // WITHOUT re-parsing the message prose.
+    expect(envelope.error.details).toEqual({
+      actionName: GATED,
+      objectName: 'account',
+      confirmationMember: AI_ACTION_CONFIRMATION_MEMBER,
+    });
+    // …and the door really did forward a request with no confirmation.
+    expect(bridge.calls).toHaveLength(1);
+    expect(bridge.calls[0][1][AI_ACTION_CONFIRMATION_MEMBER]).toBeUndefined();
+  });
+
+  it('SUCCEEDS on the retry — the member survives both strip layers', async () => {
+    const json = await call(
+      toolsCall(3, 'run_action', { actionName: GATED, recordId: 'a1', [AI_ACTION_CONFIRMATION_MEMBER]: true }),
+    );
+    // The assertion the whole card turns on: before this change the member was
+    // dropped here and this call was refused exactly like the one above.
+    expect(bridge.calls[0][1][AI_ACTION_CONFIRMATION_MEMBER]).toBe(true);
+    expect(json.result.isError).toBeFalsy();
+    expect(JSON.parse(json.result.content[0].text)).toMatchObject({ ok: true, result: { archived: true } });
+  });
+
+  it('forwards `recordId` and `params` unchanged beside the member', async () => {
+    await call(
+      toolsCall(4, 'run_action', {
+        actionName: GATED,
+        objectName: 'account',
+        recordId: 'a1',
+        params: { reason: 'dupe' },
+        [AI_ACTION_CONFIRMATION_MEMBER]: true,
+      }),
+    );
+    expect(bridge.calls[0][1]).toEqual({
+      objectName: 'account',
+      recordId: 'a1',
+      params: { reason: 'dupe' },
+      [AI_ACTION_CONFIRMATION_MEMBER]: true,
+    });
+  });
+
+  it('is a CLOSED boolean — a truthy string is refused by the door, not passed on', async () => {
+    const json = await call(
+      toolsCall(5, 'run_action', { actionName: GATED, recordId: 'a1', [AI_ACTION_CONFIRMATION_MEMBER]: 'true' }),
+    );
+    expect(json.result.isError).toBe(true);
+    // A transport artefact must never read as an attestation, and the wrong
+    // failure here would be it reaching the bridge as a truthy value.
+    expect(bridge.calls).toHaveLength(0);
+  });
+
+  it('leaves an UNCODED bridge failure as a plain message (the envelope widens, it narrows nothing)', async () => {
+    const failing = {
+      ...bridge,
+      async runAction() {
+        throw new Error('handler exploded');
+      },
+    };
+    const body = toolsCall(6, 'run_action', { actionName: 'other', recordId: 'x' });
+    const res = await runtime.handleHttpRequest(mcpRequest(body), { bridge: failing, parsedBody: body });
+    const json: any = await res.json();
+    expect(json.result.isError).toBe(true);
+    expect(json.result.content[0].text).toBe('handler exploded');
+  });
+});