Skip to content

fix(driver-sql): update() stamps updated_at into a fresh object, never the caller's data - #24906

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24835-driver-update-no-mutate
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24835-driver-update-no-mutate

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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 on main as 3fbef41b4a, and this PR does not touch its file. Cross-lane: driver-sql is domain:engine's code, declared on #6367 (comment 6289887999).

What was wrong

SqlDriver.update() assigned its updated_at stamp onto formatted. formatInput and applyWriteColumnMap are copy-on-write: a patch that needs no coercion comes back as the caller's own object. So the stamp landed in the caller's data. ObjectQL passes hookContext.input.data to driver.update and then publishes that same object as DataEvent.changes.

On PostgreSQL and MySQL the stamp is knex.fn.now(), a knex Raw holding the client, its pool and a Node Timeout. The webhook outbox's JSON.stringify(payload) threw (Converting circular structure to JSON), so no sys_http_delivery row 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 in updateMany's shape. Line numbers are at base 950f949a9c.

  • 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): payload is the stamped copy, or formatted when nothing is stamped.
  • stampUpsertUpdatedAt (:8554) is unchanged, measured. Its only caller is upsert() (:9580), and there formatted is derived from toUpsert = { ...rest } (:9482), a fresh object built from const { _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/*/src finds no other caller.
  • Not touched: applyTenantScope, any other driver method, objectql (option B, not taken), packages/spec, round 1's file.

Pins

  1. packages/drivers/driver-sql/src/sql-driver-24835-update-leaves-caller-data.test.ts, on SQLite. update() leaves its data argument toStrictEqual to a structuredClone taken before the call. It covers four cases:

    • the plain path (a table the driver's DDL created);
    • the presumed-timestamp path with the column present;
    • the presumed-timestamp path with the column absent (the fallback that drops the stamp);
    • the rotated path.

    Each case also proves the stamp really ran on that path: the stored updated_at moved off a backdated value, the presumption settled, or the column read as absent.

  2. packages/plugins/plugin-webhooks/src/webhook-delivery-real-sql-driver.test.ts runs a real ObjectQL and a real SqlDriver (SQLite) with an active declared webhook, through the production wiring: AutoEnqueuer, then MessagingService.enqueueHttp, then SqlHttpOutbox. An insert and a by-id update store 2 sys_http_delivery rows (ref_id declared:probe_task_changed). Each row's payload_json parses to plain JSON and re-serialises byte-identical, and nothing was logged as lost.

    • To support it, plugin-webhooks gains @objectstack/driver-sql as a devDependency (pnpm-lock.yaml: the importer entry only, 3 lines; pnpm install --frozen-lockfile passes).
    • Its vitest.config.ts aliases @objectstack/driver-sql to source, as it already does for core and types. The verdict is about the checkout, not dist/.
    • No PostgreSQL in a committed test: the live-PG/MySQL driver lane (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:

tree sys_http_delivery rows update row
the in-place stamp put back (ablation leg A below) 1 (data.record.created only) missing; the pin fails on the label set
this branch 5755b9d424 2 (data.record.created, data.record.updated) payload_json changes = {"title": "webhook probe"}, exactly the caller's patch

Round 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.mjs in wrap mode. Each restore was git checkout HEAD, proven by the blob ecc2686b44a1 equal to the HEAD blob and an empty git diff HEAD, both re-checked by hand.

  • Leg A, update(). const stamped = { ...formatted, … } was replaced with formatted.updated_at = this.updatedAtStamp(); const stamped = formatted;. Anchor 1 to 0; blob ecc2686b44a1 to cfad3f1e1a3b.
    • Pin 1: 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.
    • Pin 2 on SQLite: 1 passed. That is expected: an ISO string serialises, which is why SQLite never showed the loss.
    • Pin 2 on PG: 1 failed, expected [ 'data.record.created' ] to deeply equal [ 'data.record.created', …(1) ].
  • Leg B, rotatedUpdateById(). The stamped copy was replaced with Object.assign(formatted, { updated_at: … }). Anchor 1 to 0; blob ecc2686b44a1 to d6664cc75a61.
    • Pin 1: 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 7c88888b50 and under os-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 tsconfig include is src/**/*, 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).
  • The two pins alone, on 5755b9d424: pin 1 Tests 4 passed (4), pin 2 Tests 1 passed (1). The ablation legs and the PG runs above are on 5755b9d424 too. The only later change to these files is 05079c544e, which removed two as any option 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/objectstack at the final head 7c88888b50: 87 families over the 7 changed paths. One runner ran them, each under its own os-verify-lock acquisition, with every exit captured before any pipe.

  • 84 families exited 0. Among them: 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-major and check-empty-changeset.
  • 2 families are NOT MEASURED: exit 3, PREREQUISITE NOT MET. check:dual-build-cjs-loads needs every package's dist/. check:i18n needs the CLI build closure. CI's Lint & Repo Gates runs both.
  • 1 family is a declared narrowing, not run locally. check:type-check-debt (--re-measure) rebuilds the whole monorepo under the shared lock from inside its own gate. CI runs it.

