fix: address code review findings — readonly bug, transaction wrapping, tests

Fixes from 6-model consensus code review:

1. **CRITICAL** — SQLite driver `readonly` + `create: true` bug: gate `create`
   and WAL pragma on `!readonly` so readonly connections don't silently open
   read-write or crash on PRAGMA WAL.

2. **MAJOR** — Wrap `indexWarehouse` inserts in `db.transaction()` per-table
   to avoid per-statement disk fsyncs (~200x slowdown for large warehouses).

3. **MAJOR** — Fix no-op parent directory test (was creating dir before testing).
   Add 3 readonly connection tests (read existing, reject writes, refuse create).

4. **MINOR** — Extend `idx_columns_table` covering index to include
   `column_name` for `listColumns()` ORDER BY.

Author: anandgupta42

packages/drivers/src/sqlite.ts

@@ -12,11 +12,14 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
 
   return {
     async connect() {
+      const isReadonly = config.readonly === true
       db = new Database(dbPath, {
-        readonly: config.readonly === true,
-        create: true,
+        readonly: isReadonly,
+        create: !isReadonly,
       })
-      db.exec("PRAGMA journal_mode = WAL")
+      if (!isReadonly) {
+        db.exec("PRAGMA journal_mode = WAL")
+      }
     },
 
     async execute(sql: string, limit?: number, _binds?: any[]): Promise<ConnectorResult> {

packages/opencode/src/altimate/native/schema/cache.ts

@@ -66,7 +66,7 @@ CREATE INDEX IF NOT EXISTS idx_tables_search ON tables_cache(search_text);
 CREATE INDEX IF NOT EXISTS idx_columns_search ON columns_cache(search_text);
 CREATE INDEX IF NOT EXISTS idx_tables_warehouse ON tables_cache(warehouse);
 CREATE INDEX IF NOT EXISTS idx_columns_warehouse ON columns_cache(warehouse);
-CREATE INDEX IF NOT EXISTS idx_columns_table ON columns_cache(warehouse, schema_name, table_name);
+CREATE INDEX IF NOT EXISTS idx_columns_table ON columns_cache(warehouse, schema_name, table_name, column_name);
 `
 
 // ---------------------------------------------------------------------------
@@ -177,6 +177,18 @@ export class SchemaCache {
        VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
     )
 
+    // Batch inserts per-table inside a transaction to avoid per-statement disk fsyncs.
+    // The async connector calls (listTables, describeTable) run outside the transaction;
+    // only the synchronous SQLite inserts are wrapped.
+    const insertTableBatch = this.db.transaction(
+      (tableArgs: any[], columnArgsBatch: any[][]) => {
+        insertTable.run(...tableArgs)
+        for (const colArgs of columnArgsBatch) {
+          insertColumn.run(...colArgs)
+        }
+      },
+    )
+
     for (const schemaName of schemas) {
       if (schemaName.toUpperCase() === "INFORMATION_SCHEMA") continue
       totalSchemas++
@@ -191,27 +203,32 @@ export class SchemaCache {
       for (const tableInfo of tables) {
         totalTables++
         const searchText = makeSearchText(databaseName, schemaName, tableInfo.name, tableInfo.type)
-        insertTable.run(
-          warehouseName, databaseName, schemaName, tableInfo.name, tableInfo.type, searchText,
-        )
 
         let columns: Array<{ name: string; data_type: string; nullable: boolean }> = []
         try {
           columns = await connector.describeTable(schemaName, tableInfo.name)
         } catch {
-          continue
+          // continue with empty columns
         }
 
+        // Build column insert args
+        const columnArgsBatch: any[][] = []
         for (const col of columns) {
           totalColumns++
           const colSearch = makeSearchText(
             databaseName, schemaName, tableInfo.name, col.name, col.data_type,
           )
-          insertColumn.run(
+          columnArgsBatch.push([
             warehouseName, databaseName, schemaName, tableInfo.name,
             col.name, col.data_type, col.nullable ? 1 : 0, colSearch,
-          )
+          ])
         }
+
+        // Insert table + all its columns in a single transaction
+        insertTableBatch(
+          [warehouseName, databaseName, schemaName, tableInfo.name, tableInfo.type, searchText],
+          columnArgsBatch,
+        )
       }
     }
 

packages/opencode/test/altimate/schema-cache.test.ts

@@ -605,7 +605,7 @@ describe("file-based cache persistence", () => {
     cache2.close()
   })
 
-  test("creates parent directory if it doesn't exist", () => {
+  test("opens existing DB file at a custom path", () => {
     const nestedPath = join(tmpDir, "deep", "nested", "dir", "cache.db")
     mkdirSync(join(tmpDir, "deep", "nested", "dir"), { recursive: true })
     const cache = SchemaCache.create(nestedPath)
@@ -680,3 +680,55 @@ describe("SQLite driver PRAGMA handling", () => {
     await connector.close()
   })
 })
+
+// ---------------------------------------------------------------------------
+// 13. SQLite driver — readonly connection handling
+// ---------------------------------------------------------------------------
+
+describe("SQLite driver readonly connections", () => {
+  test("readonly connection can read existing database", async () => {
+    const { connect } = await import("@altimateai/drivers/sqlite")
+    const dbPath = join(tmpDir, "readonly-test.db")
+
+    // Create a database with data first
+    const writer = await connect({ type: "sqlite", path: dbPath })
+    await writer.connect()
+    await writer.execute("CREATE TABLE items (id INTEGER, name TEXT)")
+    await writer.execute("INSERT INTO items VALUES (1, 'test')")
+    await writer.close()
+
+    // Open readonly and verify reads work
+    const reader = await connect({ type: "sqlite", path: dbPath, readonly: true })
+    await reader.connect()
+    const result = await reader.execute("SELECT * FROM items")
+    expect(result.rows).toEqual([[1, "test"]])
+    await reader.close()
+  })
+
+  test("readonly connection rejects writes", async () => {
+    const { connect } = await import("@altimateai/drivers/sqlite")
+    const dbPath = join(tmpDir, "readonly-write-test.db")
+
+    // Create a database first
+    const writer = await connect({ type: "sqlite", path: dbPath })
+    await writer.connect()
+    await writer.execute("CREATE TABLE items (id INTEGER)")
+    await writer.close()
+
+    // Open readonly and verify writes fail
+    const reader = await connect({ type: "sqlite", path: dbPath, readonly: true })
+    await reader.connect()
+    expect(() => reader.execute("INSERT INTO items VALUES (1)")).toThrow()
+    await reader.close()
+  })
+
+  test("readonly connection does not create nonexistent file", async () => {
+    const { connect } = await import("@altimateai/drivers/sqlite")
+    const dbPath = join(tmpDir, "ghost-file.db")
+
+    const reader = await connect({ type: "sqlite", path: dbPath, readonly: true })
+    // Should throw because the file doesn't exist and create=false
+    expect(() => reader.connect()).toThrow()
+    expect(existsSync(dbPath)).toBe(false)
+  })
+})