Repository navigation
fix(objectql)!: a write's returned row and its prior read serve the declared fields, never an orphaned column - #21631
Conversation
…ed columns Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…ecords of a write's prior read Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…eclared fields, never an orphaned column Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…lidation rules read Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
The probes recorded which write doors, data events and hook contexts carried a column no metadata declares, before and after the engine shaping; their readings are in the PR body. The net diff carries none of them. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
… the audit ledger; changeset Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…nqueueFn Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…-double-limit) Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check2 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
Coarse fallback — 17 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 71e15b61d1b8a6207bbb93cad7de57b3ed158b8f && git checkout 71e15b61d1b8a6207bbb93cad7de57b3ed158b8f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 045b946256d988653fdca185c7fd33d6d86bd78d bfbeffed43bf59c7ce97b6049961e6068216aa3a && git checkout -B drift-repro 045b946256d988653fdca185c7fd33d6d86bd78d && git merge --no-ff bfbeffed43bf59c7ce97b6049961e6068216aa3a
node scripts/docs-audit/affected-docs.mjs --json 045b946256d988653fdca185c7fd33d6d86bd78d |
ACCEPT — PR #21631 at head
|
Fixes #21613
Clause-②: no (narrowing)
A write now answers with the object's declared fields plus the platform's system columns. A column no metadata declares, such as a field retired in an upgrade whose column additive sync leaves behind, no longer leaves the engine through a write. The same holds for the prior read a write binds as a hook's
previous. This completes #21571's read rule for writes. It is decided in the same place, the engine (packages/objectql), with the same helper (declared-read-columns.ts), on the rows as the driver returned them: before formulas, hooks, events and the door's own ingress strip. No driver source and nometadata-protocolsource is edited. There is no per-door strip, no allow-list and no flag.Measured first (before any fix)
The probes were two temporary tests on this branch:
9c73f3702b(rest) andb923112e06(plugin-audit), reverted bydfbb80ec2e. The net diff carries none of them. Harness: the composed REST harness of #21571's reach pin (RestServer, thenObjectStackProtocolImplementation, thenObjectQL, then a realSqlDriveron better-sqlite3), two boots, at5c9138b4b6.PATCH /data/:object/:idrecord.mailing_street = "1 Retired Way"POST /data/:objectnullPOST /data/:object/:id/clonenullPOST /data/:object/createManynullPOST /data/:object/batchupdate / upsert (update arm)POST /data/:object/batchcreate / upsert (create arm)nullPOST /data/:object/updateManyDELETE /data/:object/:idengine.updateby id,engine.insert,engine.insertManyoutcomesnulldata.record.created/data.record.updatedevents,afterpreviousorresultBulk
data.records.*events carry a count and no row, andchangescarries the input patch only.The audit ledger serves the prior read. Measured with a plugin-audit probe (real
ObjectQL, a SQL-shaped store,installAuditWriters):old_valuerecordedmailing_street: "c2 Retired Way", and a create'snew_valuerecorded both columns asnull;mailing_street: "c1 Retired Way"inold_valueagainstnullinnew_value. That is a phantom change, and it puts the stored value into a served row on every write.sys_audit_logis read back through the data door, so the prior read is a served row. That puts it in this card's class (H3), and shaping the result alone would have made the ledger worse. So the prior reads are shaped too.Hypothesis verdicts
H1 (confirmed):
SqlDriver.updatereturns itsselect *readback, and create andbulkCreatereturnreturning('*').ObjectQLreturned both as they came.H2 (confirmed): the engine publishes
data.record.*from the sameresultvariable the hooks get, after the driver. The webhook enqueuer copies the event'spayloadverbatim (payload: { ...payload, ... }). With the shaping placed after the driver, the event and the webhook payload are clean, with no edit inplugin-webhooksorservice-realtime. The pins below drive the realAutoEnqueuer.H3 (served, in class, shaped): see the audit ledger above. The four prior reads are shaped at the read:
driver.findOne);resultfrom);The delete event reads only the tenant column off its pre-image, which is declared.
H4 (enumerated): the write verbs that return a row are:
insert(one row and a batch);insertMany(itsokoutcomes carryrowHookContexts[i].result);updateby id.Predicate
updateanddeletereturn a count, anddeleteby id returns the driver's boolean. The engine has no upsert verb: the batch door's upsert isupdateorinsert.ObjectRepositoryand the scoped context delegate to these verbs. Every REST and protocol face above reaches one of the three, so shaping them covers every door.H5 (pinned): an
internal: truefield stays whole on the engine-level write result (the A-prime ruling) and is still stripped by the data door. The rest pin and the conformance matrix both assert it.In-process readers of undeclared columns
The suites ran with the shaping in place: objectql at
10d7f333d9, the others atf082789802(the probes were still on the branch):The 4 objectql failures were fixtures whose
readonlyWhenor validation rule reads a field the fixture never declared (record.locked,record.limit), off the prior read. #21571 saw the same shape in trim mode. The fixtures now declare those fields (engine.test.ts× 3,plugin.integration.test.ts× 1). No assertion changed. At authoring, the formula validator'sunknown fieldcheck (packages/formula/src/validate.ts) flags a production rule that names no field.The operator reads that legitimately need a retired column's values go through the driver, so they are unaffected:
os migrate plan'sunmapped_columndetection;os migrate account-issuer.Changes
packages/objectql/src/engine.ts, one shaping per site, each withdeclaredColumnSet(registry object):insert: the driver's result rows, after the one-row-per-input guard;updateby id: the driver readback;packages/rest/src/data-write-result-declared-fields.test.ts: the reach pin, on the composed harness.packages/objectql/src/write-result-declared-fields-conformance.test.ts: the engine matrix.packages/plugins/plugin-webhooks/src/webhook-payload-declared-fields.test.ts: the webhook payload through the realAutoEnqueuer.packages/plugins/plugin-audit/src/audit-ledger-declared-fields.test.ts: the served ledger..changeset/21613-write-result-declared-fields.md:@objectstack/objectqlminor, BREAKING (narrowing), ADR-0087not-required (no-migration-prescription), and the interim route (convert before upgrading, or migrate: a sanctioned, operator-only read of unmapped (orphaned) columns for data conversion, before--allow-destructivedrops them (the coupling #21571 names) #21573's operator read once it lands).File surface beyond the claim, test-only: the webhook and audit pins live in their own packages. The composed REST package has no dependency on either plugin. Adding one would need a devDependency plus a source alias, because
KNOWN_UNALIASED_TEST_IMPORTSis shrink-only. So the event both of them consume is pinned on the composed harness, and each consumer is pinned where it lives.Pins
data.record.created/updatedevent'safter;previous;internal: stripped at the door, whole onengine.update.insertMany, and update by id;previousandresult, for by id, per-row predicate update, and delete by id and by predicate;createData,updateDataandcloneData;internalwhole on the engine result and stripped at the door, formula hydrated;ObjectQLwrite, then the realtime publish, then the realAutoEnqueuer. The enqueuedpayload.aftercarries no retired key, for created and for updated.{ name }, and the deleteold_valueand createnew_valuecarry no retired key.git grepover*.test.tsfor orphaned, retired, unmapped or undeclared column readings, plus the suite runs above. No test asserted that a write's returned row or prior read carries an undeclared column, so no pin flipped. Each new pin asserts the declared values as well as the absence: ids, names and system columns.Reverse verification (fix committed first)
Each shaping was removed with
node scripts/ablation-replace.mjs: the anchor went from 1 hit to 0 and the blob changed. objectql was then rebuilt (exit 0), andablation-dist-preflight --absentconfirmed the call is absent from all 14 built files (exit 0 each). The pristine build carries every marker. Final run atbfbeffed43:insertMany, events, doorspreviouspreviousRestore:
ablation-replacerestored withgit checkout HEAD -- PATHeach time. The blob6008f2bcequals the HEAD blob,git diff HEADis empty andgit status --porcelainis clean. After a rebuild, the preflight finds all five markers present, and the pins are 12/12, 8/8, 1/1 and 3/3. Each mutation was replaced with a type-valid spelling so the DTS step builds. A first C1 run with an arity-breaking spelling failed the DTS step and was redone; its red set was the same.Local verification
test368 files, 7427 passed, andtest:repo5 passed, atbfb7b28a49(after mergingorigin/maine367002e11).typecheck(tsc, scripts and test-typecheck) exits 0 atbfbeffed43.test260 files, 4897 passed, 326 skipped;test:repo5 files, 177 passed;typecheckexits 0;bfb7b28a49. The rest test file is unchanged since.bfb7b28a49: 39 files / 621 tests and 14 / 161;typecheckexits 0 atbfbeffed43;node scripts/pm/dispatch-gates.mjs --commands(no paths) atbfbeffed43derived 71 commands. All 71 were run on that head and each exited 0.--ranreconciles with "71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN", with every exit code recorded.check-adr-0087-registration×2,check-empty-changeset×2 andcheck-tenant-audit-census×2;release-rehearsal-cloneandrelease-pending-publishself-tests;check:engine-double-contract,check:i18n,check:i18n-stale-fillandcheck:objectql-double-limit;check:objectui-changeset,check:pm-changeset-deadline-censusandcheck:query-options-erasure;check:type-check-coverage,check:type-check-debtandcheck:where-matcher.check:objectql-double-limitfirst flagged the three new store doubles as limit-blind. They now apply the caller's bound (bfbeffed43).check:dual-build-cjs-loadsandcheck:i18nfirst answered PREREQUISITE NOT MET (exit 3), which is not a measurement. Both were rerun after the build they name, and both exit 0.eslint --no-inline-config --format json. eslint reports the changeset as "File ignored because no matching configuration was supplied".eslint.config.mjsenables no type-aware linting (noparserOptions.project, no typed rules), so this diff cannot move an untouched file's verdict.@objectstack/objectql. The suites above are the ones that read write results (rest, metadata-protocol, runtime, plugin-auth, plugin-sharing, plugin-audit, plugin-webhooks, service-automation).Acceptance notes
declared-read-columns.ts's header still describes the read side only. The write-side seats carry their own comments inengine.ts. It was not edited, because the claim names it only for a new entry point and none was needed. Carrier: none.content/docs/data-modeling/queries.mdxstates the default projection for reads. A write now follows the same rule, but no sentence says so.check-affected-docsis green. Carrier: none.engine.aggregate's undeclaredgroupByname in process is unchanged (already noted on An unprojected REST data query returns ORPHANED columns that no metadata declares (fields retired in an upgrade), outside any field-level rule, untilos migrate apply --allow-destructive#21571). Carrier: none.Generated by Claude Code