fix(objectql): a TRUE readonlyWhen no longer strips hook-derived values — the conditional strip judges API-boundary callers only (#9107)

.changeset/readonly-when-supplied-values.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/objectql": minor
+---
+
+fix(objectql): a TRUE `readonlyWhen` no longer strips hook-derived values — the conditional strip judges only API-boundary callers (#9107)
+
+`stripReadonlyWhenFields` runs AFTER the before-phase hooks and was keyed on
+`name in data` over the POST-hook payload, so a value a `beforeUpdate` hook
+computed was judged exactly like a key the caller forged. Unlike the static
+`readonly` strip immediately beside it, it carried no `isSystem` exemption
+either — so a field locked by a TRUE predicate had **no server-side write path
+at all**: a hook derived it and the strip deleted it; a cron or plugin wrote it
+with `{ context: { isSystem: true } }` and the strip deleted that too.
+
+Net effect before this change: the derived-field pattern (hook-computed column)
+and a conditional form lock could not coexist on one field. An author wanting
+"visible on the form but locked" **and** "recomputed by a hook" had no
+spec-compliant spelling, and the failure was silent behind an HTTP 200.
+
+Measured downstream (steedos-labs/os-project-titanwind-ehr#1446):
+`equipment.next_maintenance_date` is hook-derived (last maintenance date + cycle
+days) and declared with an always-true `readonlyWhen` to render
+visible-but-locked. After a maintenance sign-off the recompute never landed, and
+a scheduler keyed on that date regenerated the same maintenance plan on every
+scan — a user-visible duplicate-plans loop, diagnosed only by reading the
+engine's strip order in the dist bundle.
+
+The conditional strip now carries the exact key discipline #5591 gave the static
+one, on **both** branches (by-id and multi-row) off one engine-entry snapshot: a
+key is judged only while it is still an own property of the caller's payload as
+it arrived at engine entry AND still holds that caller's value by `Object.is`. A
+key a hook added, or overwrote, is a server value and survives.
+
+**The API-boundary lock is unchanged, and a caller cannot launder a write
+through the hook phase.** To reach the exempt side of either test a value must
+differ from what arrived at engine entry — which only server code can arrange. A
+client that echoes the locked key back is stripped exactly as before; if a hook
+overwrites that key, what persists is the **hook's** value, never the client's.
+`isSystem` is still deliberately NOT an exemption for `readonlyWhen`: a state
+lock that any system-context write could bypass would not be a state lock (the
+frozen paid-invoice-lines case depends on it).
+
+What moves for callers:
+
+- A `beforeUpdate` hook may now write a field locked by a TRUE `readonlyWhen`.
+ This is the sanctioned channel for a conditionally-locked derived field.
+- `onFieldsDropped` no longer reports such a key under `readonly_when` — it is
+ written, not dropped, so reporting it would make the observability seam lie.
+- `strictReadonlyWrites` no longer refuses a write whose only `readonlyWhen`
+ "drop" was a hook's own value; a caller-supplied locked field is still refused.
+- The `ERR_READONLY_FIELD_REJECTED` refusal message's `readonlyWhen` remedy
+ clause now reads "every **API-boundary** caller, isSystem included" and names
+ the hook path. The error `code` is unchanged; a pin on the exact message text
+ moves with it.
+
+If an app relied on the strip discarding a hook's own write to a locked field,
+that write now lands — remove the hook assignment, or narrow the predicate.

content/docs/data-modeling/fields.mdx

@@ -375,6 +375,42 @@ protocol 17 (#3855): authoring it is rejected with an error naming the
 replacement, not silently stripped, and `os migrate meta --from 16` rewrites
 existing sources automatically.
 
+#### Who the lock applies to — and the derived-field write path
+
+A TRUE `readonlyWhen` predicate locks the field for **every API-boundary
+caller**. Unlike static `readonly`, passing `{ context: { isSystem: true } }`
+does **not** exempt it: a state lock any system-context write could bypass would
+not be a state lock, and the frozen-paid-invoice case depends on it.
+
+What the lock does **not** cover is the server's own trusted code. A value a
+`beforeUpdate` hook derives — or writes over a key the caller also sent — is a
+server value, not a caller write, and is never stripped. That is what makes
+"visible on the form but locked, and recomputed by a hook" a spelling the
+platform supports on one field:
+
+```typescript
+// The field renders locked, and only the hook may move it.
+next_maintenance_date: Field.date({
+ label: 'Next Maintenance',
+ readonlyWhen: P`true`,
+}),
+```
+
+```typescript
+// The hook is the write path.
+beforeUpdate: (ctx) => {
+ if ('period_days' in ctx.input.data) {
+ ctx.input.data.next_maintenance_date = addDays(today, ctx.input.data.period_days);
+ }
+},
+```
+
+A client cannot use this to launder a write. The engine snapshots the caller's
+payload **at entry**, before any middleware or hook runs, and judges a key only
+while it still holds that value — so echoing the locked key back never exempts
+it, and if a hook overwrites it, the value that persists is the hook's. Either
+way the caller's value does not land.
+
 #### Locking a detail from its master: `parent`
 
 On a **detail** object — one that declares a `master_detail` relationship — a

content/docs/kernel/contracts/data-engine.mdx

@@ -306,7 +306,7 @@ The strips these two options cover are the engine's legal ones:
 | Strip | `reason` | Verbs | Writers it skips |
 |:---|:---|:---|:---|
 | Static `readonly: true` (#2948) | `readonly` | `update` | `isSystem` |
-| A TRUE `readonlyWhen` predicate (#3042) | `readonly_when` | `update` | none — every caller, `isSystem` included |
+| A TRUE `readonlyWhen` predicate (#3042) | `readonly_when` | `update` | none at the API boundary — every caller, `isSystem` included; a value a `beforeUpdate` hook derived or overwrote is not a caller write and is never stripped (#9107) |
 | Implicitly-readonly runtime-owned type (#5503 — `RUNTIME_OWNED_FIELD_TYPES`, today `autonumber`) | `readonly` | `insert` **and** `update` | `isSystem`, `preserveAudit` (#3493) |
 | Primary-key strip of a payload `id` the update dispatch already ruled is not an identifier (#6437) | `primary_key` | `update` | none |
 

packages/objectql/src/engine-dropped-fields-primary-key.test.ts

@@ -291,10 +291,18 @@ describe('#6437 — the refusal message is composed from `drops`, not from the c
  expect(err.message).toContain('SCALAR id');
  });
 
- it('the READ-ONLY-only message is byte-identical to the pre-#6437 text', async () => {
- // The compatibility half. Every caller and pin written against #5126's /
- // #5503's wording must be untouched by a change that only teaches the
- // error about a class those payloads do not carry.
+ it('the READ-ONLY-only message is unmoved by a NEW REASON CLASS', async () => {
+ // The compatibility half, and read it for what it actually asserts: adding
+ // a `reason` (#6437's whole change) must not disturb the wording payloads
+ // that carry no such class see. That property still holds.
+ //
+ // [#9107] What it never claimed is that the sentence is frozen for all
+ // time. The remedy clause moved ONCE, deliberately, when the maintainer
+ // ruled the conditional strip judges only API-boundary callers: the old
+ // text told a server author its hook-derived write was locked out, which
+ // stopped being true. A remedy sentence that has gone false is the one
+ // thing a refusal message may not keep — so the pin moves WITH the ruling,
+ // and stays a byte-exact pin so the next unintended drift is still caught.
  const { err } = await refuse({ id: 'rec_1', settled_total: 99 }, {});
  expect(err.message).toBe(
  `Update on 'task' was REFUSED: 1 caller-supplied field(s) ` +
@@ -304,7 +312,9 @@ describe('#6437 — the refusal message is composed from `drops`, not from the c
  `server-side code that legitimately writes read-only columns, pass ` +
  `{ context: { isSystem: true } } (this exempts statically 'readonly' fields, but NOT ` +
  `fields locked by a TRUE 'readonlyWhen' predicate — those stay locked for every ` +
- `caller). To let the strip happen and merely observe it, drop ` +
+ `API-boundary caller, isSystem included. A value DERIVED by a beforeUpdate hook is ` +
+ `not a caller write and is never stripped — that is the sanctioned write path for a ` +
+ `conditionally-locked derived field). To let the strip happen and merely observe it, drop ` +
  `strictReadonlyWrites and pass options.onFieldsDropped instead (#3407).`,
  );
  });