From bb475a235053b91c1072746b816e5f2a59924726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bot=20of=20Thomas=20G=C3=BCttler?= Date: Mon, 25 May 2026 19:38:07 +0200 Subject: [PATCH] fix: auto-resolve merge failures instead of asking for manual merge (#253) (#256) --- scripts/agent_loop.py | 53 ++++++++++++++++++++++++ scripts/test_agent_loop.py | 84 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/scripts/agent_loop.py b/scripts/agent_loop.py index ba0bce1..1f92a59 100755 --- a/scripts/agent_loop.py +++ b/scripts/agent_loop.py @@ -42,6 +42,7 @@ import re import shlex import subprocess import sys +import time from datetime import datetime, timezone from pathlib import Path @@ -278,6 +279,41 @@ def _merge_pr(pr_number: int) -> None: _fgj("pr", "merge", str(pr_number), "--repo", REPO, "--merge-method", "squash") +def _handle_pr_still_open_after_merge(pr_number: int, branch: str, issue_num: int | None) -> str: + """Handle a PR that is still open after a successful _merge_pr() call. + + Returns one of: + "rebase-spawned" — merge conflict detected; rebase agent started, state written + "merged" — PR closed after a retry + "fallback" — all options exhausted; caller should set State/Question + """ + pr_data = _tea_get(f"repos/{REPO}/pulls/{pr_number}") + mergeable = (pr_data or {}).get("mergeable") + + if mergeable is False: + prompt = ( + f"Rebase branch `{branch}` onto main to resolve merge conflicts, then push. " + "Do not change any logic — only resolve conflicts and push." + ) + session_name = f"rebase-pr-{pr_number}" + pid = _start_agent(prompt, session_name) + _write_state(pid, issue_num, "pending-ci", session_name=session_name) + print(f"PR #{pr_number} has merge conflicts — spawned rebase agent (pid={pid}).") + return "rebase-spawned" + + for attempt in range(1, 3): + time.sleep(5) + try: + _merge_pr(pr_number) + except RuntimeError as e: + print(f"PR #{pr_number} merge retry {attempt} failed: {e}") + if not _find_pr_for_branch(branch): + print(f"PR #{pr_number} merged on retry {attempt}.") + return "merged" + + return "fallback" + + # ── state file ──────────────────────────────────────────────────────────────── @@ -676,6 +712,13 @@ def _run_loop() -> int: ) return 0 if _find_pr_for_branch(branch): + merge_result = _handle_pr_still_open_after_merge(pr_number, branch, pending_issue) + if merge_result == "rebase-spawned": + return 0 + if merge_result == "merged": + _close_issue(pending_issue) + print(f"Merged PR #{pr_number} and closed {_issue_url(pending_issue)}.") + return 0 print(f"PR #{pr_number} is still open after merge attempt — setting to State/Question.") _set_labels(pending_issue, add=[LABEL_QUESTION], remove=[LABEL_IN_PROGRESS]) _comment_issue( @@ -744,6 +787,16 @@ def _run_loop() -> int: # Verify the merge actually happened; fgj can exit 0 without merging # (e.g. branch-protection rules not satisfied). if _find_pr_for_branch(branch): + merge_result = _handle_pr_still_open_after_merge(pr_number, branch, issue_num) + if merge_result == "rebase-spawned": + return 0 + if merge_result == "merged": + if issue_num: + _close_issue(issue_num) + print(f"Catch-up: merged PR #{pr_number} and closed issue #{issue_num} after retry.") + else: + print(f"Catch-up: merged PR #{pr_number} after retry.") + return 0 print( f"Catch-up: PR #{pr_number} is still open after merge attempt " "— skipping to avoid infinite retry." diff --git a/scripts/test_agent_loop.py b/scripts/test_agent_loop.py index 57e1ef7..d32e878 100644 --- a/scripts/test_agent_loop.py +++ b/scripts/test_agent_loop.py @@ -785,6 +785,90 @@ class TestCatchupSkipsQuestionIssues(unittest.TestCase): mock_merge.assert_called_once_with(50) +class TestMergeFailsOpen(unittest.TestCase): + """Tests for auto-resolution when a PR is still open after the merge command.""" + + def _dead_state(self, issue: int, kind: str = "issue") -> dict: + return { + "pid": 999999999, + "issue": issue, + "started_at": "2026-01-01T00:00:00+00:00", + "type": kind, + } + + def _open_pr(self, branch: str = "issue-10-fix") -> dict: + return {"number": 5, "head": {"ref": branch}, "created_at": "2026-01-01T00:00:00+00:00"} + + def test_merge_fails_open_with_conflicts_spawns_rebase_agent(self): + """mergeable=false → rebase agent spawned, state written as pending-ci.""" + written_state = {} + + def fake_write_state(pid, issue, kind, issue_title=None, session_name=None, ci_run_id=None): + written_state["pid"] = pid + written_state["issue"] = issue + written_state["kind"] = kind + written_state["session_name"] = session_name + + with patch("agent_loop._read_state", return_value=self._dead_state(10)), \ + patch("agent_loop._find_pr_for_branch", side_effect=[self._open_pr(), self._open_pr()]), \ + patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 1, "status": "success"}), \ + patch("agent_loop._merge_pr"), \ + patch("agent_loop._tea_get", return_value={"mergeable": False}), \ + patch("agent_loop._start_agent", return_value=77) as mock_start, \ + patch("agent_loop._write_state", side_effect=fake_write_state), \ + patch("agent_loop._clear_state"): + result = agent_loop._run_loop() + + self.assertEqual(result, 0) + mock_start.assert_called_once() + prompt = mock_start.call_args[0][0] + self.assertIn("Rebase branch", prompt) + self.assertIn("issue-10-fix", prompt) + self.assertEqual(written_state.get("kind"), "pending-ci") + self.assertEqual(written_state.get("issue"), 10) + + def test_merge_fails_open_no_conflicts_retries_and_succeeds(self): + """mergeable=true, second attempt succeeds → issue closed.""" + with patch("agent_loop._read_state", return_value=self._dead_state(10)), \ + patch("agent_loop._find_pr_for_branch", + side_effect=[self._open_pr(), self._open_pr(), None]), \ + patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 1, "status": "success"}), \ + patch("agent_loop._merge_pr"), \ + patch("agent_loop._tea_get", return_value={"mergeable": True}), \ + patch("agent_loop.time.sleep"), \ + patch("agent_loop._close_issue") as mock_close, \ + patch("agent_loop._clear_state"): + result = agent_loop._run_loop() + + self.assertEqual(result, 0) + mock_close.assert_called_once_with(10) + + def test_merge_fails_open_no_conflicts_all_retries_exhausted(self): + """All retries exhausted with PR still open → falls through to State/Question.""" + with patch("agent_loop._read_state", return_value=self._dead_state(10)), \ + patch("agent_loop._find_pr_for_branch", + side_effect=[self._open_pr(), self._open_pr(), + self._open_pr(), self._open_pr()]), \ + patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 1, "status": "success"}), \ + patch("agent_loop._merge_pr"), \ + patch("agent_loop._tea_get", return_value={"mergeable": True}), \ + patch("agent_loop.time.sleep"), \ + patch("agent_loop._set_labels") as mock_labels, \ + patch("agent_loop._comment_issue") as mock_comment, \ + patch("agent_loop._close_issue") as mock_close, \ + patch("agent_loop._clear_state"): + result = agent_loop._run_loop() + + self.assertEqual(result, 0) + mock_close.assert_not_called() + mock_labels.assert_called_once_with( + 10, + add=[agent_loop.LABEL_QUESTION], + remove=[agent_loop.LABEL_IN_PROGRESS], + ) + mock_comment.assert_called_once() + + class TestHeartbeat(unittest.TestCase): """Tests for _update_heartbeat() and cmd_monitor()."""