Skip to content

Commit 19fc8d6

Browse files
fix(driver-turso): a declared index the remote face skips for a missing key column is logged at error on the durability sink (#20650)
Fixes #20537 Clause-②: no The remote Turso face now reports a declared index it skips, because a key column never materializes, at `error` on its existing durability sink. It used to report it at `warn` on the diagnostic sink. This is the remote half of what PR #20519 did for the local face. ## What was wrong (read on `cd901d7a5`) `RemoteTransport.buildDeclaredIndexDDL` (`packages/drivers/driver-turso/src/remote-transport.ts`) plans no DDL for a declared index whose key column is not a stored column. That covers a misspelt name (`statsu` on a table whose column is `status`) and a virtual `formula` field. Skipping it is right, because DDL naming a missing column would fail the whole sync. But the skip was reported through `this.diagnosticSink`, which `TursoDriver` wires to `logger.warn`: [RemoteTransport] skipping declared index "NAME" on "TABLE" — column(s) not materialized: statsu For a UNIQUE index, every write keeps succeeding and duplicates are accepted, and the only trace was a `warn`. The same transport already has a second sink for this class, `durabilitySink` (wired to `logger.error`). Its retrofit arm uses it for a unique AND a plain index it could not create. The local face (`SqlDriver.syncDeclaredIndexes`) logs this same skip through `logDurabilityFailure` for unique and plain indexes alike. ## What changed - **`remote-transport.ts`, `buildDeclaredIndexDDL`.** The not-materialised arm now calls `this.durabilitySink?.(…)`, not `this.diagnosticSink?.(…)`. That is triage's direction: no new sink and no second rule. It writes one line per skipped index per sync, as the local face does. The line gives the index name, the object and each missing column, and says whether the index was UNIQUE. It follows the AGENTS.md "Degradation log levels" shape: - the consequence: for UNIQUE, "the uniqueness it declares is NOT enforced: duplicate rows are accepted, and nothing looks broken from the outside"; for plain, every query the index serves is a full scan while results stay correct; - the fix: make every key column a stored field of the object, or remove the index. - The doc comments on `durabilitySink` and `buildDeclaredIndexDDL` now name this arm. - `turso-driver.ts` is **not** edited. Both sinks were already wired there. - No DDL, accept set or refusal changes. The same indexes are created and the same ones are skipped. - Changeset: `.changeset/20537-remote-skipped-index-durability.md`, `@objectstack/driver-turso` **patch**. ## The dispatch's hypotheses, measured - **H1 held**, with one correction. The skip is in `buildDeclaredIndexDDL`, the planner, not the sync itself, and all four sync paths call it: `syncSchema` (new and existing table) and `syncSchemasBatch` (new and existing table). One edit covers all four, and the pins drive all four. - **H2 held.** `turso-driver.ts` has `setDiagnosticSink` going to `this.logger.warn` and `setDurabilitySink` going to `(this.logger.error ?? this.logger.warn)`, at `:1656` and `:1667` on this base. No edit was needed. - **H3 held.** The local text is in `SqlDriver.syncDeclaredIndexes`, through `logDurabilityFailure`, for unique and plain alike. The remote line uses the same consequence-then-fix order. - **H4: the plain index goes on the durability sink too.** There are two pieces of evidence: - the local face routes the plain skip through `logDurabilityFailure`, and its doc says "A plain index is DDL that was supposed to run and did not, so it takes the same channel"; - the remote retrofit arm already reports a plain index it could not create on `durabilitySink`, with the #17609 rationale: queries answer correctly by scanning, nothing looks wrong, and the cost arrives as read volume. - **H5: `check:durability-log-level` does not see this site, and no vocabulary entry belongs there.** Its header says it judges `try`/`catch` blocks whose `try` calls a declared durability-critical operation. This arm has no `catch` and runs no operation: it plans no DDL. It reports through a sink receiver, which `LOGGER_RECEIVERS` does not cover by a recorded decision. So no `DURABILITY_CRITICAL_CALLEES` entry can make it visible. The gate reads 38 seams, all loud, on this head (exit 0). ## Tests `packages/drivers/driver-turso/src/remote-transport-unbuildable-declared-index.test.ts` has 18 cases. They use the real `@libsql/client` over `file::memory:`, as the declared-index parity suite does: - **The card's matrix (16 cases).** {misspelt `statsu`, `formula` column} × {UNIQUE, plain}, each over `syncSchema` and `syncSchemasBatch`, against a new table and an existing one. Each case asserts: - exactly one durability-sink line names the index, with the table and the column quoted; - `UNIQUE` is present for the unique cells and absent for the plain ones; - no diagnostic-sink line names the column or the index; - the sync resolves; - the index is absent from `sqlite_master`. - **End to end (2 cases).** `TursoDriver` in remote mode (`libsql://…` with a supplied client), through `initObjects`. The line lands on `logger.error` and never on `logger.warn`. The prose is not pinned beyond the named subjects and the `UNIQUE` word, as in the local #20432 suite. Commands, run on `90e8f132f`: - `pnpm --filter @objectstack/driver-turso exec vitest run --maxWorkers=2 src/remote-transport-unbuildable-declared-index.test.ts`: 1 file, **18 passed**. - `pnpm --filter @objectstack/driver-turso exec vitest run --maxWorkers=2` (whole package): **79 files passed, 2126 passed, 22 skipped**, verdict `command-exit 0`. - `pnpm --filter @objectstack/driver-turso typecheck`: `command-exit 0`. The package `tsconfig` includes `src/**/*`, and `tsc --noEmit --listFiles` lists the new test file once. **Reverse verification (ablation).** The fix was committed first. The mutation went through `scripts/ablation-replace.mjs` in WRAP mode, with its own trap and restore. It turned the new arm's `this.durabilitySink?.(` back into `this.diagnosticSink?.(`, and nothing else. - **Landed on disk:** anchor count went 1 to 0, and `durabilitySink?.(` / `diagnosticSink?.(` counts went 2/3 to 1/4. The blob went `1c7cb45f2828` to `672bd32cd8c6`. - **Result:** predicted all 18 red. Observed **18 failed of 18**, on a comparison (`expected [] to have a length of 1 but got +0`). - **Restored:** blob equals HEAD (`1c7cb45f2828`), `git diff HEAD` is empty, and the tree is clean. - **No `dist` step:** the suite imports the subject by a relative path (`./remote-transport.js`, `./turso-driver.js`), so vitest reads `src`. ## Gates - **Derived gates.** Derived after the final commit with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` at `90e8f132f`: 61 commands. The dispatch-time list had 48; the 13 added come from the changeset and test-file families. All 61 ran and exited 0. `--ran` with the exit codes recorded reads: "61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero)". - **Three gates needed a full build first.** `check:dual-build-cjs-loads`, `check:lean-entry-closure` and `check:type-check-debt` first exited 3 (`PREREQUISITE NOT MET`, not measured). After a full build (`turbo run build --filter='./packages/*' --filter='./packages/*/*'`, 71 of 71 tasks, with the tree clean afterwards), each exited 0. - **`pnpm check:driver-conformance`, before and after:** "50 covered cell(s), 0 in the DEBT ledger, 0 exempt" both times. - **`pnpm check:durability-log-level`:** not in the derived set; run for H5. Exit 0. - **Lint**, narrowed to the two touched `.ts` files with `eslint --no-inline-config --format json`: 2 files, 0 errors, 0 warnings. - Both files are matched by the config and neither is ignored. - `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project`), so this diff cannot move the verdict on any file it does not touch. - The full `pnpm lint` run is CI's. ## Acceptance notes - **The reason for a missing column is not given per column.** The local line says, per column, why it has no column ("not a field of the object" or "a formula field"), through `describeMissingIndexColumns`. That helper is exported from `schema-drift.ts` but not from `@objectstack/driver-sql`'s package entry. Reusing it would widen that package's public surface, outside this card's claimed files. A copy here would be the second copy this file's shared-normalizer imports exist to prevent. So the remote line names the missing columns and gives both possible reasons in one clause. Carrier: none. - **With no durability sink set, the skip is not logged at all.** Before, it went to the diagnostic sink. `TursoDriver` wires both sinks together at construction, so no composition in this repo changes. The retrofit arm already works this way, and the `durabilitySink` doc says what still surfaces a missing UNIQUE (the enveloped `conflictKeys` refusal). - **Remote drift detection is not in this card.** The remote face refuses it by design, so `os migrate plan` still cannot show the skipped index on a remote datasource. Only the log line changes here. - **The branch is two commits behind `main`** (`89801cd96`, `0cb72cfc7`: service-automation and spec migration text). Neither touches `packages/drivers`, so no merge was made. CI validates the merge ref. --- _Generated by [Claude Code](https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9b402db commit 19fc8d6

3 files changed

Lines changed: 268 additions & 8 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/driver-turso': patch
3+
---
4+
5+
fix(driver-turso): a declared index the remote face skips because a key column never materializes is logged at `error`, not `warn`
6+
7+
Clause-②: no
8+
9+
In remote mode (a `libsql://` URL), schema sync skips a declared index whose key column is not
10+
a stored column: a name that is not a field of the object (a misspelling), or a virtual
11+
`formula` field, which is computed on read and has no column. The skip itself is unchanged, since
12+
DDL naming a missing column would fail the whole sync. It used to be reported through the
13+
driver's `warn` diagnostics, so a skipped UNIQUE index left duplicates accepted while the log
14+
said `warn`.
15+
16+
The skip is now logged at `error`, on the same channel the remote face already uses for a
17+
declared index it could not create, and the local face uses for the same skip. There is one
18+
line per skipped index per sync. It names the object, the index and the missing column, says
19+
whether the index was UNIQUE, states what is not enforced (duplicates for a UNIQUE index, a full
20+
table scan for a plain one), and says how to fix it: make every key column a stored field of
21+
the object, or remove the index.
22+
23+
No DDL, accept set or refusal changes: the same indexes are created and the same ones are
24+
skipped.
Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#20537] The REMOTE face's half of #20432: a declared index the remote
5+
* transport skips, because a key column never materializes, is reported at
6+
* `error` on the durability sink.
7+
*
8+
* ## The defect this pins
9+
*
10+
* `RemoteTransport.buildDeclaredIndexDDL` plans no DDL for a declared index
11+
* whose key column is not a stored column: a misspelt name (`statsu` on a
12+
* table whose column is `status`), or a virtual `formula` field. That skip is
13+
* right, since DDL naming a column that does not exist would fail the whole
14+
* sync. But it was reported through the DIAGNOSTIC sink, which `TursoDriver`
15+
* wires to `logger.warn`. For a UNIQUE index that is the AGENTS.md
16+
* durability-degradation shape: every write keeps succeeding, duplicates are
17+
* accepted, and the only trace was a `warn`. The same transport already has a
18+
* second, durability sink (wired to `logger.error`) for exactly this class,
19+
* and its retrofit arm already reports a unique AND a plain index it could not
20+
* create there. The local face (`SqlDriver.syncDeclaredIndexes`) logs the same
21+
* skip through `logDurabilityFailure`, for unique and plain alike.
22+
*
23+
* ## What is asserted
24+
*
25+
* The CHANNEL (durability sink, never the diagnostic one), the number of lines
26+
* (one per skipped index per sync), the named subjects (table, index name,
27+
* missing column), and the one word that says which kind of index it is
28+
* (`UNIQUE`, or its absence). The prose is not pinned beyond that. The
29+
* consequence the line reports is also checked against the database: the
30+
* index really is absent.
31+
*
32+
* The matrix is {misspelt, formula} x {unique, plain}, as the card asks, over
33+
* all four ways a sync reaches the skip: `syncSchema` and `syncSchemasBatch`,
34+
* each against a new table and against one that already exists (the retrofit
35+
* leg). The four share one builder, and a card that fixes one call site is
36+
* the shape AGENTS.md Prime Directive #10 warns about.
37+
*
38+
* The client is the real `@libsql/client` over `file::memory:`, as the
39+
* declared-index parity suite uses, so "the index is absent" is read off
40+
* `sqlite_master` rather than inferred from a string. The last `describe`
41+
* drives the whole `TursoDriver` in remote mode and reads the LEVEL, because
42+
* the sink-to-level wiring lives in `turso-driver.ts`.
43+
*/
44+
45+
import { describe, it, expect, afterEach, vi } from 'vitest';
46+
import { createClient, type Client } from '@libsql/client';
47+
import { buildIndexName } from '@objectstack/driver-sql';
48+
import { RemoteTransport } from './remote-transport.js';
49+
import { TursoDriver } from './turso-driver.js';
50+
51+
type ObjectDef = {
52+
name: string;
53+
fields: Record<string, any>;
54+
indexes?: Array<{ fields: string[]; unique?: boolean }>;
55+
};
56+
57+
interface Cell {
58+
label: string;
59+
table: string;
60+
fields: Record<string, any>;
61+
/** The key column that never materializes. */
62+
column: string;
63+
unique: boolean;
64+
}
65+
66+
const CELLS: Cell[] = [
67+
{
68+
label: 'a misspelt key column, UNIQUE index',
69+
table: 'os20537_misspelt_unique',
70+
fields: { status: { type: 'text', maxLength: 64 } },
71+
column: 'statsu',
72+
unique: true,
73+
},
74+
{
75+
label: 'a misspelt key column, plain index',
76+
table: 'os20537_misspelt_plain',
77+
fields: { status: { type: 'text', maxLength: 64 } },
78+
column: 'statsu',
79+
unique: false,
80+
},
81+
{
82+
label: 'a formula key column, UNIQUE index',
83+
table: 'os20537_formula_unique',
84+
fields: { amount: { type: 'number' }, doubled: { type: 'formula', expression: 'amount * 2' } },
85+
column: 'doubled',
86+
unique: true,
87+
},
88+
{
89+
label: 'a formula key column, plain index',
90+
table: 'os20537_formula_plain',
91+
fields: { amount: { type: 'number' }, doubled: { type: 'formula', expression: 'amount * 2' } },
92+
column: 'doubled',
93+
unique: false,
94+
},
95+
];
96+
97+
const withIndex = (cell: Cell): ObjectDef => ({
98+
name: cell.table,
99+
fields: Object.fromEntries(Object.entries(cell.fields).map(([k, v]) => [k, { ...v }])),
100+
indexes: [{ fields: [cell.column], ...(cell.unique ? { unique: true } : {}) }],
101+
});
102+
103+
const withoutIndex = (cell: Cell): ObjectDef => {
104+
const { indexes: _dropped, ...rest } = withIndex(cell);
105+
return rest;
106+
};
107+
108+
type SyncPath = 'syncSchema' | 'syncSchemasBatch';
109+
const PATHS: SyncPath[] = ['syncSchema', 'syncSchemasBatch'];
110+
type TableState = 'new table' | 'existing table';
111+
const STATES: TableState[] = ['new table', 'existing table'];
112+
113+
const cleanups: Array<() => Promise<void> | void> = [];
114+
afterEach(async () => {
115+
for (const cleanup of cleanups.splice(0).reverse()) await cleanup();
116+
vi.restoreAllMocks();
117+
});
118+
119+
/** A transport over an in-memory libsql database, with both sinks captured. */
120+
function transport(): { t: RemoteTransport; client: Client; durability: string[]; diagnostic: string[] } {
121+
const client = createClient({ url: 'file::memory:' });
122+
cleanups.push(() => client.close());
123+
const t = new RemoteTransport();
124+
t.setClient(client);
125+
const durability: string[] = [];
126+
const diagnostic: string[] = [];
127+
t.setDurabilitySink((m) => durability.push(m));
128+
t.setDiagnosticSink((m) => diagnostic.push(m));
129+
return { t, client, durability, diagnostic };
130+
}
131+
132+
const sync = (t: RemoteTransport, path: SyncPath, def: ObjectDef): Promise<void> =>
133+
path === 'syncSchema'
134+
? t.syncSchema(def.name, def)
135+
: t.syncSchemasBatch([{ object: def.name, schema: def }]);
136+
137+
const indexNames = async (client: Client, table: string): Promise<string[]> =>
138+
(await client.execute({ sql: `SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`, args: [table] }))
139+
.rows.map((r) => String(r.name));
140+
141+
const tableExists = async (client: Client, table: string): Promise<boolean> =>
142+
(await client.execute({ sql: `SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?`, args: [table] }))
143+
.rows.length === 1;
144+
145+
describe('[#20537] a declared index the remote face skips is reported on the durability sink', () => {
146+
for (const cell of CELLS) {
147+
const INDEX = buildIndexName(cell.table, [cell.column], cell.unique);
148+
149+
describe(cell.label, () => {
150+
for (const path of PATHS) {
151+
for (const state of STATES) {
152+
it(`${path}, ${state}: one durability line naming the index, and no diagnostic line`, async () => {
153+
const { t, client, durability, diagnostic } = transport();
154+
if (state === 'existing table') {
155+
await sync(t, path, withoutIndex(cell));
156+
expect(await tableExists(client, cell.table)).toBe(true);
157+
durability.length = 0;
158+
diagnostic.length = 0;
159+
}
160+
161+
// The sync goes on: the skip never fails it.
162+
await expect(sync(t, path, withIndex(cell))).resolves.toBeUndefined();
163+
expect(await tableExists(client, cell.table)).toBe(true);
164+
165+
const lines = durability.filter((m) => m.includes(`"${INDEX}"`));
166+
expect(lines).toHaveLength(1);
167+
expect(lines[0]).toContain(`"${cell.table}"`);
168+
expect(lines[0]).toContain(`'${cell.column}'`);
169+
if (cell.unique) expect(lines[0]).toContain('UNIQUE');
170+
else expect(lines[0]).not.toContain('UNIQUE');
171+
// Nothing else reached the durability sink for this object.
172+
expect(durability).toEqual(lines);
173+
174+
// The level MOVED; it did not gain a second line. Before the fix this
175+
// skip reached ONLY the diagnostic sink, naming the column.
176+
expect(diagnostic.filter((m) => m.includes(cell.column) || m.includes(INDEX))).toEqual([]);
177+
178+
// The consequence the line reports is real: nothing was built.
179+
expect(await indexNames(client, cell.table)).not.toContain(INDEX);
180+
});
181+
}
182+
}
183+
});
184+
}
185+
});
186+
187+
describe('[#20537] through TursoDriver in remote mode, the skip lands on logger.error, not logger.warn', () => {
188+
for (const cell of CELLS.filter((c) => c.column === 'statsu')) {
189+
it(cell.label, async () => {
190+
const client = createClient({ url: 'file::memory:' });
191+
const driver = new TursoDriver({ url: 'libsql://unbuildable-declared-index.turso.io', client });
192+
await driver.connect();
193+
cleanups.push(() => driver.disconnect());
194+
expect(driver.transportMode).toBe('remote');
195+
const logger = (driver as unknown as { logger: { warn: (m: string) => void; error: (m: string) => void } })
196+
.logger;
197+
const error = vi.spyOn(logger, 'error').mockImplementation(() => undefined);
198+
const warn = vi.spyOn(logger, 'warn').mockImplementation(() => undefined);
199+
const INDEX = buildIndexName(cell.table, [cell.column], cell.unique);
200+
201+
await expect(driver.initObjects([withIndex(cell)] as never)).resolves.toBeUndefined();
202+
203+
const errors = error.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(`"${INDEX}"`));
204+
expect(errors).toHaveLength(1);
205+
expect(errors[0]).toContain(`'${cell.column}'`);
206+
expect(warn.mock.calls.some((c) => String(c[0]).includes(cell.column))).toBe(false);
207+
expect(await indexNames(client, cell.table)).not.toContain(INDEX);
208+
});
209+
}
210+
});

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

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,13 @@ export class RemoteTransport {
12481248
* each one scans the table — nothing is visibly wrong, and the cost arrives
12491249
* as read volume.
12501250
*
1251+
* [#20537] A declared index SKIPPED because a key column never materialized
1252+
* (a misspelt name, a virtual `formula` field) is the same class again, one
1253+
* step earlier: {@link buildDeclaredIndexDDL} plans no DDL for it at all, so
1254+
* the constraint (or the access path) is missing for the same reason and with
1255+
* the same outward look. The local face logs that skip through
1256+
* `SqlDriver.logDurabilityFailure`, so this face reports it here.
1257+
*
12511258
* Absent, the degradation is not lost: {@link retrofitDeclaredIndexes} still
12521259
* skips the index, and a missing UNIQUE resurfaces as the enveloped refusal
12531260
* in {@link upsert}. `TursoDriver` wires this to `logger.error` at construction,
@@ -2425,11 +2432,13 @@ export class RemoteTransport {
24252432
* (`COALESCE(<tenant>, '__global__')`) from the shared helper for the reason
24262433
* ADR-0120 D3 records. Neither rule is re-decided here.
24272434
*
2428-
* Columns that were never materialized (a virtual `formula` field) are
2429-
* skipped rather than emitted — the same choice `SqlDriver.syncDeclaredIndexes`
2430-
* makes, and for the same reason: DDL naming a column that does not exist
2431-
* fails the whole sync over an index nothing could have used. A name declared
2432-
* twice is emitted once, as the local face creates it once.
2435+
* Columns that were never materialized (a misspelt key name, a virtual
2436+
* `formula` field) are skipped rather than emitted — the same choice
2437+
* `SqlDriver.syncDeclaredIndexes` makes, and for the same reason: DDL naming a
2438+
* column that does not exist fails the whole sync over an index nothing could
2439+
* have used. [#20537] The skip is reported where the local face reports it,
2440+
* at `error`: on {@link durabilitySink}, not the diagnostic sink. A name
2441+
* declared twice is emitted once, as the local face creates it once.
24332442
*/
24342443
private buildDeclaredIndexDDL(
24352444
tableName: string,
@@ -2453,9 +2462,26 @@ export class RemoteTransport {
24532462
for (const index of expected) {
24542463
const missing = index.columns.filter((c) => !materializedColumns.has(c));
24552464
if (missing.length > 0) {
2456-
this.diagnosticSink?.(
2457-
`[RemoteTransport] skipping declared index "${index.name}" on "${tableName}" — ` +
2458-
`column(s) not materialized: ${missing.join(', ')}`,
2465+
// [#20537] Durability, not function — the durability sink, never the
2466+
// diagnostic one. The sync goes on and the object serves normally while
2467+
// DDL the metadata declares never runs; for a UNIQUE index that means
2468+
// duplicate rows are accepted. `SqlDriver.syncDeclaredIndexes` answers
2469+
// the same skip on the local face through `logDurabilityFailure`, for
2470+
// the same reason, and a PLAIN index takes the same channel on both
2471+
// faces (see {@link durabilitySink}). One line per skipped index per
2472+
// sync, and the line says which of the two kinds it is.
2473+
const columns = missing.map((c) => `'${c}'`).join(', ');
2474+
this.durabilitySink?.(
2475+
`[RemoteTransport] declared ${index.unique ? 'UNIQUE ' : ''}index "${index.name}" on "${tableName}" ` +
2476+
`was NOT created: no column for ${columns} (a key column must be a stored field of the object — a ` +
2477+
`name that is not a field, or a virtual formula field computed on read, has no column). ` +
2478+
(index.unique
2479+
? `The uniqueness it declares is NOT enforced: duplicate rows are accepted, and nothing looks ` +
2480+
`broken from the outside. `
2481+
: `Every query this index exists to serve is answered by scanning the whole table instead: ` +
2482+
`nothing looks broken from the outside and results stay correct. `) +
2483+
`Fix the metadata so every column in the index's fields is a stored field of the object, or ` +
2484+
`remove the index ("os validate" refuses a name that is not a field).`,
24592485
);
24602486
continue;
24612487
}

0 commit comments

Comments
 (0)