Repository navigation
Conversation
…אות, זנב תשובה נפרד, וטסטים שחסרו (#3432) שמונה מאחת-עשרה ההצעות ממומשות, שלוש מוכרעות במפורש: - SUGG-013: path_outside_root — נתיב תקין בצורתו שפותר אל מחוץ לשורש התיעוד (docs/../secrets, /etc/hosts, ../README.md בשורש ריק) נדחה בשמו, עם root בתשובה; missing_path נשאר לנתיב ריק או עם NUL בלבד. - SUGG-011: repo_not_mirrored — מראה שאין למארח (repo_not_found מהשירות) אינה not_found; invalid_commit נשאר not_found (הריפו קיים, ה-ref הוא מה שהקורא יכול לשנות); בזמן sync שניהם sync_in_progress כמו קודם. עובר גם דרך codekeeper_docs_get_section עם repo ו-path. - SUGG-021: _PARSERS ו-DOCS_PATH_POLICY מיוצאים כ-MappingProxyType — הוולידציה בייבוא היא הבטחה רק אם הטבלה אינה משתנה אחריה; _PARSER_TABLE ו- _DOCS_PATH_POLICY_TABLE הם התפר לטסטים (monkeypatch.setitem). - SUGG-020: זנב עיצוב התשובה של docs_get_section נפרד ל-_answer_from_document (ארבע צורות התשובה מתוך מסמך שכבר נפרסר). אפס-דיף: 5,935 רשומות על אותו קורפוס, 5,933 זהות בית-בית; השתיים ששונות הן בדיוק שני הפרובים של SUGG-013 (missing_path ← path_outside_root). - SUGG-009: חמישה טסטים לשתי בדיקות הנרמול ב-_validate_policy_tables (סיומת ברישיות/בלי נקודה, שורש מוחלט/עם לוכסן סוגר/עם ./). - SUGG-010: טסט caplog לשורת ה-WARNING על repo_not_configured. - SUGG-004: טסט ההסכמה בין חיפוש לקריאה אינו דורש עוד returned == allowed — repo_policy מצהיר ש-is_denied נשאר שכבה אחרונה, ושוויון מדויק דרש את ההפך; נשאר "אפס חסומים בתוצאות" + "הקובץ המותר כן חוזר" נגד ריקנות. - SUGG-005: _require_git אחד לשני הטסטים תלויי-git — דילוג בלי הבינארי במקום skip באחד וקריסה על check=True בשני. - SUGG-022 (הכרעה, מתועדת ב-docs/mcp-server.rst): סירוב הוא תשובה של הפרוטוקול ולא תקרית; אף מסלול סירוב בשכבת המטפלים אינו רושם שורה, בכל הקבצים באותה מידה; לוג למה שהמפעיל צריך לדעת, תשובה למה שהקורא צריך; ספירת סירובים — ב-PostHog. - SUGG-012 (נדחה בנימוק): תצוגת התצורה ב-services אינה יכולה לייבא את mcp_server.docs_handlers — הכיוון חד-סטרי ומוצהר בכמה מקומות; התקלה כבר קולנית בזמן ריצה (WARNING לכל קריאה + repo_not_configured). - SUGG-018 (נדחה בנימוק): שני חצאי מדיניות הסודות נשארים בשתי שפות כי אין מנוע אחד לשניהם; מה שמחזיק אותם יחד — רשימת תבניות אחת, טסט אינטגרציה על מראה אמיתית, וטסט ישיר על ארבע הצורות. טסטים: 17 נופלים על origin/main (הקודים החדשים, הפרוקסי, הוולידטור, המראה החסרה), 7 עוברים שם כפינים ומוכחים במוטציות — מחיקת ה-WARNING, טבלאות כ-dict רגיל, ומחיקת בדיקת הסיומת מהוולידטור מפילות כל אחת את הטסט שלה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
There was a problem hiding this comment.
Sorry @amirbiron, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 hour and 7 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
Reviewer's GuideThe PR closes the remaining review findings by adding two precise, user-visible refusal codes, hardening policy tables against mutation, extracting response formatting into a zero-diff helper, tightening regression tests, and documenting both behavior and explicit decisions about logging and policy duplication. Sequence diagram for precise MCP refusal codessequenceDiagram
participant Caller
participant DocsTool as docs_get_section
participant PathResolver as _resolve_docs_path
participant RepoBackend
participant Mirror
Caller->>DocsTool: docs_get_section(path)
DocsTool->>PathResolver: _resolve_docs_path(path, policy)
alt path resolves outside docs root
PathResolver-->>DocsTool: path_outside_root
DocsTool-->>Caller: ok=false, error=path_outside_root, root
else path is valid
DocsTool->>RepoBackend: get_file(repo, path, ref)
RepoBackend->>Mirror: get_file_at_commit(repo, path, ref)
alt repository mirror is missing
Mirror-->>RepoBackend: repo_not_found
RepoBackend-->>Caller: ok=false, error=repo_not_mirrored
else repository and path are available
Mirror-->>RepoBackend: file contents
RepoBackend-->>DocsTool: file contents
end
end
Flow diagram for document section response extractionflowchart LR
A[docs_get_section] --> B[Resolve repository, path, file, and parser]
B --> C[Parsed document]
C --> D[_answer_from_document]
D --> E[toc]
D --> F[section_not_found]
D --> G[ambiguous_section]
D --> H[section]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… סקירת שבעת ה-PRים (#3434–#3441) (#3443) * fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) - `candidates` היה השדה היחיד בתשובת `codekeeper_docs_get_section` בלי תקרה: `toc` חסום ב-400, `suggestions` ב-50, ורק המועמדים חזרו כולם — נמדד בסקירת #3425: 13,030 מועמדים ו-1,393,787 בתים על שאילתה בת שלושה תווים בעמוד סינתטי של 512KB. הענף נדלק מכותרות שנושאות מזהה, ומאז #3428 הקורפוס שנושא אותם (amir-bug-patterns) מוגש. - התקרה היא 50 — אותו מספר כמו MAX_IDENTIFIER_SUGGESTIONS ומאותה סיבה (מלאי בסדר הופעה ולא דירוג, גבוה מספיק שכל עמוד אמיתי ייענה במלואו), והשוויון מקובע בטסט. `candidates_truncated: true` מופיע רק כשנחתך, כמו `suggestions_truncated`: תצלום אפס-הדיף של הכלי על קורפוס ה-RST של main זהה בית-בית לפני ואחרי (5,928 רשומות, אותו sha256). - R6: החיתוך של `toc` ושל `candidates` עובר דרך עוזר אחד, `_capped`, במקום עותק שני של "עד התקרה, ודגל רק כשנחתך". - הצרכן השני מ-#3426 הוא בפועל אותו אתר קריאה לשני הפורמטים (מאז #3428), והוא כותב `.titles`; נוסף טסט שמקבע את זה במסלול ה-Markdown, כי אזהרה ב-docstring אינה מנגנון. - תיאור הפרמטר `section` ו-docs/mcp-server.rst אומרים את התקרה והדגל; רשומה ב-whats-new. שורה ריקה אחת נוספה ב-server.py לפני `_build_docs_path_doc` (E302 שהגיע עם #3428; CI בוחר רק E9/F63/F7/F82 ולכן לא נפל שם). טסטים: 135 ב-test_mcp_docs_handlers + test_doc_sections ו-36 ב-test_mcp_server_build ירוקים; טסט התקרה וטסט השוויון נופלים על origin/main ב-worktree נפרד. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * fix(mcp): תקרת גודל לגוף בקשה והגבלת קצב לפי זהות, עם סירוב שנוקב בסיבתו (#3431) עד היום המידלוור היחיד בשרת ה-MCP היה האימות: כל משתמש מאומת יכול היה לשלוח בקשות בכל גודל ובכל תדירות. שני גבולות חדשים, כל אחד בשכבה שלו, במודול חדש mcp_server/limits.py: - גודל הגוף: מידלוור ASGI טהור (BodySizeLimitMiddleware) שמסרב ב-413 {"error": "body_too_large", "max_bytes": ...} לפני שהטרנספורט של ה-SDK קורא ומפענח JSON. Content-Length שמצהיר על יותר מהתקרה נדחה לפני שנקרא בית אחד; כותרת חסרה או מכזבת נתפסת בספירה של מה שבאמת מגיע. חיץ ולא חריגה מתוך receive, כי _handle_post_request של ה-SDK עוטף את קריאת הגוף ב-try שתופס Exception ומחזיר שגיאת JSON-RPC בלי סיבה. ברירת המחדל 1MiB, נגזרת מ-MAX_CODE_SIZE (100,000 תווים, עד ~600KB כ-JSON עם ensure_ascii). - קצב לפי זהות: ב-AdminAwareFastMCP.call_tool, המתודה שה-SDK רושם כמטפל של tools/call, כלומר נקודה אחת שכל קריאת כלי עוברת בה בשני מצבי האימות (במצב OAuth PATAuthMiddleware אינו מותקן, ורק הקונטקסט של הקריאה רואה את הזהות). ההכרעה נופלת לפני שגוף סינכרוני נמסר לחוט ולפני שגוף אסינכרוני רץ; קריאה שנדחתה מחזירה תשובת כלי רגילה {"ok": false, "error": "rate_limited", "limit_per_minute": ..., "retry_after_seconds": ...}. מחוץ לבקשה (LookupError מ-request_context) אין את מי לחייב, ולכן הטסטים שקוראים לכלים ישירות ממשיכים כמו היום. 60 בדקה, מהמדידות בתגובה באישו: 0.24 שניות מעבד לעמוד RST עוין של 500KB, 0.47 למסמך Markdown הצפוף, 2.3 לצורה העוינת, מול מכסה של 0.5 מעבד (30 שניות-מעבד בדקה). - הפטור לנתיבי הדופק מבני ולא רשימה (blanket-policy-silent-block §7): /healthz אינו קריאת כלי ואין לו גוף, ומקובע בטסט על האפליקציה האמיתית. - rate_limiter.RateLimiter הקיים של הבוט משמש כמנוע (R6): ניקוי החלון אוחד ל-_live_entries, ונוספה seconds_until_allowed בשביל retry_after_seconds. - כיוון דרך MCP_MAX_REQUEST_BYTES (מינימום 65536) ו-MCP_RATE_LIMIT_PER_MINUTE (0 מכבה במפורש עם WARNING), נקראים ב-create_app ולא בזמן ייבוא; ערך פגום לעולם אינו מרחיב את הגבול (K12 §3). נרשמו ב-config_inspector_service ובתיעוד. אומת עם uvicorn אמיתי: 401 לפני 413 במצב PAT, 413 על Content-Length ועל גוף chunked בלי כותרת, 120 דגימות של /healthz תחת מגבלה של קריאה אחת בדקה כולן 200; ועם לקוח ה-MCP של ה-SDK על Streamable HTTP: הקריאה השנייה מחזירה rate_limited ו-tools/list אחריה עדיין עונה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * fix(parsers): rst_parser בודק את הקלט בכניסה, וברירת המחדל של max_sections מיושרת ל-MAX_SECTIONS (#3421, #3420) שני הפארסרים מתועדים כבני-החלפה, ועל קלט שאינו מחרוזת הם התנהגו אחרת: rst_parser.parse_document(None) החזיר מסמך ריק בשקט (מפה ריקה שמתחזה למפה של קובץ בלי כותרות), ו-17 נפל ב-AttributeError גולמי מתוך .replace, בעוד md_parser זרק TypeError שאומר מה התקבל. וברירת המחדל של max_sections הייתה None ב-rst_parser ו-MAX_SECTIONS ב-md_parser, כלומר קורא ששכח להעביר תקרה קיבל הגנה מהפארסר האחד ולא מהשני. - #3421: בדיקת כניסה אחת לשניהם, doc_sections.require_str, שמרימה TypeError עם אותן מילים ("parse_document expects str, got NoneType"). אף קורא לא נשען על הצורה הישנה: המטפל, הסורק והסקריפטים מעבירים תמיד מחרוזת. docs_get_section אינו תופס את החריגה, כמו שלא תפס אותה מ-md_parser: "חוזה נשבר" ולא "הקלט נדחה", ו-content שם תמיד מחרוזת. - #3420, ההכרעה: יישור. MAX_SECTIONS עובר ל-services.doc_sections (המודול המשותף), מיוצא מחדש משני הפארסרים, והוא ברירת המחדל בשניהם. נמדד לפני השינוי על כל 208 קובצי ה-RST ב-docs/: 1,384 סקשנים בסך הכול, הקובץ העשיר ביותר (docs/mcp-server.rst) נושא 50 מול תקרה של 50,000, אחד לאלף, ואף קובץ אינו נחסם. אפס-דיף על docs_get_section: 5,931 רשומות זהות בית-בית לפני ואחרי על אותו קורפוס. - בעקבות היישור docs_get_section אינו מעביר תקרה לאף פארסר (עד עכשיו העביר ל-rst_parser את _ceiling.MAX_SYMBOLS במפורש, כי ברירת המחדל שם הייתה None), ו-"max" בסירוב הוא doc_sections.MAX_SECTIONS, המספר שהפרסר באמת השתמש בו. הייבוא של _ceiling מהמטפל הוסר. - outline_scanners/rst.py ממשיך להעביר את _ceiling.MAX_SYMBOLS במפורש (המספר של המפה, כותרות ותוויות יחד), וטסט חדש מקבע זאת: מוטציה שמוחקת את הארגומנט מפילה אותו. - scripts/measure_md_parse_cost.py: שלוש הצורות רצות עכשיו על ברירת המחדל בשני הפרסרים, כמו הכלי; מתועד בדוקסטרינג, והסקריפט רץ מקצה לקצה. טסטים: 12 טסטים חדשים או שנערכו נופלים על origin/main; הפין של הסורק עובר שם בכוונה. שלוש מוטציות ב-worktree (הסורק בלי הארגומנט, המטפל שמעביר תקרה שוב, require_str שממיר None ל-"") מפילות כל אחת את הטסט שלה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * fix(mirror): get_file_at_commit בודק את גודל האובייקט לפני git show, וקובץ מעל התקרה אינו נטען כלל (#3433, פריט 9) עד היום git show נטען כולו לזיכרון (capture_output=True) ורק אז max_size נבדק, כלומר התקרה הייתה בדיקה בדיעבד ולא חסם על הזיכרון: קובץ של 12MB שנדחה עלה 20MiB שיא בחוט הקריאה לפני שהתשובה הייתה file_too_large. זה השורש של WARN-003 בסקירת #3429, ומה שהניח את "קריאה אחת עולה לכל היותר X" כהערה ולא כתכונה של הקוד. - _object_size: git cat-file -s <sha>:<path> מחזיר את גודל האובייקט מהמאגר בלי לקרוא אותו. שתי הפקודות פונות לאותו sha שנפתר פעם אחת ב-_validate_ref_with_git, ולכן אין חלון בין הבדיקה לקריאה שסנכרון של המראה יכול להיכנס בו (TOCTOU נבחן ונדחה: אובייקט בקומיט נתון אינו משתנה). - כשל בבדיקת הגודל הוא סירוב באותה מפה של git show, לא נפילה לקריאה בלי תקרה; פלט שאינו מספר הוא כשל ולא אפס (U3). מיפוי ה-stderr אוחד ל-_object_read_error, כי git 2.43 מדפיס את אותן הודעות לשתי הפקודות על נתיב חסר ועל קומיט שאינו מכיל אותו (נמדד). - החסם על מה שמוחזר נשאר: לנתיב של תיקייה cat-file -s מודד את אובייקט העץ ואילו git show מדפיס רשימה, ומה שחוזר לעולם אינו גדול מ-max_size. - אותה תשובה ואותו size בסירוב (גודל הבלוב), ואותה תשובה מתחת לתקרה. נמדד (VmHWM, תהליך נקי, מראה bare עם קבצי טקסט): 12MB מול תקרה של 500KB ושל 10MiB — 20.2 ו-20.4MiB שיא לפני, 0.0 אחרי; 7MB מתחת לתקרה — 14.2 לפני ו-14.5 אחרי, כי אותו כן קוראים. טסטים על מראה git אמיתית ב-tmp_path עם מרגל על subprocess.run: ארבעה נופלים על origin/main (הסירוב לפני git show, אותו sha לשתי הפקודות והסדר ביניהן, כשל בבדיקה שאינו נופל לקריאה, גודל שאינו מספר), ושלושה עוברים שם בכוונה כבקרות (מתחת לתקרה, נתיב חסר, החסם על מה שמוחזר); מוטציה שמוחקת את החסם השני מפילה את הפין שלו. התיעוד וההערות שתיארו את "טוענת את ה-blob כולו לפני בדיקת הגודל" עודכנו. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * chore(mcp): ארבעה פריטים קטנים מסקירת #3429 — SUGG-014, SUGG-001, SUGG-004, SUGG-010 (#3433) - SUGG-004: שורת הקיבולת קוראת את ThreadPoolExecutor._max_workers הפרטי דרך _installed_width, וכשהמאפיין ייעלם היא מדפיסה את הרוחב שהתבקש עם "(requested; installed width unreadable)" ו-WARNING שאומר למה — במקום שהשירות לא יעלה בגלל שורת לוג, ובמקום להדהד את הבקשה כאילו היא המצב (state-record-without-state-change). אותו כלל גם ב-WARNING של ה-fallback. - SUGG-010: md_parser.token_count — פונקציה ציבורית ומתועדת שסופרת טוקנים על _MD, המופע שהכלי מריץ — במקום שהסקריפט יקרא ל-_build_parser הפרטית ויבנה פרסר חדש לכל קובץ. מחוץ ל-__all__ בכוונה: הרשימה שם היא החוזה של "שני פארסרים בני-החלפה", וטסט מקבע את ההפרש בינה לבין זו של rst_parser. - SUGG-014: שני טסטים לקובץ memory.max שקיים אבל ריק (IndexError) או לא-מספרי (ValueError) — שניהם נופלים ל-cgroup v1 ומחזירים את הערך שלו. כיסוי השורות היה מלא; התרחיש חסר. מוטציה שמסירה כל אחת מהחריגות מה-except מפילה את הטסט שלה. - SUGG-001: האסרשן שלא יכול היה ליפול הוחלף: os.cpu_count מוצמד ל-16 כדי ש"cpu_count + 4" יהיה 20, מספר שהרצפה לעולם אינה — ומוטציה שמחזירה את ה-fallback ל-min(32, cpu_count + 4) מפילה אותו. על origin/main: שלושה טסטים נופלים (הרוחב הלא-קריא, token_count, הסקריפט דרך הפונקציה הציבורית); שלושת האחרים עוברים שם ומוכחים במוטציות. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * refactor: שלושה כללים שנכתבו פעמיים חזרו למקום אחד — CR בודד, גבול front matter, תווים נסתרים (#3419, #3427) - כלל ה-\r הבודד (סיומת שורה שאינה חד-משמעית) ישב גם ב-mcp_server/outline.py וגם ב-services/md_parser.py, ושני העותקים היו צריכים להסכים לנצח (R6). עכשיו הוא במודול עלה חדש, services/line_endings.py (CR_WITHOUT_LF, find_lone_cr), ושני הצרכנים קוראים לו; הכיוון חד-סטרי (mcp_server מייבא מ-services). טסט מוכיח מול markdown-it-py שהכלל נכון, וטסט מבני מוכיח ששני הצרכנים מייבאים אותו ואף אחד מהם אינו מחזיק עותק. - scripts/generate_ai_map.py::_body_start כתב ביד את גבול ה-front matter, ונמדד ב-#3418 שהוא חולק על התוסף ש-MyST מריץ בחמש מתוך שתים-עשרה צורות. עכשיו הוא קורא את הגבול מהפארסר של הכלי — md_parser.front_matter_end, דרך mdit_py_plugins.front_matter על אותו מופע — והסקריפט מוסיף את שורש הריפו ל-sys.path כדי לרוץ לבדו כמו קודם. אפס-דיף: AI-MAP.md שנוצר זהה בית-בית לזה שבריפו (וגם לפלט הקוד הישן). הטסט שקיבע את הפער בכוונה (test_the_front_matter_rules_still_disagree_as_measured) נערך יחד עם הסגירה, כפי שה-docstring שלו דרש: הטבלה נשארה עם עמודת אמת אחת (התוסף), ושני הקוראים נבדקים מולה; הטסט שהצמיד את המספר "חמש מתוך שתים-עשרה" לפרוזה הוסר יחד עם הפער, והפרוזה (md_parser, requirements/base.txt) מספרת עכשיו את ההיסטוריה. - #3427: known_hex4 (utils) ו-_KNOWN_ESCAPE_HEX4 (CodeNormalizer) — שתי רשימות זהות של 16 קודים שנבדקו לפני בדיקת הקטגוריה, וכל 16 הם Cf, כלומר הרשימה לא הוסיפה דבר. שתיהן נמחקו, ושתי הפונקציות אוחדו ל-strip_hidden_escapes אחת בשכבת הדומיין (טהורה, בלי I/O); utils.normalize_code מייבא אותה במפורש (הייבוא האופציונלי של הדומיין הפך לייבוא רגיל — מסלול ישן שרץ בלעדיו היה עותק שני מחדש). ענף ה-Variation Selectors (Mn, לא Cf) נשאר נפרד ונדלק רק עם remove_variation_selectors=True, כמו קודם. אפס-דיף מדוד: 39 קלטים של רצפי בריחה × 6 צירופי אפשרויות, שני הנרמולים, זהים לפני ואחרי. טסטים: מה שנוגע בשמות החדשים נופל על origin/main (line_endings, front_matter_end, strip_hidden_escapes); טסטי ההתנהגות עוברים שם בכוונה (אפס-דיף) ומוכחים במוטציות — בדיקת הקטגוריה שנמחקת מפילה את 16 טסטי הקודים, ועותק פרטי של כלל ה-CR בפארסר מפיל את הטסט המבני. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * chore(mcp): יתרת ממצאי הסקירה על #3428 — שני סירובים בשמם, טבלאות קפואות, זנב תשובה נפרד, וטסטים שחסרו (#3432) שמונה מאחת-עשרה ההצעות ממומשות, שלוש מוכרעות במפורש: - SUGG-013: path_outside_root — נתיב תקין בצורתו שפותר אל מחוץ לשורש התיעוד (docs/../secrets, /etc/hosts, ../README.md בשורש ריק) נדחה בשמו, עם root בתשובה; missing_path נשאר לנתיב ריק או עם NUL בלבד. - SUGG-011: repo_not_mirrored — מראה שאין למארח (repo_not_found מהשירות) אינה not_found; invalid_commit נשאר not_found (הריפו קיים, ה-ref הוא מה שהקורא יכול לשנות); בזמן sync שניהם sync_in_progress כמו קודם. עובר גם דרך codekeeper_docs_get_section עם repo ו-path. - SUGG-021: _PARSERS ו-DOCS_PATH_POLICY מיוצאים כ-MappingProxyType — הוולידציה בייבוא היא הבטחה רק אם הטבלה אינה משתנה אחריה; _PARSER_TABLE ו- _DOCS_PATH_POLICY_TABLE הם התפר לטסטים (monkeypatch.setitem). - SUGG-020: זנב עיצוב התשובה של docs_get_section נפרד ל-_answer_from_document (ארבע צורות התשובה מתוך מסמך שכבר נפרסר). אפס-דיף: 5,935 רשומות על אותו קורפוס, 5,933 זהות בית-בית; השתיים ששונות הן בדיוק שני הפרובים של SUGG-013 (missing_path ← path_outside_root). - SUGG-009: חמישה טסטים לשתי בדיקות הנרמול ב-_validate_policy_tables (סיומת ברישיות/בלי נקודה, שורש מוחלט/עם לוכסן סוגר/עם ./). - SUGG-010: טסט caplog לשורת ה-WARNING על repo_not_configured. - SUGG-004: טסט ההסכמה בין חיפוש לקריאה אינו דורש עוד returned == allowed — repo_policy מצהיר ש-is_denied נשאר שכבה אחרונה, ושוויון מדויק דרש את ההפך; נשאר "אפס חסומים בתוצאות" + "הקובץ המותר כן חוזר" נגד ריקנות. - SUGG-005: _require_git אחד לשני הטסטים תלויי-git — דילוג בלי הבינארי במקום skip באחד וקריסה על check=True בשני. - SUGG-022 (הכרעה, מתועדת ב-docs/mcp-server.rst): סירוב הוא תשובה של הפרוטוקול ולא תקרית; אף מסלול סירוב בשכבת המטפלים אינו רושם שורה, בכל הקבצים באותה מידה; לוג למה שהמפעיל צריך לדעת, תשובה למה שהקורא צריך; ספירת סירובים — ב-PostHog. - SUGG-012 (נדחה בנימוק): תצוגת התצורה ב-services אינה יכולה לייבא את mcp_server.docs_handlers — הכיוון חד-סטרי ומוצהר בכמה מקומות; התקלה כבר קולנית בזמן ריצה (WARNING לכל קריאה + repo_not_configured). - SUGG-018 (נדחה בנימוק): שני חצאי מדיניות הסודות נשארים בשתי שפות כי אין מנוע אחד לשניהם; מה שמחזיק אותם יחד — רשימת תבניות אחת, טסט אינטגרציה על מראה אמיתית, וטסט ישיר על ארבע הצורות. טסטים: 17 נופלים על origin/main (הקודים החדשים, הפרוקסי, הוולידטור, המראה החסרה), 7 עוברים שם כפינים ומוכחים במוטציות — מחיקת ה-WARNING, טבלאות כ-dict רגיל, ומחיקת בדיקת הסיומת מהוולידטור מפילות כל אחת את הטסט שלה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * fix(mcp): תקרת הגוף מפסיקה לקרוא גוף אנונימי לפני האימות, ויתרת ממצאי סקירת שבעת ה-PRים סגירת הממצאים מסקירת Han על #3434–#3441 (הדוח: code-review-7prs-3434-3441.md ב-CodeKeeper). SEC-001 (רגרסיה של #3431), שלושה חלקים, כל אחד עם טסט שנפל לפני התיקון: - Content-Length תקין (ספרות ASCII, בלי Transfer-Encoding) עובר בלי קריאה — השרת תוחם את הגוף. נמדד על האפליקציה במצב OAuth: 5 קריאות receive() לפני ה-401 → 0. - גוף בלי אורך מוצהר נקרא עד התקרה תחת דדליין של 30 שניות על הלולאה כולה (anyio.fail_after), ואז 408 body_read_timeout עם כמה נקרא. נמדד מול uvicorn אמיתי: לפני — אין תשובה אחרי 35 שניות; אחרי — 408 אחרי 30.005 שניות, Connection: close, והשרת סוגר את החיבור. - רק POST/PUT/PATCH נבדקות (WARN-002): GET /healthz עם Content-Length מזויף מחזיר 200 במקום 413, בשני מצבי האימות. שני הסירובים נושאים Connection: close. טסט סדר-התקנה למצב OAuth לצד זה של מצב PAT, ותיקון המשפט "הפטור מבני" בתיעוד, ב-docstring ובהערה. WARN-001: טסט בתת-תהליך שמייבא את mcp_server.app כמו uvicorn, בשני המצבים, עם ריצת בקרה — שני משתני הסביבה ו-MAX_CODE_SIZE מגיעים לאפליקציה הבנויה. WARN-003: הוחלט — התיעוד אומר שסירובי מכסה אינם נספרים ב-PostHog (ואף סירוב אינו נספר לפי סוג), והעיצוב פתוח באישו #3442. WARN-004: הצורה העוינת במדידת הפרסור רצה עם max_sections=None במפורש. הצעות שנכנסו: SUGG-022 (התקרה נגזרת מ-MAX_CODE_SIZE ב-request_bytes_for, גם בעליית השירות; טסט מצמיד ומודד שקובץ עברי מקסימלי נכנס), SUGG-009 (הסירוב הוא CallToolResult שלם — כלי עם טיפוס החזרה ושם לא מוכר מקבלים rate_limited), SUGG-023 (is None במקום or), SUGG-024 (25 קריאות מקבילות, תקציב 2 — בדיוק 2 עוברות), SUGG-011 (RateLimiter אינו יוצר רשומה לשאלה בלבד; פנקס האזהרות מנוקה), SUGG-019 (token_count ו-front_matter_end עוברים בשער הכניסה של parse_document), SUGG-013, SUGG-002, SUGG-020, SUGG-021, SUGG-005, SUGG-006, SUGG-010, SUGG-001. YAGNI-001 נמחק, YAGNI-002 נשאר ומתועד כהחלטה סגורה. עשר ההצעות שנדחו — אישו #3442. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx * fix(normalize): וריאציה-סלקטור מוסר לפי קוד התו ולא לפי צורת הכתיב — \U0000FE0F כמו \uFE0F (CodeRabbit על #3443) הענף של \UXXXXXXXX ב-strip_hidden_escapes בדק רק את הטווח האידאוגרפי (U+E0100–U+E01EF), ולכן \U0000FE0F נשאר כמות שהוא בזמן ש-\uFE0F הוסר עם remove_variation_selectors=True. עכשיו כלל אחד (_is_hidden) לשתי הצורות: Cf תמיד, ושני טווחי ה-VS רק לפי בקשה. טסט רגרסיה שנפל על הקוד הישן. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx --------- Co-authored-by: Claude <noreply@anthropic.com>
|
נכנס דרך #3443 |
✨ תיאור קצר
path_outside_root(עד היוםmissing_path) ו-repo_not_mirrored(עד היוםnot_found).claude/gracious-einstein-sevk8pנושא את ה-PR הפתוח של docs_get_section: תקרה ל-candidates, ושמירת חוזה Suggestions — לפני שחיבור ה-Markdown נוחת #3426 (fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) #3434), ו-PR נפרד דורש ענף נפרד. ה-PR הזה על ענף נגזר,claude/gracious-einstein-sevk8p-3432, מ-origin/mainהעדכני (1a45c28c).📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
path_outside_root._resolve_docs_pathדחה שתי דחיות שונות באותו קוד: "אין נתיב" (ריק/NUL) ו"הנתיב פותר אל מחוץ לשורש" (docs/../secrets,/etc/hosts,../README.mdבריפו ששורשו ריק). השנייה מקבלת קוד משלה, עםrootבתשובה — שממילא כתוב בתיאור הכלי לכל ריפו, ולכן אינו מגלה דבר;missing_pathנשאר לנתיב ריק. ארבעה טסטים קיימים שקיבעוmissing_pathעל יציאה מהשורש נערכו בכוונה, וטסט חדש מקבע ש-missing_pathנשאר למה שהוא נבנה בשבילו.repo_not_mirrored. ב-RepoBackend.get_file,repo_not_foundמשירות המראה חזר כ-not_found— "אין קובץ כזה, נסה שם אחר" — בזמן שמה שחסר הוא הריפו כולו, עניין של המפעיל. עכשיו קוד משלו;invalid_commitנשארnot_found(הריפו קיים, ה-ref הוא מה שהקורא יכול לשנות); בזמן sync שניהםsync_in_progressכמו קודם.codekeeper_docs_get_sectionמעביר אותו הלאה עםrepoו-path, ו-codekeeper_get_repo_fileכמות שהוא._validate_policy_tablesרץ פעם אחת בייבוא, ושינוי של_PARSERS/DOCS_PATH_POLICYאחריו עקף אותו בשקט. השמות המיוצאים הם עכשיוMappingProxyType(כתיבה ←TypeError), והטבלאות שמתחת —_PARSER_TABLE,_DOCS_PATH_POLICY_TABLE— הן התפר לטסטים (monkeypatch.setitemנראה דרך הפרוקסי מיד).server.pyשקורא.items()לא נגע._answer_from_document. זנב עיצוב התשובה שלdocs_get_section(ארבע הצורות:toc,section_not_found,ambiguous_section,section) נפרד לפונקציה משלו, מתוך מסמך שכבר נפרסר.docs_get_sectionנשאר עם ארבע ההחלטות שיכולות לסרב: ריפו, נתיב, קריאה, פארסר. מוכח כהעברה בלבד:scripts/docs_section_zero_diff.pyעל אותו קורפוס — 5,935 רשומות, 5,933 זהות בית-בית לפני ואחרי, ושתי הרשומות ששונות הן בדיוק שני הפרובים של הסוללה עם נתיב מחוץ לשורש (missing_path←path_outside_root, SUGG-013)../). SUGG-010 — טסטcaplogלשורת ה-WARNING עלrepo_not_configured, שמחיקתה הייתה משאירה הכול ירוק.returned == allowedהוסר.repo_policyמצהיר ש-is_deniedנשאר השכבה האחרונה — תבנית שהמנוע אינו יכול לבטא (או מבטא רחב יותר) עדיין נחסמת בקריאה; שוויון מדויק בין שני החצאים דרש את ההפך. נשארו: "אפס נתיבים חסומים בתוצאות" (תכונת הבטיחות) ו"הקובץ המותר כן חוזר" (נגד ריקנות: בלי זה מנוע שבור היה עובר). SUGG-005 —_require_git()אחד לשני הטסטים תלויי-git, באותו אידיום כמו_GIT = shutil.which("git")בשלושה קובצי טסטים אחרים.docs/mcp-server.rst: לא מוסיפים לוג לסירובים, לא בקובץ אחד ולא בשכבה. סירוב ({"ok": false, "error": ...}) הוא תשובה תקינה של הפרוטוקול — הקורא ביקש משהו שאין או שאסור — ולא תקרית של השרת; 78 מסלולי סירוב בשלושת קובצי המטפלים אינם רושמים דבר, בכל הקבצים באותה מידה. הקו: לוג למה שהמפעיל צריך לדעת (השורה היחידה, על תקלת תצורה), תשובה למה שהקורא צריך לדעת; ספירת סירובים לפי סוג — ב-PostHog שמודד כל קריאה. לוג לכל סירוב היה גם רעש וגם נתיבים שהמשתמש הקליד בלוג.services, ו-servicesאינו מייבא מ-mcp_server— הכיוון חד-סטרי, מוצהר בכמה docstrings, ובדיקת ייבוא מקבעת אותו ל-doc_sections; ייבוא עצל בתוך הבדיקה היה סתירה מתועדת בין קוד לתיעוד. התקלה שהממצא מתאר כבר קולנית בזמן ריצה: WARNING לכל קריאה (עכשיו עם טסט) ו-repo_not_configuredלקורא. הדרך שכן פותחת את זה — העברת טבלת המדיניות למודול עלה ב-services— היא שינוי מבני שראוי לאישו משלו.fnmatchעל רכיבים; ארבע צורות pathspec מול git) נשארים בשתי שפות כי אין מנוע אחד לשניהם — git אינו קורא לפייתון. מה שמחזיק אותם יחד קיים: רשימת תבניות אחת (denylist_patterns), טסט אינטגרציה על מראה אמיתית, וטסט ישיר על ארבע הצורות — וזה בדיוק "התיקון המומלץ כששכפול הכרחי" ב-duplicate-rule-second-copy.pathב-server.pyנוקב בשני הקודים החדשים;docs/mcp-server.rstמסביר אותם לידpath_denied, מוסיף שתי שורות לטבלת פתרון התקלות, ומתעד את ההכרעה על לוגים;docs/whats-new.rst— רשומה תחת 2026-09-21.🧪 בדיקות
origin/main=1a45c28c, עם שלושת קובצי הטסטים): 17 נופלים — הקודים החדשים (4 + 3 + 1 + 1), הפרוקסי, הטבלה כתפר (2), הוולידטור (5), המראה החסרה — ו-7 עוברים שם כפינים (המטפל מעבירrepo_not_mirroredהלאה,missing_pathלנתיב ריק, ה-WARNING, SUGG-004/005).dictרגיל ← טסט SUGG-021 נופל; מחיקת בדיקת הסיומת מהוולידטור ← שני טסטי הסיומת של SUGG-009 נופלים ושלושת טסטי השורש עוברים.test_mcp_docs_handlers,test_mcp_repo_backend,test_mcp_repo_policy,test_mcp_repo_handlers,test_mcp_server_build,test_mcp_outline,test_mcp_additive_params,test_mcp_search_total,test_mcp_primer,test_mcp_to_thread— ירוקים. flake8 מלא (max-line-length=127) על הקבצים שנגעתי בהם — נקי (ב-server.pyנשארE302קיים מ-feat(mcp): docs_get_section קורא גם Markdown, לפי מדיניות נתיבים לכל ריפו #3428, מחוץ לדיף). docutils על שני עמודי התיעוד — 0 אזהרות.repo_not_mirrored— המסלול נבדק דרךRepoBackendעם מראה מדומה שמחזירהrepo_not_found, שזה בדיוק מה ש-get_file_at_commitמחזיר על מראה חסרה (if not mirror_path.exists()).🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/whats-new.rstעודכןAI-MAP.md,docs/mcp-server.rst(הפסקה עלpath_deniedמולnot_found, טבלת פתרון התקלות, "אין שום לוג מהשרת"),docs/doc-authoring.rst,docs/versioning-stable-anchors.rst| המשפט: "ההבדל בין שני הקודים אינו סגנוני:not_foundאומר 'אין קובץ כזה' ומזמין לנסות שם אחר" — הנימוק שממנו נגזרו שני הקודים החדשיםדפוסי באגים שנקראו ומה שקבעו בקוד:
bugbot-rules/blanket-policy-silent-block.md— סירוב נוקב בסיבתו: שני הקודים החדשים במקום קוד שמתחזה ל"נתיב שגוי"/"קובץ חסר";bugbot-rules/silent-fallback-to-worse-path.md—not_foundעל מראה חסרה שלח סוכנים לנסות שמות אחרים, וזו הצורה השקטה שהכלל מתאר;RECURRING-PATTERNS.mdR6 +bugbot-rules/duplicate-rule-second-copy.md— ההכרעה על SUGG-018 היא בדיוק "התיקון המומלץ כששכפול הכרחי", ו-_require_gitהוא אותו אידיום שכבר קיים;bugbot-rules/state-record-without-state-change.md— הוולידציה בייבוא היא רשומה שמתארת מצב;MappingProxyTypeהוא מה שמונע מהמצב לזוז בלי שהרשומה תדע;CRITICAL-PATTERNS.mdK13/K7 — ההכרעה על SUGG-022: לוג לכל סירוב היה מכניס נתיבים שהמשתמש הקליד ללוג;claude-md-snippets/testing.md§5 +TESTING-PATTERNS.mdT2 — ריצה על הקוד הישן ומוטציות;bugbot-rules/widened-exception-scope.md— לא הורחב אףexcept.🧩 השפעות/סיכונים
path_outside_root(במקוםmissing_pathעל יציאה מהשורש) ו-repo_not_mirrored(במקוםnot_foundעל מראה חסרה). סוכן שבדקerror == "missing_path"/"not_found"על המקרים האלה יראה קוד אחר — ובשני המקרים הקוד הישן הטעה אותו.missing_pathעל נתיב ריק,not_foundעל קובץ חסר ועל ref שגוי,sync_in_progress.🔗 קישורים
🧯 סיכון / החזרה לאחור (Rollback)
🤖 Generated with Claude Code
https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
Generated by Claude Code