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()