fix(metadata-protocol): a batch row's httpStatus reads the declared status, not one spelling of it (#8570)

.changeset/batch-row-declared-http-status.md

@@ -0,0 +1,31 @@
+---
+"@objectstack/metadata-protocol": patch
+"@objectstack/types": minor
+---
+
+A bulk write's per-row `errors[].httpStatus` carries the status its producer declared, in any spelling (#8570)
+
+`toRowApiError` set the limb from `err.status` alone, so two well-defined client
+refusals shipped a batch row with no status at all: objectql's `ValidationError`
+(a 400 recognisable by shape, which deliberately declares no `status`) and
+`plugin-approvals`' record lock (a 409 spelled `statusCode`). Sibling rows in the
+same response did carry one — `rowRequiredIdError` → 400,
+`recordNotFoundError` → 404 — so a caller branching on `httpStatus` to tell "fix
+your input" from "the server broke" got an answer for some failure rows and
+silence for others, with nothing saying which. Same single-spelling defect #7525
+fixed at the HTTP door, one layer down.
+
+The limb now asks `resolveThrownHttpError` — the resolver the HTTP doors and the
+row's `message` limb already answer with — so a refusal declaring `.status`,
+`.statusCode` or the `VALIDATION_FAILED` shape reaches the row as the status it
+always meant. Rows whose throw declared nothing (a driver fault, a hook throwing
+a bare `Error`) still carry no `httpStatus`: the resolver's 500 there is the
+caller's fallback, not a producer's claim, and stamping it would add a field to
+the wire for those populations rather than restore a declared one. `code` reads
+the same resolution, so a row can no longer contradict itself.
+
+`ThrownHttpError` gains `declaredStatus` — the resolved status minus the
+fallback, absent when the throw declared none. `status` is unchanged, and every
+boundary that answers with the status itself keeps reading it; the new field is
+for sinks that mirror a status onto response DATA, where a fallback would be an
+invention.

packages/metadata-protocol/src/protocol.batch-row-driver-text.test.ts

