fix(runtime,mcp): a declarative update action whose write landed under a failing roll-up answers its own success at both action doors

.changeset/28856-action-stale-summary-answer.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/runtime': patch
+'@objectstack/mcp': patch
+---
+
+fix(runtime,mcp): a declarative update action whose write landed under a failing roll-up answers its own success at both action doors (#28856)
+
+Clause-②: no
+Metadata-protocol: none
+Skills-docs: none — no file under `skills/**` teaches what an action door answers when a parent roll-up cannot be recomputed after the action's write.
+
+When a declarative update action (`operation: 'update'`) writes its record but a parent's roll-up summary cannot be recomputed after it (`ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written"), the action now answers the result it answers when the recompute succeeds, instead of an error that invited the caller to run the action again. No new code, member, status or shape:
+
+- **REST** `POST /actions/:object/:action/:id`: `200` with the declarative result (`{ operation, object, id, record, undo? }`).
+- **MCP `run_action`**: the usual `{ ok, action, objectName, recordId, result }`.
+- The action's invocation audit row records `succeeded`, the outcome the door answers.
+
+The runtime composes the action's result once and attaches it to the refusal through the existing carrier; each door relays it. The stale summary column is logged at `error` once per field per process. A refusal before the write answers exactly as before. Other action kinds are unchanged: an `api` action's forward already lands on a data door that answers success, a `flow` action's record nodes turn the refusal into a node failure, and a script or body action is a separate follow-up.

packages/mcp/src/mcp-http-tools.ts

@@ -482,17 +482,23 @@ function errorResultWithCode(err: unknown) {
 }
 
 /**
- * [#28782, #28844] A write tool's answer to a throw: a write the engine stored
- * but whose parent roll-up it could not recompute (`ERR_SUMMARY_RECOMPUTE`,
+ * [#28782, #28844, #28856] A write tool's answer to a throw: a write the engine
+ * stored but whose parent roll-up it could not recompute (`ERR_SUMMARY_RECOMPUTE`,
  * "the triggering records WERE written") answers the write it is, read off
  * #28781's carrier the bridge attached, with the stale column logged once at
  * `error`. An agent that read an error would apply the write again. Any other
  * throw, including every refusal raised before the write, renders through
- * {@link errorResultWithCode} exactly as before.
+ * `otherwise` exactly as before: {@link errorResultWithCode} for the data tools,
+ * {@link errorResultFromThrown} for `run_action`, whose answer the runtime
+ * composed and attached.
  */
-function landedWriteResultOrError(err: unknown, objectName: string) {
+function landedWriteResultOrError(
+  err: unknown,
+  objectName: string | undefined,
+  otherwise: (err: unknown) => ReturnType<typeof errorResultWithCode> = errorResultWithCode,
+) {
   const earned = earnedAnswerOf(err);
-  if (earned === undefined) return errorResultWithCode(err);
+  if (earned === undefined) return otherwise(err);
   for (const line of staleSummaryAnswerLines(err as SummaryRecomputeRefusal, objectName)) console.error(`[MCP] ${line}`);
   return textResult(earned);
 }
@@ -1590,7 +1596,7 @@ export function registerActionTools(
         });
         return textResult(result);
       } catch (err) {
-        return errorResultFromThrown(err);
+        return landedWriteResultOrError(err, objectName, errorResultFromThrown);
       }
     },
   );

packages/mcp/src/mcp-run-action-stale-summary.test.ts

