Claude/mcp codekeeper webapp ldnzsg

FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md

@@ -1,6 +1,10 @@
-# חיבור Claude.ai ל‑CodeKeeper דרך MCP — מסמך תכנון
+# חיבור Claude.ai ל‑CodeKeeper דרך MCP — תכנון ולקחי מימוש
 
-> **סטטוס:** פאזות 0 (PAT) **+** 1 (OAuth 2.1 ל‑Claude.ai) **+** 3 (כתיבה: save + edit/append חלקיים) **+** ד' (דפדפן ריפו לאדמין, קריאה בלבד) **+** פתקים דביקים (list/create/update) **מומשו** — ראו `mcp_server/`. מחיקה (קבצים ופתקים) וכתיבת אוספים עדיין בתכנון.
+> **איך לקרוא את המסמך:** סעיפים 1–13 הם התכנון המקורי (נשמרו כפי שהיו, עם סימוני
+> סטטוס). **סעיף 14 הוא מה שגילינו אחרי שהתחלנו לכתוב קוד** — אם אתם מממשים שרת MCP
+> בפרויקט אחר, התחילו משם: כל פריט שם עלה בסבב תיקון אמיתי.
+>
+> **סטטוס:** פאזות 0 (PAT) **+** 1 (OAuth 2.1 ל‑Claude.ai) **+** 3 (כתיבה: save + edit/append חלקיים) **+** ד' (דפדפן ריפו לאדמין, קריאה בלבד) **+** פתקים דביקים (list/create/update) **+** `docs_get_section` (קריאת תיעוד RST, ציבורי) **מומשו** — ראו `mcp_server/` (18 כלים). מחיקה (קבצים ופתקים) וכתיבת אוספים עדיין בתכנון.
 > **ענף פיתוח:** `claude/mcp-codekeeper-webapp-ldnzsg`
 > **מתי להשתמש:** לפני מימוש חיבור MCP; מסמך זה הוא מקור האמת לתכנון.
 > **ראו גם:** `mcp_server/README.md` (שימוש), [CodeBot – Project Docs](https://amirbiron.github.io/CodeBot/), `CLAUDE.md` (מדיניות מחייבת).
@@ -375,7 +379,183 @@ db.delete_file(user_id, file_name)                 # מחיקה רכה (recycle
 
 ---
 
-## 14. מקורות
+## 14. מה התכנון לא צפה — לקחים מהמימוש
+
+הסעיפים 1–13 הם התכנון. הסעיף הזה הוא מה שגילינו **אחרי** שהתחלנו לכתוב קוד. אם אתם
+מממשים שרת MCP בפרויקט אחר, זה החלק שיחסוך לכם את הזמן — כל פריט כאן עלה לנו בסבב תיקון.
+
+המצב בפועל: 18 כלים, `mcp_server/` עם 17 מודולים, ארבע פאזות בפרודקשן.
+
+### 14.1 ל‑SDK אין scopes פר‑כלי — האכיפה היא בגוף הכלי
+
+זו ההפתעה הראשונה, והיא משנה איך בונים את השרת. ה‑MCP Python SDK יודע לאמת טוקן ולתת
+לכם `ctx` עם ה‑scopes שלו, אבל **אין מנגנון שאומר "הכלי הזה דורש scope write"**. הרישום של
+כלי הוא דקורטור, לא הצהרת הרשאה.
+
+המשמעות: כל כלי כותב חייב לקרוא בעצמו, בשורה הראשונה שלו, לבדיקה. אצלנו:
+
+```python
+@mcp.tool(name="codekeeper_save_file", ...)
+def save_file(ctx: Context, file_name: str, code: str, ...) -> dict:
+    require_write(ctx)   # דוחה טוקן קריאה-בלבד לפני כל נגיעה בנתונים
+    return handlers.save_file(backend, user_id=current_user_id(ctx), ...)
+```
+
+אצלנו זה עשרה מקומות: חמישה כלים קוראים `require_write`, וחמישה קוראים `require_admin`.
+אין נקודת אכיפה מרכזית אחת — יש עשר נקודות שכל אחת מהן חייבת לזכור בעצמה.
+
+**הטעות הנפוצה:** להניח שהרישום מגן. הסימפטום: כלי כתיבה עובד עם טוקן קריאה‑בלבד בבדיקה
+ידנית. הפתרון: טסט שמנסה כל כלי כותב עם טוקן `read` בלבד ומצפה לדחייה — לא לסמוך על עין.
+
+### 14.2 הסתרת כלי מ‑tools/list היא UX, לא בקרת גישה
+
+בנינו תת‑מחלקה של FastMCP שמסננת את כלי הריפו מ‑`list_tools` כשהמשתמש אינו אדמין. זה נוח,
+אבל **ה‑dispatcher מריץ גם כלי שאינו ברשימה**. מי שיודע את השם `codekeeper_get_repo_file`
+יכול לקרוא לו ישירות והוא ירוץ.
+
+לכן המבנה אצלנו הוא שתי שכבות נפרדות, ורק אחת מהן היא אבטחה:
+
+```python
+_ADMIN_TOOLS = frozenset({...})   # שכבה 1: הסתרה מהרשימה — נוחות בלבד
+# שכבה 2: require_admin(ctx) בשורה הראשונה של כל גוף כלי — זה החסם האמיתי
+```
+
+השארנו הערה מפורשת בקוד ליד ה‑frozenset ("every one of these also calls require_admin in
+its body") כי זה בדיוק סוג הקשר שנשכח בסבב התחזוקה הבא.
+
+### 14.3 שינוי scopes דורש רישום DCR מחדש
+
+זה תפס אותנו בפרודקשן. Claude.ai שומר את ה‑scopes שסוכמו ברישום הדינמי (DCR) של
+ה‑connector. כשהוספנו את scope ה‑`write`, המשתמשים הקיימים המשיכו לקבל טוקני `read` —
+והכלים החדשים נכשלו בלי סיבה נראית לעין.
+
+אין דרך "לרענן" את זה מהשרת. המשתמש חייב **להסיר את ה‑connector ולהוסיף אותו מחדש**.
+
+תכננו את ה‑scopes מראש, גם אם לא תממשו את כולם ביום הראשון. הוספת scope אחרי שיש
+משתמשים היא שינוי שובר‑תאימות מבחינתם.
+
+### 14.4 גשר זהות חתום — איך מחברים OAuth לזהות קיימת
+
+התכנון אמר "מסך ה‑authorize משתמש בסשן הטלגרם הקיים" (סעיף 6, פאזה 1). זה נכון, אבל
+בפועל צריך פתרון לבעיה שלא הופיעה בתכנון: **ה‑OAuth רץ בשירות ה‑MCP, והסשן חי בוובאפ.**
+שני תהליכים, שני שירותי Render, אין ביניהם זיכרון משותף.
+
+הפתרון: assertion זהות חתום, קצר‑מועד, שעובר בין השירותים. הוובאפ מזהה את המשתמש
+(סשן טלגרם), חותם `{user_id, txn, exp}` בסוד המשותף, ומעביר לשירות ה‑MCP; שם מאמתים
+את החתימה ומציגים מסך consent. תוקף חמש דקות — מספיק לאשר, קצר מכדי להיות שימושי אם דלף.
+
+זה בדיוק הדפוס של `webapp_tokens` שכבר היה בפרויקט (סעיף 2.3), רק בכיוון ההפוך. אם יש
+לכם כבר גשר טוקנים בין שירותים — השתמשו באותו דפוס במקום להמציא אחד.
+
+### 14.5 כלי שמקבל מזהה משאב הוא IDOR עד שהוכח אחרת
+
+הלקח הכי חד מכל הסבב הזה. הוספנו כלי **ציבורי** (`docs_get_section`) שמקבל פרמטר
+`repo` אופציונלי. הוא נראה תמים: ברירת המחדל היא הריפו שלנו, והוא רק קורא קבצי `.rst`.
+
+הבעיה: ה‑backend אימת רק את ה**פורמט** של שם הריפו, לא את ההרשאה. כלומר כל משתמש
+מאומת יכול היה להעביר `repo="SomeOtherRepo"` ולקרוא ממנו — **בדיוק מה שהשער האדמין
+של פאזה ד' נועד למנוע.** כלי ציבורי אחד עקף את כל מודל ההרשאות של דפדפן הריפו.
+
+התיקון הוא allowlist מפורש, ודחייה לפני שנוגעים ב‑backend:
+
+```python
+def _resolve_docs_repo(repo: str | None) -> str | None:
+    allowed = _allowed_docs_repos()        # CSV מ-MCP_DOCS_REPO
+    r = (repo or "").strip()
+    if not r:
+        return allowed[0]                  # ברירת מחדל
+    return r if r in allowed else None     # None ⇒ repo_not_allowed
+```
+
+**הכלל להכללה:** כל פרמטר שהוא מזהה של משאב (`repo`, `tenant`, `project`, `org`) בכלי
+שאינו מוגן — הוא IDOR עד שיש לו allowlist. ואם משנים משתנה סביבה לגבול אבטחה, תעדו
+אותו ככזה: `MCP_DOCS_REPO` מתועד אצלנו כ"רשימת ריפואים מותרים", לא כ"שם הריפו".
+
+### 14.6 נתיבים: לנרמל בשכבת ה‑handler, לא לסמוך על ה‑backend
+
+באותו סבב מצאנו שהכלי העביר גם `webapp/config.rst` (מחוץ ל‑`docs/`) וגם `../../etc/passwd`.
+ה‑backend אמנם חוסם traversal, אבל להסתמך עליו זה להעמיד שכבת אבטחה אחת בלבד.
+
+הכלל: לנרמל עם `posixpath.normpath` ואז לוודא שהתוצאה **מתחילה בקידומת המותרת**. סדר
+הפעולות חשוב — נרמול לפני הבדיקה, אחרת `docs/../secrets` עובר.
+
+```python
+norm = posixpath.normpath(p)
+if not norm.startswith("docs/"):
+    return None          # גם traversal, גם נתיב מוחלט, גם קלט עם slash מחוץ ל-docs/
+```
+
+### 14.7 תקציב טוקנים הוא דרישה תפקודית, לא אופטימיזציה
+
+`docs/environment-variables.rst` אצלנו הוא 77KB ומעל 2200 שורות. כדי לקרוא ערך אחד מטבלת
+משתני הסביבה, מודל היה צריך למשוך את כולו — ולשרוף חלון הקשר שלם על שורה אחת.
+
+מכאן נולד `docs_get_section`: מחזיר **סקשן בודד** מקובץ RST במקום קובץ שלם. שני דברים
+שכדאי להעתיק מהעיצוב שלו:
+
+**ראשית, כלי חיפוש שמחזיר מספר שורה כמעט חסר ערך בתיעוד.** מה שרלוונטי הוא באיזה
+*סעיף* אתה נמצא, לא באיזו שורה. אם אתם בונים כלי קריאה לתיעוד — חשבו במונחי סקשנים.
+
+**שנית, לעולם אל תחזירו למודל רק "לא נמצא".** מודל שקיבל `{"error": "not_found"}` תקוע:
+הוא לא רואה את הקובץ ולא יודע מה כן קיים. אצלנו כל תשובה מחזירה ערך שמיש:
+
+| מצב | מה מוחזר |
+|------|-----------|
+| בלי `section` | עץ הכותרות המלא (הכלי משמש גם לניווט) |
+| `section` לא נמצא | TOC מלא + הצעות קרובות (difflib) |
+| כותרת מופיעה פעמיים | כל המועמדים עם breadcrumb — בלי לנחש |
+| נמצא | הסקשן + שכנים + תת‑סקשנים לניווט המשך |
+
+זה מוסיף אולי שלושים שורות קוד וחוסך למודל סיבוב שלם של ניחושים.
+
+### 14.8 תיאור הכלי הוא ממשק, לא תיעוד
+
+מודל בוחר כלי לפי התיאור. אם יש לכם שני כלים חופפים — `get_repo_file` שמחזיר קובץ שלם
+ו‑`docs_get_section` שמחזיר חלק — התיאור חייב להגיד **מתי להעדיף את מי**, אחרת המודל
+יבחר את הראשון שנראה לו מתאים וימשוך 77KB.
+
+נסחו את התיאור כהוראה, לא כהגדרה: "השתמש בזה במקום `get_repo_file` כשצריך חלק ממסמך
+RST — מחזיר סקשן בודד עם ניווט במקום קובץ שלם; בלי `section` מחזיר עץ כותרות."
+
+### 14.9 פארסר עצמאי, לא לוגיקה בתוך ה‑handler
+
+את פענוח ה‑RST הוצאנו ל‑`services/rst_parser.py` — מודול טהור בלי שום תלות ב‑MCP. זה
+נראה כמו over‑engineering לכלי אחד, אבל שתי סיבות מצדיקות את זה:
+
+הראשונה, אפשר לבדוק אותו על קבצי RST אמיתיים מהריפו בלי להרים שרת MCP. הטסטים שלנו
+קוראים את `docs/environment-variables.rst` האמיתי ומוודאים שמקרי הקצה מטופלים —
+למשל ששורת `===` **בתוך** בלוק קוד לא מזוהה ככותרת.
+
+השנייה, כלים עתידיים (חיפוש בתיעוד, שליפת טבלת env-vars כדאטה מובנית) משתמשים באותו
+פארסר. הלוגיקה שקשה לכתוב נכון נכתבת פעם אחת.
+
+מקרי הקצה שתפסו אותנו ב‑RST, אם אתם מפרסרים משהו דומה: רמות כותרת נקבעות **דינמית**
+לפי סדר ההופעה של תווי הקישוט בקובץ (לא `===` = תמיד H1); כותרת חייבת להיות בעמודה 0,
+אחרת כל טבלה מוזחת נראית ככותרת; ובלוקי literal אחרי `::` מכילים שורות שנראות בדיוק
+כמו קישוט כותרת.
+
+### 14.10 השירות צריך לסנכרן את הדיסק של עצמו
+
+פאזה ד' קוראת מ‑bare mirrors על הדיסק. בתכנון זה נראה כמו פרט תפעולי, אבל בפרודקשן
+התברר: שירות ה‑MCP הוא **שירות Render נפרד עם דיסק נפרד**. ה‑sync שרץ בוובאפ מעדכן את
+הדיסק של הוובאפ — לא של ה‑MCP.
+
+התוצאה: ה‑MCP הגיש גרסאות ישנות של קבצים, בלי שום שגיאה. הוספנו worker
+(`mcp_server/repo_autosync.py`) שרץ בתוך שירות ה‑MCP ומושך `git fetch` לדיסק שלו, עם
+`MCP_REPO_AUTOSYNC_INTERVAL` (ברירת מחדל 300 שניות).
+
+**הכלל:** אם השירות שלכם קורא ממצב על דיסק שגורם אחר מייצר — ודאו מי בדיוק מעדכן את
+העותק שהשירות הזה רואה. "יש sync" זו לא תשובה; השאלה היא סנכרון של *איזה* דיסק.
+
+### 14.11 מה שעדיין לא מומש
+
+כדי שהמסמך יישאר מדויק: מחיקת קבצים (`delete_file`), כתיבה לאוספים, מחיקת פתקים
+ותזכורות — כולם נשארו בתכנון. `get_version_diff` ו‑Resources (סעיף 5.2) גם הם לא מומשו.
+זה לא פער שדורש תיקון; זה היקף שנבחר במודע.
+
+---
+
+## 15. מקורות
 - [Claude Help Center — Get started with custom connectors using remote MCP](https://support.claude.com/en/articles/11175166-get-started-with-custom-connectors-using-remote-mcp)
 - [Claude Docs — Authentication for connectors](https://claude.com/docs/connectors/building/authentication)
 - [CodeBot – Project Docs](https://amirbiron.github.io/CodeBot/)