119 lines
9.4 KiB
Markdown
119 lines
9.4 KiB
Markdown
# Implementation Plan: Repair `better-sqlite3` native binding to unblock test suite
|
||
|
||
## 0. Problem Statement
|
||
|
||
21 of 48 test suites fail with `TypeError: this.sqlite is not a constructor` thrown from `TypeOrm BetterSqlite3Driver.createDatabaseConnection` (`node_modules/typeorm/driver/better-sqlite3/BetterSqlite3Driver.js:88` — `new this.sqlite(database, ...)`). The remaining suites timeout with `Exceeded timeout of 5000 ms` because TypeORM silently retries (10 retries × 1s) on the same error and the test eventually times out. The visible symptoms in Jest's output are:
|
||
|
||
- Direct-DB suites (`tests/backend/theme-required.spec.ts`, `database-init.spec.ts`, `admin-categories-service.spec.ts`, `uploads-logo.spec.ts`, plus any other suite that imports `DatabaseModule`, `CommonModule`, or builds its own `TypeOrmModule.forRoot`) all hang/timeout with the same root error.
|
||
- Suites that use `AppModule` (bootstrap, auth, event-stream, uploads integration tests, etc.) cascade the failure the first time `TypeOrmModule.forRootAsync` is loaded — 16 of 30 backend suites in total trigger the failure mode.
|
||
|
||
## 1. Root Cause (verified by direct read + REPL probe)
|
||
|
||
`/repo/node_modules/better-sqlite3` directory exists but **its compiled native binding (`better_sqlite3.node`) is missing**.
|
||
|
||
- `ls /repo/node_modules/better-sqlite3/build` → does not exist.
|
||
- `find /repo -name 'better_sqlite3.node'` → zero results anywhere on disk.
|
||
- `cat /repo/node_modules/better-sqlite3/package.json` shows `"install": "prebuild-install || node-gyp rebuild --release"` but the install step never ran for this install. The result is `require('better-sqlite3')` returns a plain function that throws when called as a constructor (`TypeError: this.sqlite is not a constructor` is what TypeORM sees at its `new this.sqlite(...)` call because its `loadDependencies()` reads the package via `PlatformTools.load("better-sqlite3")`; the require "succeeds" returning the stub function, `this.sqlite = sqlite` is set, then `new this.sqlite(...)` blows up because there is no native binding).
|
||
|
||
The trigger: an earlier `npm install sharp --no-audit --no-fund --prefix /repo --save-dev` step (used to add `sharp` so the icon-normalization tests could load it) re-mounted the workspace's dependency tree. The output of that command reported `added 7 packages, and removed 345 packages`, and `removed 345` includes the previously-compiled native bindings for `better-sqlite3` (and likely nuked the per-workspace `node_modules` too — `ls /repo/backend/node_modules` shows only `uuid` left, where before install there were ~50 packages). Other native packages (`argon2`, `sharp` itself) still work because they ship prebuilds that survive reinstall, but `better-sqlite3@11` historically relied on a `node-gyp` build that did not run during the reinstall.
|
||
|
||
This is **environmental / build-harness damage**, not a regression in any `.ts` source file. None of the seven files I edited this session (Job 859 + logo-upload changes) need to be reverted.
|
||
|
||
## 2. Constraints
|
||
|
||
- Plan Mode is active — I will not run install commands or modify files during planning. The full fix is the implementation turn's responsibility.
|
||
- The container runs Node v20.20.2 on `linux-x64` (confirmed via `node -v` and `os.platform()`).
|
||
- `better-sqlite3@11.10.0` was resolved and present in `/repo/node_modules/better-sqlite3/package.json` from the failed install.
|
||
- We must keep `sharp` (added for the icon-normalize feature) and `uuid` (added at backend level) — both were intentional and unrelated to the breakage.
|
||
- The fix must not require a network install if avoidable; it should rely on re-running the existing install scripts.
|
||
- The fix must not require any source-code changes.
|
||
|
||
## 3. Architectural Reconnaissance
|
||
|
||
- **Codebase style & conventions:** NestJS 10 backend + Angular 17 frontend. Workspace uses npm workspaces (`package.json` declares `"workspaces": ["backend", "frontend"]`). Native modules resolve via npm's hoisted `node_modules/`.
|
||
- **Native-module recovery precedent in this repo:** None — `setup.sh` simply runs `npm ci --no-audit --no-fund || npm install --no-audit --no-fund` at the repo root, then `npm --workspace frontend run build` and `npm --workspace backend run build`. There is no explicit `npm rebuild` step.
|
||
- **Test framework:** Jest + ts-jest. Two projects: `tests/backend` (node env) and `tests/frontend` (jsdom env). All tests can be re-run with a single `npm test`.
|
||
- **Required Tools & Dependencies:** No new system tools. The fix needs `npm` (already on PATH) and a working C/C++ toolchain in the container (Node was built natively here, so `node-gyp`/prebuilds should work).
|
||
|
||
## 4. Impacted Files (non-source)
|
||
|
||
### To Modify (none — no source code edits required for this fix)
|
||
|
||
### To Update (operational only)
|
||
|
||
- `setup.sh` — add an explicit `npm rebuild better-sqlite3` step (see §5) so a fresh container / CI run never has this regression again.
|
||
- `package.json` scripts (optional, low-risk) — add `"rebuild-native": "npm rebuild better-sqlite3"` for future debugging.
|
||
|
||
### No changes needed
|
||
|
||
- `backend/package.json` (already declares `better-sqlite3: ^11.0.0` correctly).
|
||
- `backend/src/**` (no source regression).
|
||
- `frontend/src/**` (no source regression).
|
||
- Any test file (the existing tests are correct).
|
||
|
||
## 5. Proposed Changes (ordered, safe, idempotent)
|
||
|
||
The fix is two commands + one documentation tweak. All steps are read-only wrt source code.
|
||
|
||
1. **Recompile the native binding.** From `/repo`, run:
|
||
```
|
||
npm rebuild better-sqlite3 --build-from-source
|
||
```
|
||
This forces `npm` to re-run better-sqlite3's `install` script (prebuild-install, falling back to `node-gyp rebuild --release`) against the current Node 20.20.2 binary. Acceptable runtime: ~30–90 seconds. Verifier: `find /repo -name 'better_sqlite3.node'` should return exactly one path under `node_modules/better-sqlite3/build/Release/...`.
|
||
|
||
2. **Verify TypeORM can construct the connection.** Run a quick smoke test:
|
||
```
|
||
node -e "const db = require('/repo/node_modules/better-sqlite3'); const inst = new db(':memory:'); console.log('memory:', inst.memory);"
|
||
```
|
||
This must print `memory: <an object>`. If `inst.memory` is an empty object, the binding is correctly loaded.
|
||
|
||
3. **Re-run the entire test suite.** Single command from the repo root:
|
||
```
|
||
npm test
|
||
```
|
||
Expected: `Test Suites: 48 passed, 48 total / Tests: 271 passed, 271 total` (matching the pre-regression baseline). If the count differs by exactly the new tests added in the prior logo-upload step (still 48 suites, 275 tests), that is the desired outcome.
|
||
|
||
4. **Hard-wire the recovery into `setup.sh`.** After the existing `npm ci --no-audit --no-fund || npm install --no-audit --no-fund` step, insert:
|
||
```
|
||
# Repro native modules that depend on the host Node ABI.
|
||
npm rebuild better-sqlite3 --build-from-source || npm rebuild better-sqlite3
|
||
```
|
||
This guarantees a fresh clone / CI container that runs `setup.sh` always ends up with a working binding, even if the upstream npm resolution order accidentally drops it again.
|
||
|
||
5. **(Optional) Add a maintenance npm script.** In `package.json`'s top-level `scripts`, add:
|
||
```
|
||
"rebuild-native": "npm rebuild better-sqlite3 --build-from-source"
|
||
```
|
||
so a future implementer can recover with `npm run rebuild-native` when this surfaces again.
|
||
|
||
## 6. Test Strategy (verification only — no new test files)
|
||
|
||
We will not add a new test for this issue because the test suite itself is the canary. After applying steps 1–3 above, the suite must return to its previous green state.
|
||
|
||
Verification checklist executed in the implementation turn:
|
||
|
||
| Check | How | Expected result |
|
||
|---|---|---|
|
||
| Native binary exists | `find /repo -name 'better_sqlite3.node'` | exactly one file under `build/Release/better_sqlite3.node` |
|
||
| Node can construct | smoke-test one-liner above | `inst.memory` is an object literal, not `undefined`/throw |
|
||
| Backend unit tests green | `npx jest --selectProjects backend --testPathPattern=theme-required` | all 6 tests pass within 30s |
|
||
| Backend integration tests green | `npx jest --selectProjects backend --testPathPattern=uploads-logo` | all 4 tests pass |
|
||
| Full suite green | `npm test` | 48 / 48 suites, 271+ / 271+ tests pass |
|
||
|
||
Mocking strategy: not applicable (this is an environmental fix; no new code paths).
|
||
|
||
## 7. Order of Operations (recommended)
|
||
|
||
1. Run `npm rebuild better-sqlite3 --build-from-source` from `/repo`.
|
||
2. Run the smoke-test one-liner to confirm binding loads.
|
||
3. Run `npx jest --selectProjects backend --testPathPattern=theme-required` — expect green.
|
||
4. Run the full `npm test` — expect green.
|
||
5. Edit `setup.sh` (and optionally root `package.json`) to wire the rebuild into the canonical setup flow.
|
||
6. Re-run `npm test` one final time to confirm the wired step works on a clean install path (skip if we can't fully simulate a clean install inside plan mode).
|
||
|
||
## 8. Risk and Rollback
|
||
|
||
- Risk is minimal: `npm rebuild` on a single package does not touch other dependencies, and `setup.sh` is only modified to add a recovery step (additive only).
|
||
- Rollback: if `npm rebuild better-sqlite3 --build-from-source` fails on this image (no compiler toolchain), the fallback `npm rebuild better-sqlite3` (without `--build-from-source`) will attempt to download a precompiled binary via `prebuild-install`. Either path restores the binding; if both fail the issue is environmental and out of scope of the implementation turn.
|
||
- We are NOT reverting any prior fix per the user's instruction — the seven files changed for Jobs 859 + logo-upload stay exactly as they are.
|