diff --git a/.changeset/28856-action-stale-summary-answer.md b/.changeset/28856-action-stale-summary-answer.md index d07051f5bf9..f6f063c465f 100644 --- a/.changeset/28856-action-stale-summary-answer.md +++ b/.changeset/28856-action-stale-summary-answer.md @@ -15,4 +15,4 @@ When a declarative update action (`operation: 'update'`) writes its record but a - **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. +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: an `api` action's forward already lands on a data door that answers success, and a `flow` action's record nodes and a script or body action's own writes answer a landed write's success since #28861. diff --git a/.changeset/28861-body-write-stale-summary-answer.md b/.changeset/28861-body-write-stale-summary-answer.md new file mode 100644 index 00000000000..a46ba5559bb --- /dev/null +++ b/.changeset/28861-body-write-stale-summary-answer.md @@ -0,0 +1,20 @@ +--- +'@objectstack/runtime': patch +'@objectstack/service-automation': patch +--- + +fix(runtime,service-automation): a write in an action, hook or job body, or a flow record node, that landed under a failing roll-up answers its own success (#28861) + +Clause-②: yes (widening) +Metadata-protocol: none + +When a child write is stored but a parent's roll-up summary cannot be recomputed after it (`ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written"), the data doors and MCP tools already answer the write's success. The write surfaces an author's code holds now do the same, so the code's next line runs instead of stopping on a write that landed. Each answers exactly what the same write answers when every roll-up recomputes — no new code, member or shape: + +- **`ctx.engine`** in an action handler: `insert` resolves `{ id }`, `update` and `delete` resolve. A multi-id `delete` deletes every id; a stale roll-up after one id no longer leaves the later ids undeleted. +- **`ctx.api`** in an action handler, and every context it derives (`sudo()`, `withRunAs(...)`, a `transaction(fn)` callback's context): each repository write verb resolves with the engine's result. +- **A sandboxed hook, action or job body's `ctx.api.object(...)`**: the write resolves in the body with the engine's result, so a hook no longer refuses the write that fired it, an action body returns its own value, and a job run reports its own outcome. +- **Flow `create_record`, `update_record` and `delete_record`**: the node succeeds with the output it gives on a clean write (`output`, `outputVariable`, `droppedFields`, `acted`), and the run goes on to its next node. + +The stale summary column is logged at `error` once per field per process, with the fix, as on the data doors. A refusal raised before the write is stored (validation, permission, constraint) reaches the author's code exactly as before. No transaction is added: a body's earlier writes stay committed, as they always did. Inside a transaction the body opens itself (`ctx.api.transaction(fn)`), such a write now answers its success in the callback too, so that transaction commits where the refusal used to roll it back. + +The REST and MCP action doors answer a refusal carrying a write's attached answer as the action's success only for a declarative update action, whose refusal carries the action's own result. Any other action that throws such a refusal is answered as the failure it is, and its invocation audit row records `failed`. diff --git a/packages/runtime/src/action-body-write-stale-summary.test.ts b/packages/runtime/src/action-body-write-stale-summary.test.ts new file mode 100644 index 00000000000..51e701b0236 --- /dev/null +++ b/packages/runtime/src/action-body-write-stale-summary.test.ts @@ -0,0 +1,292 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28861] A write an action body makes through `ctx.engine` or `ctx.api` that + * the engine STORED, but whose parent roll-up summary it could not recompute + * after it, answers the write's own success in the body — never the + * refusal, which stopped the body's later steps and made the action door + * report a failure for a write that had landed (the caller then retried). + * + * ## The defect, measured on `origin/main` (`2af0ae2a`) before this file + * + * The engine reports that case as objectql's `SummaryRecomputeError` + * (`ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written", carrying + * `written`). `buildActionEngineFacade` (`ctx.engine`) and `buildActionApi` + * (`ctx.api`, objectql's `createContext`) handed it straight into the body, + * so a body's next line never ran, the action had no result, and both doors + * answered the throw: REST a `500`, the MCP bridge a throw with the audit row + * `failed`. + * + * ## What is pinned (seat ruling 6570987196) + * + * - each write surface returns what the call returns when every roll-up + * recomputes: `ctx.engine.insert` its `{ id }`, `update` and `delete` + * resolve, a multi-id `delete` deletes every id, and `ctx.api.object(x)`'s + * verbs answer the engine's result — so the body runs on and the door + * answers the body's OWN result, the audit row `succeeded`; + * - the stale column is logged once per field, at `error`; + * - a refusal raised before the write is stored reaches the body exactly as + * before (NEGATIVE PIN); + * - the doors read a write's attached answer only on the declarative-update + * arm (addendum 6567869052 point 2): a body that throws a refusal carrying + * a write's data answer is answered as the failure it is — never with that + * data as the action's result — at REST, on the MCP bridge and in the audit + * row (NEGATIVE PIN). + * + * The engine is a double whose writes apply and then throw objectql's own + * `SummaryRecomputeError`; `ctx.api` is objectql's real `ScopedContext` over it. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ScopedContext, SummaryRecomputeError } from '@objectstack/objectql'; +import { earnedAnswerOf, withEarnedAnswer } from '@objectstack/types'; + +import { HttpDispatcher } from './http-dispatcher.js'; + +const OBJECT = 'stv_account'; +const CHILD = 'stv_visit'; +const ACTION = { + name: 'log_visit', label: 'Log visit', objectName: OBJECT, type: 'script', target: 'logVisit', + ai: { exposed: true, description: 'Log a visit.' }, +}; + +/** One parent whose roll-up could not be recomputed, as the engine names it. */ +const FAILURES = [{ childObject: CHILD, parentObject: OBJECT, parentId: 'a1', field: 'visit_count', error: new Error('sum over text') }]; + +type Handler = (ctx: any) => Promise; + +/** + * The dispatcher over an engine whose `insert` / `update` / `delete` apply the + * write, then report the stale roll-up — or refuse before writing anything + * when the payload or the id says `refused`. `handler` is the action body. + */ +function makeRig(handler: Handler) { + const events: any[] = []; + const audit = { recordActionInvocation: vi.fn(async (e: any) => { events.push(e); }) }; + const rows = new Map>([['v0', { id: 'v0', title: 'old' }]]); + const writes: string[] = []; + const refuse = (object: string) => + Object.assign(new Error(`You do not have permission to write ${object}`), { code: 'PERMISSION_DENIED', status: 403 }); + + const objectDef = { name: OBJECT, fields: { name: { type: 'text' } }, actions: [ACTION] }; + const schemaOf = (n: string) => (n === OBJECT ? objectDef : undefined); + const ql: any = { + executeAction: vi.fn(async (object: string, key: string, ctx: any) => { + if (object === OBJECT && key === ACTION.target) return handler(ctx); + throw new Error(`Action '${key}' on object '${object}' not found`); + }), + getSchema: schemaOf, + registry: { getObject: schemaOf, getItem: () => undefined }, + find: vi.fn(async () => []), + insert: vi.fn(async (object: string, data: any) => { + if (data?.title === 'refused') throw refuse(object); + const row = { ...data }; + rows.set(String(data.id), row); + writes.push(`insert:${data.id}`); + throw new SummaryRecomputeError(FAILURES as any, { ...row }); + }), + update: vi.fn(async (object: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + const id = String(options?.where?.id); + if (id === 'refused') throw refuse(object); + const row = { ...(rows.get(id) ?? { id }), ...data }; + rows.set(id, row); + writes.push(`update:${id}`); + throw new SummaryRecomputeError(FAILURES as any, { ...row }); + }), + delete: vi.fn(async (object: string, options?: any) => { + assertEngineDeleteDispatch(options); + const id = String(options?.where?.id); + if (id === 'refused') throw refuse(object); + rows.delete(id); + writes.push(`delete:${id}`); + throw new SummaryRecomputeError(FAILURES as any, { id }); + }), + createContext: (ec: any) => new ScopedContext(ec, ql), + }; + 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 = { 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 = (): any => ({ + request: { headers: { host: 'app.example.test' } }, + environmentId: 'platform', + executionContext: { userId: 'u_member', tenantId: 'org_1', systemPermissions: [] }, + }); + + /** REST `POST /actions/:object/:action` — the console's click, the SDK's call. */ + const viaRest = async () => { + const res: any = await dispatcher.handleActions(`/${OBJECT}/${ACTION.name}`, 'POST', {}, ctx()); + return res.response; + }; + /** The real `/mcp` bridge's `runAction` — what the `run_action` tool calls. */ + const viaMcp = async () => { + const bridge = (dispatcher as any).buildMcpBridge(ctx()); + try { + return { value: await bridge.runAction(ACTION.name, { objectName: OBJECT }) }; + } catch (err: any) { + return { err }; + } + }; + return { events, rows, writes, viaRest, viaMcp }; +} + +/** Both doors over one body: the REST answer, the MCP answer, and what each wrote. */ +async function viaBothDoors(handler: Handler) { + const rest = makeRig(handler); + const restRes = await rest.viaRest(); + const mcp = makeRig(handler); + const mcpOut = await mcp.viaMcp(); + return { rest, restRes, mcp, mcpOut }; +} + +/** The body's own result answered at both doors, every write applied once, both audit rows `succeeded`. */ +function expectBodyResultAnswered(run: Awaited>, result: unknown, writes: string[]) { + expect(run.restRes.status, JSON.stringify(run.restRes.body)).toBe(200); + expect(run.restRes.body.data).toEqual(result); + expect(run.rest.writes).toEqual(writes); + expect(run.mcpOut.err, String(run.mcpOut.err)).toBeUndefined(); + expect(run.mcpOut.value).toEqual({ ok: true, action: ACTION.name, objectName: OBJECT, result }); + expect(run.mcp.writes).toEqual(writes); + for (const events of [run.rest.events, run.mcp.events]) { + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ actionName: ACTION.name, outcome: 'succeeded' }); + } +} + +let errorLog: ReturnType; +beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterAll(() => { errorLog.mockRestore(); }); + +describe('[#28861] ctx.engine: a write that landed under a failing roll-up answers its success in the body', () => { + it('insert answers the new id, and the body runs on to its own result', async () => { + const run = await viaBothDoors(async (ctx) => { + const created = await ctx.engine.insert(CHILD, { id: 'v1', title: 'visit' }); + return { created, after: 'insert' }; + }); + expectBodyResultAnswered(run, { created: { id: 'v1' }, after: 'insert' }, ['insert:v1']); + }); + + it('update resolves, and the body runs on to its own result', async () => { + const run = await viaBothDoors(async (ctx) => { + await ctx.engine.update(CHILD, 'v0', { title: 'revisited' }); + return { after: 'update' }; + }); + expectBodyResultAnswered(run, { after: 'update' }, ['update:v0']); + expect(run.rest.rows.get('v0')).toEqual({ id: 'v0', title: 'revisited' }); + }); + + it('a multi-id delete deletes every id, each landed delete answered as the success it is', async () => { + const run = await viaBothDoors(async (ctx) => { + await ctx.engine.delete(CHILD, ['v0', 'v1']); + return { after: 'delete' }; + }); + expectBodyResultAnswered(run, { after: 'delete' }, ['delete:v0', 'delete:v1']); + }); +}); + +describe('[#28861] ctx.api: a write that landed under a failing roll-up answers its success in the body', () => { + it('object(x).insert answers the stored record, and the body runs on to its own result', async () => { + const run = await viaBothDoors(async (ctx) => { + const created = await ctx.api.object(CHILD).insert({ id: 'v2', title: 'via api' }); + return { created, after: 'insert' }; + }); + expectBodyResultAnswered(run, { created: { id: 'v2', title: 'via api' }, after: 'insert' }, ['insert:v2']); + }); + + it('object(x).update and object(x).delete answer what the engine answers, and the body runs on', async () => { + const run = await viaBothDoors(async (ctx) => { + const updated = await ctx.api.object(CHILD).update({ title: 'via api' }, { where: { id: 'v0' } }); + const deleted = await ctx.api.object(CHILD).deleteById('v0'); + return { updated, deleted, after: 'delete' }; + }); + expectBodyResultAnswered( + run, + { updated: { id: 'v0', title: 'via api' }, deleted: { id: 'v0' }, after: 'delete' }, + ['update:v0', 'delete:v0'], + ); + }); + + it('a context the api derives answers the same way (sudo)', async () => { + const run = await viaBothDoors(async (ctx) => { + const created = await ctx.api.sudo().object(CHILD).insert({ id: 'v3', title: 'elevated' }); + return { created }; + }); + expectBodyResultAnswered(run, { created: { id: 'v3', title: 'elevated' } }, ['insert:v3']); + }); +}); + +describe('[#28861] a refusal raised before the write is stored reaches the body exactly as before (NEGATIVE PIN)', () => { + it.each([ + ['ctx.engine.insert', (ctx: any) => ctx.engine.insert(CHILD, { id: 'x', title: 'refused' })], + ['ctx.engine.update', (ctx: any) => ctx.engine.update(CHILD, 'refused', { title: 'x' })], + ['ctx.engine.delete', (ctx: any) => ctx.engine.delete(CHILD, 'refused')], + ['ctx.api insert', (ctx: any) => ctx.api.object(CHILD).insert({ id: 'x', title: 'refused' })], + ['ctx.api deleteById', (ctx: any) => ctx.api.object(CHILD).deleteById('refused')], + ])('%s: the body is handed the refusal, the doors answer it, nothing written', async (_label, write) => { + const seen: string[] = []; + const run = await viaBothDoors(async (ctx) => { + try { + await write(ctx); + } catch (e: any) { + seen.push(e?.code); + throw e; + } + return { never: true }; + }); + expect(seen).toEqual(['PERMISSION_DENIED', 'PERMISSION_DENIED']); + expect(run.restRes.status).toBe(403); + expect(run.restRes.body.error.code).toBe('PERMISSION_DENIED'); + expect(run.mcpOut.err?.code).toBe('PERMISSION_DENIED'); + expect(run.rest.writes).toEqual([]); + expect(run.mcp.writes).toEqual([]); + expect(run.mcp.events.map((e) => e.outcome)).toEqual(['failed']); + }); +}); + +describe('[#28861] the doors read a write\'s attached answer on the declarative arm only (NEGATIVE PIN)', () => { + /** A body that throws a refusal carrying a WRITE's data answer — never the action's own result. */ + const carrying = () => withEarnedAnswer( + new SummaryRecomputeError(FAILURES as any, { id: 'v9' }), + { object: CHILD, id: 'v9', record: { id: 'v9', title: 'a write answer' } }, + ); + /** The same refusal with nothing attached: what the doors answer for a body's failure. */ + const bare = () => new SummaryRecomputeError(FAILURES as any, { id: 'v9' }); + + it('REST answers the body\'s failure, never the attached write answer as the action\'s result', async () => { + const carried = await viaBothDoors(async () => { throw carrying(); }); + const control = await viaBothDoors(async () => { throw bare(); }); + expect(control.restRes.status).not.toBe(200); + expect(carried.restRes.status).toBe(control.restRes.status); + expect(carried.restRes.body).toEqual(control.restRes.body); + expect(JSON.stringify(carried.restRes.body)).not.toContain('a write answer'); + expect(carried.rest.events.map((e) => e.outcome)).toEqual(control.rest.events.map((e) => e.outcome)); + }); + + it('the MCP bridge throws the refusal with no answer attached, and the audit row says failed', async () => { + const carried = await viaBothDoors(async () => { throw carrying(); }); + expect(carried.mcpOut.value).toBeUndefined(); + expect(carried.mcpOut.err?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(earnedAnswerOf(carried.mcpOut.err)).toBeUndefined(); + expect(carried.mcp.events).toHaveLength(1); + expect(carried.mcp.events[0]).toMatchObject({ actionName: ACTION.name, outcome: 'failed' }); + }); +}); + +describe('[#28861] the stale column is said, at error', () => { + it('once per field per process', () => { + 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(`"${OBJECT}.visit_count"`); + expect(lines[0]).toContain(`"${CHILD}"`); + }); +}); diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 433ded2f66e..cde3af269d0 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -88,8 +88,11 @@ import { reconcileActionRegistrations as reconcileActionRegistrationsPure, resolveActionHandlerKeys, standaloneActionOwnerKey, + SummaryRecomputeError, } from '@objectstack/objectql'; import { earnedAnswerOf, staleSummaryRefusalOrThrow, withEarnedAnswer, type SummaryRecomputeRefusal } from '@objectstack/types'; +// [#28861] A landed write answers its success to the action handler's code. +import { answerLandedWritesDespiteStaleSummary, landedWriteAnswer } from './landed-write-answer.js'; // [ADR-0110] The addressing vocabulary and the D5 reconciliation moved to // @objectstack/objectql (the engine owns the map they describe, and its @@ -3191,23 +3194,35 @@ export function buildActionExecutionContext(ec: any): Record { * hash keyed) and never the stored row. [#21594] An action BODY's API is built * over this one by the sandbox (`buildSandboxApi`), whose body layers refuse a * family read or write before it reaches this seam. + * + * [#28861] A write through it that the engine stored, whose parent roll-up + * could not be recomputed after it, answers the write's success to the + * handler (`landed-write-answer.ts`), on this context and every one it + * derives. That layer sits beneath the reader seam, so the answer is served + * like any write's return. */ export function buildActionApi(_deps: ActionExecutionDeps, ql: any, ec: any): any | undefined { if (!ql || typeof ql.createContext !== 'function') return undefined; + const served = (scoped: unknown) => + serveStoredMetadataReadsThrough(answerLandedWritesDespiteStaleSummary(scoped, ACTION_API_SURFACE), ql); try { - return serveStoredMetadataReadsThrough(ql.createContext(buildActionExecutionContext(ec)), ql); + return served(ql.createContext(buildActionExecutionContext(ec))); } catch { // A malformed caller envelope must not sink the action — fall back to // the bare elevated context (the same shape hooks default to), marked // as the body's elevation like every other (#25653). try { - return serveStoredMetadataReadsThrough(ql.createContext(buildActionExecutionContext(undefined)), ql); + return served(ql.createContext(buildActionExecutionContext(undefined))); } catch { return undefined; } } } +/** [#28861] The surfaces the stale-column line names, for an action handler's two data handles. */ +const ACTION_API_SURFACE = 'action ctx.api'; +const ACTION_ENGINE_SURFACE = 'action ctx.engine'; + /** * The sentence a caller still on the WITHDRAWN bare-filter shape must read * (#15124). Exported because the wording IS the contract here: this refusal is @@ -3295,17 +3310,31 @@ function assertActionEngineFindEnvelope(object: string, query: Record): Promise<{ id: string }> { - const res = await ql.insert(object, data, { context }); + let res: unknown; + try { + res = await ql.insert(object, data, { context }); + } catch (e) { + res = landedWriteAnswer(e, object, ACTION_ENGINE_SURFACE); + } const id = (res && (res as any).id) ?? (data as any).id; return { id }; }, async update(object: string, id: string, data: Record): Promise { - await ql.update(object, data, { where: { id }, context }); + try { + await ql.update(object, data, { where: { id }, context }); + } catch (e) { + landedWriteAnswer(e, object, ACTION_ENGINE_SURFACE); + } }, // Both spellings are DECLARED contract (#15117), not a tolerance: the // spec's `ActionEngineFacade.delete` takes `string | string[]`. @@ -3324,10 +3353,17 @@ export function buildActionEngineFacade(_deps: ActionExecutionDeps, ql: any, ec? // it pulls the runtime back onto the contract that is already on the // record, here, in the spec member doc, and in the `never a null id` // pin at `packages/spec/src/ui/action-params.test.ts`. + // + // [#28861] Each id's landed delete is answered on its own, so a stale + // roll-up after one id's delete does not leave the later ids undeleted. async delete(object: string, idOrIds: string | string[]): Promise { const ids = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; for (const id of ids) { - await ql.delete(object, { where: { id }, context }); + try { + await ql.delete(object, { where: { id }, context }); + } catch (e) { + landedWriteAnswer(e, object, ACTION_ENGINE_SURFACE); + } } }, // [#15124] The ENVELOPE goes through — this arm no longer builds one. @@ -4098,15 +4134,39 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps, } catch (err) { // [#28856] A refusal carrying the action's answer is answered as the // success it is by the `run_action` tool, so the row records that. - await recordActionInvocation(deps, requestContext, envId, earnedAnswerOf(err) + // + // [#28861] Read on the declarative-update arm only: that arm composes + // the ACTION's answer and attaches it (`dispatchAdmittedBusinessAction`). + // Any other arm's refusal can carry only a WRITE's data answer — a + // handler that let out a refusal a data door attached its answer to — + // and the action that threw has no result. It is answered as the + // failure it is, and leaves this door with nothing attached, so the + // `run_action` tool, which reads every throw, cannot relay it either. + const declarative = isDeclarativeUpdateAction(action); + await recordActionInvocation(deps, requestContext, envId, declarative && earnedAnswerOf(err) ? { ...invocation, outcome: 'succeeded' } : { ...invocation, outcome: 'failed', ...actionInvocationFailure(err) }); - throw err; + throw declarative ? err : withoutWriteAnswer(err); } await recordActionInvocation(deps, requestContext, envId, { ...invocation, outcome: 'succeeded' }); return answer; } +/** + * [#28861] `err` with no write answer attached: a stale-roll-up refusal a data + * door attached its answer to comes back as the same refusal — objectql's + * class, `failures`, `written`, message and stack — with nothing attached; + * every other value is returned as is. For a non-declarative arm's throw, + * whose attached answer is a write's, never the action's. + */ +function withoutWriteAnswer(err: unknown): unknown { + if (earnedAnswerOf(err) === undefined) return err; + const stale = err as SummaryRecomputeRefusal; + const bare = new SummaryRecomputeError([...(stale.failures ?? [])] as SummaryRecomputeError['failures'], stale.written); + if (typeof stale.stack === 'string') bare.stack = stale.stack; + return bare; +} + /** * The MCP `run_action` dispatch, once every admission gate in * {@link invokeBusinessAction} has passed: the declarative update, the diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index 292d9e8f32f..4d16b3135fc 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -1071,7 +1071,14 @@ async function dispatchAdmittedRestAction( // [#28856] An action whose write landed under a failing roll-up answers // the result the runtime composed and attached (`executeDeclarativeUpdateAction`), // as when the recompute succeeds; the stale column is logged once. - const earned = earnedAnswerOf(err); + // + // [#28861] Read on the declarative-update arm only: that arm's refusal + // carries the ACTION's result. Any other arm's can carry only a WRITE's + // data answer — a handler that let out a refusal a data door attached + // its answer to — and the action that threw has no result, so it is + // answered below as the failure it is. (A handler's own `ctx.engine` / + // `ctx.api` writes answer a landed write's success and never throw it.) + const earned = actionExec.isDeclarativeUpdateAction(actionDef) ? earnedAnswerOf(err) : undefined; if (earned) { const log = deps.logger && typeof deps.logger.error === 'function' ? (line: string) => deps.logger.error(`[HttpDispatcher] ${line}`) diff --git a/packages/runtime/src/landed-write-answer.ts b/packages/runtime/src/landed-write-answer.ts new file mode 100644 index 00000000000..b8c89103aaa --- /dev/null +++ b/packages/runtime/src/landed-write-answer.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28861] A write an author's code makes, which the engine STORED, answers its + * own success to that code — even when a parent's roll-up summary could not be + * recomputed after it (seat ruling 6570987196 on #28861). + * + * ## The case + * + * The engine reports it as objectql's `SummaryRecomputeError` + * (`ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written"), carrying as + * `written` the result the call returns when every roll-up recomputes. The + * data doors and the MCP tools answer it as the write's success (#28781, + * #28843, #28854, #28882). The write surfaces an author's code holds handed it + * straight in: the body's next line never ran, its own result never existed, + * and the action, hook or job reported a failure for a write that had landed, + * so a caller that retried wrote it twice. No transaction wraps a body, so its + * earlier writes stayed committed either way. + * + * ## The answer + * + * The `written` result, returned as the call's own answer — exactly what the + * same write answers on a healthy recompute, nothing added — with the stale + * column said once per field per process at `error`, in the line the doors + * log (`staleSummaryAnswerLines`). Every other error, a refusal raised before + * the write was stored among them, is rethrown unchanged. + * + * The surfaces, each built here or calling in here: + * - `ctx.engine` in an action handler (`buildActionEngineFacade`); + * - `ctx.api` in an action handler (`buildActionApi`, objectql's + * `createContext`), and every context it derives; + * - a sandboxed hook, action or job body's `ctx.api`, whatever source the + * sandbox builds it from (`buildSandboxApi`, `sandbox/body-runner.ts`). + * + * The flow record nodes answer the same refusal the same way in + * `@objectstack/service-automation` (`builtin/crud-nodes.ts`). + * + * ⛔ Not in the engine, and not in objectql's `ScopedContext`: a bulk caller + * (the seed loader, the import runner) reads the refusal as "written, with a + * stale summary" and reports that in its own result, and platform code holding + * a `HookContext.api` reads it as before. The ruling covers what an author's + * code holds, so the answer is given where that surface is built. + */ + +import { staleSummaryAnswerLines, staleSummaryRefusalOrThrow } from '@objectstack/types'; + +import { deriveThroughSeam, WRITE_RETURN_VERBS } from './stored-metadata-reader-seam.js'; + +/** + * A write's `catch` on an author-visible surface: the stale-roll-up refusal + * answers the result the write earned (its `written`), the stale column said + * at `error` under `surface`; every other error is rethrown unchanged. + */ +export function landedWriteAnswer(e: unknown, object: string | undefined, surface: string): any { + const stale = staleSummaryRefusalOrThrow(e); + for (const line of staleSummaryAnswerLines(stale, object)) console.error(`[${surface}] ${line}`); + return stale.written; +} + +/** Marks a scoped API whose landed writes are already answered, so a second wrap is a no-op. */ +const LANDED_WRITES_ANSWERED = Symbol.for('objectstack.runtime.landedWritesAnswered'); + +/** One repository, every write verb answering its landed write; every other member handed through. */ +function answerRepositoryLandedWrites(objectName: string, repo: unknown, surface: string): unknown { + if (repo === null || typeof repo !== 'object') return repo; + return new Proxy(repo as Record, { + get(target, prop) { + const value = Reflect.get(target, prop, target); + if (typeof value !== 'function') return value; + if (!WRITE_RETURN_VERBS.has(prop)) return value.bind(target); + return async (...args: unknown[]) => { + try { + return await value.apply(target, args); + } catch (e) { + return landedWriteAnswer(e, objectName, surface); + } + }; + }, + }); +} + +/** + * The scoped data API an author's code holds (`ctx.api`), with every + * repository's write answering a landed write's success + * ({@link landedWriteAnswer}), on `api` and every context it derives + * (`object(name)`, `sudo()`, `withRunAs(...)`, a `transaction(fn)` callback's + * context, the `ctx` `beginTransaction()` returns). Everything else is the same + * object, by delegation. + * + * Wrap it BENEATH the stored-metadata reader seam, so the result it answers is + * served by that seam like any write's return. Idempotent; a value that is not + * an object is returned as is. + */ +export function answerLandedWritesDespiteStaleSummary(api: T, surface: string): T { + return deriveThroughSeam(api, LANDED_WRITES_ANSWERED, (name, repo) => answerRepositoryLandedWrites(name, repo, surface)); +} diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index 7d6a7ccb225..333dac92ac5 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -65,6 +65,7 @@ import { type FormulaServingChannel, } from '@objectstack/objectql'; import { refuseStoredMetadataBodyReads, refuseStoredMetadataBodyWrites } from '../stored-metadata-reader-seam.js'; +import { answerLandedWritesDespiteStaleSummary } from '../landed-write-answer.js'; import { isStoredMetadataBodyObject } from '@objectstack/spec/kernel'; import { isWildcardHookTarget, @@ -1153,10 +1154,19 @@ function buildEngineRepoFacade(ql: any, objectName: string, context?: any) { * body's, and is served by the reader-context seam * (`serveStoredMetadataReadsThrough`) with its writes kept (deployer code, * outside the boundary). + * + * [#28861] Beneath both refusals, every body face's write that the engine + * stored, whose parent roll-up could not be recomputed after it, answers the + * write's success in the VM (`landed-write-answer.ts`), whatever source below + * the API is built from: the body runs on to its own result. An action's + * host-built `api` already answers so (`buildActionApi`); the layer is + * idempotent. */ function buildSandboxApi(engineCtx: any, ql: any, errLabel: string) { return refuseStoredMetadataBodyWrites( - refuseStoredMetadataBodyReads(buildSandboxApiSource(engineCtx, ql, errLabel)), + refuseStoredMetadataBodyReads( + answerLandedWritesDespiteStaleSummary(buildSandboxApiSource(engineCtx, ql, errLabel), errLabel), + ), ); } diff --git a/packages/runtime/src/sandbox/body-write-stale-summary.test.ts b/packages/runtime/src/sandbox/body-write-stale-summary.test.ts new file mode 100644 index 00000000000..9795a1fdb91 --- /dev/null +++ b/packages/runtime/src/sandbox/body-write-stale-summary.test.ts @@ -0,0 +1,155 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28861] A sandboxed body's `ctx.api.object(x)` write that the engine STORED, + * but whose parent roll-up summary it could not recompute after it, answers + * the write's own success inside the VM — for a hook, an action and a job body + * alike, since all three get their API from one place (`buildSandboxApi`). + * + * ## The defect, measured on `origin/main` (`2af0ae2a`) before this file + * + * objectql's `SummaryRecomputeError` (`ERR_SUMMARY_RECOMPUTE`, "the triggering + * records WERE written") crossed into the VM as the write's rejection, so the + * body's next line never ran: a hook refused the write that fired it, an + * action had no result, a job run failed — each for a write that had landed. + * + * ## What is pinned (seat ruling 6570987196) + * + * - the write answers in the VM what the repository answers when every + * roll-up recomputes, and the body runs on to its own result: the hook's + * patch is merged, the action's value returned, the job's outcome read; + * - every source the sandbox builds its API from is covered: the engine's + * hook api, the host's `api` on an action context, and the + * `executionContext` + `createContext` fallback a job body takes; + * - a refusal raised before the write is stored reaches the body exactly as + * before (NEGATIVE PIN). + * + * The repository is objectql's real `ScopedContext` over an engine double whose + * writes apply and then throw objectql's own `SummaryRecomputeError`. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ScopedContext, SummaryRecomputeError } from '@objectstack/objectql'; + +import { actionBodyRunnerFactory, hookBodyRunnerFactory, jobBodyRunnerFactory } from './body-runner.js'; +import { QuickJSScriptRunner } from './quickjs-runner.js'; + +const runner = new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }); +const CHILD = 'stb_line'; +const FAILURES = [{ childObject: CHILD, parentObject: 'stb_order', parentId: 'o1', field: 'line_total', error: new Error('sum over text') }]; +const WRITE_CAPS = ['api.read', 'api.write']; + +/** An engine whose writes apply and then report the stale roll-up, or refuse first on a `refused` payload. */ +function makeEngine() { + const writes: string[] = []; + const refuse = () => Object.assign(new Error('Field "amount" must be a number'), { code: 'VALIDATION_FAILED', status: 400 }); + const engine: any = { + insert: vi.fn(async (_object: string, data: any) => { + if (data?.note === 'refused') throw refuse(); + writes.push(`insert:${data.id}`); + throw new SummaryRecomputeError(FAILURES as any, { ...data }); + }), + update: vi.fn(async (_object: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + writes.push(`update:${options?.where?.id}`); + throw new SummaryRecomputeError(FAILURES as any, { id: options?.where?.id, ...data }); + }), + delete: vi.fn(async (_object: string, options?: any) => { + assertEngineDeleteDispatch(options); + writes.push(`delete:${options?.where?.id}`); + throw new SummaryRecomputeError(FAILURES as any, { id: options?.where?.id }); + }), + find: vi.fn(async () => []), + }; + engine.createContext = (ec: any) => new ScopedContext(ec, engine); + return { engine, writes }; +} + +const SYSTEM = { isSystem: true, userId: 'u_member', tenantId: 'org_1' }; + +let errorLog: ReturnType; +beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterAll(() => { errorLog.mockRestore(); }); + +describe('[#28861] a sandboxed body\'s landed write answers its success in the VM', () => { + it('hook body (the engine\'s hook api): the insert answers the record, the body runs on, its patch is merged', async () => { + const { engine, writes } = makeEngine(); + const fn = hookBodyRunnerFactory(runner, { ql: engine, appId: 'stb' })({ + name: 'mirror_line', object: 'stb_order', events: ['beforeUpdate'], + body: { + language: 'js', + capabilities: WRITE_CAPS, + source: `const line = await ctx.api.object('${CHILD}').insert({ id: 'l1', amount: 5 }); + return { last_line: line.id };`, + }, + } as any)!; + const engineCtx: any = { object: 'stb_order', event: 'beforeUpdate', input: { status: 'open' }, api: new ScopedContext(SYSTEM as any, engine) }; + await fn(engineCtx); + expect(writes).toEqual(['insert:l1']); + expect(engineCtx.input.last_line).toBe('l1'); + }); + + it('action body (the host\'s api): update and delete answer, the body returns its own value', async () => { + const { engine, writes } = makeEngine(); + const fn = actionBodyRunnerFactory(runner, { ql: engine, appId: 'stb' })({ + name: 'close_lines', object: 'stb_order', type: 'script', + body: { + language: 'js', + capabilities: WRITE_CAPS, + source: `const updated = await ctx.api.object('${CHILD}').update({ amount: 0 }, { where: { id: 'l1' } }); + const deleted = await ctx.api.object('${CHILD}').delete({ where: { id: 'l2' } }); + return { updated, deleted, closed: true };`, + }, + })!; + const value = await fn({ params: {}, record: {}, api: new ScopedContext(SYSTEM as any, engine), executionContext: SYSTEM }); + expect(writes).toEqual(['update:l1', 'delete:l2']); + expect(value).toEqual({ updated: { id: 'l1', amount: 0 }, deleted: { id: 'l2' }, closed: true }); + }); + + it('job body (the createContext fallback): the insert answers, the run reports its own outcome', async () => { + const { engine, writes } = makeEngine(); + const fn = jobBodyRunnerFactory(runner, { ql: engine, appId: 'stb' })({ + name: 'nightly_lines', + body: { + language: 'js', + capabilities: WRITE_CAPS, + source: `const line = await ctx.api.object('${CHILD}').insert({ id: 'l3', amount: 1 }); + return { outcome: 'completed', reason: 'wrote ' + line.id };`, + }, + })!; + const outcome = await fn({} as any); + expect(writes).toEqual(['insert:l3']); + expect(outcome).toEqual({ outcome: 'completed', reason: 'wrote l3' }); + }); +}); + +describe('[#28861] a refusal raised before the write is stored reaches the body exactly as before (NEGATIVE PIN)', () => { + it('the body is handed the refusal\'s code and status, nothing written', async () => { + const { engine, writes } = makeEngine(); + const fn = actionBodyRunnerFactory(runner, { ql: engine, appId: 'stb' })({ + name: 'add_line', object: 'stb_order', type: 'script', + body: { + language: 'js', + capabilities: WRITE_CAPS, + source: `try { + await ctx.api.object('${CHILD}').insert({ id: 'l4', note: 'refused' }); + return { landed: true }; + } catch (e) { + return { refusedWith: e.code, status: e.status }; + }`, + }, + })!; + const value = await fn({ params: {}, record: {}, api: new ScopedContext(SYSTEM as any, engine), executionContext: SYSTEM }); + expect(value).toEqual({ refusedWith: 'VALIDATION_FAILED', status: 400 }); + expect(writes).toEqual([]); + }); +}); + +describe('[#28861] the stale column is said, at error', () => { + it('once per field per process', () => { + 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('"stb_order.line_total"'); + }); +}); diff --git a/packages/runtime/src/stored-metadata-reader-seam.ts b/packages/runtime/src/stored-metadata-reader-seam.ts index 76eabe5afe1..bd13aa08549 100644 --- a/packages/runtime/src/stored-metadata-reader-seam.ts +++ b/packages/runtime/src/stored-metadata-reader-seam.ts @@ -229,8 +229,11 @@ async function serveStoredMetadataWriteReturn(object: string, answer: A, engi const ROW_SERVING_READS: ReadonlySet = new Set(['find', 'findOne', 'aggregate']); /** The verb whose answer is a number: guarded against the evaluate oracle, nothing to serve. */ const COUNT_READ: PropertyKey = 'count'; -/** The write verbs whose RETURN can carry family content (every alias ObjectRepository exposes). */ -const WRITE_RETURN_VERBS: ReadonlySet = new Set([ +/** + * The write verbs whose RETURN can carry family content (every alias ObjectRepository exposes). + * [#28861] Also the verbs whose landed write `landed-write-answer.ts` answers: one list of what a write is. + */ +export const WRITE_RETURN_VERBS: ReadonlySet = new Set([ 'insert', 'create', 'update', 'updateById', 'upsert', 'delete', 'deleteById', 'updateMany', 'deleteMany', ]); @@ -401,9 +404,10 @@ export function refuseStoredMetadataBodyReads(api: T): T { * `object(name)`, `sudo()`, `withRunAs(...)`, the context a `transaction(fn)` * callback receives, and the `ctx` `beginTransaction()` returns — with each * repository passed through `wrapRepository`. One definition of what a scoped - * API can derive, so neither layer can leave a route around it. + * API can derive, so neither layer can leave a route around it. [#28861] The + * landed-write layer (`landed-write-answer.ts`) walks it too. */ -function deriveThroughSeam( +export function deriveThroughSeam( api: T, marker: symbol, wrapRepository: (name: string, repo: unknown) => unknown, diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index 66630adad9c..80d214becba 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -37,6 +37,7 @@ import { refuseNode } from '../guard-refusal.js'; import { parseNodeConfig } from './parse-config.js'; import { resolveRunDataContext, stampSystemInsertOwner } from '../runtime-identity.js'; import { refusalCarriedBy } from '../inner-refusal.js'; +import { staleSummaryAnswerLines, staleSummaryRefusalOrThrow } from '@objectstack/types'; /** * Resolve a node's `filter` to the one the query runs (#19939, Fork R17 → A; @@ -171,6 +172,29 @@ function droppedFieldsWarning(nodeType: string, e: DroppedFieldsEvent): string { return `${nodeType}(${e.object}): requested field(s) ${fields} were NOT written — ${why}. The write succeeded without them.`; } +/** + * [#28861] A record node's write that the data engine STORED, but whose parent + * roll-up summary it could not recompute after it, is the node's success + * (seat ruling 6570987196, seat 5's addendum 6567869052): the result the write + * earned, read off the engine's refusal (objectql's `SummaryRecomputeError`, + * `ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written", carrying as + * `written` exactly what the call returns on a healthy recompute). Answering + * it `success: false` failed the run at this node with the write committed — + * no transaction wraps a run — so every later node went unrun and a re-run + * wrote it twice. + * + * The stale column is said once per field per process at `error`, the line + * the data doors log (`staleSummaryAnswerLines`, `@objectstack/types`). Every + * other error, a refusal raised before the write was stored among them, is + * rethrown to the node's own `catch`, which answers it exactly as before. + * The node answers the same output on both paths; nothing is added to it. + */ +function landedWriteResult(e: unknown, nodeType: string, objectName: string, logger: PluginContext['logger']): unknown { + const stale = staleSummaryRefusalOrThrow(e); + for (const line of staleSummaryAnswerLines(stale, objectName)) logger.error(`[${nodeType}] ${line}`); + return stale.written; +} + /** * How many rows an `update` / `delete` actually touched (#4354). * @@ -642,10 +666,16 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // over a column that never landed. Driven end to end in // `create-record-readonly-drop.test.ts`. const dropped: DroppedFieldsEvent[] = []; - const created = await data.insert(objectName, fields, { - context: dataCtx, - onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, - }); + let created: unknown; + try { + created = await data.insert(objectName, fields, { + context: dataCtx, + onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, + }); + } catch (e) { + // [#28861] A landed insert under a failing roll-up is this node's success. + created = landedWriteResult(e, 'create_record', objectName, ctx.logger); + } const createdRecord = Array.isArray(created) ? created[0] : created; const insertedId = createdRecord && typeof createdRecord === 'object' @@ -789,19 +819,25 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): // requested fields never landed instead of reporting a clean // success while the DB truth stayed unchanged. const dropped: DroppedFieldsEvent[] = []; - const result = await data.update(objectName, fields, { - where: filter, - // #5393 — the author's declared bulk intent, forwarded - // to the engine's own word for it. Stated on EVERY call - // rather than spread in when true: `multi: false` is the - // half of the contract that makes the engine refuse a - // predicate update, and a reader of this call should see - // which half was asked for without inferring it from an - // absent key. - multi: cfg.multi === true, - context: dataCtx, - onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, - }); + let result: unknown; + try { + result = await data.update(objectName, fields, { + where: filter, + // #5393 — the author's declared bulk intent, forwarded + // to the engine's own word for it. Stated on EVERY call + // rather than spread in when true: `multi: false` is the + // half of the contract that makes the engine refuse a + // predicate update, and a reader of this call should see + // which half was asked for without inferring it from an + // absent key. + multi: cfg.multi === true, + context: dataCtx, + onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, + }); + } catch (e) { + // [#28861] A landed update under a failing roll-up is this node's success. + result = landedWriteResult(e, 'update_record', objectName, ctx.logger); + } const droppedFields = dropped.flatMap((e) => e.fields); return { success: true, @@ -883,7 +919,13 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): try { // #5393 — `multi` is the author's declaration, forwarded to // the engine's own word for it (see update_record above). - const result = await data.delete(objectName, { where: filter, multi: cfg.multi === true, context: dataCtx }); + let result: unknown; + try { + result = await data.delete(objectName, { where: filter, multi: cfg.multi === true, context: dataCtx }); + } catch (e) { + // [#28861] A landed delete under a failing roll-up is this node's success. + result = landedWriteResult(e, 'delete_record', objectName, ctx.logger); + } return { success: true, output: { result, object: objectName }, diff --git a/packages/services/service-automation/src/builtin/crud-stale-summary.test.ts b/packages/services/service-automation/src/builtin/crud-stale-summary.test.ts new file mode 100644 index 00000000000..b5a444ba4d0 --- /dev/null +++ b/packages/services/service-automation/src/builtin/crud-stale-summary.test.ts @@ -0,0 +1,136 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28861] A flow record (CRUD) node whose write the data engine STORED, but + * whose parent roll-up summary it could not recompute after it, is the node's + * success — never a flow refusal for a write that landed. + * + * ## The defect, measured on `origin/main` (`2af0ae2a`) before this file + * + * `create_record`, `update_record` and `delete_record` folded every throw of + * `data.insert` / `update` / `delete` into `success: false`, objectql's + * `SummaryRecomputeError` (`ERR_SUMMARY_RECOMPUTE`, "the triggering records + * WERE written") among them. No transaction wraps a run, so the write stayed + * committed while the run failed at that node, every later node unrun, and a + * caller that re-ran the flow wrote it again. + * + * ## What is pinned (seat ruling 6570987196, seat 5's addendum 6567869052) + * + * - each node answers the success it answers when every roll-up recomputes: + * the same `output`, `outputVariable`, `droppedFields` and `acted` count, + * read from the result the refusal carries; the run goes on to its end; + * - the stale column is logged once per field per process, at `error`, on the + * node's logger; + * - a refusal raised before the write is stored is still the node's failure, + * exactly as before (NEGATIVE PIN). + */ + +import { describe, it, expect } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { SummaryRecomputeError } from '@objectstack/objectql'; +import { AutomationEngine } from '../engine.js'; +import { registerCrudNodes } from './crud-nodes.js'; + +const CHILD = 'stf_line'; +const FAILURES = [{ childObject: CHILD, parentObject: 'stf_order', parentId: 'o1', field: 'line_total', error: new Error('sum over text') }]; + +function makeLogger(errors: string[]): any { + const l: any = { info() {}, warn() {}, error(msg: unknown) { errors.push(String(msg)); }, debug() {}, trace() {}, fatal() {} }; + l.child = () => l; + return l; +} + +/** A data engine whose writes apply and then report the stale roll-up, or refuse first on a `refused` payload. */ +function makeData() { + const writes: string[] = []; + const data: any = { + async insert(_object: string, fields: any, opts?: any) { + if (fields?.title === 'refused') { + throw Object.assign(new Error('You do not have permission to create stf_line'), { code: 'PERMISSION_DENIED', status: 403 }); + } + writes.push(`insert:${fields.id}`); + opts?.onFieldsDropped?.({ object: CHILD, fields: ['locked'], reason: 'readonly' }); + throw new SummaryRecomputeError(FAILURES as any, { ...fields }); + }, + async update(_object: string, fields: any, opts?: any) { + assertEngineUpdateDispatch(fields, opts); + writes.push(`update:${opts?.where?.id}`); + throw new SummaryRecomputeError(FAILURES as any, { id: opts?.where?.id, ...fields }); + }, + async delete(_object: string, opts?: any) { + assertEngineDeleteDispatch(opts); + writes.push(`delete:${opts?.where?.id}`); + throw new SummaryRecomputeError(FAILURES as any, { id: opts?.where?.id }); + }, + async find() { return []; }, + async findOne(object: string, query?: any) { assertEngineFindOnePredicate(object, query); return null; }, + }; + return { data, writes }; +} + +function makeStack() { + const errors: string[] = []; + const { data, writes } = makeData(); + const engine = new AutomationEngine(makeLogger([])); + registerCrudNodes(engine, { logger: makeLogger(errors), getService: (n: string) => (n === 'data' ? data : undefined) } as any); + return { engine, writes, errors }; +} + +const node = (id: string, type: string, config: Record) => ({ id, type, label: id, config }); + +function registerChain(engine: AutomationEngine, name: string, steps: ReturnType[]) { + const nodes = [{ id: 'start', type: 'start', label: 'Start' }, ...steps, { id: 'end', type: 'end', label: 'End' }]; + engine.registerFlow(name, { + name, label: name, type: 'autolaunched', + nodes, + edges: nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: nodes[i].id, target: n.id })), + } as any); +} + +describe('[#28861] a record node\'s landed write under a failing roll-up is the node\'s success', () => { + it('create_record → update_record → delete_record: each lands, the run reaches its end', async () => { + const { engine, writes, errors } = makeStack(); + registerChain(engine, 'lines', [ + node('mk', 'create_record', { objectName: CHILD, outputVariable: 'line', fields: { id: 'l1', title: 'first' } }), + node('upd', 'update_record', { objectName: CHILD, filter: { id: { dialect: 'cel', source: 'line.id' } }, fields: { title: 'second' } }), + node('del', 'delete_record', { objectName: CHILD, filter: { id: 'l1' } }), + ]); + const res: any = await engine.execute('lines', { userId: 'u1' }); + expect(res.success, JSON.stringify(res)).toBe(true); + expect(writes).toEqual(['insert:l1', 'update:l1', 'delete:l1']); + // The stale column is said once per field per process, at `error`, on + // the node's logger — three stale writes to one field, one line. + const stale = errors.filter((l) => l.includes('could not be recomputed')); + expect(stale).toHaveLength(1); + expect(stale[0]).toContain('"stf_order.line_total"'); + }); + + it('create_record answers the output it answers on a clean write: the record, droppedFields, acted 1', async () => { + const { engine } = makeStack(); + const executor: any = (engine as any).nodeExecutors.get('create_record'); + const variables = new Map(); + const out = await executor.execute( + node('mk', 'create_record', { objectName: CHILD, outputVariable: 'line', fields: { id: 'l2', title: 'x' } }), + variables, + { userId: 'u1' }, + ); + expect(out.success).toBe(true); + expect(out.output).toEqual({ id: 'l2', record: { id: 'l2', title: 'x' }, object: CHILD, droppedFields: ['locked'] }); + expect(out.metrics).toEqual({ acted: 1 }); + expect(variables.get('line')).toEqual({ id: 'l2', title: 'x' }); + }); +}); + +describe('[#28861] a refusal raised before the write is stored is still the node\'s failure (NEGATIVE PIN)', () => { + it('create_record refused: success false with the refusal carried, nothing written, the run fails', async () => { + const { engine, writes } = makeStack(); + registerChain(engine, 'refused', [ + node('mk', 'create_record', { objectName: CHILD, fields: { id: 'l9', title: 'refused' } }), + node('del', 'delete_record', { objectName: CHILD, filter: { id: 'l0' } }), + ]); + const res: any = await engine.execute('refused', { userId: 'u1' }); + expect(res.success).toBe(false); + expect(String(res.error)).toContain('create_record(stf_line) failed'); + expect(writes).toEqual([]); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 0dd27312307..3adde516bc2 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -4586,6 +4586,16 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/runtime/src/action-body-write-stale-summary.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/runtime/src/action-body-write-stale-summary.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/runtime/src/action-declarative-update-stale-summary.test.ts", "verb": "update", @@ -4851,6 +4861,16 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/runtime/src/sandbox/body-write-stale-summary.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/runtime/src/sandbox/body-write-stale-summary.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/runtime/src/security/resolve-execution-context.forward-credential.test.ts", "verb": "findOne", @@ -4926,6 +4946,21 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/services/service-automation/src/builtin/crud-stale-summary.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/services/service-automation/src/builtin/crud-stale-summary.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/services/service-automation/src/builtin/crud-stale-summary.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/services/service-automation/src/builtin/notify-both-channels-unheld-address.integration.test.ts", "verb": "findOne",