ci: put every Node the project chooses on 26, and assert it from one file - #7967
Conversation
…file .node-version (26.5.1) is the authoritative oracle, but the pin has leaked twice since #6367 made it single: CLAUDE.md's prose drifted off the file (#7599), and npm-launcher.yml was created by #6350 on the SAME DAY #6367 converted every existing workflow, keeping that day's ambient "22.23.1" literal by omission rather than by decision. - npm-launcher.yml (x2): "22.23.1" -> node-version-file: .node-version. It runs npm/perry/test/detect.test.cjs, which exercises the shipped launcher logic every installing user hits, so its Node is a behavioural input. Safe on the ubuntu-22.04 job: Node 26 needs glibc >= 2.28 and jammy has 2.35. - release-packages.yml: "20" -> "26". Node 20 reached EOL on 2026-04-30 and this is the repo's most privileged job (id-token: write, OIDC-publishes every platform package). - release-hono-server.yml: "24" -> "26". Both release workflows stay pinned to a bare MAJOR rather than node-version-file: they are publishing toolchains, and a gap-suite oracle bump must never be able to move the runtime that publishes releases. New scripts/check_node_version_consistency.py, wired as a lint step (a required context). It re-derives every restatement of a Node version from the file it quotes, and requires every literal node-version: in a workflow to be a registered exemption with a reason. Exemptions are asserted against the tree, so one that stops matching FAILS and must be updated or deleted. Reverting npm-launcher.yml to "22.23.1" reproduces the historical bug as two named failures. --self-test proves each rule can fail; both vacuity floors can fail too. Not changed, deliberately: test-compat/node-core/pinned-version.txt (v22.x runs Node's own corpus) and benchmarks/public-baseline-config.json (v22.23.1). The latter is in public_baseline.HARNESS_PATHS, so editing it alone reddens the required lint job -- measured, ci_public_baseline_check.py exits 2 with "benchmark harness changed". The pin and its ~2 h measurement are atomic by design (#7282/#7958) and the regeneration needs the quiet M1 mini, so it is registered as a self-clearing exemption carrying the runbook instead. Node 26 is faster than Node 22, so that regeneration is expected to reduce Perry's published advantage. Also: CLAUDE.md said "Two workflows are deliberately exempt" and then listed three, which is probably why the fourth pin read as accounted-for; and external-tools.json told readers to bump a NODE_PIN constant that does not exist in node_compat_matrix.mjs (it reads external-tools.json). Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
📝 WalkthroughWalkthroughThe change standardizes Node.js workflow versions around Node 26 and ChangesNode 26 standardization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LauncherWorkflow
participant ReleaseWorkflow
participant TestWorkflow
participant NodeVersionChecker
LauncherWorkflow->>LauncherWorkflow: Read .node-version
ReleaseWorkflow->>ReleaseWorkflow: Use Node 26 for npm publishing
TestWorkflow->>NodeVersionChecker: Run self-tests and consistency checks
NodeVersionChecker-->>TestWorkflow: Return diagnostics and exit status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/release-hono-server.yml:
- Around line 32-36: Update the workflow comment near the npm publishing
configuration to remove the claim that registry-url contradicts OIDC or writes a
conflicting token-based .npmrc. State instead that registry-url is unnecessary
because registry.npmjs.org is npm’s default registry, while preserving the
existing Node/npm version guidance.
In `@changelog.d/7967-node26-everywhere.md`:
- Line 1: Revise the release-note entry to state that Node 26 is the default
workflow version and authoritative oracle, while explicitly documenting Node 22
exemptions for test-compat/node-core/pinned-version.txt and
benchmarks/public-baseline-config.json. Describe the final behavior coherently
without claiming every project-selected Node version is 26.
In `@scripts/check_node_version_consistency.py`:
- Around line 273-293: Update the exemption handling in the workflow-checking
function around registered and _node_version_keys so exemptions are matched by
both workflow path and literal Node value, not path alone; only suppress a
failure when the exact (path, value) pair is registered. In the self-test
section at lines 361-395 of scripts/check_node_version_consistency.py, add
coverage for a workflow containing exempt "26" and unregistered "18" pins,
ensuring the latter still fails.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 22a78191-0c3e-44ed-b743-7981f8559f45
📒 Files selected for processing (9)
.github/workflows/npm-launcher.yml.github/workflows/release-hono-server.yml.github/workflows/release-packages.yml.github/workflows/test.ymlCLAUDE.mdchangelog.d/7967-node26-everywhere.mdexternal-tools.jsongc-handoff/NODE26-NOTES.mdscripts/check_node_version_consistency.py
| # Node 26 ships npm 11.x. OIDC Trusted Publishing needs npm >= 11.5.1; | ||
| # Node 22's npm 10.x silently fails the OIDC handshake and the registry | ||
| # returns a misleading 404 on PUT. Do NOT set `registry-url` here — it | ||
| # writes a token-based .npmrc that contradicts OIDC (the publish targets | ||
| # the default registry.npmjs.org regardless). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file=".github/workflows/release-hono-server.yml"
printf '%s\n' "== workflow excerpt =="
sed -n '20,45p' "$file"
printf '%s\n' "== setup-node references =="
rg -n -C 3 'setup-node|registry-url|trusted|OIDC|npm' .github/workflows "$file" 2>/dev/null || trueRepository: PerryTS/perry
Length of output: 50369
🌐 Web query:
actions/setup-node@v7 registry-url npm trusted publishing OIDC documentation
💡 Result:
In actions/setup-node@v7, using Trusted Publishing (OIDC) to publish to npm requires specific configuration to replace long-lived npm tokens with short-lived OIDC tokens [1][2]. Requirements: - npm version: You must use npm ≥ 11.5.1 [2]. - Node.js: Node.js 24 or newer is recommended as it includes a compatible version of npm [2]. - npm Trusted Publisher configuration: You must configure a Trusted Publisher in npm that matches your GitHub repository, workflow, and optional environment [1][2]. Workflow Configuration: To enable OIDC, you must explicitly grant the id-token: write permission in your GitHub Actions workflow [1][2]. Unlike classic token-based authentication, OIDC does not require or use the NODE_AUTH_TOKEN secret during the publish step [3][4]. Example Workflow Snippet: permissions: contents: read id-token: write # Required for OIDC steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 with: node-version: '24' registry-url: 'https://registry.npmjs.org' # Recommended to disable automatic caching if not strictly needed # to mitigate potential cache poisoning risks package-manager-cache: false - run: npm ci - run: npm run build --if-present - run: npm publish Important Considerations: - Avoiding Token Conflicts: In v7.0.0, the action no longer exports a dummy NODE_AUTH_TOKEN [5]. This change prevents issues where the action might inadvertently attempt classic authentication when only OIDC is intended, which previously caused some workflows to fail with ENEEDAUTH or E404 [3][6]. - Security Note: When using Trusted Publishing, it is recommended to set package-manager-cache: false to reduce the risk of cache poisoning, as cached data can expose credentials to attacker-controlled code [1][2]. - Matching Identity: Ensure your GitHub repository, workflow file path, and environment (if used) match exactly what is defined in the npm Trusted Publisher settings, or the publish may fail with an E404 error [1][2].
Citations:
- 1: https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md
- 2: actions/setup-node@670825a
- 3: https://github.com/actions/setup-node?tab=readme-ov-file
- 4: https://docs.npmjs.com/trusted-publishers/
- 5: https://github.com/actions/setup-node/releases/tag/v7.0.0
- 6: registry-url writes _authToken line that breaks npm Trusted Publisher OIDC when no NODE_AUTH_TOKEN is set actions/setup-node#1551
🌐 Web query:
site:docs.npmjs.com trusted publishers GitHub Actions registry-url npm
💡 Result:
When using Trusted Publishers with GitHub Actions to publish packages to npm, you do not need to configure the registry-url in the same way you would with traditional access tokens [1][2]. Trusted publishing uses OpenID Connect (OIDC) to authenticate automatically, eliminating the need for long-lived NPM_TOKEN secrets [2][3]. For your GitHub Actions workflow, follow these requirements: 1. Permissions: You must explicitly grant your workflow the ability to generate an OIDC token by setting the id-token permission to write [1]. permissions: id-token: write contents: read 2. Setup: Use actions/setup-node to prepare your environment. While you can still specify the registry-url if needed for other operations, the publishing step will use the trusted OIDC relationship automatically [1]. - uses: actions/setup-node@v6 with: node-version: '24' registry-url: 'https://registry.npmjs.org' 3. Publishing: Simply run npm publish in your workflow [1]. You do not need to provide an authentication token as an environment variable or via an.npmrc file, as the npm CLI will detect the OIDC token provided by the GitHub Actions environment [1][2]. To configure the trust relationship, use the npm trust command locally on your machine before pushing your workflow: npm trust github [package-name] --file [workflow-filename.yml] --repo [owner/repo] --allow-publish [4][5]. Using Trusted Publishers is the recommended method for publishing from CI/CD, as it improves security by removing the need for long-lived tokens and enables the generation of provenance statements for your packages [2][3].
Citations:
- 1: https://docs.npmjs.com/trusted-publishers/
- 2: https://docs.npmjs.com/using-private-packages-in-a-ci-cd-workflow/
- 3: https://docs.npmjs.com/creating-and-publishing-unscoped-public-packages/
- 4: https://docs.npmjs.com/cli/v12/commands/npm-trust/
- 5: https://docs.npmjs.com/cli/v11/commands/npm-trust/
Correct the registry-url comment. registry-url does not contradict npm Trusted Publishing. State that it is unnecessary because registry.npmjs.org is the default registry.
🤖 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/release-hono-server.yml around lines 32 - 36, Update the
workflow comment near the npm publishing configuration to remove the claim that
registry-url contradicts OIDC or writes a conflicting token-based .npmrc. State
instead that registry-url is unnecessary because registry.npmjs.org is npm’s
default registry, while preserving the existing Node/npm version guidance.
Source: MCP tools
| @@ -0,0 +1,19 @@ | |||
| **Every Node the project chooses is now 26, and one file is asserted to be the source (#7967).** `.node-version` (26.5.1) has been the authoritative oracle since #6367, but the pin leaked twice: CLAUDE.md's prose drifted off the file (#7599), and `.github/workflows/npm-launcher.yml` was created by #6350 on the *same day* #6367 converted every workflow that existed, keeping that day's ambient `"22.23.1"` literal by omission rather than by decision. CLAUDE.md compounded it by saying "**Two** workflows are deliberately exempt" and then listing three, which is why the fourth pin read as accounted-for. | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the release-note claim.
“Every Node the project chooses is now 26” is false. This PR deliberately retains Node 22 for test-compat/node-core/pinned-version.txt and benchmarks/public-baseline-config.json. State that Node 26 is the default workflow and oracle policy, with documented Node 22 exemptions.
Based on learnings: “describe the final shipped behavior as one coherent release-note entry.”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: The official name of this software platform is spelled with a capital “H”.
Context: ...prose drifted off the file (#7599), and .github/workflows/npm-launcher.yml was created...
(GITHUB)
🤖 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 `@changelog.d/7967-node26-everywhere.md` at line 1, Revise the release-note
entry to state that Node 26 is the default workflow version and authoritative
oracle, while explicitly documenting Node 22 exemptions for
test-compat/node-core/pinned-version.txt and
benchmarks/public-baseline-config.json. Describe the final behavior coherently
without claiming every project-selected Node version is 26.
Source: Learnings
| registered = {e.path for e in exemptions} | ||
| if workflows is None: | ||
| workflows = sorted((REPO / ".github/workflows").glob("*.yml")) | ||
| workflow_pins = 0 | ||
| for wf in workflows: | ||
| rel = wf.relative_to(REPO).as_posix() | ||
| for lineno, key, value in _node_version_keys(wf.read_text(encoding="utf-8")): | ||
| workflow_pins += 1 | ||
| if key == "node-version-file": | ||
| if value != GAP_ORACLE: | ||
| failures.append( | ||
| f"{rel}:{lineno}: node-version-file is {value!r}, " | ||
| f"must be {GAP_ORACLE!r}" | ||
| ) | ||
| continue | ||
| if rel not in registered: | ||
| failures.append( | ||
| f"{rel}:{lineno}: literal node-version {value!r} is not registered. " | ||
| f"Use `node-version-file: {GAP_ORACLE}`, or add an exemption with a " | ||
| f"reason to {Path(__file__).name}" | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 scripts/check_node_version_consistency.py --list
python3 scripts/check_node_version_consistency.py --self-test
python3 scripts/check_node_version_consistency.pyRepository: PerryTS/perry
Length of output: 2639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' scripts/check_node_version_consistency.py
sed -n '250,310p' scripts/check_node_version_consistency.py
sed -n '330,410p' scripts/check_node_version_consistency.pyRepository: PerryTS/perry
Length of output: 13482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
registered = {".github/workflows/release-packages.yml"}
pins = [("release-packages.yml", "26"), ("release-packages.yml", "18")]
path_only_failures = [
(path, value) for path, value in pins if path not in registered
]
value_bound_failures = [
(path, value) for path, value in pins
if (path, value) not in {(path, "26") for path in registered}
]
print("path-only failures:", path_only_failures)
print("value-bound failures:", value_bound_failures)
assert not path_only_failures
assert value_bound_failures == [("release-packages.yml", "18")]
PY
rg -n -C 3 'node-version:|node-version-file:' .github/workflows/release-packages.yml .github/workflows/release-hono-server.ymlRepository: PerryTS/perry
Length of output: 412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
registered = {".github/workflows/release-packages.yml"}
pins = [
(".github/workflows/release-packages.yml", "26"),
(".github/workflows/release-packages.yml", "18"),
]
exemptions = {
(".github/workflows/release-packages.yml", "26"),
}
path_only_failures = [
(path, value) for path, value in pins if path not in registered
]
value_bound_failures = [
(path, value) for path, value in pins if (path, value) not in exemptions
]
print("path-only failures:", path_only_failures)
print("value-bound failures:", value_bound_failures)
assert path_only_failures == []
assert value_bound_failures == [
(".github/workflows/release-packages.yml", "18")
]
PY
rg -n -C 3 'node-version:|node-version-file:' \
.github/workflows/release-packages.yml \
.github/workflows/release-hono-server.ymlRepository: PerryTS/perry
Length of output: 1423
Match each workflow exemption to its literal Node value.
registered stores only workflow paths. A registered workflow can therefore contain an unregistered node-version literal without failure. Bind exemptions to (path, value) and add a self-test with an exempt "26" pin plus an unregistered "18" pin.
📍 Affects 1 file
scripts/check_node_version_consistency.py#L273-L293(this comment)scripts/check_node_version_consistency.py#L361-L395
🤖 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 `@scripts/check_node_version_consistency.py` around lines 273 - 293, Update the
exemption handling in the workflow-checking function around registered and
_node_version_keys so exemptions are matched by both workflow path and literal
Node value, not path alone; only suppress a failure when the exact (path, value)
pair is registered. In the self-test section at lines 361-395 of
scripts/check_node_version_consistency.py, add coverage for a workflow
containing exempt "26" and unregistered "18" pins, ensuring the latter still
fails.
Source: Coding guidelines
1 participant
What
.node-version(26.5.1) is the authoritative oracle, but the pin has leaked twice since #6367 made it single. This puts every Node the project chooses on 26 and replaces the prose that was supposed to keep them aligned with a check.Inventory, with a verdict per entry
.node-versionexternal-tools.jsontools.node.versionnpm-launcher.yml(x2)"22.23.1"node-version-file: .node-versionrelease-packages.yml"20""26"release-hono-server.yml"24""26"test-compat/node-core/pinned-version.txtv22.xbenchmarks/public-baseline-config.jsonv22.23.1npm-launcher.ymlis the unaccounted-for one, and it is driftCLAUDE.md named two deliberate exemptions but there were three workflow literals. Provenance settles it:
4f50d797c(2026-07-13) creatednpm-launcher.ymlwith"22.23.1"— PR fix(npm): run on glibc older than the builder's — route Ubuntu 22.04 / Debian 12 / RHEL 9 to the static build (#6298) #6350.db4b068d8(2026-07-13, same day) is ci: standardize the gap-suite Node oracle on 26.5.0 (single .node-version pin) #6367, which introduced.node-versionand converted every workflow. Its commit message enumerates the exemptions it granted:node-core-subset.yml,release-hono-server.yml,release-packages.yml.npm-launcher.ymlis not among them — it did not exist when ci: standardize the gap-suite Node oracle on 26.5.0 (single .node-version pin) #6367 was written.So
22.23.1was that day's ambient Node, frozen by omission. It is also behaviour-sensitive: thedetect-self-testjob runsnode npm/perry/test/detect.test.cjs, exercisingnpm/perry/bin/detect.cjs— shipped JS that every installing user executes.No blocker on the
ubuntu-22.04job: Node 26 requires glibc >= 2.28 (nodejs/nodev26.xBUILDING.md, Tier 1 linux x64/arm64); jammy ships 2.35.The two release workflows stay pinned to a bare major rather than
node-version-fileon purpose: they are publishing toolchains, and a gap-suite oracle bump must never be able to move the runtime that publishes releases.The one I did not change, and why
benchmarks/public-baseline-config.jsonpinsv22.23.1for the published performance baseline — Perry's public "faster than Node" numbers are measured against a runtime four majors old. It is pre-existing (#6376, regenerated #7285); #7958 only made it declarative.It cannot be edited on its own. The file is in
public_baseline.HARNESS_PATHS, so its bytes feed the artifact'sharness_fingerprint; andvalidate_public()separately compares the config against the version recorded inbenchmarks/results/public-node-bun-v1.json. Measured, not assumed — pin flipped tov26.5.1, checker run, file restored:That checker is the "Public benchmark evidence freshness" step of the
lintjob — a required context. So shipping the pin edit alone is not "stale but honest", it is a required gate red formainand every open PR. The pin and its measurement are atomic by design (#7282/#7958).It is therefore registered as a self-clearing exemption in the new checker, carrying the full runbook. When the baseline is regenerated the config value moves, the exemption stops matching, and the check fails until the entry is deleted — so this cannot quietly become a fossil.
Regeneration required, and it cannot happen here
Host must be
perry@perry-macos.local(the Apple M1 / 8-core / 8 GB mini recorded in the artifact'shostblock) — the artifact pins host identity, so regenerating elsewhere replaces the baseline rather than updating it. ~2 h, five components, each gated on<=25%CPU active for 60 consecutive seconds. This dev box has run at load 30-200 all day with concurrent agent builds and cannot satisfy that.Does the published comparison move against us?
Direction: yes, against us. Node 26 ships a materially newer V8 than Node 22, so the published ratios should shrink. That is the honest direction and the point of the exercise.
Magnitude: not measurable on this host, and deliberately not guessed. The artifact's own quiet-host policy is the project's statement of what a trustworthy number costs; anything produced on this box would be junk wearing a decimal point.
The check
New
scripts/check_node_version_consistency.py, wired as alintstep, followingcheck_gc_doc_claims.py:llms.txt, the compat-matrix baseline) are re-derived from the file each one quotes. A rewording that removes a marker fails rather than silently ceasing to be checked.node-version:in.github/workflows/must be a registered exemption with a reason; everynode-version-file:must point at.node-version.--self-testproves every rule can fail;--listprints the whole registry.It reproduces the historical bug: reverting
npm-launcher.ymlto"22.23.1"yields two named failures.Also fixed
external-tools.jsontold readers to bump "the NODE_PIN.version constant in scripts/node_compat_matrix.mjs". There is no such constant; that script readsexternal-tools.json.Out of scope
bun: 1.3.14in the same config is not equally stale — 1.3.x is the current Bun line. It is frozen by the same atomicity rule, so it should be re-pinned in the same regeneration run.Validation
check_node_version_consistency.py --self-test+ real run green;check_gc_doc_claims.pygreen (CLAUDE.md is in its scope);ci_public_baseline_check.py+tests/test_public_baseline.pygreen (config untouched);cargo fmt --all -- --check;scripts/check_file_size.sh; all four edited workflows parse as YAML. No version bump, noCHANGELOG.md.Notes:
gc-handoff/NODE26-NOTES.md.https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
Summary by CodeRabbit
Improvements
Documentation
Chores