ci: only auto-merge self-authored PRs, drop custom review check - #242
Conversation
Replaces the closed require-review-for-others.yml with a simpler, standard approach: auto-merge just never enables for anyone other than TRUSTED_PR_AUTHORS (default getappz). Devin/external PRs sit mergeable-when-ready and get reviewed and merged by hand -- how popular solo/small-team repos actually handle this, no custom enforcement logic, no exploit surface.
📝 WalkthroughWalkthroughThe auto-merge workflow now requires pull request authors to match the configurable ChangesAuto-merge gating
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/auto-merge.yml (1)
30-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
TRUSTED_PR_AUTHORSparsing is whitespace-sensitive.The comma-wrapping technique
format(',{0},', ...)correctly prevents substring false positives (e.g.,getappzmatchinggetappz2), but it does not tolerate spaces after commas. If someone setsTRUSTED_PR_AUTHORSto"getappz, some-other-login", the lookup string becomes",getappz, some-other-login,"and the search for",some-other-login,"will fail, silently excluding that author from auto-merge.The inline example on line 29 uses no spaces, which is correct, but a user following common comma-separator conventions might add spaces. Consider adding a brief note in the comment (e.g., "no spaces after commas") or stripping spaces in the expression.
💡 Optional: document the no-spaces requirement in the comment
- # gh variable set TRUSTED_PR_AUTHORS --body "getappz,some-other-login" + # gh variable set TRUSTED_PR_AUTHORS --body "getappz,some-other-login" + # (no spaces after commas — the match is comma-delimited and exact)🤖 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/auto-merge.yml around lines 30 - 34, Update the TRUSTED_PR_AUTHORS handling in the auto-merge condition to tolerate whitespace after comma separators, while preserving exact author matching and preventing substring matches. Alternatively, document the required no-spaces format in the nearby workflow comment and example if parsing remains unchanged.
🤖 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.
Nitpick comments:
In @.github/workflows/auto-merge.yml:
- Around line 30-34: Update the TRUSTED_PR_AUTHORS handling in the auto-merge
condition to tolerate whitespace after comma separators, while preserving exact
author matching and preventing substring matches. Alternatively, document the
required no-spaces format in the nearby workflow comment and example if parsing
remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e6b70f4-acd6-4a37-9cc8-f1445fa94b32
📒 Files selected for processing (1)
.github/workflows/auto-merge.yml
Summary
Simplification after discussion — replaces the closed #241 (a custom required-status-check that re-implemented author-conditional review) with the standard practice popular solo/small-team repos actually use:
auto-merge.ymlnow only auto-enables merge forTRUSTED_PR_AUTHORS(default: justgetappz), on top of the existing fork exclusionpull_requesttriggers), no branch-protection review-count deadlockTest plan
if:condition shape as the fork-exclusion already proven working in production (ci: auto-enable merge-when-ready on every PR open #240)Summary by CodeRabbit