Skip to content

Commit 4b17b40

Browse files
ci: actually rebuild better-sqlite3 by bypassing .npmrc ignore-scripts
Commit 7f8e646 didn't actually fix CI. I missed two things, both of which made the previous fix silently no-op while appearing green locally: 1. The repo's [.npmrc:2](.npmrc#L2) has `ignore-scripts=true` set globally. That config gate overrides every form of `pnpm install`, `pnpm rebuild`, `pnpm --filter <pkg> rebuild <dep>`, and even `pnpm.onlyBuiltDependencies` allow-lists. The CI's `pnpm rebuild better-sqlite3` step ran in 700ms with zero output and produced no binding — verified by reading the previous CI log. The `--ignore-scripts` CLI flag I had been preserving was always redundant with the .npmrc setting. 2. `require('better-sqlite3')` does NOT actually load the native binding. better-sqlite3's lib/database.js only calls `require('bindings')('better_sqlite3.node')` LAZILY, inside the Database constructor (line 48). So the previous smoke test (`node -e "require('better-sqlite3')"`) returned successfully even when the binding was missing — the same flaw the faucet/Dockerfile smoke has, which the Dockerfile gets away with because its `npm rebuild` actually works in that context. Real fix: - Drop the redundant `--ignore-scripts` CLI flag from `pnpm install`. The .npmrc setting still applies, so biome/ultracite postinstalls remain skipped (the original intent). - Replace `pnpm rebuild better-sqlite3` with a direct invocation of the package's own install script from its store directory: cd node_modules/.pnpm/better-sqlite3@*/node_modules/better-sqlite3 npm run install This bypasses the .npmrc gate (because it's a script run, not a package install), and runs `prebuild-install`, which downloads the prebuilt binding from upstream's GitHub release for the current (node-20, linux-x64) tuple. Verified upstream — the prebuild for v11.10.0 / NODE_MODULE_VERSION 115 / linux-x64 exists. - Replace the broken `require()`-only smoke test with one that instantiates an in-memory Database and executes a query, forcing the .node binding to actually load: node -e "const D=require('better-sqlite3'); const db=new D(':memory:'); db.exec('SELECT 1'); db.close()" Verified locally (node 20.20.0, pnpm 9.15.9, simulating the exact CI environment, after removing the binding to mirror a clean install): - Direct install step → exit 0, prebuild-install fetches the 2.0 MB .node file to build/Release/better_sqlite3.node - Smoke step → "smoke OK" - `pnpm --filter @pushflip/faucet test` → 17/17 pass (was: 1 fail with "Could not locate the bindings file" on previous attempt)
1 parent 7f8e646 commit 4b17b40

1 file changed

Lines changed: 33 additions & 25 deletions

File tree

.github/workflows/ci.yml

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -67,35 +67,43 @@ jobs:
6767
cache: pnpm
6868

6969
- name: Install workspace deps
70-
run: pnpm install --frozen-lockfile --ignore-scripts
71-
# --ignore-scripts: same reason as the Dockerfile — skip
72-
# biome/ultracite postinstalls that aren't needed for CI gates.
70+
run: pnpm install --frozen-lockfile
71+
# The repo-level `.npmrc` already sets `ignore-scripts=true`,
72+
# which skips biome/ultracite postinstalls AND better-sqlite3's
73+
# `prebuild-install || node-gyp rebuild` install script. The
74+
# better-sqlite3 binding is restored by the dedicated step
75+
# below. The `--ignore-scripts` CLI flag was previously passed
76+
# here too — it was redundant with the .npmrc setting, so
77+
# removed for clarity.
7378

7479
- name: Build better-sqlite3 native binding
75-
# --ignore-scripts above skips better-sqlite3's own postinstall,
76-
# which is the step that fetches/compiles its .node binary.
77-
# Without this rebuild, faucet tests that touch the nicknames
78-
# DB fail with "Could not locate the bindings file". Rebuilding
79-
# only this one package keeps the security posture of
80-
# --ignore-scripts intact (biome/ultracite postinstalls stay
81-
# blocked). Mirrors the faucet/dealer Dockerfiles, which use
82-
# the same `--ignore-scripts` + targeted rebuild + require()
83-
# smoke pattern.
84-
run: pnpm rebuild better-sqlite3
80+
# The repo's `.npmrc ignore-scripts=true` blocks every form of
81+
# `pnpm install/rebuild` from running install scripts, even
82+
# with explicit allow-lists like `pnpm.onlyBuiltDependencies`
83+
# — verified empirically (commit 7f8e646's `pnpm rebuild
84+
# better-sqlite3` step ran in 700ms with zero output and
85+
# produced no binding). The reliable bypass is to invoke the
86+
# package's own install script directly via `npm run install`
87+
# from its directory: that runs `prebuild-install`, which
88+
# downloads the precompiled binding from upstream's GitHub
89+
# release for the current (node-20, linux-x64) tuple. The
90+
# `bash -c` form lets the glob expand the pnpm-store path
91+
# without committing to a specific better-sqlite3 version.
92+
run: |
93+
cd node_modules/.pnpm/better-sqlite3@*/node_modules/better-sqlite3
94+
npm run install
8595
8696
- name: Verify better-sqlite3 native binding loads
87-
# Load-bearing guard, not paranoia. `pnpm rebuild` can silently
88-
# succeed-while-failing: if upstream ever drops the prebuild
89-
# for our (node-20, linux-x64, glibc) tuple it falls back to
90-
# a from-source build that needs python3/make/g++; if any of
91-
# those are missing the fallback fails to produce a binding
92-
# but the rebuild step itself still exits 0. The require()
93-
# call below fails loudly with the same error the faucet
94-
# tests would hit, giving a clear signal at install time
95-
# rather than several steps later inside `pnpm test`. Pattern
96-
# mirrored from faucet/Dockerfile (see comments there for the
97-
# full rationale).
98-
run: pnpm --filter @pushflip/faucet exec node -e "require('better-sqlite3')"
97+
# Load-bearing guard. `require('better-sqlite3')` alone does
98+
# NOT trigger the binding load — better-sqlite3's lib/database.js
99+
# only calls `require('bindings')('better_sqlite3.node')` lazily
100+
# inside the Database constructor (line 48). So this step
101+
# instantiates an in-memory Database AND executes a query,
102+
# which forces the .node file to load. Without this, a missing
103+
# binding would slip past the smoke and only surface inside
104+
# `pnpm test`, several steps later, with a less actionable
105+
# failure context.
106+
run: pnpm --filter @pushflip/faucet exec node -e "const D=require('better-sqlite3'); const db=new D(':memory:'); db.exec('SELECT 1'); db.close()"
99107

100108
- name: Lint (app)
101109
run: pnpm --filter @pushflip/app lint

0 commit comments

Comments
 (0)