Refuse an undeclared field a before-hook writes — the post-hook half of the declared-field door, one envelope on every driver - #13864
Conversation
📓 Docs Drift Check1 anchor(s) derived from 2 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
Coarse fallback — 20 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 55fddc0d04e7e3be2c1e46beff8f327c90892ea2 && git checkout 55fddc0d04e7e3be2c1e46beff8f327c90892ea2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 55519d50363147da03df0af068fb82fcc6d1d8c1 901c2fa309a4229b1f915e0a94553517304013e9 && git checkout -B drift-repro 55519d50363147da03df0af068fb82fcc6d1d8c1 && git merge --no-ff 901c2fa309a4229b1f915e0a94553517304013e9
node scripts/docs-audit/affected-docs.mjs --json 55519d50363147da03df0af068fb82fcc6d1d8c1 |
PM review — ACCEPT on substance. One cross-PR hazard flagged, and it joins a contract-review queue that now holds two PRs.
1. ⭐ The census did the one thing that makes a census worth having: it caught its own blindnessThe dispatch order demanded A2.2 be re-run from scratch, not inherited from the seat that died on the Fable rate limit. It was — and the first pass matched only dot and string-literal keys, missing ⭐ A census that reports finding its own blind spot is worth more than one that reports a clean number, because the second kind gives you no way to tell the two apart. This is the second time this session a seat has self-corrected a null result rather than shipping it, and it is the behaviour that makes these measurements load-bearing at all. 2. ⭐ The strongest possible outcome on the "platform-tolerated" set — and it is the outcome I could not have predictedThe order made this explicit: enumerate the "platform-tolerated" key set, that qualifier is the thing a naive post-hook check would break on. The answer:
⇒ The door did not need widening at all. A2.1 therefore holds in its strongest form: a second call site, no new logic, no new predicate. ⛔ An exemption list invented to make the door pass would have been the failure mode here, and the census removed the need for one rather than justifying one. Three things keep that honest, and all three are pinned rather than asserted:
3. ⭐ A2.4 held by a sharper mechanism than the order statedI claimed the security corollary; the seat improved it. 4. The ablation names its survivors, which is what makes it a measurement13 of 17 new tests fail under mutation; the 4 survivors are exactly the acceptance controls (two platform-stamp cases, the declared-key positive control, the pre-hook-door control) — which should pass without the new code. ⭐ An ablation that only reports "it went red" cannot distinguish a load-bearing pin from a test that fails for any reason. This one predicts its survivors and gets them right. Mutation proved on disk (2 injected markers, 0 surviving call arguments, blob moved off HEAD); restore byte-identical under a 5. ⛔ I checked the one thing that looked like a re-baseline. It is the opposite.
Likewise 6. Pre-hook door: untouched, and the pin proves it⛔ Zone 1.1 required that #8737's placement not move, because moving it re-opens #8682. Not moved — and the pin that it still refuses a caller key before any hook runs is asserted in this PR rather than assumed from the old suite. Correct.
|
CI red — diagnosed, fix dispatched. ⭐ It is a ratchet moving in the good direction.
This diff caused it by improving things. Rewriting ⛔ Recording explicitly that this is not the re-baselining this lane forbids. Lowering a ratchet ceiling to match a genuine improvement is what a shrink-only ledger is for, and the gate itself prescribes the command. Raising a ceiling, or relaxing an assertion, is what is banned. Neither applies.
⭐ A lane-level gap this exposed, worth recordingThe seat derived its gate family mechanically from the actual diff ( ⇒ The derived gate family does not cover every gate that job runs. That is a gap between two tools, not a seat error, and the next seat will hit it the same way. Flagging it here rather than filing a card until I have seen it twice. Status unchanged otherwise: still draft, still held on Generated by Claude Code |
Second red — and ⛔ its remedy is the opposite of the first. Recording the distinction.
|
| Gate | Direction | Correct action |
|---|---|---|
query-options-erasure |
DOWN 240 → 236 | Ceiling now sits above reality ⇒ lower it (--update). ✅ Sanctioned; the gate prescribes it. |
TEST_DEBT |
UP 251 → 252 | ⛔ Do NOT raise the entry. Fix the error. The gate calls raising it MAINTAINER-ONLY and says "do not take this path to get CI green." |
⛔ Flagged to the seat explicitly, because "ratchet complains ⇒ run --update" is a tempting pattern-match and here it would be exactly the re-baseline this lane forbids — and that the gate forbids too.
⭐ The seat predicted this gap itself
Its report stated, before CI ran:
packages/objectqlandpackages/runtimetypechecks exclude*.test.ts, so their green says nothing about the two new test files (--listFileshit count 0 for each); those are covered bycheck:type-check-debtin CI.
⇒ The +1 is almost certainly in the new engine-post-hook-undeclared-field.test.ts or the repaired fixtures. That declared blind spot is why this took one log read to diagnose instead of three. ⭐ It is also the second card this session to hit the same underlying defect class — packages/objectql's tsc program excluding its own tests is open as #13676, and packages/formula's identical shape was measured on #13594.
TypeScript Type Check (job 99542184737) is the aggregator and sits downstream of this lane; it should clear when the ledger does. ⛔ Not chased separately.
Both fixes are to land in one validated push, with both gates re-run first. Status otherwise unchanged: draft, held on needs:contract-review.
Generated by Claude Code
…use helper
TEST_DEBT for @objectstack/objectql went 251 -> 252 in CI. The +1 is one
TS6133 ('data' is declared but its value is never read) in the new
engine-post-hook-undeclared-field.test.ts: the refuse helper took the payload
it never reads, because the SQL arm quotes a fixed value in its statement
string on purpose — that string reproduces the shape driver-sql used to leak,
not the double's own payload.
Fixed at the source rather than by raising the entry: the ledger is
shrink-only and the gate names raising it maintainer-only. objectql's own
typecheck excludes **/*.test.ts, which is why the package's local green said
nothing about this file and the debt ledger is what caught it.
…0 -> 236 The ceiling is now above reality, and a ceiling left above reality silently licenses that many new erasures. Written by the gate's own prescribed `pnpm check:query-options-erasure --update`. The four sites left with the rewrite of undeclared-field-write-driver-split.integration.test.ts (its two divergence arms became one it.each over both families) and the three fixture repairs that went with it. The diff is one line — testSurface.sites 240 -> 236; the 67 non-test sites across 17 files are untouched, and nothing anywhere is raised.
Both CI fixes pushed and verified — ⭐ the two ratchets got opposite, correct treatmentsHead moved 1.
|
…st-hook-undeclared-field-door
…nch's engine.ts insertions The page anchors ~145 citations by file:line; the two pure insertions this branch makes in packages/objectql/src/engine.ts (+67 at old 9491, +28 at old 10469) shifted eight cited lines by +67/+95. Mechanical repair via `node scripts/check-system-context-census.mjs --fix` — line numbers only, same semantic sites (the engine.ts diff is insertion-only, so the old->new line map is exact). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
Contract review (Clause ②) — REWORKReviewed at head Carrier action
Three things to fix
One finding worth reading even though it changes no verdictThe A2.2 census has a second blind spot of the class it already self-caught once. It found and fixed the computed-key form ( What the review confirmed as soundThe narrowing is real and correctly located; the pre-hook door is provably unmoved (the engine.ts diff deletes zero lines) with a positive-control pin asserting refusal before any hook runs; the three fixture repairs were each verified spec-illegal beforehand in source rather than adapted to the new gate; the 8→12 assertion count holds with nothing skipped or deleted; and the tolerance set is exactly the three the PR claims, with Generated by Claude Code |
Fixes #13657
The declared-field door (#8682 insert, #8738 update) sits in front of the
before*hooks — deliberately: PR #8737 moved it there so no work, above all no autonumber, is consumed by a payload about to be refused. That left the payload the hooks themselves produce unjudged, and from there the three drivers did three different things.This adds the post-hook, pre-statement half of the same door. ⛔ The pre-hook door is not moved, merged, or weakened — moving it re-opens #8682, and the pin that it still refuses a caller key before any hook runs is asserted in this PR.
The measurement this card turns on — A2.2, re-run here
packages/,examples/andapps/, and it caught its own blindness once: the first pass matched only dot and string-literal keys, so it misseddata[SEARCH_COMPANION_FIELD] = …inplugin-pinyin-search. Computed-key support was added and the scan re-run. Positive controls fire (input.probability,data.priority,ctx.input.title— declared keys, all present in the output).sys_stamp_audit_insert/_update—objectql/src/plugin.tsobject: '*'— every objectcreated_at,updated_atobject: '*'created_by,updated_by,tenant_idhasField(objectName, …)bindSearchCompanionHooks—plugin-pinyin-search__searchschema.fields[__search]installCommentAccessHooks—plugin-auditsys_commentauthor_idsys-comment.object.ts)installAttachmentAccessHooks—service-storagesys_attachmentuploaded_bysys-attachment.object.ts)applyCopyOnClaim—service-storageif (!(field in data)) continue)examples/app-crmopportunity hookcrm_opportunityprobabilityexamples/app-todotask hooktodo_taskpriority,status,completed_dateexamples/app-showcaseL2 bodiesshowcase_task,showcase_inquirytitle;status,sourceExcluded, with reason:
plugin.ts_packageId(stamps a metadata definition read from a stored row, not a record payload) ·spec/conversions/registry.tsctx.input.fp/trace(conversion-migration fixture strings) ·seed-loader.tsorganization_idandimport-runner.tsdata.id(caller-side payloads, already governed by the pre-hook door) ·record-change-trigger.tsmaterializeDeclaredFields(iterates declared names only) ·objectql-adapter.tsnormaliseIdentifierWrite(rewrites present keys only).The "platform-tolerated" set, enumerated
Exactly
id,created_at,updated_at— and they are alreadyPLATFORM_PROVISIONED_COLUMNSinundeclaredWriteFieldErrors, tolerated since #8682 for the same reason the audit hook writes two of them unconditionally: SQL drivers create them as built-in columns on every table whether the object declares them or not. So the post-hook door needed no new tolerance and no exemption list — the set the census found is the set the function already had.Two facts keep that honest, both pinned:
registerObjectinjectscreated_at, created_by, organization_id, owner_id, owning_business_unit_id, updated_at, updated_by, so the audit family is declared. Measured, not assumed.systemFields: falseobject (the hard opt-out — seed / migration tables), where the registry injects nothing. That is the object the new tests use, so the tolerance is exercised rather than shadowed.tenant_idis the nearest miss and is refused: the platform writes it, but only behindhasField, and the registry injects it under no setting. Pinned, so that guard cannot quietly become optional.⇒ A2.2 HELD. STOP condition 1 does not fire. No shipped hook writes an undeclared, untolerated key.
Zone 2 verdicts
undeclaredWriteFieldErrorsreusable unchangeddriver-sql(better-sqlite3) anddriver-memory:memoryno longer persists the shadow column. Doors exercised are named below.FieldMasker.detectForbiddenWritesreports only fields explicitlyeditable: false; an absent entry is fail-open. An undeclared key can carry no entry, so field-level security could never gate it. Additionally the FLS write gate readsopCtx.datainside the middleware, whichexecuteWithMiddlewareruns strictly before the executor holding the hooks — so no hook-written key is presented to it at all.insert intoand no bound values. It is unreachable because no driver is reached — the leak lived in the driver's own error string. ⛔ Not widened: the pre-existingmapDataErrordriver-string branch is untouched.Verification — at final commit
b5fd717e17pnpm --filter @objectstack/objectql test— 251 files / 4339 tests passpnpm --filter @objectstack/runtime test— 202 files / 3015 tests passengine-post-hook-undeclared-field.test.ts— 17 tests, three driver flavoursundeclared-field-write-driver-split.integration.test.ts— 12 tests (was 8), real sandbox + real driversplugin-audit22 ·plugin-sharing30 ·plugin-pinyin-search2 ·service-storage33 ·trigger-record-change7 ·plugin-approvals34 ·example-crm45 tests ·example-todo106 tests ·example-showcase364 tests ·metadata-protocol2040 tests ·dogfoodhook suites 8 ·clihook suites 41 ·linttargeted 197node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the actual diff (not a hand-written list): 33 gates, all green.check:doc-authoringcaught a tracker id I had put in a runtime lint string — stripped to an adjacent comment, per the rule that a runtime string reaches readers who cannot resolve#NNNN.check:dual-build-cjs-loads— it exits 3PREREQUISITE NOT MET("⛔ This is NOT a pass: nothing was measured") because 26 packages have nodist/in this container. It needs a fullpnpm build; CI runs it. This diff changes no package exports or build config.Ablation
Both post-hook doors neutered (fed an empty row list), mutation proved on disk (2 injected markers, 0 surviving call arguments, blob hash moved off
HEAD): 13 of 17 new tests fail. The 4 survivors are exactly the acceptance controls — the two platform-stamp cases, the declared-key positive control, and the pre-hook-door control — which should pass without the new code. Restore verified byte-identical to theHEADblob withgit diff HEADempty, via atraponEXIT INT TERMwith absolute paths. No rebuild was needed and none is claimed: the new tests import./engine.jsrelatively from source, and the package's vitest config declares no alias, sodist/is not in that resolution path.What changed outside
packages/objectql, and whyThe behaviour change falsified prose and a pin that asserted the old split. Leaving them would ship a false statement and a red
main.packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts— this file existed to pin the divergence. Its two arms are now oneit.eachover both families asserting the convergence. Assertion count goes up (8 → 12); nothing is skipped, disabled or deleted. Itssys_organizationexpected-noiserequiredset is narrowed to none — the documented remedy for "a table read on only SOME of a file's paths", which is what the tenancy probe became once the refusal short-circuits ahead of it. The capture still withholds the noise where it fires and still fails when no capture is installed.packages/lint/src/validate-hook-body-writes.ts— thectx.inputmessage and the header described the split as current runtime behaviour. Corrected. The rule is not retired: it fires at author time and names the field, object and body, where the runtime refusal fires at write time on whichever record first reaches the branch.content/docs/automation/hook-bodies.mdx— same correction to "What still happens at runtime".Three fixtures repaired, each the same defect this card closes
Each had a hook writing a field its own object never declared — spec-illegal before this PR, merely invisible.
engine.test.ts(batch-hook) — mocked field map now declaresstamped, the field its own hook writes.engine-undeclared-update-field.test.ts—stubFieldsnow declaresdescription, which the harness's hook stamps.idis still the one name the registry does not inject, so the tolerance under test is unchanged.search-companion-read-projection-conformance.test.ts— itsbindCompanionStampdouble was looser than the hook it doubles: it lackedstampCompanion'sif (!schema?.fields?.[SEARCH_COMPANION_FIELD]) return;guard, so in thedeclared: falsearm it stamped a column the real hook returns early on. The guard is restored. (In that arm the real plugin binds no hooks at all — pinyin is off.) The arm's subject is untouched: the stored row still carries the blob from the fixture seed, and every read door must still project it away.Scope
out of scope: #13858— filed from this work, not addressed here. Three sibling lint messages (ctx.apiwrites in hook bodies, action bodies, flow nodes) still describe a driver-dependent outcome for caller-supplied payloads, which the #8682/#8738 pre-hook door already refuses. That staleness predates this card and wants its own reproduction.Sibling-seat surfaces: this diff touches
packages/objectql,packages/runtime(one test file),packages/lint(one rule's prose) andcontent/docs. It does not touchpackages/metadata-protocol/src/protocol.ts(#13576, #13445) orpackages/drivers/driver-memory(#13435) — the runtime test consumes driver-memory as its ruled test consumer, but the driver itself is unmodified. Inpackages/objectql/src/engine.tsthe edits are at the two write-path door sites (~9490 and ~10470); the driver-registry region (~5215, ~7850) that #13578 works in is untouched.Clause ② self-declaration — from the actual diff
YES, content limb. This narrows what a write accepts: a hook-written undeclared key is stored today on⚠️ The implementation seat ran at the default tier after the
memoryand is refused after. Path limb NO — nothing underpackages/spec/src/**.needs:contract-reviewattached at creation.CONTRACT_REVIEW_TIERexhaustion recorded on the card; the label requirement is unconditional and is met here.Generated by Claude Code