Repository navigation
ci(branch-cleanup): only delete the merged commit; keep heads of open PRs - #16
Conversation
… PRs From CodeRabbit review of the same workflow on sibling repos: - Keep the branch when another open PR uses it as its head, not only as its base. - Read the branch's current SHA first and keep it if it no longer points at the merged PR head (someone pushed after the merge). REST has no conditional delete, so only the instant between that read and the DELETE is unguarded; the comment says so. - Comment each job permission (zizmor undocumented-permissions). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012rYJxu978iUSGFZ1UuqTsi
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
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 |
🧪 Runtime gate✅ Runtime crawl: no runtime errors1 page(s) opened in a real browser (desktop + phone), safe controls exercised, forms tried with dummy data, links followed client-side, missing-route probe run. 37s. Coverage: 1/1 routes · 0/0 controls operated · 0/0 forms submitted · 29.3% of the app's own JavaScript executed Against the default branch: routes ±0, controls ±0, JS ±0 pts. No visual changes against the last default-branch run. Pages visited
AI explorer not run: no major/framework signal in this PR. Add the Screenshots, diffs and the app log: runtime-gate artifact. |
rishitank
left a comment
There was a problem hiding this comment.
Self-review by Claude. CodeRabbit is rate-limited on this repo right now, so there's no line-by-line review.
What I checked (.github/workflows/branch-cleanup.yml, read in full):
- New inputs.
HEAD_SHAcomes in throughenv:frompull_request.head.sha. No${{ }}insiderun:. - Head-PR check.
gh pr list --head, with the same output contract (a count) as the base check. The PR that just merged is closed, so it isn't counted. - SHA guard.
GET git/ref/heads/<encoded>then--jq .object.sha. A 404 means already deleted, so it exits 0. Any other error fails the job and keeps the branch. A mismatch withHEAD_SHAkeeps the branch with a notice. - Delete path. The
DELETEhandling is unchanged: 422/404 count as success, and anything else fails red. - Permissions. Each one now has an explanatory comment, and the scope is unchanged.
Tests. actionlint and shellcheck are clean, and actionlint passed in CI. The stub-gh matrix covers every path.
Fixed: nothing beyond the PR's own changes.
Residual, for Rishi: REST has no conditional delete, so the read and the DELETE are separate calls. A push that lands between them, a sub-second window, is not caught.
Why
#15 added
branch-cleanup.yml, and merging it deleted its own branch, as intended (run 37871307016: "Deleted branch ci/branch-cleanup."). CodeRabbit then reviewed the same file on sibling repos and raised two valid gaps:What changed
gh pr list --head), in addition to the existing base check.GET git/ref/heads/<ref>, URL-encoded) and keeps the branch, with a notice, when that SHA is no longerpull_request.head.sha. A 404 there means the branch is already gone, which counts as success.DELETEisn't caught. A comment in the workflow says so.undocumented-permissions).{}workflow permissions,env:-only inputs, and 422/404 onDELETEtreated as already deleted.Checks
ghruns pass for every path: allowlist, base PR, head PR, ref already gone, ref moved, 500 on read (fails), delete OK, 422 on delete, 403 on delete (fails).ci/branch-cleanupagain, this time after the SHA check.🤖 Generated with Claude Code
https://claude.ai/code/session_012rYJxu978iUSGFZ1UuqTsi