From 6e9b63d5e74155352b786234b698dfa22455a451 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 17:44:55 +0000 Subject: [PATCH] docs(objectql): record the lock-face/static-face asymmetry and pin both faces `isCallerSuppliedValue`'s docblock claimed the readonlyWhen seam runs "the identical two-part test `stripReadonlyFields` applies" and is "textually parallel" with it. Neither has held since #14088 moved the static face onto a hook-write record and #14472 did the same to the insert-side twin. The divergence is deliberate and lock-motivated, so it is now stated where the test lives, with the reason each face guards a different thing. Adds the static face's missing measurement pin (a lone self-assigning hook leaves the CALLER value on the key) beside #14088's own suite, and cross-links it with `LOCK 3b`, which pins the opposite verdict for the identical hook spelling. Comment- and test-only; no behaviour moves. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../lock-face-value-equality-recorded.md | 30 ++++++++++ ...ngine-readonly-strip-caller-values.test.ts | 48 ++++++++++++++++ ...ngine-readonly-when-derived-writes.test.ts | 30 ++++++++++ .../objectql/src/validation/rule-validator.ts | 55 ++++++++++++++++++- 4 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 .changeset/lock-face-value-equality-recorded.md diff --git a/.changeset/lock-face-value-equality-recorded.md b/.changeset/lock-face-value-equality-recorded.md new file mode 100644 index 0000000000..46a53f3fb0 --- /dev/null +++ b/.changeset/lock-face-value-equality-recorded.md @@ -0,0 +1,30 @@ +--- +'@objectstack/objectql': patch +--- + +Record the deliberate asymmetry between the two hook-vs-caller seams, and pin both faces + +`stripReadonlyFields` (#14088) and its insert-side twin `stripRuntimeOwnedFields` (#14472) +decide "hook write or caller forgery?" from a RECORD of the keys the before-phase hook +chain assigned. `isCallerSuppliedValue`, behind `stripReadonlyWhen{Fields,FieldsMulti}`, +stays on VALUE EQUALITY. That divergence is now a ruled, documented decision instead of a +docblock claiming the two tests are identical. + +Why the faces differ: the static face guards an author-declared `readonly` or a +runtime-owned column, where hook authorship *is* the exemption on offer — so "an +assignment ran" is the right evidence, and the record's blindness to the value is what +makes it correct. The lock face guards a `readonlyWhen` STATE lock, whose whole guarantee +is that no caller write survives a TRUE predicate; there, the same blindness would let a +line spelled `data.x = data.x` — or a normalisation that is the identity for canonical +input — hand the caller's own value hook ownership and silently unlock the lock. + +No behaviour changes. On a `readonlyWhen` lock, a before-phase assignment that writes back +the value already on the key is still not a hook write: the caller's value is stripped, +with the same warning and the same `onFieldsDropped` / `strictReadonlyWrites` reporting. +The accepted residual — a hook that genuinely derives a locked field loses its write when +the caller echoed the identical value — is stated in the code rather than left to be +rediscovered; no instance of it exists in the tree. + +Each face now carries a measurement pin, written to be read side by side: `MEASURED: a +lone self-assigning hook leaves the CALLER value on the key` on the static face, and +`LOCK 3b` on the lock face, pinning the opposite verdict for the identical hook spelling. diff --git a/packages/objectql/src/engine-readonly-strip-caller-values.test.ts b/packages/objectql/src/engine-readonly-strip-caller-values.test.ts index 189a345111..986d3b0aaf 100644 --- a/packages/objectql/src/engine-readonly-strip-caller-values.test.ts +++ b/packages/objectql/src/engine-readonly-strip-caller-values.test.ts @@ -714,6 +714,54 @@ describe('the strip reads hook-write PROVENANCE, not value equality (#14088)', ( expect(task('t_18').completed_at).toBeNull(); }); + // ── The measured consequence of "an assignment ran", stated out loud ─────── + + it('MEASURED: a lone self-assigning hook leaves the CALLER value on the key', async () => { + // ⚠️ RECORDING BEHAVIOUR, NOT BLESSING IT. The direct consequence of the + // mechanism #14088 chose: the record says an ASSIGNMENT RAN and is + // deliberately blind to the VALUE (that blindness is the whole repair — it + // is what separates "the hook wrote the null the caller also sent" from + // "the hook never touched the key"). So `ctx.input.data.completed_at = + // ctx.input.data.completed_at`, which computes nothing, is a `set`, the + // caller's forged timestamp becomes hook-owned, and it survives the strip. + // + // Pinned so the consequence is VISIBLE rather than discovered later. The + // transition hook is deliberately NOT registered here, so the self-assign + // is the only write to `completed_at` — which is what makes the surviving + // value the caller's rather than a platform stamp. + // + // ⚠️ READ THIS BESIDE `LOCK 3b` (#9107, + // `engine-readonly-when-derived-writes.test.ts`), which pins the OPPOSITE + // verdict for the SAME hook spelling: there the caller's value is stripped + // to `null`. The two are not in conflict and neither is stale — they are + // the two faces of one recorded asymmetry (#14259, maintainer ruling B): + // + // - HERE the strip guards an author-declared `readonly` COLUMN, where hook + // authorship IS the exemption on offer, so "an assignment ran" is the + // right evidence and being blind to the value is correct; + // - THERE it guards a `readonlyWhen` STATE LOCK, whose entire guarantee is + // that no caller write survives a TRUE predicate (#4889's frozen + // paid-invoice lines). The same blindness would let this exact line + // launder the caller's own value past the lock, so that seam keeps value + // equality on purpose. Measured: threading the record in there turns + // `LOCK 3b` red. + // + // A future ruling that reverses this INVERTS both pins together; it never + // deletes either. `isCallerSuppliedValue`'s docblock carries the argument. + const FORGED = '1999-01-01T00:00:00.000Z'; + engine.registerHook('beforeUpdate', async (ctx: any) => { + ctx.input.data.completed_at = ctx.input.data.completed_at; + }, { object: 'duly_task', priority: 50 }); + seedDone('t_20'); + + await engine.update('duly_task', { + id: 't_20', status: 'in_progress', completed_at: FORGED, + }); + + expect(task('t_20').completed_at).toBe(FORGED); + expect(warns).toEqual([]); + }); + it('the recording is transparent to a hook reading its own payload', async () => { // Hooks read `ctx.input.data` for diagnostics (plugin-auth's identity write // guard NAMES the keys it found). The recording view must be indistinguishable diff --git a/packages/objectql/src/engine-readonly-when-derived-writes.test.ts b/packages/objectql/src/engine-readonly-when-derived-writes.test.ts index 217c9e1278..5bf7397a44 100644 --- a/packages/objectql/src/engine-readonly-when-derived-writes.test.ts +++ b/packages/objectql/src/engine-readonly-when-derived-writes.test.ts @@ -221,6 +221,36 @@ describe('readonlyWhen strips CALLER-submitted values only (#9107)', () => { // `ctx.input.data.x = ctx.input.data.x` has written nothing, and `Object.is` // says so. Pinned because "a hook touched this key" is exactly the weaker // rule that WOULD open a laundering path. + // + // ⚠️ READ THIS BESIDE `MEASURED: a lone self-assigning hook leaves the + // CALLER value on the key` (`engine-readonly-strip-caller-values.test.ts`, + // and its insert-side twin in + // `engine-hook-provenance-sibling-seams.test.ts`), which pins the OPPOSITE + // verdict for the IDENTICAL hook spelling: there the caller's forged value + // SURVIVES. Neither pin is stale. They are the two faces of one recorded, + // deliberate asymmetry (#14259, maintainer ruling B): + // + // - THERE the strip guards an author-declared `readonly` or runtime-owned + // COLUMN. Hook authorship IS the exemption on offer, so #14088's record + // of "the chain assigned this key" is the right evidence, and its + // blindness to the VALUE is exactly what makes it correct — it is what + // separates "the hook wrote the null the caller also sent" from "the + // hook never touched the key". + // - HERE the strip guards a `readonlyWhen` STATE LOCK, whose entire + // guarantee is that NO caller write survives a TRUE predicate (#4889's + // frozen paid-invoice lines). That same blindness would let this exact + // line — or a normalisation that is the identity for canonical input — + // hand the CALLER's value hook ownership and silently unlock the lock. + // So this seam keeps VALUE EQUALITY, on purpose. Measured on #14472's + // branch: threading the record into `isCallerSuppliedValue` turned this + // very test red (`closed_note` committed the forgery where the lock had + // stripped it to `null`). + // + // Accepted residual, stated rather than hidden: a hook that genuinely + // DERIVES a locked field loses its write when the caller echoed the + // identical value. No instance exists in the tree. A ruling that reverses + // this INVERTS this pin and its sibling together; it never deletes either. + // `isCallerSuppliedValue`'s docblock carries the argument. engine.registerHook('beforeUpdate', async (ctx: any) => { if (Object.prototype.hasOwnProperty.call(ctx.input.data, 'closed_note')) { ctx.input.data.closed_note = ctx.input.data.closed_note; diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 10b9faf794..6013782d06 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -535,12 +535,17 @@ function readonlyWhenBindings( * * So `supplied` is the CALLER's payload as snapshotted at ENGINE ENTRY, before * any middleware or hook ran, and a key is judged only when it is still the - * caller's — the identical two-part test `stripReadonlyFields` applies: + * caller's — a two-part test, spelled out in {@link isCallerSuppliedValue}: * * 1. the key is an OWN property of `supplied` (a key a hook ADDED is not), and * 2. the payload still holds the caller's VALUE by `Object.is` (a key a hook * OVERWROTE now carries a platform value, not a forgery). * + * `stripReadonlyFields` began with those same two parts and NO LONGER HAS THEM: + * #14088 gave it a hook-write RECORD, asked ahead of part 2. This seam keeps + * value equality — a deliberate, lock-motivated divergence (#14259), argued + * where the test itself lives ({@link isCallerSuppliedValue}). + * * ⚠️ **This does not weaken the lock at the API boundary, and the reason is * that a caller cannot reach the exempt side of either test.** To be treated as * hook-written, a value must differ from what arrived at engine entry — which @@ -568,8 +573,52 @@ interface ReadonlyWhenStripOptions { /** * Is this key still the CALLER's to be judged? (#9107 — see * {@link ReadonlyWhenStripOptions}.) Shared by the single-id and bulk strips so - * the two can never disagree about what "caller-supplied" means, and written to - * be textually parallel with the same test inside {@link stripReadonlyFields}. + * those two can never disagree about what "caller-supplied" means. + * + * ## ⚠️ It is NOT the same test as {@link stripReadonlyFields}' any more (#14259) + * + * It once was, and this docblock said so. #14088 moved that one off value + * equality onto a RECORD of the keys the before-phase hook chain actually + * assigned (`options.hookWrittenKeys`), and #14472 did the same to the + * insert-side {@link stripRuntimeOwnedFields}. This predicate deliberately + * stayed behind. The divergence is not drift, and it is not a port nobody got + * to — it is decided by what each face GUARDS: + * + * - **The static face** ({@link stripReadonlyFields} and its insert-side twin) + * guards an author-declared `readonly` or a runtime-owned COLUMN. Hook + * authorship is precisely the exemption it means to grant, so "the chain + * assigned this key" is the right evidence for it, and value equality was + * only ever a proxy for it — a proxy that collapses *the hook deliberately + * wrote the value the caller also sent* into *the hook never touched it*. + * - **The lock face** (this predicate, behind {@link stripReadonlyWhenFields} + * and {@link stripReadonlyWhenFieldsMulti}) guards a `readonlyWhen` STATE + * lock, whose entire guarantee is that NO caller write survives a TRUE + * predicate (#4889's frozen paid-invoice lines). A record of assignments is + * blind to the value by design — that blindness is what makes it correct on + * the static face — so here it would let an innocuous before-phase line + * spelled `data.x = data.x`, or a normalisation that is the identity for + * canonical input, hand the CALLER's own value hook ownership and silently + * unlock the lock. Measured, not feared: threading the record into this + * predicate turned the #9107 pin `LOCK 3b` red. + * + * So on a `readonlyWhen` lock a hook assignment that writes back the value + * already on the key **is not a hook write**: the caller's value is stripped, + * with the existing warning and the same `onFieldsDropped` / + * `strictReadonlyWrites` reporting. The residual is accepted and recorded + * rather than hidden — a hook that genuinely DERIVES a locked field loses its + * write when the caller echoed the identical value. No instance of that exists + * in the tree; the shape that would buy a repair is hook ownership declared as + * an act on the hook context, which both faces would then read. + * + * Each face carries a measurement pin, written to be read side by side: + * - static face — `MEASURED: a lone self-assigning hook leaves the CALLER + * value on the key` in `engine-readonly-strip-caller-values.test.ts` + * (update side) and `engine-hook-provenance-sibling-seams.test.ts` (insert); + * - lock face — `LOCK 3b` in `engine-readonly-when-derived-writes.test.ts`, + * which pins the OPPOSITE verdict on the same hook spelling. + * + * Both record what IS. Reversing this ruling inverts them; it never deletes + * them. */ function isCallerSuppliedValue( data: Record,