From d12fd37ce58ea223fd6c75a9fd7daaf7e7e70035 Mon Sep 17 00:00:00 2001 From: Clayde Date: Wed, 8 Apr 2026 09:17:29 +0000 Subject: [PATCH] Fix #66: Prune closed issues from state on each tick Add _prune_closed_issues() helper that checks each state.json entry against the GitHub API and removes any whose issue is closed. Called once per tick before the main processing loop, ensuring closed issues never accumulate in state and are never acted on. Co-Authored-By: Claude Sonnet 4.6 --- src/clayde/orchestrator.py | 33 ++++++++++++++- tests/test_orchestrator.py | 84 +++++++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/clayde/orchestrator.py b/src/clayde/orchestrator.py index 0221074..3ede831 100644 --- a/src/clayde/orchestrator.py +++ b/src/clayde/orchestrator.py @@ -37,7 +37,7 @@ parse_issue_url, ) from clayde.safety import get_new_visible_comments, has_visible_content, is_plan_approved -from clayde.state import IssueStatus, get_issue_state, load_state, update_issue_state +from clayde.state import IssueStatus, get_issue_state, load_state, save_state, update_issue_state from clayde.tasks import implement, plan, review from clayde.telemetry import get_tracer, init_tracer @@ -220,6 +220,31 @@ def _has_new_comments(g: Github, owner: str, repo: str, number: int, issue_state return bool(get_new_visible_comments(comments, last_seen)) +def _prune_closed_issues(g: Github, issues_state: dict) -> None: + """Remove closed issues from state to prevent stale entries accumulating.""" + to_prune = [] + for url, ist in issues_state.items(): + owner = ist.get("owner") + repo = ist.get("repo") + number = ist.get("number") + if not owner or not repo or not number: + continue + try: + issue = fetch_issue(g, owner, repo, number) + if issue.state == "closed": + to_prune.append(url) + except Exception as e: + log.warning("[%s] Failed to check issue state for pruning: %s — skipping", _issue_label(ist), e) + + if to_prune: + state = load_state() + for url in to_prune: + ist = issues_state[url] + log.info("[%s] Pruning closed issue from state", _issue_label(ist)) + state["issues"].pop(url, None) + save_state(state) + + def main(): settings = get_settings() @@ -256,6 +281,12 @@ def main(): tick_span.set_attribute("issues.assigned_count", len(assigned)) + # Prune closed issues from state before any other processing + _prune_closed_issues(g, issues_state) + + # Reload state after pruning + issues_state = load_state().get("issues", {}) + if not assigned: log.info("No assigned issues. Going back to sleep.") provider.force_flush() diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index ae0b283..9053d69 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -10,6 +10,7 @@ _handle_new_issue, _handle_pr_open, _has_new_comments, + _prune_closed_issues, main, ) @@ -157,7 +158,7 @@ def test_recovers_transient_state_to_interrupted(self, transient_status): patch("clayde.orchestrator.is_claude_available", return_value=True), \ patch("clayde.orchestrator.get_github_client"), \ patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", side_effect=[state, recovered_state]), \ + patch("clayde.orchestrator.load_state", side_effect=[state, state, recovered_state]), \ patch("clayde.orchestrator.update_issue_state") as mock_update, \ patch("clayde.orchestrator._handle_interrupted") as mock_handle: main() @@ -455,3 +456,84 @@ def test_no_new_comments(self): patch("clayde.orchestrator.get_new_visible_comments", return_value=[]): entry = {"last_seen_comment_id": 100} assert _has_new_comments(g, "o", "r", 1, entry) is False + + +class TestPruneClosedIssues: + def _make_issue_state(self, owner="o", repo="r", number=1): + return {"owner": owner, "repo": repo, "number": number} + + def test_prunes_closed_issue(self): + g = MagicMock() + gh_issue = MagicMock() + gh_issue.state = "closed" + issues_state = {"https://github.com/o/r/issues/1": self._make_issue_state()} + with patch("clayde.orchestrator.fetch_issue", return_value=gh_issue), \ + patch("clayde.orchestrator.load_state", return_value={"issues": dict(issues_state)}), \ + patch("clayde.orchestrator.save_state") as mock_save: + _prune_closed_issues(g, issues_state) + saved = mock_save.call_args[0][0] + assert "https://github.com/o/r/issues/1" not in saved["issues"] + + def test_keeps_open_issue(self): + g = MagicMock() + gh_issue = MagicMock() + gh_issue.state = "open" + issues_state = {"https://github.com/o/r/issues/1": self._make_issue_state()} + with patch("clayde.orchestrator.fetch_issue", return_value=gh_issue), \ + patch("clayde.orchestrator.save_state") as mock_save: + _prune_closed_issues(g, issues_state) + mock_save.assert_not_called() + + def test_skips_entry_missing_fields(self): + g = MagicMock() + issues_state = {"url1": {"status": "done"}} # no owner/repo/number + with patch("clayde.orchestrator.fetch_issue") as mock_fetch, \ + patch("clayde.orchestrator.save_state") as mock_save: + _prune_closed_issues(g, issues_state) + mock_fetch.assert_not_called() + mock_save.assert_not_called() + + def test_skips_on_api_error(self): + g = MagicMock() + issues_state = {"https://github.com/o/r/issues/1": self._make_issue_state()} + with patch("clayde.orchestrator.fetch_issue", side_effect=Exception("API error")), \ + patch("clayde.orchestrator.save_state") as mock_save: + _prune_closed_issues(g, issues_state) + mock_save.assert_not_called() + + def test_prunes_multiple_closed_in_one_save(self): + g = MagicMock() + gh_issue = MagicMock() + gh_issue.state = "closed" + url1 = "https://github.com/o/r/issues/1" + url2 = "https://github.com/o/r/issues/2" + issues_state = { + url1: self._make_issue_state(number=1), + url2: self._make_issue_state(number=2), + } + with patch("clayde.orchestrator.fetch_issue", return_value=gh_issue), \ + patch("clayde.orchestrator.load_state", return_value={"issues": dict(issues_state)}), \ + patch("clayde.orchestrator.save_state") as mock_save: + _prune_closed_issues(g, issues_state) + # Only one save call (batched) + assert mock_save.call_count == 1 + saved = mock_save.call_args[0][0] + assert url1 not in saved["issues"] + assert url2 not in saved["issues"] + + def test_main_calls_prune(self): + """main() calls _prune_closed_issues before processing issues.""" + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + state = {"issues": {}} + with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ + patch("clayde.orchestrator.setup_logging"), \ + patch("clayde.orchestrator.init_tracer"), \ + patch("clayde.orchestrator.is_claude_available", return_value=True), \ + patch("clayde.orchestrator.get_github_client"), \ + patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ + patch("clayde.orchestrator.load_state", return_value=state), \ + patch("clayde.orchestrator._prune_closed_issues") as mock_prune, \ + patch("clayde.orchestrator._handle_new_issue"): + main() + mock_prune.assert_called_once()