Skip to content

fix(objectql)!: under strictReadonlyWrites, a write whose value a stored expression replaces is refused, and nothing is written - #25086

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-25000-strict-stored-expression
Oct 21, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-25000-strict-stored-expression

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Closes #25000
Clause-②: no (narrowing: under strictReadonlyWrites, a write whose value a stored expression replaces is refused instead of reported; it ships now under the amended 「甲」, #6015 6303865025)

What changes

strictReadonlyWrites refuses every drop that onFieldsDropped reports. Since the stored_expression reason landed (#24690, PR #24778 7532ea3c08), one reported drop was left out of the refusal: a caller value for a stored (non-formula) field that declares an expression, which the engine replaces with the computed value. Under strict such a write completed and was only reported. The amended ruling 「甲」 (#6015 6303865025) ships the refusal in 18.x as a BREAKING minor, and triage re-timed this card to 18.x (6305854988).

  • Insert (single, batch, insertMany). The strict check in engine.insert now judges insertDrops, the one event list that includes the stored-expression replacement, after the compute pass. Before, it judged only the strips' drops, ahead of that pass. A replaced value throws ReadonlyFieldRejectedError (operation: 'insert') before validation and before any driver dispatch, and no listener fires. insertMany delegates to insert, so it refuses the whole call, as it does for the other strict drops.
  • Update by id and predicate update. The stored_expression report now goes through emitDrop, the channel every strip reports through, and no longer calls notifyDrop directly. On both branches the one assertNoStrictDrops() call now comes after the stored-expression pass. So under strict the write is refused before the statement and nothing is written. An object that declares no stored expression skips that pass, so its refusal happens exactly where it did before.
  • One refusal names everything. A payload that carries another strip's drop as well as a replaced value is now refused once, naming both, with stored_expression last. Before, it was refused for the other drop alone.
  • Wording. REASON_PHRASE.stored_expression (written by spec(data): once PR #24673 lands, a client value for a stored-expression field is overwritten with no report — DroppedFieldsEvent.reason has no value for "written, but recomputed", so onFieldsDropped / droppedFields stay silent #24690 ahead of this refusal) is reused. The per-reason message gains a stored_expression remedy (leave the field out, or send the computed value). That remedy is spoken only when the reason is present, so every other refusal's message is unchanged byte for byte, by the same rule the computed remedy follows.
  • Docs. The TSDoc in packages/spec/src/contracts/data-engine.ts (strictReadonlyWrites) and packages/spec/src/data/data-engine.zod.ts (DroppedFieldsEvent.reason) no longer names stored_expression as the arm strict does not refuse. The objectql comments in engine.ts and readonly-strict-errors.ts follow. No .describe() text moved: check:generated reports all 14 artifacts current, so the generated reference docs have nothing to regenerate.

Unchanged: a strict write that sends the computed value, or no value, is not refused. A stored expression field that is also readonly is still refused once, as readonly. Without strict, the write completes with the computed value and is reported. The error code is unchanged.

Tests

packages/objectql/src/engine-stored-expression-dropped-fields.test.ts: the strict block changes from "reported, not refused" to "refused, nothing written". It covers update by id, insert single, batch, insertMany and predicate update. Each refusal asserts the code, drops (and operation/fields where they matter), that the store is unchanged, and that the listener stayed silent. A non-strict control sits beside each refusal (the write succeeds and is reported), together with the strict controls for "computed value sent / no value". The union case is also covered (readonly_when + stored_expression in one refusal), and so is the readonly-only insert refusal.

All runs were taken on the final code commit a8e9d3611c:

  • pnpm --filter @objectstack/objectql test: 455 files / 8569 tests passed. pnpm --filter @objectstack/objectql typecheck: tsc --noEmit clean, and check:test-typecheck OK (ledger held).
  • pnpm --filter @objectstack/spec build && check:generated: all 14 generated artifacts up to date. spec test: 705 files / 21424 passed (1 todo). spec typecheck: OK. spec test:repo: 72 files / 1237 passed.
  • node scripts/pm/dispatch-gates.mjs --commands derived 94 commands from merge base 688052e14, and all 94 exited 0. On the first pass, check:dual-build-cjs-loads and check:type-check-debt printed PREREQUISITE NOT MET (exit 3, nothing measured). I re-ran check:type-check-debt under the verify lock: its closure build ran and it re-measured OK, none above its recorded number. That full build also produced the missing dist/ trees, and check:dual-build-cjs-loads then passed (112 entry points across 69 packages load). dispatch-gates --ran reconciles 94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN. The changeset gates are among the 94: check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check:changeset-gate-self-tests.

Ablation. The fix was committed first. Each leg went through scripts/ablation-replace.mjs, with an anchor hit of 1 to 0 and the blob proven back to the HEAD blob 3e8c0ec2bd1a with git diff HEAD empty. The suite imports ./engine.js from source, so no build is involved.

  • A, update paths: emitDrop lets stored_expression past strict. Three tests go red (update by id, predicate update, the union case), and the other 22 stay green.
  • B, insert: the strict check ignores a stored_expression-only list. Three tests go red (insert single, batch, insertMany), and the other 22 stay green.

Producers measured

No non-test caller in this repository passes strictReadonlyWrites: the examples, packages/qa dogfood and the runtime and REST doors pass none. The option is also not on the serializable options bag, so no wire caller can set it. No in-repo producer moves. In the objectql suite, the in-package strict tests on stored-expression objects (engine-stored-expression-write.test.ts, engine-write-formula-hydration.test.ts, engine-formula-write-strip.test.ts) stay green unchanged.

Changeset

.changeset/25000-strict-stored-expression-refusal.md: @objectstack/objectql minor, with a BREAKING section that maps FROM (completed, reported) TO (refused, nothing written), gives the remedy, and carries the claim's Clause-② line and an ADR-0087 not-required (no-migration-prescription) marker (the precedent shape of bb4f5cc005). @objectstack/spec is not listed: only TSDoc comments moved there, and no .describe() text or export did.

Acceptance notes

Open PRs touching the same files were checked: #25072 and #24893 edit engine.ts in disjoint hunks (lines 4628–6300 and 9043–12250). No open PR touches data-engine.zod.ts, contracts/data-engine.ts or readonly-strict-errors.ts.


Generated by Claude Code

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/spec, touching 5 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/data-engine.mdx (via WriteObservabilityOptions (symbol, a top-level interface))
What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 — 145 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 5050442bb91feff30aa8d4211daac6158331058a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 20b027eca9197bba1e0dea98c36d5180d365b794 — the merge of head 54a1cf34b6544898ecb4606b18b83f33cf5eb2cc into base 5050442bb91feff30aa8d4211daac6158331058a, 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 20b027eca9197bba1e0dea98c36d5180d365b794 && git checkout 20b027eca9197bba1e0dea98c36d5180d365b794
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5050442bb91feff30aa8d4211daac6158331058a 54a1cf34b6544898ecb4606b18b83f33cf5eb2cc && git checkout -B drift-repro 5050442bb91feff30aa8d4211daac6158331058a && git merge --no-ff 54a1cf34b6544898ecb4606b18b83f33cf5eb2cc

node scripts/docs-audit/affected-docs.mjs --json 5050442bb91feff30aa8d4211daac6158331058a

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 5050442bb91feff30aa8d4211daac6158331058a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…ed-expression pass

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

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 54a1cf34b6544898ecb4606b18b83f33cf5eb2cc
Local-runs: none
Scope: packages/objectql/src/engine.ts, packages/objectql/src/readonly-strict-errors.ts, packages/objectql/src/engine-stored-expression-dropped-fields.test.ts, packages/spec/src/contracts/data-engine.ts, packages/spec/src/data/data-engine.zod.ts + .changeset/25000-strict-stored-expression-refusal.md

① Derived judgments

Round 2. Round 1, at head a8e9d3611c, failed on one blocker: on the predicate path, settleBulkStoredExpressions' ValidationError fired before assertNoStrictDrops(), which broke the #5126 ordering. The branch since then is one fix commit, 0e9eba5e78, plus two clean merges of origin/main. Each merge's diffs against both parents equal the other side's changes, so no conflict resolution touched unrelated logic.

The blocker is fixed on all three paths. Every throwing stored-expression pass sits in a try. The catch runs assertNoStrictDrops() and then rethrows the original error:

  • Predicate update, engine.ts:18836-18852: readPriorRows, both computes and settle.
  • By-id update, :18527-18540.
  • Insert, :16819-16831: the catch uses the strip-only droppedFieldEvents. insertMany routes through insert.

At the moment of the throw, the stored_expression replacement is not yet in the drop set. With no strict strip drops, the original error passes through unchanged. On the no-throw path, one refusal still names every strip drop plus the replacement, with stored_expression last. The new ordering pin is discriminating: its non-strict control is pinned to VALIDATION_FAILED.

Round 1's verified list still holds:

  • every strict refusal precedes validation and any driver dispatch;
  • no other caller of computeStoredExpressions;
  • error code ERR_READONLY_FIELD_REJECTED;
  • every other message byte-identical.

Non-blocking:

  • by-id keeps emitDrop and applyStoredExpressionsToPatch outside its try, while the predicate path keeps them inside. Both are pure, so the asymmetry is cosmetic;
  • insert assigns computed values onto the in-memory row copies before the refusal. Nothing reaches a driver.

② Semver level

③ Boundary flags

Implemented-by: claude/issue-25000-strict-stored-expression
Reviewed-by: session_01D6F9axfoPzR7t8G1J7Ytzz

VERDICT: PASS

Seat re-read of the check-runs at landing: all 42 runs on 54a1cf34b6 have completed: 35 succeeded, the rest were skipped, and none is red.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation protocol:data size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

objectql 18.x: under strictReadonlyWrites, refuse a write whose value a stored expression replaces (stored_expression), the deferred half of #24690

2 participants