From cdd7ca4d3db0f07366221c145b2961043ad06ae0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 01:13:29 +0000 Subject: [PATCH 1/2] fix(objectql, runtime): a converted stale-roll-up refusal keeps its lead and cause through a sandboxed hook, and is hedged under a failed rollback Claude-Session: https://claude.ai/code/session_01JQwxbZhgmEnK3HMysTCHGU Co-authored-by: Claude --- .../29527-converted-refusal-keeps-lead.md | 14 ++ .../src/engine-unlanded-refusal-lead.test.ts | 70 ++++++- packages/objectql/src/index.ts | 3 + packages/objectql/src/summary-errors.ts | 40 +++- ...sandbox-hook-unlanded-refusal-lead.test.ts | 193 ++++++++++++++++++ .../runtime/src/sandbox/quickjs-runner.ts | 16 +- 6 files changed, 327 insertions(+), 9 deletions(-) create mode 100644 .changeset/29527-converted-refusal-keeps-lead.md create mode 100644 packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts diff --git a/.changeset/29527-converted-refusal-keeps-lead.md b/.changeset/29527-converted-refusal-keeps-lead.md new file mode 100644 index 00000000000..e3f9b91005d --- /dev/null +++ b/.changeset/29527-converted-refusal-keeps-lead.md @@ -0,0 +1,14 @@ +--- +'@objectstack/objectql': patch +'@objectstack/runtime': patch +--- + +fix(objectql, runtime): a converted stale-roll-up refusal that a sandboxed hook body lets leave keeps its lead and its `cause`, and one that escapes a transaction whose rollback threw no longer claims a rollback + +Clause-②: no +Metadata-protocol: none + +- A sandboxed hook body whose `ctx.api` write was refused because a nested write's roll-up could not be recomputed (a delete whose cascade stopped, a transaction that rolled back) used to hand that refusal out of the QuickJS runner as a `SandboxError`. The hook-dispatch seam did not recognise it, so the outer write's caller read the hook's own write ("The delete of …") and the original `ERR_SUMMARY_RECOMPUTE` refusal was gone from `cause`. The runner now hands the engine's own refusal back out when the body let it leave unchanged, as it already did for the duplicate-record refusal, so the seam leads with the write the hook is bound to (every hook event, the read events included) and `cause` keeps the original refusal. A body that catches it and throws its own error, or rewrites it first, is relayed as before. +- `ObjectQL.transaction` whose rollback threw now hedges an already-converted refusal too: it leads with "The transaction did not complete, and its rollback failed too", instead of keeping the cascade's or hook's "under a transaction that rolls back nothing was written". +- Status and code are unchanged at every door: `500 INTERNAL_ERROR`. +- `@objectstack/objectql` exports `isUnlandedWriteRefusal(e)`, the predicate the runner asks of a host call's rejection. diff --git a/packages/objectql/src/engine-unlanded-refusal-lead.test.ts b/packages/objectql/src/engine-unlanded-refusal-lead.test.ts index 6eba2c26fd9..538aa48bdf6 100644 --- a/packages/objectql/src/engine-unlanded-refusal-lead.test.ts +++ b/packages/objectql/src/engine-unlanded-refusal-lead.test.ts @@ -17,9 +17,14 @@ * so nothing written inside it was stored". Now, when the rollback threw, * the lead says that nothing is known about what is stored. * + * [#29527] …and that holds for a refusal an earlier conversion already + * made: a cascade-stopped or hook-stopped refusal escaping the transaction + * said "under a transaction that rolls back nothing was written". It is + * re-led too, its `cause` kept. + * * Controls: a delete of the sub itself still answers `ERR_SUMMARY_RECOMPUTE` * with its own result; a transaction whose rollback succeeds still says it - * rolled back. + * rolled back, and leaves a converted refusal exactly as it escaped. * * The driver rolls back for real (a snapshot per transaction), refuses every * write to `bucket` while `refuseBucket` is set (the roll-up recompute's parent @@ -29,7 +34,14 @@ import { describe, it, expect } from 'vitest'; import { ObjectQL } from './engine.js'; -import { SummaryRecomputeError, cascadeStoppedRefusal, hookStoppedRefusal } from './summary-errors.js'; +import { + SummaryRecomputeError, + cascadeStoppedRefusal, + hookStoppedRefusal, + isUnlandedWriteRefusal, + rollbackFailedRefusal, + rolledBackRefusal, +} from './summary-errors.js'; type Row = Record; @@ -222,4 +234,58 @@ describe('ObjectQL.transaction never claims "rolled back" when its rollback thre expect(driver.rolledBack).toHaveLength(1); expect(ids(driver.rowsOf('sub'))).toEqual(['s1']); }); + + it('[#29527] a cascade-stopped refusal escaping it, the rollback throwing: re-led, the clause about a rollback gone', async () => { + const { engine, driver } = await makeEngine(); + driver.state.failRollback = true; + const err = await refusal(engine.transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx }))); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); + expect(err.message.startsWith( + 'The transaction did not complete, and its rollback failed too, so what was written inside it may or may not be stored.', + ), err.message).toBe(true); + expect(err.message).not.toContain('nothing was written'); + expect(err.message).toContain('bucket.sub_count'); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.cause?.failures?.[0]?.childObject).toBe('sub'); + expect(driver.rolledBack).toHaveLength(0); + }); + + it('[#29527] CONTROL the rollback succeeds: the cascade-stopped refusal escapes exactly as it was made', async () => { + const { engine, driver } = await makeEngine(); + const err = await refusal(engine.transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx }))); + expect(err.message.startsWith('The delete of "line" did not complete: its cascade stopped'), err.message).toBe(true); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(driver.rolledBack).toHaveLength(1); + }); + + it('[#29527] every converted refusal is hedged when the rollback threw, and kept when it ran', () => { + const original = new SummaryRecomputeError([{ childObject: 'sub', parentObject: 'bucket', parentId: 'b1', field: 'sub_count', error: null }], true); + for (const converted of [cascadeStoppedRefusal(original, 'line'), hookStoppedRefusal(original, 'afterInsert', 'case', 'cleanup')] as any[]) { + const failed = rollbackFailedRefusal(converted) as any; + expect(failed.message.startsWith('The transaction did not complete, and its rollback failed too'), failed.message).toBe(true); + expect(failed.message).not.toContain('nothing was written'); + expect(failed.cause).toBe(original); + expect(failed.code).toBe('INTERNAL_ERROR'); + expect(failed.status).toBe(500); + expect(rolledBackRefusal(converted)).toBe(converted); + } + const other = new Error('something else'); + expect(rollbackFailedRefusal(other)).toBe(other); + }); +}); + +describe('[#29527] isUnlandedWriteRefusal answers the conversion and nothing else', () => { + it('a conversion, at every depth, is one; the original refusal, a plain error and a look-alike are not', () => { + const original = new SummaryRecomputeError([{ childObject: 'sub', parentObject: 'bucket', parentId: 'b1', field: 'sub_count', error: null }], true); + const inner = cascadeStoppedRefusal(original, 'line'); + for (const e of [inner, cascadeStoppedRefusal(inner, 'acct'), hookStoppedRefusal(inner, 'beforeFind', 'case', 'h'), rollbackFailedRefusal(inner)]) { + expect(isUnlandedWriteRefusal(e)).toBe(true); + } + const lookAlike = Object.assign(new Error((inner as Error).message), { status: 500, code: 'INTERNAL_ERROR', cause: original }); + for (const e of [original, new Error('x'), lookAlike, { ...(inner as object) }, undefined]) { + expect(isUnlandedWriteRefusal(e)).toBe(false); + } + }); }); diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 48b188caeda..c65ba17d008 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -194,6 +194,9 @@ export { RELATION_FILTER_ID_CAP } from './relation-filter-lowering.js'; // instead. The class stays exported exactly as it was — this adds an // affordance, it removes nothing. export { SummaryRecomputeError, SUMMARY_RECOMPUTE_CODE } from './summary-errors.js'; +// [#29527] The QuickJS runner asks it of a host call's rejection, so a sandboxed +// body that lets a converted refusal leave hands back the engine's own error. +export { isUnlandedWriteRefusal } from './summary-errors.js'; export type { SummaryRecomputeFailure } from './summary-errors.js'; // [#5126] Thrown by `update` when `options.strictReadonlyWrites` is set and the // payload would have had read-only fields stripped. Exported so an in-process diff --git a/packages/objectql/src/summary-errors.ts b/packages/objectql/src/summary-errors.ts index cc64c15be75..6cdd4cf14ce 100644 --- a/packages/objectql/src/summary-errors.ts +++ b/packages/objectql/src/summary-errors.ts @@ -108,11 +108,13 @@ export class SummaryRecomputeError extends Error { * converts naming the child, then the parent's walk receives that) or a hook * whose own write was such a delete. The caller of the outer write is told * about the outer write, and `cause` stays the original refusal, so the - * summaries named are the same at every level. A transaction does not re-lead: - * a conversion that escapes it already names the write that did not complete. + * summaries named are the same at every level. A transaction whose rollback + * ran does not re-lead: a conversion that escapes it already names the write + * that did not complete, and its "under a transaction that rolls back" clause + * holds. [#29527] One whose rollback threw does ({@link rollbackFailedRefusal}). */ function unlandedWriteRefusal(e: unknown, lead: string, relead = false): unknown { - const original = relead && e instanceof UnlandedWriteRefusal ? e.cause : e; + const original = relead && isUnlandedWriteRefusal(e) ? (e as UnlandedWriteRefusal).cause : e; if (!(original instanceof Error) || (original as { code?: unknown }).code !== SUMMARY_RECOMPUTE_CODE) return e; const failures = (original as { failures?: unknown }).failures; const named = Array.isArray(failures) @@ -142,8 +144,8 @@ function unlandedWriteRefusal(e: unknown, lead: string, relead = false): unknown * declared 5xx with no code drops `code` from the data door's body. * * A class so a later conversion can tell an earlier one apart (the re-lead - * above). Module-private and never exported: no reader outside this file - * matches it, so the published surface does not move. `name` stays `'Error'`. + * above). The class itself stays module-private, and `name` stays `'Error'`; + * a reader outside this file asks {@link isUnlandedWriteRefusal}. */ class UnlandedWriteRefusal extends Error { readonly status = 500; @@ -152,9 +154,33 @@ class UnlandedWriteRefusal extends Error { constructor(message: string, original: Error) { super(message); (this as { cause?: unknown }).cause = original; + Object.defineProperty(this, UNLANDED_WRITE_REFUSAL_BRAND, { value: true }); } } +/** + * [#29527] The brand {@link isUnlandedWriteRefusal} reads. A registered symbol, + * not `instanceof`: this package ships two realms (ESM and CJS) and a reader + * holding the other realm's copy of the class would get `false`, silently. + * Not enumerable, so no serialiser and no `{ ...err }` spread carries it. + */ +const UNLANDED_WRITE_REFUSAL_BRAND = Symbol.for('@objectstack/objectql.unlanded-write-refusal'); + +/** + * [#29527] Is `e` the refusal {@link unlandedWriteRefusal} built: a write that + * did not land because a nested write's stale-roll-up refusal stopped it? + * + * Exported for the one host-side reader that has to tell it apart: the QuickJS + * runner, which hands this refusal back out of a sandboxed body unchanged when + * the body let it leave (as it does the engine's duplicate-record refusal), so + * the hook-dispatch seam that converts next still re-leads it and keeps its + * `cause`. Matched on the HOST-side error a host call rejected with, never on + * anything a body wrote. + */ +export function isUnlandedWriteRefusal(e: unknown): boolean { + return e instanceof Error && (e as unknown as Record)[UNLANDED_WRITE_REFUSAL_BRAND] === true; +} + /** {@link unlandedWriteRefusal} for a delete whose cascade walk the refusal stopped. */ export function cascadeStoppedRefusal(e: unknown, object: string): unknown { return unlandedWriteRefusal( @@ -212,5 +238,9 @@ export function rollbackFailedRefusal(e: unknown): unknown { e, 'The transaction did not complete, and its rollback failed too, so what was written inside it may or may ' + 'not be stored.', + // [#29527] Re-lead an earlier conversion too. A cascade- or hook-stopped + // refusal that escapes this transaction says "under a transaction that + // rolls back nothing was written", which the failed rollback made unknown. + true, ); } diff --git a/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts b/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts new file mode 100644 index 00000000000..de9648b23c7 --- /dev/null +++ b/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts @@ -0,0 +1,193 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29527] A converted refusal a sandboxed hook body lets leave keeps its lead + * and its `cause` at the write the hook is bound to. + * + * ## What was wrong + * + * objectql converts a nested write's stale-roll-up refusal once the write it + * stopped did not land (`summary-errors.ts`): a delete whose cascade stopped, + * a transaction that rolled back, a hook that stopped. The hook-dispatch seam + * (`triggerHooks`) converts again on the way out and RE-LEADS an earlier + * conversion, so the outer write's caller is told about the outer write and + * `cause` stays the original refusal. + * + * A sandboxed hook body's `ctx.api` write is that write one VM hop further. + * Its converted refusal crossed out of the QuickJS runner as a `SandboxError` + * (`500`, `INTERNAL_ERROR` lifted, class and `cause` dropped), which the seam + * no longer recognised, so it passed through unchanged: the caller of + * `insert('hcase')` read `The delete of "hacct" did not complete`, the hook's + * own write, and the summaries the refusal named were gone from `cause`. + * + * ## What is pinned + * + * The runner hands the engine's own refusal back out when the body let it + * leave unchanged (as it already did the engine's duplicate-record refusal), + * so the seam re-leads it: + * + * - `beforeInsert`, `afterInsert` and `beforeFind` hooks, each a sandboxed body + * deleting an account whose cascade a line's failing roll-up stops. The three + * events share one dispatch seam (`afterFind` and the other events with + * them); `beforeFind` stands for the read events; + * - the status and code are unchanged, `500 INTERNAL_ERROR`; + * - nothing the hook's write touched is stored, and an `afterInsert` hook's + * own record is rolled back with it; + * - CONTROL a body that catches the refusal and throws its own error speaks + * for itself, and leaves the runner as a `SandboxError` exactly as before; + * - CONTROL a body that rewrites the refusal's message before re-throwing it + * is a speaker too. + * + * The rig is `action-delete-cascade-summary-rollback.test.ts`'s: a real + * `ObjectQL` over `SqlDriver` on better-sqlite3, which rolls back for real, and + * the line's roll-up into `hbucket.memo_sum` is a sum over a text field, which + * every recompute refuses. Hook bodies run through `hookBodyRunnerFactory` over + * a real `QuickJSScriptRunner`. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { hookBodyRunnerFactory } from './sandbox/body-runner.js'; +import { QuickJSScriptRunner, SandboxError } from './sandbox/quickjs-runner.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const TEST_PACKAGE_ID = 'objectstack-test'; +const ACCOUNT = 'hacct'; +const BUCKET = 'hbucket'; +const LINE = 'hline'; +const CASE = 'hcase'; +const FINDER = 'hfinder'; + +const text = (name: string) => ({ name, type: 'text' as const, label: name }); +const id = { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }; + +const OBJECTS = [ + { name: ACCOUNT, label: 'Account', fields: { id, name: text('name') } }, + { + name: BUCKET, label: 'Bucket', + fields: { + id, name: text('name'), + // A sum over a text field: every recompute of it is refused. + memo_sum: { name: 'memo_sum', type: 'summary' as const, label: 'Memo sum', summaryOperations: { object: LINE, function: 'sum', field: 'memo' } }, + }, + }, + { + name: LINE, label: 'Line', + fields: { + id, name: text('name'), memo: text('memo'), + account: { name: 'account', type: 'master_detail' as const, reference: ACCOUNT, deleteBehavior: 'cascade', label: 'Account' }, + bucket: { name: 'bucket', type: 'lookup' as const, reference: BUCKET, label: 'Bucket' }, + }, + }, + { name: CASE, label: 'Case', fields: { id, name: text('name'), mode: text('mode') } }, + { name: FINDER, label: 'Finder', fields: { id, name: text('name') } }, +]; + +/** Deletes the account; its cascade is stopped by the first line's refused roll-up. */ +const PURGE = `await ctx.api.object('${ACCOUNT}').delete({ where: { id: 'a1' } });`; +const WRITE_CAPS = ['api.read', 'api.write']; + +/** A sandboxed hook on `hcase`, active only for records whose `mode` names it. */ +const caseHook = (name: string, event: string, mode: string, source: string) => ({ + name, object: CASE, events: [event], + body: { language: 'js', capabilities: WRITE_CAPS, source: `if (ctx.input?.mode !== '${mode}') return; ${source}` }, +}); + +const HOOKS = [ + caseHook('purge_before', 'beforeInsert', 'before', PURGE), + caseHook('purge_after', 'afterInsert', 'after', PURGE), + caseHook('purge_speaks', 'beforeInsert', 'speaks', `try { ${PURGE} } catch (e) { throw new Error('The account could not be purged'); }`), + caseHook('purge_rewrites', 'beforeInsert', 'rewrites', `try { ${PURGE} } catch (e) { e.message = 'Purge refused: ' + e.message; throw e; }`), + { name: 'purge_on_find', object: FINDER, events: ['beforeFind'], body: { language: 'js', capabilities: WRITE_CAPS, source: PURGE } }, +]; + +let engine: ObjectQL; +let errorLog: ReturnType; + +beforeAll(async () => { + errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true); + await engine.init(); + for (const o of OBJECTS) engine.registry.registerObject(o as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + await engine.insert(ACCOUNT, { id: 'a1', name: 'A' }); + await engine.insert(BUCKET, { id: 'b1', name: 'B' }); + for (const lineId of ['l1', 'l2']) { + await engine.insert(LINE, { id: lineId, name: lineId, account: 'a1', bucket: 'b1' }).catch((e: any) => { + // The seed itself meets the failing roll-up; its rows are written. + if (e?.code !== 'ERR_SUMMARY_RECOMPUTE') throw e; + }); + } + engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'hcase_app' })); + bindHooksToEngine(engine, HOOKS as any, { packageId: TEST_PACKAGE_ID }); +}, 60_000); + +afterAll(async () => { + errorLog?.mockRestore(); + try { await engine?.destroy(); } catch { /* noop */ } +}); + +const refusal = (p: Promise): Promise => p.then(() => null, (e: unknown) => e); +const rowIds = async (object: string) => + ((await engine.find(object, { fields: ['id'] })) as Array<{ id: string }>).map((r) => r.id).sort(); + +/** The re-led refusal: the outer write leads, the line's own refusal is `cause`, the status unchanged. */ +function expectReLed(err: any, lead: string) { + expect(err, 'the write was refused').toBeInstanceOf(Error); + expect(err.status).toBe(500); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.message.startsWith(lead), err.message).toBe(true); + expect(err.message).not.toContain(`"${ACCOUNT}"`); + expect(err.message).toContain(`${BUCKET}.memo_sum`); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.cause?.failures?.[0]?.childObject).toBe(LINE); +} + +async function expectNothingPurged() { + expect(await rowIds(ACCOUNT)).toEqual(['a1']); + expect(await rowIds(LINE)).toEqual(['l1', 'l2']); +} + +describe('[#29527] a converted refusal a sandboxed hook body lets leave is re-led for the write the hook is bound to', () => { + it('beforeInsert: the caller reads the insert of the case, `cause` is the line\'s own refusal', async () => { + const err = await refusal(engine.insert(CASE, { id: 'c_before', name: 'x', mode: 'before' })); + expectReLed(err, `The insert of "${CASE}" did not complete: its beforeInsert hook "purge_before" stopped.`); + expect(err).not.toBeInstanceOf(SandboxError); + await expectNothingPurged(); + expect(await rowIds(CASE)).not.toContain('c_before'); + }); + + it('afterInsert: re-led the same way, and the case\'s own row is rolled back with the hook\'s', async () => { + const err = await refusal(engine.insert(CASE, { id: 'c_after', name: 'x', mode: 'after' })); + expectReLed(err, `The insert of "${CASE}" did not complete: its afterInsert hook "purge_after" stopped.`); + await expectNothingPurged(); + expect(await rowIds(CASE)).not.toContain('c_after'); + }); + + it('beforeFind: the read events share the seam, so a find is re-led too', async () => { + const err = await refusal(engine.find(FINDER, {})); + expectReLed(err, `The find of "${FINDER}" did not complete: its beforeFind hook "purge_on_find" stopped.`); + await expectNothingPurged(); + }); +}); + +describe('[#29527] CONTROL a body that speaks for itself keeps the relay it had', () => { + it('a body that catches the refusal and throws its own error: a SandboxError with the body\'s sentence', async () => { + const err = await refusal(engine.insert(CASE, { id: 'c_speaks', name: 'x', mode: 'speaks' })); + expect(err).toBeInstanceOf(SandboxError); + expect(err.innerMessage).toBe('The account could not be purged'); + expect(err.cause).toBeUndefined(); + await expectNothingPurged(); + }); + + it('a body that rewrites the refusal\'s message before re-throwing it: a SandboxError, not re-led', async () => { + const err = await refusal(engine.insert(CASE, { id: 'c_rewrites', name: 'x', mode: 'rewrites' })); + expect(err).toBeInstanceOf(SandboxError); + expect(err.message).toContain('Purge refused: '); + expect(err.message).not.toContain(`The insert of "${CASE}"`); + await expectNothingPurged(); + }); +}); diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index f779c2c2fd7..5e56e6d1499 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -40,6 +40,7 @@ import { } from 'quickjs-emscripten'; import { resolveSandboxTimeoutMs, sandboxBusinessMessage, isEngineDuplicateRecordEnvelope, INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; import type { HookBody, ScriptBody, ExpressionBody, HookBodyCapability } from '@objectstack/spec/data'; +import { isUnlandedWriteRefusal } from '@objectstack/objectql'; import type { ScriptContext, ScriptOrigin, @@ -1605,6 +1606,16 @@ function hostErrorToVm(vm: QuickJSContext, err: unknown): QuickJSHandle { * shared predicate on the HOST-side error, so the runner and the doors cannot * disagree about which errors are the engine's. Every other host error crosses * exactly as it did. + * + * [#29527] …and for objectql's converted stale-roll-up refusal, a write that + * did not land because a nested write's roll-up could not be recomputed + * (`isUnlandedWriteRefusal`, objectql's own predicate). Its reader is not a + * door but the hook-dispatch seam a sandboxed hook body returns to: that seam + * converts again and re-leads an earlier conversion, so the outer write's + * caller reads the outer write and `cause` keeps the original refusal. Crossing + * as a `SandboxError` it was neither recognised nor carried its `cause`. Its + * status and code are the ones the `SandboxError` lifted, `500 + * INTERNAL_ERROR`, so no door's answer moves. */ interface HeldEngineRefusal { /** A dup of the VM error {@link hostErrorToVm} built; released before `vm.dispose()`. */ @@ -1620,10 +1631,11 @@ const HELD_ENGINE_REFUSALS = new WeakMap(); /** * [#23702] Remember `errH` when `err` is the engine's unique-constraint - * envelope. Best-effort: a failure leaves the error crossing as it always did. + * envelope, or [#29527] objectql's converted stale-roll-up refusal. + * Best-effort: a failure leaves the error crossing as it always did. */ function holdEngineRefusal(vm: QuickJSContext, errH: QuickJSHandle, err: unknown): void { - if (!isEngineDuplicateRecordEnvelope(err)) return; + if (!isEngineDuplicateRecordEnvelope(err) && !isUnlandedWriteRefusal(err)) return; try { const snapshot = crossingSnapshot(vm, errH); if (snapshot === undefined) return; From ac992d4d011a18e3c4195a219469e63285a67a64 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 01:18:34 +0000 Subject: [PATCH 2/2] test(runtime): the action door's answer does not move when an action body lets a converted refusal leave Claude-Session: https://claude.ai/code/session_01JQwxbZhgmEnK3HMysTCHGU Co-authored-by: Claude --- ...sandbox-hook-unlanded-refusal-lead.test.ts | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts b/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts index de9648b23c7..23e9d993677 100644 --- a/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts +++ b/packages/runtime/src/sandbox-hook-unlanded-refusal-lead.test.ts @@ -36,7 +36,10 @@ * - CONTROL a body that catches the refusal and throws its own error speaks * for itself, and leaves the runner as a `SandboxError` exactly as before; * - CONTROL a body that rewrites the refusal's message before re-throwing it - * is a speaker too. + * is a speaker too; + * - an ACTION body that lets the same refusal leave now hands back the + * engine's error too, and the action door's answer does not move: `500 + * INTERNAL_ERROR`, the delete's own sentence, `success: false`. * * The rig is `action-delete-cascade-summary-rollback.test.ts`'s: a real * `ObjectQL` over `SqlDriver` on better-sqlite3, which rolls back for real, and @@ -48,7 +51,9 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; -import { hookBodyRunnerFactory } from './sandbox/body-runner.js'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { actionBodyRunnerFactory, hookBodyRunnerFactory } from './sandbox/body-runner.js'; import { QuickJSScriptRunner, SandboxError } from './sandbox/quickjs-runner.js'; import { installDefaultOrganization } from './default-organization.testkit.js'; @@ -62,6 +67,9 @@ const FINDER = 'hfinder'; const text = (name: string) => ({ name, type: 'text' as const, label: name }); const id = { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }; +/** Deletes the account; its cascade is stopped by the first line's refused roll-up. */ +const PURGE = `await ctx.api.object('${ACCOUNT}').delete({ where: { id: 'a1' } });`; + const OBJECTS = [ { name: ACCOUNT, label: 'Account', fields: { id, name: text('name') } }, { @@ -80,12 +88,13 @@ const OBJECTS = [ bucket: { name: 'bucket', type: 'lookup' as const, reference: BUCKET, label: 'Bucket' }, }, }, - { name: CASE, label: 'Case', fields: { id, name: text('name'), mode: text('mode') } }, + { + name: CASE, label: 'Case', fields: { id, name: text('name'), mode: text('mode') }, + actions: [{ name: 'purge', label: 'Purge', type: 'script', body: { language: 'js', capabilities: ['api.read', 'api.write'], source: `${PURGE} return { purged: true };` } }], + }, { name: FINDER, label: 'Finder', fields: { id, name: text('name') } }, ]; -/** Deletes the account; its cascade is stopped by the first line's refused roll-up. */ -const PURGE = `await ctx.api.object('${ACCOUNT}').delete({ where: { id: 'a1' } });`; const WRITE_CAPS = ['api.read', 'api.write']; /** A sandboxed hook on `hcase`, active only for records whose `mode` names it. */ @@ -103,6 +112,7 @@ const HOOKS = [ ]; let engine: ObjectQL; +let dispatcher: HttpDispatcher; let errorLog: ReturnType; beforeAll(async () => { @@ -123,6 +133,14 @@ beforeAll(async () => { } engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'hcase_app' })); bindHooksToEngine(engine, HOOKS as any, { packageId: TEST_PACKAGE_ID }); + const purge = (OBJECTS.find((o) => o.name === CASE) as any).actions[0]; + (engine as any).registerAction(CASE, 'purge', actionBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'hcase_app' })({ ...purge, object: CASE })); + const protocol = new ObjectStackProtocolImplementation(engine as any); + const services: Record = { objectql: engine, data: engine, protocol }; + const get = (n: string) => services[n] ?? null; + const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) }; + dispatcher = new HttpDispatcher(kernel); + (dispatcher as any).timedResolveExecutionContext = async () => ({ userId: 'u_member', isSystem: false }); }, 60_000); afterAll(async () => { @@ -191,3 +209,17 @@ describe('[#29527] CONTROL a body that speaks for itself keeps the relay it had' await expectNothingPurged(); }); }); + +describe('[#29527] the action door\'s answer does not move for an action body that lets the refusal leave', () => { + it('500 INTERNAL_ERROR, the delete\'s own sentence, success false, nothing deleted', async () => { + const path = `/actions/${CASE}/purge`; + const request = new Request(`http://localhost/api/v1${path}`, { method: 'POST', headers: { 'content-type': 'application/json' }, body: '{}' }); + const res: any = await dispatcher.dispatch('POST', path, {}, {}, { request } as any, '/api/v1'); + const response = res.response as { status: number; body: any }; + expect(response.status, JSON.stringify(response.body)).toBe(500); + expect(response.body?.success, JSON.stringify(response.body)).toBe(false); + expect(response.body?.error?.code, JSON.stringify(response.body)).toBe('INTERNAL_ERROR'); + expect(response.body?.error?.message).toContain(`The delete of "${ACCOUNT}" did not complete`); + await expectNothingPurged(); + }); +});