Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,24 @@ jobs:
- name: Platform-aware parity allowlist self-test
run: python3 scripts/parity_known_failures.py --self-test

# Moving-GC gate wiring. The GC gates are the ones this repo has most
# often found unable to fail (CLAUDE.md's four hazards), and every miss so
# far was caught by a human re-deriving it mid-incident. This asserts the
# mechanical half from `lint`, which IS a required context: each gate job
# must actually execute on main-line code (a push to `main`, or the
# nightly `schedule` — a tag-only run adjudicates nothing, it fires after
# every merge it was supposed to judge), must not carry job-level
# `continue-on-error`, must not swallow its gating step's exit status, and
# must not let a new merge cancel the previous main run.
#
# It cannot check branch protection's required-context list — that is
# server-side state, not a file in the tree — and says so; `--list` prints
# what is and is not covered.
- name: Moving-GC gate wiring
run: |
python3 scripts/gc_gate_wiring_check.py --self-test
python3 scripts/gc_gate_wiring_check.py

# ---------------------------------------------------------------------------
# Clippy — enforces the deny-level lints in [workspace.lints] (root
# Cargo.toml). `cargo clippy` exits nonzero only on `deny` lints, so
Expand Down Expand Up @@ -910,14 +928,38 @@ jobs:
#
# Cost split: a PR runs the 4-arm subset, whose arms all share one
# compile-time environment, so the corpus is compiled ONCE and run four
# times. push / workflow_dispatch run the full arm list (19 arms, 9 compile
# groups) as the deeper net. NOTE: this job is not yet in branch protection's
# required contexts — adding it there is what makes the gate blocking.
# times. push / schedule / workflow_dispatch run the full arm list as the
# deeper net. NOTE: this job is not yet in branch protection's required
# contexts — adding it there is what makes the gate blocking.
Comment on lines +931 to +933

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

gc-stress still cannot block a merge.

This comment states that the job is not in branch protection's required contexts. Adding schedule fixes main-line execution, and it does not make the gate blocking. A red gc-stress on the nightly main run produces a notification, not a merge block, so the same class of miss can still reach main through a PR.

The required-context list is server-side state and cannot change in this diff. Record the follow-up so it is not lost.

Do you want me to open an issue that tracks adding gc-stress, gc-moving-witnesses, gc-root-dominance, and gc-ratchet to branch protection's required contexts?

As per coding guidelines: "A CI gate must be able to fail: do not use continue-on-error, ensure it is required by branch protection, avoid unconditional cancellation of main runs, and assert that the tested subject actually executed."

🤖 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 931 - 933, Document a follow-up
issue for adding gc-stress, gc-moving-witnesses, gc-root-dominance, and
gc-ratchet to branch protection’s required contexts, since workflow changes
cannot make these gates merge-blocking. Update the relevant gc-stress workflow
comment or tracking documentation so this required server-side action is
explicitly recorded.

Source: Coding guidelines

#
# ***`schedule` IS LOAD-BEARING IN THE `if:` BELOW (#7194).*** Without it this
# job had NO main-line execution at all, and the hole is invisible from either
# end on its own:
#
# * this workflow's `push:` trigger is TAGS ONLY — "Direct pushes to main do
# NOT trigger tests", stated at the top of this file — so `push` in the
# `if:` only ever means a release tag;
# * the nightly cron, which the concurrency comment above calls "the only
# backstop for integration-suite regressions a scoped PR run can't see",
# fires as `schedule`, which the `if:` did not list. Measured: twelve
# consecutive nightly `main` runs, `gc-stress` reported `skipped` in every
# one.
#
# So between release tags, nothing in CI ran scripts/gc_repsel_matrix.sh on
# `main` — and the matrix is the only place the `requires=move`
# allocation-point arms execute over the representation corpus. That is how
# test_gap_repsel_p4a3_ptr_numarray stayed red on ten arms for over a week
# with no CI event to say so, forcing three separate PRs (#7193, #7233, #7196)
# to hand-exonerate the same seventy cells. This is CLAUDE.md hazard 4 in its
# purest form: the job existed, was correctly written, and its subject never
# ran. scripts/gc_gate_wiring_check.py asserts this from `lint` so it cannot
# silently come back.
# ---------------------------------------------------------------------------
gc-stress:
if: >-
github.event_name == 'push' ||
github.event_name == 'pull_request' ||
github.event_name == 'schedule' ||
github.event_name == 'workflow_dispatch'
runs-on: ubuntu-latest
timeout-minutes: 90
Expand Down
41 changes: 41 additions & 0 deletions changelog.d/7253-gc-gate-main-line-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
### CI

- **The moving-GC matrix now runs on `main`.** `gc-stress` carries
`scripts/gc_repsel_matrix.sh`, the only CI execution of the `requires=move`
allocation-point arms over the representation corpus, and it had **no
main-line run at all**. `test.yml`'s `push:` trigger is tags-only ("Direct
pushes to main do NOT trigger tests"), so `push` in the job's `if:` only ever
meant a release tag; and the nightly cron — which the same file calls "the
only backstop for integration-suite regressions a scoped PR run can't see" —
fires as `schedule`, which the `if:` did not list. Twelve consecutive nightly
`main` runs reported `gc-stress` as `skipped`. Between release tags nothing
adjudicated the matrix on `main`, which is how `test_gap_repsel_p4a3_ptr_numarray`
(#7194) stayed red on ten arms for over a week and forced three separate PRs
to hand-exonerate the same seventy cells. `schedule` is now in the `if:`.

- **New `lint` gate: `scripts/gc_gate_wiring_check.py`.** Asserts, for each of
the four moving-GC gate jobs (`gc-stress`, `gc-moving-witnesses`,
`gc-root-dominance`, `gc-ratchet`), that it executes on main-line code, carries
no job-level `continue-on-error`, does not swallow its gating step's exit
status, and does not let a new merge cancel the previous `main` run — CLAUDE.md's
four ways a gate can be unable to fail, mechanised. A **tag-only** run does not
count as main-line: a gate that speaks only at release time cannot name the
merge that broke it. The checker fails on the pre-fix tree, naming `gc-stress`,
and passes after; `--self-test` covers seven cases. It reports, and cannot
enforce, that none of these jobs is in branch protection's required contexts.

### Fixed (verification only — no product change)

- **#7194 `test_gap_repsel_p4a3_ptr_numarray` is closed as fixed by #7249.**
Bisected across the five GC/codegen merges that followed the issue's last
measurement, one build per hop and ten runs each: red at `c9cd73ba5`,
`8b024958f` (#7242), `c7893c4ac` (#7250) and `061b24163` (#7252); green at
`64c1f56fb` (#7249, the globalThis bootstrap no-move window). The base
reproduction is byte-identical to the issue's evidence, including the
`scavenged=6514`/`6515` split across arm groups; the failure was one lost
`counts[v]++` out of 5000, deterministic 10/10. Because #7249 changes *where*
minor #0 lands, the head build collects less than the failing base did
(`cycles=1 scavenged=3585` vs `cycles=2 scavenged=6515`), so green was
re-established *above* the base's movement rather than below it: under loop
polls + `PERRY_GC_ZEAL=1` the test survives **14 373 evacuating minors**
byte-exact, 10/10.
Loading
Loading