Skip to content

Commit 4720f93

Browse files
committed
fix: repair pagination_cursors composite PK and isolate test suites
- Bump CURRENT_SCHEMA_VERSION to 6 and add migration 5→6 that detects when pagination_cursors was created with a single-column PK (command_key TEXT PRIMARY KEY) instead of the required composite PK (PRIMARY KEY (command_key, context)), then drops and recreates it. Data loss is safe since cursors are ephemeral (5-min TTL). - Add repairWrongPrimaryKeys() to repairSchema() so the auto-repair path also catches this for databases that bypass migrations. - Extend isSchemaError() to recognise the 'ON CONFLICT clause does not match' SQLite error so it triggers auto-repair instead of crashing. - Add wrong_primary_key SchemaIssue variant and surface it in getSchemaIssues() and sentry cli fix output. - Change the default `bun test` script from `bun test` (all files in one process) to `bun run test:unit && bun run test:isolated` (separate processes) so that mock.module() state from test/isolated/ does not leak into subsequent test files and cause ~130 spurious failures locally.
1 parent 995fd40 commit 4720f93

File tree

3 files changed

+115
-19
lines changed

3 files changed

+115
-19
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"typecheck": "tsc --noEmit",
1818
"lint": "bunx ultracite check",
1919
"lint:fix": "bunx ultracite fix",
20-
"test": "bun test",
20+
"test": "bun run test:unit && bun run test:isolated",
2121
"test:unit": "bun test test/lib test/commands test/types --coverage --coverage-reporter=lcov",
2222
"test:isolated": "bun test test/isolated",
2323
"test:e2e": "bun test test/e2e",

src/commands/cli/fix.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ function formatIssue(issue: SchemaIssue): string {
2424
if (issue.type === "missing_table") {
2525
return `Missing table: ${issue.table}`;
2626
}
27-
return `Missing column: ${issue.table}.${issue.column}`;
27+
if (issue.type === "missing_column") {
28+
return `Missing column: ${issue.table}.${issue.column}`;
29+
}
30+
return `Wrong primary key: ${issue.table}`;
2831
}
2932

3033
/** Expected permissions for the config directory (owner rwx) */

src/lib/db/schema.ts

Lines changed: 110 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import type { Database } from "bun:sqlite";
1515
import { stringifyUnknown } from "../errors.js";
1616

17-
export const CURRENT_SCHEMA_VERSION = 5;
17+
export const CURRENT_SCHEMA_VERSION = 6;
1818

1919
/** Environment variable to disable auto-repair */
2020
const NO_AUTO_REPAIR_ENV = "SENTRY_CLI_NO_AUTO_REPAIR";
@@ -330,6 +330,31 @@ export function hasColumn(
330330
return result.count > 0;
331331
}
332332

