ci: get CI running again - #1043
Closed
sirtimid wants to merge 3 commits into
Closed
Conversation
The org now refuses any action referenced by tag:
The action actions/checkout@v7 is not allowed in
Consensys-Incorporated/ocap-kernel because all actions must be pinned
to a full-length commit SHA.
That is a setup-job failure, so it takes down every job that references
one before a step runs. Nothing has run green in this repository since
2026-09-03; the policy was turned on after the last commit that touched
CI, which is why merging main does not help.
Each tag is replaced by the commit it currently resolves to, with the
release it names in a trailing comment. Dependabot reads that comment,
so `github-actions` updates keep working and arrive as a SHA bump.
Local `./.github/...` references are exempt and left alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimid
marked this pull request as draft
September 8, 2026 13:55
…g out for them
Pinning our own references was necessary but not sufficient. The policy
resolves actions transitively, and the MetaMask actions we call reach mutable
tags one layer down:
action-checkout-and-setup -> cache/restore@v6, checkout@v7, setup-node@v6,
action-retry-command@v1, cache/save@v6
github-tools/check-changelog -> checkout@v6 x2, setup-node@v6
github-tools/check-skip-merge-queue -> github-script@v9 x2
All three are on their latest releases, so no upgrade reaches them, and they
live in an organization we no longer belong to. A pin one layer deep is no pin
at all, so the only way to make the chain true is to hold it.
`action-checkout-and-setup` becomes an explicit `actions/checkout` plus a local
`setup-environment` action -- Node, Corepack, Yarn's download cache, install.
Checkout cannot live inside it: `uses: ./...` resolves against the workspace,
so the repository has to be on disk before the runner can find the file.
The local action is narrower than the one it replaces. It drops the
node_modules cache and the lookup-only fast path, keeping only Yarn's download
cache, because those turn on a cache key that has to account for the native
rebuilds `postinstall` runs. `yarn install` runs every job now, which costs
time and buys the guarantee that what is on disk matches the lockfile.
`check-changelog` cloned github-tools to run a script over the diff. The local
job asks the same question directly: every package with a CHANGELOG that this
pull request touches must have that changelog in the diff too. It compares
against the merge base rather than the base tip, so commits landing on main
afterwards are not read as this branch's. `no-changelog` still opts out.
`check-skip-merge-queue` is ported as-is, its two scripts unchanged, onto a
pinned `actions/github-script`.
The release path still calls `action-npm-publish` and `action-is-release`,
which have the same defect. Both are gated on `push`, so they block releases
rather than pull requests, and neither is part of the `all-jobs-complete` gate.
Publishing is where a mistake is expensive and OIDC permissions were only just
fixed in #1041, so they are better handled upstream than rewritten here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimid
changed the title
… on Node 24.20
On Node 24.20.0, `better-sqlite3` 12.x aborts the process while it is shutting
down:
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 reads as a worker dying rather
than as a test failure, and vitest reports only that the fork exited.
Not a missing `close`. The suites that hit it close their databases; a plain
script, a vitest worker holding an open database, and one holding an open
database plus a live worker thread are all clean on 24.20.0. It takes the whole
kernel to provoke, and the same code on 24.18.0 is clean, which places it
between the two Node releases rather than in this repository.
The tests never ran into it because CI floats on `24.x` and had not run since
2026-09-03, when the runner still resolved that to 24.19.0.
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 Node floor is the one this package already sets. Verified on 24.20.0: the
unit suite, the integration suite, and the `kernel-node-runtime` e2e all run
without the abort, and kernel-store passes on 22.23.2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
sirtimid
changed the title
sirtimid
marked this pull request as ready for review
September 8, 2026 16:04
Contributor
Coverage Report
File CoverageNo changed files found. |
rekmarks-consensys-1
requested changes
Sep 8, 2026
rekmarks-consensys-1
left a comment
rekmarks-consensys-1
left a comment
Contributor
There was a problem hiding this comment.
Let me see if we can fix this at the policy level before proceeding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI has been red on every PR since the org move. Last green run was #1042 on 2026-09-03. Three things were wrong; this fixes all three.
1. Actions must be pinned to a SHA
Consensys-Incorporatedrequires it,MetaMaskdidn't. It fails at job setup, before anything runs:Merging
maindoesn't help —mainis broken the same way. All 35 references now name a commit, with the version in a trailing comment so Dependabot keeps working. No version changes.2. The policy applies to nested actions too
Pinning ours wasn't enough: the MetaMask composite actions we call reach mutable tags one layer down, and they're on their latest releases, so there's no upgrade that fixes it.
We now own those steps instead:
action-checkout-and-setup→actions/checkout+ a localsetup-environmentaction (Node, Corepack, Yarn cache, install)check-changelog→ a local job: every package you touch that has aCHANGELOG.mdneeds it in the diff.no-changelogstill opts outcheck-skip-merge-queue→ ported as-is onto a pinnedactions/github-scriptThe release path still uses
action-npm-publishandaction-is-release, which have the same problem. Both only run onpush, so they block releases, not PRs. Better fixed upstream than rewritten here.3. better-sqlite3 crashes on Node 24.20
Once CI actually ran, tests aborted:
Not a missing
close()— a plain script, a vitest worker holding an open database, and one with a live worker thread are all fine on 24.20.0. It takes the whole kernel to provoke, and the same code is clean on 24.18.0.Our matrix floats on
24.x, which resolved to 24.19.0 in September and 24.20.0 now. Bumping tobetter-sqlite313.0.3 fixes it. The major touches none of the API this driver uses, and its Node floor is the one kernel-store already declares.Checked
actionlint1.6.25 (same version CI uses) clean; no unpinned externaluses:leftkernel-node-runtimee2e went from 9 failures + abort to runningOne flaky test to be aware of:
remote-comms.test.ts > handles remote intentional disconnect without reconnectingfails locally for me on 24.18.0, 24.19.0 and 24.20.0 alike, with and without the bump. Unrelated to this PR, but worth watching.Nothing else can go green until this merges.
🤖 Generated with Claude Code
Note
Medium Risk
Broad CI workflow changes gate every PR and merge queue; kernel-store’s major native dependency bump affects runtime behavior on supported Node versions, while release still depends on upstream MetaMask actions (SHA-pinned only at the call site).
Overview
Restores CI for the org’s full SHA pinning policy by replacing every mutable
uses:tag with a commit SHA (version kept in comments for Dependabot).MetaMask composite actions that still pulled unpinned
checkout/setup-node/cache/github-scriptinternally are replaced with local steps: pinnedactions/checkoutplussetup-environment(Node, Corepack, Yarn download cache,yarn install --immutableevery run—nonode_modulescache), an inlined changelog job (merge-base diff, per-packageCHANGELOG.md,no-changeloglabel), and a portedcheck-skip-merge-queueaction. Release workflows still call pinned MetaMask publish/is-release actions; publish jobs passcache: falseon setup where credentials matter.Separately,
@metamask/kernel-storebumpsbetter-sqlite3^12.4.1→^13.0.3(documented as breaking for consumers on Node < 22) to stop process aborts on Node 24.20 during native teardown in CI tests.Reviewed by Cursor Bugbot for commit 905a499. Bugbot is set up for automated code reviews on this repo. Configure here.