#3485 fix(mcp): מחרוזת שנראית כמו JSON מגיעה לכלי כמחרוזת — pre_parse_json מדלג על פרמטרי str (#3471)

✨ תיאור קצר

לפני הוולידציה ה-SDK של mcp מריץ FuncMetadata.pre_parse_json: כל מחרוזת שנשלחת לפרמטר שאינו מוצהר str בדיוק עוברת json.loads, ו-None, רשימה או מילון מחליפים אותה. בפרמטר str | None זה היה באג שקט: "null" נהיה "לא נשלח", ולכן query="null" ו-section="null" החזירו את הקובץ המלא. התיקון יושב בתפר שכבר קיים — AdminAwareFastMCP.add_tool — ומוציא מהפענוח כל פרמטר שה-annotation שלו כולל str, כך שהמחרוזת מגיעה לכלי כמו שנשלחה.

Closes #3471

📦 שינויים עיקריים

• קוד (Backend)

• בוט טלגרם

• מסד נתונים/מיגרציות

• תיעוד (docs/)

• DevOps/CI/CD

• mcp_server/server.py — AdminAwareFastMCP.add_tool מאתר את הכלי שה-SDK יצר זה עתה (השם שלא היה רשום לפני הקריאה — בלי לגזור מחדש את כלל השמות של ה-SDK), ומחליף את tool.fn_metadata ב-_RawStringMetadata: תת-מחלקה של FuncMetadata שמוציאה את מפתחות המחרוזת לפני שה-SDK מפענח, ומעבירה את כל השאר אליו. _offload_to_thread, _declares_write ו-tool.fn לא השתנו.

• הכלל (_keeps_raw_string) נגזר מ-FieldInfo.annotation — אותו שדה ש-pre_parse_json קורא — ולא מרשימת שמות, ומכריע במפורש על כל צורה:

  • str, לבד או לצד None/int/float/bool ← מדלגים על הפענוח.
  • בלי str ← ה-SDK ממשיך כרגיל (lines, items — Claude Desktop שולח רשימות כמחרוזות JSON).
  • str לצד רשימה/מילון/tuple/set/מודל, Any/object, או צורה שהכלל לא מכיר ← TypeError ברישום, עם הסיבה, והכלי מוסר — כמו כלי כתיבה אסינכרוני.
  • שם השדה וגם ה-alias (ה-SDK משנה שם לפרמטר שמצל על מתודה של BaseModel, למשל json).
  • מטא-דאטה שאינה בדיוק FuncMetadata ← TypeError, כדי שמחלקה חדשה של ה-SDK לא תאבד בהעתקה את מה שהיא מוסיפה.

• ההערה ב-build_mcp טענה ש-PostHog "no tool schema is touched" — בסתירה ל-instrument_mcp_server, שמזריק context לכל סכימה. תוקנה.

• docs/mcp-server.rst — נמחקו שתי השורות שתיעדו את "null" (במצבי הקצה של mcp-file-sections, ובטבלת "פתרון תקלות"), והפסקה על pre_parse_json בסעיף content_changed עודכנה: מעכשיו זה נכון לכל פרמטר שמקבל מחרוזת.

• docs/whats-new.rst — רשומה ל-2026-09-29.

• tests/test_mcp_content_sha256.py — רק ה-docstring: המוטציה code: str | None הפילה שם שלוש בדיקות לפני התיקון, ועכשיו עוברת (נמדד בשני הכיוונים). הטסט עצמו לא השתנה.

🧪 בדיקות

• Unit
• Integration (session אמיתי של הפרוטוקול, PostHog אמיתי עם send=False)
• Manual

tests/test_mcp_pre_parse_json.py (55 בדיקות), כולן דרך call_tool:

1. כל פרמטר מחרוזת בכל כלי רשום מקבל את "null", "[1, 2]" ו-'{"a": 1}' כמו שנשלחו. הפרמטרים נגזרים מהסכימה שמוצהרת ללקוח (קוד של pydantic) ולא מהכלל שנבדק, ולכן הרשימה אינה טאוטולוגית. המספר 28 לא מקובע; נבדק רק שרשימת האישו מוכלת במה שכוסה.
2. בקרה שלילית: lines="[5, 10]" ב-get_file וב-get_repo_file, ו-items כמחרוזת JSON ב-read_batch, עדיין מפוענחים.
3. קצה-לקצה עם גוף אמיתי: query="null" מחפש את המילה; "[1, 2]"/'{"a": 1}' הם חיפוש מילולי; כותרת ששמה null נקראת בשמה; קובץ ששמו null נמצא; docs_get_section(section="null") מחזיר סעיף ולא מפה; update_note(content="null") כותב את המילה — נבדק במצב שהגיע לשכבת הכתיבה, לא בתשובת הכלי.
4. PostHog דלוק: build_mcp אמיתי, לקוח Posthog אמיתי, וכל הקריאות ב-session אמיתי. נבדק ש-context מופיע כחובה בכל סכימה (כלומר אלו הסכימות של הייצור), שהמחרוזות מגיעות כמו שנשלחו, ושה-context המוזרק לא מגיע לכלי.
5. הכלל ברישום: str | list[str] (ועוד תשע צורות) מפיל את הרישום עם הסיבה הנכונה, והכלי אינו רשום אחרי הסירוב.
6. ה-SDK: הגרסה הנעוצה ב-requirements/base.txt = המותקנת; ה-SDK עדיין קורא ל-self.pre_parse_json (נספר); וה-SDK לבדו עדיין ממיר "null" ל-None (אם יתקנו upstream — הטסט ייפול, וזה הסימן לבדוק אם השכבה עדיין נחוצה).

