From 510f276717b5dc78615e731a888e8b0ac9b239e9 Mon Sep 17 00:00:00 2001 From: Chaim Marcus Date: Tue, 1 Sep 2026 23:33:15 +0300 Subject: [PATCH] =?UTF-8?q?fix(sync):=20onWebhook=20=D7=92=D7=95=D7=96?= =?UTF-8?q?=D7=A8=20=D7=AA=D7=95=D7=95=D7=99=D7=95=D7=AA-=D7=A1=D7=98?= =?UTF-8?q?=D7=98=D7=95=D7=A1=20=D7=9E-/api/status-model=20(legal-ai=20#61?= =?UTF-8?q?6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit מפת-התוויות העברית הקשיחה ב-onWebhook הייתה מקור-האמת השני לאותו enum של סטטוסי-תיק, והספיקה לסטות: חסרו בה analyst_verified ו-research_complete (שנוספו ל-case_status_model.py ב-30.6), והיו בה חמישה מפתחות מתים — uploading, brainstorming, drafting, in_progress ו-qa_failed. #604 תיקן את המפה הראשונה (CASE_STATUS_TO_ISSUE_STATUS); זו השנייה. - statusLabels נמחקת; התווית נגזרת מ-GET /api/status-model דרך labelFor() הקיימת — אותו SSOT שהג'וב sync-case-status כבר צורך (G2). - resolveStatusLabel() חדשה ב-sync-target.ts (המודול הטהור): מרכיבה את labelFor ומחזירה גם את **סיבת** הנפילה-לגולמי, כדי שההיגיון יהיה בר-בדיקה — worker.ts אינו ניתן ל-import בטסט (runWorker ברמת-המודול). - אין בליעה שקטה (כלל-הנדסה §6): סטטוס שנטען ואינו במודל → logger.warn עם מספר-התיק והסטטוס; מודל שלא נטען כלל (כשל-רשת או LegalApi חסר) → שני logger.error נבדלים. בכל המקרים התגובה עדיין מתפרסמת והערת-ה-CEO עדיין נורית — תווית חסרה אינה מפילה webhook. - oldStatus מתורגם אף הוא לתווית (היה מודפס כמפתח-אנגלית בתוך משפט עברי). - legalApi עשה hoist ליד pluginCtx ומועבר מפורשות ל-handleCaseStatusWebhook — אותו מופע, לא שני. - AC2: הוסרו שתי רשימות-הסטטוסים הקשיחות הנוספות ב-worker.ts — ה-enum המיושן ב-legal_case_update (חסם 8 סטטוסים חוקיים והתיר את in_progress שאינו case.status; המניפסט כבר מגדיר את השדה כמחרוזת חופשית, והוולידציה האמיתית היא server-side) והמנייה בתיאור legal_case_list. Invariants: G2 (מקור-אמת אחד לתוויות, בשני הצרכנים) · G1 (נרמול-במקור — הפלאגין מושך מה-SSOT במקום להחזיק העתק סטטי) · G12 (legal-ai לא נגע כלל; אפס סמל ספציפי-Paperclip נכנס אליו) · X7 INV-INT9 (onWebhook עדיין אינו כותב issue.status — המשבצת השמורה נשארת שמורה) · כלל-הנדסה §6. tsc --noEmit נקי · biome check src/ נקי · node --test 39/39. Co-Authored-By: Claude Opus 5 (1M context) --- src/sync-target.test.ts | 51 ++++++++++++++++++++++++++ src/sync-target.ts | 33 +++++++++++++++++ src/worker.ts | 81 +++++++++++++++++++++++++++++------------ 3 files changed, 141 insertions(+), 24 deletions(-) diff --git a/src/sync-target.test.ts b/src/sync-target.test.ts index 1d0908c..184af7e 100644 --- a/src/sync-target.test.ts +++ b/src/sync-target.test.ts @@ -11,6 +11,7 @@ import { labelFor, pickSyncTargetIssue, resolveIssueStatus, + resolveStatusLabel, type SyncCandidate, } from "./sync-target.ts"; @@ -336,3 +337,53 @@ test("רגרסיה: companyId של המועמד-הנבחר שורד את pickSyn assert.equal(result.target?.id, "issue-b"); assert.equal(result.target?.companyId, "company-b"); }); + +test("resolveStatusLabel: סטטוס מוכר → תווית מה-SSOT, בלי fallback", () => { + assert.deepEqual(resolveStatusLabel(STATUS_MODEL, "new"), { + label: "חדש", + fallback: null, + }); +}); + +test("resolveStatusLabel: סטטוס שאינו במודל → warning-worthy fallback + ערך גולמי, בלי זריקה", () => { + assert.doesNotThrow(() => + resolveStatusLabel(STATUS_MODEL, "totally_unknown_status"), + ); + assert.deepEqual(resolveStatusLabel(STATUS_MODEL, "totally_unknown_status"), { + label: "totally_unknown_status", + fallback: "unknown_status", + }); +}); + +test("resolveStatusLabel: מודל לא-זמין (null) → model_unavailable, נבדל מסטטוס-לא-מוכר", () => { + assert.deepEqual(resolveStatusLabel(null, "processing"), { + label: "processing", + fallback: "model_unavailable", + }); +}); + +test("רגרסיה #616: כל הסטטוסים שהיו במפה הקשיחה ועודם חיים מקבלים תווית מה-SSOT", () => { + const survivingKeys = [ + "new", + "processing", + "documents_ready", + "outcome_set", + "direction_approved", + "qa_review", + "drafted", + "exported", + "reviewed", + "final", + ].filter((key) => STATUS_MODEL.some((s) => s.key === key)); + + for (const key of survivingKeys) { + const result = resolveStatusLabel(STATUS_MODEL, key); + assert.equal(result.fallback, null, `${key}: expected no fallback`); + assert.equal( + typeof result.label, + "string", + `${key}: expected string label`, + ); + assert.notEqual(result.label, "", `${key}: expected non-empty label`); + } +}); diff --git a/src/sync-target.ts b/src/sync-target.ts index 55e1c37..e40771c 100644 --- a/src/sync-target.ts +++ b/src/sync-target.ts @@ -115,3 +115,36 @@ export function labelFor( ): string | null { return statuses.find((s) => s.key === caseStatus)?.label ?? null; } + +/** למה נפלנו לערך-הגולמי, או `null` כשהסטטוס מוכר. הקורא **חייב** לדווח על ערך לא-null. */ +export type StatusLabelFallback = "unknown_status" | "model_unavailable" | null; + +export interface StatusLabelResolution { + label: string; + fallback: StatusLabelFallback; +} + +/** + * התווית להצגה עבור `caseStatus`, יחד עם **סיבת** נפילה-לגולמי אם הייתה. + * + * ההחלטה מופרדת כאן מהדיווח (`ctx.logger`) בכוונה: `worker.ts` אינו ניתן + * ל-import בטסט (הוא מריץ `runWorker(...)` ברמת-המודול), ולכן ההיגיון שחייב + * כיסוי-טסט חי במודול הטהור הזה — בדיוק כמו `pickSyncTargetIssue` (#446) + * ו-`resolveIssueStatus` (#604). legal-ai issue #616. + * + * `statuses === null` = מודל-הסטטוסים לא נטען כלל (כשל-רשת) — נבדל מסטטוס + * שנטען ולא נמצא, כי הן שתי תקלות שונות עם אבחון שונה. + */ +export function resolveStatusLabel( + statuses: readonly StatusModelEntry[] | null, + caseStatus: string, +): StatusLabelResolution { + if (statuses === null) { + return { label: caseStatus, fallback: "model_unavailable" }; + } + const label = labelFor(statuses, caseStatus); + if (label === null) { + return { label: caseStatus, fallback: "unknown_status" }; + } + return { label, fallback: null }; +} diff --git a/src/worker.ts b/src/worker.ts index fbef8e6..fb79b25 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -10,6 +10,7 @@ import { resolveApiBaseFrom, runJobHandler, } from "./company-scope.js"; +import type { StatusModelEntry } from "./legal-api.js"; import { LegalApi } from "./legal-api.js"; import { formatSyncRunSummary, @@ -23,12 +24,18 @@ import { labelFor, pickSyncTargetIssue, resolveIssueStatus, + resolveStatusLabel, type SyncCandidate, } from "./sync-target.js"; // Hoisted so onWebhook can access the context after setup() completes. let pluginCtx: PluginContext | null = null; +// Hoisted for the same reason as pluginCtx: onWebhook is a sibling of setup() +// and has no closure over the LegalApi instance created there. Same instance, +// not a second one — see legal-ai issue #616. +let legalApi: LegalApi | null = null; + // Per-company CEO agent IDs (shared between setup and onWebhook). const CEO_AGENT_IDS: Record = { "42a7acd0-30c5-4cbd-ac97-7424f65df294": @@ -103,6 +110,7 @@ const plugin = definePlugin({ // Lazy: the URL is fetched on the first request, from inside a handler // that has company context — not here. See resolveApiBase() above. const api = new LegalApi(() => resolveApiBase(ctx)); + legalApi = api; ctx.logger.info("Legal AI plugin starting"); @@ -113,7 +121,7 @@ const plugin = definePlugin({ { displayName: "רשימת תיקי ערר", description: - "List all appeal cases in the legal system. Returns case number, title, and status (new/in_progress/drafted/reviewed/final).", + "List all appeal cases in the legal system. Returns case number, title, and status (canonical case-status keys — see GET /api/status-model).", parametersSchema: { type: "object", properties: {}, @@ -254,10 +262,16 @@ const plugin = definePlugin({ type: "string", description: "Case number (e.g. 123/24)", }, + // ללא enum בכוונה: הרישום קורה ב-setup(), שאין לו הקשר-חברה + // ולכן אינו יכול לשלוף את /api/status-model (ראה resolveApiBase + // למעלה) — והעתק סטטי היה נסחף שוב. המניפסט (manifest.ts) כבר + // מגדיר את השדה כמחרוזת חופשית, והוולידציה האמיתית היא + // server-side (case_status_model.py + השומר הקדימה-בלבד + // ב-tools/cases.py). legal-ai issue #616. status: { type: "string", - enum: ["new", "in_progress", "drafted", "reviewed", "final"], - description: "New case status", + description: + "New case status — a canonical key from GET /api/status-model", }, title: { type: "string" }, subject: { type: "string" }, @@ -1488,7 +1502,7 @@ const plugin = definePlugin({ } try { - await handleCaseStatusWebhook(pluginCtx, input); + await handleCaseStatusWebhook(pluginCtx, legalApi, input); } catch (err) { // מחיקת סמן-האידמפוטנטיות **לפני** שהשגיאה יוצאת — אחרת מסירה // חוזרת של אותו webhook תוך 5 דק' תדולג בשקט כ"כבר נשלח", למרות @@ -1520,6 +1534,7 @@ const plugin = definePlugin({ */ async function handleCaseStatusWebhook( pluginCtx: PluginContext, + legalApi: LegalApi | null, input: PluginWebhookInput, ): Promise { const { endpointKey, parsedBody } = input; @@ -1703,30 +1718,48 @@ async function handleCaseStatusWebhook( return; } - // Status label map (Hebrew) - const statusLabels: Record = { - new: "📂 תיק חדש", - uploading: "📤 העלאת מסמכים", - processing: "⚙️ עיבוד מסמכים", - documents_ready: "📁 מסמכים מוכנים", - outcome_set: "🎯 תוצאה הוזנה", - brainstorming: "💡 סיעור מוחות", - direction_approved: "✅ כיוון אושר", - drafting: "✍️ כתיבה בתהליך", - qa_review: "🔍 בדיקת איכות", - in_progress: "🔄 בעבודה", - drafted: "✍️ טיוטה מוכנה", - qa_failed: "❌ QA נכשל", - exported: "📄 יוצא ל-DOCX", - reviewed: "✅ נבדק", - final: "🎯 סופי", - }; - const label = statusLabels[newStatus] ?? newStatus; + // התוויות נגזרות מ-`GET /api/status-model` — אותו SSOT שהג'וב + // `sync-case-status` כבר צורך (#604). המפה הקשיחה שהייתה כאן כבר סטתה: + // חסרו בה מפתחות חדשים מהמודל, והיו בה מפתחות מתים שכבר לא קיימים בו. + // legal-ai issue #616. + let statuses: StatusModelEntry[] | null = null; + if (!legalApi) { + // לא אמור לקרות (מושם ב-setup, כמו pluginCtx) — אבל אם קרה, זו נפילה + // לערך-גולמי שחייבת עקבה, בדיוק כמו כשל-הרשת שמתחתיה. + pluginCtx.logger.error( + "onWebhook: LegalApi unavailable — labels fall back to raw keys", + { caseNumber, newStatus }, + ); + } else { + try { + statuses = (await legalApi.getStatusModel()).statuses; + } catch (err) { + pluginCtx.logger.error( + "onWebhook: status-model fetch failed — labels fall back to raw keys", + { caseNumber, newStatus, error: String(err) }, + ); + } + } + const resolved = resolveStatusLabel(statuses, newStatus); + if (resolved.fallback === "unknown_status") { + // לא בליעה שקטה: הסטטוס נטען ופשוט אינו במודל הקנוני. + pluginCtx.logger.warn( + "onWebhook: status not in canonical status-model — using raw key as label", + { caseNumber, oldStatus, newStatus }, + ); + } + // `model_unavailable` כבר דווח כ-error למעלה — אין הכפלת-דיווח. + const label = resolved.label; + // `oldStatus` מתורגם אף הוא, אך **בלי** לוג: הוא ריק לגיטימית בתיק-חדש + // ועשוי להיות מפתח היסטורי שכבר אינו במודל — warn עליו היה רעש כרוני. + const oldLabel = oldStatus + ? resolveStatusLabel(statuses, oldStatus).label + : oldStatus; // Post a Hebrew status comment on the linked issue await pluginCtx.issues.createComment( linkedIssueId, - `**עדכון סטטוס תיק ${caseNumber}:** ${label} (היה: ${oldStatus})`, + `**עדכון סטטוס תיק ${caseNumber}:** ${label} (היה: ${oldLabel})`, companyId, );