Merge pull request 'fix(agents): ניתוב הערת-יו"ר לא ל-issue סגור — תמיד ל-CEO החי' (#412) from worktree-comment-routing-fix into main

web/agent_platform_port.py

@@ -47,6 +47,7 @@ from web.paperclip_client import (
     get_agents_for_case as pc_get_agents_for_case,
     get_agents_for_company as pc_get_agents,
     get_case_issues as pc_get_case_issues,
+    pick_default_comment_target as pc_pick_default_comment_target,
     get_issue_comments as pc_get_issue_comments,
     get_issue_interactions as pc_get_issue_interactions,
     get_project_url,
@@ -158,6 +159,7 @@ __all__ = [
     # issues / workflow
     "pc_create_workflow_issue",
     "pc_get_case_issues",
+    "pc_pick_default_comment_target",
     # agents / wakeups
     "pc_get_agents_for_case",
     "pc_get_agents",

web/app.py

@@ -69,6 +69,7 @@ from web.agent_platform_port import (
     pc_get_agents,
     pc_get_agents_for_case,
     pc_get_case_issues,
+    pc_pick_default_comment_target,
     pc_get_issue_comments,
     pc_get_issue_interactions,
     pc_get_run_events,
@@ -4609,7 +4610,8 @@ class AgentCommentRequest(BaseModel):
 async def api_post_agent_comment(case_number: str, req: AgentCommentRequest):
     """Post a comment on a Paperclip issue linked to a case.
 
-    If issue_id is omitted, the most recent non-done issue is used.
+    If ``issue_id`` is omitted, routing prefers the live CEO main issue and
+    never a closed (done/cancelled) issue — see ``_pick_default_comment_target``.
     """
     issues = await pc_get_case_issues(case_number)
     if not issues:
@@ -4620,14 +4622,14 @@ async def api_post_agent_comment(case_number: str, req: AgentCommentRequest):
         if not target:
             raise HTTPException(404, f"Issue {req.issue_id} לא שייך לתיק {case_number}")
     else:
-        # Pick the most recent non-done issue, or the last one
-        active = [i for i in issues if i["status"] != "done"]
-        target = active[-1] if active else issues[-1]
+        target = pc_pick_default_comment_target(issues)
 
     result = await pc_post_comment(target["id"], target["company_id"], req.body)
 
-    # Find the identifier for the response
+    # Echo the resolved target so the UI can show where it landed and flag it
+    # when the chair explicitly targeted a closed issue (whose wakeup is skipped).
     result["issue_identifier"] = target.get("identifier", "")
+    result["issue_status"] = target.get("status", "")
     return result
 
 

web/paperclip_client.py

@@ -484,7 +484,8 @@ async def get_case_issues(case_number: str) -> list[dict]:
             """SELECT DISTINCT ON (i.id)
                       i.id, i.title, i.status, i.identifier, i.priority,
                       i.assignee_agent_id, a.name AS assignee_name,
-                      i.started_at, i.completed_at, i.created_at, i.company_id
+                      i.started_at, i.completed_at, i.created_at, i.company_id,
+                      i.parent_id
                FROM issues i
                LEFT JOIN agents a ON i.assignee_agent_id = a.id
                LEFT JOIN plugin_state ps ON ps.scope_id = i.id::text
@@ -510,6 +511,7 @@ async def get_case_issues(case_number: str) -> list[dict]:
                 "completed_at": r["completed_at"].isoformat() if r["completed_at"] else None,
                 "created_at": r["created_at"].isoformat() if r["created_at"] else None,
                 "company_id": str(r["company_id"]),
+                "parent_id": str(r["parent_id"]) if r["parent_id"] else None,
             }
             for r in sorted_rows
         ]
@@ -517,6 +519,43 @@ async def get_case_issues(case_number: str) -> list[dict]:
         await conn.close()
 
 
+# A comment posted to a closed issue never wakes an agent — Paperclip skips the
+# wakeup for done/cancelled issues, so the chair's instruction is silently
+# swallowed. The default-target picker MUST therefore avoid these statuses.
+CLOSED_ISSUE_STATUSES = frozenset({"done", "cancelled"})
+
+
+def pick_default_comment_target(issues: list[dict]) -> dict:
+    """Choose which issue a chair comment routes to when none is specified.
+
+    ``issues`` is ordered oldest→newest (see :func:`get_case_issues`). Comment
+    routing is meant to reach the CEO, whose live "התחל תהליך ניסוח" issue is
+    top-level (``parent_id is None``) and open. Preference order:
+      1. newest OPEN top-level issue — the live CEO main issue;
+      2. newest OPEN issue of any depth — a live sub-issue if no main is open;
+      3. newest top-level issue even if closed — better than a closed child;
+      4. newest issue overall — last resort.
+    Excluding closed statuses is the fix for the recurring bug where the newest
+    non-done issue was a *cancelled* child, so the wakeup was skipped and the
+    instruction vanished. ``issues`` must be non-empty.
+    """
+    def is_open(i: dict) -> bool:
+        return i["status"] not in CLOSED_ISSUE_STATUSES
+
+    def is_top(i: dict) -> bool:
+        return i.get("parent_id") is None
+
+    for pred in (
+        lambda i: is_open(i) and is_top(i),
+        is_open,
+        is_top,
+    ):
+        matches = [i for i in issues if pred(i)]
+        if matches:
+            return matches[-1]
+    return issues[-1]
+
+
 async def get_issue_comments(issue_ids: list[str]) -> list[dict]:
     """Get all comments on a list of Paperclip issues, with agent metadata."""
     if not issue_ids:

web/tests/test_comment_target.py

@@ -0,0 +1,78 @@
+"""Tests for pick_default_comment_target — chair-comment routing.
+
+Regression guard for the recurring bug where a chair instruction typed into the
+"כתוב הוראה לסוכנים..." box was routed to a *cancelled* child issue, so
+Paperclip skipped the wakeup and the instruction was silently swallowed.
+
+The picker must never default to a closed (done/cancelled) issue and must prefer
+the live top-level CEO issue. Pure function — no DB, no FastAPI.
+"""
+
+from __future__ import annotations
+
+import os
+import sys
+from pathlib import Path
+
+import pytest
+
+os.environ.setdefault("PAPERCLIP_DB_URL", "postgres://x:x@127.0.0.1:54329/paperclip")
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))  # web/
+
+pc = pytest.importorskip("paperclip_client", reason="web deps unavailable")
+
+
+def _iss(id_, status, parent_id=None):
+    return {"id": id_, "status": status, "parent_id": parent_id, "identifier": id_}
+
+
+def test_prefers_open_top_level_over_newer_cancelled_child():
+    """The exact 1027-04-26 scenario: newest non-done issue is a cancelled child;
+    routing must still land on the live top-level CEO issue."""
+    issues = [
+        _iss("main-old", "done"),                       # oldest top-level, closed
+        _iss("main-live", "in_review"),                 # live CEO main issue
+        _iss("child-cancelled", "cancelled", "main-live"),  # newest non-done → the trap
+        _iss("child-done", "done", "main-live"),
+    ]
+    assert pc.pick_default_comment_target(issues)["id"] == "main-live"
+
+
+def test_never_returns_cancelled_when_an_open_issue_exists():
+    issues = [
+        _iss("a", "done"),
+        _iss("b", "in_progress", "a"),
+        _iss("c", "cancelled", "a"),
+    ]
+    assert pc.pick_default_comment_target(issues)["status"] != "cancelled"
+
+
+def test_falls_back_to_open_child_when_no_open_top_level():
+    issues = [
+        _iss("main", "done"),
+        _iss("child-live", "in_progress", "main"),
+    ]
+    assert pc.pick_default_comment_target(issues)["id"] == "child-live"
+
+
+def test_prefers_top_level_when_everything_closed():
+    """No open issue anywhere — pick the newest top-level (better than a child)."""
+    issues = [
+        _iss("main-1", "done"),
+        _iss("main-2", "cancelled"),
+        _iss("child", "done", "main-2"),
+    ]
+    assert pc.pick_default_comment_target(issues)["id"] == "main-2"
+
+
+def test_single_issue():
+    issues = [_iss("only", "cancelled")]
+    assert pc.pick_default_comment_target(issues)["id"] == "only"
+
+
+def test_picks_newest_open_top_level():
+    issues = [
+        _iss("main-1", "in_review"),
+        _iss("main-2", "in_review"),
+    ]
+    assert pc.pick_default_comment_target(issues)["id"] == "main-2"