@@ -0,0 +1,128 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#28856] The MCP `run_action` tool answers an action whose write the engine
+ * STORED but whose parent roll-up it could not recompute with the action's
+ * own answer — never the refusal, which told an agent the action failed and
+ * invited it to run the action again, repeating every side effect.
+ *
+ * ## The defect, measured on `origin/main` (`3b920f6d`) before this file
+ *
+ * The runtime bridge's `runAction` threw the engine's `ERR_SUMMARY_RECOMPUTE`
+ * refusal and the tool rendered it as an `isError` result. The runtime now
+ * composes the action's `run_action` answer and attaches it to the refusal
+ * through #28781's carrier (`invokeBusinessAction`, pinned by
+ * `packages/runtime/src/action-declarative-update-stale-summary.test.ts`);
+ * this file pins the tool's half: it relays that answer.
+ *
+ * ## What is pinned (over the HTTP `/mcp` transport)
+ *
+ * - the attached answer is the tool's result, not an error;
+ * - a refusal before the write keeps its existing rendering, `details` and all;
+ * - a stale refusal with NO answer attached (an action kind whose result the
+ *   runtime cannot compose, #28861) still answers its error;
+ * - the stale column is logged once, at `error`.
+ */
+
+import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
+import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts';
+import { withEarnedAnswer } from '@objectstack/types';
+
+import { MCPServerRuntime } from './mcp-server-runtime.js';
+import type { McpActionBridge, McpDataBridge } from './mcp-http-tools.js';
+
+type Bridge = McpDataBridge & McpActionBridge;
+
+const OBJECT = 'stl_case';
+const RECORD_ID = 'case_1';
+const ACTION = 'close_case';
+
+/** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */
+function staleRefusal(): Error {
+  return Object.assign(new Error('Roll-up summary recompute failed after retries for 1 parent record(s); the triggering records WERE written.'), {
+    code: 'ERR_SUMMARY_RECOMPUTE',
+    written: { id: RECORD_ID },
+    failures: [{ childObject: OBJECT, parentObject: 'stl_account', parentId: 'a1', field: 'open_cases', error: new Error('sum over text') }],
+  });
+}
+
+/** The runtime's answer for a declarative update action (`invokeBusinessAction`). */
+const ANSWER = {
+  ok: true,
+  action: ACTION,
+  objectName: OBJECT,
+  recordId: RECORD_ID,
+  result: { operation: 'update', object: OBJECT, id: RECORD_ID, record: { id: RECORD_ID, status: 'closed' } },
+};
+
+function bridgeThrowing(thrown: () => Error): Bridge {
+  return {
+    async listObjects() { return [{ name: OBJECT, label: 'Case', fieldCount: 1 }]; },
+    async describeObject() { return { name: OBJECT, fields: [{ name: 'status', type: 'text' }] }; },
+    async query() { return { object: OBJECT, records: [], total: 0 }; },
+    async get(_object, id) { return { id, status: 'open' }; },
+    async create() { return {}; },
+    async update() { return {}; },
+    async remove() { return {}; },
+    async listActions() {
+      return [{ name: ACTION, objectName: OBJECT, type: 'script', requiresRecord: true, requiresConfirmation: false }];
+    },
+    async runAction() { throw thrown(); },
+    async resumeRun() { return {}; },
+  };
+}
+
+let nextId = 1;
+async function runAction(bridge: Bridge) {
+  const body = {
+    jsonrpc: '2.0', id: nextId++, method: 'tools/call',
+    params: { name: 'run_action', arguments: { actionName: ACTION, objectName: OBJECT, recordId: RECORD_ID } },
+  };
+  const runtime = new MCPServerRuntime({ name: 'pin', version: '0.0.0' });
+  const res = await runtime.handleHttpRequest(
+    new Request('http://localhost/api/v1/mcp', {
+      method: 'POST',
+      headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
+      body: JSON.stringify(body),
+    }),
+    { bridge, parsedBody: body },
+  );
+  expect(res.status).toBe(200);
+  const json = (await res.json()) as { result?: any; error?: unknown };
+  expect(json.error, JSON.stringify(json.error)).toBeUndefined();
+  return json.result as { content: Array<{ text: string }>; isError?: boolean };
+}
+
+let errorLog: ReturnType<typeof vi.spyOn>;
+beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); });
+afterAll(() => { errorLog.mockRestore(); });
+
+describe('[#28856] MCP run_action answers an action whose write landed under a failing roll-up as the action it is', () => {
+  it('relays the answer the runtime attached to the refusal', async () => {
+    const result = await runAction(bridgeThrowing(() => withEarnedAnswer(staleRefusal() as any, ANSWER)));
+    expect(result.isError, result.content[0]?.text).toBeFalsy();
+    expect(JSON.parse(result.content[0]!.text)).toEqual(ANSWER);
+  });
+
+  it('a refusal before the write keeps its rendering, details included', async () => {
+    const result = await runAction(bridgeThrowing(() => Object.assign(
+      new Error(`Action '${ACTION}' on '${OBJECT}' declares ai.requiresConfirmation: true — nothing was run.`),
+      { code: 'ACTION_CONFIRMATION_REQUIRED', status: 428, details: { actionName: ACTION, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER } },
+    )));
+    expect(result.isError).toBe(true);
+    const { error } = JSON.parse(result.content[0]!.text);
+    expect(error).toMatchObject({ code: 'ACTION_CONFIRMATION_REQUIRED', status: 428, details: { actionName: ACTION } });
+  });
+
+  it('a stale refusal with no answer attached still answers its error', async () => {
+    const result = await runAction(bridgeThrowing(staleRefusal));
+    expect(result.isError).toBe(true);
+    expect(JSON.parse(result.content[0]!.text).error.code).toBe('ERR_SUMMARY_RECOMPUTE');
+  });
+
+  it('the stale column is logged once, at error', () => {
+    const lines = errorLog.mock.calls.map((c: unknown[]) => String(c[0])).filter((l: string) => l.includes('could not be recomputed'));
+    expect(lines).toHaveLength(1);
+    expect(lines[0]).toContain('"stl_account.open_cases"');
+  });
+});

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

