Merge pull request #67 from BraveNewCapital/copilot/fix-execution-reconciliation-bugs

Make `closed_unmerged` terminal in repo-architect execution and fix execution artifact upload

Author: Steake

.github/workflows/repo-architect-execution.yml

@@ -125,8 +125,9 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: repo-architect-execution-${{ github.run_id }}
-          path: .agent
+          path: .agent/work_state.json
           if-no-files-found: warn
+          include-hidden-files: true
           retention-days: 7
 
       - name: Write workflow summary

repo_architect.py

@@ -2328,9 +2328,9 @@ def select_ready_issue(
         if it.get("lane"):
             blocked_lanes.add(it["lane"])
 
-    # Also block superseded/blocked items by fingerprint
+    # Block terminal, superseded, or explicitly blocked items by identity.
     for it in items:
-        if it.get("blocked") or it.get("superseded"):
+        if it.get("merged") or it.get("closed_unmerged") or it.get("blocked") or it.get("superseded"):
             if it.get("fingerprint"):
                 blocked_fingerprints.add(it["fingerprint"])
             if it.get("issue_number"):
@@ -2953,18 +2953,25 @@ def reconcile_pr_state(
         new_item["updated_at"] = now
         if pr_class == "merged":
             new_item["merged"] = True
+            new_item["closed_unmerged"] = False
             new_item["delegation_state"] = "delegation-confirmed"
             new_item["lifecycle_fact_state"] = "merged"
             new_item["lifecycle_inferred_state"] = "completed"
         elif pr_class == "closed_unmerged":
+            new_item["merged"] = False
             new_item["closed_unmerged"] = True
+            new_item["delegation_state"] = "delegation-failed"
             new_item["lifecycle_fact_state"] = "closed-unmerged"
             new_item["lifecycle_inferred_state"] = "needs-replanning"
         elif pr_class == "draft":
+            new_item["merged"] = False
+            new_item["closed_unmerged"] = False
             new_item["delegation_state"] = "delegation-confirmed"
             new_item["lifecycle_fact_state"] = "pr-draft"
             new_item["lifecycle_inferred_state"] = "in-progress"
         elif pr_class == "open":
+            new_item["merged"] = False
+            new_item["closed_unmerged"] = False
             new_item["delegation_state"] = "delegation-confirmed"
             new_item["lifecycle_fact_state"] = "pr-open"
             new_item["lifecycle_inferred_state"] = "in-progress"
@@ -3077,6 +3084,32 @@ def _active_fingerprints_in_work_state(work_state: Dict[str, Any]) -> Set[str]:
     }
 
 
+def _execution_no_selection_message(
+    reconcile_result: Dict[str, Any], work_state: Dict[str, Any]
+) -> str:
+    """Explain why execution found nothing ready when reconciliation terminalized items."""
+    closed_unmerged_issues = sorted({
+        int(detail["issue"])
+        for detail in reconcile_result.get("details", [])
+        if detail.get("pr_state") == "closed_unmerged" and detail.get("issue") is not None
+    })
+    if not closed_unmerged_issues:
+        closed_unmerged_issues = sorted({
+            int(it["issue_number"])
+            for it in work_state.get("items", [])
+            if it.get("closed_unmerged") and it.get("issue_number") is not None
+        })
+    if not closed_unmerged_issues:
+        return "No ready issues available for delegation."
+
+    issue_list = ", ".join(f"#{issue_number}" for issue_number in closed_unmerged_issues)
+    noun = "issue remains" if len(closed_unmerged_issues) == 1 else "issues remain"
+    return (
+        f"No ready issues available for delegation. Tracked {noun} "
+        f"in closed-unmerged state ({issue_list}) and require planner intervention."
+    )
+
+
 def run_execution_cycle(config: Config) -> Dict[str, Any]:
     """Execute one execution-lane pass: select + delegate one ready issue.
 
@@ -3103,14 +3136,15 @@ def run_execution_cycle(config: Config) -> Dict[str, Any]:
     selected = select_ready_issue(config, work_state)
     if selected is None:
         save_work_state(config, work_state)
+        message = _execution_no_selection_message(reconcile_result, work_state)
         return {
             "status": "execution_cycle_complete",
             "mode": EXECUTION_MODE,
             "dry_run": not config.enable_live_delegation,
             "selected_issue": None,
             "delegation": None,
             "reconcile": reconcile_result,
-            "message": "No ready issues available for delegation.",
+            "message": message,
         }
 
     delegation_result = delegate_to_copilot(config, selected, work_state, run_id)

tests/test_repo_architect.py

@@ -2644,6 +2644,29 @@ def test_lifecycle_labels_exclude_issue(self) -> None:
         issue_labels = {"arch-gap", "copilot-task", "needs-implementation", "in-progress"}
         self.assertTrue(issue_labels & blocking, "in-progress should block selection")
 
+    def test_closed_unmerged_work_state_excludes_issue_even_if_labels_lag(self) -> None:
+        """Terminal closed-unmerged work items must stay ineligible even before label sync."""
+        config = self._make_config_exec(
+            github_token="tok", github_repo="x/y", max_concurrent_delegated=5
+        )
+        fp = "aabbccddeeff"
+        issue = self._make_issue(number=12, body=f"<!-- arch-gap-fingerprint: {fp} -->")
+        ws: Dict[str, Any] = {
+            "items": [{
+                "fingerprint": fp,
+                "issue_number": 12,
+                "delegation_state": "delegation-failed",
+                "merged": False,
+                "closed_unmerged": True,
+                "lane": "import_cycles",
+                "blocked": False,
+                "superseded": False,
+            }]
+        }
+        with patch.object(ra, "_list_github_issues_by_labels", return_value=[issue]):
+            result = ra.select_ready_issue(config, ws)
+        self.assertIsNone(result)
+
     def test_priority_ordering(self) -> None:
         """Priority critical < high < medium < low in rank (lower index = higher priority)."""
         rank = {p: i for i, p in enumerate(ra.ISSUE_PRIORITY_LEVELS)}
@@ -2869,6 +2892,76 @@ def test_run_execution_cycle_no_credentials_returns_no_selection(self) -> None:
         self.assertEqual(result["status"], "execution_cycle_complete")
         self.assertIsNone(result["selected_issue"])
 
+    def test_run_execution_cycle_reports_closed_unmerged_as_non_ready(self) -> None:
+        """Execution output should truthfully report closed-unmerged items as non-ready."""
+        with tempfile.TemporaryDirectory() as tmp:
+            root = _make_git_root(tmp)
+            config = dataclasses.replace(
+                _make_config(root, mode="execution"),
+                github_token="tok",
+                github_repo="x/y",
+            )
+            ws: Dict[str, Any] = {
+                "items": [{
+                    "fingerprint": "aabbccddeeff",
+                    "issue_number": 42,
+                    "lane": "import_cycles",
+                    "delegation_state": "ready-for-delegation",
+                    "merged": False,
+                    "closed_unmerged": False,
+                    "blocked": False,
+                    "superseded": False,
+                    "pr_number": None,
+                    "pr_url": None,
+                    "pr_state": None,
+                    "updated_at": "2025-01-01T00:00:00+00:00",
+                    "objective": "",
+                    "assignee": None,
+                    "created_at": "2025-01-01T00:00:00+00:00",
+                    "run_id": "r7",
+                    "gap_title": "Fix cycles",
+                    "gap_subsystem": "runtime",
+                    "issue_state": "open",
+                }]
+            }
+            issue = {
+                "number": 42,
+                "title": "Fix cycles",
+                "html_url": "https://github.com/x/y/issues/42",
+                "body": "<!-- arch-gap-fingerprint: aabbccddeeff -->\nLane: import_cycles",
+                "labels": [
+                    {"name": "arch-gap"},
+                    {"name": "copilot-task"},
+                    {"name": "needs-implementation"},
+                ],
+                "state": "open",
+            }
+            closed_pr = {
+                "number": 64,
+                "title": "PR #64",
+                "html_url": "https://github.com/x/y/pull/64",
+                "state": "closed",
+                "draft": False,
+                "body": "Fixes #42",
+                "merged_at": None,
+                "head": {"ref": "feature/issue-64"},
+                "created_at": dt.datetime.now(dt.timezone.utc).isoformat().replace("+00:00", "Z"),
+                "updated_at": dt.datetime.now(dt.timezone.utc).isoformat().replace("+00:00", "Z"),
+            }
+            with patch.object(ra, "load_work_state", return_value=ws), \
+                    patch.object(ra, "save_work_state", return_value=None), \
+                    patch.object(ra, "_list_prs_for_repo", return_value=[closed_pr]), \
+                    patch.object(ra, "_list_github_issues_by_labels", return_value=[issue]), \
+                    patch.object(ra, "_update_issue_lifecycle_labels_for_pr", return_value=None):
+                result = ra.run_execution_cycle(config)
+        self.assertIsNone(result["selected_issue"])
+        self.assertIsNone(result["delegation"])
+        self.assertIn("closed-unmerged state", result["message"])
+        self.assertIn("#42", result["message"])
+        self.assertEqual(result["reconcile"]["details"][0]["new_delegation"], "delegation-failed")
+        self.assertEqual(ws["items"][0]["delegation_state"], "delegation-failed")
+        self.assertTrue(ws["items"][0]["closed_unmerged"])
+
 
 # ---------------------------------------------------------------------------
 # 38. PR reconciliation
@@ -3097,6 +3190,29 @@ def test_closed_unmerged_not_auto_superseded(self) -> None:
         self.assertEqual(ws["items"][0]["lifecycle_fact_state"], "closed-unmerged")
         self.assertNotEqual(ws["items"][0]["lifecycle_fact_state"], "superseded-by-pr")
 
+    def test_reconcile_closed_unmerged_marks_terminal_non_ready_state(self) -> None:
+        """Closed-unmerged reconciliation should make the work item terminal and non-ready."""
+        config = _make_config(mode="reconcile")
+        ws: Dict[str, Any] = {"items": [{
+            "fingerprint": "aa11bb22cc33", "issue_number": 42, "lane": "runtime",
+            "delegation_state": "ready-for-delegation", "merged": False, "closed_unmerged": False,
+            "blocked": False, "superseded": False, "pr_number": None, "pr_url": None, "pr_state": None,
+            "updated_at": "2025-01-01T00:00:00+00:00", "objective": "", "assignee": None,
+            "created_at": "2025-01-01T00:00:00+00:00", "run_id": "r5",
+            "gap_title": "x", "gap_subsystem": "runtime", "issue_state": "open",
+        }]}
+        closed_pr = self._make_pr(201, state="closed", body="fixes #42", merged_at=None)
+        with patch.object(ra, "_list_prs_for_repo", return_value=[closed_pr]):
+            result = ra.reconcile_pr_state(config, ws)
+        self.assertEqual(result["updated"], 1)
+        self.assertTrue(ws["items"][0]["closed_unmerged"])
+        self.assertFalse(ws["items"][0]["merged"])
+        self.assertEqual(ws["items"][0]["delegation_state"], "delegation-failed")
+        self.assertEqual(ws["items"][0]["lifecycle_fact_state"], "closed-unmerged")
+        self.assertEqual(ws["items"][0]["lifecycle_inferred_state"], "needs-replanning")
+        self.assertEqual(result["details"][0]["new_delegation"], "delegation-failed")
+        self.assertEqual(result["details"][0]["pr_state"], "closed_unmerged")
+
     def test_stale_not_auto_blocked_dependency(self) -> None:
         """Stale state should not be encoded as blocked-by-dependency without explicit evidence."""
         config = _make_config(mode="reconcile")