fix(metadata-protocol): gate a batch row's errors[].message on a declared client refusal (#8569)

* wip(#8502): withhold undeclared driver text from batch row errors[].message

* fix(metadata-protocol): gate batch row errors[].message on a declared client refusal (#8502)

* test(objectql): re-spell the batch-atomic driver-text pin for the #8502 withhold

* test: type the engine query options and drop a dead helper (#8502)

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/batch-row-driver-text-withhold.md

@@ -0,0 +1,34 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+Withhold undeclared driver text from a bulk write's per-row `errors[].message` (#8502)
+
+`toRowApiError` interpolated whatever it caught into a batch row's message, so a
+driver fault under `deleteManyData` answered
+`{ code: "INTERNAL_ERROR", message: "SQLITE_ERROR: no such table: leave_request" }`
+on response DATA riding a 200 — where no HTTP boundary's 5xx withhold can reach
+it. Driven against a real driver the leaked text is worse than the tidy example:
+a delete's raw message carries the failing statement's `WHERE` clause and its
+bound record id, and a create's carries the whole `INSERT` with its values. The
+causal row's message is also copied onto every `NOT_ATTEMPTED` / `ROLLED_BACK`
+sibling, so one leaked sentence was repeated across the batch.
+
+A caught sentence now reaches a caller only when its producer declared a
+client-facing refusal, asked through `resolveThrownHttpError` — the same
+resolver the HTTP doors answer with — so all three declarations this sink
+actually receives are honoured: a 4xx `status`, a 4xx `statusCode`, and the
+`VALIDATION_FAILED` shape that carries neither. Per-field authoring feedback
+from the engine's validator, `RECORD_NOT_FOUND`, `VALIDATION_FAILED` and
+`plugin-approvals`' `RECORD_LOCKED` are unchanged, byte for byte. Anything
+undeclared — a driver fault, or a hook that throws a bare `Error` — gets a
+stable sentence naming the operation, and the original goes to the server log.
+
+The `code` limb is untouched (#8441 already gates it on catalog membership), and
+no `httpStatus` is minted where the wire did not carry one.
+
+**Behaviour change for hook authors**: a hook that refuses by throwing an
+undeclared `Error` no longer has its sentence echoed on the row. Declare the
+refusal — a 4xx `status` or `statusCode`, or `validationFailure(message, fields)`
+from `@objectstack/types` — and the message is served verbatim, as it now is on
+the single-record path.

packages/metadata-protocol/src/protocol.batch-atomic.test.ts

@@ -104,8 +104,15 @@ describe('batchData atomic — rollback is real and the response admits it (ADR-
         // "Attempted, undone" vs "never ran" is a CODE, not a message-prefix
         // regex (#4793) — the message keeps the human-readable cause.
         expect(res.results[0].errors?.[0]?.code).toBe('ROLLED_BACK');
-        expect(res.results[0].errors?.[0]?.message).toContain('insert exploded'); // carries the cause
-        expect(res.results[1].errors?.[0]?.message).toBe('insert exploded');      // the causal row, verbatim
+        // [#8502] `insert exploded` is a BARE `Error` — it declares no client
+        // refusal, so its sentence is withheld and the row says the stable
+        // operation-named line instead. The claim under test is unchanged and
+        // is about PROPAGATION: whatever the causal row says, the rolled-back
+        // row quotes it, so a caller reading row 0 learns why row 1 stopped
+        // the batch. Asserted against the causal row's own message rather than
+        // a literal, so the two cannot drift apart.
+        expect(res.results[1].errors?.[0]?.message).toBe('The create of this record failed. The reason is in the server log.');
+        expect(res.results[0].errors?.[0]?.message).toContain(res.results[1].errors?.[0]?.message); // carries the cause
         expect(res.results[2].errors?.[0]?.code).toBe('NOT_ATTEMPTED');
         // Rows correlate to the request array by `index` (#4793).
         expect(res.results.map((r: any) => r.index)).toEqual([0, 1, 2]);
@@ -162,7 +169,8 @@ describe('batchData atomic — rollback is real and the response admits it (ADR-
         expect(res.succeeded).toBe(0);
         expect(res.results[0].errors?.[0]?.code).toBe('ROLLED_BACK');
         expect(res.results[0].id).toBe('rec-1'); // ids survive so a caller can reconcile
-        expect(res.results[1].errors?.[0]?.message).toBe('update exploded');
+        // [#8502] withheld: a bare `Error` declares no client refusal.
+        expect(res.results[1].errors?.[0]?.message).toBe('The update of this record failed. The reason is in the server log.');
     });
 });
 
@@ -236,7 +244,13 @@ describe('batchData atomic — precedence and opt-in (ADR-0119 D4)', () => {
 
         expect(t.rollbacks).toHaveLength(1);
         expect(t.insert).not.toHaveBeenCalled();          // no blind fallback
-        expect(res.results[0].errors?.[0]?.message).toBe('update exploded'); // the real cause survives
+        // [#8502] The cause is withheld from the RESPONSE (bare `Error`), so
+        // "the real cause survives" is now carried by the two structural
+        // assertions above — the update was attempted and no fallback insert
+        // ran — plus the row naming the UPSERT it was doing. What must never
+        // appear is the fallback insert's duplicate-key text.
+        expect(res.results[0].errors?.[0]?.message).toBe('The upsert of this record failed. The reason is in the server log.');
+        expect(res.results[0].errors?.[0]?.message).not.toContain('duplicate key');
     });
 });
 

