From 77e581299d22148773bd1dcb8dea23cc2d484663 Mon Sep 17 00:00:00 2001 From: Thomas SharedInbox Date: Sun, 24 May 2026 14:08:13 +0200 Subject: [PATCH] fix: filter out schedule/deploy workflow runs in CI checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _latest_main_ci_run() was using event != pull_request which still matched deploy.yml schedule runs when their prettyref == "main", blocking the loop from picking up new issues. _latest_ci_run_for_branch() had the same issue: the else branch matched any non-pull_request event including schedule runs. Both functions now explicitly filter for event == "push" only. Tests updated: rename _latest_ci_run → _latest_main_ci_run, mock _open_issue_prs to prevent real API calls in unit tests, and update _find_pr_for_branch side_effect to reflect the upstream post-merge PR-still-open verification check. Co-Authored-By: Claude Sonnet 4.6 --- scripts/agent_loop.py | 8 ++--- scripts/test_agent_loop.py | 63 ++++++++++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/scripts/agent_loop.py b/scripts/agent_loop.py index 3416b48..ca102bf 100755 --- a/scripts/agent_loop.py +++ b/scripts/agent_loop.py @@ -146,16 +146,16 @@ def _ready_issues() -> list[dict]: def _latest_main_ci_run() -> dict | None: - """Return the latest CI run on the main branch (excludes PR runs). + """Return the latest CI run on the main branch (excludes PR and schedule runs). Using the global latest run (limit=1) is wrong: a passing or failing run - on a PR branch could mask the true state of main. We filter to non-PR + on a PR branch could mask the true state of main. We filter to push events on the 'main' prettyref so section-3 logic only reacts to main. """ data = _tea_get(f"repos/{REPO}/actions/runs?limit=20") runs = (data or {}).get("workflow_runs", []) for run in runs: - if run.get("event") != "pull_request" and run.get("prettyref") == "main": + if run.get("event") == "push" and run.get("prettyref") == "main": return run return None @@ -177,7 +177,7 @@ def _latest_ci_run_for_branch(branch: str) -> dict | None: return run except (json.JSONDecodeError, AttributeError): pass - else: + elif run.get("event") == "push": if run.get("prettyref") == branch: return run return None diff --git a/scripts/test_agent_loop.py b/scripts/test_agent_loop.py index 362811d..a9fa391 100644 --- a/scripts/test_agent_loop.py +++ b/scripts/test_agent_loop.py @@ -200,7 +200,8 @@ class TestMain(unittest.TestCase): return 55 with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[self._make_issue(10)]), \ patch("agent_loop._set_labels", side_effect=fake_set_labels), \ patch("agent_loop._start_agent", side_effect=fake_start_agent), \ @@ -226,7 +227,8 @@ class TestMain(unittest.TestCase): captured["remove"] = remove with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[self._make_issue(7)]), \ patch("agent_loop._set_labels", side_effect=fake_set_labels), \ patch("agent_loop._start_agent", return_value=99), \ @@ -239,7 +241,8 @@ class TestMain(unittest.TestCase): def test_no_ready_issues_does_nothing(self): """main() exits cleanly with 0 when there are no ready issues.""" with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[]), \ patch("agent_loop._set_labels") as mock_labels, \ patch("agent_loop._start_agent") as mock_start: @@ -258,7 +261,8 @@ class TestMain(unittest.TestCase): return 77 with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[self._make_issue(42)]), \ patch("agent_loop._set_labels"), \ patch("agent_loop._start_agent", side_effect=fake_start_agent), \ @@ -292,8 +296,9 @@ class TestPendingCi(unittest.TestCase): def test_closes_issue_when_ci_passes_after_agent_finishes(self): """After issue agent finishes, loop merges the PR and closes the issue once CI is green.""" + # First call: PR found open. Second call (post-merge verification): PR closed. with patch("agent_loop._read_state", return_value=self._dead_state(10)), \ - patch("agent_loop._find_pr_for_branch", side_effect=self._find_pr_open), \ + patch("agent_loop._find_pr_for_branch", side_effect=[self._open_pr(), None]), \ patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 1, "status": "success"}), \ patch("agent_loop._merge_pr") as mock_merge, \ patch("agent_loop._close_issue") as mock_close, \ @@ -308,7 +313,7 @@ class TestPendingCi(unittest.TestCase): """'CI passed' line includes the CI run URL when a run is available.""" buf = io.StringIO() with patch("agent_loop._read_state", return_value=self._dead_state(10)), \ - patch("agent_loop._find_pr_for_branch", side_effect=self._find_pr_open), \ + patch("agent_loop._find_pr_for_branch", side_effect=[self._open_pr(), None]), \ patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 4145144, "status": "success"}), \ patch("agent_loop._merge_pr"), \ patch("agent_loop._close_issue"), \ @@ -418,7 +423,7 @@ class TestPendingCi(unittest.TestCase): def test_closes_issue_after_ci_fix_and_ci_passes(self): """After ci-fix agent finishes and CI passes on PR branch, the pending issue is closed.""" with patch("agent_loop._read_state", return_value=self._dead_state(10, "ci-fix")), \ - patch("agent_loop._find_pr_for_branch", side_effect=self._find_pr_open), \ + patch("agent_loop._find_pr_for_branch", side_effect=[self._open_pr(), None]), \ patch("agent_loop._latest_ci_run_for_branch", return_value={"id": 1, "status": "success"}), \ patch("agent_loop._merge_pr") as mock_merge, \ patch("agent_loop._close_issue") as mock_close, \ @@ -435,7 +440,8 @@ class TestPendingCi(unittest.TestCase): "pid": 999999999, "issue": None, "started_at": "2026-01-01T00:00:00+00:00", "type": "ci-fix", }), \ - patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "success"}), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value={"id": 1, "status": "success"}), \ patch("agent_loop._close_issue") as mock_close, \ patch("agent_loop._ready_issues", return_value=[]), \ patch("agent_loop._clear_state"): @@ -451,7 +457,8 @@ class TestOutputFormat(unittest.TestCase): def test_output_starts_with_header(self): buf = io.StringIO() with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[]), \ contextlib.redirect_stdout(buf): agent_loop._run_loop() @@ -462,7 +469,8 @@ class TestOutputFormat(unittest.TestCase): def test_no_agent_loop_prefix_in_output(self): buf = io.StringIO() with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[]), \ contextlib.redirect_stdout(buf): agent_loop._run_loop() @@ -472,7 +480,8 @@ class TestOutputFormat(unittest.TestCase): run = {"id": 4145144, "status": "running"} buf = io.StringIO() with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=run), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=run), \ contextlib.redirect_stdout(buf): agent_loop._run_loop() self.assertIn("https://codeberg.org/guettli/sharedinbox/actions/runs/4145144", @@ -482,7 +491,8 @@ class TestOutputFormat(unittest.TestCase): issue = {"number": 128, "title": "Fix something", "body": "", "labels": []} buf = io.StringIO() with patch("agent_loop._read_state", return_value=None), \ - patch("agent_loop._latest_ci_run", return_value=None), \ + patch("agent_loop._open_issue_prs", return_value=[]), \ + patch("agent_loop._latest_main_ci_run", return_value=None), \ patch("agent_loop._ready_issues", return_value=[issue]), \ patch("agent_loop._set_labels"), \ patch("agent_loop._start_agent", return_value=99), \ @@ -494,6 +504,35 @@ class TestOutputFormat(unittest.TestCase): self.assertIn("Fix something", output) +class TestLatestMainCiRun(unittest.TestCase): + """_latest_main_ci_run() must return only push-to-main runs, ignoring schedule/deploy workflows.""" + + def test_skips_schedule_runs_returns_push_to_main(self): + runs = [ + {"event": "schedule", "prettyref": "main", "status": "success", "id": 1}, + {"event": "push", "prettyref": "main", "status": "success", "id": 2}, + ] + with patch("agent_loop._tea_get", return_value={"workflow_runs": runs}): + result = agent_loop._latest_main_ci_run() + self.assertIsNotNone(result) + self.assertEqual(result["id"], 2) + + def test_returns_none_when_only_schedule_runs_exist(self): + runs = [ + {"event": "schedule", "prettyref": "main", "status": "success", "id": 1}, + ] + with patch("agent_loop._tea_get", return_value={"workflow_runs": runs}): + result = agent_loop._latest_main_ci_run() + self.assertIsNone(result) + + def test_returns_push_to_main_run(self): + runs = [{"event": "push", "prettyref": "main", "status": "running", "id": 42}] + with patch("agent_loop._tea_get", return_value={"workflow_runs": runs}): + result = agent_loop._latest_main_ci_run() + self.assertIsNotNone(result) + self.assertEqual(result["id"], 42) + + class TestLatestCiRunForBranch(unittest.TestCase): """Tests for _latest_ci_run_for_branch — Forgejo API field mapping."""