Skip to content

fix(driver-sql,objectql): JSON-field values must not depend on DDL having run (#10995) - #11070

Merged
os-elon merged 2 commits into
mainfrom
claude/issue-10995-pg-json-binding
Aug 22, 2026
Merged

fix(driver-sql,objectql): JSON-field values must not depend on DDL having run (#10995)#11070
os-elon merged 2 commits into
mainfrom
claude/issue-10995-pg-json-binding

Conversation

@os-elon

@os-elonos-elon commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10995

The card's matrix is real. Its inferred mechanism is not — and that changes where the fix goes.

The card said, explicitly, "Mechanism (inferred from the signature — verify at the driver)".
Verified at the driver, on a live Postgres 16, and the guess does not hold: formatInput
doesJSON.stringify a JSON field's value on every non-SQLite dialect, and has since the
driver moved into packages/drivers:

// packages/drivers/driver-sql/src/sql-driver.ts — formatInput}else{// PostgreSQL: every value must be valid JSON so the native// jsonb column accepts it. JSON.stringify wraps strings in// quotes, leaves numbers/booleans unchanged as literals.copy[field]=JSON.stringify(copy[field]);}

What gates it is the line above: const jsonFields = this.jsonFields[object]. That registry —
along with the boolean / numeric / date / datetime / time / auto_number registries and the
tenant-isolation column — is filled only as the first step of a DDL call (initObjects,
which syncSchema() delegates to), or by registerExternalObject for federated objects.

So the defect is not a missing JSON.stringify. It is that the correctness of every value
this driver binds depended on DDL having run in the same process.
Two in-repo postures reach
the write path with those registries empty:

  1. skipSchemaSync / OS_SKIP_SCHEMA_SYNC=1 — published in
    content/docs/deployment/environment-variables.mdx as "skip the implicit db:sync on
    boot. Use after running migrations manually"
    , and named in plugin.ts for exactly the
    runtime the card was found on: "Cold-start-sensitive runtimes (Cloudflare Containers,
    Lambda) can opt out … an out-of-band migration must have already created every table"
    .
    That branch called nothing, so the boot told its drivers nothing.
  2. A datasource ObjectStack is only a guest in (schemaMode !== 'managed', ADR-0015):
    initObjects threw at the DDL gate before registering anything, so managed objects on a
    federated datasource were never described to the driver either.

With the registry empty, the value goes to node-postgres and is encoded by its per-type
defaults — which reproduces the card's matrix exactly, on INSERT and on UPDATE:

value on a json fieldmeasured, empty registry, live PG 16
{a:1} objectJSON text — accidentally correct
42 number42 — already valid JSON
{items:[1,2]}correct (the outer object is stringified whole)
[{type:'app',…}]Postgres ARRAY LITERAL → 22P02 invalid input syntax for type json → 500
'x' bare stringraw x, not JSON text ("x" would be) → 500
[] empty arrayarray literal {}valid JSON, so accepted and silently stored as an empty OBJECT

Why every existing suite is blind to it

formatInput ends with a bind-safety net that stringifies any leftover object/array — gated
on isSqlite, because better-sqlite3 cannot bind them at all. On SQLite an empty registry is
therefore invisible: the right bytes still land on disk (they just read back as text, since
the read-side parse is keyed by the same registry). Tenant environments run Turso/SQLite, so
the seed and data suites exercise a different dialect branch of the same function.
Postgres has no such net, and both control planes are Postgres. That blindness is part of the
defect surface, and the fix comes with coverage on the Postgres path specifically.

The fix: registration is separable from DDL

This is the ruling #7737/#10629 already made for federated objects — "that flag is about DDL,
and this binding is DDL-free"
— extended to the managed ones.

  • packages/drivers/driver-sql — the in-memory half of initObjects becomes
    registerManagedObjectMetadata(), exposed as registerObjectMetadata(objects): no
    CREATE TABLE, no ALTER TABLE, no existence probe, no round-trip. That property is what
    keeps it affordable on the cold-start budget skipSchemaSync exists to protect. initObjects
    now calls it before the ADR-0015 gate, so a guest datasource is described even though its
    DDL is (still) refused — the refusal itself is byte-for-byte unchanged.
  • packages/specIDataDriver.registerObjectMetadata?(), declared optional beside its
    federated sibling registerExternalObject?(). Drivers whose encoding does not depend on
    declared field types (memory, mongodb) omit it and are skipped.
  • packages/objectqlstart()'s two schema seams and the metadata:reloaded hook now go
    through one installRegisteredSchemas(): sync when the flag is off, registerSchemasWithoutDdl()
    when it is on. The skip line now reports what it registered, so a boot can be audited; a driver
    without the route is counted, not thrown at; a registration that throws is reported at error
    per the degradation-log-level rule.

What was deliberately NOT done: extending the SQLite bind-safety net to Postgres. It cannot
fix the bare-string row ('x' on a jsonb column and 'x' on a text column are
indistinguishable without knowing the column type), it would break a genuine native text[]
column on a table the driver was never told about, and it is a tolerant fallback at the consumer
for a defect whose producer is knowable — the shape AGENTS.md's contract-first rule refuses.

Evidence — three named defect controls, red before, green after

Live Postgres 16 (OS_TEST_POSTGRES_URL, TimeZone=Asia/Shanghai, process TZ=America/New_York),
through the real SqlDriver.create() / update() paths. Not SQLite: the SQLite cell appears
only as the expected non-effect in §4.

packages/drivers/driver-sql/src/sql-driver-json-binding-without-ddl.test.ts

pinpre-fix (same assertions, same posture, sql-driver.ts at origin/main)post-fix
§1a non-empty array round-trips (insert + update)error: insert into "os10995_red_pref" … - invalid input syntax for type jsonpass
§1b bare string / true / false round-trip (insert + update)same 22P02pass
§1c [] round-trips as [], not{}AssertionError: expected false to be true on Array.isArray(inserted) — it came back an objectpass
§3 guest datasource (DDL refused) still encodes correctlyerror: insert into "os10995_red_guest" … - invalid input syntax for type jsonpass
§2 objects / nested arrays / numbers / a text columnpassed pre-fix — the expected non-effectpass
 Tests 4 failed | 1 passed (5) ← pre-fix tree
Tests 6 passed (6) ← post-fix

packages/objectql/src/skip-schema-sync-registers-object-metadata.test.ts pins the boot half
(no database, so it runs on every CI PR): the flag routes to the DDL-free registration and to no
DDL; the flag OFF still syncs and does not take the metadata route; federated objects stay with
registerExternalObject; a driver without the route degrades without throwing; a failed
registration is reported at error. Pre-fix: 6 failed (plugin.installRegisteredSchemas is not a function — the seam did not exist; the old branch logged a line and returned).

Reverse verification was run from the committed state and restoration proved on disk:
git hash-object == git rev-parse HEAD:PATH for both mutated files, git diff --exit-code
0, porcelain empty. The driver's tests import ../src/index.js — source, not dist — so the
mutation is in the file the run executes; no rebuild step is involved for the file under test,
and its dependency closure (@objectstack/spec, @objectstack/core) was built beforehand.

Suites and gates (all at a68f1a2103)

pnpm --filter @objectstack/driver-sql test → 109 passed | 1 skipped (110 files), 2059 tests [live PG]
pnpm --filter @objectstack/objectql test → 227 passed (227 files), 4023 tests
pnpm --filter @objectstack/spec test → 415 passed (415 files), 11062 tests
typecheck (spec, objectql, driver-sql) → Done, exit 0

Gate union from node scripts/pm/dispatch-gates.mjs (no path arguments, re-derived on the final
commit — identical to the earlier derivation): all 24 path-matched families green, plus the five
convention-triggered ones. Two are worth naming: check:query-options-erasure went red first
(the new pins pushed the test surface 240 → 243 with as any query bags) and is green after
typing them — "test surface: 240 site(s) … at the ceiling"; check:type-check-debt --re-measure
reports "OK — 33 ledger entr(ies) re-measured … none above its recorded number" after a full
workspace build. Every exit code was captured before any pipe.

Out of scope, filed

sql-driver.ts also carries #10997 and #11009. Neither is touched here: #10997 is not addressed in this PR, and #11009 remains open.


Generated by Claude Code

…un (#10995)
On a Postgres deployment that manages DDL out-of-band (`skipSchemaSync` /
`OS_SKIP_SCHEMA_SYNC=1`), writing an array to a JSON field returned 500
`DATABASE_ERROR`, a bare string returned 500, and an empty array was accepted
and silently stored as an empty object.
`formatInput` does stringify JSON-field values on every non-SQLite dialect, but
only for fields in the per-object `jsonFields` registry — and that registry was
built exclusively as the first step of a DDL call, so a boot that skips schema
sync served writes with every coercion registry empty and let node-postgres'
per-type defaults encode the value: object -> JSON text (accidentally correct),
array -> Postgres array literal -> `22P02`, `[]` -> `{}` (valid JSON, hence
accepted and corrupted), bare string -> raw -> `22P02`. SQLite hid it behind a
dialect-local bind-safety net, which is why the Turso/SQLite suites are blind.
Registration is now separable from DDL, per the #7737/#10629 ruling for
federated objects: `SqlDriver.registerObjectMetadata()` (declared optional on
`IDataDriver`) installs the coercion registries with no DDL and no round-trip,
a `skipSchemaSync` boot and every metadata reload take that route, and
`initObjects` registers before the ADR-0015 DDL gate refuses so guest
datasources are covered too. The refusal itself is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
`check:query-options-erasure` counts `as any` on a driver query bag in test
code too — the new pins pushed the test surface 240 -> 243. The options here
are on-contract, so they are typed rather than grandfathered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/driver-sql, @objectstack/objectql, @objectstack/spec, touching 10 documentable anchor(s).

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via IDataDriver (symbol), SqlDriver (symbol))
  • content/docs/data-modeling/objects.mdx(via ObjectQLPlugin (symbol))
  • content/docs/data-modeling/queries.mdx(via IDataDriver (symbol))
  • content/docs/deployment/troubleshooting.mdx(via auto_number (literal))
  • content/docs/kernel/contracts/data-engine.mdx(via IDataDriver (symbol))
  • content/docs/kernel/contracts/index.mdx(via IDataDriver (symbol))
  • content/docs/kernel/services-checklist.mdx(via ObjectQLPlugin (symbol))
  • content/docs/kernel/services.mdx(via IDataDriver (symbol), ObjectQLPlugin (symbol))
  • content/docs/permissions/authentication.mdx(via ObjectQLPlugin (symbol))
  • content/docs/plugins/packages.mdx(via ObjectQLPlugin (symbol), SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via ObjectQLPlugin (symbol), SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via IDataDriver (symbol), SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via IDataDriver (symbol), ObjectQLPlugin (symbol), SqlDriver (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 129 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 3e26359a7d87fbe0e03c7236346166c71a821ec0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 56ac86d3ef24d55c77c4b5bb839025b3809851bd — the merge of head a68f1a21035d5a805721e24087c3c68acecca88c into base 3e26359a7d87fbe0e03c7236346166c71a821ec0, 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 56ac86d3ef24d55c77c4b5bb839025b3809851bd && git checkout 56ac86d3ef24d55c77c4b5bb839025b3809851bd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3e26359a7d87fbe0e03c7236346166c71a821ec0 a68f1a21035d5a805721e24087c3c68acecca88c && git checkout -B drift-repro 3e26359a7d87fbe0e03c7236346166c71a821ec0 && git merge --no-ff a68f1a21035d5a805721e24087c3c68acecca88c
node scripts/docs-audit/affected-docs.mjs --json 3e26359a7d87fbe0e03c7236346166c71a821ec0

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 3e26359a7d87fbe0e03c7236346166c71a821ec0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32581338193 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.41s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 80 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-sql (PG): JSON-field values are bound without JSON.stringify — non-empty arrays and bare strings 500, empty array silently stored as {}

2 participants

@os-elon@claude