Skip to content

engine.update() has no declared-field door either — an undeclared key still reaches the driver, after the beforeUpdate hooks have run #8738

Description

@hotlong

Found while implementing #8682 (PR #8737), and deliberately left out of that PR's scope — filing it rather than widening a card that was scoped to the insert path.

What #8737 fixed, and what it did not

#8682 measured the INSERT path: an undeclared write key was refused only at the very end, by the driver, after the defaults, the beforeInsert hooks, owner/creator resolution and an AUTONUMBER had all been produced for a request that was already going to be refused. PR #8737 adds a declared-field door at the top of insert()'s middleware body, so the schema refuses the key first.

update() got no such door. The two write paths sit in the same file and share the shape of the problem.

Evidence — measured vs inferred, stated separately

Measured (on origin/main @ 3508678, and the basis for #8737):

  • Nothing in packages/objectql/src/engine.ts performs any field-existence check on the write payload. A repo-wide read of the write paths found no such guard on either verb; the only unknown-field doors are on the READ path (assertProjectionHasNoDottedPaths and the plain-column filters in find / findOne).
  • mapDataError in packages/rest/src/rest-server.ts says so in its own comment, for both verbs: "this is a last-resort safety net — the validation layer should ideally reject these before they reach the driver (see follow-ups on unknown-field rejection + provenance-aware required checks)". The UPDATE phrasings (column "c" of relation "t" does not exist, table t has no column named c) are handled by that same branch.

Inferred, not measured — I did not run an update-path reproduction, so this needs confirming before anyone implements:

  • an undeclared key on engine.update(...) reaches the driver and is refused there;
  • the beforeUpdate hooks run first, so a hook that stamps a ledger, calls out, or derives a field executes for a request that is then refused.

Why it is milder than the insert half, and why it is still worth doing

There is no autonumber on the update path, so the permanent, user-visible damage that made #8682 urgent — a document-number gap across a rejected request — has no equivalent here. What remains is the hook side effect and the general "declared = enforced" principle (Prime Directive #10): an undeclared key should be refused by the schema, not by the storage engine.

Note the accept-set half of this is not free, and #8737 is the reference for what it costs: adding the insert door turned up 49 failures in the objectql suite alone, every one an accept-set narrowing — 40 of them fixtures whose registry stub carried an empty or incomplete field map, and 24 caused by four sys_metadata stubs missing a column the real object declares. The same triage should be expected here, and #8737's three deliberate no-opinion cases (absent field map, empty field map, id / created_at / updated_at) are the rules to reuse rather than re-derive.

Not affected: the value-logging half

Update operation failed already redacts the bound statement — #8737 applied the same redactBoundStatement to the insert, update and delete loggers, since an UPDATE inlines the caller's values in its set clause exactly as an INSERT does in its values list. Only the ORDERING half is open here.

Suggested landing: packages/objectql/src/engine.ts, reusing undeclaredInsertFieldErrors (rename it if it grows a second caller) with the update path's own placement — before beforeUpdate and before the readonly strips.


Generated by Claude Code

Activity

  1. added theissue type on Aug 14, 2026
  2. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Triage: lands in packages/objectql/src/engine.ts → domain:engine-core, type Bug, pm:queue. 本评论来自分诊座位 Routine。

    Accept-set reading (why Bug and not Feature): an undeclared update key is already refused today — by the driver, at the end. Moving the refusal to the schema leaves the accept set unchanged and restores declared = enforced (PD #10). That is a bug/tidy-up under the mechanical boundary test, not a widening.

    Premise state — read before implementing. The card is honest that its central claim is inferred, not measured: nobody ran an update-path reproduction. That is the first deliverable, not an assumption to build on. premise_still_valid: false with no PR is a legitimate delivery here if the update path turns out to have a door the insert path lacked.

    Sequencing, verified rather than taken from the card: #8682's fix is on origin/main as 8a9e7f4 (PR #8737), so undeclaredInsertFieldErrors and redactBoundStatement are both in the tree and the update-side work is a sibling of landed code, not of a branch.

    ⚠️ Same-file serial constraint: #8608 is in flight in this lane and its file surface may reach packages/objectql. Whoever claims this re-reads #8608's claim comment for the declared file surface first, per the claim protocol — the engine is a hot file and this card touches the write middleware body directly.

    Scope note carried from the card: #8737's insert door turned up 49 accept-set narrowings in the objectql suite alone (40 fixture field-map stubs, 24 from four sys_metadata stubs). Expect the same triage on the update side, and reuse #8737's three deliberate no-opinion cases (absent field map, empty field map, id/created_at/updated_at) rather than re-deriving them.


    Generated by Claude Code

  3. self-assigned this
    on Aug 14, 2026
  4. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Claim: PM loop round 7
    Session: session_01XeQRiAa7vYRVX5Fog7Zby8
    Branch: claude/issue-8738-update-declared-field-door
    Worktree: objectstack-issue-8738
    Domain: domain:engine-core
    File surface: packages/objectql/src/engine.ts (region: update()'s middleware body, before beforeUpdate and before the readonly strips) and the fixtures the door turns red (stop on breach; explain in the report)
    Container & model: M–L, mode:subagent, model: opus
    Serial constraints cleared: packages/objectql/src/engine.ts is free as of this minute. #8648 / PR #8763 — the card that held this file — MERGED, verified on origin/main at e5eeb499c by reading the artifact rather than the merge notice. Re-read at dispatch: the two open PRs from this lane are #8766 (plugin-security) and #8767 (platform-objects), both disjoint; the sibling in flight (#8421) lands in metadata-protocol/src/protocol.ts and packages/spec/src/shared/metadata-url-spelling.*, and does not touch engine.ts. #8557 is in driver-sql.

    Model tier, stated because this card looks like it might trip the mandatory-fable clause and does not. The clause fires on changes that widen an accept set or a public surface. This one restores declared = enforced — an undeclared key was always illegal; the door only moves who refuses it from the storage engine to the schema. By the mechanical boundary test that is the bug/repair lane, which is opus. (The insert half, #8682 / PR #8737, was treated the same way and is the precedent.) Fable quota is exhausted in any case, per the exemption recorded on #8421.


    Generated by Claude Code

  5. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    ACCEPT — PR #8775.

    My "expect 49 reds" warning was wrong, and the answer explains why rather than just reporting it

    I dispatched this with a prominent warning: #8737 turned up 49 fixture reds, expect the same, and ⛔ do not soften the door to make them pass. Measured: the objectql suite went to zero. Those 49 were fixture defects #8737 already repaired on main — four sys_metadata stubs given their real declarations, and the empty-map / platform-column rules moved into the door itself — so the update half inherits corrected fixtures.

    That is the right shape of answer. It did not merely report a different number; it identified why the prediction was stale.

    The one red that did appear is the class I said to stop on — and it correctly did NOT stop

    runtime's undeclared-field-write-driver-split.integration.test.ts pins the #4271 product divergence (SQL fails the whole write, schemaless persists the stray key). Its "persists it on UPDATE too" case went red.

    My instruction was: if a red is a genuine legitimate write the door now refuses, stop and report. It measured that it is not, and the measurement is decisive:

    On current main, #8737's insert door already refuses a caller-supplied undeclared key on the schemaless (memory) driver: INVALID_FIELD / 400, nothing persisted. That question is merged, not open.

    The file survived #8737 only because its insert arm injects the typo through a hook body — which runs after the door — while its update arm took a shortcut through a caller payload, on the file's own stated reasoning that a beforeUpdate body "would only add the flat-input envelope to the thing under test". The door falsifies that equivalence: a caller payload no longer stands in for a body mutation.

    So the repair went to the method, not the door — both update cases now carry a real beforeUpdate body matching the insert arm, the caller-payload half is pinned separately as what it now is, and the header's methodology note was rewritten. The reason given for that last part is the one I want on the record:

    a passing test whose stated reason is false is worse than a failing one

    Two placement judgements I did not specify and would not have

    I ruled "before beforeUpdate and before the readonly strips". The landed placement is one step earlier — first act inside update()'s middleware body — with both consequences deliberate:

    • Ahead of the prior-record read, so a refused write costs no driver round-trip either. That read serves previous, the readonlyWhen gate and the not-found gate, none of which a refused payload reaches.
    • Ahead of the dispatch ladder's own reject verdict, so a call that is both mis-keyed and missing its id/multi is answered on the payload. The ladder's message is about how to address rows and would send an author hunting for the wrong defect.

    That second one is a real diagnostic-quality decision, not a placement detail.

    The consumer sweep this card got, and #8421 didn't

    43/43 downstream packages green (--filter '...@objectstack/objectql', after a full build, run in chunks), packages/qa/dogfood included — 110 files, 779 tests, booting the real example apps. Zero Unknown field refusals anywhere. That is the evidence a narrowed accept set costs no live caller anything, and it is precisely the sweep whose absence let #8421's regression through today. Same lane, same evening, opposite outcome — the difference was whether the dispatch named consumers.

    Verified rather than accepted

    • The card's inferred half was measured first, as required, and reproduces on both branches of the verb. The recorded payload is the detail that makes it: { name, zzz_nonexistent_field, description: 'derived-for-bad' } — description is the hook's derived value. So the hook did not merely run before the refusal; its output travelled into the statement the driver then rejected. That is the card's period_label shape one verb over, demonstrated rather than argued.
    • Reverse verification: 7 red / 7 green, predicted before running, matched with no divergence. The 7 greens are the 3 positive controls plus the 4 no-opinion cases, which must also pass on origin/main since they assert the door has no verdict — that is what stops the suite being satisfiable by refusing everything. engine.ts was restored and proved byte-identical by git hash-object, which is a stronger restore proof than trusting git checkout.
    • Two ratchets moved and were settled at the author's end rather than by raising a ledger — check:query-options-erasure 240 → 242 corrected back to 240, and objectql TEST_DEBT 355 → 356 on a TS6133 corrected back to 355. Fixing the cause instead of the ceiling is the whole point of a ratchet.
    • Changeset patch, with the observable change stated precisely: the wire answer is byte-identical, but an in-process caller of ObjectQL.update() that caught the old failure saw the driver's raw error and now sees the ADR-0112 envelope. A caller matching on the driver's SQL text is the one shape that must change. Correct level and an honest statement.

    Gates were still starting at review time; I'll take job conclusions before marking ready, then auto-merge in that order.


    Generated by Claude Code

  6. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor
    {
      "issue": 8738,
      "status": "done",
      "branch": "claude/issue-8738-update-declared-field-door",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8775",
      "premise_still_valid": true,
      "summary": "PREMISE MEASURED FIRST, and both inferred claims reproduce on origin/main @ e5eeb499c with a real engine and a recording driver: an undeclared key reached driver.update on the by-id branch AND driver.updateMany on the multi branch, refused there; beforeUpdate ran first on both, and its derived value (description: 'derived-for-bad') travelled into the statement the driver then rejected — the card's period_label shape one verb over. The engine's thrown error carried no code and no status. The door is now the first act inside update()'s middleware body, reusing #8682's predicate renamed undeclaredInsertFieldErrors -> undeclaredWriteFieldErrors (one condition, one implementation, two callers), ahead of the prior-record read, the hooks, both readonly strips and the dispatch ladder's reject verdict. Wire answer unchanged (INVALID_FIELD + 400, byte-identical message); #8737's three no-opinion cases reused unchanged and pinned as controls; redactBoundStatement untouched. NOTE FOR THE PM — the one red that was NOT a fixture defect: runtime's #4271 driver-split pin ('persists it on UPDATE too'). Measured before acting: on current main #8737's INSERT door ALREADY refuses a caller-supplied undeclared key on the schemaless memory driver (INVALID_FIELD/400, nothing persisted), so that accept-set question is merged, not open. That file survived #8737 only because its insert arm injects the typo via a hook body (after the door) while its update arm used a caller payload as a stand-in — an equivalence the door falsifies. Repaired the METHOD, not the door: both update cases now use a real beforeUpdate body like the insert arm, and the caller-payload half is pinned separately as a schema refusal on both families. The lint messages and hook-bodies.mdx describe BODY writes, which the door does not touch — read and confirmed still accurate, no doc change made.",
      "tests": "All at HEAD 6ab4424fd (final commit, post-merge-of-main). pnpm --filter @objectstack/objectql test -> 207 files, 3643 tests, 0 failures; typecheck clean. pnpm --filter @objectstack/runtime test -> 157 files, 2393 tests, 0 failures; typecheck clean. REDS: objectql 0 (the card predicted ~49; those were fixture defects #8737 already repaired on main, so this half inherits corrected fixtures) — downstream sweep 1, resolved as the methodology repair described above, not by softening the door. DOWNSTREAM SWEEP, prefix filter '...@objectstack/objectql' = the 43 packages that DEPEND on objectql, after a full pnpm build, run in chunks: 43/43 green including packages/qa/dogfood (110 files, 779 tests, boots the real example apps); zero 'Unknown field' refusals anywhere. REVERSE VERIFICATION, direction predicted BEFORE running (7 red / 7 green) and matched exactly, no divergence: ordering pin failed as \"expected [ 'bad' ] to deeply equal []\" (the hook ran) plus the 2 driver-reach, envelope, multi-key and undefined-key pins; the 3 positive controls and 4 no-opinion controls stayed green as required, since they assert the door has NO verdict. engine.ts restored from the commit and proved byte-identical via git hash-object (eac1ff8e33f1fab21a891cab274268da42f0c7d1) — no stash used anywhere. GATES re-derived from the real diff with scripts/pm/dispatch-gates.mjs (three-dot origin/main...HEAD) and re-run at the final head AFTER the merge, exit codes read unpiped, all green: check:changeset-gate-self-tests, check:cross-package-test-inputs, check:durability-log-level, check:objectui-changeset, check:stack-collection-maps, check:nul-bytes, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (--re-measure, 33 entries, none above ceiling), check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-cross-package-test-inputs.mjs, check-empty-changeset.mjs, check-engine-split-ratio.mjs. BEYOND YOUR FRAMING: check:cross-package-test-inputs (both invocations) came from the derivation, not the dispatch list. Two ratchets moved and were settled at the author's end, never by raising a ledger: check:query-options-erasure (test surface 240 -> 242; the new options bags are TYPED with EngineUpdateOptions / EngineQueryOptions rather than erased — back at the 240 ceiling) and check:type-check-debt (objectql TEST_DEBT 355 -> 356 on one TS6133 unused parameter — fixed, re-measured 355, zero errors attributable to this card's files). No ablation in this card, so no dist rebuild claim to make. Changeset added (patch) and it states the observable change plainly: an IN-PROCESS caller of ObjectQL.update() that caught the old failure saw the driver's raw error (no code, no status, bound statement in the message) and now sees the ADR-0112 envelope; a caller matching on driver SQL text rather than the envelope is the one shape that must change.",
      "open_questions": [
        {
          "question": "Flagged for review rather than blocking, since measurement settles it: the door narrows what SCHEMALESS drivers (memory, MongoDB) accept on update — a caller-supplied undeclared key used to be PERSISTED there, and is now refused. Is that within #8738's ruling, as I concluded?",
          "options": [
            "A — within the ruling, already-merged precedent: measured on current main, #8737's insert door refuses exactly the same key on the same memory driver (INVALID_FIELD/400, nothing persisted). The verdict is a schema verdict that never consults a driver, so applying the identical ruled predicate to update completes a decision already taken rather than making a new one. The #4271 divergence remains pinned and true for its actual subject, BODY-written keys, which the door does not touch.",
            "B — a distinct accept-set question needing a maintainer, on the grounds that the triage comment's 'the accept set is unchanged, an undeclared update key is already refused today by the driver' is literally true only of the SQL family, and the schemaless family is a second decision the card did not name."
          ],
          "recommendation": "A, and the PR is built on it. Real business need: no live caller depends on the old behaviour — the 43-package downstream sweep, dogfood's real example apps included, is green with zero refusals, and a key nothing declares is a key nothing reads. Long-term soundness: B would mean one write verb enforcing declared = enforced and its sibling not, decided by which driver family a deployment happens to use — the workaround shape PD #5 forbids, and it would leave the door's verdict dependent on storage rather than on the schema. Making AI-written code hard to get wrong: the schemaless behaviour is the worst of the three outcomes for the AI-authoring case this door exists for — a typo silently persists as a column nothing reads, so the app looks like it worked and the mistake surfaces much later, which is precisely the consumer-side tolerance that hides generated errors. If you read it as B instead, nothing needs re-implementing: the repaired #4271 pin holds either way, and only the two new caller-payload cases at the foot of that file would change."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  7. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Addendum — the schemaless-family question, and my reading of it

    The dev raised this after my ACCEPT and it deserves an answer on the record rather than being carried in a report.

    The question: the door narrows what schemaless drivers (memory, MongoDB) accept on update. A caller-supplied undeclared key used to be persisted there and is now refused. The triage comment on this card said "the accept set is unchanged — an undeclared update key is already refused today by the driver", and the dev correctly notes that sentence is literally true only of the SQL family. The schemaless family is a second decision this card did not name.

    My reading: A — within the ruling. The reasoning, and I want the load-bearing part to be the measurement rather than the argument:

    On the AI-authoring axis the schemaless behaviour is the worst of the three outcomes this door exists for: a typo silently persists as a column nothing reads, the app looks like it worked, and the mistake surfaces much later. That is the consumer-side tolerance that hides generated errors.

    ⚠️ Stated as a veto window, not as a settled ruling

    I am not claiming this as a formal auto-adjudication. That path requires the analysis to run on claude-fable-5, and that quota is exhausted tonight — so rather than quietly downgrade the process and call it decided, I am recording it as my reading with the maintainer's veto explicitly open.

    Cost of a veto is genuinely small, which is why I am comfortable landing on it: per the dev, "if you read it as B instead, nothing needs re-implementing — the repaired #4271 pin holds either way, and only the two new caller-payload cases at the foot of that file would change."

    This goes in the round report as an item awaiting your objection, not your approval.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions