ci: cross-Node-major install canary + strict postinstall opt-in#208
Conversation
PR #206 introduced a tree-sitter native-dep heal that turned a hard install failure on Node 24 into a silent "WARNING" + exit 0. Because ci.yaml only ran on Node 22 (where tree-sitter's linux-x64 prebuild ABI matches and the heal short-circuits), the regression was invisible at PR time and only surfaced post-merge on release.yaml (pinned to Node 24.12), where it broke v0.7.55 mid-release. Close the detection gap with two pieces: 1. New `cross-node-install` job in ci.yaml — matrix [22, 24], runs `npm install && npm run build` only (no full test suite — the bug class is install/build-time, not runtime). Marked `continue-on-error: true` so PRs are not blocked while the known tree-sitter@0.21 + Node 24 break is being resolved; the job is still visible in the PR checks table as yellow/red. Flip to `continue-on-error: false` once the underlying incompatibility is fixed (or matrix entries adjusted). 2. Opt-in strict mode for scripts/ensure-tree-sitter.mjs — when HIVEMIND_STRICT_POSTINSTALL=1 is set (only by this repo's own CI), the "bindings still unavailable" warning becomes `exit 1` instead of `exit 0`. Downstream consumers of @deeplake/hivemind never see this flag, so their install stays non-fatal as before. The strict path turns a buried tsc "Cannot find module 'tree-sitter'" failure 30 seconds later into an immediate, labeled install-time error. The canary alone would have caught PR #206's regression at the typecheck step (tsc fails to resolve `tree-sitter`); strict mode makes the failure faster and more diagnostic. Follow-up: once Node 24 install is healthy, also align release.yaml + publish-smoke-test.yaml to set HIVEMIND_STRICT_POSTINSTALL=1 and add Node 26 to the matrix when it lands.
📝 WalkthroughWalkthroughThis PR introduces an opt-in strict postinstall mode for tree-sitter native bindings, controlled by the ChangesStrict postinstall mode and Node canary
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage ReportNo Generated for commit 5e1d795. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 321-326: Replace the tag refs with pinned commit SHAs for the
GitHub Actions used and disable checkout credential persistence: update the
checkout step referencing actions/checkout (currently actions/checkout@v6.0.2)
to use a specific commit SHA and add persist-credentials: false, and similarly
pin actions/setup-node (currently actions/setup-node@v6.4.0) to its commit SHA;
ensure the steps named "actions/checkout" and "Setup Node.js"/uses
"actions/setup-node" are updated accordingly to use commit SHAs and the checkout
step includes persist-credentials: false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f550200-8200-4710-8cae-6d6434e03ef5
📒 Files selected for processing (2)
.github/workflows/ci.yamlscripts/ensure-tree-sitter.mjs
| - uses: actions/checkout@v6.0.2 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v6.4.0 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced workflow segment
sed -n '260,360p' .github/workflows/ci.yaml | nl -ba | sed -n '1,160p'
# Search for persist-credentials usage and whether checkout steps are pinned elsewhere
rg -n "persist-credentials" .github/workflows/ci.yaml .github/workflows || true
rg -n "uses:\s*actions/(checkout|setup-node)@" .github/workflows/ci.yaml .github/workflows || true
rg -n "actions/checkout@v|actions/setup-node@v" .github/workflows/ci.yaml .github/workflows || true
# Check if the job is named cross-node-install and capture its bounds
rg -n "cross-node-install" .github/workflows/ci.yamlRepository: activeloopai/hivemind
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
WF=".github/workflows/ci.yaml"
# Show the referenced region with line numbers (no nl dependency)
awk 'NR>=260 && NR<=360 {printf "%6d %s\n", NR, $0}' "$WF"
# Search for persist-credentials usage
rg -n "persist-credentials" .github/workflows/ci.yaml .github/workflows || true
# Search for unpinned checkout/setup-node usage patterns in workflows
rg -n "uses:\s*actions/(checkout|setup-node)@" .github/workflows/ci.yaml .github/workflows || true
# Search for other actions/checkout/setup-node with SHA pinning elsewhere
rg -n "uses:\s*actions/(checkout|setup-node)@[0-9a-f]{7,40}" .github/workflows/ci.yaml .github/workflows || true
# Locate the job block for cross-node-install (for context)
rg -n "cross-node-install" .github/workflows/ci.yamlRepository: activeloopai/hivemind
Length of output: 6051
Pin GitHub Actions and disable checkout credential persistence in cross-node-install
- In
.github/workflows/ci.yaml(cross-node-install),actions/checkout@v6.0.2andactions/setup-node@v6.4.0use tag refs instead of commit SHAs. - Update the checkout step to set
persist-credentials: falsefor least-privilege CI.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 321-321: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 321-321: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 324-324: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yaml around lines 321 - 326, Replace the tag refs with
pinned commit SHAs for the GitHub Actions used and disable checkout credential
persistence: update the checkout step referencing actions/checkout (currently
actions/checkout@v6.0.2) to use a specific commit SHA and add
persist-credentials: false, and similarly pin actions/setup-node (currently
actions/setup-node@v6.4.0) to its commit SHA; ensure the steps named
"actions/checkout" and "Setup Node.js"/uses "actions/setup-node" are updated
accordingly to use commit SHAs and the checkout step includes
persist-credentials: false.
There was a problem hiding this comment.
Thanks for the flag — this is a real supply-chain hardening best-practice, but I'm going to leave PR #208 as-is and not apply the suggestion here. Reasoning:
- Consistency with existing repo convention. Every other
actions/*reference inci.yaml,release.yaml, andpublish-smoke-test.yamluses tag refs (@v6.0.2,@v6.4.0,@v4.0.0, …). The newcross-node-installjob is following that established pattern. - Partial pinning provides no real benefit. A compromised
actions/checkout@v6.0.2re-tag would still hit the other unprotected steps across the repo — pinning only the new job hardens nothing. - Right scope is a separate PR. SHA-pinning +
persist-credentials: falseis a repo-wide hygiene change. Mixing it into a CI canary PR creates noisy review and risks divergent state if the broader rollout stalls.
Tracking this as a follow-up for a dedicated hardening PR that covers all workflow files at once. Will tag back here when that lands.
There was a problem hiding this comment.
@efenocchi, that's a well-reasoned call — partial pinning in isolation doesn't move the security needle when the rest of the repo is unpinned, and a dedicated hardening PR is the right scope for this. Happy to open a tracking GitHub issue for the repo-wide SHA-pinning + persist-credentials: false rollout if that would help keep it from falling through the cracks. Just let me know.
🐇 (_) noted and stepping aside
✏️ Learnings added
Learnt from: efenocchi
Repo: activeloopai/hivemind PR: 208
File: .github/workflows/ci.yaml:321-326
Timestamp: 2026-05-26T17:14:33.852Z
Learning: In the activeloopai/hivemind repository, all GitHub Actions references in ci.yaml, release.yaml, and publish-smoke-test.yaml intentionally use tag refs (e.g., actions/checkoutv6.0.2, actions/setup-nodev6.4.0) rather than commit SHA pins. This is a known gap that the maintainers plan to address in a dedicated repo-wide hardening PR rather than per-job. Do not flag individual jobs for not using SHA-pinned actions until that dedicated PR lands.
Learnt from: efenocchi
Repo: activeloopai/hivemind PR: 208
File: .github/workflows/ci.yaml:321-326
Timestamp: 2026-05-26T17:14:33.852Z
Learning: In the activeloopai/hivemind repository, persist-credentials: false is not set on any checkout step across workflow files (ci.yaml, release.yaml, publish-smoke-test.yaml). The maintainers are aware and tracking a future repo-wide hardening PR to add SHA pinning and persist-credentials: false to all workflow files simultaneously.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
Why
PR #206 (
fix(deps): build tree-sitter from source on linux-arm64 / Node >=22) introduced a postinstall heal that turned a hard install failure on Node 24 into a silentWARNING: tree-sitter bindings still unavailable+exit 0. Becauseci.yamlruns only on Node 22 — where the linux-x64 prebuild ABI matches and the heal short-circuits cleanly — the regression was invisible at PR time and only surfaced post-merge in theReleaseworkflow (run 26430516153), which was pinned to Node 24.12. That broke v0.7.55 mid-release.The unblock is in #207 (pin release.yaml to Node 22). This PR closes the detection gap so the next regression of this class fails on the PR instead of on
main.What
Two pieces:
1. New
cross-node-installjob inci.yaml[22, 24]— every Node major theenginesfield (>=22.0.0) admitsnpm install && npm run buildonly — the bug class is install/build-time, not runtime, so running the full test suite on every matrix slot doubles CI cost for no extra signalcontinue-on-error: true— keeps PRs unblocked while the known upstream tree-sitter@0.21 + Node 24 incompatibility is being resolved. Signal is still visible in the PR checks table as yellow/red. Flip tofalseonce Node 24 install is healthy.2. Opt-in strict mode for
scripts/ensure-tree-sitter.mjsHIVEMIND_STRICT_POSTINSTALL=1(set by this repo's own CI workflows) turnsWARNING: bindings still unavailableintoexit 1instead ofexit 0@deeplake/hivemindnever see this flag, so their install stays non-fatal as beforetsc: Cannot find module 'tree-sitter'30 seconds later into an immediate, labeled install-time failureHow this would have caught #206 → #207
PR #206 in the new canary's Node 24 slot:
npm installruns the postinstall healWARNING+HIVEMIND_STRICT_POSTINSTALL=1→exit 1[ensure-tree-sitter] WARNING: ... (strict mode — failing this install)Even with strict mode off, the next step (
npm run build) would have failed tsc withCannot find module 'tree-sitter'— same failure as the Release run, but on the PR.Test plan
test(Node 22) andduplicationjobs unaffectedensure-tree-sitter.mjsexits 1 only when bothHIVEMIND_STRICT_POSTINSTALL=1ANDbindingsLoad() === false; healthy bindings still exit 0Follow-up (not in this PR)
release.yamlandpublish-smoke-test.yamlenv to also setHIVEMIND_STRICT_POSTINSTALL=1once ci(release): pin release + publish jobs to Node 22 #207 landscontinue-on-errortofalseto make Node 24 install a hard gateSummary by CodeRabbit