fix(ci): only bypass admin review for dependabot patch/minor - #134
Conversation
admin-approval.yml previously short-circuited to success for ALL Dependabot PRs. That worked while only admins produced APPROVED reviews, but CodeRabbit started auto-approving — and a major-bump Dependabot PR (#127) reached mergeStateStatus=CLEAN with no admin involvement: CodeRabbit's review satisfied the count rule, the unconditional bypass satisfied the status check. Changes: - admin-approval.yml: new fetch-metadata step (continue-on-error, Dependabot-gated). Bypass only fires for update-type in {patch, minor}. Major / unknown / empty falls through to the same admin-review evaluation as human PRs. Fail-safe: a metadata-fetch error leaves UPDATE_TYPE empty → no bypass → blocked PR, not auto-merged bump. - housekeeping.yml: exempt Dependabot PRs from the 72-char title cap. Format regex still enforced (Dependabot already uses lowercase deps:/ci:/docs: prefixes). Closes the half of #130 about long grouped-update titles. - AGENTS.md: update gate description in three places to reflect the new "patch/minor bypass; majors require admin review" model. Note that count:0 in the ruleset is intentional — Admin approval status is the single admin-review gate; a bot's APPROVED review is no longer load-bearing. Ruleset change (do after merge): gh api -X PUT repos/Wave-RF/WaveHouse/rulesets/15353356 \ -F 'rules[3].parameters.required_approving_review_count=0' (or via GitHub UI on ruleset 15353356; keep all other rule params unchanged, including dismiss_stale_reviews_on_push:true.) Closes #130. The go-mod-tidy half of the issue is intentionally deferred — manual `make tidy && go mod tidy` after a failing CI run is the accepted workflow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the CI automation logic to close a security gap where major Dependabot version bumps could be merged without human admin review due to bot-generated approvals. It introduces granular bypass logic for the admin-approval workflow and relaxes title length constraints for automated dependency updates, while maintaining strict standards for human-authored pull requests. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThis PR refines Dependabot CI and merge gate behavior: the admin-approval workflow now conditionally bypasses only for patch/minor updates (via metadata fetch and UPDATE_TYPE check), housekeeping exempts Dependabot from title length limits while preserving format validation, and AGENTS.md documents the new governance rules. ChangesDependabot-aware CI and merge gates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@AGENTS.md`:
- Around line 344-349: Add an [Unreleased] CHANGELOG.md entry describing the
governance/workflow change: note that
`.github/workflows/dependabot-automerge.yml` now auto-approves and auto-merges
Dependabot patch/minor bumps (bypassing the `Admin approval` status check),
while major bumps require human admin approval (per `admin-approval.yml`) and
are not auto-bypassed; include that the ruleset's
`required_approving_review_count` remains 0 and that the Admin approval status
check is the single admin-review gate, and place this under the "Changed"
subsection of the [Unreleased] section.
🪄 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 Plus
Run ID: 42e50e2c-421c-4a1e-9ccb-b3042ff72425
📒 Files selected for processing (3)
.github/workflows/admin-approval.yml.github/workflows/housekeeping.ymlAGENTS.md
|
1 [MUST], 0 [SHOULD], 0 [MAY] — see inline thread for detail. The workflow logic is correct: the One gap: Iterate — add a |
There was a problem hiding this comment.
Code Review
This pull request updates AGENTS.md to document refined governance procedures for Dependabot, specifically closing a security gap by requiring manual admin approval for major version updates while maintaining auto-merge for patch and minor bumps. It also explains the repository's reliance on a custom status check over the native review count rule and notes a title-length exemption for Dependabot. Feedback from the review includes a requirement to document these notable changes in the CHANGELOG.md as per the style guide and a suggestion to clarify which workflow file implements the approval bypass logic for better documentation consistency.
Resolved conflicts in admin-approval.yml, housekeeping.yml, and AGENTS.md between main's #134 patch/minor-only Dependabot bypass and this branch's comment-tightening pass. Took main's new logic (UPDATE_TYPE env var, scoped bypass, Dependabot title-length exemption) and re-tightened the accompanying comments / AGENTS.md prose to match this branch's style. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Closes #130 (the title-length and admin-bypass halves; the go-mod-tidy half is intentionally deferred — manual
make tidy && go mod tidyafter a failing CI run is the accepted workflow).admin-approval.ymlpreviously short-circuited tosuccessfor all Dependabot PRs, on the assumption that "no bot approves itself + no auto-merge enabled = major bumps will sit." That assumption broke once CodeRabbit started auto-approving Dependabot PRs after autofixes. PR #127 (major-bump: TypeScript 5→6, Astro 5→6, etc.) reachedmergeStateStatus: CLEAN, reviewDecision: APPROVED— one click from main — with no admin review:APPROVEDreview satisfied the ruleset'srequired_approving_review_count: 1Admin approval: successChanges
admin-approval.ymlFetch Dependabot metadatastep at the top, gated on Dependabot author (continue-on-error: trueso a metadata fetch failure on a Dependabot PR fail-safes to "no bypass" rather than erroring the workflow).update-typeisversion-update:semver-patchorversion-update:semver-minor. Majors, unknown types, and emptyUPDATE_TYPE(metadata fetch failed) fall through to the same admin-review evaluation as human PRs.housekeeping.ymldeps:/ci:/docs:prefixes, so they pass naturally. Fixes the half of bug: dependabot PRs don't always pass CI #130 about title-length blocking grouped bumps (e.g., deps: bump github.com/nats-io/nats.go from 1.51.0 to 1.52.0 in the go-deps group #128's 80-char title).AGENTS.mdrequired_approving_review_count: 0in the ruleset is intentional — theAdmin approvalstatus check is the single admin-review gate, so any bot'sAPPROVEDreview is no longer load-bearing.Ruleset change (manual, after merge)
The workflow changes alone close the security hole (majors now post
Admin approval: pending, blocking merge). The ruleset count-rule change is defense-in-depth — without it, CodeRabbit's approval can still satisfycount: 1on a major bump even though theAdmin approvalstatus is now correctly pending.Easiest path is the GitHub UI: Settings → Rules → main branch protection (ruleset 15353356) → Require a pull request before merging → set "Required approving reviews" to 0. All other rule params should stay unchanged — especially
dismiss_stale_reviews_on_push: true,strict_required_status_checks_policy: true, the required status checks list, and the admin bypass actor.(
gh apiworks too but requires fetching the current ruleset, modifying therequired_approving_review_countfield inside thepull_requestrule, and PUT-ing the full payload back. The UI is one click.)Behavior matrix after this PR (workflow only, before ruleset change)
After the ruleset change (count → 0), the third row stays blocked regardless of CodeRabbit's approval state, closing the defense-in-depth gap.
Test plan
Admin approvalstatus postspendingand the PR is blocked until an admin approves.count: 0) and verify the same major PR remains blocked even if CodeRabbit's review isAPPROVED.Notes
housekeeping.yml's reviewer-assign skip-for-Dependabot logic, but it's still correct:dependabot-automerge.ymlalready assigns both admins on majors, so housekeeping's skip avoids a double-request. Left as-is.APPROVEDtoCOMMENTED) is intentionally NOT part of this PR — it's a separate dashboard setting, not a repo change.🤖 Generated with Claude Code
Summary by CodeRabbit