@@ -307,9 +307,13 @@ describe('[#8502] section 2 — the authored population survives, in all THREE d
 
  expect(res.results[0].errors[0].message).toBe('title must be ≤ 4 characters (got 15)');
  expect(res.results[0].errors[0].code).toBe('VALIDATION_FAILED');
- // NO `httpStatus`: the producer declared none, and #8502 does not mint
- // one — that would be an ADDITION to the wire, a separate decision.
- expect(res.results[0].errors[0].httpStatus).toBeUndefined();
+ // `httpStatus: 400` since #8570 — the separate decision this line used
+ // to defer ("the producer declared no `.status`, and minting one is an
+ // ADDITION to the wire") was taken there: the limb now reads the same
+ // resolution this one does, so the validation SHAPE declares its 400.
+ // The undeclared populations still gain nothing; that half is pinned in
+ // `protocol.batch-row-http-status.test.ts` §3.
+ expect(res.results[0].errors[0].httpStatus).toBe(400);
  });
 
  it('a 4xx `statusCode` is quoted — THIS sink’s own population, met by neither sibling', async () => {
@@ -324,6 +328,9 @@ describe('[#8502] section 2 — the authored population survives, in all THREE d
  "RECORD_LOCKED: record 'r1' of 'leave_request' is locked while an approval is in progress",
  );
  expect(res.results[0].errors[0].code).toBe('RECORD_LOCKED');
+ // The `statusCode` spelling reaches `httpStatus` too since #8570 — this
+ // row is the card's second measured one.
+ expect(res.results[0].errors[0].httpStatus).toBe(409);
  });
 
  it('an UNDECLARED hook refusal is withheld — the measured cost of a positive list', async () => {

packages/metadata-protocol/src/protocol.ts

@@ -1491,16 +1491,67 @@ type BatchDataRowResult = BatchOperationResult;
  * ⛔ `fallback` is REQUIRED, not defaulted. All three catches that build a row
  * know their operation, and a default would let a fourth one be added that
  * silently reports the wrong verb.
+ *
+ * ## `httpStatus` reads the DECLARATION, not one spelling of it (#8570)
+ *
+ * This limb read `err.status` and nothing else, so of the producers measured at
+ * these catches — a real `ObjectQL` over a real `SqlDriver`, driven through all
+ * three bulk-write loops — two well-defined client refusals shipped a row with
+ * no status at all:
+ *
+ * | producer | code | `.status` | `.statusCode` | validation shape | was | is |
+ * |---|---|---|---|---|---|---|
+ * | {@link rowRequiredIdError} | VALIDATION_FAILED | **400** | — | no | 400 | 400 |
+ * | `recordNotFoundError` (`@objectstack/core`) | RECORD_NOT_FOUND | **404** | — | no | 404 | 404 |
+ * | objectql `ValidationError` | VALIDATION_FAILED | — | — | **yes** | — | **400** |
+ * | plugin-approvals' record lock | RECORD_LOCKED | — | **409** | no | — | **409** |
+ * | an app hook throwing a bare `Error` | — | — | — | no | — | — |
+ * | driver fault (`SqliteError`, …) | SQLITE_* | — | — | no | — | — |
+ *
+ * The first two are siblings of the last four *in the same response*: a caller
+ * branching on `httpStatus` to tell "fix your input" from "the server broke"
+ * got an answer for some failure rows and nothing for others, with no signal
+ * saying which. The single-spelling defect is #7525's, fixed there at the HTTP
+ * door; this is the same defect on the row.
+ *
+ * The question is answered by {@link resolveThrownHttpError}, IMPORTED — the
+ * `message` limb beside it already delegates there (#8502), and a second local
+ * chain would be the third derivation of "what status is this throw" in one
+ * function. ⛔ Do not re-spell it as `status ?? statusCode ?? validation`.
+ *
+ * ## ⛔ `declaredStatus`, never `status` — the over-broad direction is real
+ *
+ * That resolver answers for EVERY throw: its `status` is 500 for a bare hook
+ * `Error` and for a `SqliteError`, because 500 is the caller's fallback.
+ * Stamping that would put `httpStatus: 500` on the last two rows of the table,
+ * which never carried one — an ADDITION to the wire for those populations, and
+ * a claim the producer never made. `declaredStatus` is the same resolution
+ * minus the fallback: present exactly when the throw declared a status in one
+ * of the three spellings, absent otherwise. So a declared refusal gains the
+ * status it always meant, and an undeclared fault keeps carrying none.
+ *
+ * The gate is DECLARED-ness and deliberately not the 4xx band that
+ * {@link clientFacingRowFailureText} uses. That limb decides disclosure of free
+ * text, where a 5xx must be withheld; this one decides a number the producer
+ * itself authored, and a row already ships `httpStatus: 503` today when the
+ * refusal spells `.status` — narrowing to 4xx would WITHDRAW a status the wire
+ * carries, which is a different decision from this one.
+ *
+ * Both limbs read the same resolution for a second reason: they must agree.
+ * Deriving `code` from `err.status` while `httpStatus` came from
+ * `declaredStatus` would mint incoherent rows — `{ code: 'INTERNAL_ERROR',
+ * httpStatus: 409 }` for a `statusCode`-spelled refusal whose own code the
+ * ledger does not know.
  */
 function toRowApiError(err: any, fallback: string): ApiError {
  const thrown = typeof err?.code === 'string' && ErrorCode.safeParse(err.code).success
  ? (err.code as ApiError['code'])
  : undefined;
- const status = typeof err?.status === 'number' ? err.status : undefined;
+ const { declaredStatus } = resolveThrownHttpError(err);
  return {
- code: thrown ?? (status !== undefined ? standardErrorCodeForHttpStatus(status) : 'INTERNAL_ERROR'),
+ code: thrown ?? (declaredStatus !== undefined ? standardErrorCodeForHttpStatus(declaredStatus) : 'INTERNAL_ERROR'),
  message: clientFacingRowFailureText(err, fallback),
- ...(status !== undefined ? { httpStatus: status } : {}),
+ ...(declaredStatus !== undefined ? { httpStatus: declaredStatus } : {}),
  };
 }
 

packages/plugins/plugin-approvals/package.json

@@ -27,6 +27,7 @@
  },
  "devDependencies": {
  "@objectstack/driver-sql": "workspace:*",
+ "@objectstack/metadata-protocol": "workspace:*",
  "@objectstack/objectql": "workspace:*",
  "@objectstack/service-automation": "workspace:*",
  "@objectstack/trigger-record-change": "workspace:*",

packages/plugins/plugin-approvals/src/record-lock-batch-row-status.integration.test.ts

@@ -0,0 +1,179 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#8570] The record lock's refusal, as a BATCH ROW — driven through the REAL
+ * hook, a real {@link ObjectQL} and a real sqlite driver.
+ *
+ * ## The row this file exists for
+ *
+ * Measured on the real stack while #8502 was being verified, a bulk update of a
+ * record this plugin holds locked answered:
+ *
+ * ```json
+ * { "code": "RECORD_LOCKED", "message": "RECORD_LOCKED: record 'ok1' of 'm8502_task' is locked while an approval is in progress" }
+ * ```
+ *
+ * — a deliberate **409** shipping with no `httpStatus` at all, while sibling
+ * rows of the same response carried one. `toRowApiError` read `err.status`, and
+ * {@link lockedError} spells its refusal `statusCode`, which is the same
+ * single-spelling defect that made `/api/v1/data` answer 500 to this very
+ * refusal until #7525.
+ *
+ * ## Why the pin lives HERE
+ *
+ * `metadata-protocol` cannot import this plugin, and its own pins therefore
+ * stand in for this producer with a double whose shape was measured. This file
+ * is the half that needs no double: the error is raised by the actual
+ * `beforeUpdate` hook `bindApprovalLockHook` binds, against an actual pending
+ * `sys_approval_request` row, and the response row is built by the actual
+ * `updateManyData` loop. If the hook ever re-spells its refusal — `.status`,
+ * or a plain `Error` — this file goes red where a double would happily keep
+ * asserting the old shape.
+ *
+ * The rig is `record-lock-multi-update.integration.test.ts`'s, for the same
+ * reason it gives: the store is better-sqlite3 through `@objectstack/driver-sql`,
+ * so the predicates are compiled and executed by the SQL builder rather than by
+ * fixture code written by the same author as the assertion.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { ObjectQL } from '@objectstack/objectql';
+import { SqlDriver } from '@objectstack/driver-sql';
+import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
+import { resolveThrownHttpError } from '@objectstack/types';
+import { bindApprovalLockHook } from './lifecycle-hooks.js';
+
+const opportunity = {
+ name: 'opportunity',
+ label: 'Opportunity',
+ fields: {
+ id: { name: 'id', type: 'text' as const, primaryKey: true },
+ name: { name: 'name', type: 'text' as const },
+ amount: { name: 'amount', type: 'number' as const },
+ approval_status: { name: 'approval_status', type: 'text' as const },
+ },
+};
+
+/** The lock hook reads pending requests off this object. */
+const approvalRequest = {
+ name: 'sys_approval_request',
+ label: 'Approval Request',
+ fields: {
+ id: { name: 'id', type: 'text' as const, primaryKey: true },
+ object_name: { name: 'object_name', type: 'text' as const },
+ record_id: { name: 'record_id', type: 'text' as const },
+ status: { name: 'status', type: 'text' as const },
+ flow_run_id: { name: 'flow_run_id', type: 'text' as const },
+ node_config_json: { name: 'node_config_json', type: 'text' as const },
+ },
+};
+
+describe('[#8570] a locked record\'s batch row carries the 409 the hook declared', () => {
+ let engine: ObjectQL;
+ let protocol: any;
+ /** Held by a pending approval. */
+ let lockedId: string;
+ /** Same object, no approval — the row that must still succeed. */
+ let freeId: string;
+
+ afterEach(async () => {
+ try { await engine?.destroy(); } catch { /* noop */ }
+ });
+
+ beforeEach(async () => {
+ engine = new ObjectQL();
+ engine.registerDriver(new SqlDriver({
+ client: 'better-sqlite3',
+ connection: { filename: ':memory:' },
+ useNullAsDefault: true,
+ }), true);
+ await engine.init();
+ // `packageId` is REQUIRED by `registerObject` — passed rather than elided
+ // so this file adds no raw `tsc` error to the package's TEST_DEBT ledger,
+ // which is measured with the test layer in the program (the package's own
+ // `typecheck` script excludes `**/*.test.ts`, so it cannot see this).
+ for (const o of [opportunity, approvalRequest]) {
+ engine.registry.registerObject(o as any, 'com.objectstack.test.8570');
+ }
+ // Real DDL through the real path.
+ await engine.syncSchemas();
+
+ lockedId = String((await engine.insert('opportunity', { name: 'Deal', amount: 100 })).id);
+ freeId = String((await engine.insert('opportunity', { name: 'Other', amount: 100 })).id);
+ await engine.insert('sys_approval_request', {
+ object_name: 'opportunity',
+ record_id: lockedId,
+ status: 'pending',
+ flow_run_id: 'run_1',
+ node_config_json: JSON.stringify({ lockRecord: true, approvalStatusField: 'approval_status' }),
+ }, { context: { isSystem: true } } as any);
+
+ bindApprovalLockHook(engine as any);
+ protocol = new ObjectStackProtocolImplementation(engine as any);
+ });
+
+ it('the card\'s second row, verbatim, now carrying 409', async () => {
+ const res: any = await protocol.updateManyData({
+ object: 'opportunity',
+ records: [{ id: lockedId, data: { amount: 999 } }],
+ });
+
+ expect(res.results[0].success).toBe(false);
+ expect(res.results[0].errors[0]).toEqual({
+ code: 'RECORD_LOCKED',
+ message: `RECORD_LOCKED: record '${lockedId}' of 'opportunity' is locked while an approval is in progress`,
+ httpStatus: 409,
+ });
+
+ // The refusal was a refusal: nothing reached the store.
+ expect((await engine.findOne('opportunity', { where: { id: lockedId } }))?.amount).toBe(100);
+ });
+
+ it('the hook really declares its 409 in the `statusCode` spelling ONLY', async () => {
+ // Non-vacuity for the row above, taken off the REAL producer rather than
+ // asserted about it: if `lockedError` ever grew a `.status`, the row would
+ // be green against the pre-#8570 limb too, and this file would stop
+ // measuring anything.
+ let thrown: any = null;
+ try {
+ await engine.update('opportunity', { amount: 999 }, { where: { id: lockedId } } as any);
+ } catch (e) { thrown = e; }
+
+ expect(thrown).not.toBeNull();
+ expect(thrown.code).toBe('RECORD_LOCKED');
+ expect(thrown.statusCode).toBe(409);
+ expect(thrown.status).toBeUndefined();
+ expect(Object.getOwnPropertyNames(thrown)).toEqual(['stack', 'message', 'code', 'statusCode']);
+ // The production recogniser, on the real throw — and the field the row's
+ // limb reads, which is what separates a declared refusal from a fault.
+ expect(resolveThrownHttpError(thrown).declaredStatus).toBe(409);
+ });
+
+ it('an unlocked row in the SAME batch still succeeds and carries no error', async () => {
+ // The asymmetry the card is about is per-row, so the mixed response is the
+ // shape a caller actually has to reconcile.
+ const res: any = await protocol.updateManyData({
+ object: 'opportunity',
+ records: [
+ { id: freeId, data: { amount: 555 } },
+ { id: lockedId, data: { amount: 999 } },
+ ],
+ options: { continueOnError: true },
+ });
+
+ expect(res.results[0].success).toBe(true);
+ expect(res.results[0].errors).toBeUndefined();
+ expect(res.results[1].errors[0].httpStatus).toBe(409);
+ expect((await engine.findOne('opportunity', { where: { id: freeId } }))?.amount).toBe(555);
+ });
+
+ it('the batchData upsert loop answers the same way — not just updateManyData', async () => {
+ const res: any = await protocol.batchData({
+ object: 'opportunity',
+ request: { operation: 'upsert', records: [{ id: lockedId, data: { amount: 999 } }] },
+ });
+
+ expect(res.results[0].errors[0].code).toBe('RECORD_LOCKED');
+ expect(res.results[0].errors[0].httpStatus).toBe(409);
+ });
+});

packages/plugins/plugin-approvals/vitest.config.ts

@@ -0,0 +1,33 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { defineConfig } from 'vitest/config';
+import path from 'node:path';
+
+export default defineConfig({
+ resolve: {
+ // [#8570] `record-lock-batch-row-status.integration.test.ts` drives the
+ // REAL lock hook through the REAL bulk-write loops, so it imports
+ // `@objectstack/metadata-protocol` as a value. That specifier resolves
+ // through `exports` to `dist/` — a build artifact — which would make the
+ // pin a verdict about build state rather than about the source in the
+ // checkout (`pnpm check:test-source-alias`, #7668/#7778). Aliased to
+ // source, which is that gate's prescribed fix; registering the package as
+ // an unaliased importer is explicitly NOT (the registry is shrink-only).
+ //
+ // ANCHORED regex, array form: a bare string `find` matches by PREFIX, so
+ // with a FILE replacement it would also swallow any subpath and resolve it
+ // to `…/metadata-protocol/src/index.ts/<subpath>` — `ENOTDIR`, at run
+ // time, from a config that reads as correct.
+ alias: [
+ {
+ find: /^@objectstack\/metadata-protocol$/,
+ replacement: path.resolve(__dirname, '../../metadata-protocol/src/index.ts'),
+ },
+ ],
+ },
+ test: {
+ globals: true,
+ environment: 'node',
+ include: ['src/**/*.test.ts'],
+ },
+});