From f58ddba93ee64cc9ccbfd1661d24d4e095f2b5a7 Mon Sep 17 00:00:00 2001 From: Chaim Date: Thu, 16 Jul 2026 12:32:20 +0000 Subject: [PATCH] =?UTF-8?q?fix(routing):=20=D7=94=D7=95=D7=A8=D7=90=D7=AA-?= =?UTF-8?q?=D7=99=D7=95"=D7=A8=20=D7=AA=D7=A2=D7=91=D7=95=D7=93=20=D7=91?= =?UTF-8?q?=D7=93=D7=A8=D7=9A=20=D7=94=D7=98=D7=91=D7=A2=D7=99=D7=AA=20?= =?UTF-8?q?=E2=80=94=20=D7=94=D7=A2=D7=A8=D7=94=20=D7=A4=D7=95=D7=AA=D7=97?= =?UTF-8?q?=D7=AA=20=D7=94=D7=A8=D7=A6=D7=AA-CEO?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **התסמין:** היו"ר כותב הוראה על תיק והסוכן "מתעלם". ההרצה מדווחת succeeded/exit 0 ואפילו עולה כסף — ולכן זה שרד חודשים בלי שמישהו שם לב. **שתי סיבות-שורש בלתי-תלויות, שתיהן מאומתות:** 1. ה-wakeup הישיר על ה-issue של התיק מבוטל תמיד: תיק שממתין ליו"ר הוא `in_review` ומשויך *לאדם*, ו-Paperclip עונה `issue_assignee_changed` → "אעיר את הבעלים החדש" → הבעלים אדם → אף סוכן לא מתעורר. 2. רשת-הביטחון בפלאגין מוסרת את ההוראה כ-`payload.prompt`, וה-runner קורא רק `payload.issueId` — הפרומפט נבלע. 18/18 מדידה ב-DB: 0% מסירה, אי-פעם. **התיקון:** המסלול הטבעי (הערה בטאב-הסוכנים) מפרסם את ההערה בשרשור כרגיל **וגם** פותח הרצת-CEO עם ההוראה, דרך `pc_open_ceo_run` — הפרימיטיב היחיד שאומת כמגיע לסוכן (הוכח היום על 1043-02-26: 3,212→7,697 מילים). אותו תיקון ל-interaction-response, שסובל מאותה תקלה (החצי השני של #228). **G2 — הסרת מסלול מקביל:** ה-wakeup הנדון-לביטול הוסר מ-`post_comment`; הוא היה המסלול השני, זה ששותק. כעת מסלול-מסירה קנוני אחד. `mark_comment_routed` מסמן את ההערה כמנותבת כדי שה-sweep לא יירה הרצת-סרק על מה שכבר טופל. **G12 — שער-הפלטפורמה:** המגע החדש (`mark_comment_routed`) נחשף דרך `agent_platform_port.py` בלבד; leak-guard עובר. התיקון יושב ב-backend (REST) ולא בפלאגין, ולכן אינו חשוף לבאג ה-invocation-scope שמפיל את ה-sweep. תועד ב-docs/paperclip-quirks.md §7 עם המלכודות שנתקלנו בהן. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/paperclip-quirks.md | 37 +++++++++++++++++++++++ web/agent_platform_port.py | 5 +++ web/app.py | 52 +++++++++++++++++++++++++++++--- web/paperclip_client.py | 62 +++++++++++++++++++++++++------------- 4 files changed, 130 insertions(+), 26 deletions(-) diff --git a/docs/paperclip-quirks.md b/docs/paperclip-quirks.md index d25adf9..d1889ca 100644 --- a/docs/paperclip-quirks.md +++ b/docs/paperclip-quirks.md @@ -275,3 +275,40 @@ WARN [plugin] issue.comment.created event missing issueId in payload, skipping - **תוקן בצד שלנו** (PR #2, נפרס דרך `npm run build` + `pm2 restart paperclip` — הפלאגין נטען מ-`/home/chaim/plugin-legal-ai` לפי `package_path`). - **לקח כללי**: ל-events של הפלאגין — מזהה-הישות-הראשית הוא תמיד `event.entityId`; אל תניח ששדות נמצאים ב-`payload` בלי לאמת מול ה-`.d.ts` של ה-SDK או מול לוג חי. - TaskMaster: `legal-ai` #149. + +--- + +## 7. `agents.invoke({prompt})` — הפרומפט נבלע; רק `payload.issueId` נמסר + +**התסמין (מנקודת-מבט היו"ר):** כותבים הוראה לסוכן — והוא "מתעלם". ההרצה נראית תקינה +לחלוטין: `status=succeeded`, `exit_code=0`, בלי שגיאה, ואפילו עולה כסף. הסוכן מדווח +"כל התורים ריקים — אין עבודה". + +**הסיבה:** `ctx.agents.invoke(agentId, companyId, {prompt, reason})` הוא ה-API **המתועד** +של ה-Plugin SDK ("*Invoke an agent with a prompt payload*"), ו-`prompt` הוא שדה חובה. +Paperclip מקבל אותו, שומר אותו ב-`agent_wakeup_requests.payload.prompt` — **וה-runner קורא +רק `payload.issueId`**. הפרומפט לא מגיע למודל. ה-`reason` כן שורד (ל-`context_snapshot.wakeReason`), +ולכן זה נראה כאילו משהו נמסר. + +מדידה ב-DB (2026-07-16): **18 מתוך 18** ה-wakeups שאי-פעם נשאו `payload.prompt` לא הגיעו +לאף הרצה — 0% מסירה. מול 107,172 wakeups עם `issueId` שעובדים. אימות ישיר: בלוג-ההרצה +שנבלעה אין ולו אזכור אחד של מספר-התיק, והמודל פותח ב-*"This heartbeat has no scoped wake payload"*. + +**מה עושים במקום:** נושאים את ההוראה על **issue**, לא ב-payload — issue-ילד שכבר משויך +ל-CEO, עם ההוראה ב-`description`, ואז wakeup עם `payload.issueId`. זה בדיוק +`open_ceo_run` ב-[`web/paperclip_client.py`](../web/paperclip_client.py), והמסלול היחיד +שאומת כמגיע לסוכן. + +**מלכודות שנלוות לזה — אל תיפול בהן שוב:** +- **`issues.create` של ה-SDK עם `assigneeAgentId` *לא* מעיר את הסוכן.** ה-REST המקביל + (`POST /api/issues/:id/children`) *כן* — הוא פולט wakeup `issue_assigned`. אל תכליל + מהתנהגות REST על ה-SDK; זה נבדק ונמצא שונה. +- **`ctx.issues.requestWakeup` (ה-primitive הנכון ב-SDK) נכשל מתוך scheduled job** עם + `missing, expired, or unknown invocation scope` — למרות ש-capability `issues.wakeup` + מוצהר ומותקן. אותה שגיאה מפילה גם `companies.list` / `issues.list` / `issues.listComments` + בתוך ה-sweep, ולכן **רשת-הביטחון `route-pending-comments` אינה אמינה**. לא נחקר לעומק — + לכן ניתוב-ההערות הועבר ל-backend של legal-ai (REST), שאינו חשוף לבעיה. + +**סטטוס:** נעקף — `POST /api/cases/{case}/agents/comment` ו-`/agents/interaction-response` +פותחים הרצת-CEO ישירות (REST) ומסמנים את ההערה כמנותבת, כדי שה-sweep השבור לא יירה +הרצת-סרק. TaskMaster `legal-ai` #228. diff --git a/web/agent_platform_port.py b/web/agent_platform_port.py index cb06257..477e1a7 100644 --- a/web/agent_platform_port.py +++ b/web/agent_platform_port.py @@ -63,6 +63,7 @@ from web.paperclip_client import ( cancel_interaction as _cancel_interaction, cancel_run as _cancel_run, escalate_issue as _escalate_issue, + mark_comment_routed as _mark_comment_routed, post_comment as _post_comment, reap_stale_interactions as _reap_stale_interactions, reset_agent_session as _reset_agent_session, @@ -114,6 +115,9 @@ pc_wake_analyst_for_protocol_analysis = instrument( pc_post_comment = instrument( "agent.comment", keys=("issue_id", "company_id"), )(_post_comment) +pc_mark_comment_routed = instrument( + "agent.comment", keys=("issue_id", "comment_id"), +)(_mark_comment_routed) pc_cancel_interaction = instrument( "interaction.cancelled", keys=("issue_id", "interaction_id"), )(_cancel_interaction) @@ -179,6 +183,7 @@ __all__ = [ "pc_get_generation_run_status", # comments / interactions "pc_post_comment", + "pc_mark_comment_routed", "pc_get_issue_comments", "pc_get_issue_interactions", "pc_accept_interaction", diff --git a/web/app.py b/web/app.py index def762a..772e0c2 100644 --- a/web/app.py +++ b/web/app.py @@ -91,6 +91,7 @@ from web.agent_platform_port import ( pc_wake_ceo, pc_wake_ceo_for_action, pc_open_ceo_run, + pc_mark_comment_routed, pc_wake_ceo_for_feedback_fold, pc_wake_curator_for_final, pc_wake_for_precedent_extraction, @@ -4660,8 +4661,9 @@ async def api_case_agents(case_number: str): class AgentCommentRequest(BaseModel): body: str issue_id: str | None = None - # "פתח הרצה חדשה" — open a fresh CEO-owned run for this instruction instead of - # appending to an existing issue (agents-tab target selector). Overrides issue_id. + # "פתח הרצה חדשה" — skip the comment entirely and only open a CEO run. Kept for + # the agents-tab target selector; the default path below now opens a run too, so + # this only differs in *not* recording the instruction on the parent thread. new_run: bool = False @@ -4705,10 +4707,31 @@ async def api_post_agent_comment(case_number: str, req: AgentCommentRequest): result = await pc_post_comment(target["id"], target["company_id"], req.body) + # The comment alone reaches nobody: a case issue awaiting the chair is + # `in_review` and owned by a human, so Paperclip cancels any wakeup on it + # (`issue_assignee_changed` → "wake the new owner" → the owner is a person). + # The plugin sweep that was meant to catch this delivers the instruction as + # `payload.prompt`, which the runner drops. So the comment is recorded for the + # thread, and the instruction is carried by a CEO-owned child run — the one + # path verified to reach the agent (TaskMaster #228; see paperclip-quirks.md). + run = await pc_open_ceo_run(case_number, req.body) + if run.get("status") != "ok": + raise HTTPException( + 502, + f"ההערה נשמרה אך פתיחת ההרצה נכשלה — הסוכן לא יטפל בה: " + f"{run.get('reason', 'unknown')}", + ) + # Claim the comment so the sweep does not re-route it: the CEO answers on the + # child issue, leaving the parent with no agent reply, which the sweep reads + # as "still pending" forever. + await pc_mark_comment_routed(target["id"], result.get("comment_id", "")) + # 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", "") + result["run_issue_id"] = run["issue_id"] + result["run_issue_identifier"] = run.get("identifier", "") return result @@ -4725,8 +4748,10 @@ async def api_post_interaction_response( ): """Submit a user's answer to a Paperclip issue-thread interaction. - Routes to /respond | /accept | /reject based on `action`. Paperclip - auto-wakes the issue assignee after a successful submission. + Routes to /respond | /accept | /reject based on `action`, then opens a CEO run + carrying the answer. Paperclip's own `wake_assignee` cannot deliver it: the + issue is owned by the chair while it waits for her, so the "assignee" it wakes + is a person and no agent ever runs (TaskMaster #228). """ issues = await pc_get_case_issues(case_number) if not any(i["id"] == req.issue_id for i in issues): @@ -4738,7 +4763,7 @@ async def api_post_interaction_response( "reject": pc_reject_interaction, } try: - return await handlers[req.action]( + result = await handlers[req.action]( req.issue_id, req.interaction_id, req.payload, ) except httpx.HTTPStatusError as e: @@ -4747,6 +4772,23 @@ async def api_post_interaction_response( except Exception as e: raise HTTPException(502, f"שגיאת Paperclip: {e}") + answer = json.dumps(req.payload, ensure_ascii=False) + run = await pc_open_ceo_run( + case_number, + f'תשובת היו"ר לשאלה שהוצגה לה ({req.action}):\n\n{answer}\n\n' + f"קרא את ה-interaction המקורי על ה-issue והמשך משם.", + ) + if run.get("status") != "ok": + raise HTTPException( + 502, + f"התשובה נשמרה אך פתיחת ההרצה נכשלה — הסוכן לא ימשיך: " + f"{run.get('reason', 'unknown')}", + ) + if isinstance(result, dict): + result["run_issue_id"] = run["issue_id"] + result["run_issue_identifier"] = run.get("identifier", "") + return result + class InteractionDismissRequest(BaseModel): issue_id: str diff --git a/web/paperclip_client.py b/web/paperclip_client.py index 19598f6..e590c8a 100644 --- a/web/paperclip_client.py +++ b/web/paperclip_client.py @@ -327,6 +327,40 @@ async def _create_issue( return issue_id, identifier +async def mark_comment_routed(issue_id: str, comment_id: str) -> dict: + """Claim a chair comment as already routed, so the plugin sweep skips it. + + The plugin's `route-pending-comments` sweep re-routes any user comment newer + than the newest agent comment on the same issue. A chair instruction answered + on a CEO **child** issue leaves the parent with no agent reply, so the sweep + would keep re-routing it forever. Writing the sweep's own marker + (`last-routed-comment-id`, plugin state, issue scope) is what tells it the + comment is handled — this is the same key `markCommentRouted` sets in + `plugin-legal-ai/src/worker.ts`. + """ + if not comment_id: + return {"ok": False, "error": "no_comment_id"} + try: + conn = await asyncpg.connect(PAPERCLIP_DB_URL) + try: + await conn.execute( + """INSERT INTO plugin_state (plugin_id, scope_kind, scope_id, namespace, state_key, value_json) + VALUES ($1::uuid, 'issue', $2, 'default', 'last-routed-comment-id', $3::jsonb) + ON CONFLICT (plugin_id, scope_kind, scope_id, namespace, state_key) + DO UPDATE SET value_json = $3::jsonb""", + PLUGIN_ID, issue_id, json.dumps(comment_id), + ) + finally: + await conn.close() + return {"ok": True, "issue_id": issue_id, "comment_id": comment_id} + except Exception as e: + logger.warning( + "mark_comment_routed failed for issue %s comment %s: %s", + issue_id, comment_id, e, + ) + return {"ok": False, "error": str(e)} + + async def _link_case_to_issue(conn: asyncpg.Connection, issue_id: str, case_number: str) -> None: """Store the legal-ai case number in plugin state, linked to the issue.""" await conn.execute( @@ -655,10 +689,14 @@ async def get_agents_for_case(company_id: str, issue_ids: list[str]) -> list[dic async def post_comment(issue_id: str, company_id: str, body: str) -> dict: - """Post a comment on a Paperclip issue. + """Post a comment on a Paperclip issue. Records only — wakes nobody. - Tries the Board API first (triggers plugin events for CEO routing). - Falls back to direct DB insert + CEO wakeup if API fails. + Delivering the instruction is the caller's job, via :func:`open_ceo_run`. This + function used to also wake the CEO on ``issue_id``, but that wakeup is dead on + arrival for the case issues the chair actually comments on: they are + `in_review` and owned by her, so Paperclip cancels the run with + `issue_assignee_changed` before it starts. Keeping it would leave two parallel + delivery paths — one that works and one that silently doesn't (INV-G2). """ # Try Board API first — this triggers the event bus if PAPERCLIP_BOARD_API_KEY: @@ -675,7 +713,6 @@ async def post_comment(issue_id: str, company_id: str, body: str) -> dict: except Exception: logger.debug("Board API comment failed for issue %s, falling back to DB", issue_id) - # Fallback: direct DB insert + explicit CEO wakeup comment_id = str(uuid.uuid4()) conn = await asyncpg.connect(PAPERCLIP_DB_URL) try: @@ -688,23 +725,6 @@ async def post_comment(issue_id: str, company_id: str, body: str) -> dict: finally: await conn.close() - # Wake the correct CEO for this company - ceo_id = CEO_AGENTS.get(company_id, CEO_AGENT_ID) - try: - await pc_request( - "POST", - f"/api/agents/{ceo_id}/wakeup", - json={ - "source": "on_demand", - "triggerDetail": "manual", - "reason": f"user_comment_{issue_id}", - "payload": {"issueId": issue_id, "mutation": "comment"}, - }, - raise_on_error=True, - ) - except Exception: - logger.warning("Failed to wake CEO after DB comment on issue %s", issue_id) - return {"comment_id": comment_id, "issue_id": issue_id, "method": "db_fallback"}