fix(driver-turso): remote 分页读补齐确定性排序，与 local 面共用同一条规则 (#5653)

.changeset/turso-remote-pagination-tiebreaker.md

@@ -0,0 +1,32 @@
+---
+'@objectstack/driver-turso': patch
+---
+
+fix(driver-turso): remote 分页读补齐确定性排序，与 local 面共用同一条规则
+
+`TursoDriver` 在 remote 传输(`libsql://` / `https://` 等 URL)下的分页读不满足
+`IDataDriver.find` 的确定性分页 MUST：`RemoteTransport.buildSelectSQL` 把调用方的
+`orderBy` 原样拼进 SQL 后直接接 `LIMIT` / `OFFSET`，不追加任何唯一列，无序分页读
+更是完全不排序。SQLite 不承诺并列行在两条语句之间排布一致，所以表一大、计划一变，
+`ORDER BY status LIMIT 50 OFFSET 50` 翻页时就会有记录出现两次、另一条永远不出现 ——
+每一页都是满的、每一行都合法，从任何单个响应里都看不出来。
+
+同一个驱动的 local 面早已按 #4363 办事，于是一个驱动的两条传输对同一个分页查询给出
+不同的排序保证，而传输模式只由 URL 决定。
+
+修法是**复用**而不是复制：`TursoDriver.find` / `findOne` 现在通过继承来的
+`SqlDriver.orderKeysFor()` 解析出完整排序键再交给传输层，三态规则只有一份实现 ——
+
+| `orderBy` | 分页 | 结果 |
+|---|---|---|
+| 非空 | 任意 | 调用方的键 + `id` |
+| 空 | 有 `limit`/`offset` | 单独 `id` |
+| 空 | 都没有 | 不加 ORDER BY（#4363 carve-out，原样保留） |
+
+`findOne` 的语义一并保住：它的 `limit: 1` 由传输层自己注入，若在 `buildSelectSQL`
+里判定就会被误读成「页大小为 1 的第一页」，从而给系统里最热的读加上
+`ORDER BY id LIMIT 1` —— 正是让计划器放弃谓词自身索引的形状。
+
+唯一列的判定沿用 local 面同样保守的前提：只有本驱动自己建的表才追加 `id`
+(`RemoteTransport` 建表时无条件写入 `"id" TEXT PRIMARY KEY`)；不是自己建的表保持
+原样并告警一次，绝不凭空发明排序列。

packages/drivers/driver-turso/src/remote-pagination-tiebreaker.test.ts

@@ -0,0 +1,201 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The ORDER BY the REMOTE transport actually sends (#5653) — the clause-level
+ * companion to `turso-remote-pagination-conformance.test.ts`.
+ *
+ * That suite asks the contract's question ("is a page walk a partition of the
+ * result set?") on rows, which is the only instrument that can catch a *lost*
+ * clause. This one asks the question the fix turns on, which rows cannot
+ * distinguish on a twelve-row in-memory table: **which clause went out**. Both
+ * are needed, and neither substitutes for the other — a plan that happens to
+ * return insertion order satisfies the row assertions while the statement
+ * carries no tie-breaker at all, which is exactly how the gap #5653 names
+ * survived under a green suite.
+ *
+ * What is pinned here is the three-state table `SqlDriver.orderKeysFor`
+ * implements, observed through the SQL the remote transport emits:
+ *
+ * | `orderBy` | paged | ORDER BY sent |
+ * |---|---|---|
+ * | non-empty | either | caller's keys, then `"id"` |
+ * | empty | `limit`/`offset` present | `"id"` alone |
+ * | empty | neither | **none** — objectstack#4363's carve-out |
+ *
+ * plus the two conditions that keep it honest: a `findOne` (whose `limit: 1`
+ * the transport injects *itself*) must NOT be read as a page, and a table this
+ * driver never created gets no invented `id` column.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { TursoDriver } from './turso-driver.js';
+
+const TICKET_OBJECT = {
+  name: 'ticket',
+  fields: {
+    status: { type: 'string' },
+    rank: { type: 'integer' },
+  },
+};
+
+/**
+ * A remote driver over a client that records every statement and answers with
+ * no rows. Rows are irrelevant here — the statement is the measurement.
+ */
+async function makeRecordingRemoteDriver(options?: { sync?: boolean }) {
+  const statements: Array<{ sql: string; args: unknown[] }> = [];
+  const record = (stmt: any) => {
+    statements.push({ sql: stmt?.sql ?? String(stmt), args: stmt?.args ?? [] });
+    return { rows: [], columns: [], rowsAffected: 0 };
+  };
+  const client = {
+    execute: vi.fn(async (stmt: any) => record(stmt)),
+    batch: vi.fn(async (stmts: any[]) => stmts.map(record)),
+    close: vi.fn(),
+  };
+  const driver = new TursoDriver({
+    url: 'libsql://tiebreaker.turso.io',
+    client: client as never,
+  });
+  await driver.connect();
+  expect(driver.transportMode).toBe('remote');
+  if (options?.sync !== false) {
+    await driver.syncSchema(TICKET_OBJECT.name, TICKET_OBJECT);
+  }
+  statements.length = 0; // drop the DDL round-trip; only reads are under test
+  return { driver, statements };
+}
+
+/** The ORDER BY clause of the last statement sent, or `null` if it had none. */
+const lastOrderBy = (statements: Array<{ sql: string }>): string | null => {
+  const sql = statements[statements.length - 1]?.sql ?? '';
+  const match = /ORDER BY (.*?)(?: LIMIT | OFFSET |$)/.exec(sql);
+  return match ? match[1] : null;
+};
+
+describe('TursoDriver remote — the ORDER BY a paged read goes out with (#5653)', () => {
+  it('appends the id tie-breaker after the caller sort keys on a paged read', async () => {
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', {
+      orderBy: [{ field: 'status', order: 'asc' }],
+      limit: 5,
+      offset: 5,
+    });
+    expect(lastOrderBy(statements)).toBe('"status" ASC, "id" ASC');
+    expect(statements[statements.length - 1].sql).toContain('LIMIT ? OFFSET ?');
+    await driver.disconnect();
+  });
+
+  it('orders a paged read with NO orderBy by id alone', async () => {
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', { limit: 5, offset: 5 });
+    expect(lastOrderBy(statements)).toBe('"id" ASC');
+    await driver.disconnect();
+  });
+
+  it('leaves an UNPAGED unordered read with no ORDER BY at all (#4363 carve-out)', async () => {
+    // The carve-out is half the fix, not a leftover: an unpaged read hands back
+    // the whole matching set, so there is no partial view to be wrong about,
+    // and an imposed sort would only change plan selection for the majority of
+    // reads in the system.
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', {});
+    expect(statements[statements.length - 1].sql).not.toContain('ORDER BY');
+    expect(lastOrderBy(statements)).toBeNull();
+    await driver.disconnect();
+  });
+
+  it('still appends the tie-breaker to an UNPAGED sorted read', async () => {
+    // Row one of the table: a caller who named a non-unique key gets a total
+    // order whether or not this particular statement is sliced — which is what
+    // makes the paged walk and the whole-set read agree.
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', { orderBy: [{ field: 'status', order: 'asc' }] });
+    expect(lastOrderBy(statements)).toBe('"status" ASC, "id" ASC');
+    await driver.disconnect();
+  });
+
+  it('counts `limit` alone as paged — page one is not exempt', async () => {
+    // A page one that disagrees with the ordering pages two onward use leaves
+    // the defect fully intact while looking like a fix.
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', { limit: 50 });
+    expect(lastOrderBy(statements)).toBe('"id" ASC');
+    await driver.disconnect();
+  });
+
+  it('follows the last requested key direction, so a compound index still walks in one pass', async () => {
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', {
+      orderBy: [{ field: 'status', order: 'asc' }, { field: 'rank', order: 'desc' }],
+      limit: 5,
+    });
+    expect(lastOrderBy(statements)).toBe('"status" ASC, "rank" DESC, "id" DESC');
+    await driver.disconnect();
+  });
+
+  it('does not repeat a key the caller already sorted by', async () => {
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.find('ticket', { orderBy: [{ field: 'id', order: 'desc' }], limit: 5 });
+    expect(lastOrderBy(statements)).toBe('"id" DESC');
+    await driver.disconnect();
+  });
+
+  it('leaves findOne unsorted despite the `limit: 1` the transport injects itself', async () => {
+    // `RemoteTransport.findOne` spells an id lookup as `find(..., limit: 1)`.
+    // Read as a page that would earn `ORDER BY id LIMIT 1` — the shape that
+    // makes a planner drop the predicate's own index and walk the primary key
+    // (~100× on the measurement in `SqlDriver.findRows`). findOne promises *a*
+    // matching record, never a position in a sequence, so there is no partition
+    // to preserve and nothing to buy with that.
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.findOne('ticket', { where: { id: 't1' } });
+    const sql = statements[statements.length - 1].sql;
+    expect(sql).toContain('LIMIT ?');
+    expect(sql).not.toContain('ORDER BY');
+    await driver.disconnect();
+  });
+
+  it('still completes a findOne that asked for its OWN order — singleRowLookup withholds nothing there', async () => {
+    const { driver, statements } = await makeRecordingRemoteDriver();
+    await driver.findOne('ticket', {
+      where: { status: 'open' },
+      orderBy: [{ field: 'rank', order: 'asc' }],
+    });
+    // Row one of the table is "non-empty `orderBy` | either | caller's keys +
+    // id", and `either` means it — `singleRowLookup` only governs the row below
+    // it, where the caller named no key at all and the alternative would be to
+    // invent a whole sort for a lookup that never promised a position. A caller
+    // who DID name a key asked for a defined order among its ties, so the
+    // tie-breaker completes it here exactly as it does locally
+    // (`SqlDriver.findRows` reaches `orderKeysFor` with the same flag and the
+    // same non-empty key list, and appends the same column).
+    expect(lastOrderBy(statements)).toBe('"rank" ASC, "id" ASC');
+    await driver.disconnect();
+  });
+
+  it('invents no ordering column for a table this driver did not create', async () => {
+    // The precondition the whole rule rests on: `id` is known to exist because
+    // `RemoteTransport.buildCreateTableSQL` wrote it. On a table that arrived
+    // some other way, `ORDER BY id` risks failing the entire statement, so the
+    // conservative answer — prior behaviour exactly — is the right one.
+    const { driver, statements } = await makeRecordingRemoteDriver({ sync: false });
+    await driver.find('unmanaged_table', { limit: 5, offset: 5 });
+    expect(statements[statements.length - 1].sql).not.toContain('ORDER BY');
+    await driver.disconnect();
+  });
+
+  it('says so once when a paged unsorted read cannot be made deterministic', async () => {
+    // A MUST that quietly does not hold is the invisible failure the rule was
+    // written against, so the unserviceable case is announced rather than left
+    // to a user counting records. Once per object, not once per query.
+    const { driver } = await makeRecordingRemoteDriver({ sync: false });
+    const warn = vi.spyOn((driver as unknown as { logger: { warn: (m: string) => void } }).logger, 'warn');
+    await driver.find('unmanaged_table', { limit: 5 });
+    await driver.find('unmanaged_table', { limit: 5, offset: 5 });
+    const matching = warn.mock.calls.filter((c) => /NOT deterministic/.test(String(c[0])));
+    expect(matching).toHaveLength(1);
+    warn.mockRestore();
+    await driver.disconnect();
+  });
+});

