fix(objectql): a strict strip refusal still pre-empts a throwing stored-expression pass

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz

Author: claude

.changeset/25000-strict-stored-expression-refusal.md

@@ -1,5 +1,6 @@
 ---
 '@objectstack/objectql': minor
+'@objectstack/spec': patch
 ---
 
 fix(objectql)!: under `strictReadonlyWrites`, a write whose value a stored field's `expression` replaces (`stored_expression`) is refused, and nothing is written
@@ -15,6 +16,8 @@ Clause-②: no (narrowing: under `strictReadonlyWrites`, a write whose value a s
 - **FROM:** a write with `strictReadonlyWrites: true` that sent a value for a stored expression field, different from the value the engine computes, completed. The row held the computed value, and `onFieldsDropped` received a `stored_expression` event.
 - **TO:** the same write throws `ReadonlyFieldRejectedError` (`code: 'ERR_READONLY_FIELD_REJECTED'`), whose `drops` carries the `stored_expression` event. Nothing is written, and `onFieldsDropped` does not fire. This holds on insert (a single row, a batch, and `insertMany`, which refuses the whole call as it does for the other strict drops), on update by id, and on a predicate update.
 - **One refusal names everything.** When the same payload also carries another strip's drop (`readonly`, `readonly_when`, `primary_key`, `computed`), the one refusal names all of them together, `stored_expression` last. Before, such a write was refused for the other drops alone. The message's per-reason form gains a `stored_expression` remedy, spoken only when that reason is present, so every other refusal's message is unchanged.
+- **Ordering is kept.** The strips' refusal still comes first: if the stored-expression pass itself throws, for example a predicate update whose patch computes different values on different matched rows (`VALIDATION_FAILED`), a strict write that also carries a strip's drop is still refused with `ERR_READONLY_FIELD_REJECTED` naming those drops, as before.
+- **`@objectstack/spec` (patch):** the TSDoc on `WriteObservabilityOptions.strictReadonlyWrites` and `DroppedFieldsEvent.reason`, which ships in the published declarations, now says the `stored_expression` arm is refused under strict. No schema, `.describe()` text or export changes.
 - **Unchanged:** a strict write that sends the computed value, or no value for the field, is not refused, because nothing it sent was replaced. A stored expression field also declared `readonly` is still refused once, as `readonly`. Without `strictReadonlyWrites`, the write completes with the computed value and the replacement is reported, as before. The error code is unchanged.
 - **The fix for a strict caller:** leave the stored expression field out of the payload, or send the value its expression computes. To let the replacement happen and merely observe it, drop `strictReadonlyWrites` and pass `onFieldsDropped`.
 

packages/objectql/src/engine-stored-expression-dropped-fields.test.ts

@@ -130,6 +130,8 @@ describe('[#24690] a caller value a stored expression replaced is reported as `s
         quantity: { type: 'number', readonlyWhen: "parent.status == 'paid'" },
         unit_price: { type: 'currency', readonlyWhen: "parent.status == 'paid'" },
         description: { type: 'text' },
+        // A static lock, so a predicate update can carry a strip's drop.
+        note: { type: 'text', readonly: true },
         amount: { type: 'currency', min: 0, expression: { dialect: 'cel', source: 'record.quantity * record.unit_price' } },
       },
     } as any);
@@ -375,6 +377,27 @@ describe('[#24690] a caller value a stored expression replaced is reported as `s
       expect(l.events).toEqual([]);
     });
 
