Skip to content

fix(driver-turso): remote mode materializes every declared object-level index (#17609) - #17615

Merged
hotlong merged 5 commits into
mainfrom
claude/issue-17609-remote-declared-indexes
Sep 11, 2026
Merged

fix(driver-turso): remote mode materializes every declared object-level index (#17609)#17615
hotlong merged 5 commits into
mainfrom
claude/issue-17609-remote-declared-indexes

Conversation

@hotlong

@hotlong hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Closes #17609

Clause-②: no — the driver now materializes indexes the metadata already declares; packages/spec/src/** is untouched, and no export, schema key or accepted/refused authoring input changes.

Premise, verified against origin/main @ 3ef96b471 before any edit

  • Remote mode: TursoDriver.initObjectsRemoteTransport.syncSchemasBatch. Every index DDL in remote-transport.ts came from buildUniqueIndexDDLuniqueIndexesFromFields (field-level unique only); emit sites :1841 / :1868 / :1941 / :1981, sole template :2196. Confirmed as the card states.
  • No other remote path creates declared indexes: git grep finds no other CREATE INDEX in driver-turso/src, and nothing outside driver-sql calls an index-sync method.
  • One correction: SqlDriver.syncDeclaredIndexes is at sql-driver.ts:12464 on that commit, not :9812. Nothing else in the premise moved.
  • Reproduced locally before the fix: the new parity suite, run on base + test file only (bd207df81), was 7 failed / 7; the remote face held only sqlite_autoindex_sys_notification_delivery_1 and planned the claim query as SCAN sys_notification_delivery + USE TEMP B-TREE FOR ORDER BY.

What changes

Only packages/drivers/driver-turso (plus a changeset). driver-sql is not modified — the remote face imports the already-exported normalizeDeclaredIndex beside uniqueIndexesFromFields / organizationKeyPartSql.

  • buildDeclaredIndexDDL (replaces buildUniqueIndexDDL): field-level unique via uniqueIndexesFromFields plus each declared entry via normalizeDeclaredIndex — the same two normalizers SqlDriver.syncDeclaredIndexes and expectedIndexes compose. Names come from buildIndexName (or the author's name); keys include the NULL-safe COALESCE(organization_id, '__global__') part for unique: 'organization'. No naming or keying rule is re-implemented. Emits CREATE [UNIQUE] INDEX IF NOT EXISTS. Unmaterialized columns (virtual formula) are skipped with a diagnostic, as locally.
  • New tables: every declared index rides the CREATE TABLE batch — no extra round trip.
  • Existing tables (retrofit): the existing column-probe read batch gains ONE statement, SELECT name FROM sqlite_master WHERE type='index'. Only indexes whose name is absent are issued, in one write batch kept outside the main DDL batch (the blast-radius separation of the field-level unique retrofit is preserved). No row is written.
  • retrofitDeclaredIndexes (replaces syncUniqueIndexes): batch → on failure, per statement → each failure goes to the durability sink (logger.error) naming index, table, DDL and the database's own cause. A unique index over existing duplicates is not forced and no row is repaired. A plain index that fails is reported the same way, with its consequence (full scans).
  • syncSchema (single-object path) behaves the same.
  • Author-declared index names are escaped, not refused. IndexSchema.name is any string and the local face quotes whatever it is given; holding it to SAFE_IDENTIFIER would have failed the remote face's whole schema sync — every object in the boot — over a declaration the other face accepts. An index name is one name, never a reference: the class aliasIdentifierSql already escapes. Key columns stay gated. Corpus at 3ef96b471: 85 files declare indexes: [, 0 author-given names are non-identifiers, so nothing in-tree changes behaviour.

Acceptance evidence

All pins live in packages/drivers/driver-turso/src/turso-local-remote-declared-index-parity.test.ts. Local face = new TursoDriver({ url: ':memory:' }) (the SqlDriver path). Remote face = new TursoDriver({ url: 'libsql://…', client }) with the real @libsql/client over file::memory: — not the better-sqlite3 stub, because a libsql write batch is transactional and the failure disposition depends on it (measured: one unique failure in a write batch rolls back a plain CREATE INDEX in the same batch).

Fixtures: sys_notification_delivery and sys_job_queue shapes (fields + indexes verbatim from service-messaging / platform-objects), and os17609_scoped — tenant column, field-level unique scoped by it, unique: 'organization', unique: 'global', two author-named indexes (one containing a space and a double quote), and an index over a formula field.

1 — every declared index, same name and key; new tables in batch; retrofit IF NOT EXISTS, zero data change

  • a fresh database carries the same index set — names, uniqueness, origin and key parts: per table, PRAGMA index_list + PRAGMA index_xinfo + the stored COALESCE text, local vs remote, deep-equal — anchored to buildIndexName-computed sets so two faces that both lost every index cannot pass:
    • sys_notification_delivery (6, the card's local count): idx_sys_notification_delivery_ad1bb657, idx_sys_notification_delivery_b3081700, idx_sys_notification_delivery_notification_id, idx_sys_notification_delivery_status_claimed_at, uniq_sys_notification_delivery_f5f71821, sqlite_autoindex_sys_notification_delivery_1
    • sys_job_queue (4, the card's local count): idx_sys_job_queue_queue_status_scheduled_for, idx_sys_job_queue_idempotency_key_queue, idx_sys_job_queue_status + sqlite_autoindex_sys_job_queue_1
    • os17609_scoped: uniq_os17609_scoped_organization_id_code (field-level, NULL-safe), uniq_os17609_scoped_organization_id_slug (organization, NULL-safe), uniq_os17609_scoped_external_id (global, verbatim), idx_os17609_scoped_region_slug + os17609_scoped_by_region, os17609 scoped-by "slug", sqlite_autoindex_os17609_scoped_1; the formula index is absent on both faces
  • the single-object syncSchema path lands the same set as the batch path
  • retrofits every declared index onto tables that already exist — and changes no row: tables provisioned without object-level indexes and holding rows; the next initObjects leaves every row byte-identical and total_changes() unchanged, every retrofit statement is IF NOT EXISTS, and the resulting shapes equal the local face's.

2 — failures are not silent

  • an object-level UNIQUE over existing duplicates: not created, named on the durability channel, no row repaired: two rows share (notification_id, recipient_id, channel). initObjects resolves; the dedup index is absent while the four plain indexes land (the transactional batch rolled them back — the per-statement fallback lands them); exactly one logger.error report names the index and table and carries UNIQUE constraint failed; nothing on warn; both duplicate rows remain.
  • a PLAIN index the server refuses: named on the durability channel with its cause; the rest still land: an injected SQLITE_FULL for the claim index → exactly one logger.error report naming it with the cause; the other four declared indexes land.

3 — parity test

The suite above: 7 pins, both faces, sqlite_master compared, covering both hot-table shapes.

4 — boot cost

  • steady state costs zero index DDL — one index-name read, riding a round trip the sync already pays: a second initObjects over existing indexes sends 0 index DDL statements. syncSchemasBatch alone, measured exactly: 2 round trips (batch, batch), 2N + 1 statements (N = 3 objects → 7), independent of how many indexes are declared (14 here). The index-name read is the last statement of the batch that already carries the N PRAGMA table_info probes.
  • Before, the same seam sent 2N + U statements over 3 round trips every boot when U > 0 (U = field-level unique indexes on existing tables, re-sent as CREATE UNIQUE INDEX IF NOT EXISTS); the red base run shows that statement re-sent for uniq_os17609_scoped_organization_id_code. Applied to the card's tenant A shape (100 tables, 4 field-level uniques) that formula gives 204 statements / 3 round trips before and 201 / 2 after — arithmetic from the measured formula, not a production reading.
  • The one-time cost moves to the first build after upgrade: each existing remote database builds its missing indexes once.

5 — EXPLAIN QUERY PLAN on the remote face

Pinned in serves the delivery claim query from the declared index — the same plan on both faces (asserts SEARCH … USING INDEX, no SCAN, plan text equal to local). Captured from the built package, real @libsql/client over file::memory:, one database across both steps:

EXPLAIN QUERY PLAN SELECT id FROM sys_notification_delivery WHERE status='pending' AND partition_key=? AND next_attempt_at<=? ORDER BY next_attempt_at LIMIT 50

remote — table as provisioned without object-level indexes (the pre-fix physical state):
  indexes: sqlite_autoindex_sys_notification_delivery_1
  SCAN sys_notification_delivery
  USE TEMP B-TREE FOR ORDER BY
remote — next initObjects on the SAME database with this fix (retrofit):
  indexes: idx_sys_notification_delivery_ad1bb657, idx_sys_notification_delivery_b3081700, idx_sys_notification_delivery_notification_id, idx_sys_notification_delivery_status_claimed_at, sqlite_autoindex_sys_notification_delivery_1, uniq_sys_notification_delivery_f5f71821
  SEARCH sys_notification_delivery USING INDEX idx_sys_notification_delivery_b3081700 (status=? AND partition_key=? AND next_attempt_at<?)
local — reference:
  indexes: (identical to the retrofitted remote set)
  SEARCH sys_notification_delivery USING INDEX idx_sys_notification_delivery_b3081700 (status=? AND partition_key=? AND next_attempt_at<?)

Reverse verification

Directions predicted before each run. The suite imports ./turso-driver.js from src, so the mutated src/remote-transport.ts is what ran (the local face's @objectstack/driver-sql resolves to its built dist, which no leg touched). Each leg: marker counted on disk before the run; restore by git checkout HEAD --, proven by the blob hash equalling HEAD's and an empty git diff HEAD.

  • Base + test only (bd207df81): 7 failed / 7. Fix (6da2d2684): 7 passed.
  • Final HEAD 6241e6e51, three ablations of src/remote-transport.ts:
    • AmissingIndexes returns every planned index (no existing-name skip). Predicted 1 failed | 6 passed, only the steady-state pin. Observed 1 failed | 6 passed: steady state costs zero index DDL….
    • B — the retrofit fallback's durability report silenced. Predicted 2 failed | 5 passed, both failure-report pins. Observed 2 failed | 5 passed: exactly those two.
    • C — the index name spelled raw (a bare double-quoted name) instead of escaped. Predicted 4 failed | 3 passed: every pin that syncs os17609_scoped, whose quote-bearing index name breaks the raw statement, and none of the three that sync only sys_notification_delivery. Observed 4 failed | 3 passed: fresh database, syncSchema path, retrofit, steady state.
    • Every leg: marker counted once on disk before the run; restored blob 90cf0ed7 equals HEAD's; git diff HEAD empty afterwards.

Local verification (HEAD 6241e6e51)

Exit codes were captured before any pipe; each reading quotes the gate's own verdict line.

Package — @objectstack/driver-turso

  • pnpm --filter @objectstack/driver-turso test — exit 0: 52 files, 1243 tests passed.
  • pnpm --filter @objectstack/driver-turso typecheck — exit 0. tsc --listFiles counts all 52 test files, the new suite among them.
  • pnpm --filter @objectstack/driver-turso build — exit 0: check-dts-emitted: @objectstack/driver-turso - 1/1 declared declaration file(s) present.
  • Dependency closure, at 3ef96b471 before any edit: pnpm turbo run build --filter=@objectstack/driver-turso^... --concurrency=2 — 7 successful, 0 cached.
  • @objectstack/driver-sql is not modified, so no driver-sql test / build / typecheck is owed.

Lint — a declared narrowing, with its proof (the repo-wide pnpm lint is CI's run)

  • pnpm exec eslint --no-inline-config --format json over the 4 touched .ts files — exit 0. The JSON reports 4 files, 0 errors, 0 warnings, and no ignore notice on any of them, so all 4 are inside eslint's own lint population.
  • Invariance: eslint.config.mjs never enables type-aware linting — it says so itself (no parserOptions.project, no typed @typescript-eslint rules) — so this diff cannot move a verdict in any untouched file. The fifth changed path is the changeset (Markdown, not linted).

Derived gatesnode scripts/pm/dispatch-gates.mjs --commands on this tree derives 62 commands. Also run: the 4 roster gates it flags under this PR's own directories (check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity) and check:durability-log-level.

  • 68 exit 0. Among them:
    • pnpm check:durability-log-level35 durability-critical catch seam(s), all loud, rethrowing or propagating to the caller.
    • pnpm check:nul-bytesOK (scanned 8367 text file(s) … no raw ASCII control bytes).
    • pnpm check:cross-package-test-inputs28 package(s) read outside themselves, all declared (the new suite reads nothing outside its package).
    • pnpm check:test-source-alias, pnpm check:type-source-resolution, pnpm check:object-def-param-keys, pnpm check:driver-conformance, pnpm check:published-files — OK.
    • pnpm check:type-check-coverage76/80 workspace packages type-checked.
    • node scripts/check-adr-0087-registration.mjs --base origin/mainthis PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen); node scripts/check-empty-changeset.mjs --base origin/mainNo empty-frontmatter changeset introduced.
    • node scripts/check-changeset-no-major.mjs --base origin/main — exit 0, but locally it printed no pull_request payload was available to read a declaration from: the Clause-② limb reads this PR body, which only CI can hand it.
    • pnpm check:lean-entry-closure — first run exit 3 (PREREQUISITE NOT MET: no built objectql); after pnpm turbo run build --filter=@objectstack/objectql --concurrency=2 (15 tasks, all cached) re-run exit 0: 2 published condition(s) measured from a real load … Admitted set held exactly (15 packages).
    • pnpm check:dts-closure and pnpm check:sourcemap-no-sources-content — exit 0 over the packages built in this worktree (driver-turso among them), not the whole repo.
  • 2 NOT MEASURED (exit 3, PREREQUISITE NOT MET — the gates' own words: neither a pass nor a failure):
    • pnpm check:dual-build-cjs-loads — reads every package's built output; 78 packages have no dist/ here. It needs a full pnpm build, which this seat did not run beside another seat's build. CI builds first.
    • pnpm check:type-check-debt--re-measure needs built type entry points for 24 workspace dependencies of the ledgered packages; same reason. driver-turso is not in that ledger.
  • Reconciliation: node scripts/pm/dispatch-gates.mjs --ran over a record carrying every command as command :: exit N62 derived famil(ies) accounted for — 60 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3), 0 unrun. Derived on this tree, 2 commits behind origin/main 0918c4411; the tool reports none of those commits touched what the derivation reads.
  • dispatch-gates also names what no local command covers, all NOT MEASURED here and owned by CI: 5 families whose argv takes a workflow value, 5 path-scheduled CI jobs (Test Core, Temporal Conformance, Dogfood Regression Gate, Dogfood Verify CLI, Build Core), 11 wide-population families and 45 roster families.

Lock: every scripts/pm/os-verify-lock.sh call ended VERDICT … UNLOCKED (declared) — a macOS host with no flock — so the shared verify lock was never taken and nothing was serialized.

Acceptance notes


Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-turso, touching 11 documentable anchor(s).

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

  • content/docs/data-modeling/drivers.mdx (via syncSchema (symbol, a method of class RemoteTransport), syncSchemasBatch (symbol, a method of class RemoteTransport))
  • content/docs/protocol/kernel/lifecycle.mdx (via syncSchema (symbol, a method of class RemoteTransport))
  • content/docs/protocol/objectql/types.mdx (via syncSchema (symbol, a method of class RemoteTransport))

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

  • content/docs/releases/implementation-status.mdx (via syncSchema (symbol, a method of class RemoteTransport))

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
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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.

Coarse fallback — 6 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 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2d2e21a8b85214ea6ec30293ef07479c0e307e36 — the merge of head 6241e6e511b82ccaa60772c2033ee4039f70f1b9 into base 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4, 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 2d2e21a8b85214ea6ec30293ef07479c0e307e36 && git checkout 2d2e21a8b85214ea6ec30293ef07479c0e307e36
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 6241e6e511b82ccaa60772c2033ee4039f70f1b9 && git checkout -B drift-repro 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 && git merge --no-ff 6241e6e511b82ccaa60772c2033ee4039f70f1b9

node scripts/docs-audit/affected-docs.mjs --json 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4

⚠️ 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 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@hotlong

hotlong commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

PM 复核(maintainer direct dispatch,Session c5c0ce54…

逐项核对的是实际 diff,不是席位报告:

读数 结论
改动文件 5 个:changeset、remote-transport.tsturso-driver.ts、新对等测试、remote-transport-unsafe-identifier-envelope.test.ts 与卡片范围一致
Clause-② 路径肢 packages/spec/src/** 改动 0driver-sql 改动 0 no 成立
retrofit 是否只做 DDL diff 新增行里只有 CREATE [UNIQUE] INDEX IF NOT EXISTS无任何 DELETE / UPDATE / INSERT 满足「零改数据」
命名与键是否与本地面同源 复用已导出的 normalizeDeclaredIndex + uniqueIndexesFromFields(名字经 buildIndexName),未另起命名 满足验收 1
失败是否响亮 诊断出口在构造期接到 logger.error 满足验收 2

越界点:作者声明的索引从「拒绝」改为「转义」——审过,接受

  • 实现:aliasIdentifierSql(alias) → "${String(alias).replace(/"/g, '""')}",标准引号标识符加双写转义;该函数已用于 groupBy 输出键(drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 做过同样的迁移),本 PR 没有新写转义
  • 边界没有扩大到引用位置:索引键列仍然走 assertSafeIdentifier(PR head remote-transport.ts:2260
  • 被改的既有测试 remote-transport-unsafe-identifier-envelope.test.ts 只改了注释,把「index name」从仍拒绝的位置列表里移出并指向新对等测试;其余位置(objectfieldgroupBy FIELD、DDL 表名/列名、索引键列)仍拒绝
  • 理由成立:拒绝会让远程面因一个本地面能正常引号化的合法名字而整批 schema 同步失败;语料中非标识符形态的声明索引名为 0

验证口径说明

席位推送后工作树已清理,我没有在本地重跑新对等测试;以本 PR 的 CI(Test Core 分片、Type Check、Lint & Repo Gates 等)作为独立验证。席位报告的两项本地 NOT MEASURED(check:dual-build-cjs-loadscheck:type-check-debt --re-measure,需全仓构建)同样交给 CI。

下一步:CI 全部结束且无失败 → 转 ready → 入合并队列。有任何红,按失败日志回派同一席位。

@hotlong
hotlong marked this pull request as ready for review September 11, 2026 04:33
@hotlong
hotlong enabled auto-merge September 11, 2026 04:33
@hotlong
hotlong added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit bdea10a Sep 11, 2026
41 checks passed
@hotlong
hotlong deleted the claude/issue-17609-remote-declared-indexes branch September 11, 2026 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant