Triage bot: have BOT_CLEANUP resolve merge conflicts against main - #4882
Merged
Conversation
… reporting them /pr-cleanup previously only gathered review feedback and CI status - a PR that was merely stale/conflicting against main (no comments, no CI) fell through as "nothing to address", even when GitHub reported it CONFLICTING (observed on PR #4036). Adds a new step 2 that merges origin/main into the branch and resolves any conflicts by hand, aborting and asking for a manual rebase instead of guessing when the conflict is too large to resolve safely. Grants the BOT_CLEANUP flow the git-merge permission it needs to act on this (no other flow checks out an existing branch that can be behind). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new git merge* permission is broader than intended and should be narrowed to origin/main-only spellings to align with the documented guardrails and reduce risk.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the triage bot’s /pr-cleanup workflow so BOT_CLEANUP can sync a PR branch with main (via git merge origin/main) and resolve merge conflicts, addressing cases where a PR has no CI/comments but is still unmergeable due to conflicts.
Changes:
- Extend BOT_CLEANUP’s allowed tool surface to include
git merge, enabling conflict resolution againstorigin/main. - Add regression tests ensuring the
git mergegrant is present only for BOT_CLEANUP. - Update the
pr-cleanupskill instructions to include a dedicated “sync with main / resolve conflicts” step, and add “footgun” to the cspell dictionary.
File summaries
| File | Description |
|---|---|
tools/triage_daemon.py |
Adds git merge to BOT_CLEANUP’s allowed tools so the flow can sync a PR branch with main. |
tools/test_triage_daemon.py |
Adds tests to confirm git merge is granted only to the cleanup flow. |
.cspell/custom-dictionary-workspace.txt |
Adds “footgun” to keep spell-checking clean for the new commentary. |
.claude/skills/pr-cleanup/SKILL.md |
Documents the new merge/conflict-resolution step and updates step numbering/guardrails accordingly. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Issue #4881: the first triage attempt against glm-5.3-flash:cloud completed its real work (comment posted, BOT_TRIAGED applied), then kept running until Claude Code's Anthropic-priced cost estimate crossed the $10 cap and aborted with a non-zero exit - which made the daemon retry an already-finished issue. claude_budget_args() omits --max-budget-usd entirely for whichever flows are running against an Ollama model; --max-turns remains the circuit-breaker there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ditor hangs - Narrow the BOT_CLEANUP git-merge permission from an unscoped "git merge*" to enumerated origin/main-scoped spellings (bare, --no-edit, --abort), matching pr-cleanup/SKILL.md's own "only ever merge origin/main" guardrail and this file's established pattern for the flag-before-target literal- prefix-matching footgun. - Switch the skill's example commands to --no-edit, both to stay covered by the narrowed allowlist and to avoid hanging on an interactive editor prompt with no one there to dismiss it. - Fix SKILL.md wording that implied a fast-forward merge always creates a merge commit, and a test docstring's awkward "reach without a use" phrasing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Investigated why
/pr-cleanup's run on PR #4036 didn't fix its conflicts (~/predbat-triage-bot/logs/pr-4036-cleanup.log) — GitHub reports that PR asmergeable: CONFLICTING, but the skill only ever gathered review feedback and CI status, so a PR with neither (like #4036: zero inline comments, no CI configured) fell through as "nothing to address" regardless of its merge state..claude/skills/pr-cleanup/SKILL.md: new step 2, "Sync with main and resolve conflicts" —git merge origin/main, then resolve any conflicts by hand (reading both sides' intent viagit log/git diff, never blindly keeping "ours" or "theirs", checking no conflict markers remain). If the conflict is extensive enough that a safe resolution would mean re-implementing significant logic, it aborts the merge and posts a comment asking for a manual rebase instead of guessing - same "don't guess, ask" philosophy the rest of the skill already follows for ambiguous review feedback. Renumbered the remaining steps and updated the quality-gate/commit-push logic to account for the merge commit (which may already exist locally before any other fix is implemented).tools/triage_daemon.py: addedBash(git merge*)toALLOWED_TOOLS_CLEANUP- without this the new instruction would have been silently denied at runtime, since no flow previously hadgit mergepermitted at all.origin/main*specifically).Test plan
python3 tools/test_triage_daemon.py— 146/146 passing, including new regression tests confirminggit merge*is granted toBOT_CLEANUPand to no other flow./run_pre_commit(coverage/) — clean, including the "triage daemon unit tests" hook and the full./run_all --quicksuite🤖 Generated with Claude Code