Repository navigation
Conversation
…וקובץ מעל התקרה אינו נטען כלל (#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
|
ⓘ 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. |
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 42 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
🧯 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 changes get_file_at_commit from read-then-check to check-then-read by probing the Git object size with cat-file before git show, preventing oversized files from entering memory while preserving response semantics, error mappings, and a defensive output-size check. Tests validate command ordering and skipped reads using real bare mirrors, and documentation reflects the improved memory behavior. Sequence diagram for bounded Git file retrievalsequenceDiagram
participant Caller
participant MirrorService
participant Git
Caller->>MirrorService: get_file_at_commit(repo_name, file_path, commit, max_size)
MirrorService->>Git: _validate_ref_with_git(commit)
Git-->>MirrorService: resolved_commit
MirrorService->>Git: _object_size(resolved_commit, safe_file_path)
Git-->>MirrorService: cat-file size or error
alt size probe fails
MirrorService-->>Caller: file_not_in_commit or git_error
else file_size > max_size
MirrorService-->>Caller: file_too_large with size
else file_size <= max_size
MirrorService->>Git: git show resolved_commit:safe_file_path
Git-->>MirrorService: raw content
MirrorService-->>Caller: content or file_too_large
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesהשינוי מעביר את בדיקת גודל האובייקט לפני קריאת קבצים מוגבלת בגודל
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 68.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 4 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
נכנס דרך #3443 |
✨ תיאור קצר
get_file_at_commitgit showנטען כולו לזיכרון (capture_output=True) ורק אזmax_sizeנבדק — כלומר התקרה הייתה בדיקה בדיעבד ולא חסם על הזיכרון. קובץ של 12MB שנדחה עלה 20MiB שיא בחוט הקריאה לפני שהתשובה הייתהfile_too_large. מעכשיוgit cat-file -s <sha>:<path>שואל את מאגר האובייקטים לגודל לפניgit show, וקובץ מעל התקרה נדחה לפני שנקרא בית אחד: אותה תשובה, אותוsize, אפס זיכרון. "קריאה אחת עולה לכל היותר X" הופך מהערה לתכונה של הקוד.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-3433, מ-origin/mainהעדכני (1a45c28c). המנה השנייה של מעקב אחרי סקירת PR #3429 (מאגר הקריאות של ה-MCP): שמונה הצעות שלא תוקנו ב-PR, וחסימת קריאת המראה במקור #3433 (SUGG-014, SUGG-001, SUGG-004, SUGG-010) תבוא ב-PR נפרד, לפי סדר העבודה.📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
_object_size(חדש):git cat-file -s <sha>:<path>מחזיר את גודל האובייקט מהמאגר בלי לקרוא אותו. שתי הפקודות פונות לאותוshaשנפתר פעם אחת ב-_validate_ref_with_git— לא ל-HEAD— ולכן אין חלון בין הבדיקה לקריאה שסנכרון של המראה יכול להיכנס בו: אובייקט בקומיט נתון אינו משתנה. TOCTOU נבחן ונדחה מהסיבה הזאת, וטסט מקבע ששתי הפקודות נושאות את אותוshaושהבדיקה קודמת לקריאה.cat-fileל-showנבחרcat-file(האישו הציע גם קריאה בזרם עם תקרת בתים): הוא נותן את הגודל המדויק לתשובתfile_too_large— כמו היום — במקום "יותר מהתקרה", אינו דורש הרג של תת-תהליך באמצע זרם, ומחירו תהליך git אחד קצר לכל קריאה במסלול שהוא אדמין (lines=/outline) או 500KB לכל היותר (docs_get_section).silent-fallback-to-worse-path):stderrממופה באותה מפה שלgit show, ופלט שאינו מספר הואgit_errorולא אפס (U3: פלט של תת-תהליך הוא קלט חיצוני, ו"אפס" היה עובר את התקרה בשקט). המיפוי אוחד ל-_object_read_error— הגדרה אחת לשתי הפקודות (R6) — אחרי שנמדד ש-git 2.43 מדפיס את אותן הודעות לשתיהן:path 'x' does not exist in '<sha>'על נתיב חסר, ו-exists on disk, but not inעל קומיט שאינו מכיל אותו.cat-file -sמודד את אובייקט העץ ואילוgit showמדפיס רשימה מעוצבת. מה שחוזר מהפונקציה לעולם אינו גדול מ-max_size, יהיה סוג האובייקט אשר יהיה; טסט מקבע זאת עם מאגר "שמשקר", ומוטציה שמוחקת את הבדיקה מפילה אותו.size,lines,resolved_commitזהים (בקרה); מעל התקרה — אותו dict עםsizeשל הבלוב; נתיב חסר —file_not_in_commit.repo_backendוהוובאפ (repo_browser.py) צורכים את אותם שדות ולא נגעתי בהם.max_size, ההערה לידRANGE_READ_MAX_BYTESב-repo_backend.py, ההערה ליד_PARSE_COST_BYTESב-server.py(שהפנתה ל-מעקב אחרי סקירת PR #3429 (מאגר הקריאות של ה-MCP): שמונה הצעות שלא תוקנו ב-PR, וחסימת קריאת המראה במקור #3433 כפתוח), הפסקה ב-"מודל הריצה של הכלים" ב-docs/mcp-server.rst, ו-docs/whats-new.rst. ההחלטה מ-fix(mcp): מאגר הקריאות נגזר ממכסת הזיכרון של הקונטיינר, לא ממעבדי המארח (#3391) #3429 שהמחלק נשאר מתומחר לפי הפרסור עומדת — ההחזקה של קובץ מתחת לתקרה (7MB ← 37MiB) לא השתנתה, ומה שנעלם הוא ההחזקה של קובץ שנדחה.get_file_content(הקורא השני, דרך_run_git_commandעםtext=True) עדיין קורא בלי שום תקרה. הקוראים שלו:repo_sync_service(אינדוקס),webapp/app.pyו-repo_browser.py. פריט 9 מדבר עלget_file_at_commit, והוספת תקרה ל-get_file_contentהיא שינוי התנהגות לקוראים שאין להםmax_sizeבכלל — ראוי לאישו משלו.🧪 בדיקות
tests/test_git_mirror_service.py(7 טסטים חדשים) על מראה git אמיתית שנבנית ב-tmp_path(bare clone, כמו בייצור) עם מרגל עלsubprocess.runשרושם אילו פקודות git רצו — כי התכונה הנבדקת היא מה לא נקרא, ואת זה רואים רק ברשימת הפקודות: קובץ מעל התקרה נדחה מ-cat-fileו-git showאינו רץ; מתחת לתקרה נקרא בדיוק כמו קודם (בקרה); נתיב חסר ממופה ל-file_not_in_commitדרך הבדיקה; שתי הפקודות נושאות את אותוshaשנפתר, והבדיקה קודמת לקריאה; כשל בבדיקה אינו נופל לקריאה; גודל שאינו מספר הוא כשל; ומה שמוחזר נבדק מול התקרה גם כשהמאגר דיווח גודל קטן.origin/main=1a45c28c): ארבעה נופלים (הסירוב לפניgit show, אותוshaוהסדר, כשל בבדיקה, גודל שאינו מספר), ושלושה עוברים שם בכוונה — שתי הבקרות והפין של החסם השני.VmHWMבתהליך נקי, מראה bare עם קבצי JS טקסטואליים):file_too_largefile_too_large, אותוsizefile_too_largefile_too_large, אותוsizetest_git_mirror_service,test_git_mirror_last_commit,test_git_history,test_repo_browser_multi,test_mcp_repo_backend,test_mcp_additive_params,test_mcp_outline,test_mcp_docs_handlers,test_mcp_search_total,test_mcp_search_pattern_mode,test_mcp_repo_policy,test_mcp_server_build,test_mcp_to_thread— ירוקים. flake8 עם ה-selection של CI (E9,F63,F7,F82) — 0. (ב-git_mirror_service.pyישF841/W391קיימים מלפני, מחוץ לדיף; ב-server.pyE302מ-feat(mcp): docs_get_section קורא גם Markdown, לפי מדיניות נתיבים לכל ריפו #3428 שכבר מתוקן בשני PRים פתוחים.) docutils על שני עמודי התיעוד — 0 אזהרות.md_preview.bundle.js.mapשל 11.7MB) — הקומפוזיציה זהה למדידה כאן (bare mirror,git cat-file -sואזgit show), ו-git בייצור הוא כזה שמדפיס את אותן הודעות (הטסט על נתיב חסר מקבע את המיפוי דרך git אמיתי).🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/whats-new.rstעודכןAI-MAP.md,docs/mcp-server.rst("מודל הריצה של הכלים", טבלת הגבולות),docs/doc-authoring.rst,docs/versioning-stable-anchors.rst| המשפט: "והמראה טוענת את ה-blob כולו לזיכרון לפני בדיקת הגודל" — המשפט שהשתנהדפוסי באגים שנקראו ומה שקבעו בקוד:
CRITICAL-PATTERNS.mdK11 +bugbot-rules/return-value-failure-unchecked.md—subprocess.runמחזירreturncodeואינו זורק: הוא נבדק בשני המקומות, ו-_object_sizeמחזיר ערך כשל שהקורא בודק ("error" in probe);bugbot-rules/silent-fallback-to-worse-path.md— כשל בבדיקה אינו נופל ל-git showבלי תקרה;CORE-PATTERNS.mdU1 +bugbot-rules/race-toctou.md— check-then-act על<sha>:<path>שאינו משתנה, לכן לא ממצא, וזה מקובע בטסט על אותוsha;CORE-PATTERNS.mdU3 +bugbot-rules/external-input-isinstance.md— פלטcat-fileמפוענח תחתexcept ValueErrorצר, ופלט פגום הוא כשל;bugbot-rules/silent-truncation-at-sink.md— התקרה דוחה ואינה חותכת, בשני החסמים;bugbot-rules/work-disproportionate-to-answer.md(R8) — הממצא עצמו: 20MiB שנקראו בשביל תשובה של "גדול מדי";CRITICAL-PATTERNS.mdK13 — ה-stderrעובר_sanitize_outputלפני שהוא נכנס לתשובה או ללוג, כמו קודם;RECURRING-PATTERNS.mdR6 — מיפוי השגיאות אוחד לפונקציה אחת במקום להיכתב פעם שנייה;bugbot-rules/state-record-without-state-change.md— ההערות והתיעוד שתיארו "נטען לפני הבדיקה" עודכנו יחד עם הקוד;claude-md-snippets/testing.md§5 +TESTING-PATTERNS.mdT2 — ריצה על הקוד הישן ומוטציה;bugbot-rules/widened-exception-scope.md— לא הורחב אףexcept.🧩 השפעות/סיכונים
git cat-file -sנוסף לפני כלgit showב-get_file_at_commit— כמה מילישניות על מראה חמה; בתמורה, קובץ מעל התקרה אינו נטען עוד לזיכרון של חוט הקריאה (20MiB לקובץ של 12MB, נמדד). התשובות ללקוחות זהות.get_file_content,max_sizeשל הקוראים,RANGE_READ_MAX_BYTES, ותמחור מאגר הקריאות.🔗 קישורים
docs/mcp-server.rst— "מודל הריצה של הכלים"🧯 סיכון / החזרה לאחור (Rollback)
🤖 Generated with Claude Code
https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
Generated by Claude Code
Summary by Sourcery
Enforce file-size limits before reading Git objects while preserving existing responses and safeguards.
Bug Fixes:
Enhancements:
Documentation:
Tests: