fix(objectql): judge a parent-scoped readonlyWhen against the header the write stores (#19853)

[objectstack-fleet]

Fixes #19853

Clause-②: no

What a caller could do before, and what happens now

Before. On a master-detail child whose parent field is readonly: true, a caller with edit rights could change a field locked by a parent-scoped readonlyWhen by naming a different, unlocked parent in the same update. The engine resolved parent from the FK the payload named, judged the lock against that header, and only afterwards stripped the read-only FK — so the row stayed under the locked parent with the locked field rewritten. The same happened when the FK carried its own readonlyWhen lock that kept the row where it was, and on bulk (multi: true) updates for every matched row.

Now. The engine settles whether the update really moves the row BEFORE it judges any parent-scoped lock, and judges every lock (and requiredWhen, which shares the binding) against the parent the row is stored under afterwards. A legitimate move — FK writable, or an isSystem / preserveAudit write the static strip exempts — is still judged against the parent it moves to.

A1 — reproduction on origin/main 2bbb462, engine untouched

Real ObjectQL engine, in-memory driver (the suite's own), the card's shape: inv_line.amount has readonlyWhen: "parent.status == 'paid'", inv_line.invoice is master_detail with readonly: true, l1 (amount 100) under paid inv_a, inv_b open.

• update(l1, { amount: 999, invoice: 'inv_b' }) committed and stored { invoice: 'inv_a', amount: 999 } — reproduced.
• Control update(l1, { amount: 555 }) — locked, amount stayed 100.
• Same run, the other arms: FK's own record-scoped lock → stored amount: 999 under inv_a; FK's own parent-scoped lock → same; bulk stripped repoint → same per row; reverse direction (naming the PAID invoice for a line under an open one) → the amount was dropped (over-lock); requiredWhen: "parent.status == 'paid'" → a note-only edit of an open line naming the paid invoice was refused po_ref is required, and clearing po_ref on a paid line by naming the open one was accepted.

A2 — every strip that can move the FK on this path, from the code

Order on both update branches, after the before-phase hooks: primary-key strip (id only) → readonlyWhen strip → static readonly strip (skipped for isSystem) → assertNoStrictDrops → evaluateValidationRules → reference check → driver.

• readonlyWhen strip — can take the FK when the FK carries its own lock. Ran AFTER parent was resolved from the named FK.
• Static readonly strip — takes a non-system caller's FK when it is readonly: true (unless preserveAudit keeps it, or a hook wrote it). Ran after the readonlyWhen strip.
• Not strips: field-level security refuses a forbidden FK outright (plugin-security step 2.5 throws PermissionDeniedError); the Update-side: a readonly field is stripped from persistence but still reaches beforeUpdate, so hook-derived columns persist values computed from data the row never contains #16344 hide pass withholds static-readonly values from hooks and restores them before any engine-owned read (net zero); beforeUpdate hooks, normalizeMultiValueFields and validateRecord all run before parent is resolved, so their effect is already in the payload the resolution reads; the primary-key strip touches id only.

The fix — packages/objectql/src/engine.ts

settleMasterDetailLanding (one private method, used by the by-id and the bulk branch) runs before the readonlyWhen strip:

1. the FK's own readonlyWhen lock is judged ALONE (the strip's supplied subset holds only the FK) against the header the FK names — Parent-scoped readonlyWhen is unenforced server-side — the field lock fails open, so a paid invoice's frozen lines can be rewritten over the API #4889's verdict for the FK itself, byte-identical to before; the strip that judges the other fields is then handed a supplied without the FK, so that verdict is never re-asked against a different header;
2. the static strip's verdict on the FK is asked of the same stripReadonlyFields, silently, with the same supplied, hookWrittenKeys and preserveAudit; its isSystem gate is read once per branch into a const that both the settlement and the real strip consume (keeps the system-context census at 110 sites, and the two cannot disagree);
3. parent is resolved from the view the write stores — the payload, or the payload without the FK — so masterIdOf falls through to the prior row's FK when the repoint does not land.

The requiredWhen non-regression pre-check's "does this write repoint" is asked of the same stored view. The header read keeps opCtx.context (the #19837 tenant scope) exactly as it was.

A3 — what moves

• parent for every parent-scoped readonlyWhen and requiredWhen is the header the row is stored under (the fix).
• onFieldsDropped for the card's write: [{amount, readonly_when}, {invoice, readonly}] (was [{invoice, readonly}]); a strictReadonlyWrites refusal names ['amount', 'invoice'] with both drops (was ['invoice']).
• Over-lock removed: naming a locked parent beside a stripped FK no longer drops a field of a row that stays under an open parent.
• requiredWhen moves with the shared binding, both directions (see the declaration note below).
• Header reads: a stripped repoint reads the header the row keeps instead of the named one (same count, pinned); the requiredWhen pre-check no longer buys a second read for a repoint that does not land; a parent-scoped lock on the FK ITSELF that refuses the landing costs two reads (named, then kept) where it cost one.
• Warn-line order only: when the FK's own lock fires, its line now prints before the other fields' lines. Report order, strict fields order and message text are unchanged.
• Unmoved: the FK's own verdict; strip order; isSystem / preserveAudit behaviour; every write whose payload does not name the FK.

A4 — the legitimate repoint

With invoice writable (inv_line_free): paid → open update(f1, { invoice: 'inv_b', amount: 999 }) is judged against inv_b and commits both; open → paid update(f2, { invoice: 'inv_a', amount: 999 }) is judged against inv_a and drops the amount while the move lands. In both the judged header is the one the row is stored under because the FK lands: no strip takes it.

Tests

New describe block in packages/objectql/src/engine-readonly-when-parent.test.ts (16 cases): the card, the control, the report, the strict envelope (code ERR_READONLY_FIELD_REJECTED, name, fields, drops; the engine error carries no HTTP status — that mapping is downstream and not measured here), the reverse over-lock, the one-header read, isSystem, preserveAudit, both legitimate repoint directions, the FK's own record- and parent-scoped locks, both requiredWhen directions (refusal asserted on code VALIDATION_FAILED + fields), and bulk in both directions.

Measured on 0a538af9d7:

• pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2 → 305 files / 5105 tests passed (exit 0).
• pnpm --filter @objectstack/objectql test:repo → 1 file / 5 tests passed; pnpm --filter @objectstack/objectql typecheck → exit 0, check:test-typecheck: OK.
• Ablation (committed fix; engine.ts restored to its 2bbb462335 blob ffcda81540 and verified on disk, settleMasterDetailLanding count 0): the parent suite ran 11 failed / 27 passed — every new pin except the five whose behaviour is unchanged (control, isSystem, preserveAudit, both legitimate repoints). Restored with git checkout HEAD --, blob 624fc1e38a equals HEAD:packages/objectql/src/engine.ts, git diff HEAD empty, rerun 38/38 passed. Script carried an EXIT/INT/TERM trap.

Gates on 0a538af9d7

• node scripts/pm/dispatch-gates.mjs --commands → 64 commands; all run with exit codes captured before any pipe; --ran → 64 derived famil(ies) accounted for — 64 run, 0 NOT-MEASURED, exit 0.
• check:dual-build-cjs-loads and check:type-check-debt first answered PREREQUISITE NOT MET (no workspace dist); after turbo run build over ./packages/* and ./packages/*/* (72/72 tasks), both exit 0 (type-check-debt needed an objectql rebuild after the ablation restore touched engine.ts's mtime).
• node scripts/check-issue-citations.mjs (live, the verdict CI blocks on) → exit 0, 13 citations resolve.
• node scripts/check-system-context-census.mjs → exit 0 (110 sites). It was red on the first commit (d4496752a1) for a new isSystem read inside the helper; the second commit hoists the one read per branch instead.
• pnpm check:query-options-erasure → exit 0.
• Lint, narrowed and proven: eslint's own config (ESLint.calculateConfigForFile / isPathIgnored) puts engine.ts and the test file in the linted population and ignores the changeset; eslint --no-inline-config --format json over the three paths → 3 results, 0 errors, 0 warnings on the two linted files; the config enables no type-aware linting and no cross-file rule (its only file reads are two baselines this diff does not touch), so this diff cannot move a verdict on any untouched file. pnpm lint itself is CI's.

Neighbour PR #19728

Textually disjoint: this diff does not touch the evaluateValidationRules lines, the import line, the region after resolveMasterDetailParents or system-context.mdx. Driver-free bare probe (git clone --bare --shared, no merge driver registered): merge-tree --write-tree of 0a538af9d7 against 3b9c5f2fca → exit 0; against origin/main b940f32a56 → exit 0. The merged tree carries both changes.

Acceptance notes

• Sibling on the record root, same defect class, not fixed here (listed for the seat to file). A record-scoped readonlyWhen that reads a static-readonly field is judged against the caller's forged value of that field, which the static strip then removes. Measured on 0a538af9d7: status readonly: true, amount readonlyWhen: "record.status == 'closed'", row closed; update(t1, { amount: 555 }) stays 100, update(t1, { status: 'open', amount: 999 }) stores { status: 'closed', amount: 999 }. Judging the conditional strip over the post-static view would close both roots at once, but it moves reason attribution for fields declaring both locks and the strict message text, so it is a separate decision.
• Parent-scoped readonlyWhen is unenforced server-side — the field lock fails open, so a paid invoice's frozen lines can be rewritten over the API #4889's landing rule still decides the FK's OWN parent-scoped lock: invoice: { readonlyWhen: "parent.status == 'paid'" } judges a move off a paid invoice against the invoice it moves to. Documented behaviour, unchanged here; noted, not filed.

Declaration note for the contract review

The claim declares no widening; copied above as given. Measured movement in update()'s refuse/accept answer, only when a payload names an FK that a strip then takes back out and the object has a parent-scoped requiredWhen: a write refused VALIDATION_FAILED against the named header now commits (the row stays under a header that does not require the field), and a write that cleared a required field under a header that does require it is now refused. Every other movement is a strip (the write still commits) or a strict refusal that was already a refusal. Whether that requirement movement counts as an accept-set change is the review's call.

Seat's disposition (after the contract review): Clause-②: no stands, and patch is right. The accept-direction movement removes a refusal the published text already negated. content/docs/data-modeling/fields.mdx ("Locking a detail from its master: parent") says: "parent = the record on the other end of this object's master_detail field", and "The server binds parent on the update path by reading the master the row points at (a repointing write is judged against the master it lands on)". A row whose repoint never lands points at, and lands on, its stored master, so refusing it against the named header contradicted that text. The refuse-direction movement is a restored guarantee (a requirement bypass now refused). Neither moves an authorable key, an export or an error code.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 68 pages)
• 1 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
• a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b940f32a568e6b07b97b1b08572c632ecfb1660f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from e43cf07983260885ea161f1e6f2a0d396c01d5e1 — the merge of head 0a538af9d76981e45ac4affafdd1db71e140ee74 into base b940f32a568e6b07b97b1b08572c632ecfb1660f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e43cf07983260885ea161f1e6f2a0d396c01d5e1 && git checkout e43cf07983260885ea161f1e6f2a0d396c01d5e1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b940f32a568e6b07b97b1b08572c632ecfb1660f 0a538af9d76981e45ac4affafdd1db71e140ee74 && git checkout -B drift-repro b940f32a568e6b07b97b1b08572c632ecfb1660f && git merge --no-ff 0a538af9d76981e45ac4affafdd1db71e140ee74

node scripts/docs-audit/affected-docs.mjs --json b940f32a568e6b07b97b1b08572c632ecfb1660f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

[objectstack-fleet]

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 0a538af9d76981e45ac4affafdd1db71e140ee74

① Derived judgments

Head = branch tip = PR head; merge-base 2bbb462335; three files. Measured in a detached throwaway worktree, removed afterwards.

• (a) By-id: TRUE. settleMasterDetailLanding runs before the readonlyWhen strip. The FK's own lock is judged alone against the header the FK names. The static strip's verdict comes from the same stripReadonlyFields over the same suppliedValues, hookWrittenKeys and preserveAudit, with the isSystem gate read once into the const the real strip consumes. parent then resolves from the view the write stores. Every combination checked judges the non-FK parent-scoped locks against the header the row keeps: a readonly FK from a non-system caller; isSystem; preserveAudit; a hook-written FK and a hook overwriting a caller-named FK; the FK's own record-scoped lock; its parent-scoped lock refusing, or naming an unresolvable header; strictReadonlyWrites. Nothing else on either branch moves the FK after the settle point: field-level security throws rather than strips.
• (b) Bulk: TRUE. The same helper, per matched row; the FK's own lock goes through stripReadonlyWhenFieldsMulti (安全:readonlyWhen 服务端剥离仅覆盖单条 update 路径,多行 updateMany 不强制(条件只读可被批量绕过) #3042's rule).
• (c) The FK's own lock is still judged against the header it names: TRUE, consistent and documented (Parent-scoped readonlyWhen is unenforced server-side — the field lock fails open, so a paid invoice's frozen lines can be rewritten over the API #4889; content/docs/data-modeling/fields.mdx). The docblock's read-count note ("two only when a parent-scoped lock on the FK itself refuses the landing") is imprecise: a readonly FK with a non-refusing parent-scoped lock also costs two reads. No caller acts on it.
• (d) The requiredWhen binding moves with the shared header (Parent-scoped requiredWhen is unenforced server-side — the same gap #4889 closed for readonlyWhen, one slot over #4977): TRUE in both directions, and each movement is the declared rule, not a new one. Both pins pass at head and fail on the base blob.
• (e) The [finding] master-detail parent binding reads the header with no tenant — parent.* predicates leak another org's header fields; the dangling-reference audit is blind to cross-org references #19837 tenant scope on the header read is untouched.
• (f) Reporting: TRUE. The FK's verdict and the rest arrive as one readonly_when event, then the static strip reports readonly; the strict refusal names both fields on the card's shape. All pinned.
• (g) Every changeset sentence holds. Ablation (engine.ts restored to the base blob): 11 of the 16 new pins fail and exactly the five unchanged-behaviour pins pass; restored: 38/38. All 16 test names describe their bodies. Checks at review time: 28 success, 3 skipped, 0 failed, 2 in progress.

② Semver level

Clause-②: no and patch are right. Nothing authorable, exported or in the error-code ledger moves. The refuse-direction movement is a restored guarantee. The accept-direction movement removes a refusal the published text already negated; per execution-duties such a no must cite that text: content/docs/data-modeling/fields.mdx, "parent = the record on the other end of this object's master_detail field" and "The server binds parent on the update path by reading the master the row points at (a repointing write is judged against the master it lands on)". A row whose repoint never lands points at, and lands on, its stored master. (The seat has added this citation to the PR body.)

③ Boundary flags

1. The class (a) sibling on the record root: REAL, reproduced at head and on the base blob. status readonly: true, amount readonlyWhen: "record.status == 'closed'": update(t1, { status: 'open', amount: 999 }) stores { status: 'closed', amount: 999 } while the control { amount: 555 } stays 100. The same shape through the FK: amount readonlyWhen: "record.invoice == 'inv_a'" with a read-only invoice: update(r1, { invoice: 'inv_b', amount: 999 }) stores { invoice: 'inv_a', amount: 999 }. Pre-existing and outside this card's property. FILE SEPARATELY (priority:p1, security suggested); it must not block this PR.
2. The Parent-scoped readonlyWhen is unenforced server-side — the field lock fails open, so a paid invoice's frozen lines can be rewritten over the API #4889 landing rule on the FK's own parent-scoped lock: documented and pinned, unchanged. ACCEPTABLE.
3. Neighbour PR feat(formula,objectql): read one hop through a lookup in a validation predicate #19728: merge-tree clean against its tip 3b9c5f2fca and against origin/main 1f89ba0d70. ACCEPTABLE.
4. The read-count note in the docblock and the PR body: ACCEPTABLE; a one-clause correction can ride a later touch.
5. User docs are untouched and still true after the fix. ACCEPTABLE.

Implemented-by: claude/issue-19853-readonlywhen-post-strip-parent
Reviewed-by: session_01TEhopqrWQYBycZzyJHpAZr

VERDICT: PASS

---

Generated by Claude Code