Repository navigation
fix(driver-sql): update() stamps updated_at into a fresh object, never the caller's data - #24906
Conversation
…r the caller's data update() and rotatedUpdateById() assigned the updated_at stamp onto formatted, which formatInput/applyWriteColumnMap hand back as the caller's own object when nothing needs coercion. ObjectQL publishes that object as DataEvent.changes; on PostgreSQL/MySQL the stamp is a knex Raw holding the client, pool and a timer, so the webhook outbox could not serialise any update event and every webhook update delivery was lost. Stamp into a fresh object, as updateMany already does. Pins: driver-sql update() leaves its data argument deep-equal on the plain, presumed and rotated paths; plugin-webhooks stores a plain-JSON delivery row for an insert and an update on a real SqlDriver. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…ivery pin The query-options-erasure ratchet counts an 'as any' on engine options in tests; both options bags are already their contract types. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…iver-update-no-mutate
📓 Docs Drift Check1 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 13 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 5c87bf6b352ed8d56b312185b5172c2501753bec && git checkout 5c87bf6b352ed8d56b312185b5172c2501753bec
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fcb40ad673a08b231d34190a60d692f462c5df23 7c88888b5060e0a06306ca32fc774d98e1654a89 && git checkout -B drift-repro fcb40ad673a08b231d34190a60d692f462c5df23 && git merge --no-ff 7c88888b5060e0a06306ca32fc774d98e1654a89
node scripts/docs-audit/affected-docs.mjs --json fcb40ad673a08b231d34190a60d692f462c5df23 |
席位核对 · ACCEPT ·
|
… into 7b-0 Conflicts in seven fixture files take #24893's resolution (4dadd7e): where 7a-2 now writes the grant with an organization the pre-18 relax goes; where main still lands an organization-less row the relax stays. sharing-rule-criteria-required takes main's version verbatim (both sides had added the organization key). plugin-webhooks' new real-SqlDriver case gets the lean engine's Default Organization. Claude-Session: https://claude.ai/code/session_018EszgtA3V2KXBX5tf59Hi4 Co-authored-by: Claude <noreply@anthropic.com>
Fixes #24835
Clause-②: no
Round 2 of the card, option A of the fork in report
6289852046: the producer fix. Round 1 was PR #24862, the log level of a lost delivery. It landed onmainas3fbef41b4a, and this PR does not touch its file. Cross-lane:driver-sqlisdomain:engine's code, declared on #6367 (comment6289887999).What was wrong
SqlDriver.update()assigned itsupdated_atstamp ontoformatted.formatInputandapplyWriteColumnMapare copy-on-write: a patch that needs no coercion comes back as the caller's own object. So the stamp landed in the caller'sdata. ObjectQL passeshookContext.input.datatodriver.updateand then publishes that same object asDataEvent.changes.On PostgreSQL and MySQL the stamp is
knex.fn.now(), a knexRawholding the client, its pool and a NodeTimeout. The webhook outbox'sJSON.stringify(payload)threw (Converting circular structure to JSON), so nosys_http_deliveryrow was written for any record update. The record write still answered 200, and the webhook still read active. On SQLite the stamp is an ISO string: it serialises, and it is just as foreign to the caller's patch.rotatedUpdateById()had the same in-place stamp.updateMany()already built a fresh object.What this PR changes
packages/drivers/driver-sql/src/sql-driver.ts, two sites, both now inupdateMany's shape. Line numbers are at base950f949a9c.update()(:8820):const stamped = { ...formatted, updated_at: this.updatedAtStamp() }, passed to both the presumed-timestamp path and the plain path.updateWithPresumedTimestamp's fallback already builds its own unstamped copy.rotatedUpdateById()(:10249):payloadis the stamped copy, orformattedwhen nothing is stamped.stampUpsertUpdatedAt(:8554) is unchanged, measured. Its only caller isupsert()(:9580), and thereformattedis derived fromtoUpsert = { ...rest }(:9482), a fresh object built fromconst { _id, ...rest } = data(:9481). The stamp can land only in the driver's own copy, never in the caller's object.grep stampUpsertUpdatedAt packages/drivers/*/srcfinds no other caller.applyTenantScope, any other driver method, objectql (option B, not taken),packages/spec, round 1's file.Pins
packages/drivers/driver-sql/src/sql-driver-24835-update-leaves-caller-data.test.ts, on SQLite.update()leaves itsdataargumenttoStrictEqualto astructuredClonetaken before the call. It covers four cases:Each case also proves the stamp really ran on that path: the stored
updated_atmoved off a backdated value, the presumption settled, or the column read asabsent.packages/plugins/plugin-webhooks/src/webhook-delivery-real-sql-driver.test.tsruns a realObjectQLand a realSqlDriver(SQLite) with an active declared webhook, through the production wiring:AutoEnqueuer, thenMessagingService.enqueueHttp, thenSqlHttpOutbox. An insert and a by-id update store 2sys_http_deliveryrows (ref_iddeclared:probe_task_changed). Each row'spayload_jsonparses to plain JSON and re-serialises byte-identical, and nothing was logged as lost.plugin-webhooksgains@objectstack/driver-sqlas a devDependency (pnpm-lock.yaml: the importer entry only, 3 lines;pnpm install --frozen-lockfilepasses).vitest.config.tsaliases@objectstack/driver-sqlto source, as it already does forcoreandtypes. The verdict is about the checkout, notdist/.Temporal Conformance (live PG + MySQL)) does not run on pull requests.Measured on a throwaway local PostgreSQL 16.15 cluster
I ran pin 2's own body against PG through a generated, uncommitted copy that changes only the driver config:
sys_http_deliveryrowsdata.record.createdonly)5755b9d424data.record.created,data.record.updated)payload_jsonchanges={"title": "webhook probe"}, exactly the caller's patchRound 1 measured the same 1 row at base
41646a5788, with the card's WARN. The cluster was stopped and deleted afterwards.Ablation (one leg per changed site)
Both legs used
node scripts/ablation-replace.mjsin wrap mode. Each restore wasgit checkout HEAD, proven by the blobecc2686b44a1equal to the HEAD blob and an emptygit diff HEAD, both re-checked by hand.update().const stamped = { ...formatted, … }was replaced withformatted.updated_at = this.updatedAtStamp(); const stamped = formatted;. Anchor 1 to 0; blobecc2686b44a1tocfad3f1e1a3b.Tests 3 failed | 1 passed (4). The plain and both presumed cases are red. The rotated case stays green, because it goes through the other site.1 passed. That is expected: an ISO string serialises, which is why SQLite never showed the loss.1 failed,expected [ 'data.record.created' ] to deeply equal [ 'data.record.created', …(1) ].rotatedUpdateById(). The stamped copy was replaced withObject.assign(formatted, { updated_at: … }). Anchor 1 to 0; blobecc2686b44a1tod6664cc75a61.Tests 1 failed | 3 passed (4). Only the rotated case is red.Both subjects are imported from source (pin 1 relative, pin 2 through the alias), so no
dist/leg was needed.Tests
All runs below are on the final head
7c88888b50and underos-verify-lock, unless a line names another head.pnpm exec turbo run build --filter='@objectstack/plugin-webhooks^...' --filter='@objectstack/driver-sql...':BUILD_EXIT=0,Tasks: 16 successful, 16 total.pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2:Test Files 225 passed | 11 skipped (236),Tests 3691 passed | 208 skipped (3899). The skips are this package's live-PG/MySQL cells, which are not provisioned locally.pnpm --filter @objectstack/driver-sql typecheck: exit 0. The tsconfigincludeissrc/**/*, so the new pin is compiled.pnpm --filter @objectstack/plugin-webhooks exec vitest run --maxWorkers=2:Test Files 22 passed (22),Tests 227 passed (227).pnpm --filter @objectstack/plugin-webhooks typecheck: exit 0 (check:test-typecheck: OK).5755b9d424: pin 1Tests 4 passed (4), pin 2Tests 1 passed (1). The ablation legs and the PG runs above are on5755b9d424too. The only later change to these files is05079c544e, which removed twoas anyoption casts from pin 2 (the query-options-erasure ratchet counts them).Gates
The local subset was derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat the final head7c88888b50: 87 families over the 7 changed paths. One runner ran them, each under its ownos-verify-lockacquisition, with every exit captured before any pipe.check:undeclared-dep-imports(the new devDependency),check:test-source-alias(the new alias),check:cross-package-test-inputs,check:engine-double-contract,check:query-options-erasure,check:driver-conformance,check:durability-log-level,check:turbo-task-graph,check:workspace-manifest-cycles,check:override-consistency,check:published-files,check:type-check-coverage,check:nul-bytes,check:doc-authoring,check-adr-0087-registration --base origin/main,check-changeset-fixed,check-changeset-no-majorandcheck-empty-changeset.check:dual-build-cjs-loadsneeds every package'sdist/.check:i18nneeds the CLI build closure. CI'sLint & Repo Gatesruns both.check:type-check-debt(--re-measure) rebuilds the whole monorepo under the shared lock from inside its own gate. CI runs it.--ranreconciliation:87 derived, 84 run, 2 NOT-MEASURED, 1 UNRUN(the UNRUN is the declaredcheck:type-check-debt).An earlier run of the same battery, at
5755b9d424, found one real red:check:query-options-erasure,test surface grew 236 → 237, from anas anyon engine options in pin 2.05079c544eremoved the casts, and the gate is green on the final head. CI check-runs were not concluded when this body was written; the report on #24835 cites them.Acceptance notes
origin/main7035682c8d(merge commit7c88888b50). It brought round 1's landing (3fbef41b4a, the error-level report of a lost delivery) and a version-packages commit, both intoplugin-webhooks. The merge was clean, and the branch's delta against main is still exactly the 7 files above.changes, and ofhookContext.input.dataafter the driver call, now sees the caller's patch rather than a dialect value object. Examples:afterUpdatehooks and realtime subscribers. Only the webhook reader is pinned here.Generated by Claude Code