Skip to content

docs: the four ways a CI gate can be unable to fail #7049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 1 commit into from
Jul 30, 2026
Merged

docs: the four ways a CI gate can be unable to fail #7049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,17 @@ Build outputs are invisible to `git status`, so a clean tree tells you nothing a
- **`conformance-smoke` shards are flaky.** Before believing a red shard, re-run it and A/B the named tests against a pristine `main` build; several are already in `test-parity/known_failures.json`.
- **Integration suites under `crates/*/tests/*.rs` do not run per-PR** (nightly/tag only) — a regression there can land green and sit red for days. Prefer putting acceptance coverage in `cargo-test`-visible unit tests (#5960).

### ★ Four ways a gate can be unable to fail

All four look fine on the Actions page. None can turn a merge red. When adding or reviewing a gate, check all four — each has bitten this repo, three of them within one week:

1. **`continue-on-error: true`** — `gc-stress` carried it for months while being the only job covering GC correctness.
2. **Not in branch protection's required contexts** — `gc-stress` again. This is why #6925's `PERRY_PTR_SHAPE_LOCALS=0` regression landed visibly red and survived three merges. A job that reports failure without blocking is documentation, not a gate.
3. **`concurrency` with unconditional `cancel-in-progress`** — on a branch with a slow runner queue, every new merge cancels the previous run before it reaches a runner. `gc-ratchet` had three consecutive `main` runs cancelled, zero executed. Scope cancellation to `pull_request` and let `main` runs queue.
4. **The gate runs but its subject never did** — the most dangerous, because the job is genuinely green. `PERRY_GC_FORCE_EVACUATE` was inert for every `gc()`-driven test (#6942/#6946); the matrix's `--pressure` knob disabled the very path it was measuring (#7024); its `moved=` counter summed two different collectors, so a cell could pass having run zero copying minors (#7025). **A gate must assert its subject was live**, not merely that nothing threw — e.g. `copied_objects > 0` before a green verdict.

Corollary: a *new* gate has never been green, so promoting it to required immediately blocks every open PR. Run it once, then promote. Leaving that second step undone is how (2) happens.
Comment on lines +205 to +214

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

GitHub status checks required branches cancelled check blocks merge not satisfied pending

💡 Result:

On GitHub, a required status check must reach a successful conclusion for a pull request to be merged into a protected branch [1][2]. If a required status check is in a "cancelled" or "pending" state, it will block the merge [3][4][5]. The following details clarify how these states affect merging: Pending: A status check remains in a "pending" state if it has not yet reported a result, or if the workflow was skipped (e.g., due to path or branch filtering) [4][6]. Because the check has not reached a successful conclusion, it blocks the merge [3][4]. Cancelled: If a required status check is cancelled before it completes, its conclusion is "cancelled" [2]. This does not satisfy the requirement for a successful, skipped, or neutral conclusion, and therefore it blocks the merge [1][2][5]. To resolve these issues, you must ensure the required check is triggered and reaches a successful, skipped, or neutral conclusion [1][2][3]. If a check is stuck in "pending" or "cancelled," you may need to: - Rerun the workflow or check to ensure it completes successfully [2][5]. - Verify that your workflow filters (such as paths or branches) are not causing the check to be skipped unexpectedly [4][6]. - Ensure that the required check name matches exactly what is defined in your branch protection rules [3][5]. - If you are using merge queues, ensure your workflows include the merge_group event trigger [7][8][6].

Citations:

Correct the cancellation wording for required checks.

A cancelled required status check can block merging because it is not a successful/skipped/neutral conclusion. Reword the cancellation item to say the rule is to scope cancellation so required checks on main are not cancelled, rather than claiming it universally cannot turn a merge red. Apply this to both CLAUDE.md and changelog.d/7047-gate-cannot-fail-patterns.md.

📍 Affects 2 files
  • CLAUDE.md#L205-L214 (this comment)
  • changelog.d/7047-gate-cannot-fail-patterns.md#L1-L12
🤖 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 205 - 214, Revise the cancellation guidance in the
four-way gate checklist so it no longer claims cancellation cannot turn a merge
red; state that required checks can block merging when cancelled, and that
cancellation must be scoped to pull_request so required checks on main are
allowed to queue. Apply the same wording correction in CLAUDE.md (the
cancellation item around lines 205-214) and
changelog.d/7047-gate-cannot-fail-patterns.md (lines 1-12); update both sites
directly.

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

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
rg -n '^\[workspace\.package\]|^version\s*=' Cargo.toml
rg -n '^Current Version' CLAUDE.md
git diff --name-only -- Cargo.toml CLAUDE.md changelog.d/7047-gate-cannot-fail-patterns.md

Repository: PerryTS/perry

Length of output: 200

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "Top-level tracked files:"
git ls-files | sed -n '1,80p'

echo
echo "CLAUDE.md current version occurrences:"
rg -n -C 2 '^current version|^Current Version' CLAUDE.md || true

echo
echo "changelog.d contents count and names:"
git ls-files 'changelog.d/*' | sed -n '1,120p' | wc -l
git ls-files 'changelog.d/*' | sed -n '1,160p'

echo
echo "Relevant git status and diff names:"
git status --short
git diff --name-only -- Cargo.toml CLAUDE.md changelog.d/ 2>/dev/null || true
git diff --cached --name-only -- Cargo.toml CLAUDE.md changelog.d/ 2>/dev/null || true

echo
echo "Search references to gate patterns or added lines:"
rg -n "Four ways a gate can be unable to fail|PERRY_GC_FORCE_EVACUATE|gc-ratchet|continue-on-error: true|concurrency|cancel-in-progress|required contexts" CLAUDE.md CHANGELOG.md changelog.d Cargo.toml 2>/dev/null || true

Repository: PerryTS/perry

Length of output: 28604

Add the required version metadata before merging to main.

This Markdown-only change is not tied to a Cargo.toml version bump. Include the changelog fragment, update [workspace.package].version, and update CLAUDE.md’s Current Version line as part of the merge commit.

🤖 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 205 - 214, Before merging this documentation change
to main, add the required changelog fragment, update the version field under
[workspace.package] in Cargo.toml, and update the Current Version line in
CLAUDE.md to the new version.

Source: Coding guidelines


### Known-weak areas (symptom is often not the bug)
- **Async-to-generator transform, body locals.** It boxes every body local into a shared mutable cell typed `Any`. Two consequences seen in the wild: per-iteration `let`/`const` bindings collapse for closures created in a loop, and computed numeric-key calls (`arr[i](x)`) lose their type proof and silently resolve by *method name*, evaporating the call.
- **Native base-class subclassing.** A native base's surface is installed at `super()` time and its parent edge lives in the class registry; keying any of that on a literal `extends` name loses it for fieldless classes, indirect subclasses, and class expressions.
Expand Down
12 changes: 12 additions & 0 deletions changelog.d/7047-gate-cannot-fail-patterns.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Documented the four distinct ways a CI gate can be structurally unable to fail,
in `CLAUDE.md`. All four look green-adjacent on the Actions page and none can turn
a merge red: `continue-on-error: true`; absence from branch protection's required
contexts; `concurrency` with unconditional `cancel-in-progress` on a slow-queue
branch; and the subtlest — the gate runs and passes while its subject never
actually executed.

Each has bitten this repo, three of them inside one week. The fourth is the
dangerous one because the job is genuinely green: `PERRY_GC_FORCE_EVACUATE` was
inert for every `gc()`-driven test, and the GC matrix's `--pressure` knob disabled
the very path it was measuring. The rule that follows is that a gate must assert
its subject was live, not merely that nothing threw.
Loading