ריצת בקרה על הקוד שלפני התיקון (5d7fdc2, ב-worktree נפרד): 43 נופלות, 12 עוברות. ה-12 הן בדיוק מה שאמור לעבור בשני המצבים: הבקרה השלילית, str בדיוק, שני שומרי ה-SDK, ובדיקת ההכלה של רשימת האישו. הנפילה בטסט הפרמטרי היא 28 שורות של sent 'null', the tool received None — הבאג כלשונו.

מוטציות (על עותק ב-git worktree, כל אחת החלפה מדויקת אחת):

מוטציה	תוצאה
M1 לבטל את הפענוח לכל המפתחות	7 נופלות — הבקרה השלילית, ריצת PostHog, ו-list/dict/מודל
M2 str לצד container לא מסורב	5 נופלות
M3 Any מאבד את הסירוב המפורש שלו	3 נופלות (לפי הסיבה בהודעה)
M4 ה-alias לא מוחזק	1 נופלת
M5 add_tool לא מחליף מטא-דאטה	43 נופלות
M6 כלי מסורב נשאר רשום	10 נופלות
M7 צורה לא מוכרת נשמרת (ניחוש)	2 נופלות
M8 רק str | None נחשב טקסט	9 נופלות
M9 מודל לא נחשב container	2 נופלות
M10 Annotated לא מקולף	14 נופלות
M11 typing.Union לא מטופל	17 נופלות
M12 מחלקת המטא-דאטה לא נבדקת	1 נופלת
M13 str | list[str] | None בכלי אמיתי (codekeeper_list_versions)	השרת לא נבנה; TypeError נוקב בכלי ובפרמטר

שאר הסוויטה (-n auto --dist=loadscope, כמו ב-CI, Python 3.11):

• כל טסטי ה-MCP: 1723 עברו, 36 דולגו (לפני: 1668 עברו, 36 דולגו — אותם דילוגים של "דורש מונגו אמיתי").
• הסוויטה כולה: 6787 עברו, 5 נפלו — כולן ב-tests/test_infrastructure.py::TestEnvironmentTools, ובודקות ש-isort/autopep8 ב-PATH. אומת שהן נופלות זהה על הבסיס, ועוברות כשה-venv ב-PATH — סביבתי, לא קשור.
• כל 1460 הטסטים שקוראים קובצי תיעוד רצו שוב אחרי עריכת ה-rst — עברו.
• flake8 (החוסם) נקי; ruff ו-mypy — אותם ממצאים בדיוק כמו בבסיס, אין חדשים.
• לא אומת: ריצה על Python 3.12 (ה-CI מריץ גם אותו); הטסטים נשענים על typing ו-pydantic באופן שזהה בשתי הגרסאות, אבל לא הרצתי.

🧪 בדיקות נדרשות ב‑PR

• 🔍 Code Quality & Security
• Unit Tests (3.11)
• Unit Tests (3.12)

📝 סוג שינוי

• feat: פיצ'ר חדש
• fix: תיקון באג
• docs: שינוי תיעוד בלבד
• refactor: שינוי קוד ללא שינוי התנהגות
• perf: שיפור ביצועים
• chore/ci: תשתית/CI
• breaking change: שינוי שובר תאימות

✅ צ'קליסט

• הקוד עוקב אחרי הסגנון (Black/isort/flake8/mypy)
• בדיקות רצות ועוברות
• תיעוד עודכן (README/Docs)
• אם נוספו ג'ובים חדשים (Background Jobs) — לא רלוונטי
• אם נוספו/שונו משתני סביבה — לא רלוונטי
• אם נוספו/השתנו טוקנים — לא רלוונטי
• אין סודות/מפתחות בקוד
• אין מחיקות מסוכנות/פעולות על root
• הודעת הקומיט תואמת Conventional Commits
• CHANGELOG עודכן אם נדרש (docs/whats-new.rst)
• כל ה‑Required Checks לעיל ירוקים — ממתין ל-CI
• צילום/וידאו UI — לא רלוונטי
• עיינתי במסמכי אתר התיעוד — נתיב: docs/mcp-server.rst | המשפט: "הקולבק מקבל את מילון הארגומנטים הגולמי — לפני ש-pydantic מסלק ממנו מפתחות שאינם בסכימה של הכלי"

