sqlite3: pin agent invocation shapes; add dot-commands + -init/-batch#5
Conversation
Adds a 66-test pinning suite that mirrors how reasoning agents actually invoke `sqlite3` in production (sourced from a Braintrust review of ~924 spans across the four flowglad-pay-agent* projects), plus two patches to close the most common gaps. Tests (`packages/just-bash/src/commands/sqlite3/`): - sqlite3.invocation-shapes.test.ts (S1-S11): :memory:, multi-statement, `< script.sql`, `| stdin`, `$(cat ...)`, `-header -separator $'\t'`, output redirect, sequential calls, relative-path db. - sqlite3.sql-features.test.ts (F1-F7, X4): CREATE TABLE AS SELECT, bulk INSERT scripts, window functions, CTEs incl. WITH RECURSIVE, strftime, sqlite_master, CASE WHEN aggregates, scalar subqueries, PRAGMA table_info, indices/views, transactions. - sqlite3.dot-commands.test.ts (D1-D7 + rejection set): .tables, .schema, .headers, .mode csv|tabs|json, .separator, .nullvalue, .read recursive. D8 .import / D9 .dump are kept as `it.todo` and tracked in docs. - sqlite3.flags.test.ts (X1-X2): -init <file>, -batch (accepted no-op). Patches (ship in dist via existing build): - dot-commands.ts (new): preprocessor that translates dot-commands to SQL or formatter mutations before the worker sees the script. Supports .tables/.schema/.indexes/.databases, .headers/.header, .mode, .separator, .nullvalue, .read (recursive, max depth 8), .quit/.exit; accepts no-op for .help/.show/.timer/etc; rejects .import/.dump/.shell/ .system/.open and friends with a clear error. - sqlite3.ts: adds -init <file> (reads + prepends script via ctx.fs) and -batch (no-op). Wires the dot-command preprocessor before the worker call; dot-errors surface to stderr with -bail short-circuit. Triage: GitHub issues are disabled on this fork, so deferred work is tracked in `packages/just-bash/docs/sqlite3-invocation-shapes.md` (D8 .import, D9 .dump, X3 ATTACH-to-file, S2 missing `file` builtin). Verified: 204 sqlite3 tests pass + 2 todo across 13 files; 13,197 tests pass in `test:run`; `tsc --noEmit` clean; `pnpm knip` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![flowglad-review-service[bot]](https://avatars.githubusercontent.com/u/178508461?s=60&v=4){kind=small}
Review SummaryThe dot-command preprocessor and
3 comments posted · Model: |
- Drop dead `hadError && options.bail` from final exitCode (the in-loop bail return makes that branch unreachable). Remove now-unused `hadError` and clarify the comment. - Fix misleading "-init then -cmd" comment; the code prepends -cmd first, then -init, so execution is init -> cmd -> SQL. - Skip lines starting with `--` in the dot-command preprocessor so SQL comments aren't tokenized. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryThree issues found in the new dot-commands preprocessor and its integration into sqlite3.ts:
3 comments posted · Model: |
- Trim sql after dot-command preprocessing and short-circuit when the
input was pure formatter mutations / dot-commands with no remaining
SQL. Avoids sending whitespace to the worker; matches real sqlite3.
- Switch the .tables ESCAPE character from `\` to `~` to avoid the
JS-template-literal -> SQL double-escape ambiguity.
- In preprocessDotCommandsInternal, split multi-line dot-command output
(notably from .read) into individual outLines entries so the final
outLines.join("\n") behaves uniformly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryOne new
1 comment posted · Model: |
Real sqlite3 treats .mode and .separator as orthogonal: csv/tabs modes hardcode their separators in the output formatter, and list mode reads from the configured separator. The previous code mutated mutation.separator inside the .mode handler, which clobbered an explicit .separator that ran *before* the .mode (a script like `.separator ;` then `.mode list` would silently revert to `|`). Drop the separator side-effect from the .mode handler. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryThe incremental diff lands the dot-command preprocessor (
2 comments posted · Model: |
- The "no SQL provided" guard now only fires when no SQL source was given at all (no -init, no positional SQL arg, no stdin). An empty -init file with nothing else falls through to the dot-command preprocessor's clean short-circuit, matching real sqlite3. - The dot-command scanner now requires `.` to be followed by an alphabetic character before treating a line as a dot-command. SQL fragments like `.5` (numeric continuation) no longer get tokenized and rejected as unknown commands. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryThe incremental diff correctly addresses most prior-round comments: the
1 comment posted · Model: |
`sqlArg` and `options.init` are typed `string | null`, so use `=== null` to test absence rather than falsiness. With falsy checks, an explicit empty SQL argument (`sqlite3 :memory: ""`) was treated the same as a missing argument and produced "no SQL provided" + exit 1, but real sqlite3 silently exits 0 in that case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryOne new correctness issue found in this incremental diff:
1 comment posted · Model: |
Previously .quit and .exit emitted empty SQL and the loop kept iterating, so SQL on lines after .quit was still sent to the worker and executed. A script like ".quit\nDROP TABLE t" would still drop the table. Add a quit signal to the translateDotCommand return type and propagate it through PreprocessResult so that: - the main loop returns immediately on .quit, dropping everything below; - a .quit inside a .read'd file also stops the outer parent (matches real sqlite3, where .quit exits the process regardless of nesting). Add regression tests for .quit, .exit, and nested .quit-via-.read. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryThe current diff addresses several previously outstanding issues (dead
1 comment posted · Model: |
`options.init` is `string | null`. Using `!== null` (instead of a truthy check) means `-init ""` actually attempts the read and surfaces a clear "cannot open" error, matching the no-SQL guard below which already treats an explicit empty string as "provided". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryAll previously-outstanding issues from turns 1–7 appear to have been addressed in this diff: the
1 comment posted · Model: |
The destructured `head` is typed `string` but is actually `string | undefined` if `tokens` is empty (TypeScript's index-signature inference hides this). The caller's `/^\.[a-zA-Z]/` guard means tokens is always non-empty in practice, but add an explicit check so a future caller change can't surface as a confusing "unknown command: undefined" error from the switch's default branch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review SummaryNo new findings in this incremental diff. The dot-command preprocessor (
0 comments posted · Model: |
1 participant
Summary
sqlite3in production, sourced from a Braintrust review of ~924 spans across the fourflowglad-pay-agent*projects..tables,.schema,.headers,.mode,.separator,.nullvalue,.read) and the-init <file>+-batchflags.packages/just-bash/docs/sqlite3-invocation-shapes.mdsince GitHub issues are disabled on this fork.What's in the test suite
Four new files under
packages/just-bash/src/commands/sqlite3/:sqlite3.invocation-shapes.test.ts(14 tests) — S1–S11::memory:, multi-statement single arg,< script.sql,| stdin,$(cat …),-header -separator $'\t', output redirect, sequential calls, relative-path db.sqlite3.sql-features.test.ts(17 tests) — F1–F7 + X4:CREATE TABLE … AS SELECT, bulkINSERTscripts, window functions, CTEs incl.WITH RECURSIVE,strftime,sqlite_master,CASE WHEN, scalar subqueries,PRAGMA table_info, indices/views, transactions.sqlite3.dot-commands.test.ts(28 + 2 todo) — D1–D7 plus the rejection set. D8.importand D9.dumpare kept asit.todoand tracked in docs.sqlite3.flags.test.ts(7 tests) — X1-init <file>, X2-batch.What's in the patch
dot-commands.ts(new): pure-function preprocessor that walks the SQL line by line and either translates a dot-command to SQL (.tables→SELECT name FROM sqlite_master WHERE type='table' …) or accumulates a formatter mutation (.mode csv→{ mode: "csv", separator: "," })..readis recursive (max depth 8). Explicitly unsupported commands (.import,.dump,.shell,.system,.open, …) are rejected with a clear error.sqlite3.ts: adds-init <file>(reads viactx.fs, prepends to SQL) and-batch(no-op since just-bash is never interactive). Wires the dot-command preprocessor in before the worker call, applies any formatter mutation toFormatOptions, and surfaces dot-command errors tostderrwith-bailshort-circuit.Limitations (encoded in tests)
sqlite3applies them statement by statement. Pinned indot-commands / "interleaved mode + query"..tablesformat: one name per line, not realsqlite3's 3-column space-padded format.sql.jsquirk —ROUND(x, 2)does not emit clean two-decimal output.999.99round-trips as999.99000000000001. Documented indocs/sqlite3-invocation-shapes.md.Deferred work
Tracked in
packages/just-bash/docs/sqlite3-invocation-shapes.md:.import <file> <table>— could collapse the dominantawk → sql → < scriptETL pattern into one line..dump— schema + INSERT serialization.ATTACH DATABASE 'other.db'to real-FS files — blocked bysql.jssandbox; needs a multi-buffer worker protocol.filebuiltin missing (agents use it for capability probes).Test plan
pnpm typecheck— cleanpnpm knip— cleanpnpm lint— only flags a pre-existing python3/worker.ts violation on main (verified bygit stash+ re-run)pnpm test:wasm src/commands/sqlite3/— 204 passed + 2 todo across 13 filespnpm test:run— 13,197 passed | 97 skipped (388 files)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need.