Skip to content

BOT_REVIEW and BOT_CLEANUP flows for pull requests - #4747

Merged
springfall2008 merged 2 commits into
mainfrom
feat/bot-pr-review-cleanup
Aug 26, 2026
Merged

BOT_REVIEW and BOT_CLEANUP flows for pull requests#4747
springfall2008 merged 2 commits into
mainfrom
feat/bot-pr-review-cleanup

Conversation

@springfall2008

Copy link
Copy Markdown
Owner

Summary

Extends the triage bot's label-driven flows to pull requests, reusing all existing daemon infrastructure (polling, permission-set derivation, label-based state, failure handling).

  • BOT_REVIEW on a PR (a second meaning — on an issue it's unchanged) runs the built-in /code-review <pr> high --comment, posting findings as inline PR comments. Comment-only: the review permission set adds nothing but a scoped gh api grant, no write/push/commit access at all.
  • BOT_CLEANUP on a PR (new) runs a new .claude/skills/pr-cleanup/SKILL.md skill that addresses outstanding review feedback (from any source — human reviewers, Copilot, a prior bot review) and CI failures. It's built on superpowers:receiving-code-review's evaluation discipline: verify before implementing, push back with reasoning on anything wrong or unclear. That skill assumes a live human partner to consult; where it says to stop and ask, the daemon version posts a comment explaining what's unclear and stops instead. Runs ./run_pre_commit + the relevant test as a quality gate before committing, then pushes to the PR's own branch and replies in-thread (not as new top-level comments) to addressed review comments.

Both flows need gh api access — inline PR review comments aren't visible via gh pr view (I hit this gap myself reviewing PR #4740 interactively), and thread replies need the REST endpoint directly (.../pulls/{pr}/comments/{id}/replies). Every existing flow currently blocks gh api entirely, so this carves out a narrow exception scoped to this repo only (Bash(gh api repos/springfall2008/batpred/*)), the same technique already used to carve git push/git commit out of the base denial list for the /issue-pr flow.

Same label lifecycle as everything else: success removes the trigger label; a failed invocation swaps to the existing BOT_FAILED with an explanatory comment rather than retrying every poll cycle.

Before this is usable

The BOT_CLEANUP label needs creating on the repo (BOT_REVIEW/BOT_FAILED already exist from earlier work). /code-review's exact CLI argument syntax (/code-review <pr> high --comment) is my best inference from its description — it's a built-in command with no readable source, so this needs a live end-to-end check before relying on it.

Test plan

  • python3 tools/test_triage_daemon.py -v — 84/84 passing
  • ./run_pre_commit (full suite) — clean
  • coverage/venv quick suite (unit_test.py --quick) — clean, 0 failures
  • End-to-end: add BOT_REVIEW to a real PR and confirm /code-review's argument syntax and comment-posting actually work as invoked
  • End-to-end: add BOT_CLEANUP to a real PR with outstanding feedback and confirm the fix/push/reply flow

BOT_REVIEW gains a second meaning when applied to a PR (as opposed to
an issue, unchanged): it runs the built-in /code-review command at
the "high" effort level with --comment, posting findings as inline
PR comments. Comment-only - no write/push/commit access at all.

BOT_CLEANUP (new) runs a new /pr-cleanup skill that addresses review
feedback (from any source: human reviewers, Copilot, a prior bot
review) and CI failures, guided by superpowers:receiving-code-review
for evaluating feedback rather than blindly implementing it. Where
that skill would ask a human partner, the daemon version posts a
comment and stops instead, since there's no one to ask. Runs
pre-commit/tests as a quality gate before committing, then pushes to
the PR's own branch and replies in-thread to addressed comments.

Both flows need gh api access (inline PR comments aren't visible via
gh pr view, and thread replies need the REST endpoint directly),
which every existing flow currently blocks outright. Scoped to this
repo only (Bash(gh api repos/springfall2008/batpred/*)), carved out
of the blanket gh api denial the same way git push/commit already are
for the PR flow.
Copilot AI lite review requested due to automatic review settings August 25, 2026 18:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The intended scoping of gh api access is not actually enforced due to the existing blanket Bash(gh *) allowlist, materially expanding permissions beyond what the PR describes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Extends the existing label-driven triage daemon to support pull requests, adding a PR review flow (BOT_REVIEW) that runs /code-review and a PR cleanup flow (BOT_CLEANUP) that runs a new /pr-cleanup skill for addressing feedback/CI failures.

Changes:

  • Add PR polling and orchestration for BOT_REVIEW and BOT_CLEANUP labels in the triage daemon, including per-PR log files and label swap-on-failure behavior.
  • Introduce new allow/deny permission sets intended to permit limited gh api usage for PR review comments and thread replies.
  • Add a new .claude/skills/pr-cleanup skill describing the cleanup workflow (fetch feedback, fix, quality gate, push, reply-in-thread).
File summaries
File Description
tools/triage_daemon.py Adds PR polling + new BOT_REVIEW/BOT_CLEANUP flows and new tool permission sets for review/cleanup runs.
tools/test_triage_daemon.py Adds unit tests covering the new PR URL helper, PR fetch/label functions, and the new permission-set deltas.
.claude/skills/pr-cleanup/SKILL.md Adds a new skill definition and step-by-step procedure for cleanup-on-PR (fix, run quality gate, push, reply to threads).
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/triage_daemon.py
Confirmed valid: removing Bash(gh api*) from the deny list while
_ALLOWED_TOOLS_BASE still carried the catch-all Bash(gh *) meant the
new narrow allow (Bash(gh api repos/.../*)) did nothing - the broad
allow already covered every gh api call once the specific deny was
lifted, including calls against other repos and dangerous endpoints
(merge/close via REST) that the explicit gh pr merge*/close* denials
never touch since those match on CLI subcommand text, not the
underlying API surface. Deny-wins-over-allow is the only precedence
rule this permission model has; a narrower allow gets no priority
over a broader one.

Fix: ALLOWED_TOOLS_REVIEW/ALLOWED_TOOLS_CLEANUP no longer inherit the
blanket gh grant. They list the specific gh subcommands each flow
actually needs instead, so there's no catch-all for the scoped gh api
entry to hide behind. Split _ALLOWED_TOOLS_BASE into
_ALLOWED_TOOLS_NON_GH (unchanged, still used by every existing flow)
plus the gh wildcard, kept separate so the new flows can take the
non-gh portion without the broad gh grant.

While rebuilding this properly, tightened two more things noticed
along the way: dropped "gh pr review*" from the review flow (would
have allowed formal --approve/--request-changes, not just comments),
and stopped BOT_CLEANUP inheriting "gh pr create*" from
_ALLOWED_TOOLS_PR_EXTRA (it pushes to the existing PR's branch, never
opens a new one).

New regression tests assert the specific gh entries in each flow's
allowlist exactly, and explicitly assert "Bash(gh *)" is absent from
both - the class of test that would have caught this the first time.
@springfall2008
springfall2008 merged commit 59c8374 into main Aug 26, 2026
2 checks passed
@springfall2008
springfall2008 deleted the feat/bot-pr-review-cleanup branch August 26, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants