Add @gtbuchanan/libsql-termux-shim package - #338
Conversation
libsql publishes prebuilt native bindings for darwin, linux, and win32
only, so `require("@libsql/android-arm64")` fails on Termux and every
dependent dies at startup. promptfoo is the motivating case: its results
database is mandatory and reached through drizzle, so it cannot run a
single eval on Android. No env var disables the database.
Node ships SQLite in core, so reimplement the binding in JavaScript
rather than cross-compile. libsql invokes each native function as
`fn.call(handle, ...)` where the handle is whatever databaseOpen or
databasePrepareSync returned, so both sides belong to the binding and
plain objects serve as handles. DatabaseSync/StatementSync are
synchronous like libsql's *Sync natives, and run() already returns the
{ changes, lastInsertRowid } shape, making most of the mapping direct.
Three places where it is not, all load-bearing:
- raw(true) must throw for a statement returning no columns.
@libsql/client calls it inside a try/catch purely to detect whether a
statement yields rows, routing BEGIN/COMMIT to run() when it throws.
Succeeding sends BEGIN down the query path and the client then fails
the batch with TRANSACTION_CLOSED.
- Transaction state is derived from the SQL, there being no autocommit
accessor. Both entry points matter: libsql's transaction() issues
BEGIN through exec(), @libsql/client through prepare().run().
- Rows are rebuilt as plain objects, since node:sqlite returns
null-prototype rows where the real binding returns ordinary ones.
Scoped to local databases; embedded replicas need libsql's replication
protocol and throw. Ships CommonJS because libsql's own wrapper is CJS,
with `type: module` retained so the package's TypeScript files stay ESM.
The two file-level lint suppressions are forced by the binding contract:
`this` is the handle, and the exported names are what libsql
destructures by.
Consumers alias the missing target as an `os: ["android"]` optional
dependency, and must also ungate libsql's own `os` allowlist in
`.pnpmfile.cjs` — pnpm otherwise refuses to resolve before reaching the
shim. See the package README.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesTermux libsql binding shim
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LibsqlWrapper
participant TermuxShim
participant NodeSqlite
LibsqlWrapper->>TermuxShim: call(handle, statement operation)
TermuxShim->>NodeSqlite: prepare, bind, execute, or read rows
NodeSqlite-->>TermuxShim: result, metadata, or row batch
TermuxShim-->>LibsqlWrapper: normalized libsql-compatible result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@codecov.yml`:
- Around line 22-26: Update the libsql-termux-shim coverage configuration around
component_id to prevent the untested scripts/build.ts from lowering patch
coverage: either add coverage for its build-layout behavior, or narrow the
component paths to src/** and configure the build script separately. Preserve
the existing coverage gate requirements.
In `@packages/libsql-termux-shim/README.md`:
- Around line 38-51: Complete the .pnpmfile.cjs example by adding the
afterAllResolved hook alongside readPackage. Have it remove the os field from
each libsql package entry in the lockfile and return the lockfile, then export
both hooks through module.exports.hooks.
In `@packages/libsql-termux-shim/src/index.cjs`:
- Around line 127-131: Update the `databaseOpen` function to construct
`DatabaseSync` with the underlying backend’s extension-loading opt-in enabled,
using `allowExtension: true` or the equivalent supported option, while
preserving the existing in-memory versus file path selection.
In `@packages/libsql-termux-shim/test/index.test.ts`:
- Around line 202-212: Extend the “unsupported operations” suite to cover
databaseSyncUntilSync, databaseMaxWriteReplicationIndex, databaseAuthorizer, and
databaseLoadExtension with their expected native-binding rejection behavior. Add
assertions confirming databaseInterrupt and statementInterrupt are no-ops. Also
add coverage for statementRaw(false), verifying it resets the raw-statement
state correctly.
In `@packages/libsql-termux-shim/tsconfig.build.json`:
- Around line 7-10: Remove the unused "bin" entry from the include list in
tsconfig.build.json, leaving only the "src" directory included for compilation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d7a6043b-0981-4f28-bbff-7bf67bfa622c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.changeset/libsql-termux-shim.mdAGENTS.mdREADME.mdcodecov.ymlpackages/libsql-termux-shim/README.mdpackages/libsql-termux-shim/eslint.config.tspackages/libsql-termux-shim/package.jsonpackages/libsql-termux-shim/scripts/build.tspackages/libsql-termux-shim/src/index.cjspackages/libsql-termux-shim/test/index.test.tspackages/libsql-termux-shim/tsconfig.build.jsonpackages/libsql-termux-shim/tsconfig.jsonpackages/libsql-termux-shim/vitest.config.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
gtbuchanan/tooling(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Every pull request must include a changeset; use an empty changeset for changes that do not affect published packages.
Files:
.changeset/libsql-termux-shim.md
**/vitest.config.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/vitest.config.ts: Configure each package's Vitest tests withconfigurePackage()from@gtbuchanan/vitest-config, including path aliases, test includes, coverage, setup files, and mock reset.
UseconfigureGlobal()to define theslowtag and customize its timeout through theslowoption when needed.
Files:
packages/libsql-termux-shim/vitest.config.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Suppress ESLint diagnostics only with a
--reason suffix, and document exported declarations with JSDoc.
Files:
packages/libsql-termux-shim/vitest.config.tspackages/libsql-termux-shim/scripts/build.tspackages/libsql-termux-shim/eslint.config.tspackages/libsql-termux-shim/test/index.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When asserting on
CommandResult, useexpect(result).toMatchObject({ exitCode: 0 })instead of asserting directly onresult.exitCode.
Files:
packages/libsql-termux-shim/vitest.config.tspackages/libsql-termux-shim/scripts/build.tspackages/libsql-termux-shim/eslint.config.tspackages/libsql-termux-shim/test/index.test.ts
**/eslint.config.ts
📄 CodeRabbit inference engine (AGENTS.md)
Configure package ESLint files with
configure()from@gtbuchanan/eslint-config; follow the package-versus-workspace-root configuration split.
Files:
packages/libsql-termux-shim/eslint.config.ts
README.md
📄 CodeRabbit inference engine (AGENTS.md)
When adding or removing a package, update both the packages table and the structure tree in
README.md.
Files:
README.md
**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Reusable GitHub Actions workflows must use
workflow_callonly; direct triggers belong in thepr.ymlandrelease.ymlpipeline workflows.
Files:
codecov.yml
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Mark slow Vitest tests with the native
slowtag, using either{ tags: ['slow'] }or@module-tag slow.
Files:
packages/libsql-termux-shim/test/index.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Generate incidental test data with
@gtbuchanan/test-utils/buildersor@faker-js/faker, capture it in a local, and assert against that captured value; hard-code literals only when the exact string drives SUT branching.
Files:
packages/libsql-termux-shim/test/index.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: gtbuchanan/tooling
Timestamp: 2026-07-30T12:53:44.775Z
Learning: Run `pnpm build`, `pnpm test:slow`, and `pnpm test:e2e` with `--concurrency=1` on Termux to avoid out-of-memory failures.
📚 Learning: 2026-06-17T04:54:26.509Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 183
File: .changeset/mise-release-age-excludes.md:4-4
Timestamp: 2026-06-17T04:54:26.509Z
Learning: In the gtbuchanan/tooling repo, `.changeset/*.md` files should intentionally omit the Markdown H1 heading (MD041). Follow the changesets CLI convention: `frontmatter` followed by a blank line and then a plain summary text line. Do not prepend the summary with `# `, because the changesets CLI consumes that summary verbatim as the CHANGELOG entry; adding `# ` would introduce an unwanted stray H1 into the generated changelog. MD041 is not enforced by the repo’s `eslint-markdownlint` config for these files; any MD041 warnings for `.changeset/*.md` come from markdownlint-cli2 defaults rather than the repo’s own lint configuration.
Applied to files:
.changeset/libsql-termux-shim.md
📚 Learning: 2026-06-04T02:26:02.824Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 139
File: packages/cli/test/coverage-codecov-upload.test.ts:2-2
Timestamp: 2026-06-04T02:26:02.824Z
Learning: In tests within **/{test,e2e,__tests__}/**/*.{test,spec}.{ts,tsx,js,jsx}, follow the AGENTS.md faker convention: direct `faker-js/faker` usage is allowed only for one-off primitives when the produced value’s shape is exactly what faker returns (e.g., `faker.git.commitSha()`, `faker.string.uuid()`). Use `gtbuchanan/test-utils/builders` only when there is a domain-shaped value worth centralizing (e.g., scoped package names, semver ranges, GitHub URLs). Do not wrap a native faker generator in a builder for a plain primitive (e.g., a raw commit SHA), since that adds indirection without centralizing any domain shape.
Applied to files:
packages/libsql-termux-shim/test/index.test.ts
📚 Learning: 2026-06-15T19:58:27.007Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 162
File: packages/cli/test/typecheck-pkl.test.ts:8-23
Timestamp: 2026-06-15T19:58:27.007Z
Learning: When reviewing tests (e.g., in packages/**/test/**/*.test.ts), if the system-under-test explicitly branches on specific string-literal values (such as sort order, extension filters, or reserved filename exclusions like "PklProject"), require those branching inputs to be hardcoded literals in the test rather than generated via test-data builders. Do not recommend replacing these literals with builder patterns, because it would obscure what behavior is being exercised. Allow builder patterns only for incidental, domain-shaped data where the exact values do not affect the assertion logic (e.g., scoped package names or semver ranges used as opaque inputs).
Applied to files:
packages/libsql-termux-shim/test/index.test.ts
🪛 markdownlint-cli2 (0.23.1)
.changeset/libsql-termux-shim.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (13)
packages/libsql-termux-shim/package.json (1)
1-47: LGTM!AGENTS.md (1)
40-40: LGTM!README.md (1)
16-16: LGTM!packages/libsql-termux-shim/README.md (1)
26-32: 📐 Maintainability & Code Quality | 💤 Low valueVerify the pinned shim version matches the first published release.
The install example pins
@gtbuchanan/libsql-termux-shim@^0.1.0, butpackage.jsoncurrently declares"version": "0.0.0". If the changeset doesn't bump straight to0.1.0on first publish, this example will point at a version that was never published.packages/libsql-termux-shim/src/index.cjs (2)
1-421: JSDoc/eslint-disable hygiene: compliant.Every exported function has JSDoc, and all ESLint suppressions include a
--reasoned justification, consistent with the module-wide contract explanation at the top of the file.
268-274: 🎯 Functional CorrectnessNo change needed for
statementRaw(false).
statementRawshould only throw when@libsql/clientprobesraw(true)on a no-column statement; the documented contract does not require permitting/disabling raw mode on that path, and the current implementation matchesraw(true),raw(false), and no argument consistently.> Likely an incorrect or invalid review comment.packages/libsql-termux-shim/test/index.test.ts (1)
84-123: LGTM!packages/libsql-termux-shim/tsconfig.json (1)
1-15: LGTM!packages/libsql-termux-shim/eslint.config.ts (1)
1-5: LGTM!packages/libsql-termux-shim/vitest.config.ts (1)
1-3: LGTM!packages/libsql-termux-shim/scripts/build.ts (1)
1-21: LGTM!codecov.yml (1)
86-89: LGTM!.changeset/libsql-termux-shim.md (1)
1-9: LGTM!
Codecov flagged missing coverage. Chasing it surfaced two real problems rather than just untested lines. pack:npm does not fail when the build artifact is missing. Deleting dist/source/src/index.cjs and packing exits 0 and produces a tarball containing only LICENSE, package.json, and README.md — no shim at all. The package would publish completely broken and only fail inside a consumer's require. Nothing else asserts that copy happens, so scripts/build.ts earns a test: it clears the artifact, runs the script, and compares the result against the source byte for byte. databaseLoadExtension could never have worked. libsql permits extension loading per call, but node:sqlite decides it when the connection is constructed, so enableLoadExtension throws "disabled at database creation" for every database this shim opens. Opting every connection in to allowExtension so an unused call could work trades real capability for hypothetical fidelity, so the entry point now reports the gap the way the replica methods do. Also covers the paths that were merely untested — close, database-level safe integers, the authorizer, syncUntil, an empty path, a statement invoked without bind parameters, and the null column metadata an expression column produces — taking src and scripts to full line and branch coverage. The bind parameter is documented optional, which is what the shim already implemented. The node:sqlite import suppression records stale plugin data rather than a real gap: eslint-plugin-n lists the module experimental from 22.5.0 with no stabilization entry, so no engines range clears it, yet Node emits no ExperimentalWarning on the versions this package supports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review feedback on the shim. The README showed only `readPackage` while the prose said both hooks are required, which would have left a consumer's lockfile still carrying libsql's `os` field and failing the next install. The example now defines `afterAllResolved` and exports both. Adds the raw-mode reset case: `raw(false)` after `raw(true)` should return object rows again. That round trip was the one statementRaw behavior nothing exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why
libsql publishes prebuilt native bindings for darwin, linux, and win32 only — there is no
@libsql/android-arm64. Anything that reaches libsql therefore fails at startup on Termux:promptfoo is the motivating case. Its results database is mandatory and reached through drizzle, so it cannot run a single eval on Android, and no environment variable disables it. This blocks running skill evals on Termux at all — the same class of wall as turbo, already documented in
AGENTS.md.Node ships SQLite in core, so the binding can be reimplemented in JavaScript rather than cross-compiled.
How
libsql's JS wrapper invokes each native function as
fn.call(handle, ...), where the handle is whateverdatabaseOpen/databasePrepareSyncreturned. Both sides belong to the binding, so plain objects serve as handles.DatabaseSync/StatementSyncare synchronous like libsql's*Syncnatives andrun()already returns the{ changes, lastInsertRowid }shape, so most of the mapping is direct.Three places where it isn't, all load-bearing:
raw(true)must throw for a statement returning no columns.@libsql/clientcalls it inside atry/catchpurely to detect whether a statement yields rows, routingBEGIN/COMMIT/INSERTtorun()when it throws. Succeeding sendsBEGINdown the query path and the client then fails the batch withTRANSACTION_CLOSED.transaction()issuesBEGINthroughexec(), while@libsql/clientissues it throughprepare().run().node:sqlitereturns null-prototype rows where the real binding returns ordinary ones — a difference that otherwise leaks to anything inspecting a row's prototype.Scoped to local databases. Embedded replicas (
syncUrl,sync(),syncUntil()) need libsql's replication protocol and throw rather than silently misbehave.Notes for review
require()s it.type: moduleis retained so the package's TypeScript files stay ESM — without it,import.metaineslint.config.ts/scripts/build.tsis rejected as CJS output..cjsrather than compiled TypeScript, socompile:tsis a copy script, mirroring@gtbuchanan/pnpm-termux-shim. JSDoc types meantypecheck:tsstill covers it.unicorn/no-this-outside-of-class(thisis the handle, across the whole module) andunicorn/consistent-boolean-name(databaseInTransaction/statementIsReaderare the identifiers libsql destructures by, so they can't be renamed). Localized suppressions are scoped next-line or to a block, each with a reason.Consumer requirement beyond this package
Aliasing the missing target is necessary but not sufficient —
libsqlitself declares anosallowlist omitting android, so pnpm refuses to resolve before ever reaching the shim. Consumers need the ungating in both.pnpmfile.cjshooks (readPackagefor resolution,afterAllResolvedfor the field pnpm records in the lockfile). Documented in the package README.Verification
fn.call(handle, ...)), covering the round trip, both transaction paths, theraw()throw, column metadata mapping, BigInt reads, and the unsupported entry points. They run in the fast bucket.gtb verifyreports no drift; the fullpnpm buildpasses.@libsql/android-arm64resolved to this package, promptfoo 0.121.19 runs a complete Agent Skill eval on Termux —skill-usedand ajavascriptassertion both passing — against the Termux Claude Code build viapath_to_claude_code_executable.🤖 Generated with Claude Code