From 18635e7c7d79d2b7553751ecda9b3cbebc01ab9b Mon Sep 17 00:00:00 2001 From: gavrielc Date: Wed, 6 May 2026 21:12:25 +0300 Subject: [PATCH] fix(scripts/q): use stmt.reader instead of keyword sniffing for SELECT detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first-keyword check (`WITH` → SELECT path) was wrong for CTEs that precede mutations (e.g. `WITH stale AS (...) DELETE FROM t WHERE ...`). These would be routed through `db.prepare().all()` instead of executing the mutation. Use better-sqlite3's `stmt.reader` property, which asks SQLite's own parser whether the statement returns data. Single mutations go through `stmt.run()`; compound statements (which `prepare()` rejects) fall back to `db.exec()`. Add a regression test for WITH...DELETE. Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/q.test.ts | 11 +++++++++++ scripts/q.ts | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/scripts/q.test.ts b/scripts/q.test.ts index 4685e2be9..4901db5d1 100644 --- a/scripts/q.test.ts +++ b/scripts/q.test.ts @@ -84,6 +84,17 @@ describe('scripts/q.ts', () => { expect(ids).toEqual([2, 9]); }); + it('WITH...DELETE is treated as a mutation, not a query', () => { + const r = run("WITH stale AS (SELECT id FROM t WHERE name = 'alice') DELETE FROM t WHERE id IN (SELECT id FROM stale)"); + expect(r.status).toBe(0); + expect(r.stdout).toBe(''); + + const db = new Database(dbPath, { readonly: true }); + const rows = db.prepare('SELECT name FROM t').all() as { name: string }[]; + db.close(); + expect(rows).toEqual([{ name: 'bob' }]); + }); + it('exits 2 with usage when args are missing', () => { const r = spawnSync('pnpm', ['exec', 'tsx', Q], { encoding: 'utf-8', diff --git a/scripts/q.ts b/scripts/q.ts index 71a467603..3d1ba7494 100644 --- a/scripts/q.ts +++ b/scripts/q.ts @@ -4,10 +4,11 @@ * Usage: * pnpm exec tsx scripts/q.ts "" * - * Detects SELECT vs mutation on the first keyword. SELECT/WITH queries - * print rows in sqlite3 CLI default ("list") format — pipe-separated, - * no header — so existing skill text reads identically. Anything else - * runs through db.exec() and prints nothing on success. + * Uses better-sqlite3's stmt.reader property to distinguish queries + * (SELECT / WITH...SELECT) from mutations. Queries print rows in + * sqlite3 CLI default ("list") format — pipe-separated, no header — + * so existing skill text reads identically. Mutations run via + * stmt.run() (single statement) or db.exec() (compound). * * Why this exists: setup/verify.ts:5 codifies that NanoClaw avoids * depending on the sqlite3 CLI binary; setup never installs or probes @@ -28,18 +29,29 @@ if (!dbPath || sql === undefined) { const db = new Database(dbPath); try { - const firstKeyword = sql.trim().split(/\s+/)[0]?.toUpperCase() ?? ''; - if (firstKeyword === 'SELECT' || firstKeyword === 'WITH') { - const rows = db.prepare(sql).all() as Record[]; - for (const row of rows) { - console.log( - Object.values(row) - .map((v) => (v === null ? '' : String(v))) - .join('|'), - ); + try { + const stmt = db.prepare(sql); + if (stmt.reader) { + const rows = stmt.all() as Record[]; + for (const row of rows) { + console.log( + Object.values(row) + .map((v) => (v === null ? '' : String(v))) + .join('|'), + ); + } + } else { + stmt.run(); + } + } catch (e: unknown) { + // better-sqlite3 throws on compound statements ("contains more than + // one statement"). Compound SQL in skills is always mutations + // (e.g. "DELETE ...; INSERT ...;"), so fall back to db.exec(). + if (e instanceof Error && /more than one statement/i.test(e.message)) { + db.exec(sql); + } else { + throw e; } - } else { - db.exec(sql); } } finally { db.close();