@@ -0,0 +1,189 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#28856] A declarative update action (`operation: 'update'`) whose write the
+ * engine STORED but whose parent roll-up it could not recompute answers the
+ * action it is, at both action doors — never the refusal, which told a caller
+ * the action failed and invited it to run the action again.
+ *
+ * ## The defect, measured on `origin/main` (`3b920f6d`) before this file
+ *
+ * The engine reports that case as `ERR_SUMMARY_RECOMPUTE` ("the triggering
+ * records WERE written", carrying `written`). The action's one write goes
+ * through `callData('update')`, whose producers (the protocol's `updateData`,
+ * and the ObjectQL fallback this rig runs) attach the WRITE's answer since
+ * #28781 / #28782. `executeDeclarativeUpdateAction` never composed the
+ * action's result from it, and neither door read the carrier:
+ *
+ * - REST `POST /actions/:object/:action` answered `500` with the refusal;
+ * - the MCP bridge's `runAction` threw it, and the audit row said `failed`.
+ *
+ * ## What is pinned
+ *
+ * - REST answers `200` with the declarative result it answers when the
+ *   recompute succeeds, and the audit row says `succeeded`;
+ * - the MCP bridge's `runAction` throws the refusal carrying the action's
+ *   `run_action` answer (`{ ok, action, objectName, recordId, result }`) for
+ *   the tool to relay, and the audit row says `succeeded`;
+ * - the update is applied exactly once at each door;
+ * - a refusal before the write still answers exactly as before (ruling 5);
+ * - the stale column is logged once, at `error`, by the REST door.
+ *
+ * The engine is a double whose `update` applies the write and then throws the
+ * refusal as objectql's `SummaryRecomputeError` does; the real engine's throw
+ * is pinned by #28781's REST suite.
+ */
+
+import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
+import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
+import { earnedAnswerOf } from '@objectstack/types';
+
+import { HttpDispatcher } from './http-dispatcher.js';
+
+const OBJECT = 'stl_case';
+const RECORD = 'case_1';
+const CLOSE = {
+    name: 'close_case', label: 'Close', objectName: OBJECT, operation: 'update',
+    patch: { status: 'closed' }, undoable: true, ai: { exposed: true, description: 'Close the case.' },
+};
+
+/** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */
+function staleRefusal(written: unknown): Error {
+    return Object.assign(new Error('Roll-up summary recompute failed after retries for 1 parent record(s); the triggering records WERE written.'), {
+        code: 'ERR_SUMMARY_RECOMPUTE',
+        written,
+        failures: [{ childObject: OBJECT, parentObject: 'stl_account', parentId: 'a1', field: 'open_cases', error: new Error('sum over text') }],
+    });
+}
+
+/**
+ * The dispatcher over an engine whose `update` applies the write, then
+ * reports the stale roll-up — or refuses before writing anything when the
+ * caller is `u_reader`.
+ */
+function makeRig() {
+    const events: any[] = [];
+    const audit = { recordActionInvocation: vi.fn(async (e: any) => { events.push(e); }) };
+    const row: Record<string, unknown> = { id: RECORD, status: 'open', owner: 'u_member' };
+    const writes: string[] = [];
+
+    const objectDef = {
+        name: OBJECT,
+        fields: { status: { type: 'text' }, owner: { type: 'text' } },
+        actions: [CLOSE],
+    };
+    const schemaOf = (n: string) => (n === OBJECT ? objectDef : undefined);
+    const ql: any = {
+        executeAction: vi.fn(async (object: string, key: string) => {
+            throw new Error(`Action '${key}' on object '${object}' not found`);
+        }),
+        getSchema: schemaOf,
+        registry: { getObject: schemaOf, getItem: () => undefined },
+        find: vi.fn(async (_object: string, q: any) => {
+            const rows = q?.where?.id === RECORD ? [{ ...row }] : [];
+            return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows;
+        }),
+        update: vi.fn(async (object: string, data: any, options?: any) => {
+            assertEngineUpdateDispatch(data, options);
+            if (options?.context?.userId === 'u_reader') {
+                throw Object.assign(new Error(`You do not have permission to update ${object}`), { code: 'PERMISSION_DENIED', status: 403 });
+            }
+            Object.assign(row, data);
+            writes.push(`update:${options?.where?.id}`);
+            throw staleRefusal({ ...row });
+        }),
+        insert: vi.fn(),
+        delete: vi.fn(),
+    };
+    const metadata: any = {
+        load: vi.fn(async () => null),
+        loadDiagnosed: vi.fn(async () => ({ data: null, degraded: false, errors: [] })),
+        loadMany: vi.fn(async () => []),
+        listObjects: vi.fn(async () => [objectDef]),
+        getObject: vi.fn(async (n: string) => schemaOf(n)),
+    };
+    const services: Record<string, unknown> = { objectql: ql, data: ql, metadata, audit };
+    const resolve = (n: string) => services[n] ?? null;
+    const kernel: any = { getService: resolve, getServiceAsync: async (n: string) => resolve(n), context: { getService: resolve } };
+    const dispatcher = new HttpDispatcher(kernel);
+
+    const ctx = (userId: string): any => ({
+        request: { headers: { host: 'app.example.test' } },
+        environmentId: 'platform',
+        executionContext: { userId, tenantId: 'org_1', systemPermissions: [] },
+    });
+
+    /** REST `POST /actions/:object/:action/:id` — the console's click, the SDK's call. */
+    const viaRest = async (userId = 'u_member') => {
+        const res: any = await dispatcher.handleActions(`/${OBJECT}/${CLOSE.name}/${RECORD}`, 'POST', {}, ctx(userId));
+        return res.response;
+    };
+    /** The real `/mcp` bridge's `runAction` — what the `run_action` tool calls. */
+    const viaMcp = async (userId = 'u_member') => {
+        const bridge = (dispatcher as any).buildMcpBridge(ctx(userId));
+        try {
+            return { value: await bridge.runAction(CLOSE.name, { objectName: OBJECT, recordId: RECORD }) };
+        } catch (err: any) {
+            return { err };
+        }
+    };
+    return { events, row, writes, viaRest, viaMcp };
+}
+
+const DECLARATIVE_RESULT = {
+    operation: 'update',
+    object: OBJECT,
+    id: RECORD,
+    record: { id: RECORD, status: 'closed', owner: 'u_member' },
+    undo: { type: 'update', objectName: OBJECT, recordId: RECORD, undoData: { status: 'open' }, redoData: { status: 'closed' } },
+};
+
+let errorLog: ReturnType<typeof vi.spyOn>;
+beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); });
+afterAll(() => { errorLog.mockRestore(); });
+
+describe('[#28856] a declarative update action whose write landed under a failing roll-up answers the action it is', () => {
+    it('REST action door: 200 with the declarative result, the update applied once, audit row succeeded', async () => {
+        const rig = makeRig();
+        const res = await rig.viaRest();
+        expect(res.status, JSON.stringify(res.body)).toBe(200);
+        expect(res.body.success).toBe(true);
+        expect(res.body.data).toEqual(DECLARATIVE_RESULT);
+        expect(rig.writes).toEqual([`update:${RECORD}`]);
+        expect(rig.row.status).toBe('closed');
+        expect(rig.events).toHaveLength(1);
+        expect(rig.events[0]).toMatchObject({ actionName: CLOSE.name, outcome: 'succeeded' });
+        expect(rig.events[0]).not.toHaveProperty('status');
+        expect(rig.events[0]).not.toHaveProperty('errorCode');
+    });
+
+    it('MCP bridge runAction: the refusal carries the run_action answer, the update applied once, audit row succeeded', async () => {
+        const rig = makeRig();
+        const out = await rig.viaMcp();
+        expect(out.value).toBeUndefined();
+        expect(earnedAnswerOf(out.err)).toEqual({ ok: true, action: CLOSE.name, objectName: OBJECT, recordId: RECORD, result: DECLARATIVE_RESULT });
+        expect(rig.writes).toEqual([`update:${RECORD}`]);
+        expect(rig.events).toHaveLength(1);
+        expect(rig.events[0]).toMatchObject({ actionName: CLOSE.name, outcome: 'succeeded' });
+        expect(rig.events[0]).not.toHaveProperty('errorCode');
+    });
+
+    it('a refusal before the write answers exactly as before at both doors, nothing written, audit row failed', async () => {
+        const rig = makeRig();
+        const res = await rig.viaRest('u_reader');
+        expect(res.status).toBe(403);
+        expect(res.body.error.code).toBe('PERMISSION_DENIED');
+        const out = await rig.viaMcp('u_reader');
+        expect(out.err?.code).toBe('PERMISSION_DENIED');
+        expect(earnedAnswerOf(out.err)).toBeUndefined();
+        expect(rig.writes).toEqual([]);
+        expect(rig.row.status).toBe('open');
+        expect(rig.events.map((e) => e.outcome)).toEqual(['failed', 'failed']);
+    });
+
+    it('the stale column is logged once, at error', () => {
+        const lines = errorLog.mock.calls.map((c: unknown[]) => String(c[0])).filter((l: string) => l.includes('could not be recomputed'));
+        expect(lines).toHaveLength(1);
+        expect(lines[0]).toContain('"stl_account.open_cases"');
+    });
+});