From 938d7f145ce22873565f340866cd40cbf4a5a5dd Mon Sep 17 00:00:00 2001 From: Parikshith Date: Tue, 8 Sep 2026 16:22:39 +0530 Subject: [PATCH 1/2] fix(ci): pin Node to 24.18.1 to match runtime; surface app startup logs The `test` job intermittently fails at "Wait for app availability" because `npm run dev` crashes on startup: node::RemoveEnvironmentCleanupHook(...) at ../src/api/hooks.cc:142 Assertion failed: (env) != nullptr Statement::~Statement() [better-sqlite3/build/Release/better_sqlite3.node] Node >= 24.19.0 added node::ObjectWrap cleanup hooks that are incompatible with NAN-style native addons such as better-sqlite3 (11.9.1, this project's pinned version), tripping that assertion on teardown. It is intermittent (a race during native cleanup), so the job fails on some runs and passes on others. The app ships on Node 24.18.1 (both Dockerfiles and .nvmrc), which predates the breaking change, so production is unaffected. Only CI hit it, because `setup-node` with `node-version: '24'` floats to the latest 24.x (24.20.0 at time of failure). Pinning CI to 24.18.1 makes it test the version that actually ships and avoids the regression. Also capture `npm run dev` output to app.log and fail fast (printing it) if the process exits early, so a future startup failure is diagnosable instead of surfacing only as "App failed to start" after a 25s wait. (Longer term, upgrading better-sqlite3 to v12+ restores Node 24.19+ support; pinning the runtime is the minimal, prod-matching fix.) Co-Authored-By: Claude Opus 4.8 --- .github/workflows/linting.yml | 4 +++- .github/workflows/test.yml | 20 +++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index e75349117..55e872dee 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -26,7 +26,9 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: '24' + # Match the version the app ships on (Dockerfile / .nvmrc) + # rather than floating to the latest 24.x. + node-version: '24.18.1' - name: Install dependencies run: npm ci diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98798ab06..f9fbf6fa7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,7 +19,12 @@ jobs: - name: Install Node uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: '24' + # Pin to the version the app actually ships on (Dockerfile / .nvmrc). + # A bare '24' floats to the latest 24.x; Node >= 24.19.0 added + # node::ObjectWrap cleanup hooks that crash better-sqlite3 on teardown + # ("Assertion failed: (env) != nullptr" in RemoveEnvironmentCleanupHook), + # which intermittently kills `npm run dev` in the step below. + node-version: '24.18.1' - name: Copy config file run: cp config/config.example.yml config/config.yml @@ -43,7 +48,9 @@ jobs: run: npx tsc --noEmit - name: Start app in background - run: nohup npm run dev & + run: | + nohup npm run dev > app.log 2>&1 & + echo $! > app.pid - name: Wait for app availability run: | @@ -52,10 +59,17 @@ jobs: echo "App is up" exit 0 fi + # Fail fast (and show why) if the process already exited. + if ! kill -0 "$(cat app.pid)" 2>/dev/null; then + echo "App process exited before becoming available. Output:" + cat app.log + exit 1 + fi echo "Waiting for the app... attempt $i" sleep 5 done - echo "App failed to start" + echo "App failed to start. Output:" + cat app.log exit 1 build-sqlite: From c19cb706e8a374bb60fabb679f94060c602a3f95 Mon Sep 17 00:00:00 2001 From: Parikshith Date: Tue, 8 Sep 2026 16:28:35 +0530 Subject: [PATCH 2/2] fix(ci): harden app-readiness PID check; clarify version-source comment Address review feedback on the readiness step: - Read the PID safely (`cat app.pid 2>/dev/null || true`) and treat a missing/empty PID as "not running", so a failed nohup no longer produces a confusing `kill` usage error and misleading message. - Guard the diagnostic dump behind a helper so a missing app.log prints "(app.log not found)" instead of erroring. - The exact 24.18.1 pin comes from the Dockerfiles; .nvmrc only pins the 24 major. Corrected the comments to say so rather than citing .nvmrc as the source of the patch version. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/linting.yml | 5 +++-- .github/workflows/test.yml | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 55e872dee..4bd0d2a90 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -26,8 +26,9 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - # Match the version the app ships on (Dockerfile / .nvmrc) - # rather than floating to the latest 24.x. + # Match the version the app ships on (Dockerfile / + # Dockerfile.dev both use 24.18.1) rather than floating + # to the latest 24.x. node-version: '24.18.1' - name: Install dependencies diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f9fbf6fa7..e53630826 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,7 +19,8 @@ jobs: - name: Install Node uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - # Pin to the version the app actually ships on (Dockerfile / .nvmrc). + # Pin to the version the app actually ships on (Dockerfile / + # Dockerfile.dev both use 24.18.1; .nvmrc pins the 24 major). # A bare '24' floats to the latest 24.x; Node >= 24.19.0 added # node::ObjectWrap cleanup hooks that crash better-sqlite3 on teardown # ("Assertion failed: (env) != nullptr" in RemoveEnvironmentCleanupHook), @@ -54,22 +55,28 @@ jobs: - name: Wait for app availability run: | + print_log() { + if [ -f app.log ]; then cat app.log; else echo "(app.log not found)"; fi + } for i in {1..5}; do if curl --silent --fail http://localhost:3002/auth/login; then echo "App is up" exit 0 fi - # Fail fast (and show why) if the process already exited. - if ! kill -0 "$(cat app.pid)" 2>/dev/null; then + # Fail fast (and show why) if the process is gone. Read the PID + # safely: if it's missing/empty (e.g. nohup never started), treat + # that as "not running" instead of passing a bad arg to kill. + pid="$(cat app.pid 2>/dev/null || true)" + if [ -z "$pid" ] || ! kill -0 "$pid" 2>/dev/null; then echo "App process exited before becoming available. Output:" - cat app.log + print_log exit 1 fi echo "Waiting for the app... attempt $i" sleep 5 done echo "App failed to start. Output:" - cat app.log + print_log exit 1 build-sqlite: