ci: standardize the gap-suite Node oracle on 26.5.0 (single .node-version pin)#6367
Merged
Conversation
added 3 commits
July 13, 2026 12:24
The gap suite (conformance-smoke, a required check) byte-diffs every
test-files/test_gap_*.ts against `node --experimental-strip-types`, so Node is
the oracle and its version is a correctness input. CI pinned Node 22 while the
suite grew Node 24/26 features. When node itself cannot run a test it exits
non-zero, run_parity_tests.sh classifies it `node_fail`, and the test is
dropped from the gate — so a feature Perry gets wrong is invisible by
construction rather than red.
Measured over all 299 gap tests, same Perry build, node 22 vs 24 vs 26:
node 22 -> 18 node_fail (18 tests excluded from the gate)
node 24 -> 12 node_fail
node 26 -> 4 node_fail
Node 26 restores 14 tests to the gate; Node 24 only 6. The 8 that only 26
un-hides are Temporal (7) plus Uint8Array base64/hex — whole feature families
that Node 24 cannot see at all. No test regresses (there is no test that runs
on 22 but node_fails on 26), so 26 strictly dominates.
A 22+26 matrix was rejected on evidence, not cost: 8 tests produce *different*
output on 22 vs 26 (BigInt RangeError text, Intl locale error text, v8 heap
node_fields, diagnostics_channel store semantics, SQLite ExperimentalWarning).
Perry emits one byte string, so it cannot byte-match both oracles at once — a
matrix would be permanently red by construction.
Pins now come from .node-version, so the oracle cannot drift silently and local
runs are comparable to CI. Exempt, each documented inline:
- node-core-subset.yml runs Node's OWN test corpus and must match the ref in
test-compat/node-core/pinned-version.txt; now derived from that file so the
coupling is explicit instead of looking like drift.
- release-hono-server.yml (24) and release-packages.yml (20) are npm
publishing toolchains, not oracles.
…aring The gap suite is a byte-diff against node, so the oracle version decides which tests are even *visible* to the required gate — a test whose feature the pinned node lacks is dropped as node_fail, not reported red. Say so where contributors already read about the suite, and point at .node-version as the single pin.
…y the Node 26 oracle Raising the gap-suite oracle from Node 22 to 26 restores 14 tests that node 22 could not run (and that the harness therefore dropped as `node_fail` rather than reporting). Measured against the new oracle, 13 of the 14 PASS — the whole Temporal family (7), Float16Array, Uint8Array base64/hex, static helpers, async_advanced, the constants tail, and the error batch. Perry already implements all of these; the required gate had simply never checked them. The 14th, test_gap_disposablestack_2875, fails: `.disposed` returns undefined where node returns false/true. That is a pre-existing gap in Perry's DisposableStack surface (crates/perry-runtime/src/disposable.rs), already tracked as #6364 — it is newly *visible*, not newly broken. Triaged here so the oracle bump lands green instead of blocking every PR in the repo.
📝 WalkthroughWalkthroughChangesNode version alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (2)
.github/workflows/node-core-subset.yml (1)
78-101: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse consistent file-reading approach for
pinned-version.txt.Line 93 reads the file with
tr -d '[:space:]'(strips all whitespace), but line 106 reads the same file with$(cat ...)(strips only trailing newlines). If the file ever contains trailing spaces or carriage returns, the two reads would produce different values, potentially causinggit clone --branch "$node_ref"to fail at line 107 while the version resolution at line 93 succeeds.♻️ Suggested consistency fix for line 106
- name: Sparse checkout pinned Node.js tests run: | set -euo pipefail - node_ref="$(cat test-compat/node-core/pinned-version.txt)" + node_ref="$(tr -d '[:space:]' < test-compat/node-core/pinned-version.txt)" git clone --no-checkout --depth 1 --branch "$node_ref" \🤖 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/node-core-subset.yml around lines 78 - 101, Use the same whitespace-normalizing read for pinned-version.txt in the later clone/setup flow as the node_core_version resolution, replacing the inconsistent cat-based read that assigns node_ref. Ensure both consumers derive the branch value from identical trimmed content so git clone --branch receives the normalized pinned ref.CLAUDE.md (1)
16-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider trimming the historical narrative in line 18 for conciseness.
The paragraph contains essential operational guidance (oracle pin,
node_faildrop behavior, exact-pin rationale) that belongs here, but also embeds historical CI details — specific test names, issue numbers, and the Node 22→24→26 baseline history — that read like changelog-adjacent content. Per the coding guidelines,CLAUDE.mdshould be kept concise and detailed per-version history should live inCHANGELOG.md. Consider moving the historical enumeration ("CI sat on Node 22…hid 14 tests — all of Temporal, plus DisposableStack, Float16Array, andUint8Arraybase64/hex (#6364)") toCHANGELOG.mdor the issue tracker, keeping only the actionable directive.As per coding guidelines: "Keep
CLAUDE.mdconcise; keep detailed per-version changelogs inCHANGELOG.md, notCLAUDE.md."🤖 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 `@CLAUDE.md` around lines 16 - 20, Trim the pinned-Node guidance in CLAUDE.md by removing the historical CI narrative, including the Node 22→24→26 history, hidden-test count, feature list, and issue reference. Preserve the actionable oracle requirements: use the exact .node-version, explain node_fail handling, and retain the rationale for exact patch pinning; move historical details to CHANGELOG.md or the issue tracker if they must be retained.Source: Coding guidelines
🤖 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/test.yml:
- Around line 1422-1426: Update the comments accompanying node-version-file in
the affected package smoke jobs to retain the .node-version
single-source-of-truth note while replacing the gap/parity-oracle explanation
with the actual fixture-specific rationale for pinning Node. Apply the same
comment correction to all three corresponding job blocks, without changing the
Node configuration.
---
Nitpick comments:
In @.github/workflows/node-core-subset.yml:
- Around line 78-101: Use the same whitespace-normalizing read for
pinned-version.txt in the later clone/setup flow as the node_core_version
resolution, replacing the inconsistent cat-based read that assigns node_ref.
Ensure both consumers derive the branch value from identical trimmed content so
git clone --branch receives the normalized pinned ref.
In `@CLAUDE.md`:
- Around line 16-20: Trim the pinned-Node guidance in CLAUDE.md by removing the
historical CI narrative, including the Node 22→24→26 history, hidden-test count,
feature list, and issue reference. Preserve the actionable oracle requirements:
use the exact .node-version, explain node_fail handling, and retain the
rationale for exact patch pinning; move historical details to CHANGELOG.md or
the issue tracker if they must be retained.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7126daa-743f-458d-aaca-61ef9c998a34
📒 Files selected for processing (11)
.github/workflows/benchmark.yml.github/workflows/feature-matrix.yml.github/workflows/node-core-subset.yml.github/workflows/node-suite-guard.yml.github/workflows/npm-package-sweep.yml.github/workflows/release-hono-server.yml.github/workflows/release-packages.yml.github/workflows/test.yml.node-versionCLAUDE.mdtest-parity/known_failures.json
Comment on lines
+1422
to
+1426
| # Single source of truth: .node-version at the repo root. Node is the | ||
| # gap/parity oracle (we byte-diff against it), so the version is a | ||
| # correctness input, not an incidental toolchain detail — never pin it | ||
| # inline here. See CLAUDE.md ("TypeScript Parity Status"). | ||
| node-version-file: .node-version |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use job-specific rationale for the Node pin.
These package smoke jobs do not run the gap/parity oracle steps in this workflow, but the comments describe Node as that oracle. Keep the single-source-of-truth note, and replace the copied oracle explanation with the fixture-specific reason for pinning Node.
Also applies to: 1488-1492, 1541-1545
🤖 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/test.yml around lines 1422 - 1426, Update the comments
accompanying node-version-file in the affected package smoke jobs to retain the
.node-version single-source-of-truth note while replacing the gap/parity-oracle
explanation with the actual fixture-specific rationale for pinning Node. Apply
the same comment correction to all three corresponding job blocks, without
changing the Node configuration.
This was referenced Jul 16, 2026
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
1 participant
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.
Summary
conformance-smoke(a required check) compiles everytest-files/test_gap_*.tsand byte-diffs it againstnode --experimental-strip-types. Node is the oracle, so its version is a correctness input — but CI pinned Node 22 while the suite grew Node 24/26 features, and the pins were incoherent across the repo (20, 22, 24 and 26 all in use).The failure mode is silent, not red. When node can't run a test, node exits non-zero,
run_parity_tests.shclassifies itnode_fail, and the test is dropped from the gate entirely. So anything Perry gets wrong about a Node 24/26 feature is invisible by construction.This standardizes the oracle on Node 26.5.0, behind a single
.node-versionpin.Measured delta (this is the point — the numbers came first)
All 299 gap tests, run under node 22 / 24 / 26 with the harness's exact invocation, then the same Perry build diffed against each oracle.
Tests the oracle hides (
node_fail⇒ excluded from the gate):node_failNode 26 restores 14 tests to the gate. Node 24 would restore only 6.
The 14 Node 22 was hiding — and what they do under the new oracle:
test_gap_temporal_now,_duration,_options,_plaindate_plaintime,_plaindatetime,_instant_zoned,_construct_overflowtest_gap_float16array_2902test_gap_u8_base64_hex_2901test_gap_2899_2779_2777_static_helperstest_gap_async_advancedtest_gap_constants_tail_3683plustest_gap_error_2836_2838_2904_2882test_gap_disposablestack_287513 of 14 pass. Perry already implements Temporal, Float16Array,
Uint8Arraybase64/hex and DisposableStack — the required gate had simply never checked a single one of them. That is the blind spot this closes.Tests whose expected output changes between 22 and 26 (both oracles exit 0, but node's output differs): 8. Six are already triaged in
known_failures.json(gate-safe under either oracle); the remaining two —test_gap_console_methods(timer ms) andtest_gap_url_batch2_3057_3060_3062(DEP0169 warning) — are invariant after the harness's ownnormalize_output, verified by extracting and running that function. So they carry no risk.Net: the only newly-untriaged failure the bump exposes is
disposablestack_2875, and it is triaged here. The gate lands green.Sanity check: my Node-26 measurement independently reproduces the known local baseline — 4
node_fail⇒ 295 runnable, matching the 277/295 + 14 triaged + 2 untriaged figure.Why Node 26, not 24
Uint8Arraybase64/hex. Standardizing on 24 would leave Perry's entire Temporal implementation permanently unverified by the required gate.node_fails on 26. Node 26 strictly dominates.feature-matrix/node-suite-guardwere already on 26.Why not a 22+26 matrix
Rejected on evidence, not cost. Eight tests produce different output on 22 vs 26 (BigInt
RangeErrortext, Intl locale error text,v8heapnode_fields,diagnostics_channelstore semantics, SQLiteExperimentalWarning). Perry emits one byte string, so it cannot byte-match both oracles simultaneously — a matrix would be permanently red by construction, not merely expensive.For the record, the cost would also have been real: the gap suite is 8 shards × 19–37 min, so a second oracle roughly doubles the repo's most expensive required gate, at roughly +150–300 runner-minutes per PR.
Coherence: one pin, three documented exemptions
.node-version(26.5.0) is now the single source of truth; every test workflow reads it viasetup-node'snode-version-file. The pin is exact on purpose — node patch releases change observable output (we measured error-message text changing), so a byte-diff oracle must not drift silently under us.Standardized:
test.yml(×6, incl.conformance-smoke+parity),npm-package-sweep,feature-matrix,node-suite-guard,benchmark.Exempt, and each now says so inline so the next person doesn't "fix" it into breakage:
node-core-subset.yml— runs Node's owntest/parallelcorpus, sparse-checked-out at the ref intest-compat/node-core/pinned-version.txt(v22.x). Those tests are written against that Node line's internals, so the runtime must match the corpus, not the oracle. It now derives its version from that same file, so the coupling is explicit and can't break — previously a bare inline"22"looked exactly like drift.release-hono-server.yml(24) andrelease-packages.yml(20) — npm publishing toolchains (OIDC Trusted Publishing needs npm ≥ 11.5.1); they never run a parity diff. Checked, not assumed.Out of scope
test_gap_handle_band_object_ops(#6363) is an untriaged failure onmain, but it is oracle-invariant — node exits 0 and emits byte-identical output on 22, 24 and 26 — so this change cannot affect its verdict either way. Left alone deliberately.Validation
python3 -c "import yaml,...; yaml.safe_load(...)"over all 14 workflows — valid._schemacontract.Summary by CodeRabbit
Chores
Documentation
Tests