packages/drivers/driver-turso/src/remote-transport.ts

@@ -941,6 +941,24 @@ export class RemoteTransport {
 
   /**
    * Build a SELECT SQL statement from a QueryAST-like object.
+   *
+   * **The ORDER BY below is rendered, not decided.** `query.orderBy` is already
+   * the COMPLETE sort key list by the time it gets here — caller's keys plus
+   * whatever the deterministic-paging contract requires
+   * (`IDataDriver.find` / objectstack#4363) — because `TursoDriver` resolves it
+   * through the inherited `SqlDriver.orderKeysFor` in `toRemoteReadQuery`
+   * before handing the query over. Reusing that one method is what stops this
+   * driver's two transports from giving the same paged query different ordering
+   * guarantees on nothing but a URL (#5653, ADR-0053 D-A1).
+   *
+   * So: do NOT grow a tie-breaker rule of your own in here. Besides being the
+   * second copy the fix was about, this method cannot see the distinction the
+   * rule turns on — {@link findOne} spells an id lookup as
+   * `find(object, { ...query, limit: 1 })`, which by this point is
+   * indistinguishable from page one of a walk with page size 1, and the two
+   * want opposite clauses (see `toRemoteReadQuery`). An empty or absent
+   * `orderBy` is likewise an ANSWER — #4363's carve-out for an unpaged
+   * unordered read — and emitting no ORDER BY for it is correct.
    */
   private buildSelectSQL(object: string, query: any): { sql: string; args: any[] } {
     const fields = query.fields && Array.isArray(query.fields) && query.fields.length > 0