When a catch-up PR merge fails (PR stays open after the merge command), the loop sets the issue to State/Question and comments on it. But on the next cron tick the same PR is still open with passing CI, so it tries again — spamming the issue with identical comments every minute. Fix: before attempting a catch-up merge, fetch the issue's current labels via `_get_issue_labels()`. If `State/Question` is already set, skip the PR entirely. Closes #239 Co-authored-by: Thomas SharedInbox <sharedinbox@thomas-guettler.de> Reviewed-on: https://codeberg.org/guettli/sharedinbox/pulls/242
This commit was merged in pull request #242.
This commit is contained in:
committed by
guettli
co-authored by
guettli
Thomas SharedInbox
parent
32ba916cbf
commit
27bef3356e
@@ -263,6 +263,14 @@ def _latest_ci_run_for_pr(pr_number: int) -> dict | None:
|
||||
return None
|
||||
|
||||
|
||||
def _get_issue_labels(issue: int) -> list[str]:
|
||||
"""Return label names for an issue."""
|
||||
data = _tea_get(f"repos/{REPO}/issues/{issue}")
|
||||
if not data:
|
||||
return []
|
||||
return [lbl["name"] for lbl in data.get("labels", [])]
|
||||
|
||||
|
||||
def _merge_pr(pr_number: int) -> None:
|
||||
"""Squash-merge a PR via fgj."""
|
||||
_fgj("pr", "merge", str(pr_number), "--repo", REPO, "--merge-method", "squash")
|
||||
@@ -684,6 +692,9 @@ def _run_loop() -> int:
|
||||
continue
|
||||
|
||||
if pr_run and pr_run.get("status") == "success":
|
||||
if issue_num and LABEL_QUESTION in _get_issue_labels(issue_num):
|
||||
print(f"Catch-up: PR #{pr_number} — issue #{issue_num} is State/Question, skipping.")
|
||||
continue
|
||||
print(f"Catch-up: CI passed on PR #{pr_number} ({pr_url}) — merging.")
|
||||
try:
|
||||
_merge_pr(pr_number)
|
||||
|
||||
@@ -744,5 +744,45 @@ class TestRunLoopResumeCommand(unittest.TestCase):
|
||||
self.assertNotIn("Resume:", output)
|
||||
|
||||
|
||||
|
||||
class TestCatchupSkipsQuestionIssues(unittest.TestCase):
|
||||
"""Catch-up must not retry merging a PR whose issue is already State/Question."""
|
||||
|
||||
def _make_pr(self, pr_number=50, branch="issue-10-fix"):
|
||||
return {"number": pr_number, "head": {"ref": branch}}
|
||||
|
||||
def test_skips_merge_when_issue_has_question_label(self):
|
||||
pr = self._make_pr()
|
||||
ci_run = {"id": 999, "status": "success"}
|
||||
with patch("agent_loop._read_state", return_value=None), \
|
||||
patch("agent_loop._open_issue_prs", return_value=[pr]), \
|
||||
patch("agent_loop._latest_ci_run_for_pr", return_value=ci_run), \
|
||||
patch("agent_loop._get_issue_labels", return_value=[agent_loop.LABEL_QUESTION]), \
|
||||
patch("agent_loop._merge_pr") as mock_merge, \
|
||||
patch("agent_loop._comment_issue") as mock_comment, \
|
||||
patch("agent_loop._set_labels") as mock_labels, \
|
||||
patch("agent_loop._latest_main_ci_run", return_value=None), \
|
||||
patch("agent_loop._ready_issues", return_value=[]):
|
||||
result = agent_loop._run_loop()
|
||||
self.assertEqual(result, 0)
|
||||
mock_merge.assert_not_called()
|
||||
mock_comment.assert_not_called()
|
||||
mock_labels.assert_not_called()
|
||||
|
||||
def test_proceeds_with_merge_when_issue_lacks_question_label(self):
|
||||
pr = self._make_pr()
|
||||
ci_run = {"id": 999, "status": "success"}
|
||||
with patch("agent_loop._read_state", return_value=None), \
|
||||
patch("agent_loop._open_issue_prs", return_value=[pr]), \
|
||||
patch("agent_loop._latest_ci_run_for_pr", return_value=ci_run), \
|
||||
patch("agent_loop._get_issue_labels", return_value=[agent_loop.LABEL_IN_PROGRESS]), \
|
||||
patch("agent_loop._merge_pr") as mock_merge, \
|
||||
patch("agent_loop._find_pr_for_branch", return_value=None), \
|
||||
patch("agent_loop._close_issue"):
|
||||
result = agent_loop._run_loop()
|
||||
self.assertEqual(result, 0)
|
||||
mock_merge.assert_called_once_with(50)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user