fix(branch-protection): stop requiring a conditionally-posted status context - #2872
Conversation
…context `Health 45 Agents Guard / guard` cannot be a required status check. It is a commit status posted by agents-guard.yml only inside the "Report agents guard commit status" step, whose condition is `always() && steps.eligibility.outputs.should-run == 'true'`, and eligibility requires one of the agent labels (agent:codex, agents:auto-pilot, ...). On any PR without such a label the status is never posted, and a required check that never reports leaves the PR permanently un-mergeable. Verified absent on the heads of all 12 most recent PRs (#2850-#2862), where the only statuses are `Gate / gate` and `CodeRabbit`. This was not merely theoretical: health-44 passes .github/config/required-contexts.json to `enforce_gate_branch_protection.py --apply` on workflow_dispatch/workflow_call whenever an enforcement token is present, so dispatching that workflow today would have added the context and frozen every non-agent PR in the repo. Remove it from the required-contexts set (shared config, the tool's DEFAULT_CONTEXTS, both health-40 invocations) and record why in the config so it is not re-added. `Gate / gate` remains required-worthy: it is posted on every PR by pr-00-gate.yml and passes on all 12 most recent PRs. agents-guard.yml still posts its status; it is simply informational rather than gating. Also give health-40's "Root allowlist guard" `if: always()`. Steps run sequentially, so the failing branch-protection snapshot step was skipping it — which is how six unlisted repo-root files accumulated unseen for a month. Refs #2858. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR standardizes branch protection on the universally posted ChangesGate enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
GitHub Actions
Inactive
Workflow source neededPR #2872 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
There was a problem hiding this comment.
Pull request overview
This PR removes a conditionally-posted status context (Health 45 Agents Guard / guard) from the repository’s “required status checks” enforcement surfaces, ensuring branch protection cannot accidentally require a check that is absent on non-agent-labelled PRs (preventing those PRs from becoming permanently un-mergeable). It also adjusts the repo selfcheck workflow so the repo-root allowlist guard runs even if earlier branch-protection snapshotting fails.
Changes:
- Reduce branch-protection “default required contexts” to only
Gate / gate, and update related help/docstring examples. - Update repo selfcheck (
health-40) to stop passing the guard context to the enforcement tool and ensure “Root allowlist guard” runs withif: always(). - Update
.github/config/required-contexts.jsonto only listGate / gateand document why the guard context must not be re-added.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tools/enforce_gate_branch_protection.py |
Removes the guard context from DEFAULT_CONTEXTS and aligns CLI/docs with the new default. |
.github/workflows/health-40-repo-selfcheck.yml |
Stops enforcing the guard context and ensures the root allowlist guard step runs even after earlier failures. |
.github/config/required-contexts.json |
Drops the guard context from the required-contexts config and adds an inline rationale to prevent reintroduction. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c1d729b48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ] | ||
| "Gate / gate" | ||
| ], | ||
| "_note": "Only universally-posted contexts belong here: health-44 can pass this file to enforce_gate_branch_protection.py --apply, so any context listed becomes a REQUIRED status check. 'Health 45 Agents Guard / guard' is deliberately absent: agents-guard.yml posts that status only when the PR carries an agent label (agent:codex, agents:auto-pilot, ...), so requiring it would leave every other PR permanently un-mergeable. See issue #2858." |
There was a problem hiding this comment.
For an agent-labelled PR that violates the guard policy, this removal allows the PR to merge once Gate passes even though the guard job fails. In .github/workflows/agents-guard.yml, the workflow is triggered for every pull request and the guard job itself has no job-level condition; only the legacy commit-status reporting step is conditional. Consequently, Health 45 Agents Guard / guard is also emitted as a workflow check run on every PR, so its absence from the commit statuses API does not show that the required check is missing. Retain this required context so the failure at the workflow's final guard step continues to block protected changes.
Useful? React with 👍 / 👎.
Automated Status SummaryHead SHA: 331d24d
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
(#2889) testgen_gate.py was a single-purpose acceptance harness for #2620, which closed on 2026-06-28. It is dead weight: - Nothing references it. The only two matches for "testgen_gate" in the repo are its own usage-print strings. - It hardcodes exactly one target, tests/scripts/test_repo_review_round1_schema.py, which still exists, passes 22 tests, and is already collected by the normal pytest suite that Gate runs. The gate re-validates what CI covers. - It was the last tracked file at the repo root not on config/root-allowlist.txt, so it is precisely the "one-off debris / stray fixer script" that health-40's Root allowlist guard exists to catch (issue #2277). Deleting rather than allowlisting: allowlisting would bless redundant code and leave the guard permanently unable to flag it. With this removed, the root allowlist guard reports clean — which matters now that #2872 gave it `if: always()` so it actually runs. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
(#2889) testgen_gate.py was a single-purpose acceptance harness for #2620, which closed on 2026-06-28. It is dead weight: - Nothing references it. The only two matches for "testgen_gate" in the repo are its own usage-print strings. - It hardcodes exactly one target, tests/scripts/test_repo_review_round1_schema.py, which still exists, passes 22 tests, and is already collected by the normal pytest suite that Gate runs. The gate re-validates what CI covers. - It was the last tracked file at the repo root not on config/root-allowlist.txt, so it is precisely the "one-off debris / stray fixer script" that health-40's Root allowlist guard exists to catch (issue #2277). Deleting rather than allowlisting: allowlisting would bless redundant code and leave the guard permanently unable to flag it. With this removed, the root allowlist guard reports clean — which matters now that #2872 gave it `if: always()` so it actually runs. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes the code-side half of #2858. The ruleset edit itself still needs an admin.
Why
Health 45 Agents Guard / guardcannot be a required status check.It is a commit status posted by
agents-guard.yml:690-701, inside the step "Report agents guard commit status" (:594), whose condition is:steps.eligibility(:46) requires one ofagent:auto, agent:codex, agent:claude, agent:copilot, agents:auto-pilot, agents:keepaliveon the PR. On a PR without an agent label the status is never posted, and a required check that never reports leaves the PR permanently un-mergeable.Verified: on the head of #2856 the statuses API returns only
Gate / gate [success]andCodeRabbit [success]. The guard status is absent there and on all 12 most recent PRs (#2850-#2862).This was not theoretical.
health-44-gate-branch-protection.yml:220-225passes this same config toenforce_gate_branch_protection.py --applywhenevergithub.event_name != 'pull_request'and an enforcement token is present. Dispatching health-44 today would have added the context and frozen every non-agent PR in the repo — including the ones fixing it.What changed
.github/config/required-contexts.json— drop the context, leavingGate / gate, and record the reason inline so it is not re-added. (Not in the sync manifest, so consumers are unaffected.)tools/enforce_gate_branch_protection.py—DEFAULT_CONTEXTSreduced toGate / gatewith the rationale as a comment; docstring example and--contexthelp text updated to match.health-40-repo-selfcheck.yml— remove--context "Health 45 Agents Guard / guard"from both invocations.health-40-repo-selfcheck.yml— give "Root allowlist guard"if: always(). Steps run sequentially, so the failing snapshot step was skipping it; that is how six unlisted repo-root files accumulated unseen for a month. The comment is explicit that this rescues only that step —Collect repository signalsandAggregate & Summarizestill skip, because the aggregate consumes the collect step's outputs and making the whole job resilient needs a different shape.agents-guard.ymlis unchanged: it still posts its status, now purely informational. Requiring it would need it to post on every PR (e.g.success/ "not applicable" when ineligible) — noted as the alternative on #2858, deliberately not done here since it edits a security-sensitive guard with apull_request_targetpath.Gate / gateremains required-worthy: posted on every PR bypr-00-gate.yml, pass on all 12 most recent PRs.Verification
The existing tests that pass
Health 45 Agents Guard / guardexplicitly via--contextor as payload fixtures are left alone — they exercise arbitrary-context handling and do not depend on the default.🤖 Generated with Claude Code
Summary by CodeRabbit