333+
/**
334+
* Check if a table has the expected composite primary key.
335+
*
336+
* Inspects the CREATE TABLE DDL stored in sqlite_master to verify
337+
* the table has a table-level PRIMARY KEY constraint matching the
338+
* expected columns. Returns false if the table uses per-column
339+
* PRIMARY KEY instead (e.g., `command_key TEXT PRIMARY KEY`).
340+
*/
341+
function hasCompositePrimaryKey(
342+
db: Database,
343+
table: string,
344+
expectedColumns: string[]
345+
): boolean {
346+
const row = db
347+
.query("SELECT sql FROM sqlite_master WHERE type='table' AND name=?")
348+
.get(table) as { sql: string } | undefined;
349+
350+
if (!row) {
351+
return false;
352+
}
353+
354+
const expectedPK = `PRIMARY KEY (${expectedColumns.join(", ")})`;
355+
return row.sql.includes(expectedPK);
356+
}
357+
333358
/** Add a column to a table if it doesn't exist */
334359
function addColumnIfMissing(
335360
db: Database,
@@ -345,7 +370,33 @@ function addColumnIfMissing(
345370
/** Schema issue types for diagnostics */
346371
export type SchemaIssue =
347372
| { type: "missing_table"; table: string }
348-
| { type: "missing_column"; table: string; column: string };
373+
| { type: "missing_column"; table: string; column: string }
374+
| { type: "wrong_primary_key"; table: string };
375+
376+
function findMissingColumns(db: Database, tableName: string): SchemaIssue[] {
377+
const columns = EXPECTED_COLUMNS[tableName];
378+
if (!columns) {
379+
return [];
380+
}
381+
return columns
382+
.filter((col) => !hasColumn(db, tableName, col.name))
383+
.map((col) => ({
384+
type: "missing_column" as const,
385+
table: tableName,
386+
column: col.name,
387+
}));
388+
}
389+
390+
function findPrimaryKeyIssues(db: Database, tableName: string): SchemaIssue[] {
391+
const schema = TABLE_SCHEMAS[tableName];
392+
if (
393+
schema?.compositePrimaryKey &&
394+
!hasCompositePrimaryKey(db, tableName, schema.compositePrimaryKey)
395+
) {
396+
return [{ type: "wrong_primary_key", table: tableName }];
397+
}
398+
return [];
399+
}
349400

350401
/**
351402
* Check schema and return list of issues.
@@ -360,18 +411,8 @@ export function getSchemaIssues(db: Database): SchemaIssue[] {
360411
continue;
361412
}
362413

363-
const columns = EXPECTED_COLUMNS[tableName];
364-
if (columns) {
365-
for (const col of columns) {
366-
if (!hasColumn(db, tableName, col.name)) {
367-
issues.push({
368-
type: "missing_column",
369-
table: tableName,
370-
column: col.name,
371-
});
372-
}
373-
}
374-
}
414+
issues.push(...findMissingColumns(db, tableName));
415+
issues.push(...findPrimaryKeyIssues(db, tableName));
375416
}
376417

377418
return issues;
@@ -421,8 +462,42 @@ function repairMissingColumns(db: Database, result: RepairResult): void {
421462
}
422463

423464
/**
424-
* Repair schema issues by creating missing tables and adding missing columns.
425-
* This is a non-destructive operation that only adds missing schema elements.
465+
* Drop and recreate tables that have incorrect primary key constraints.
466+
*
467+
* This fixes the CLI-72 bug where pagination_cursors was created with a
468+
* single-column PK (`command_key TEXT PRIMARY KEY`) instead of the expected
469+
* composite PK (`PRIMARY KEY (command_key, context)`). SQLite does not
470+
* support ALTER TABLE to change primary keys, so the table must be dropped
471+
* and recreated. The data loss is acceptable since pagination cursors are
472+
* ephemeral (5-minute TTL).
473+
*/
474+
function repairWrongPrimaryKeys(db: Database, result: RepairResult): void {
475+
for (const [tableName, schema] of Object.entries(TABLE_SCHEMAS)) {
476+
if (!schema.compositePrimaryKey) {
477+
continue;
478+
}
479+
if (!tableExists(db, tableName)) {
480+
continue;
481+
}
482+
if (hasCompositePrimaryKey(db, tableName, schema.compositePrimaryKey)) {
483+
continue;
484+
}
485+
try {
486+
db.exec(`DROP TABLE ${tableName}`);
487+
db.exec(EXPECTED_TABLES[tableName] as string);
488+
result.fixed.push(
489+
`Recreated table ${tableName} with correct primary key`
490+
);
491+
} catch (e) {
492+
const msg = stringifyUnknown(e);
493+
result.failed.push(`Failed to recreate table ${tableName}: ${msg}`);
494+
}
495+
}
496+
}
497+
498+
/**
499+
* Repair schema issues by creating missing tables, adding missing columns,
500+
* and recreating tables with incorrect primary key constraints.
426501
*
427502
* @param db - The raw database connection (not the traced wrapper)
428503
* @returns Lists of fixed and failed repairs
@@ -432,6 +507,7 @@ export function repairSchema(db: Database): RepairResult {
432507

433508
repairMissingTables(db, result);
434509
repairMissingColumns(db, result);
510+
repairWrongPrimaryKeys(db, result);
435511

436512
if (result.fixed.length > 0) {
437513
try {
@@ -458,7 +534,8 @@ function isSchemaError(error: unknown): boolean {
458534
return (
459535
msg.includes("no such column") ||
460536
msg.includes("no such table") ||
461-
msg.includes("has no column named")
537+
msg.includes("has no column named") ||
538+
msg.includes("on conflict clause does not match")
462539
);
463540
}
464541
return false;
@@ -615,6 +692,22 @@ export function runMigrations(db: Database): void {
615692
db.exec(EXPECTED_TABLES.pagination_cursors as string);
616693
}
617694

695+
// Migration 5 -> 6: Repair pagination_cursors if created with wrong PK (CLI-72)
696+
// Earlier versions could create the table with a single-column PK instead of
697+
// the composite PK (command_key, context). DROP + CREATE is safe because
698+
// pagination cursors are ephemeral (5-minute TTL).
699+
if (
700+
currentVersion < 6 &&
701+
tableExists(db, "pagination_cursors") &&
702+
!hasCompositePrimaryKey(db, "pagination_cursors", [
703+
"command_key",
704+
"context",
705+
])
706+
) {
707+
db.exec("DROP TABLE pagination_cursors");
708+
db.exec(EXPECTED_TABLES.pagination_cursors as string);
709+
}
710+
618711
if (currentVersion < CURRENT_SCHEMA_VERSION) {
619712
db.query("UPDATE schema_version SET version = ?").run(
620713
CURRENT_SCHEMA_VERSION

0 commit comments

Comments
 (0)