ci: machine-block needs-* labels at the PR gate (#3751) - #3754
Conversation
M4 capability enforcement (epic #3612): the "no needs-* label may merge" rule has been prose policy in CLAUDE.md with no gate, so PRs carrying needs-deep-review have merged and shipped regressions this session (#3637 -> #3687/#3703, #3627 -> #3728/#3738). Add .github/workflows/needs-label-gate.yml: a new, additive pull_request check (opened/synchronize/labeled/unlabeled/reopened) that fails when the PR carries any needs-* label and reports the offending label(s) by name. It reads labels straight from the PR event payload (no hardcoded list) and re-evaluates on label add/ remove so it clears automatically. Least-privilege permissions (contents: read, pull-requests: read). Register the lane in policy/ci-lane-whitelist.toml so workflow-policy-lint's advisory lane-whitelist check has an entry for it. Does not touch the two existing required checks (Perl LSP Rust Small Result, ripr+ New Gap Gate) or pr-title-check.yml's needs-issue-link handling. Making this check *required* (branch-protection admin) is an explicit external blocker, out of scope here -- see PR body. Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Reviewer's GuideAdds a new CI workflow that fails PRs carrying any Sequence diagram for the new needs-label-gate CI workflowsequenceDiagram
participant GitHub
participant Needs_Label_Gate as needs-label-gate_workflow
participant GithubScript as actions_github_script
GitHub->>Needs_Label_Gate: pull_request [opened/synchronize/labeled/unlabeled/reopened]
Needs_Label_Gate->>GithubScript: run script
GithubScript->>GithubScript: extract pull_request.labels
GithubScript->>GithubScript: blockers = labels.filter(name.startsWith('needs-'))
alt [blockers.length > 0]
GithubScript->>GitHub: core.setFailed("Merge gate: needs-* label(s) present")
else [no needs-* labels]
GithubScript->>GitHub: core.info("No needs-* labels present. Gate clear.")
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughAdds a GitHub Actions workflow that evaluates ChangesNeeds-label merge gate
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Droid finished @EffortlessSteven's task —— View job Phase 2 (validation) complete. Summary: LGTM. The new needs-label-gate workflow correctly implements the M4 capability using a read-only actions/github-script on pull_request events with minimal permissions, and the matching lane whitelist entry follows the established schema (frontdoor/hygiene/self_hosted_workflow_nano, blocking=false until admin-promoted). No high-confidence actionable issues found: trigger types cover all label-state transitions, no concurrency block correctly avoids LABEL_EVENT_CANCELS_PR_RUN, the startsWith('needs-') filter matches the documented intent (including needs-issue-link as explicitly called out), and the lane entry is structurally consistent with siblings like pr_title_check and pr_plan. Validated: 0 candidates (none generated in Pass 1) No inline review comments to post. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5191ec0571
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/needs-label-gate.yml:
- Around line 23-36: Add a concurrency block to the needs-label-gate workflow,
using a pull-request-specific group derived from the workflow name and PR
number, and cancel-in-progress: true so newer label events cancel stale runs
before they report status.
- Line 39: Pin the actions/github-script step to the immutable commit SHA
3a2844b7e9c422d3c10d287c895573f7108da1b3 instead of the floating `@v9` reference
in the merge-gate workflow.
In `@policy/ci-lane-whitelist.toml`:
- Around line 1084-1086: Remove the duplicate consecutive [[lane]] table header
near the needs_label_gate entry, leaving exactly one header associated with id =
"needs_label_gate" so the TOML contains no empty lane element.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f511167-6788-4725-ac3f-cf99b17bdd26
📒 Files selected for processing (2)
.github/workflows/needs-label-gate.ymlpolicy/ci-lane-whitelist.toml
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Fixes real findings from the first review pass on PR #3754: - P1 (chatgpt-codex): bot-authored needs-* label changes made via a workflow's own GITHUB_TOKEN (pipeline-labels.yml/pr-title-check.yml) don't trigger labeled/unlabeled pull_request events for OTHER workflows -- a documented GitHub Actions suppression. Add a workflow_run trigger (on Pipeline Labels / PR Title Check completion) that re-fetches live PR label state and re-evaluates, closing the gap without touching either label-writer workflow. - Major (coderabbit): add a concurrency block so rapid label events don't leave a stale status. Gated cancel-in-progress on `github.event.action == 'synchronize'` only (not unconditional true) to avoid tripping this repo's own workflow-policy-lint LABEL_EVENT_CANCELS_PR_RUN rule for labeled/unlabeled-triggered workflows -- verified via a local `cargo xtask workflow-policy-lint --check-lane-whitelist` run (0 errors, 0 new warnings). - Minor (coderabbit): pin actions/github-script to the commit SHA for v9.0.0 (3a2844b7e9c422d3c10d287c895573f7108da1b3), independently verified via `gh api repos/actions/github-script/git/tags/v9.0.0` dereferenced through the tag object to the commit. Not fixed (verified false positive, see review reply): the claimed duplicate `[[lane]]` header in policy/ci-lane-whitelist.toml is not present in the file -- confirmed by direct read and a clean `tomllib.load` parse (47 lane entries, single needs_label_gate entry). Not fixed (out of scope, unrelated to this PR): CI Gate shard (meta) failure is pre-existing `cargo fmt` debt in 4 crates this PR's 2-file diff never touches (perl-parser-core, perl-workspace, perl-lsp-rs, perl-core-test-runner) and is advisory only per .ci/policies/required-checks.toml (only Perl LSP Rust Small Result and ripr+ New Gap Gate are required). Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
|
Droid finished @EffortlessSteven's task —— View job Summary: LGTM. The head SHA (92f1c32) has addressed every prior reviewer finding: the Validated: 0 candidates (none generated in Pass 1) No inline review comments to post. |
Follow-up on review findings (head now 92f1c32)Fixed 3 real findings, verified 1 false positive, and root-caused the
All 4 review threads replied-to and resolved. Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA |
Maintainer review found a real architectural defect in the previous workflow_run backstop: core.setFailed() fails the job against GITHUB_SHA, which for workflow_run events is the DEFAULT BRANCH head, not the PR head (GitHub Actions docs, "Events that trigger workflows"). So when Pipeline Labels/PR Title Check add or remove a needs-* label via their own GITHUB_TOKEN (suppressing the normal labeled/unlabeled pull_request retrigger), the previous backstop would fail on main while the PR's own check stayed stale-green -- exactly the bypass this gate exists to close. Re-architected so every path (pull_request and workflow_run) resolves a PR number, fetches the LIVE PR (head sha + labels) via `pulls.get` (never trusts a payload sha/label snapshot), and posts a commit status (context "needs-label-gate") to that resolved head via `repos.createCommitStatus`. Added narrowly-scoped `statuses: write` (no other write permission). core.setFailed() is now only called for the pull_request path (correctly attached there already); the workflow_run path relies solely on the posted commit status so it never reddens main's own status. Verified before implementing: - `gh api repos/.../rulesets` -> a "merge-queue" ruleset exists but `enforcement: "disabled"` -- merge_group events don't fire on this repo today, so no merge_group handling was added (documented in the workflow header; revisit if that ruleset is ever enabled). - `gh api repos/.../branches/main/protection` -> both existing required checks are bound to `app_id: 15368` (GitHub Actions app). createCommitStatus called with the workflow's own GITHUB_TOKEN is attributed to that same app, so this context will bind correctly if registered as required later. Concurrency group now resolves the PR number for the workflow_run case too (from the raw payload, since expressions can't call the API), not github.run_id. Proof: extracted the exact resolve->fetch->post logic into a standalone Node dry-run against a mocked pulls.get/createCommitStatus harness. 8 cases confirm the status always targets the resolved PR head sha and never the (distinct, fixture) default-branch sha -- covering a direct pull_request label add, a bot-added needs-deep-review via workflow_run (the defect scenario), and a bot-cleared needs-issue-link via workflow_run. Re-ran `cargo xtask workflow-policy-lint --check-lane-whitelist` locally: 0 errors, 0 new warnings (statuses: write and the concurrency expression don't trip any local lint rule). Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
|
Droid encountered an error —— View job |
Maintainer review: gh pr checks 3754 showed "needs-label-gate" TWICE with the same context string -- the job's own check-run (job id happened to equal the status context) and the commit status this job posts. If an admin later registers "needs-label-gate" as a required context, that ambiguity is dangerous: a stale-green pull_request-path check-run could satisfy the required context even after the workflow_run path posts a failing commit-status for a bot-added label, reopening the exact bypass this gate exists to close. Rename the job id from `needs-label-gate` to `post-needs-label-status` so its own check-run carries a distinct context. The `context: 'needs-label-gate'` string passed to createCommitStatus is unchanged -- that status remains the sole carrier of the "needs-label-gate" context, from both the pull_request and workflow_run paths. Updated the policy/ci-lane-whitelist.toml lane entry's `job` field to match (and noted the workflow_run trigger in allowed_triggers/evidence, which was accurate but not yet reflected there). Verified: `cargo xtask workflow-policy-lint --check-lane-whitelist` locally -- 0 errors, 0 new warnings (in particular no STALE_WHITELIST_JOB regression from the rename, since the lane entry was updated in the same commit). Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
|
Droid finished @EffortlessSteven's task —— View job Summary: LGTM. The head SHA (95d6453) reflects all prior reviewer findings resolved: the Validated: 0 candidates (none generated in Pass 1) No inline review comments to post. |
…3771) Review/audit agent profiles previously carried no tools: restriction, so they inherited every tool (Edit/Write/NotebookEdit/Agent/mutating-Bash) — only a prompt-level "REVIEW ONLY" instruction kept them from writing. That is not mechanical enforcement (workflow subagents run in acceptEdits and inherit parent tools). M4b makes the read-only boundary mechanical + regression-guarded: - Config: 18 review/audit agents get an explicit fail-closed tools: allowlist (Read, Grep, Glob, Bash, WebSearch, WebFetch, TodoWrite), excluding Edit/Write/NotebookEdit/MultiEdit/Agent — the Explore shape. Writers (builder, pr-responder, green-*, red-tdd, ops, lead-*, spec-planner) are untouched. - Machine check: `cargo xtask check-agent-capabilities` parses .claude/agents/*.md and fails if any review/audit agent grants a write/mutating tool or lacks an explicit allowlist. Wired as a #[test] (runs under cargo test) + a new Agent Capability Gate workflow + a just target. Fails on a deliberately-broken fixture, passes on the corrected set. - Read-only shell: pre-tool-use.sh rejects mutating git/gh/filesystem commands before execution when CLAUDE_AGENT_READONLY=1, while read-only inspection (git diff, gh pr view, cargo check) passes. Guard test: .claude/hooks/tests/test_pre_tool_use_readonly.sh. Claim boundary: M4b only (mechanical read-only boundary + machine check + negative demonstration). Does not cover M4a (label gate #3754) or M5 (build isolation). Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
Intent
Repo-authorable half of M4 (epic #3612): make the CLAUDE.md rule "no
needs-*label may merge" machine-checkable instead of prose. Today a PR carryingneeds-deep-reviewcan and has merged past that rule.Controlling issue
#3751 (M4 capability enforcement; program state #3750).
Audit of existing workflows (done before writing anything)
.github/workflows/pr-title-check.yml— ownsneeds-issue-link(adds it when a title uses the(#0000)placeholder, self-clears it onedited/synchronizeonce a real issue number is present). This PR does not touch that file or that label's lifecycle..github/workflows/pipeline-labels.yml— ownsin-review/needs-deep-review/merge-readytransitions on PR review events. Not touched..ci/policies/label-contradictions.toml+.github/workflows/methodology-gate.yml— already encode a narrower related rule (merge-ready/auto-mergecannot coexist withneeds-*), but it only fires whenmerge-ready/auto-mergeis also present, runs in--enforce-less (non-blocking) mode, and doesn't re-trigger onlabeled/unlabeled. No existing workflow implements "fail on ANYneeds-*label, independent ofmerge-ready, re-evaluated on every label change (including bot-authored ones)." That's the gap this PR fills.Scope and architecture (revised twice during review — see below)
New, additive workflow:
.github/workflows/needs-label-gate.yml. Triggers:pull_request: [opened, synchronize, labeled, unlabeled, reopened]andworkflow_run(onPipeline Labels/PR Title Checkcompletion — see "bot-authored label backstop" below).The verdict is published as a commit status (Statuses API, context
needs-label-gate) on the resolved PR head SHA, fetched live viapulls.getfor every triggering path. This design went through two review-driven corrections, both real defects caught by independent review (not self-found):core.setFailed()in apull_request/workflow_run-triggered job. Maintainer review caught that forworkflow_runevents, the job's ownGITHUB_SHAis the default-branch head, not the PR head (GitHub Actions docs) — so a bot-authored label change (pipeline-labels.yml/pr-title-check.ymlwriting with their ownGITHUB_TOKEN, which suppresseslabeled/unlabeledretriggers for other workflows) would fail the job againstmainwhile the PR's own check stayed stale-green. Fixed by resolving the PR number, fetching the live PR (head SHA + labels) viapulls.get, and posting an explicitcreateCommitStatusto that resolved head — this is now the sole authoritative signal for theworkflow_runpath. Added narrowly-scopedstatuses: write(only new permission;contents/pull-requestsstay read-only).needs-label-gate, from the job id) as the commit status it posts. If a required-check admin later registersneeds-label-gate, a stale-greenpull_request-path check-run could satisfy that context even after theworkflow_runpath posts a failing status — reopening the exact bypass this gate exists to close. Fixed by renaming the job id topost-needs-label-status; only the commit status carriesneeds-label-gatenow. Verified viagh pr checks 3754: the context appears exactly once (0s duration, no/job/URL suffix — i.e. the status, not a check-run).Permissions:
contents: read,pull-requests: read,statuses: write(least privilege for what the mechanism needs — no label-write permission anywhere).Added a
[[lane]]entry topolicy/ci-lane-whitelist.tomlfor this workflow (job field kept in sync with the rename) so the advisoryworkflow-policy-lint --check-lane-whitelistgate doesn't flag it as unregistered.Bot-authored label backstop
GitHub suppresses
labeled/unlabeledpull_requesttriggers for events caused by a workflow's ownGITHUB_TOKEN. Bothpipeline-labels.yml(adds/removesneeds-deep-review) andpr-title-check.yml(adds/removesneeds-issue-link) write labels with their default token. Theworkflow_runtrigger re-checks live label state after either workflow completes and posts the same commit status to the real PR head — closing the gap without touching either file. Note:workflow_runtrigger definitions are read from the default branch, so this backstop only becomes fully active once this file lands onmain; the directpull_requesttriggers already cover the common (human-driven) case on this PR itself.needs-issue-linkinteractionThis gate deliberately includes
needs-issue-linkin what it blocks — no carve-out, per the issue's explicit instruction that the policy is "noneeds-*may merge." A PR opened with the(#0000)placeholder will show this gate red until the title is corrected, at which pointpr-title-check.yml's existing self-clear logic removes the label and this gate's next re-evaluation (viasynchronize/labeled/unlabeled, or theworkflow_runbackstop) goes green.Merge queue and required-check app binding (verified live, not assumed)
gh api repos/.../rulesets→ amerge-queueruleset exists butenforcement: "disabled"—merge_groupevents don't fire on this repo today, so nomerge_grouphandling was added (would be dead code; documented in the workflow header, revisit if that ruleset is ever enabled).gh api repos/.../branches/main/protection→ the two existing required checks are bound toapp_id: 15368(the GitHub Actions app).createCommitStatuscalled with the workflow's ownGITHUB_TOKENis attributed to that same app, soneeds-label-gatewill bind correctly if/when an admin registers it as required.Non-goals (per issue #3751)
needs-*labels exist or their semantics.Perl LSP Rust Small Result,ripr+ New Gap Gate) or.ci/policies/required-checks.toml.needs-label-gatecontext to the branch-protection required-checks list (or ruleset) for this to actually block merge. Until then it is visible red/green but advisory.pipeline-labels.ymlorpr-title-check.yml.Behavioral proof
yaml.safe_load,tomllib.load).cargo xtask workflow-policy-lint --check-lane-whitelistrun locally after every revision: 0 errors, 0 new warnings throughout (19 pre-existing warnings, all in unrelated files). Confirms: noLABEL_EVENT_CANCELS_PR_RUNviolation (concurrencycancel-in-progressis conditioned ongithub.event.action == 'synchronize', not unconditional), noSTALE_WHITELIST_JOBregression after the job rename (lane entry updated in the same commit).needs-*,needs-issue-linkspecifically,merge-ready+needs-*coexisting, label removed) plus 4 cases against theworkflow_runre-fetch path (no associated PRs, bot-added label caught, bot-cleared label clears, multiple associated PRs) plus 8 cases proving the commit status always targets the resolved PR head SHA, using deliberately distinct fixture SHAs for "PR head" vs. "default branch head" to prove theworkflow_runpath never targets the latter (the exact defect from review pass 1).gh pr checks 3754shows the mechanism working against the real head SHA, not just mocks. Final state on head95d645360:needs-label-gateappears exactly once (confirmed viagrep -c), carried solely by the commit status.What was not run
95d645360): on this PR — clean →needs-label-gatecommit-status pass ("No needs-* labels present"); addedneeds-deep-review→ fail ("Blocked by: needs-deep-review") on the head; removed it → pass again. The authoritative signal is the commit-status (0s, no/job/URL); the informational job check-run is separately namedpost-needs-label-status.grep -c "^needs-label-gate"ongh pr checks= 1.xtaskfor the lint checks (CI-config change; no Rust production code path touched).Claim boundary
This PR ships the mechanism — a workflow that correctly resolves the PR head SHA (including for bot-authored,
GITHUB_TOKEN-suppressed label changes) and posts a single, unambiguous commit status for it — proven by dry-run plus live evidence on this PR. It does not claim the merge gate is enforced; that requires the one remaining admin step described above.Risk & rollback
Purely additive: a new workflow file plus an advisory-policy TOML entry. No existing job, permission, or required check is modified. Rollback is
git revert; no other workflow depends on this one.Remaining work
needs-label-gatestatus context as a required check onmain(binds toapp_id: 15368, already verified compatible).needs-deep-review(both as a human and, ideally, by exercising thepipeline-labels.ymlbot path) → confirm merge is blocked by GitHub itself → remove the label → confirm it clears and merge becomes available.label-contradictions.toml's narrowermerge-ready/auto-mergevsneeds-*check into one canonical enforcement point in a follow-up.Claude-Session: https://claude.ai/code/session_011o9wuB2nsoT5fF6NmATgSA
Claim boundary (honest)
This gate is event-driven and eventually-consistent, NOT an atomic merge rule. A prior
successremains on an unchanged commit SHA until a label-triggered (orworkflow_run) run posts the new verdict — so a stale-green interval can exist between a label add and the re-post. The correct claim is: the required status converges to current label state through the covered event paths. It is not yet: a PR carryingneeds-*is physically incapable of merging under every interleaving. Closing the bot-write window (setneeds-label-gatepending on the head before label mutation via a canonical status publisher, then publish the final verdict after) is tracked as a follow-up; the human-label interval (labels change without changing SHA) relies additionally on review-convergence + auto-merge discipline. Merge this as advisory M4a machinery; do NOT make it required until the bot-path is live-proven on a scratch PR and the publisher hardening lands.Enforcement authority (verified via API, 2026-07-11)
The two current required checks are in classic branch protection (
repos/.../branches/main/protection/required_status_checks), bound toapp_id 15368(GitHub Actions),strict=false. The activemainruleset (16664791) carries NO required_status_checks (onlyrequired_review_thread_resolution). So the admin registration target is classic branch protection (add contextneeds-label-gate, GitHub-Actions source), preserving the existing two + strictness — unless the maintainer chooses to migrate status checks to the ruleset.