fix(agents): ניתוב הערת-יו"ר לא ל-issue סגור (cancelled) — תמיד ל-CEO החי
All checks were successful
G12 Leak-Guard / leak-guard (pull_request) Successful in 34s
Lint — undefined names / undefined-names (pull_request) Successful in 11s

בעיה חוזרת: הקלדת הוראה בתיבת "כתוב הוראה לסוכנים..." נבלעה בשקט. השרת בחר
את היעד ב-`active[-1]` תוך סינון `done` בלבד — לא `cancelled`. כשה-child האחרון
בוטל (למשל 1027-04-26: "חישוב טיעונים" cancelled שנוצר אחרי ה-issue הראשי),
ההערה נותבה אליו; Paperclip מדלג על wakeup ל-issue מבוטל → ההוראה לא הגיעה לאיש.

תיקון (G1 — נרמול במקור, לא תיקון-בקריאה):
- `pick_default_comment_target` חדש ב-paperclip_client — מעדיף את ה-issue הראשי
  החי של ה-CEO (top-level+open), נופל ל-open כלשהו, ואז top-level, ואף פעם לא
  מעדיף child-סגור. `get_case_issues` מחזיר כעת `parent_id`.
- app.py מנתב דרכו; מחזיר `issue_status` כדי שה-UI יוכל לסמן יעד-סגור.
- נחשף דרך ה-Port (G12) כ-`pc_pick_default_comment_target`.
- 6 בדיקות ל-picker (כולל תרחיש 1027-04-26 המדויק).

Invariants: G1 (נרמול-במקור), G2 (אין מסלול-ניתוב מקביל), G12 (מגע-Paperclip דרך ה-Port).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-07 15:05:51 +00:00
parent 1d8c4e29c8
commit 67d5835eb8
4 changed files with 127 additions and 6 deletions

View File

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

View File

@@ -69,6 +69,7 @@ from web.agent_platform_port import (
pc_get_agents, pc_get_agents,
pc_get_agents_for_case, pc_get_agents_for_case,
pc_get_case_issues, pc_get_case_issues,
pc_pick_default_comment_target,
pc_get_issue_comments, pc_get_issue_comments,
pc_get_issue_interactions, pc_get_issue_interactions,
pc_get_run_events, pc_get_run_events,
@@ -4609,7 +4610,8 @@ class AgentCommentRequest(BaseModel):
async def api_post_agent_comment(case_number: str, req: AgentCommentRequest): async def api_post_agent_comment(case_number: str, req: AgentCommentRequest):
"""Post a comment on a Paperclip issue linked to a case. """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) issues = await pc_get_case_issues(case_number)
if not issues: if not issues:
@@ -4620,14 +4622,14 @@ async def api_post_agent_comment(case_number: str, req: AgentCommentRequest):
if not target: if not target:
raise HTTPException(404, f"Issue {req.issue_id} לא שייך לתיק {case_number}") raise HTTPException(404, f"Issue {req.issue_id} לא שייך לתיק {case_number}")
else: else:
# Pick the most recent non-done issue, or the last one target = pc_pick_default_comment_target(issues)
active = [i for i in issues if i["status"] != "done"]
target = active[-1] if active else issues[-1]
result = await pc_post_comment(target["id"], target["company_id"], req.body) 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_identifier"] = target.get("identifier", "")
result["issue_status"] = target.get("status", "")
return result return result

View File

@@ -484,7 +484,8 @@ async def get_case_issues(case_number: str) -> list[dict]:
"""SELECT DISTINCT ON (i.id) """SELECT DISTINCT ON (i.id)
i.id, i.title, i.status, i.identifier, i.priority, i.id, i.title, i.status, i.identifier, i.priority,
i.assignee_agent_id, a.name AS assignee_name, 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 FROM issues i
LEFT JOIN agents a ON i.assignee_agent_id = a.id LEFT JOIN agents a ON i.assignee_agent_id = a.id
LEFT JOIN plugin_state ps ON ps.scope_id = i.id::text 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, "completed_at": r["completed_at"].isoformat() if r["completed_at"] else None,
"created_at": r["created_at"].isoformat() if r["created_at"] else None, "created_at": r["created_at"].isoformat() if r["created_at"] else None,
"company_id": str(r["company_id"]), "company_id": str(r["company_id"]),
"parent_id": str(r["parent_id"]) if r["parent_id"] else None,
} }
for r in sorted_rows for r in sorted_rows
] ]
@@ -517,6 +519,43 @@ async def get_case_issues(case_number: str) -> list[dict]:
await conn.close() 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]: async def get_issue_comments(issue_ids: list[str]) -> list[dict]:
"""Get all comments on a list of Paperclip issues, with agent metadata.""" """Get all comments on a list of Paperclip issues, with agent metadata."""
if not issue_ids: if not issue_ids:

View File

@@ -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"