fix(objectql)!: a hook's own write's stale-roll-up refusal never answers for the outer write; the unlanded-write refusal re-leads, declares 500, and never claims a failed rollback

.changeset/29422-hook-write-stale-summary-refusal.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql)!: a hook's own write's stale-roll-up refusal (`ERR_SUMMARY_RECOMPUTE`) never answers for the write the hook is bound to, and the unlanded-write refusal leads with the outer write and declares `500 INTERNAL_ERROR`
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+Narrowing note: a write whose hook let its own write's stale-roll-up refusal escape used to answer success with the hook's record. It now refuses, and an aborting `after*` hook's unit rolls both rows back. The action door also answers these refusals `500` instead of `400`. Hence the `!` in the title, the full dogfood run and the seat's contract review. The claim was amended by seat 5 on #29422.
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling or stored shape is added, removed, renamed or re-shaped, and no exported declaration is added, removed or narrowed. What narrows is engine answers: a stale-roll-up refusal that a hook's own write raised now leaves the outer write as an internal-failure refusal, and the refusal a write answers when a nested write's refusal stopped it now declares status 500 and code INTERNAL_ERROR. The remedy is the summary field's `summaryOperations` or a retry, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
+
+**BREAKING** for these answers only. `ERR_SUMMARY_RECOMPUTE` means "this write is stored, only a parent roll-up recompute failed". The data doors (REST, MCP) and the action, sandbox and flow writers answer it as the write's own success, using the refusal's `written` as the result.
+
+- **A hook's own write.** Suppose a hook writes through `ctx.api` and lets that write's refusal escape. The refusal reached the caller of the write the hook is bound to, carrying the HOOK's record. So `POST /api/v1/data/:object` answered `201` with the id and record of the row the hook wrote. Under an aborting `after*` hook the write's own unit then committed both rows. Now the refusal is converted where it leaves the handler, for `before*` and `after*` hooks alike, and for `beforeFind` / `afterFind` too: a find hook whose own write refuses now fails the read with the same declared `500` (the read failed before as well, only its sentence and status were inferred). The write refuses with an internal failure that names it, the hook and the stale summary, and keeps the original refusal as `cause`. An aborting `after*` hook's unit rolls both rows back, as `HookSchema.onError` `abort` says. With no unit (a code-registered hook, a driver without transactions, a caller's own unit), the message says that what was written before that point stays written. Every other error a hook throws passes through unchanged, and a hook declared `onError: 'log'` still does not fail the write.
+- **The refusal's lead on a nested cascade.** When a grandchild's delete refused, the parent's caller read `The delete of "<child>" did not complete`. Each outer cascade walk and each hook now re-leads the refusal, so the caller reads the write it asked for. `cause` stays the original refusal at every depth.
+- **A declared status.** The refusal a write answers when a nested write's refusal stopped it (a cascade delete, a rolled-back transaction, and now a hook) declares `status: 500` and `code: 'INTERNAL_ERROR'`. It no longer carries no `code`, which supersedes the sentence of the pending `@objectstack/objectql` changeset that said it did. For the refusal itself, the REST data door's answer is byte-identical: `500 { error: "Internal server error", code: "INTERNAL_ERROR" }`, now declared instead of reached by falling through every message heuristic. **The action door now answers these refusals `500 INTERNAL_ERROR`, not `400`**, with the same sentence. **A converted refusal relayed out of a sandboxed hook body** (its `ctx.api` cascade delete or transaction was stopped, and the body let the error escape) **now answers `500 INTERNAL_ERROR` at the REST data door, where it answered `400` with the inner sentence.** The dispatcher data door and the MCP tools keep status `500` and code `INTERNAL_ERROR` for these refusals, but now withhold the prose, as they do for any declared 5xx.
+- **A failed rollback.** When `ObjectQL.transaction`'s rollback itself threw, the refusal still said "The transaction rolled back, so nothing written inside it was stored". It now says that the rollback failed and that what was written inside it may or may not be stored.
+
+**The remedy.** Fix the summary field's `summaryOperations` (the engine's warning names why the recompute was refused), then retry. A hook that can tolerate a stale summary on its own write catches `ERR_SUMMARY_RECOMPUTE` itself.

packages/objectql/src/engine-cascade-summary-rollback.test.ts

@@ -144,7 +144,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren
     const { engine, driver } = await makeEngine(true);
     const err: any = await engine.delete('acct', { where: { id: 'a1' } }).then(() => null, (e: unknown) => e);
     expect(err).toBeInstanceOf(Error);
-    expect(err.code).toBeUndefined();
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
     expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
     expect(err.written).toBeUndefined();
     expect(driver.rolledBack).toHaveLength(1);
