fix(bot,webapp): שמירת סקילים + Config Inspector + נוסח ZIP חדש ו"קבצי גיבוי" - #3199
Conversation
שלושה כלים חדשים מעל קולקציית sticky_notes הקיימת של הוובאפ: - codekeeper_list_notes — פתקי הקובץ (קריאה טהורה, בלי ה-backfill של ה-GET בוובאפ) - codekeeper_create_note — פתק חדש על קובץ קיים; line מעגן לשורת מקור, בלעדיו הפתק נוצר צף עם sentinel __floating__ מפורש (אחרת ה-JS מעגן אוטומטית לשורה הקרובה ודורס את הכוונה) - codekeeper_update_note — עדכון חלקי לפי note_id, עם annotation נפרד (destructive+idempotent) כי פתק נדרס במקום ואין לו היסטוריית גרסאות עקרונות: - כותבים בדיוק את סכמת הוובאפ (scope_id מ-sticky_notes_scope.make_scope_id הקנוני, ברירות מחדל בפריטת הקליינט) — פתק מה-MCP מופיע מיד ב-UI - זהות תמיד מהטוקן; יצירה/עדכון מאחורי require_write - תוכן >5000 תווים נדחה בשגיאה (לא קיטום שקט — סוכן לא ישים לב לאובדן) - מגן אנטי-לולאה: עד 200 פתקים לקובץ ביצירה - אינדקס (user_id, scope_id) שחסר היום נוצר lazy/best-effort — משרת גם את שאילתת ה-scope הזהה של הוובאפ - בלי מחיקה ובלי תזכורות (non-goal מתועד); אפס שינויי קוד בוובאפ — דיפלוי לשירות ה-MCP בלבד בדיקות: 19 טסטים הרמטיים חדשים (סניטציה, ולידציות, sentinel, צבעים, scope filter מול make_scope_id האמיתי, סריאליזציה) + עדכון טסט הרישום. 188 טסטי MCP עוברים; black/flake8/doc8 נקיים; Sphinx נבנה עם 0 אזהרות. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
הוספת פריט "תיאור" בתפריט 3-הנקודות בעמוד הצפייה בקובץ (view_file), ראשון ברשימה, שפותח מודאל קטן לעריכת שדה התיאור — חוסך את הכניסה לעריכת קובץ מלאה רק כדי לשנות תיאור. "נעץ לדשבורד" עלה לשני, "שתף" לשלישי. - משתמש ב-endpoint הקיים POST /api/file/<id>/quick-update שמעדכן את התיאור in-place (מטא-דאטה, בלי גרסה חדשה) — אין קוד שרת חדש - מודאל בדפוס מודאל השיתוף הקיים (Escape, לחיצה על הרקע, טוסט הצלחה), עם textarea ומונה תווים (עד 500, תואם למגבלת ה-endpoint) - עדכון חי של התצוגה מתחת לשם ושל תווית התפריט בלי רענון עמוד - זמין לכל הקבצים (התיאור הוא מטא-דאטה אוניברסלי) - אין בעיית מודאל-בתוך-מודאל: התפריט (dropdown) נסגר לפני שהמודאל נפתח, בדיוק כמו "שתף קובץ" הקיים - שינוי template בלבד; דורש דיפלוי לוובאפ בדיקות: Jinja parse תקין, תחביר JS תקין; אין קוד Python שהשתנה. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
באג: אחרי deploy של תבנית, משתמשים שצפו בקובץ לאחרונה המשיכו לראות את
הגרסה הישנה (בלי אלמנטים חדשים) הרבה זמן; אחרים כן ראו חדש. השורש:
ה-ETag של /file/<id> ו-/md/<id> חושב מנתוני הקובץ בלבד (updated_at/תוכן/
version/theme) בלי גרסת ה-deploy — אז קובץ שלא נערך החזיר ETag זהה בין
deploys → הדפדפן קיבל 304 והציג HTML ישן מה-cache. תורם שני: מסלול
If-Modified-Since מבוסס updated_at החזיר 304 בנפרד.
תיקון שורשי (webapp/app.py + cache_manager.py):
- _compute_file_etag כולל עכשיו את _STATIC_VERSION (גרסת deploy) — כל
deploy מבטל ETags ישנים. קורא מרכזי אחד ⇒ מכסה view_file וגם md_preview.
- שלושת מסלולי If-Modified-Since מכבדים RFC 7232 §3.3: מדלגים כשקיים
If-None-Match (אחרת 304 מיושן גם אחרי שה-ETag השתנה).
- מפתח ה-cache צד-שרת של md_preview כולל את גרסת ה-deploy (אחרת HTML
מרונדר ישן מוגש עד 30 דק').
- נלווה: invalidate_file_related מבטל עכשיו גם את המפתח האמיתי
web:md_preview:user:*:{file_id}:* (היה prefix שגוי — עריכת קובץ לא
ביטלה את cache ה-md שלו).
ב-Render גרסת ה-deploy מגיעה מ-RENDER_GIT_COMMIT (משתנה לכל commit).
אימות: py_compile + flake8 + 9 טסטי cache/invalidation עוברים. את לוגיקת
ה-ETag אימתתי בבידוד (הרצת הפונקציה האמיתית: גרסת deploy משנה את ה-ETag,
יציב לאותו קלט, ותוכן עדיין משנה). טסט יחידה שמייבא webapp.app לא ישים —
טסטי ה-webapp מדולגים ב-CI ("צינור הבוט") ו-flask לא זמין בסביבה.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מתחת לכל קובץ באוסף שיש לו תיאור מופיע אייקון ℹ️; לחיצה פותחת מודאל קטן (קריאה בלבד) עם התיאור — בלי להיכנס לקובץ. - Backend: get_collection_items מצרף עכשיו את ה-description של הקובץ לכל פריט, דרך אותו batch שכבר מחשב is_file_active (הרחבת ה-projection ל-file_name+description) — בלי N+1 ובלי שדות כבדים (Smart Projection נשמר; description ≤500 תווים). קובץ בלי תיאור/לא-פעיל ⇒ "". - Frontend: אייקון ℹ️ ב-.collection-card__meta רק אם יש תיאור; openDescriptionModal בדפוס .collection-modal הקיים (Escape/רקע סוגרים, textContent — בטוח מ-XSS). - מצב workspace לא נכלל בשלב זה. בדיקות: 3 טסטי enrichment חדשים (fakes שתומכים ב-projection) + 49 טסטי collections_manager עוברים; node --check ל-JS; py_compile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
אייקון התיאור בכרטיס קובץ עבר משורת ה-meta אל צד שם הקובץ (משמאל, RTL) באותה שורה, עם רווח. כשהשם ארוך ונשבר לשתי שורות — האייקון "קופץ" לשורת 4 הכפתורים, משמאל להם (פונקציה layoutDescIcons שמנצלת את זיהוי is-wrapped של autoFitText, בסדר reset→autoFit→move כדי למנוע oscillation). ארכיון לאוספים: שדה is_archived חדש (נפרד מ-is_active), toggle 🗄️ "הצג ארכיון" בסיידבר, וכפתור ארכב/שחזר בכותרת האוסף (מגודר ל-non-workspace). list_collections קיבל archived_only ו-include_archived; ברירת המחדל מחריגה מאורכבים (ne:True מכסה גם אוספים ישנים ללא השדה). הקאש כבר מבחין לפי querystring, וה-PUT מנקה את שתי התצוגות. הגיבוי האישי משתמש ב-include_archived=True כדי לא לפספס אוספים בארכיון. - database/collections_manager.py: doc-build, allow-list, list filter, serializer, index+backfill - webapp/collections_api.py: פרמטר archived ב-GET, דילוג על "שולחן עבודה" בתצוגת ארכיון - webapp/static/js/collections.js + collections.css: אייקון, toggle, כפתורי ארכב/שחזר - services/personal_backup_service.py: include_archived=True בגיבוי ובבדיקת כפילות - tests/test_collections_archive.py: ארכוב/שחזור/include/legacy Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
main קלט את אייקון-התיאור הבסיסי דרך squash (#3191), ולכן נוצר קונפליקט מול העבודה החדשה בענף (הזזת האייקון + ארכיון). מיזגתי את main לענף ופתרתי: - collections.css: נשמרה הגרסה שלי (superset — בסיס .desc-info + flex + Slot 2 + toggle ארכיון). - collections.js: הוחזר לגרסה שלי כדי למנוע שכפול של כפתור התיאור שהמיזוג האוטומטי יצר. תוצאת המיזוג זהה בדיוק לעבודה שכבר נבדקה (36 טסטים ירוקים). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
לפי theming_and_css.rst אסורים צבעים קשיחים בקבצי רכיבים — רק var(--token). שתי חריגות הומרו: - מודאל "ערוך תיאור" (view_file.html): הרקע הכהה הקבוע (#1f2a44, #fff, rgba לבנים, focus בצבע primary קשיח) הוחלף בטוקנים סמנטיים — bg-secondary/tertiary, text-primary/secondary/muted, glass-border, primary; ה-scrim וה-shadow קיבלו טוקן-רכיב עם fallback (var(--modal-backdrop, ...), var(--solid-surface-shadow, ...)). כך המודאל מקבל את צבעי הערכה גם בערכות בהירות (rose-pine-dawn, classic). - כפתור "הצג ארכיון" במצב לחוץ (collections.css): rgba לבנים קשיחים הוחלפו בטוקני glass קיימים (glass-hover/glass-border/glass). מודאל ה-ℹ️ באוספים נבדק ונמצא תקין (יורש var(--collections-modal-*)) — ללא שינוי. מודאל השיתוף הסמוך הוא legacy קיים ולא נכלל (חוב נפרד). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…קט collections.css main קלט את הארכיון והזזת האייקון דרך squash (#3192), והענף ממשיך עם תיקון הטוקנים (991b1ad) שנגע באותה שורה. נשמרה גרסת הטוקנים של #toggleArchivedBtn (var(--glass-*)) — ההבדל היחיד מול main, שהוחלף בכוונה. תוצאת המיזוג זהה לחלוטין לעץ שכבר נבדק. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
באג שורש: המחרוזת `סה""כ` פיצלה f-strings לשתי מחרוזות סמוכות — השנייה (בלי
קידומת f) הציגה placeholders כטקסט מילולי (למשל "{len(items)}"). תוקן ב-3 מקומות
(handlers/documents.py, conversation_handlers.py, handlers/save_flow.py) ע"י
מעבר ל-f-string רציף אחד עם מרכאות בודדות. כך "✅ נוסף: X (סה"כ N קבצים)" מציג
את המספר בפועל, וכן כותרת רשימת ה-ZIP השמורים ומסך איסוף הקוד הארוך.
פיצ'ר: שלב בחירת שם ל-ZIP. אחרי "✅ סיום" הבוט מבקש שם (או "⏭️ דלג" לשם אוטומטי):
- conversation_handlers.py: helper משותף finalize_zip_create + _cleanup_zip_state;
zip_create_finish מציב awaiting_zip_name ומבקש שם; callback חדש zip_create_skip_name.
- main.py: hook בראש handle_text_message שתופס את השם (נבדק ראשון כדי שלא ייבלע/ייחשב קוד).
- שם מנוקה דרך TextUtils.clean_filename + סיומת .zip; fallback ל-my-files-<timestamp>.zip.
טסט: חיזוק test_handle_document_collects_zip_items לאימות שהמספר מוצג בפועל (הגנת רגרסיה).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…hread, מסלול ביטול מענה ל-review findings ב-PR #3194: - ניקוי שם רשומת ZIP (utils.safe_zip_entry_name): basename בלבד, דחיית נתיב מוחלט/ מקונן ו-"."/".." — הגנת Zip-Slip. משמש בבניית הארכיון במקום השם הגולמי. - בניית ה-ZIP חולצה ל-utils.build_zip_bytes (טהור) ורצה תחת asyncio.to_thread כדי לא לחסום את לולאת האירועים (בהתאם לכלל ה-Performance ב-CLAUDE.md). - אכיפת מגבלות איסוף (ZIP_CREATE_MAX_FILES=50, ZIP_CREATE_MAX_TOTAL_BYTES=45MB) בזמן צבירת הקבצים ב-handlers/documents.py, עם הגנה כפולה גם בשלב הבנייה. - מסלול ביטול במצב "המתנה לשם": zip_create_cancel עובר דרך _cleanup_zip_state (מנקה גם awaiting_zip_name), ונוסף כפתור "❌ ביטול" למקלדת בקשת השם — כך שמשתמש שמתחרט לא ישלח ארכיון בטעות בהודעת טקסט כלשהי. - טסטים: tests/test_zip_bundle_utils.py (ניקוי שמות + מגבלות, נבדק ע"י פענוח ה-ZIP). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…ון docstring מענה ל-review (סבב 2) ב-PR #3194: - handlers/documents.py: בדיקות מגבלת ה-ZIP (מספר קבצים + גודל לפי document.file_size) הוזזו לפני get_file()/download_to_memory() — לא מורידים לזיכרון קובץ שנדחה מראש. בדיקת len(raw) נשמרה כאימות סופי לפער אפשרי מול הגודל המוצהר. - utils.build_zip_bytes: מניעת שמות רשומה כפולים — הראשון נשמר, הבאים מקבלים סיומת ממספרת (x.txt, x_2.txt) עם שמירת הסיומת. - utils: תיקון docstring (שורה ריקה אחרי רשימת ה-bullets) — מבטל אזהרת docutils/RTD. - tests: חיזוק test_...skips (file_1/file_2 מדויק) + regression לכפילות שמות. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…ך-יתר של URL שלושה תיקונים שורשיים ב-Config Inspector (שרץ בתהליך ה-webapp וקורא os.getenv): 1) הפרדה לפי שירות: הצלבנו את כל 236 המשתנים מול עמודת "רכיב" ב- docs/environment-variables.rst וגם מול הקוד עצמו (import-closure סטטי של webapp/bot/mcp + חיפוש הקוראים של כל KEY). 41 משתנים שנקראים רק בבוט (34, כולל ה-webserver הפנימי שרץ בתוך תהליך הבוט), ב-MCP (6) או בסקריפטים (1) הוסטו מהעמוד הראשי ל"עמוד 2" חדש (טאב "שירותים אחרים") שמציג מטא-דאטה בלבד — בלי Status ובלי Active Value, כי ערכיהם חיים בתהליכים אחרים ואינם נגישים מה-webapp. שדה service חדש ב-ConfigDefinition + get_other_services_entries(). OTEL_EXPORTER_* נשארו בעמוד webapp (ה-SDK קורא אותם מה-env בכל תהליך); BOT_USERNAME נשאר webapp ו-BOT_TOKEN נוסף כהגדרה חסרה (נקרא ב-auth_routes). 2) סטטוס "Set" חדש: ערך שהוגדר בסביבה (למשל ברנדר) כשאין ברירת מחדל בקוד אינו "Modified" — אין דיפולט שממנו סטינו. determine_status מחזיר Set במקרה זה (MCP_SERVER_URL, GITHUB_TOKENS, GITHUB_WEBHOOK_SECRET, ALERTMANAGER_WEBHOOK_SECRET, ALERT_TELEGRAM_BOT_TOKEN ודומיהם). נוספו set_count לסקירה, ספירה בכרטיסי הקטגוריות ו-pill כחול (טוקן --info) ב-UI. 3) ביטול מיסוך-יתר: "URL" הוסר מ-SENSITIVE_PATTERNS — כתובת ציבורית (MCP_SERVER_URL, WEBAPP_URL, PROMETHEUS_URL, PUBLIC_BASE_URL...) אינה סוד. URL שמגלם credentials (MONGODB_URL) נשאר ממוסך דרך sensitive=True מפורש, ו-TOKEN/SECRET/URI/KEY ממשיכים להיתפס בתבניות. בנוסף: תוקנו 5 שורות "רכיב" שגויות ב-docs/environment-variables.rst שהתגלו בהצלבה (ENABLE_INTERNAL_SHARE_WEB, SENTRY_WEBHOOK_SECRET, SENTRY_WEBHOOK_DEDUP_WINDOW_SECONDS, DUMMY_BOT_TOKEN → Bot; BOT_TOKEN → Bot/WebApp). טסטים: 32 ב-test_config_inspector_service.py (כולל 3 מחלקות חדשות: סטטוס Set, מיסוך URL, הפרדת שירותים) — ירוקים. תחביר Jinja אומת. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…פים בעמוד 2, ותיקון העתק-הכל שלושה תיקונים בהמשך למשוב: - ה-webserver הוא שירות Render נפרד (ההרצה הפנימית בתוך תהליך הבוט בוטלה): ה-closure שלו חושב בנפרד מהבוט, ומשתנים שנקראים בו (SENTRY_WEBHOOK_SECRET, SENTRY_WEBHOOK_DEDUP_WINDOW_SECONDS) מסומנים service=webserver. עודכן גם "רכיב" ב-environment-variables.rst (Webserver) ותואר ENABLE_INTERNAL_SHARE_WEB. - משתנים משותפים: השדה service הוחלף ב-services (tuple) — משתנה יכול להשתייך לכמה שירותים. עמוד 2 מציג עכשיו את כל 220 המשתנים ששייכים לשירות שאינו webapp, כולל המשותפים (למשל MONGODB_URL: bot + mcp + webserver), עם ציון השירותים בכל שורה ותג "גם Webapp" למשתנים שערכיהם מוצגים בעמוד הראשון. עמוד 1 נשאר 196. - תיקון P2 מה-review: "העתק הכל" הוגבל ל-#inspectorPageWebapp — טבלת השירותים האחרים (מטא-דאטה בלי ערכים) לא מייצרת יותר שורות KEY= ריקות בייצוא ה-.env. טסטים: 33 ב-test_config_inspector_service.py (עודכנו לסמנטיקת services + טסט משתנה-משותף-בשני-העמודים) — ירוקים. תחביר Jinja אומת. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
סקילים (ZIP עם SKILL.md) נכנסו בטעות למסלול הגיבויים, מה שגרם לשלוש בעיות: save_backup_bytes דוחס מחדש ומזריק metadata.json (לא byte-for-byte), cleanup_expired_backups מוחק לפי retention, ו-restore עם purge הרסני. הפתרון — אחסון עצמאי לחלוטין: - SkillManager חדש (file_manager.py): קולקציית GridFS "skills" נפרדת, שמירת bytes as-is (fs.put ישיר), תמיד מונגו בלי תלות ב-BACKUPS_STORAGE ובלי env var. skill_id ייחודי (timestamp+uuid) מונע התנגשות; שם קובץ עם סיומת ייחוד מונע דריסה. - ניתוב בהעלאה: _maybe_store_zip_copy מציג שני כפתורים "סקיל"/"גיבוי" במקום שמירה אוטומטית; ה-bytes נשמרים זמנית עד לבחירה מפורשת (עזרי stash ב-utils.py, מחיקות מוגבלות ל-allowlist ייעודי). - SkillMenuHandler חדש (prefix skill_): רשימה + הורדה/מחיקה/תיוג/הערה, כפתור "📝 סקילים" בתפריט "הצג את כל הקבצים שלי". תיוג/הערה דרך ה-facade הגנרי הקיים. טסטים: שמירה+הורדה byte-for-byte, בידוד מ-cleanup של הגיבויים, ושני סקילים עם אותו שם שאינם דורסים. טסטי הגיבויים הקיימים נשארים ירוקים. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
תיקון כשלי CI/RTD ויישום ממצאי code review על פיצ'ר הסקילים. CI/RTD: - עדכון test_documents להתנהגות "בחירה מפורשת" (כפתורים) במקום שמירה אוטומטית - עדכון סדר תפריט "הצג את כל הקבצים" בשני טסטי patch_coverage (skill_list) - תיקון docstring שהכשיל את RTD (הסרת * חשוף שנפרש כ-emphasis) אבטחה/נכונות: - main: logger.exception בשמירת הערות, בלי חשיפת שגיאת DB למשתמש - file_manager: נרמול user_id ל-int לפני שמירת סקיל (עקביות שאילתת list_skills) - config_inspector: מיסוך URL עם credentials מוטמעים (user:pass@) גם בשם לא-רגיש - utils: אימות token בטוח לשם קובץ + הרשאות 0o700 לתיקיית ה-pending - conversation: ניקוי ה-pending רק אחרי שמירה מוצלחת (מאפשר retry בכשל) ביצועים/עקביות: - skill_menu: איסוף דירוגים ב-thread, הורדה בסריקת GridFS אחת, safe_edit_message_text - conversation: load ל-to_thread + safe_edit על תשובות ה-routing - documents: קבוע PENDING_ZIP_TTL_SECONDS + הגבלת ZIP ממתינים למשתמש Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
הבאג בפרודקשן ("save_skill_bytes: GridFS 'skills' לא זמין"): ב-_get_skills_gridfs
נעשתה בדיקה ``if not mongo_db``, אבל get_mongo_db() מחזיר אובייקט Database אמיתי של
pymongo — ו-bool()/not עליו זורק NotImplementedError. החריגה נבלעה ב-except הכללי,
המתודה החזירה None, והסקיל לא נשמר. חיבור המונגו עצמו תקין (קטעי קוד כן נשמרים).
השורש: pymongo אוסר truth-value testing על Database/Collection/MongoClient. שאר
מתודות ה-facade כבר משוות נכון עם ``is None`` — רק שני עוזרי ה-GridFS השתמשו ב-not.
התיקון (root-cause, שני המקומות):
- SkillManager._get_skills_gridfs: ``if not mongo_db`` → ``if mongo_db is None``.
- BackupManager._get_gridfs: אותו באג רדום (מוסתר בפרודקשן ע"י BACKUPS_STORAGE=fs,
שמחזיר None עוד קודם) — תוקן גם הוא כדי שלא יתפוצץ במצב BACKUPS_STORAGE=mongo.
טסטים: הטסטים הקיימים מוקים את _get_skills_gridfs ולכן פספסו את המסלול האמיתי.
נוספו טסטי רגרסיה שקוראים למתודה האמיתית עם Database אמיתי של pymongo (connect=False,
בלי שרת) ומוודאים שמוחזר GridFS ולא None. אומת red→green. 11 טסטים ירוקים.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughה-PR מוסיף אחסון וניהול סקילים ב-GridFS, זרימת בחירה לשמירת ZIP כסקיל או כגיבוי, ותפריט סקילים בבוט. בנוסף, תצורת השירותים וממשק בדיקת משתני הסביבה הורחבו עם סטטוסי Set ופיצול לפי שירות. Changesניהול ZIP וסקילים
בדיקת תצורה
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
file_manager.py (4)
1279-1292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueאפשר לקצר עם
@dataclass.
SkillInfoהוא מחזיק-נתונים טהור;@dataclassיחסוך את ה-__init__הידני וייתן__repr__/__eq__בחינם (שימושי בטסטים).♻️ הצעה
+@dataclass class SkillInfo: """מידע על סקיל שמור (ארכיון קוד לטווח ארוך, נפרד לגמרי מגיבויים)""" - def __init__(self, skill_id: str, user_id: int, created_at: datetime, file_count: int, - total_size: int, original_name: str, file_name: str, - metadata: Optional[Dict[str, Any]]): - self.skill_id = skill_id - self.user_id = user_id - self.created_at = created_at - self.file_count = file_count - self.total_size = total_size - self.original_name = original_name # השם המקורי המלא (לתצוגה) - self.file_name = file_name # שם ה-GridFS הייחודי בפועל - self.metadata = metadata + skill_id: str + user_id: int + created_at: datetime + file_count: int + total_size: int + original_name: str # השם המקורי המלא (לתצוגה) + file_name: str # שם ה-GridFS הייחודי בפועל + metadata: Optional[Dict[str, Any]] = None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@file_manager.py` around lines 1279 - 1292, המירו את מחלקת הנתונים SkillInfo לשימוש ב-@dataclass, הגדירו את אותם שדות עם הטיפוסים הקיימים ובאותו סדר, והסירו את __init__ הידני תוך שמירה על ההתנהגות והממשק הנוכחיים.
1444-1449: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueאפשר לצמצם את הסריקה ב-
delete_skillsלשאילתה ממוקדת.כרגע נסרקים כל הסקילים של המשתמש ומסוננים בפייתון. שאילתה עם
{"metadata.user_id": user_id, "metadata.skill_id": {"$in": list(wanted)}}תעשה זאת ב-Mongo. שימו לב שזה ידרוש הרחבה קטנה של_FakeGridFS._matchב-tests/test_skill_manager.pyלתמיכה ב-$in.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@file_manager.py` around lines 1444 - 1449, Update delete_skills to query only the requested skills by adding the metadata.skill_id $in filter alongside metadata.user_id in fs.find, using list(wanted), and remove the redundant Python-side skill membership check. Extend _FakeGridFS._match in tests/test_skill_manager.py to support $in comparisons.
1383-1383: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winנרמלו
user_idל-intגם בקריאה/מחיקה.
save_skill_bytesמנרמל את הבעלים ל-int, ו-get_skill_bytesאף מטפל בבעלים שנשמר כמחרוזת — אבלlist_skills/delete_skillsמעבירים את הפרמטר כמות שהוא לשאילתה. קריאה עם"123"(למשל משכבת webapp) תחזיר ריק בשקט.int(user_id)בכניסה לשתי המתודות סוגר את הפער.Also applies to: 1445-1445
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@file_manager.py` at line 1383, Normalize user_id with int() at the start of both list_skills and delete_skills, before constructing their filesystem queries or deletion filters. Ensure the existing find flow around the metadata.user_id query and the corresponding delete path use the normalized integer value, while preserving current behavior for already-integer inputs.
1339-1373: 🚀 Performance & Scalability | 🔵 Trivialשקלו מכסה/מגבלת גודל לסקילים.
בניגוד לגיבויים, כאן אין retention ואין cleanup (מכוון), אבל גם אין תקרה לגודל בודד או למספר סקילים למשתמש — משתמש יחיד יכול למלא את ה-GridFS. שווה לשקול
SKILLS_MAX_PER_USER/SKILLS_MAX_BYTESואינדקס עלmetadata.user_id+metadata.skill_idבקולקצייתskills.files, כיlist_skills/get_skill_bytes/delete_skillsסורקות לפיהם.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@file_manager.py` around lines 1339 - 1373, Add configurable per-user skill limits to save_skill_bytes, enforcing both maximum skill count and total stored bytes before fs.put; reject the upload when either limit would be exceeded. Add or ensure indexes on metadata.user_id and metadata.skill_id in the skills.files collection, reusing the existing configuration and index-management patterns where available.skill_menu_handler.py (2)
191-205: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
except Exception: passשקט מדי בשליחת תפריט התיוג.אם
send_messageנכשל המשתמש לא יקבל שום חיווי ולא יישאר עקבות בלוג. עדיףlogger.exception+ הודעת שגיאה קצרה, כמו ביתר המתודות בקובץ.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skill_menu_handler.py` around lines 191 - 205, Update send_rating_prompt to replace the silent except Exception: pass with logger.exception and a brief user-facing error message when send_message fails, matching the error-handling pattern used by the other methods in the file.
63-66: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winכל פעולה על סקיל בודד סורקת את כל הסקילים של המשתמש.
_find_skillקורא ל-list_skills(סריקת GridFS מלאה) ומסנן בפייתון. ב-_download_by_idזה קורה אחריget_skill_bytes— כלומר שתי סריקות של הקולקציה בכל לחיצה על "הורדה", וגם בכל כניסה ל"פרטים". שווה להוסיף ל-SkillManagerמתודהget_skill_info(user_id, skill_id)שמבצעתfind({"metadata.skill_id": ...})ממוקד, או להחזיר את המטא יחד עם ה-bytes.Also applies to: 170-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skill_menu_handler.py` around lines 63 - 66, Replace the full-list scan in _find_skill and the duplicate lookup in _download_by_id with a targeted SkillManager.get_skill_info(user_id, skill_id) query using the skill ID metadata filter, or reuse metadata returned alongside get_skill_bytes. Ensure single-skill actions no longer call _get_skills/list_skills or scan the collection.utils.py (1)
1686-1691: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueהאזהרה של ה-static analysis כאן היא false positive — אבל שווה תיעוד קצר.
ה-
tokenמאומת מול_SAFE_TOKEN_RE(^[A-Za-z0-9_-]+$) שאינו מאפשר/,\או., ולכן_pending_zip_dir() / f"{token}.bin"לא יכול לצאת מהתיקייה — אין path traversal. הוספת הערה קצרה מעל ה-open(אוassert _is_under_pending_dir(path)) תמנע דגלים חוזרים בסריקות עתידיות.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils.py` around lines 1686 - 1691, The static-analysis warning is a false positive because token validation already prevents path traversal. Add a brief comment immediately before opening path in the pending-zip write flow, referencing _SAFE_TOKEN_RE’s restricted character set and confirming that the constructed path remains within _pending_zip_dir().Source: Linters/SAST tools
conversation_handlers.py (1)
910-920: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winספירת הקבצים ב-ZIP רצה בלולאת האירועים.
zipfile.ZipFile(...).namelist()על עד ~20MB היא פעולת CPU/פענוח קצרה אך לא אפסית, והיא היחידה בפונקציה שלא עברה ל-thread. אפשר לאחד אותה עם קריאת השמירה.⚡ הצעה
- file_count = 0 - try: - import zipfile as _zipfile - with _zipfile.ZipFile(BytesIO(raw)) as _zf: - file_count = sum(1 for n in _zf.namelist() if not n.endswith("/")) - except Exception: - file_count = 0 + def _count_entries(data: bytes) -> int: + try: + import zipfile as _zipfile + with _zipfile.ZipFile(BytesIO(data)) as _zf: + return sum(1 for n in _zf.namelist() if not n.endswith("/")) + except Exception: + return 0 + + file_count = await asyncio.to_thread(_count_entries, raw)As per coding guidelines: "wrap heavy computations with
await asyncio.to_thread(...)to avoid blocking Event Loop".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@conversation_handlers.py` around lines 910 - 920, Move the ZIP file counting currently performed inline in the “zip_route_skill” branch into the existing asyncio.to_thread flow, preferably by combining it with save_skill_bytes in a helper executed in the worker thread. Preserve the current file_count behavior, including excluding directory entries and falling back to zero on errors, while keeping the event loop non-blocking.Source: Coding guidelines
main.py (1)
3598-3615: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winשתי זרימות ההערה כמעט זהות — ושמירת ההערה חוסמת את ה-event loop.
הבלוק החדש הוא העתק כמעט מדויק של זרימת
waiting_for_backup_note_forשמעליו; אפשר לחלץ helper שמקבלentity_idו-back_callback. בנוסף,db.save_backup_noteהיא קריאת Mongo סינכרונית בתוך קורוטינה — כדאי לעטוף ב-await asyncio.to_thread(...).As per coding guidelines: "wrap heavy computations with
await asyncio.to_thread(...)to avoid blocking Event Loop".♻️ סקיצת helper
async def _save_note_flow(update, context, entity_id: str, back_cb: str, text: str) -> bool: try: from database import db ok = await asyncio.to_thread(db.save_backup_note, update.effective_user.id, entity_id, (text or '')[:1000]) if ok: await update.message.reply_text( "✅ ההערה נשמרה!", reply_markup=InlineKeyboardMarkup([[InlineKeyboardButton("🔙 חזרה", callback_data=back_cb)]]), ) context.user_data['suppress_code_hint_once'] = True else: await update.message.reply_text("❌ שמירת ההערה נכשלה") except Exception: logger.exception("שמירת הערה נכשלה") await update.message.reply_text("❌ שמירת ההערה נכשלה, נסה שוב מאוחר יותר") return True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.py` around lines 3598 - 3615, Extract the duplicated backup and skill note handling into a shared async helper accepting entity_id and back_cb, and route both waiting_for_backup_note_for and waiting_for_skill_note_for flows through it. Within the helper, invoke db.save_backup_note via await asyncio.to_thread(...) while preserving the existing success, failure, exception logging, reply, and suppress_code_hint_once behavior.Source: Coding guidelines
tests/test_skill_manager.py (1)
152-161: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueשקלו לסגור את ה-
MongoClientבסוף הטסט.עם
connect=Falseאין חיבור מיידי, אך האובייקט נשאר פתוח לכל אורך ה-session ועלול להשאיר תהליכוני monitor אם ייגע ברשת בעתיד.fixtureעםyield+client.close()ינקה את זה.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_skill_manager.py` around lines 152 - 161, Update _real_pymongo_db to manage the MongoClient with a yield-based fixture, yielding the selected database and calling client.close() during teardown; preserve the existing connect=False construction and returned database behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@file_manager.py`:
- Around line 1350-1356: עדכן את הודעת האזהרה בזרימת נרמול user_id סביב raw_uid
ו-user_id כך שלא תרשום את מזהה המשתמש הגולמי. השאר את הטיפול בשגיאה והחזרת None
ללא שינוי, ודווח במקום זאת רק על סוג הערך שסופק, ללא חשיפת PII.
In `@handlers/documents.py`:
- Around line 1037-1042: Update the pending ZIP handling around
stash_pending_zip_bytes to await asyncio.to_thread(stash_pending_zip_bytes,
raw_bytes, token), preserving the existing exception logging and False return
behavior; also run cleanup_stale_pending_zips through asyncio.to_thread because
it scans the directory synchronously.
- Around line 1050-1064: Update the ZIP pending-entry flow surrounding the
InlineKeyboardMarkup and reply_text call so that if sending the action prompt
fails, the corresponding token/file entry is removed before the outer exception
handler returns False. Preserve the existing successful reply behavior and
ensure cleanup targets only the entry created for this ZIP.
In `@skill_menu_handler.py`:
- Around line 255-305: הוסף קריאה ל-query.answer() לפני עריכת ההודעה בכל ענפי
callback הרלוונטיים: skill_delete_one_confirm, skill_delete_one_execute,
skill_rate ו-send_rating_prompt. ודא שהמענה מתבצע בתחילת כל ענף, לפני
safe_edit_message_text או כל פעולה שעלולה להסתיים בעדכון ההודעה, תוך שמירה על
התנהגות הודעות השגיאה הקיימת.
In `@tests/handlers/test_documents.py`:
- Line 309: The test leaves real pending ZIP files in temporary storage. After
asserting pending_zip in the relevant test flow, iterate over each pending entry
and call cleanup_pending_zip so every file created by stash_pending_zip_bytes is
removed while preserving the existing assertion.
In `@tests/test_patch_coverage.py`:
- Around line 46-51: Update the order comment above the callbacks assertion in
the relevant test to include skill_list and accurately state that six callbacks
are expected, matching the six entries asserted by callbacks[:6].
---
Nitpick comments:
In `@conversation_handlers.py`:
- Around line 910-920: Move the ZIP file counting currently performed inline in
the “zip_route_skill” branch into the existing asyncio.to_thread flow,
preferably by combining it with save_skill_bytes in a helper executed in the
worker thread. Preserve the current file_count behavior, including excluding
directory entries and falling back to zero on errors, while keeping the event
loop non-blocking.
In `@file_manager.py`:
- Around line 1279-1292: המירו את מחלקת הנתונים SkillInfo לשימוש ב-@dataclass,
הגדירו את אותם שדות עם הטיפוסים הקיימים ובאותו סדר, והסירו את __init__ הידני תוך
שמירה על ההתנהגות והממשק הנוכחיים.
- Around line 1444-1449: Update delete_skills to query only the requested skills
by adding the metadata.skill_id $in filter alongside metadata.user_id in
fs.find, using list(wanted), and remove the redundant Python-side skill
membership check. Extend _FakeGridFS._match in tests/test_skill_manager.py to
support $in comparisons.
- Line 1383: Normalize user_id with int() at the start of both list_skills and
delete_skills, before constructing their filesystem queries or deletion filters.
Ensure the existing find flow around the metadata.user_id query and the
corresponding delete path use the normalized integer value, while preserving
current behavior for already-integer inputs.
- Around line 1339-1373: Add configurable per-user skill limits to
save_skill_bytes, enforcing both maximum skill count and total stored bytes
before fs.put; reject the upload when either limit would be exceeded. Add or
ensure indexes on metadata.user_id and metadata.skill_id in the skills.files
collection, reusing the existing configuration and index-management patterns
where available.
In `@main.py`:
- Around line 3598-3615: Extract the duplicated backup and skill note handling
into a shared async helper accepting entity_id and back_cb, and route both
waiting_for_backup_note_for and waiting_for_skill_note_for flows through it.
Within the helper, invoke db.save_backup_note via await asyncio.to_thread(...)
while preserving the existing success, failure, exception logging, reply, and
suppress_code_hint_once behavior.
In `@skill_menu_handler.py`:
- Around line 191-205: Update send_rating_prompt to replace the silent except
Exception: pass with logger.exception and a brief user-facing error message when
send_message fails, matching the error-handling pattern used by the other
methods in the file.
- Around line 63-66: Replace the full-list scan in _find_skill and the duplicate
lookup in _download_by_id with a targeted SkillManager.get_skill_info(user_id,
skill_id) query using the skill ID metadata filter, or reuse metadata returned
alongside get_skill_bytes. Ensure single-skill actions no longer call
_get_skills/list_skills or scan the collection.
In `@tests/test_skill_manager.py`:
- Around line 152-161: Update _real_pymongo_db to manage the MongoClient with a
yield-based fixture, yielding the selected database and calling client.close()
during teardown; preserve the existing connect=False construction and returned
database behavior.
In `@utils.py`:
- Around line 1686-1691: The static-analysis warning is a false positive because
token validation already prevents path traversal. Add a brief comment
immediately before opening path in the pending-zip write flow, referencing
_SAFE_TOKEN_RE’s restricted character set and confirming that the constructed
path remains within _pending_zip_dir().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cbcd66c6-7720-45fc-8e9c-bc32b30cf439
📒 Files selected for processing (14)
conversation_handlers.pydocs/environment-variables.rstfile_manager.pyhandlers/documents.pymain.pyservices/config_inspector_service.pyskill_menu_handler.pytests/handlers/test_documents.pytests/test_config_inspector_service.pytests/test_patch_coverage.pytests/test_skill_manager.pyutils.pywebapp/app.pywebapp/templates/admin_config_inspector.html
…וך שירותים שלושה תיקונים בעקבות פידבק על העמוד: 1. רווחים שנעלמו: עטיפת הטאבים (.inspector-page) היא display:block, ולכן הילדים (כרטיסי סיכום, שורת סינון, קטגוריות, טבלה) איבדו את ה-gap של ה-flex ההורה (.config-page). העמוד הפעיל הוא עכשיו flex column עם אותו gap — הרווחים חזרו. 2. שורת סינון לעמוד 2 (שירותים אחרים): קטגוריה + שירות (אין שם Status כי אין ערכים חיים), עם סנן/איפוס/העתק הכל. הסינון client-side; "העתק הכל" מעתיק KEY=default עבור השורות הגלויות ומציין בטוסט שאלה ערכי ברירת מחדל. 3. דיוק שיוך שירותים (68 משתנים): מיפוי אוטומטי של צריכה בפועל — לכל משתנה נבדק באילו קבצים הוא נקרא (getenv/config.X) והאם הקובץ שייך לשירות או נטען בסגירת ה-imports שלו (entry: mcp_server/app.py, services/webserver.py). משתנים שסווגו ל-MCP/Webserver בלי שימוש אמיתי הוסרו משם (21 מ-MCP, 65 מ-Webserver — למשל UPTIME_*, PUSH_*, VAPID_*, MAINTENANCE_*, ALERTMANAGER_*). חריגים ידניים: PORT נשאר (נצרך בפקודת ההרצה), PROFILER_* נשארו ב-webserver (profiler_handler נטען שם בפועל). בפרט BACKUPS_STORAGE/BACKUPS_DIR עברו ל-Bot בלבד (גם ב-rst): רק הבוט טוען את file_manager; לוובאפ מנגנון גיבוי נפרד (WEBAPP_BACKUPS_DIR). הגיבויים האלה הם ZIP של קטעי הקוד (get_user_files) + גיבויי GitHub/Drive — לא אוספים/סימניות. טסטים: 67 ירוקים (config_inspector, skill_manager, mcp_docs, rst_parser), Jinja template מתקמפל. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
⏱️ 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! |
…לתות ממוקדות תיקונים בעקבות code review (כל הממצאים אומתו מול הקוד): אבטחה/פרטיות: - file_manager: אזהרת user_id לא תקין רושמת רק את סוג הערך, לא את הערך עצמו (PII). - utils: הערה מבהירה ש-_SAFE_TOKEN_RE ([A-Za-z0-9_-]) מונע path traversal ב-stash (false positive של static analysis — אין תיקון קוד). נכונות: - documents: stash_pending_zip_bytes ו-cleanup_stale_pending_zips רצים ב-to_thread (I/O של דיסק בתוך handler אסינכרוני); אם שליחת הודעת הבחירה נכשלת — הקובץ והרשומה שנוצרו עבור אותו ZIP מנוקים (בלי כפתורים אין דרך להשלים את הבחירה). - skill_menu: query.answer() לפני עריכה גם ב-delete_confirm/delete_execute וב-send_rating_prompt; ב-skill_rate המענה לפני עריכת ההצלחה בלבד — answer מוקדם גורף היה מבטל את ה-show_alert של הודעות השגיאה הקיימות. - file_manager: נרמול user_id ל-int בכל המתודות (list/get/info/delete) — קלט str לא היה מוצא כלום מול שמירה כ-int. מכסות ואינדקסים (לסקילים אין retention — בלי מכסה האחסון גדל ללא גבול): - SKILLS_MAX_PER_USER (ברירת מחדל 100) ו-SKILLS_MAX_TOTAL_BYTES (ברירת מחדל 1GB, 0 = כיבוי) נאכפים לפני fs.put; תועדו ב-rst וב-config inspector (שירות Bot). - אינדקסים על skills.files: (user_id, skill_id) ו-(skill_id) — best-effort פעם אחת. ביצועים/מבנה: - get_skill_info חדש: שליפת סקיל בודד בשאילתה ממוקדת; _find_skill ופרטי/הורדת סקיל כבר לא סורקים את כל הרשימה. delete_skills מסנן ב-DB עם $in. - conversation: ספירת קבצי ה-ZIP אוחדה עם השמירה להלפר אחד שרץ ב-to_thread. - main: זרימות הערה לגיבוי/סקיל אוחדו להלפר משותף; save_backup_note ב-to_thread. - SkillInfo הומר ל-dataclass; send_rating_prompt מלוגג כשל ומודיע במקום pass. טסטים: fixture עם client.close(); fake תומך $in; טסטי מכסות, נרמול str, ו-get_skill_info; ניקוי קבצי pending בשני טסטים שהשאירו קבצים ב-tmp (אומת 0 שאריות); עדכון הערת הסדר ב-test_patch_coverage. 77 טסטים ירוקים. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
|
@claude /review |
…906d5 — נפתר לטובת הנוסח החדש)
What
שלושה נושאים: (1) תיקון שורש לבאג "זה לא שומר סקילים" + סבב review; (2) תיקוני Config Inspector — רווחים, סינון בעמוד 2, ודיוק שיוך שירותים; (3) נוסח חדש להודעת קליטת ZIP, שינוי התווית "📦 קבצי ZIP" → "📦 קבצי גיבוי" בכל המופעים, ותשתית אימוג'י מותאם (custom emoji) עם fallback.
Why
הסקילים:
_get_skills_gridfsבדקif not mongo_db, אבל pymongo זורקNotImplementedErrorעלbool()של Database — החריגה נבלעה והוחזר None. תוקן ל-is None(גם ב-BackupManager), עם טסטי רגרסיה על Database אמיתי (אומת red→green). בסבב ה-review נוספו: הסרת PII מלוג, ניקוי pending בכשל שליחה, מכסות פר-משתמש (SKILLS_MAX_PER_USER/SKILLS_MAX_TOTAL_BYTES), אינדקסים, שאילתות ממוקדות (get_skill_info,$in), ו-query.answer()בענפים חסרים.Config Inspector: עטיפת הטאבים ביטלה את ה-flex gap (הרווחים נעלמו); בעמוד 2 חסרה שורת סינון; ו-68 משתנים שויכו ל-MCP/Webserver בלי שימוש בקוד של אותם שירותים (מופה אוטומטית מול גרף ה-imports).
BACKUPS_STORAGE/BACKUPS_DIRעברו ל-Bot בלבד.ממשק ה-ZIP: ההודעה אחרי העלאת ZIP עברה ל"איפה לשמור אותו?" עם תיאורי קטגוריות ואייקון 🧩 לסקיל; התווית "קבצי ZIP" הוחלפה ב"קבצי גיבוי" ורוכזה לקבוע
BTN_BACKUP_ZIPS(i18n) — שינוי תצוגה בלבד, אומת ששוםcallback_data/מפתח לא השתנה; ונוסףtg_emojiגנרי +CUSTOM_EMOJI_ZIP_IDלאייקון מותאם בגוף ההודעה, עם שליחה חוזרת עם 📁 אם טלגרם דוחה את ה-entity.✨ תיאור קצר
📦 שינויים עיקריים
פירוט נקודות:
file_manager.py:is Noneבמקוםnotעל pymongo Database; מכסות; אינדקסים;get_skill_info;$inבמחיקה; נרמול user_id; SkillInfo כ-dataclass.webapp/templates/admin_config_inspector.html+services/config_inspector_service.py: flex gap, שורת סינון client-side לעמוד 2, עדכוןservicesב-68 הגדרות.handlers/documents.py: נוסח הודעה חדש, כפתור "🧩 סקיל", אייקון מותאם עם BadRequest fallback; I/O ב-to_thread.i18n/strings_he.py: קבועBTN_BACKUP_ZIPS— נצרך ב-6 מופעי כפתור + טקסטים מפנים (📚, Batch, GitHub upload, Drive).config.py+utils.py:CUSTOM_EMOJI_ZIP_ID(pydantic, ברירת מחדל None) +tg_emojiגנרי.docs/environment-variables.rst,GUIDES/BOT_USER_GUIDE.md,docs/handlers/drive_menu.rst: תיעוד משתנים חדשים ותוויות.🧪 בדיקות
🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
Rollback: revert של הקומיטים; אין מיגרציות. משתני ENV חדשים אופציונליים (ברירות מחדל בטוחות). עיינתי ב-CodeBot – Project Docs.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K