feat: close issues in Python loop after CI passes, not in agent (#134)

Previously issue agents were instructed to close the issue via prompt text
immediately after pushing. If CI then failed, the issue was already closed.

Now the loop tracks a pending_issue across cron ticks:
- When an agent finishes (issue or ci-fix), the issue number is extracted
  from state before it is cleared.
- If CI is still running, a "pending-ci" state preserves the issue number.
- If CI fails, the ci-fix agent is started with the issue number in state
  so it survives the fix cycle.
- Once CI passes, _close_issue() is called from Python — never by the agent.

The agent prompt no longer instructs the agent to close the issue.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

scripts/agent_loop.py

@@ -7,12 +7,15 @@ Flow
 1. Agent already running?
    a. Age > 1 h  → kill it, set its issue to State/Question, exit 1
    b. Age ≤ 1 h  → print status, exit 0  (let it keep working)
-2. No agent running → check Codeberg CI
-   a. CI is running         → print "CI running, waiting", exit 0
-   b. Latest CI failed      → start fix-CI agent, save state, exit 0
-   c. CI ok (or no run yet) → find oldest Ready issue, start issue agent,
+2. No agent running → extract pending_issue from state (if any), then check CI
+   a. CI is running         → save pending-ci state, exit 0
+   b. Latest CI failed      → start fix-CI agent (preserving pending_issue), exit 0
+   c. CI ok + pending_issue → close the issue (CI passed), exit 0
+   d. CI ok (or no run yet) → find oldest Ready issue, start issue agent,
                               save state, exit 0
-   d. No Ready issues       → print "nothing to do", exit 0
+   e. No Ready issues       → print "nothing to do", exit 0
+
+Issue agents must NOT close the issue themselves; the loop closes it after CI passes.
 
 State file: ~/.sharedinbox-agent-state.json
   { "pid": 12345, "issue": 91,
@@ -154,7 +157,7 @@ def _read_state() -> dict | None:
     return None
 
 
-def _write_state(pid: int, issue: int | None, kind: str, issue_title: str | None = None) -> None:
+def _write_state(pid: int | None, issue: int | None, kind: str, issue_title: str | None = None) -> None:
     data: dict = {
         "pid": pid,
         "issue": issue,
@@ -320,8 +323,10 @@ def _run_loop() -> int:
         )
         return 0
 
-    # Agent not running (or no state) — clean up stale state.
+    # Agent not running (or no state) — extract any pending issue, then clean up.
+    pending_issue: int | None = None
     if state:
+        pending_issue = state.get("issue")
         _clear_state()
 
     # ── 2. Check CI ───────────────────────────────────────────────────────────
@@ -329,6 +334,8 @@ def _run_loop() -> int:
 
     if run and run.get("status") == "running":
         print(f"CI run {_ci_run_url(run['id'])} is still running. Waiting.")
+        if pending_issue:
+            _write_state(None, pending_issue, "pending-ci")
         return 0
 
     if run and run.get("status") in ("failure", "error"):
@@ -342,10 +349,16 @@ def _run_loop() -> int:
             "When done, stop."
         )
         pid = _start_agent(prompt, "ci-fix")
-        _write_state(pid, None, "ci-fix")
+        _write_state(pid, pending_issue, "ci-fix")
         return 0
 
-    # CI is ok (or no run) — find a Ready issue.
+    # CI is ok (or no run).
+    if pending_issue:
+        _close_issue(pending_issue)
+        print(f"CI passed — closed {_issue_url(pending_issue)}.")
+        return 0
+
+    # Find a Ready issue.
     issues = _ready_issues()
     if not issues:
         print("No issues with State/Ready. Nothing to do.")
@@ -382,7 +395,7 @@ Instructions:
 - Push to origin/main.
 - If you hit a blocker you cannot resolve, set the issue label to State/Question
   and stop (do NOT close the issue).
-- When the work is done and pushed, close the issue and stop.
+- When the work is done and pushed, stop. The loop will close the issue after CI passes.
 """
 
     pid = _start_agent(prompt, f"issue-{issue_number}")

scripts/test_agent_loop.py

@@ -223,6 +223,129 @@ class TestMain(unittest.TestCase):
         mock_labels.assert_not_called()
         mock_start.assert_not_called()
 
+    def test_prompt_does_not_tell_agent_to_close_issue(self):
+        """Agents must not close issues; the loop handles closing after CI passes."""
+        captured_prompt = {}
+
+        def fake_start_agent(prompt, session_name):
+            captured_prompt["prompt"] = prompt
+            return 77
+
+        with patch("agent_loop._read_state", return_value=None), \
+             patch("agent_loop._latest_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), \
+             patch("agent_loop._write_state"):
+            agent_loop._run_loop()
+
+        prompt = captured_prompt.get("prompt", "")
+        # "do NOT close the issue" (blocker instruction) is fine; what must be
+        # absent is any affirmative instruction to close on completion.
+        self.assertNotIn("close the issue and stop", prompt.lower())
+
+
+class TestPendingCi(unittest.TestCase):
+    """Tests for the pending-CI state: issue closed only after CI passes."""
+
+    def _dead_state(self, issue: int, kind: str = "issue") -> dict:
+        return {
+            "pid": 999999999,  # non-existent PID
+            "issue": issue,
+            "started_at": "2026-01-01T00:00:00+00:00",
+            "type": kind,
+        }
+
+    def test_closes_issue_when_ci_passes_after_agent_finishes(self):
+        """After issue agent finishes, loop closes the issue once CI is green."""
+        with patch("agent_loop._read_state", return_value=self._dead_state(10)), \
+             patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "success"}), \
+             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_does_not_close_issue_when_ci_fails(self):
+        """After issue agent finishes, loop must NOT close the issue if CI failed."""
+        with patch("agent_loop._read_state", return_value=self._dead_state(10)), \
+             patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "failure"}), \
+             patch("agent_loop._close_issue") as mock_close, \
+             patch("agent_loop._start_agent", return_value=55), \
+             patch("agent_loop._write_state"), \
+             patch("agent_loop._clear_state"):
+            result = agent_loop._run_loop()
+
+        self.assertEqual(result, 0)
+        mock_close.assert_not_called()
+
+    def test_saves_pending_ci_state_while_ci_running(self):
+        """When CI is still running after agent finishes, pending issue is preserved."""
+        written = {}
+
+        def fake_write_state(pid, issue, kind, issue_title=None):
+            written["pid"] = pid
+            written["issue"] = issue
+            written["kind"] = kind
+
+        with patch("agent_loop._read_state", return_value=self._dead_state(10)), \
+             patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "running"}), \
+             patch("agent_loop._write_state", side_effect=fake_write_state), \
+             patch("agent_loop._clear_state"):
+            result = agent_loop._run_loop()
+
+        self.assertEqual(result, 0)
+        self.assertEqual(written.get("issue"), 10)
+        self.assertEqual(written.get("kind"), "pending-ci")
+        self.assertIsNone(written.get("pid"))
+
+    def test_ci_fix_preserves_pending_issue_in_state(self):
+        """When CI fails after agent finishes, ci-fix state includes the pending issue."""
+        written = {}
+
+        def fake_write_state(pid, issue, kind, issue_title=None):
+            written["pid"] = pid
+            written["issue"] = issue
+            written["kind"] = kind
+
+        with patch("agent_loop._read_state", return_value=self._dead_state(10)), \
+             patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "failure"}), \
+             patch("agent_loop._start_agent", return_value=55), \
+             patch("agent_loop._write_state", side_effect=fake_write_state), \
+             patch("agent_loop._clear_state"):
+            result = agent_loop._run_loop()
+
+        self.assertEqual(result, 0)
+        self.assertEqual(written.get("issue"), 10)
+        self.assertEqual(written.get("kind"), "ci-fix")
+
+    def test_closes_issue_after_ci_fix_and_ci_passes(self):
+        """After ci-fix agent finishes and CI passes, the pending issue is closed."""
+        with patch("agent_loop._read_state", return_value=self._dead_state(10, "ci-fix")), \
+             patch("agent_loop._latest_ci_run", return_value={"id": 1, "status": "success"}), \
+             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_no_pending_issue_ci_fix_without_issue(self):
+        """ci-fix for a manual push (no pending issue) does not try to close anything."""
+        with patch("agent_loop._read_state", return_value={
+            "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._close_issue") as mock_close, \
+             patch("agent_loop._ready_issues", return_value=[]), \
+             patch("agent_loop._clear_state"):
+            result = agent_loop._run_loop()
+
+        self.assertEqual(result, 0)
+        mock_close.assert_not_called()
+
 
 class TestOutputFormat(unittest.TestCase):
     """Verify output format: no [agent_loop] prefix, URLs in output."""