Merge train 206: the three gap regressions keeping main red, Node pin, zizmor trigger (v0.5.1584) - #10397
Merged
Conversation
added 8 commits
September 17, 2026 00:22
Three unrelated parity failures, one commit because they are what the `gap-suite` shards report on `main`. 1. `test_gap_disposablestack_2875` — `new SuppressedError(...) instanceof Error` answered `false`. `SuppressedError` was missing from `is_native_error_subclass_constructor`, so its prototype pair was never linked into the Error family; latent until `js_instanceof` started answering `x instanceof Error` from the instance's recorded prototype chain, which then short-circuited the class-registry fact that used to carry it. Adds the missing link, which is what ECMA-262 specifies. 2. `test_gap_2899_2779_2777_static_helpers` — group arrays serialized as `null`. `js_array_alloc` births an array flagged "every slot is an unboxed double"; a producer that direct-writes its element words never clears it, and `json::stringify_primitive_array` now trusts the flag. Re-derives the flag from the slots in `rebuild_array_layout_from_slots`, the choke point those producers already call. Clear-only. 3. `test_gap_iterator_prototype_next_patch` — a patched `%ArrayIteratorPrototype%.next` (and its Map / Set / String siblings) did not drive spread, `Array.from` or call-spread, because each ends in a runtime element-COPY arm that never calls `.next()`. Extends #10086's prototype-escape signal to all four families and makes those arms decline on it. This fixture has been red since it landed. Validation: all three byte-identical to node v26.5.1; a base-vs-fix sweep over 448 gap fixtures shows no other output change; `perry-runtime --lib` 3970 passed.
The fixture logged from inside a patched-prototype window. Node constructs
SafeMap out of internal/per_context/primordials lazily, and run_parity_tests.sh
runs the oracle with FORCE_COLOR=0 -- the path that defers that construction
into the window. Node therefore died on its own internals:
node:internal/per_context/primordials:449
class SafeMap extends Map {},
reporting Node exit 1 against Perry exit 0, so the test could not pass whatever
the runtime did. It is reproducible with the harness's exact invocation and NOT
with a bare `node file.ts`, which is why it read as a Perry failure for ten
days and why a local run kept disagreeing with CI.
Output is now buffered and flushed after each restore. Every value is still
computed inside the patched window -- that is the subject -- and the emitted
text is byte-identical to the unbuffered run, so EXPECTED is unchanged.
crates/perry/tests/bun_embedded_compression.rs shells out to
scripts/test-bun-embedded-compression.mjs, whose first assertion is that
process.versions.node equals the .node-version pin — the loader and byte
behaviour it gates on is version-specific, so the assertion is the point of
the test, not incidental.
Neither the cargo-test job nor the cargo-test-perry shards ran setup-node, so
both inherited whatever node the ubuntu-latest image ships. That is 22.23.2
today against a 26.5.1 pin, which is why standalone_compressed_asset_regression
fails on main with
AssertionError: + '22.23.2' - '26.5.1'
at scripts/test-bun-embedded-compression.mjs:10:8
and takes the whole job red. Every other job that runs node-backed tests
(lint, e2e-scoped, windows-build) already had the step; these two were the gap.
Pinned from the FILE rather than a literal: check_node_version_consistency.py
is a lint step and requires every literal node-version: to be a registered
exemption. Both shards are pinned, not just the one observed failing — which
shard a node-backed test lands in is decided by ci_cargo_test_shard.py, so
pinning only one moves the failure with the sharding instead of removing it.
zizmor has been red on main since 2026-09-06, the day gate-failure-watch.yml landed, on one high finding: dangerous-triggers against its `workflow_run`. The audit flags that trigger categorically -- "almost always used insecurely", at Medium confidence -- and this workflow is one of the cases where it is not. Both insecure uses are already closed in the file. The `observe` job's `if:` admits only schedule, or dispatch/push on main or a v* tag, so a fork PR's run can never reach the write-capable token; and the checkout pins `ref: main` with `persist-credentials: false` and runs only the default-branch scripts/gate_failure_watch.py, so the triggering run's code is never executed and none of its artifacts are downloaded. Permissions are actions:read, contents:read, issues:write. Suppressed inline rather than in .github/zizmor.yml so the justification sits next to the trigger it excuses, and carries its own ratchet: an artifact download, a head_sha checkout or a looser `if:` means deleting the marker. Removing the trigger is not an option -- observing another workflow's completion is the feature, and #9830 measured a correctly-failing scheduled workflow staying red for nineteen days unnoticed. Verified with the pinned zizmor 1.28.0 (external-tools.json) using the workflow's own invocation: `zizmor .github/ --min-severity high` goes from exit 14 with one high finding to exit 0, "No findings to report". The marker has to trail the `on:` key -- the same text as a comment block above it does not suppress anything, which is why this was checked against the real tool rather than reasoned about.
This was referenced Sep 16, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (24)
📝 WalkthroughWalkthroughThe pull request updates CI Node.js pinning and workflow security documentation, advances the workspace version, and fixes runtime behavior for iterator prototype patches, grouped-array JSON serialization, and ChangesCI safeguards and release metadata
Iterator prototype escape handling
Array layout reclassification
SuppressedError prototype heritage
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: High Sequence Diagram(s)sequenceDiagram
participant UserCode
participant js_object_get_prototype_of
participant note_iterator_prototype_exposed
participant array_from_spread_value
participant js_iterator_to_array
UserCode->>js_object_get_prototype_of: expose iterator prototype
js_object_get_prototype_of->>note_iterator_prototype_exposed: record family exposure
note_iterator_prototype_exposed-->>array_from_spread_value: mark iteration non-pristine
array_from_spread_value->>js_iterator_to_array: execute iterator protocol
js_iterator_to_array-->>UserCode: return materialized values
Possibly related PRs
Suggested labels: ✨ Finishing Touches📝 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 |
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.
This train lands #10387, #10383 and #10386 as v0.5.1584. All three sit on
f9c3e51e93, touch no file in common, and cherry-pick clean.main's gap-suite shards red. Two are genuine regressions with a named introducing commit; the third never passed.test_gap_disposablestack_2875—SuppressedErrorwas missing fromis_native_error_subclass_constructor, so the Error-family loop never linked its prototype pair. Latent untila109736407put a recorded-prototype walk above the class-registry fact that used to carry it. Fixed by the ECMA-262 link itself, not a special case ininstanceof.test_gap_2899_2779_2777_static_helpers—js_array_allocstampsGC_ARRAY_RAW_F64_LAYOUTon a fresh length-0 array, where it is vacuously true;group_by_make_arraythen writes element words directly, bypassing the helpers that would clear it. Harmless untildc39693e69madestringify_primitive_arrayread the flag as proof, so a NaN-boxed string went out asnull. Fixed at the choke point every direct-slot writer already calls, and clear-only — it never sets the flag.test_gap_iterator_prototype_next_patch— element-copy fast arms materialize results without calling.next(), so a per-call proof cannot see a patch from inside them; they now decline on the perf(hir): array destructuring always materializes its temporary, costing a flat 76x Node for a two-variable swap #10086 escape signal..node-versionin thecargo-testjobs. Workflow-only.gate-failure-watch'sworkflow_runtrigger is safe. Workflow-only, and it is the change that makeszizmorgreen:zizmorcurrently fails onmain, and this PR's own branch is the one run that passes.Seven source commits, each verified to preserve its patch-id and authorship byte-for-byte. No train repairs were needed; the train adds only the version bump.
Validation
Validated head
7d46ba22a4. Five-package release build pinned and hash-verified, and re-verified after the gap run (artifacts_match_pin_after_gap) so nothing rebuilt underneath the fixtures.All three target fixtures now pass locally, against those pinned artifacts:
main's one known runtime failure (gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds), identical to the previous train.crates/perry/tests/issue_9846_iterator_prototype_next_patch.rspasses. This suite ise2e-scopedin CI and is the direct acceptance test for the iterator arms, so it was run explicitly against the pinned artifacts, with the-staticwrappers built first so it could not link a stale archive. The pinned artifact hashes are unchanged by that run.main.iterator,disposable,error,group,json,array,spread,static,helper— weighted atarrayandjsonbecause fix(runtime): the three gap tests keeping main red #10387 changes array internals and the raw-f64 flag is exactly what JSON consumed.Every red in those filters, attributed
--filteris a substring match over all fixtures, so these runs also selecttest_issue_*andtest_ws_*names that CI's gap suite (--filter test_gap_) never runs. Stale snapshot entries for those surface as noise. Each was chased down rather than assumed:test_guarded_raw_numeric_arrays— the one that genuinely looked like it could be this train's fault, given the subject. Direct A/B againstmain's artifact set: fails identically on both arms, with distinct build stamps (c091bb1807/7d46ba22a4) proving each arm ran its own binary. Pre-existing.test_issue_1240_fastify_request_json,test_issue_1293_...—pass -> skipped. Both fixtures declare// parity-skip: background serveronmain; the harness honours that and skips before compiling. Nothing to do with this train.test_issue_4826_array_headers— the node oracle itself times out (hencenode_fail), and both arms fail to link for want oflibperry_ext_http.a, which is not in the pinned five-package set.test_ws_static_constants_6117,test_issue_556_table_array— both arms fail identically (link for want oflibperry_ext_ws.a; and both time out, respectively). This train touches no websocket code.test_gap_json_lazy_defineproperty_index— a recorded known failure in the snapshot (issue JSON.parse lazy array: Object.defineProperty index accessor is bypassed by reads #10097), counted correctly.Source CI: #10387 is green on all six gap shards — the first green on shards 3, 4 and 5 in over a week. All three PRs' remaining red is what every PR inherits: benchmark-evidence freshness in
lint, and thepr-gatefan-in over it.Before merging, the pushed head and unchanged main are checked again. After merging, the rewritten commits are checked for preserved authorship and the main tree must match the validated train exactly.
Summary by CodeRabbit
Bug Fixes
Array.frombehavior when built-in iterator methods are customized.SuppressedErrorinheritance andinstanceof Errorbehavior.Chores
Tests
SuppressedErrorbehavior.