--ran reconciliation: 87 derived, 84 run, 2 NOT-MEASURED, 1 UNRUN (the UNRUN is the declared check: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 an as any on engine options in pin 2. 05079c544e removed 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

  • Before opening this PR I merged origin/main 7035682c8d (merge commit 7c88888b50). It brought round 1's landing (3fbef41b4a, the error-level report of a lost delivery) and a version-packages commit, both into plugin-webhooks. The merge was clean, and the branch's delta against main is still exactly the 7 files above.
  • Every other reader of the update event's changes, and of hookContext.input.data after the driver call, now sees the caller's patch rather than a dialect value object. Examples: afterUpdate hooks and realtime subscribers. Only the webhook reader is pinned here.

Generated by Claude Code

…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>
@github-actions github-actions Bot added size/m dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-webhooks/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-webhooks/vitest.config.ts) — pages documenting those are invisible to this run
  • 1 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 — 13 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 fcb40ad673a08b231d34190a60d692f462c5df23 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5c87bf6b352ed8d56b312185b5172c2501753bec — the merge of head 7c88888b5060e0a06306ca32fc774d98e1654a89 into base fcb40ad673a08b231d34190a60d692f462c5df23, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-20T14:15Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main a7199ef974, not against the report (os-dev-report 6292317661 on #24835). Round 2 of #24835, cross-lane into domain:engine's driver-sql, declared on #6367 (6289887999); no objection there at ACCEPT. Head under review:

7c88888b5060e0a06306ca32fc774d98e1654a89
  1. Shape.
  2. The producer fix, read in the code.
    • update() and rotatedUpdateById() now stamp updated_at into a fresh object ({ ...formatted, updated_at: this.updatedAtStamp() }), the shape updateMany already uses. Before, they assigned it onto formatted. Because formatInput and applyWriteColumnMap are copy-on-write, formatted IS the caller's data for a patch that needs no coercion.
    • On PostgreSQL and MySQL that stamp is a knex Raw holding the client, its pool and a timer. ObjectQL publishes the same object as DataEvent.changes, so every serialising reader threw, the webhook outbox first.
    • stampUpsertUpdatedAt is measured unchanged. Its only caller, upsert(), hands it the driver's own copy (toUpsert = { ...rest }).
    • Untouched: applyTenantScope, PR feat(driver-sql,objectql)!: a tenant-scoped read no longer answers a row with no organization (18.x step 7, part 7a-1) #24825's regions, and every other method.
  3. Pins and ablations.
  4. Changeset, @objectstack/driver-sql patch, read sentence by sentence: what was lost (every webhook update delivery on PostgreSQL and MySQL) and what is unchanged. Each matches 2 and 3.
  5. Gates.
    • At ACCEPT, every check-run on this head is success or an expected skip, among them TypeScript Type Check, Lint & Repo Gates, Test Core 1–6/6, Build Core, Governed Surface Queue Guard and Validate Package Dependencies.
    • Locally, 84 of 87 derived families exited 0, among them check:driver-conformance, check:workspace-manifest-cycles, check:test-source-alias and check:cross-package-test-inputs.
    • 2 were NOT MEASURED: check:dual-build-cjs-loads and check:i18n, exit 3, prerequisite.
    • 1 was a declared narrowing: check:type-check-debt, whose --re-measure rebuilds the monorepo.
    • driver-sql: 3691 passed. plugin-webhooks: 227 passed.
  6. Accepted deviations.
    • Pin 2's support files: plugin-webhooks' devDependency on @objectstack/driver-sql, its anchored vitest source alias (the remedy check:test-source-alias prescribes), and the 3-line pnpm-lock.yaml importer entry.
      • A plain pnpm install also rewrote two unrelated vitest esbuild peer strings. The dev restored the lockfile and inserted only the importer entry.
      • pnpm install --frozen-lockfile passes, and CI's Validate Package Dependencies judges it.
      • The edge adds no cycle (check:workspace-manifest-cycles green).
    • The PG "before" count was read with the in-place stamp put back on this branch. Round 1's base reading was the same 1 row.
    • One battery was superseded by two whole batteries on committed heads.
    • One merge of main.
    • The scratch PG cluster ran beside the worktree as postgres and was deleted.
    • Model-free trailers.
  7. Out-of-scope dispositions. The other formatInput / applyWriteColumnMap callers that stamp into formatted (create and the bulk paths) were not measured for caller-object mutation. There is no reproduction. Disposition: Acceptance notes; the engine lane is told on [PM seat] domain:engine — 🟢 os-project-manager #6367 at landing.
  8. Landing clause. Tier S: not governed, under the human-merge threshold. Next step: ready + auto-merge through the relay once every check on the current head is green or an expected skip, and [PM seat] domain:engine — 🟢 os-project-manager #6367 still holds no objection. A push resets this reading.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 14:17
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 14:17
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit eee65af Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24835-driver-update-no-mutate branch October 20, 2026 14:49
os-steve pushed a commit that referenced this pull request Oct 20, 2026
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants