Repository navigation
fix(driver-sqlite-wasm): a text value round-trips byte-for-byte — U+0000 no longer truncates it, a leading U+FEFF is no longer dropped on read - #19998
Conversation
…no truncation at U+0000, no leading U+FEFF dropped on read Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
…the round trip separately, with filters and refusals Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
… verdict Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
…arameter form carries a NUL-bearing text whole — no statement is refused Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
…lite-wasm-text-roundtrip
…uery instead of erasing them Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 11 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 d2b0296585e97defce22c21b20183ff466d62e47 && git checkout d2b0296585e97defce22c21b20183ff466d62e47
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1f05ea4fb296357dedf86ed34a234a1e779b1381 98cb908738599a72538e5f32df4531dc357e61d2 && git checkout -B drift-repro 1f05ea4fb296357dedf86ed34a234a1e779b1381 && git merge --no-ff 98cb908738599a72538e5f32df4531dc357e61d2
node scripts/docs-audit/affected-docs.mjs --json 1f05ea4fb296357dedf86ed34a234a1e779b1381
|
…s change made false — sql.js now reads a leading U+FEFF and an embedded NUL back verbatim Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Scope read:
Probes ran in a scratch worktree at head, on a turbo build of the two drivers' dependency closure; ① Derived judgments
② Semver level
③ Boundary flagsThe corrected note is S1. "A cell the engine does not read back verbatim is left as stored and keeps reading as it did, where the old statement rewrote it: on any engine, text holding invalid UTF-8."
S2. "
S3. "A legacy text with a leading U+FEFF or an embedded NUL is read back verbatim on all three, and is rewritten like any other plain string."
S4 (unchanged). "Each rewrite is a compare-and-set on the text it was decided from, …": TRUE by code. S5 (unchanged). "This covers every local SQLite face that inherits the backfill: …": TRUE by code-read. The backfill is gated on
The backfill bullet in Every other sentence of the 19978 changeset: TRUE against head. That covers the title and PR body (it does not ship; the findings are still recorded):
Implemented-by: VERDICT: FAIL What must change:
After the patch, a new same-head record is needed under ruling 1A. Nothing in ① or ② needs to move. Isolated reviewer: a contract-review-tier subagent, fed only the card, the PR, ruling 1A and AGENTS.md; the seat adopted its record. |
… sentences — TursoDriver local runs on better-sqlite3, not libsql Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Scope read:
Probes ran on a detached scratch worktree at head, on a turbo build of the three drivers' dependency closure. The wasm dist was confirmed to carry ① Derived judgments
② Semver levelStill consistent, and nothing needs to move.
③ Boundary flagsThe corrected note: S1. "A cell the engine does not read back verbatim is left as stored and keeps reading as it did, where the old statement rewrote it: text holding invalid UTF-8, measured on better-sqlite3 and sql.js."
S2. "
S3. "A legacy text with a leading U+FEFF or an embedded NUL is read back verbatim on all three faces, and is rewritten like any other plain string."
S4 (unchanged). "Each rewrite is a compare-and-set on the text it was decided from, …": the code's S5 (unchanged). "This covers every local SQLite face that inherits the backfill: …": by code,
PR body (it does not ship; the findings are still recorded):
Implemented-by: VERDICT: PASS No shipped sentence is false at this head. The one change to the accept set that this fix causes elsewhere, driver-sql's json backfill now converging BOM/NUL legacy cells on the sql.js face, is declared in the 19978 note. Under ruling 1A, this record names the corrected note and judges each rewritten sentence. It is the confirmation of the DELIBERATE CORRECTION red. The two PR-body findings do not ship, and the seat corrects them in the body. Isolated reviewer: a contract-review-tier subagent, second read on this PR, fed only the card, the PR, the prior record, ruling 1A and AGENTS.md; adopted by the seat. |
Pending release-note correction on this PR:
|
Fixes #19978
Clause-②: no
SqliteWasmDrivernow stores and reads a text value byte-for-byte, asSqlDriveron better-sqlite3 does. Before this change, an embedded U+0000 cut the stored value short, and a leading U+FEFF was dropped when the value was read. Neither raised. The fix lives entirely in this package's sql.js transport (knex-wasm-dialect.tsplus a newsqljs-exact-text.ts). sql.js is not patched. No code line inSqlDrivermoves, and no public export moves. One doc-comment parenthetical insql-driver.tsand one sentence of a pending changeset are corrected, because this fix makes them false (see "Deliberate correction of a pending changeset" below).Where each byte was lost (measured first; the card's readings were a scratch probe nobody had re-run)
sql.js 1.14.1 is what the lockfile installs (
pnpm-lock.yaml:sql.js@1.14.1) and whatnode_modules/sql.js/package.jsonreports. The control is better-sqlite3 via driver-sql's devDependency. The probes are scratch files, not committed.Raw sql.js, write side. Bind a JS string, then read
hex(v). The hex is ASCII, so the read decode cannot hide a write-side loss:'a'+ U+0000 +'b'61006261610062'ab'+ U+00006162006162616200'hello'EFBBBF68656C6C6FEFBBBF68656C6C6FEFBBBF68656C6C6F'he'+ U+FEFF +'llo'6865EFBBBF6C6C6FRaw sql.js, read side. Cells are planted with
cast(x'…' as text), so no bind is involved:getStringgetBlobon the same cell610062"a"610062"a\u0000b"EFBBBF78"x"EFBBBF78"x"EFBBBF+ 33 ASCII bytesH1 holds, and the loss has two separate causes. The write loses a U+0000. The read loses both a U+0000 and a leading U+FEFF. A U+FEFF was always stored intact.
H2: the exact functions.
Statement.prototype.bindStringcallssqlite3_bind_text(this.stmt, pos, strptr, -1, 0)(dist/sql-wasm-debug.jslines 684–695). The length-1tells SQLite to read up to the first NUL.Statement.prototype.getStringreturnssqlite3_column_text, cwrapped with return type"string", soUTF8ToStringdecodes it. That function stops at the first NUL (findStringEnd) and decodes through a module-levelnew TextDecoder()whose defaultignoreBOM: falsedrops a leading BOM. In the minifieddist/sql-wasm.jsthis isZa=new TextDecoderwithz=(a,b,c)=>a?Za.decode(C.subarray(a,$a(C,a,b,c))):"", so the BOM is dropped at every length. The debug build decodes strings of 16 bytes or fewer by hand, and those keep it. That is why a long-BOM case is pinned too.Client_WasmSqlite._queryinknex-wasm-dialect.tsowns both. It binds throughstmt.bind/db.runand decodes throughstmt.getAsObject().wasm-connection.tsowns neither.Driver level, base
a7581b326:create→findOne, andfindwith{ v: value }.The fix (H3: it fits in our adapter)
Read. A cell that
stmt.get()returns as a string is a TEXT cell. For each one,readExactRowre-reads the stored bytes withStatement.getBlob(sqlite3_column_bytes+sqlite3_column_blob; SQLite hands a UTF-8 TEXT value tocolumn_blobunconverted) and decodes them withTextDecoder('utf-8', { ignoreBOM: true }).getBlobis not in@types/sql.js, but it is the method sql.js's ownget()calls for a BLOB column. It keeps its name in all three 1.14.1 builds this package can load (sql-wasm.js,sql-wasm-browser.js,sql-wasm-debug.js);getStringis renamed in the minified two. IfgetBlobis absent, the read throws and does not fall back to the lossy decode.Write. Only a string binding that holds U+0000 is changed; nothing else can be truncated. It is bound as its UTF-8 bytes (a
Uint8Array, which sql.js binds as a BLOB with an explicit length), and every parameter token that receives it is wrapped as+CAST(PARAM AS TEXT). The unary plus is load-bearing.CAST(… AS TEXT)alone carries TEXT affinity and changes a comparison. Measured on sql.js: comparing the integer5as less than the text' x'answers 1 against a bound text, 0 throughCAST(? AS TEXT), and 1 through+CAST(? AS TEXT)(better-sqlite3 bound: 1). A numeric-affinity column stores'12'as integer through either path. A statement with no such binding comes back as the same string and the same array.Which token receives which binding follows SQLite's own numbering rule, which
sqljs-exact-text.tsapplies:?takes the largest index so far + 1;?NNNtakesNNN;:name/@name/#name/$nametake the index of their first occurrence;[…]or comments is a parameter.Wrapping adds or removes no parameter token, so no index moves. No statement form is refused, which is why this is
Clause-②: noand not a narrowing.Ruled out: an explicit-length
sqlite3_bind_textthrough the statement pointer. That pointer isthis.stmtonly in the debug build; the minified builds rename it (this.Qa), so it is not a usable seam.H4: why the shared case table is not edited here
VALUE_ROUNDTRIP_CASESlives inpackages/spec/src/data/value-roundtrip-conformance.ts. The claim's file surface allowed a shared case file underpackages/drivers/driver-sql/src/"that the SQLite family's conformance suites read". No such file exists, and the real one is inpackages/spec, so this is the stop-on-breach case and the table is not edited. There is a second reason, a substantive one.sql-driver-value-roundtrip-conformance.test.tsruns that table through the dialect matrix, including the live Postgres CI job. Postgres documents that itstexttype cannot store the character with code zero, so a U+0000 row would make that cell fail on Postgres. That is a platform decision (refuse U+0000 everywhere? answer per dialect?), not a driver fix. It is NOT MEASURED here, because there is no live Postgres in this container. A leading-U+FEFF row is a better candidate for the shared table, but how the MySQL and Postgres drivers decode it is unmeasured here. Both questions go to the seat in the report. Meanwhile the answer is pinned in this package, against the written value and its own UTF-8.Tests
New file
sqlite-wasm-text-bytes-roundtrip.test.ts, 35 cases. Each seam is pinned on its own, because one can hide the other:hex(v)equals the written string's UTF-8, for U+0000 in the middle and trailing, U+FEFF leading (short and long) and in the middle, and a plain control.610062,EFBBBF78, a long BOM value, a plain control).create→findOne, andupdate(whose returned row is also checked).'a'no longer matches the stored'a'+ U+0000 +'b'; before, the comparand was cut at the same NUL, so it did.$inplaces each NUL-bearing comparand.$containsU+FEFF and$startsWithU+FEFF select the right rows and read them back whole.'it''s ?',`a?`,[b?],"?", both comment forms and the identifierc$d;?NNNand:namethrough the driver;getBlobthrows.Ablations. Each ran from the committed state through
scripts/ablation-replace.mjs. Every mutation was proved on disk (anchor 1 → 0, blob changed), and every restore was proved (blob equal to HEAD,git diff HEADempty). The subject resolves fromsrc(relative imports), so no build was involved:stmt.getAsObject(). 11 of 35 red: every read pin, the update pin,$containsU+FEFF (its values), and the two numbered/named pins. Every write-hex pin and every equality filter stayed green.truncatable.size === 0→truncatable.size >= 0). 10 of 35 red: the U+0000 hex pins and reads, update, the prefix-equality pin, and the placement pins. Every read-seam pin stayed green. A first attempt used a replacement that was a substring of its anchor. The tool refused it (replacement count 1 → 1) and restored, and no test ran; it was repeated with a distinct replacement.r: 0, not 1) and the two literal-placement pins.?NNNnumbered as a bare?. 2 red: the?2/?1driver pin and the mixed-statement pin.Package runs at
58ee6c042:pnpm --filter @objectstack/driver-sqlite-wasm test: 30 files, 556 passed.typecheck(tsc --noEmit): exit 0. Its program includes both new files, checked with--listFiles.Deliberate correction of a pending changeset
The dispatch asked whether the "not read back verbatim" sentence in
.changeset/19912-json-backfill-depth-limit.mdstill holds. That changeset is still pending and belongs to a landed PR. It says the backfill leaves as stored, "onSqliteWasmDriver, a legacy text with a leading U+FEFF or an embedded NUL (sql.js drops both when it reads the text)". After the fix (head58ee6c042) that is false. Measured with a scratch probe: legacy json TEXT cells were planted, then the backfill ran via a secondinitObjects.SqliteWasmDrivernow converges these cells exactly as better-sqlite3 does.ablation-dist-preflightconfirmed the ablation was present indist/for the control. After the rebuild it was absent again and the tree was clean. The same parenthetical sits inSqlDriver.backfillCanonicalJsonEncoding's doc block inpackages/drivers/driver-sql/src/sql-driver.ts("sql.js drops an embedded NUL and a leading U+FEFF"). The seat ruled that both are corrected in this PR (claim amendment 5818397702 on #19978). Patch round 1 (6a195b37c) made the edits below, and patch round 2 (98cb90873) corrected them after contract review 5819169241 (FAIL). Round 1 had labelledTursoDriver's local mode as libsql, but every non-remote arm ofTursoDriver.toKnexConfighands Knexclient: 'better-sqlite3'. The measurement behind the edits was taken fresh at58ee6c042on three local faces, which run on two engines:SqlDriveron better-sqlite3,TursoDriverin local mode (better-sqlite3 through Knex, url:memory:), andSqliteWasmDriver(sql.js). All three convertEFBBBF78→22EFBBBF7822,610062→22615C75303030306222and the control68656C6C6F→2268656C6C6F22. All three leave invalid UTF-8 (FF78,61C3) as stored, because each reads it back with U+FFFD in place of the invalid bytes.SqlDriver'ssqlite3/sqliteclients are NOT MEASURED, because that client is not installed in the container. libsql is not a local engine here and is not cited. The reviewer read it directly (@libsql/client0.17.4,:memory:): a storedFF78panics the native binding, and a stored610062reads back as"a"..changeset/19912-json-backfill-depth-limit.md(pending, landed with PR #19972; one sentence of the merge base rewritten into three):SqliteWasmDriver, a legacy text with a leading U+FEFF or an embedded NUL (sql.js drops both when it reads the text); on any engine, text holding invalid UTF-8."SqlDriveron better-sqlite3,TursoDriverin local mode (which runs on better-sqlite3 too) andSqliteWasmDriver(sql.js) were each measured to read such a cell back with U+FFFD in place of the invalid bytes, so the text the rewrite would be decided from is not the stored text, and the cell is left alone. A legacy text with a leading U+FEFF or an embedded NUL is read back verbatim on all three faces, and is rewritten like any other plain string."packages/drivers/driver-sql/src/sql-driver.ts, in thebackfillCanonicalJsonEncodingdoc block. Two comment lines change and no code line moves. A build of driver-sql from the merge base and one from the head differ in exactly those two comment lines ofdist/index.js/dist/index.mjs, re-proved in round 2. The driver-sql suite reads 2680 passed / 170 skipped on both copies (round 1).This PR's own changeset gains one bullet: the local
Field.jsonbackfill now converts those legacy cells on this driver too, with the measured bytes.Check Changesetis red by design on this head.check-empty-changeset.mjsreads the 19912 edit as the DELIBERATE CORRECTION class ("do NOT restore it -- say so on the PR and get it confirmed"). The edit is not restored andskip-changesetis not applied. Under ruling 1A (#19940, 5814546887), the confirmation is a same-head at-tier contract-review PASS that names this note and judges each rewritten sentence.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackon this head gives 59 commands. All 59 exit 0 at58ee6c042.--ranreconciliation: 59 derived, 59 run, 0 NOT-MEASURED (derived from recorded exit codes).check:dual-build-cjs-loads,check:lean-entry-closureandcheck:type-check-debtfirst answered PREREQUISITE NOT MET (exit 3). They were re-run afterturbo run build --filter='./packages/*' --filter='./packages/*/*'and answered 0.check:query-options-erasurefirst went red: the test surface grew from 236 to 242, from this test'sas anyquery casts. The queries are now typed asDriverQuery, and the gate is green.pnpm check:driver-conformancereadsOK — 50 covered cell(s), 0 in the DEBT ledger, 0 exemptboth on basea7581b326and at58ee6c042. No cell was added or removed.node scripts/check-issue-citations.mjs --base origin/main: exit 0.6a195b37c.dispatch-gatesre-derives 62 commands: the 59 above, pluscheck:dispatcher-error-vocabulary,check:object-def-param-keysandcheck:tenant-chokepoint, whichsql-driver.tsbrings in. All 62 ran after a./packages/*build. 61 exit 0, andcheck-empty-changesetexits 1 by design (the correction above).check-issue-citations --base origin/mainexits 2 at this head, but only becauseorigin/mainmoved past the merge base: fix(plugin-security, objectql): a row-level check holds for every row of an array insert and a predicate update #19988 removed three#16608citations from a file this diff does not touch. Against the merge base67ebc84a7it exits 0. The driver-sqlite-wasm suite reads 30 files / 556 passed, and typecheck exits 0 for driver-sqlite-wasm and driver-sql.check:driver-conformanceis unchanged: 50 covered, 0 in debt.98cb90873(text only, the same two files).check-changeset-no-major,check-adr-0087-registration,check-issue-citations --base 67ebc84a7,check:doc-authoring,check:nul-bytesand driver-sql typecheck all exit 0.check-empty-changesetexits 1 by design: one::errorannotation, on the 19912 note.dispatch-gatesre-derives the same 62 families.pnpm exec eslint --no-inline-config --format jsonover the three changed TypeScript files gives 3 files, 0 errors, 0 warnings.eslint.config.mjssets noparserOptions.project(0 hits forproject:/projectService), so linting is not type-aware and this diff cannot move any untouched file's verdict.pnpm lintis CI's.Acceptance notes
$containsor$startsWithwhose comparand holds U+0000 is matched on both SQLite faces, better-sqlite3 included, with the GLOB pattern cut at that U+0000 (wildcards after it included) against each value cut at its own first U+0000. So$contains 'a'+U+0000 answers only the row stored as'a'+U+0000+'b'(measured by the second contract review).glob()cuts both the pattern and the value at their first U+0000. So a comparand that starts with U+0000 makes$containsand$endsWithmatch every row, and makes$startsWithanswer only the rows that are empty before their first U+0000 (re-measured in patch round 2 on better-sqlite3 and sql.js, which answer identically). The SQLite text predicate isGLOB, andglob()reads its pattern as a C string. Measured on both faces with the probe above. That is a filter that silently gives the wrong answer, reproducible today. It is not fixed here; the seat filed it as driver-sql (SQLite faces): a$contains/$startsWith/$endsWithcomparand holding U+0000 is cut at the NUL byglob(), so the filter answers wrongly; one that starts with U+0000 makes$contains/$endsWithmatch every row #19999.WasmSqliteConnection'sdefaultLocateFile()callsrequire.resolve('sql.js/package.json'). That throwsERR_PACKAGE_PATH_NOT_EXPORTEDon sql.js 1.14.1, whoseexportsmap has no./package.json. So it always returnsundefined, and sql.js then locates its own.wasm, which works. Behaviour is unaffected; only the docblock's claim is dead. Noted, not filed. Carrier: none.origin/main(67ebc84a7) was merged in before opening. It shares no path with this diff, and the lockfile did not move. The dependency closure was rebuilt and the package suite re-run on the merged head.Generated by Claude Code