@@ -156,7 +157,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren
     const { engine, driver } = await makeEngine(false);
     const err: any = await engine.delete('acct', { where: { id: 'a1' } }).then(() => null, (e: unknown) => e);
     expect(err).toBeInstanceOf(Error);
-    expect(err.code).toBeUndefined();
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
     expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
     expect(ids(driver.rowsOf('acct'))).toEqual(['a1']);
     // The dependent the walk reached first is gone: nothing can roll it back.
@@ -167,7 +169,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren
     const { engine, driver } = await makeEngine(false);
     const err: any = await engine.delete('acct', { where: { name: 'A' }, multi: true }).then(() => null, (e: unknown) => e);
     expect(err).toBeInstanceOf(Error);
-    expect(err.code).toBeUndefined();
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
     expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
     expect(ids(driver.rowsOf('acct'))).toEqual(['a1']);
   });
@@ -178,7 +181,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren
       .transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx }))
       .then(() => null, (e: unknown) => e);
     expect(err).toBeInstanceOf(Error);
-    expect(err.code).toBeUndefined();
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
     expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
     expect(driver.rolledBack).toHaveLength(1);
     expect(ids(driver.rowsOf('line'))).toEqual(['l1', 'l2']);
@@ -191,7 +195,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren
       .transaction(async (trx: any) => trx.object('line').deleteById('l1'))
       .then(() => null, (e: unknown) => e);
     expect(err).toBeInstanceOf(Error);
-    expect(err.code).toBeUndefined();
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
     expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
     expect(driver.rolledBack).toHaveLength(1);
     expect(ids(driver.rowsOf('line'))).toEqual(['l1', 'l2']);

packages/objectql/src/engine-hook-stale-summary-refusal.test.ts

