Skip to content

fix: move kernel-store to better-sqlite3 13, fix the renovatebot workflow - #1113

Merged
rekmarks-consensys-1 merged 4 commits into
mainfrom
rekmarks-consensys-1-patch-1
Sep 18, 2026
Merged

rekmarks-consensys-1 merged 4 commits into
mainfrom
rekmarks-consensys-1-patch-1

Conversation

@rekmarks-consensys-1

@rekmarks-consensys-1 rekmarks-consensys-1 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Two fixes, folded together because the second is what lets CI run at all.

renovatebot.yml: ubuntu-24.04ubuntu-latest

ubuntu-24.04 is not a valid runner slug here, so the job could not start. The remaining hunks are prettier on the workflow file and renovate.json.

kernel-store: better-sqlite3 12 → 13 (folds in #1045)

On Node 24.20.0, 12.x aborts the process during teardown:

node::RemoveEnvironmentCleanupHook(Isolate*, CleanupHook, void*) at ../src/api/hooks.cc:142
Assertion failed: (env) != nullptr
  Statement::~Statement() [better_sqlite3.node]

The destructor runs after the environment is gone and asserts on the way out. Everything it kills has already passed, so it surfaces as Worker exited unexpectedly rather than as a test failure anyone can read.

Nothing in this repository changed to cause it. CI's matrix floats on 24.x, and had not run since 2026-09-03, when the runner still resolved that to 24.19.0. It now resolves to 24.20.0.

It is not a missing close

The suites that hit this close their databases correctly. Narrowed on 24.20.0:

Repro Result
Plain script, statement alive at exit, no close() clean
Vitest worker + real driver, database left open clean
Vitest worker + database + live worker thread clean
Full integration test aborts
Full integration test on Node 24.18.0 clean

Same code, same test, only the Node patch differs — which places it between the two Node releases rather than in our teardown.

Why 13.x is safe here

It is a major, but it moves none of the surface this driver uses — prepare, run, get, all, iterate, pluck, transaction, exec, close — and its engines.node floor is >=22, the same one this package already declares. The changelog entry is marked breaking for that engine floor alone.

Pinning the Node matrix to a known-good patch would also go green, and would hide this until the next bump. The floating 24.x is a separate reproducibility question worth having on its own terms.

Testing

Verified on Node 24.20.0 with the bump:

  • Unit suite: 53/53 tasks
  • Integration suite: all pass, no abort, no worker timeout
  • kernel-node-runtime e2e: went from 9 failures plus the abort to running normally
  • Node 22.23.2: kernel-store passes

One caveat: remote-comms.test.ts > handles remote intentional disconnect without reconnecting fails locally for me on 24.18.0, 24.19.0 and 24.20.0 alike, with and without this bump. Worth watching in CI, but it is not related to either change here.

🤖 Generated with Claude Code


Note

Medium Risk
The native better-sqlite3 major bump affects install/build and the Node SQLite path for consumers; CI runner change is low risk.

Overview
Updates the renovatebot workflow to run on ubuntu-latest instead of the invalid ubuntu-24.04 slug, with minor YAML comment/quote formatting. renovate.json is collapsed to a single-line extends entry only.

In @metamask/kernel-store, upgrades better-sqlite3 from ^12.4.1 to ^13.0.3 (lockfile included) and documents it as breaking in the changelog: Node 22+ is required (already the package engines floor), mainly to avoid a Node 24 teardown crash in 12.x that can kill test workers after tests pass. No application driver code changes are part of this diff.

Reviewed by Cursor Bugbot for commit 39bb2f2. Bugbot is set up for automated code reviews on this repo. Configure here.

Fix ubuntu version slug.

Signed-off-by: Erik Marks <erik.marks@consensys.com>
… on Node 24.20 (#1045)

## The failure

On Node 24.20.0, `better-sqlite3` 12.x aborts the process during
shutdown:

```
node::RemoveEnvironmentCleanupHook(Isolate*, CleanupHook, void*) at ../src/api/hooks.cc:142
Assertion failed: (env) != nullptr
  Statement::~Statement() [better_sqlite3.node]
```

The destructor runs after the environment is gone and asserts on the way
out. Everything it kills has already passed, so it surfaces as `Worker
exited unexpectedly` rather than as a test failure anyone can read.

Nothing in this repository changed. CI's matrix floats on `24.x`, and
had not run since 2026-09-03, when the runner still resolved that to
**24.19.0**. It now resolves to **24.20.0**.

## It is not a missing `close`

The suites that hit this close their databases correctly. I narrowed it
on 24.20.0:

| Repro | Result |
| --- | --- |
| Plain script, statement alive at exit, no `close()` | clean |
| Vitest worker + real driver, database left open | clean |
| Vitest worker + database + live worker thread | clean |
| Full integration test | **aborts** |
| Full integration test on Node 24.18.0 | clean |

Same code, same test, only the Node patch differs — which places it
between the two Node releases rather than in our teardown.

## The fix

13.x is a major, but it moves none of the surface this driver uses —
`prepare`, `run`, `get`, `all`, `iterate`, `pluck`, `transaction`,
`exec`, `close` — and its `engines.node` floor is `>=22`, the same one
this package already declares.

Verified on Node 24.20.0 with the bump:

- Unit suite: 53/53 tasks
- Integration suite: all pass, no abort, no worker timeout
- `kernel-node-runtime` e2e: went from **9 failures plus the abort** to
running normally
- Node 22.23.2: kernel-store passes

## One caveat, so nobody is surprised

`remote-comms.test.ts > handles remote intentional disconnect without
reconnecting` fails locally for me, and it does so on **24.18.0, 24.19.0
and 24.20.0 alike**, with and without this bump. It is flaky in my
environment rather than related to either the Node bump or this change —
I initially misread it as a second Node regression, then reproduced the
failure on 24.18.0 twice in a row. Worth watching in CI, but it is not
what this PR is about.

## Why not pin Node instead

Pinning the matrix to a known-good patch would go green too, and hide
this until the next time someone bumps Node. The floating `24.x` is a
separate reproducibility question worth having on its own terms; this is
the actual defect.

Needs #1043 to land before CI here can run at all.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Major bump of a native SQLite dependency in the storage layer; API
usage is unchanged but install/build and runtime behavior depend on the
new binary and Node 22+.
> 
> **Overview**
> **Upgrades `@metamask/kernel-store` to `better-sqlite3` ^13.0.3** so
Node **24.20** no longer aborts during process teardown
(`Statement::~Statement` / cleanup-hook assert with 12.x), which was
showing up as unexplained Vitest worker deaths after tests passed.
> 
> The changelog marks this as **BREAKING** for the dependency (Node
≥22); that matches the package’s existing `engines` floor. No changes to
the SQLite driver wrapper—only `package.json`, lockfile resolution
(13.0.3, updated native build deps), and changelog notes.
> 
> Incidental edits: **renovatebot** workflow uses `ubuntu-latest`
instead of `ubuntu-24.04`, plus small YAML/JSON formatting in that
workflow and `renovate.json`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
b79417a. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Erik Marks <erik.marks@consensys.com>
@socket-security

socket-security Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedbetter-sqlite3@​12.4.6 ⏵ 13.0.3100 +1210010090100

View full report

@rekmarks-consensys-1 rekmarks-consensys-1 changed the title Fix ubuntu version slug in workflows/renovatebot.yml fix: move kernel-store to better-sqlite3 13, fix the renovatebot workflow Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 73.05%
🟰 ±0%
9790 / 13401
🔵 Statements 72.9%
🟰 ±0%
9954 / 13653
🔵 Functions 73.6%
🟰 ±0%
2301 / 3126
🔵 Branches 67.27%
🟰 ±0%
4023 / 5980
File CoverageNo changed files found.
Generated in workflow #4833 for commit 39bb2f2 by the Vitest Coverage Report Action

@rekmarks-consensys-1
rekmarks-consensys-1 added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit 413f0a6 Sep 18, 2026
50 of 54 checks passed
@rekmarks-consensys-1
rekmarks-consensys-1 deleted the rekmarks-consensys-1-patch-1 branch September 18, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants