GLOOK-41: translate DATE_ADD for SQLite — org report 500s when a report has spend data

src/lib/__tests__/unit/mysql-date-expressions.test.ts

@@ -0,0 +1,43 @@
+/**
+ * Enforcement: every MySQL date expression written in src/lib SQL must have a
+ * translateSQL rule.
+ *
+ * GLOOK-41 shipped because nothing connected "someone wrote DATE_ADD" to "the
+ * SQLite translator knows DATE_ADD". The mocked suites around the caller never
+ * reach translateSQL, so CI stayed green and the gap surfaced as a 500. This
+ * test closes that loop: a new unhandled form fails here instead.
+ */
+import fs from 'fs';
+import path from 'path';
+import { translateSQL } from '@/lib/db/sqlite';
+
+const LIB = path.join(process.cwd(), 'src', 'lib');
+
+function walk(dir: string): string[] {
+  return fs.readdirSync(dir, { withFileTypes: true }).flatMap((e) => {
+    const full = path.join(dir, e.name);
+    if (e.isDirectory()) return e.name === '__tests__' ? [] : walk(full);
+    return e.isFile() && full.endsWith('.ts') && !full.endsWith('db/sqlite.ts') ? [full] : [];
+  });
+}
+
+describe('MySQL date expressions in src/lib SQL', () => {
+  it('are all handled by translateSQL', () => {
+    const pattern = /DATE_ADD\s*\([^)]*INTERVAL[^)]*\)|DATE_SUB\s*\([^)]*INTERVAL[^)]*\)/gi;
+    const offenders: string[] = [];
+
+    for (const file of walk(LIB)) {
+      const src = fs.readFileSync(file, 'utf8');
+      for (const expr of src.match(pattern) ?? []) {
+        // Wrap in a minimal statement so translateSQL sees realistic input.
+        try {
+          translateSQL(`SELECT 1 WHERE x < ${expr}`);
+        } catch {
+          offenders.push(`${path.relative(process.cwd(), file)}: ${expr}`);
+        }
+      }
+    }
+
+    expect(offenders).toEqual([]);
+  });
+});

src/lib/__tests__/unit/sqlite-date-add.test.ts

@@ -0,0 +1,135 @@
+/**
+ * GLOOK-41 regression: the org report's spend-window query uses
+ * `DATE_ADD(?, INTERVAL 1 DAY)` (src/lib/report/org.ts). translateSQL had rules
+ * for DATE_SUB but none for DATE_ADD, so the statement reached better-sqlite3
+ * verbatim and failed with `near "1": syntax error`.
+ *
+ * Two layers on purpose:
+ *  - translateSQL is pure, so the rewrite itself is asserted directly. That is
+ *    where sign, magnitude and quoting live, and it needs no driver.
+ *  - one case drives the REAL SQLite driver, because the value of this fix is
+ *    that better-sqlite3 accepts the output. The mocked suites around
+ *    getOrgReport stayed green through this bug precisely because none of them
+ *    reach translateSQL.
+ */
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { translateSQL } from '@/lib/db/sqlite';
+
+describe('translateSQL — MySQL date expressions', () => {
+  it('rewrites the org spend-window DATE_ADD, keeping the bound parameter', () => {
+    expect(
+      translateSQL('SELECT 1 WHERE committed_at < DATE_ADD(?, INTERVAL 1 DAY)'),
+    ).toBe("SELECT 1 WHERE committed_at < datetime(?, '+1 days')");
+  });
+
+  it('adds days rather than subtracting them', () => {
+    // The rule was written by copying the DATE_SUB block; flipping '+' to '-'
+    // is the slip that copying invites, and a shape-only assertion would miss it.
+    expect(translateSQL('SELECT DATE_ADD(?, INTERVAL 7 DAY)')).toContain("'+7 days'");
+    expect(translateSQL('SELECT DATE_SUB(?, INTERVAL 7 DAY)')).toContain("'-7 days'");
+  });
+
+  it('keeps DATE_SUB(NOW(), …) on localtime so it agrees with NOW()', () => {
+    // Without 'localtime' the two disagree by the host's UTC offset, silently
+    // shifting every 90-day window in projects/untracked.ts, epic-stats.ts and
+    // epic-summary.ts on any non-UTC host.
+    const out = translateSQL('SELECT NOW() AS a, DATE_SUB(NOW(), INTERVAL 90 DAY) AS b');
+    expect(out).toContain("datetime('now','localtime') AS a");
+    expect(out).toContain("datetime('now', 'localtime', '-90 days') AS b");
+  });
+
+  it('leaves a MySQL date expression inside a string literal byte-identical', () => {
+    // Anchoring the pattern to `?`/identifier is NOT sufficient on its own:
+    // 'DATE_ADD(a , INTERVAL 9 DAY)' still matches an identifier capture, and the
+    // replacement injects quotes, which would terminate the literal and
+    // restructure the statement. The rewrites run per non-literal chunk instead.
+    const sql = "SELECT 1 WHERE msg = 'DATE_ADD(a , INTERVAL 9 DAY)'";
+    expect(translateSQL(sql)).toBe(sql);
+  });
+
+  it('handles a doubled quote inside a literal', () => {
+    const sql = "SELECT 1 WHERE note = 'it''s a DATE_SUB(x , INTERVAL 3 DAY) note'";
+    expect(translateSQL(sql)).toBe(sql);
+  });
+
+  it('rewrites a real expression while leaving a literal beside it untouched', () => {
+    expect(
+      translateSQL("SELECT 1 WHERE msg='DATE_ADD(q , INTERVAL 5 DAY)' AND t < DATE_ADD(?, INTERVAL 1 DAY)"),
+    ).toBe("SELECT 1 WHERE msg='DATE_ADD(q , INTERVAL 5 DAY)' AND t < datetime(?, '+1 days')");
+  });
+
+  it('does not let SQL-looking text stored as data trip the guard', () => {
+    // commit_analyses stores commit messages; a message mentioning INTERVAL must
+    // not turn a working query into a thrown error.
+    const sql = "INSERT INTO commit_analyses (message) VALUES ('perf: use INTERVAL 7 DAY window')";
+    expect(() => translateSQL(sql)).not.toThrow();
+  });
+
+  it('throws instead of passing an untranslated form to the driver', () => {
+    // Passthrough was the root cause of GLOOK-41: unmatched SQL reached the
+    // driver and 500d at request time. These shapes have no rule; each must fail
+    // loudly and greppably rather than at a user's request.
+    for (const sql of [
+      'SELECT DATE_ADD(?, INTERVAL 1 MONTH)',
+      'SELECT DATE_ADD(?, INTERVAL ? DAY)',
+      'SELECT DATE_ADD(DATE(x), INTERVAL 1 DAY)',
+      'SELECT DATE_SUB(?, INTERVAL 2 HOUR)',
+    ]) {
+      expect(() => translateSQL(sql)).toThrow(/untranslated MySQL date expression/);
+    }
+  });
+});
+
+describe('SQLite driver accepts the translated spend-window clause', () => {
+  const prevSqlitePath = process.env.SQLITE_PATH;
+  const prevDbType = process.env.DB_TYPE;
+  let tmpDir: string;
+  let db: { execute: <T = any>(sql: string, params?: any[]) => Promise<[T[], any]> };
+
+  beforeAll(async () => {
+    // mkdtempSync, not a guessable path in the shared temp dir — matches
+    // cc-apply-breakdowns.test.ts, prompt-loader.test.ts and logger.test.ts.
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'glooker-glook41-'));
+    process.env.SQLITE_PATH = path.join(tmpDir, 'test.db');
+    process.env.DB_TYPE = 'sqlite';
+    const { createSQLiteDB } = await import('@/lib/db/sqlite');
+    // One handle for the whole suite: each createSQLiteDB() re-runs the schema
+    // and every ALTER migration.
+    db = createSQLiteDB();
+  });
+
+  afterAll(() => {
+    // Restore, or later files in this Jest worker inherit a deleted DB path.
+    if (prevSqlitePath === undefined) delete process.env.SQLITE_PATH;
+    else process.env.SQLITE_PATH = prevSqlitePath;
+    if (prevDbType === undefined) delete process.env.DB_TYPE;
+    else process.env.DB_TYPE = prevDbType;
+    try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* already gone */ }
+  });
+
+  it('lands the boundary on exact midnight of the following day', async () => {
+    const [rows] = await db.execute(
+      `SELECT DATE_ADD(?, INTERVAL 1 DAY) AS boundary`,
+      ['2026-03-18'],
+    ) as [any[], any];
+    // Pinned exactly: a time component here would be a real MySQL/SQLite difference.
+    expect(String(rows[0].boundary)).toBe('2026-03-19 00:00:00');
+  });
+
+  it('keeps the spend window exclusive at the end date', async () => {
+    // Inputs use the format actually stored in commit_analyses — GitHub's
+    // ISO-8601 `…T…Z` (src/lib/github.ts → src/lib/report-runner.ts), compared
+    // as TEXT against datetime()'s space-separated output.
+    const inWindow = async (committedAt: string) => {
+      const [rows] = await db.execute(
+        `SELECT 1 AS hit WHERE ? < DATE_ADD(?, INTERVAL 1 DAY)`,
+        [committedAt, '2026-03-31'],
+      ) as [any[], any];
+      return rows.length === 1;
+    };
+    expect(await inWindow('2026-03-31T23:59:59Z')).toBe(true);
+    expect(await inWindow('2026-04-01T00:00:00Z')).toBe(false);
+  });
+});

src/lib/db/sqlite.ts

@@ -386,25 +386,89 @@ export function createSQLiteDB(): DB {
   return dbApi;
 }
 
-function translateSQL(sql: string): string {
-  let s = sql;
-
-  // INSERT IGNORE → INSERT OR IGNORE
-  s = s.replace(/INSERT\s+IGNORE\s+INTO/gi, 'INSERT OR IGNORE INTO');
-
-  // NOW() → datetime('now','localtime')
-  s = s.replace(/NOW\(\)/gi, "datetime('now','localtime')");
-
-  // LEFT(col, N) → SUBSTR(col, 1, N)
-  s = s.replace(/LEFT\s*\(([^,]+),\s*(\d+)\)/gi, 'SUBSTR($1, 1, $2)');
+/**
+ * Apply `fn` only to the parts of `sql` that sit OUTSIDE single-quoted string
+ * literals, leaving literal contents byte-for-byte intact.
+ *
+ * Every rule in translateSQL is a regex over raw SQL text, and several of them
+ * inject quote characters. Without this, a literal whose contents happen to look
+ * like MySQL syntax gets rewritten, and the injected quotes terminate the literal
+ * and restructure the statement. Anchoring the patterns narrows that but does not
+ * close it — `'DATE_ADD(a , INTERVAL 9 DAY)'` still matches an identifier capture.
+ *
+ * SQLite escapes a quote inside a literal by doubling it ('').
+ */
+function outsideStringLiterals(sql: string, fn: (chunk: string) => string): string {
+  const parts: string[] = [];
+  let i = 0;
+  let chunkStart = 0;
+
+  while (i < sql.length) {
+    if (sql[i] !== "'") { i++; continue; }
+
+    parts.push(fn(sql.slice(chunkStart, i)));
+
+    let j = i + 1;
+    while (j < sql.length) {
+      if (sql[j] === "'") {
+        if (sql[j + 1] === "'") { j += 2; continue; } // escaped quote, keep scanning
+        break;
+      }
+      j++;
+    }
+    const end = Math.min(j + 1, sql.length);
+    parts.push(sql.slice(i, end)); // literal, verbatim
+    i = chunkStart = end;
+  }
 
-  // DATE_SUB(NOW(), INTERVAL N DAY) → datetime('now', '-N days')
-  s = s.replace(/DATE_SUB\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi,
-    (_match, days) => `datetime('now', '-${days} days')`);
-  // Also handle case where NOW() hasn't been translated yet
-  s = s.replace(/DATE_SUB\s*\(\s*NOW\(\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi,
-    (_match, days) => `datetime('now', '-${days} days')`);
+  parts.push(fn(sql.slice(chunkStart)));
+  return parts.join('');
+}
 
+export function translateSQL(sql: string): string {
+  // Token-level rewrites, applied only outside string literals.
+  let s = outsideStringLiterals(sql, (chunk) => {
+    let c = chunk;
+
+    // INSERT IGNORE → INSERT OR IGNORE
+    c = c.replace(/INSERT\s+IGNORE\s+INTO/gi, 'INSERT OR IGNORE INTO');
+
+    // NOW() → datetime('now','localtime')
+    c = c.replace(/NOW\(\)/gi, "datetime('now','localtime')");
+
+    // LEFT(col, N) → SUBSTR(col, 1, N)
+    c = c.replace(/LEFT\s*\(([^,]+),\s*(\d+)\)/gi, 'SUBSTR($1, 1, $2)');
+
+    // DATE_SUB(NOW(), INTERVAL N DAY) → datetime('now', 'localtime', '-N days')
+    // `localtime` is required: NOW() became datetime('now','localtime') just above,
+    // so omitting it makes NOW() and DATE_SUB(NOW(), …) disagree by the host's UTC
+    // offset — a silent window shift on any non-UTC host. Six callers rely on this
+    // (projects/untracked.ts, projects/epic-stats.ts, projects/epic-summary.ts).
+    c = c.replace(/DATE_SUB\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi,
+      (_m, days) => `datetime('now', 'localtime', '-${days} days')`);
+
+    // DATE_SUB / DATE_ADD (<bound param or column>, INTERVAL N DAY)
+    // Anchored to `?` or a bare identifier: the expression is spliced through
+    // verbatim, so a bound `?` stays a bound parameter and the rewrite introduces
+    // no new placeholders (no positional-parameter shift).
+    c = c.replace(/DATE_SUB\s*\(\s*(\?|[A-Za-z_][A-Za-z0-9_.]*)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi,
+      (_m, expr, days) => `datetime(${expr}, '-${days} days')`);
+    c = c.replace(/DATE_ADD\s*\(\s*(\?|[A-Za-z_][A-Za-z0-9_.]*)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi,
+      (_m, expr, days) => `datetime(${expr}, '+${days} days')`);
+
+    // No DATE_ADD(NOW(), …) rule on purpose: nothing in the codebase writes that
+    // shape, and an unexercised rule is exactly where the localtime skew hid. The
+    // guard below makes the omission loud rather than silently wrong.
+    return c;
+  });
+
+  // Guard against the passthrough default. translateSQL is a whitelist rewriter:
+  // anything unmatched previously reached better-sqlite3 verbatim and failed at
+  // request time (GLOOK-41 surfaced as `near "1": syntax error`). Mocked suites
+  // never call this function, so such gaps stayed green in CI and were found by
+  // users. Fail loudly instead — and mysql-date-expressions.test.ts turns any new
+  // offender into a CI failure. Only non-literal text is inspected, so SQL stored
+  // as data cannot trip it.
   // ON DUPLICATE KEY UPDATE ... VALUES(col) → ON CONFLICT(...) DO UPDATE SET col = excluded.col
   const odkuMatch = s.match(/ON\s+DUPLICATE\s+KEY\s+UPDATE\s+([\s\S]+)$/i);
   if (odkuMatch) {
@@ -440,5 +504,24 @@ function translateSQL(sql: string): string {
       `ON CONFLICT(${conflict}) DO UPDATE SET ${updateClause}`);
   }
 
+  // translateSQL is a whitelist rewriter with a passthrough default: anything it does
+  // not match reaches better-sqlite3 verbatim and fails at request time with a
+  // token-level parse error (GLOOK-41 was exactly this — `near "1": syntax error`).
+  // Mocked test suites never call this function, so such gaps stay green in CI and are
+  // found by users. Fail loudly here instead: a thrown error names the statement and is
+  // greppable, and `mysql-date-expressions.test.ts` turns any new offender into a CI
+  // failure rather than a production 500.
+  // Only non-literal text is inspected — SQL-looking text stored as data (a commit
+  // message, a Jira summary) must not be able to trip this. Reuses the same
+  // tokenizer as the rewrites, so "what counts as a literal" has one definition.
+  let nonLiteral = '';
+  outsideStringLiterals(s, (chunk) => { nonLiteral += ` ${chunk} `; return chunk; });
+  if (/\bDATE_ADD\b|\bDATE_SUB\b|\bINTERVAL\b/i.test(nonLiteral)) {
+    throw new Error(
+      `translateSQL: untranslated MySQL date expression reached the SQLite driver. ` +
+      `Add a rule to translateSQL() in src/lib/db/sqlite.ts. SQL: ${sql}`,
+    );
+  }
+
   return s;
 }