@@ -0,0 +1,221 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * A hook's own `ctx.api` write that raises the stale-roll-up refusal never
+ * answers for the write the hook is bound to.
+ *
+ * `SummaryRecomputeError` (`ERR_SUMMARY_RECOMPUTE`) means "this write is
+ * stored, only a parent's roll-up recompute failed". It carries the write's
+ * result as `written`, and every answering surface turns it into that write's
+ * own success. A hook that writes through `ctx.api` makes a write of its own.
+ * When that write raised the refusal and the handler let it escape, the refusal
+ * reached the OUTER write's caller unchanged. So the caller read the hook's
+ * record as the result of its own write: measured through the REST data door
+ * before this file, `POST /data/case` answered `201` with the id and record of
+ * the row the hook wrote. Under an `after*` hook's unit (`inAfterAbortUnit`) the
+ * unit then committed, because the refusal says "written".
+ *
+ * Pinned here, at the one seam a handler's error leaves through
+ * (`triggerHooks`):
+ *  - an aborting `afterInsert` / `afterUpdate` / `afterDelete` hook: the outer
+ *    write refuses with an error that is NOT the stale-roll-up refusal (a
+ *    declared `500 INTERNAL_ERROR`, no `written`, the refusal kept as `cause`),
+ *    leading with the outer write, and its unit rolls both rows back;
+ *  - a code-registered hook (no unit opens): the same refusal, whose text says
+ *    what stays written, and both rows are stored;
+ *  - a `before*` hook: the same refusal, and the outer row was never written.
+ *
+ * Controls: a hook declared `onError: 'log'` does not fail the outer write; the
+ * outer write's OWN refusal still answers `ERR_SUMMARY_RECOMPUTE` carrying its
+ * own result (and its unit still commits); any other error a hook throws passes
+ * through unchanged.
+ *
+ * The driver rolls back for real (a snapshot per transaction), and refuses
+ * every write to `bucket` while `refuseBucket` is set: that is the roll-up
+ * recompute's parent update, refused without a transient message, so no retry
+ * runs.
+ */
+
+import { describe, it, expect } from 'vitest';
+import type { Hook } from '@objectstack/spec/data';
+import { ObjectQL } from './engine.js';
+import { bindHooksToEngine } from './hook-binder.js';
+
+type Row = Record<string, unknown>;
+
+function makeDriver() {
+  let stores = new Map<string, Map<string, Row>>();
+  const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!);
+  const matches = (row: Row, where: any) =>
+    !where || Object.entries(where).every(([k, v]) => k.startsWith('$') || row[k] === v);
+  const begun: unknown[] = [];
+  const committed: unknown[] = [];
+  const rolledBack: unknown[] = [];
+  const snapshots = new Map<unknown, typeof stores>();
+  const state = { refuseBucket: false };
+  let n = 0;
+  const driver: any = {
+    name: 'primary', version: '0.0.0', supports: {}, begun, committed, rolledBack, state,
+    rowsOf: (o: string) => Array.from(storeFor(o).values()),
+    async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+    async syncSchema() {},
+    async find(o: string, ast: any) {
+      const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
+      return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
+    },
+    async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; },
+    async create(o: string, data: Row) {
+      const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` };
+      storeFor(o).set(String(row.id), row);
+      return row;
+    },
+    async update(o: string, id: string, data: Row) {
+      if (state.refuseBucket && o === 'bucket') throw new Error('bucket store refused the write');
+      const row = { ...storeFor(o).get(String(id)), ...data, id };
+      storeFor(o).set(String(id), row);
+      return row;
+    },
+    async delete(o: string, id: string) { return storeFor(o).delete(String(id)); },
+    async count(o: string, ast: any) { return (await this.find(o, ast)).length; },
+    async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
+    async bulkUpdate() { return []; }, async bulkDelete() {},
+    async beginTransaction() {
+      const handle = { trx: begun.length + 1 };
+      snapshots.set(handle, new Map(Array.from(stores, ([o, s]) => [o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))])));
+      begun.push(handle);
+      return handle;
+    },
+    async commit(h: unknown) { snapshots.delete(h); committed.push(h); },
+    async rollback(h: unknown) { stores = snapshots.get(h) ?? stores; rolledBack.push(h); },
+  };
+  return driver;
+}
+
+const text = (name: string) => ({ name, type: 'text' });
+/** The hook's own write: a `line`, whose roll-up into `bucket` the driver refuses. */
+const writeLine = (ctx: any) => ctx.api.object('line').insert({ name: `from-${ctx.event}`, bucket: 'b1' });
+const hook = (events: string[], handler: (ctx: any) => unknown, extra: Partial<Hook> = {}) =>
+  ({ name: 'line_writer', object: 'case', events, handler, ...extra }) as unknown as Hook;
+
+async function makeEngine(hooks: Hook[]) {
+  const log = () => {};
+  const engine = new ObjectQL({ logger: { debug: log, info: log, warn: log, error: log } } as any);
+  (engine as any).summaryRetryOptions = { sleep: async () => {}, backoffBaseMs: 0 };
+  const driver = makeDriver();
+  engine.registerDriver(driver, true);
+  await engine.init();
+  engine.registry.registerObject({ name: 'case', label: 'case', fields: { id: { ...text('id'), primaryKey: true }, name: text('name') } } as any, '__test__');
+  engine.registry.registerObject({
+    name: 'bucket', label: 'bucket',
+    fields: {
+      id: { ...text('id'), primaryKey: true }, name: text('name'),
+      line_count: { name: 'line_count', type: 'summary', summaryOperations: { object: 'line', function: 'count' } },
+    },
+  } as any, '__test__');
+  engine.registry.registerObject({
+    name: 'line', label: 'line',
+    fields: { id: { ...text('id'), primaryKey: true }, name: text('name'), bucket: { name: 'bucket', type: 'master_detail', reference: 'bucket' } },
+  } as any, '__test__');
+  await engine.insert('bucket', { id: 'b1', name: 'B' });
+  await engine.insert('case', { id: 'c0', name: 'existing' });
+  bindHooksToEngine(engine, hooks, { packageId: 'app:t' });
+  driver.begun.length = 0;
+  driver.committed.length = 0;
+  driver.rolledBack.length = 0;
+  driver.state.refuseBucket = true;
+  return { engine, driver };
+}
+
+const refusal = (p: Promise<unknown>): Promise<any> => p.then(() => null, (e: unknown) => e);
+const names = (rows: Row[]) => rows.map((r) => r.name).sort();
+
+/** The converted refusal: an internal error naming the OUTER write, the hook write's refusal kept as `cause`. */
+function expectHookStopped(err: any, lead: string) {
+  expect(err).toBeInstanceOf(Error);
+  expect(err.code).toBe('INTERNAL_ERROR');
+  expect(err.status).toBe(500);
+  expect(err.written).toBeUndefined();
+  expect(err.failures).toBeUndefined();
+  expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
+  expect(err.cause?.written?.name).toMatch(/^from-/);
+  expect(err.message.startsWith(lead), err.message).toBe(true);
+  expect(err.message).toContain('bucket.line_count');
+}
+
+describe('a hook\'s own ctx.api write that raises ERR_SUMMARY_RECOMPUTE never answers for the write it is bound to', () => {
+  it('afterInsert (aborting, unit open): the insert refuses naming itself, and the unit rolls both rows back', async () => {
+    const { engine, driver } = await makeEngine([hook(['afterInsert'], writeLine)]);
+    const err = await refusal(engine.insert('case', { name: 'c-1' }));
+    expectHookStopped(err, 'The insert of "case" did not complete: its afterInsert hook "line_writer" stopped.');
+    expect(driver.begun).toHaveLength(1);
+    expect(driver.committed).toHaveLength(0);
+    expect(driver.rolledBack).toHaveLength(1);
+    expect(names(driver.rowsOf('case'))).toEqual(['existing']);
+    expect(driver.rowsOf('line')).toHaveLength(0);
+  });
+
+  it('afterUpdate and afterDelete (aborting, unit open): refused naming the outer write, the stored row unchanged', async () => {
+    const upd = await makeEngine([hook(['afterUpdate'], writeLine)]);
+    const updErr = await refusal(upd.engine.update('case', { id: 'c0', name: 'renamed' }));
+    expectHookStopped(updErr, 'The update of "case" did not complete: its afterUpdate hook "line_writer" stopped.');
+    expect(upd.driver.rolledBack).toHaveLength(1);
+    expect(names(upd.driver.rowsOf('case'))).toEqual(['existing']);
+    expect(upd.driver.rowsOf('line')).toHaveLength(0);
+
+    const del = await makeEngine([hook(['afterDelete'], writeLine)]);
+    const delErr = await refusal(del.engine.delete('case', { where: { id: 'c0' } }));
+    expectHookStopped(delErr, 'The delete of "case" did not complete: its afterDelete hook "line_writer" stopped.');
+    expect(del.driver.rolledBack).toHaveLength(1);
+    expect(names(del.driver.rowsOf('case'))).toEqual(['existing']);
+    expect(del.driver.rowsOf('line')).toHaveLength(0);
+  });
+
+  it('a code-registered afterInsert (no unit opens): the same refusal, saying what stays written, and both rows stored', async () => {
+    const { engine, driver } = await makeEngine([]);
+    engine.registerHook('afterInsert', writeLine, { object: 'case', hookName: 'code_line_writer' });
+    const err = await refusal(engine.insert('case', { name: 'c-1' }));
+    expectHookStopped(err, 'The insert of "case" did not complete: its afterInsert hook "code_line_writer" stopped.');
+    expect(err.message).toContain(
+      'Under a transaction that rolls back nothing was written; otherwise what was written before that point stays written.',
+    );
+    expect(driver.begun).toHaveLength(0);
+    expect(names(driver.rowsOf('case'))).toEqual(['c-1', 'existing']);
+    expect(names(driver.rowsOf('line'))).toEqual(['from-afterInsert']);
+  });
+
+  it('beforeInsert: the same refusal, and the outer row was never written', async () => {
+    const { engine, driver } = await makeEngine([hook(['beforeInsert'], writeLine)]);
+    const err = await refusal(engine.insert('case', { name: 'c-1' }));
+    expectHookStopped(err, 'The insert of "case" did not complete: its beforeInsert hook "line_writer" stopped.');
+    expect(names(driver.rowsOf('case'))).toEqual(['existing']);
+    // No unit opens for a before* hook, so the hook's own write stays, as the text says.
+    expect(names(driver.rowsOf('line'))).toEqual(['from-beforeInsert']);
+  });
+
+  it('CONTROL onError: \'log\': the hook\'s refusal is logged, and the outer insert answers its own row', async () => {
+    const { engine, driver } = await makeEngine([hook(['afterInsert'], writeLine, { onError: 'log' })]);
+    const row: any = await engine.insert('case', { name: 'c-1' });
+    expect(row?.name).toBe('c-1');
+    expect(names(driver.rowsOf('case'))).toEqual(['c-1', 'existing']);
+    expect(names(driver.rowsOf('line'))).toEqual(['from-afterInsert']);
+  });
+
+  it('CONTROL the outer write\'s OWN refusal still answers ERR_SUMMARY_RECOMPUTE with its own result, and its unit commits', async () => {
+    const { engine, driver } = await makeEngine([
+      { name: 'observe', object: 'line', events: ['afterInsert'], handler: () => {} } as unknown as Hook,
+    ]);
+    const err = await refusal(engine.insert('line', { name: 'direct', bucket: 'b1' }));
+    expect(err?.code).toBe('ERR_SUMMARY_RECOMPUTE');
+    expect(err.written?.name).toBe('direct');
+    expect(driver.begun).toHaveLength(1);
+    expect(driver.committed).toHaveLength(1);
+    expect(driver.rolledBack).toHaveLength(0);
+    expect(names(driver.rowsOf('line'))).toEqual(['direct']);
+  });
+
+  it('CONTROL any other error a hook throws passes through unchanged', async () => {
+    const boom = new Error('refused by the hook');
+    const { engine } = await makeEngine([hook(['afterInsert'], () => { throw boom; })]);
+    expect(await refusal(engine.insert('case', { name: 'c-1' }))).toBe(boom);
+  });
+});