packages/metadata-protocol/src/protocol.batch-row-conformance.test.ts

@@ -129,7 +129,11 @@ describe('batchData rows conform to BatchOperationResultSchema (#4793)', () => {
         expectConformantResponse(res, 3);
         expect(res.results[0].data).toMatchObject({ title: 'A' });
         // An unclassified engine throw is a 500 in row form.
-        expect(res.results[1].errors[0]).toMatchObject({ code: 'INTERNAL_ERROR', message: 'insert exploded' });
+        // [#8502] `code` is unchanged; the message is the withheld stable line.
+        expect(res.results[1].errors[0]).toMatchObject({
+            code: 'INTERNAL_ERROR',
+            message: 'The create of this record failed. The reason is in the server log.',
+        });
         expect(res.results[1].data).toBeUndefined();
     });
 
@@ -185,8 +189,9 @@ describe('batchData rows conform to BatchOperationResultSchema (#4793)', () => {
 
         expectConformantResponse(res, 3);
         expect(res.results[0].errors[0].code).toBe('ROLLED_BACK');
-        expect(res.results[0].errors[0].message).toContain('insert exploded'); // human-readable cause
-        expect(res.results[1].errors[0].message).toBe('insert exploded');      // causal row keeps its own error
+        // [#8502] Same propagation claim, against the causal row's own text.
+        expect(res.results[1].errors[0].message).toBe('The create of this record failed. The reason is in the server log.');
+        expect(res.results[0].errors[0].message).toContain(res.results[1].errors[0].message); // human-readable cause
         expect(res.results[2].errors[0].code).toBe('NOT_ATTEMPTED');
         // No reverted write may carry a record payload.
         for (const row of res.results) expect(row.data).toBeUndefined();
@@ -210,7 +215,7 @@ describe('updateManyData rows conform to BatchOperationResultSchema (#4793)', ()
 
         expectConformantResponse(res, 3);
         expect(res.results[0].data).toMatchObject({ id: 'a', title: 'a-new' });
-        expect(res.results[1].errors[0].message).toBe('update exploded');
+        expect(res.results[1].errors[0].message).toBe('The update of this record failed. The reason is in the server log.'); // [#8502]
     });
 
     it('atomic rollback — all three row classes, as codes', async () => {