עיון ב-CodeBot – Project Docs: AI-MAP.md; ב-docs/mcp-server.rst — "מצבי הקצה" של mcp-file-sections, "פתרון תקלות", "טביעת אצבע לתוכן" › "תשובות הכתיבה: content_changed", "מדידת שימוש" › "לכידת הכוונה (context)" ו"מצב הקריאה (ck_read_mode)"; docs/doc-authoring.rst (לפני עריכת ה-rst — כולל "ספירות בפרוזה"); docs/versioning-stable-anchors.rst (לא נמחק ולא שונה אף עוגן). מחוץ לריפו (Ck): Ck-MCP-Handoff.md — "מודל הריצה של הכלים" כולו, "מוסכמות" כולו, "הערך שהכלי מקבל אינו תמיד הערך שנשלח", "שער הפרטיות", "run_in_executor אינו מעתיק contextvars"; לקחי-RST-להרחבת-האאוטליין-ל-md.md סעיפים 10 ו-13.

🧩 השפעות/סיכונים

• שינוי התנהגות מכוון: מחרוזת שנשלחת לפרמטר מסוג "מחרוזת או null" מגיעה לכלי כמחרוזת. סוכן ששלח "null" כדי לומר "לא נשלח" (אין סיבה לעשות זאת — משמיטים את הפרמטר, או שולחים null של JSON) יקבל עכשיו חיפוש של המילה.
• שדרוג ה-SDK: הבדיקה ב-pre_parse_json זהה גם ב-mcp 1.30.0 וגם ב-2.2.0 (נקרא מהמקור, 29.09) — שדרוג לא מייתר את התיקון. ב-2.x mcp.server.fastmcp מוחלף במודול שזורק ModuleNotFoundError בכוונה, כלומר שדרוג כזה ייפול בייבוא ולא יחזיר את הבאג בשקט.
• ביצועים: שתי קריאות list_tools() לכל רישום (בעלייה בלבד), ובכל קריאת כלי — שני dict comprehension על הארגומנטים. גוף טקסט גדול (פתק, למשל) כבר לא עובר json.loads.
• ממצא שלא טופל כאן: FastMCP(tools=[...]) רושם כלים בלי לעבור ב-add_tool, כך שכלי שיירשם כך יעקוף גם את _offload_to_thread וגם את התיקון הזה. אף אחד לא משתמש בזה היום, ושני טסטים מבניים (test_every_registered_tool_is_a_coroutine_function, test_every_registered_tool_carries_the_string_preserving_metadata) תופסים את זה ב-build_mcp.

🔗 קישורים

• Issues קשורים: #3471 (נסגר ב-PR הזה); #3484 — ארגומנט ששמו אינו בסכימה (למשל secton="K11") עדיין נזרק בשקט. נמדד כאן דרך call_tool, עם PostHog ובלעדיו, ונפתח כאישו נפרד כי התיקון צריך להתחשב ב-context שמוזרק רק בייצור.
• Docs Preview: ה-check של Read the Docs על ה-PR.
• מסמכים/מפרטים רלוונטיים:
• Branch Protection & PR Rules

🧯 סיכון / החזרה לאחור (Rollback)

• Revert של הקומיט מחזיר את ההתנהגות הקודמת במלואה: אין שינוי במסד, בסכימות שהלקוח רואה או בקונפיגורציה.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MEsSHNcaSFuF5dKwQkQcLC

---

Generated by Claude Code

Summary by Sourcery

Preserve text arguments at the MCP boundary while retaining JSON parsing for collection-oriented parameters.

Bug Fixes:

• Preserve JSON-looking strings for MCP tool parameters that accept text, including optional and scalar-union string annotations, so values such as "null", arrays, and objects reach tools unchanged.

Enhancements:

• Retain SDK JSON parsing for parameters that do not accept strings and reject ambiguous annotation shapes during tool registration instead of guessing.
• Update MCP documentation and changelog entries to reflect the corrected string-handling behavior and PostHog schema instrumentation.

Documentation:

• Remove outdated documentation describing JSON-looking "null" values as omitted and clarify the scope of MCP pre-parsing behavior.

Tests:

• Add comprehensive MCP tests covering tool calls, production PostHog sessions, registration validation, SDK behavior, aliases, and regression scenarios.