+    it('predicate update: a strip\'s drop is refused BEFORE a divergent settle — ERR_READONLY_FIELD_REJECTED, not VALIDATION_FAILED (#5126 ordering)', async () => {
+      const l = listener();
+      const before = snapshot();
+      // quantity: 10 computes amount 100 on line_draft and 70 on line_draft_2,
+      // which one statement cannot store; `note` is taken by the static strip.
+      const err = await refusal(engine.update(LINE, { note: 'x', quantity: 10 }, { where: { invoice: 'INV-1004' }, multi: true, ...STRICT, onFieldsDropped: l.onFieldsDropped } as any));
+      expect(err.drops).toEqual([{ object: LINE, fields: ['note'], reason: 'readonly' }]);
+      expect(snapshot()).toEqual(before);
+      expect(l.events).toEqual([]);
+    });
+
+    it('control, predicate update: the same divergent payload without strict is refused by the settle, VALIDATION_FAILED', async () => {
+      const before = snapshot();
+      const err: any = await engine
+        .update(LINE, { note: 'x', quantity: 10 }, { where: { invoice: 'INV-1004' }, multi: true } as any)
+        .catch((e: unknown) => e);
+      expect(err?.code).toBe('VALIDATION_FAILED');
+      expect(err?.fields).toEqual([expect.objectContaining({ field: 'amount', code: 'invalid_value' })]);
+      expect(snapshot()).toEqual(before);
+    });
+
     it('insert: a value the static `readonly` strip took is refused as `readonly` alone, never also as `stored_expression`', async () => {
       const l = listener();
       const err = await refusal(engine.insert('qa_locked_line', { quantity: 4, unit_price: 2, amount: 5 }, { ...STRICT, onFieldsDropped: l.onFieldsDropped }));

packages/objectql/src/engine.ts

@@ -16734,9 +16734,23 @@ export class ObjectQL implements IObjectQLEngine {
           const liveIndexes = rows
             .map((row, i) => (rowErrors[i] === undefined && row != null && typeof row === 'object' ? i : -1))
             .filter((i) => i >= 0);
-          const computed = await computeStoredExpressions(
-            storedExpressionPlan, liveIndexes.map((i) => rows[i] as Record<string, unknown>), opCtx.context, storedExpressionServing,
-          );
+          let computed: Array<Record<string, unknown>>;
+          try {
+            computed = await computeStoredExpressions(
+              storedExpressionPlan, liveIndexes.map((i) => rows[i] as Record<string, unknown>), opCtx.context, storedExpressionServing,
+            );
+          } catch (err) {
+            // A pass that THROWS is pre-empted by the strict verdict on the
+            // strips' drops (#5126's ordering — the check used to sit ahead
+            // of this pass), then rethrown.
+            const stripDrops = droppedFieldEvents(object, computedDropped, insertDropped);
+            if (stripDrops.length > 0 && options?.strictReadonlyWrites === true) {
+              throw new ReadonlyFieldRejectedError(
+                object, [...new Set(stripDrops.flatMap((d) => d.fields))], stripDrops, 'insert',
+              );
+            }
+            throw err;
+          }
           liveIndexes.forEach((i, k) => {
             const row = rows[i] as Record<string, unknown>;
             const taken = insertDroppedPerRow[i] ?? [];
@@ -18403,9 +18417,18 @@ export class ObjectQL implements IObjectQLEngine {
                // every drop and replacement together. The computation writes
                // nothing (it evaluates over a copy), and an object declaring no
                // stored expression skips it, so its refusal is where it stood.
+               // A pass that THROWS (a permission or related-row read) is
+               // pre-empted by the strict verdict on the strips' drops already
+               // collected — #5126's ordering, as on the predicate branch.
                if (storedExpressionPlan.length > 0) {
                    const patch = hookContext.input.data as Record<string, unknown>;
-                   const [computed] = await computeStoredExpressions(storedExpressionPlan, [{ ...priorRecord, ...patch }], opCtx.context, storedExpressionServing);
+                   let computed: Record<string, unknown> | undefined;
+                   try {
+                       [computed] = await computeStoredExpressions(storedExpressionPlan, [{ ...priorRecord, ...patch }], opCtx.context, storedExpressionServing);
+                   } catch (err) {
+                       assertNoStrictDrops();
+                       throw err;
+                   }
                    emitDrop(replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, [computed!]), 'stored_expression');
                    applyStoredExpressionsToPatch(patch, computed!, priorRecord);
                }
@@ -18693,20 +18716,31 @@ export class ObjectQL implements IObjectQLEngine {
                // the column left as it is) is reported under
                // `stored_expression`; [#25000] through `emitDrop`, so under
                // strict it is REFUSED before the statement and no row is written,
-               // in one refusal with every strip's drop. The pass writes nothing;
-               // a patch it cannot store as one statement is refused by
-               // `settleBulkStoredExpressions` on its own, before this judgment.
+               // in one refusal with every strip's drop. The pass writes nothing.
+               // ⛔ #5126's ordering survives it: a pass that THROWS — above
+               // all `settleBulkStoredExpressions` refusing a patch that computes
+               // different values on different rows (`VALIDATION_FAILED`) — is
+               // pre-empted by the strict verdict on the strips' drops already
+               // collected, so a strict caller whose payload a strip took is told
+               // `ERR_READONLY_FIELD_REJECTED` first, exactly as when the assert
+               // sat ahead of the whole pass. With no throw, one refusal names
+               // every strip drop plus the replacement.
                if (storedExpressionPlan.length > 0) {
-                   const matched = ((await readPriorRows()) ?? []).filter((row) => row != null && typeof row === 'object');
-                   const patch = hookContext.input.data as Record<string, unknown>;
-                   const before = await computeStoredExpressions(storedExpressionPlan, matched, opCtx.context, storedExpressionServing);
-                   const after = await computeStoredExpressions(storedExpressionPlan, matched.map((row) => ({ ...row, ...patch })), opCtx.context, storedExpressionServing);
-                   const settled = settleBulkStoredExpressions(storedExpressionPlan, patch, before, after, object);
-                   emitDrop(
-                     replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, matched.map((row) => ({ ...row, ...settled }))),
-                     'stored_expression',
-                   );
-                   hookContext.input.data = settled as any;
+                   try {
+                       const matched = ((await readPriorRows()) ?? []).filter((row) => row != null && typeof row === 'object');
+                       const patch = hookContext.input.data as Record<string, unknown>;
+                       const before = await computeStoredExpressions(storedExpressionPlan, matched, opCtx.context, storedExpressionServing);
+                       const after = await computeStoredExpressions(storedExpressionPlan, matched.map((row) => ({ ...row, ...patch })), opCtx.context, storedExpressionServing);
+                       const settled = settleBulkStoredExpressions(storedExpressionPlan, patch, before, after, object);
+                       emitDrop(
+                         replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, matched.map((row) => ({ ...row, ...settled }))),
+                         'stored_expression',
+                       );
+                       hookContext.input.data = settled as any;
+                   } catch (err) {
+                       assertNoStrictDrops();
+                       throw err;
+                   }
                }
                assertNoStrictDrops();
                // [#21663] The predicate-path twin of the by-id second pass, at the