feat(bot): סוג קובץ חדש "סקיל" — אחסון נפרד מגיבויים - #3197
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
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
|
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 (10)
📝 WalkthroughWalkthroughה-PR מוסיף אחסון וניהול סקילים מתוך ZIP, כולל בחירת יעד, תפריט בוט, דירוגים והערות. בנוסף, מפקח התצורה מסווג משתנים לפי שירות, מציג סטטוס SET ומוסיף תצוגה נפרדת לשירותים אחרים. Changesאחסון וזרימות סקילים
מפקח תצורה לפי שירות
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f84c9f7ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…פים בעמוד 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
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
file_manager.py (1)
1322-1322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
import uuidמיותר — כבר מיובא ברמת המודול (שורה 14).♻️ ניקוי
- import uuid try:🤖 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 1322, Remove the redundant local `import uuid` from the code at this location, since `uuid` is already imported at module scope. Keep the existing module-level import and all surrounding logic unchanged.main.py (1)
3597-3614: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winהבלוק הזה הוא שכפול כמעט מדויק של זרימת ההערה לגיבוי שמעליו.
ההבדל היחיד בין שני הבלוקים הוא שם ה-flag ופרפיקס ה-callback. חילוץ helper קטן ימנע סחיפה בין השניים בעתיד.
♻️ הצעה
async def _handle_note_flow(update, context, flag_key: str, back_prefix: str) -> bool: target_id = context.user_data.pop(flag_key, None) if not target_id: return False try: from database import db ok = db.save_backup_note(update.effective_user.id, target_id, (text or '')[:1000]) if ok: await update.message.reply_text( "✅ ההערה נשמרה!", reply_markup=InlineKeyboardMarkup( [[InlineKeyboardButton("🔙 חזרה", callback_data=f"{back_prefix}:{target_id}")]] ), ) context.user_data['suppress_code_hint_once'] = True else: await update.message.reply_text("❌ שמירת ההערה נכשלה") except Exception: logger.exception("save note failed for %s", flag_key) 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 3597 - 3614, חלץ את זרימת שמירת ההערה המשותפת ל-helper אסינכרוני כגון _handle_note_flow, המקבל את מפתח ה-flag ואת פרפיקס ה-callback. עדכן את בלוקי הערת הסקיל והגיבוי להשתמש באותו helper, תוך שמירה על שמירת הטקסט, הודעות ההצלחה/כישלון, דיכוי רמז הקוד והחזרת True; השתמש ב-logger.exception עבור שגיאות במקום לשכפל את הטיפול בכל בלוק.conversation_handlers.py (1)
894-894: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winקריאת דיסק סינכרונית של עד 20MB על לולאת האירועים.
load_pending_zip_bytesהוא I/O חוסם, ושאר הפונקציה כבר מקפידה עלasyncio.to_threadלשמירה. שווה עקביות.♻️ תיקון מוצע
- raw = load_pending_zip_bytes((entry or {}).get("path", "")) if entry else None + raw = ( + await asyncio.to_thread(load_pending_zip_bytes, (entry or {}).get("path", "")) + if entry else 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 `@conversation_handlers.py` at line 894, עדכן את הקריאה ל־load_pending_zip_bytes בתוך הפונקציה הנוכחית כך שתתבצע באמצעות asyncio.to_thread, תוך שמירה על התנאי הקיים והחזרת None כשאין entry. אל תשנה את התנהגות טעינת הנתיב או את שאר זרימת הפונקציה.skill_menu_handler.py (1)
150-152: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winשתי סריקות GridFS מלאות לכל הורדה.
_find_skillמריץlist_skills(סריקה על כל הסקילים של המשתמש) רק כדי לשלוף אתoriginal_name, ומיד אחריוget_skill_bytesסורק שוב. שווה להחזיר גם את המטא-דאטה מקריאה אחת, או לפחות לדלג על_find_skillכשה-bytes לא נמצאו.🤖 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 150 - 152, Eliminate the unconditional double GridFS scan in the download flow around `_find_skill` and `skill_manager.get_skill_bytes`: fetch the bytes first and return the existing not-found response immediately when they are absent, then call `_find_skill` only for successful downloads to obtain `original_name`. Preserve the current metadata-based response behavior for found skills.services/config_inspector_service.py (1)
90-107: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winהסרת "URL" מ-SENSITIVE_PATTERNS מבוססת על תיוג ידני בלבד.
לאחר ההסרה, ערך שמכיל credentials בתוך URL (כמו
https://user:pass@host) יוסתר רק אם המפתח תואם תבנית אחרת (URI/TOKEN/...) או מסומןsensitive=Trueבאופן מפורש. כרגע כל המשתנים הידועים שעלולים להכיל credentials מתויגים נכון (MONGODB_URL,PUSH_DELIVERY_URL,OBS_AI_EXPLAIN_URLוכו'), אבל אין רשת ביטחון אוטומטית למשתנה URL עתידי שישכח לתייג sensitive=True.שווה לשקול הוספת בדיקה מבוססת-ערך (regex לזיהוי
user:pass@בתוך ה-URL) בתוךmask_value, כהגנה נוספת (defense-in-depth) שלא תלויה רק בזיכרון של המפתח שהגדיר את המשתנה.Claude Code עשה כאן עבודה יפה בליווי כל שינוי ב-services עם הסבר ותיעוד ב-docstring — ממש מקצועי. CodeKeeper forever 💫
🔒 הצעת תוספת הגנה במיסוך ערכים
+ # זיהוי credentials מוטמעים בכתובת (user:pass@host) גם אם שם המפתח לא "רגיש" + _CREDENTIALS_IN_VALUE_RE = re.compile(r"://[^/\s:@]+:[^/\s@]+@") + def mask_value(self, value: str, key: str) -> str: if not value: return value - if self.is_sensitive_key(key): + if self.is_sensitive_key(key) or self._CREDENTIALS_IN_VALUE_RE.search(value): return self.MASKED_VALUE return value🤖 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 `@services/config_inspector_service.py` around lines 90 - 107, הוסף ב־mask_value זיהוי מבוסס־ערך של כתובות URL המכילות credentials בתבנית user:pass@, והסתר ערכים כאלה גם כאשר שם המשתנה אינו תואם ל־SENSITIVE_PATTERNS ואינו מסומן sensitive=True. שמור על התנהגות המיסוך הקיימת עבור שאר הערכים.
🤖 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 `@conversation_handlers.py`:
- Around line 923-929: Route all four edit_message_text calls in
_handle_zip_route, the skill-list update in skill_menu_handler.py lines 106-106,
and the deletion/tagging confirmation screens in handle_callback_query at
skill_menu_handler.py lines 238-238 through
TelegramUtils.safe_edit_message_text. Use the existing wrapper so only “message
is not modified” is suppressed while other BadRequest errors propagate.
- Around line 919-929: Move cleanup_pending_zip and pending.pop in the save flow
so they run only after save_skill_bytes or save_backup_bytes returns a
successful identifier; preserve the failure path’s pending token and temporary
bytes so the user can retry without re-uploading, while keeping the existing
success and failure messages.
In `@file_manager.py`:
- Around line 1347-1359: Normalize metadata["user_id"] to an int before
constructing final_md and calling fs.put in the skill save flow. Update the
local user_id used in skill_id generation as well, while preserving existing
behavior for valid integer inputs and handling invalid or missing values
according to the surrounding validation conventions.
In `@handlers/documents.py`:
- Around line 1011-1040: Limit the number of pending ZIP entries per user in the
pending ZIP flow before stashing a new file, removing the oldest entries and
their files when the configured capacity is exceeded. Replace the hardcoded
3600-second stale check in this cleanup block with the shared
PENDING_ZIP_TTL_SECONDS constant from utils.py, preserving the existing cleanup
behavior.
In `@main.py`:
- Around line 3612-3613: Update the exception handler around the note-saving
operation to log the full exception via logger.exception while replying with a
generic Hebrew error message that does not interpolate e or expose database
details. Preserve the existing failure-response flow in
update.message.reply_text.
In `@skill_menu_handler.py`:
- Around line 84-100: העבירו את שליפת דירוגי הגיבוי מתוך לולאת הרינדור
האסינכרונית אל thread worker, בדומה לעטיפת _get_skills ב-asyncio.to_thread. אספו
את הדירוגים עבור כל פריטי items בביצוע מרוכז או מקביל לפני בניית lines
ו-keyboard, ואז השתמשו בתוצאות ללא קריאות סינכרוניות בתוך הלולאה; שמרו על דירוג
ריק במקרה של facade חסר או שגיאה.
In `@utils.py`:
- Around line 1657-1679: עדכן את `_pending_zip_dir` ואת
`stash_pending_zip_bytes` כדי לאמת ש-token הוא מזהה בטוח לשם קובץ ללא מפרידי
נתיב או רכיבי traversal, ולדחות ערכים לא תקינים לפני הכתיבה. צור את תיקיית
ה-pending בהרשאות פרטיות למשתמש בלבד, כולל הידוק הרשאות גם אם התיקייה כבר קיימת.
שמור על כתיבה רק לנתיב שנגזר מה-token המאומת.
---
Nitpick comments:
In `@conversation_handlers.py`:
- Line 894: עדכן את הקריאה ל־load_pending_zip_bytes בתוך הפונקציה הנוכחית כך
שתתבצע באמצעות asyncio.to_thread, תוך שמירה על התנאי הקיים והחזרת None כשאין
entry. אל תשנה את התנהגות טעינת הנתיב או את שאר זרימת הפונקציה.
In `@file_manager.py`:
- Line 1322: Remove the redundant local `import uuid` from the code at this
location, since `uuid` is already imported at module scope. Keep the existing
module-level import and all surrounding logic unchanged.
In `@main.py`:
- Around line 3597-3614: חלץ את זרימת שמירת ההערה המשותפת ל-helper אסינכרוני
כגון _handle_note_flow, המקבל את מפתח ה-flag ואת פרפיקס ה-callback. עדכן את
בלוקי הערת הסקיל והגיבוי להשתמש באותו helper, תוך שמירה על שמירת הטקסט, הודעות
ההצלחה/כישלון, דיכוי רמז הקוד והחזרת True; השתמש ב-logger.exception עבור שגיאות
במקום לשכפל את הטיפול בכל בלוק.
In `@services/config_inspector_service.py`:
- Around line 90-107: הוסף ב־mask_value זיהוי מבוסס־ערך של כתובות URL המכילות
credentials בתבנית user:pass@, והסתר ערכים כאלה גם כאשר שם המשתנה אינו תואם
ל־SENSITIVE_PATTERNS ואינו מסומן sensitive=True. שמור על התנהגות המיסוך הקיימת
עבור שאר הערכים.
In `@skill_menu_handler.py`:
- Around line 150-152: Eliminate the unconditional double GridFS scan in the
download flow around `_find_skill` and `skill_manager.get_skill_bytes`: fetch
the bytes first and return the existing not-found response immediately when they
are absent, then call `_find_skill` only for successful downloads to obtain
`original_name`. Preserve the current metadata-based response behavior for found
skills.
🪄 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: 4fd57f71-2c8e-479a-bfe5-2cf1b4a90320
📒 Files selected for processing (12)
conversation_handlers.pydocs/environment-variables.rstfile_manager.pyhandlers/documents.pymain.pyservices/config_inspector_service.pyskill_menu_handler.pytests/test_config_inspector_service.pytests/test_skill_manager.pyutils.pywebapp/app.pywebapp/templates/admin_config_inspector.html
|
@claude הארנב הציל אותנו |
תיקון כשלי 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
✨ תיאור קצר
פיצ'ר חדש בבוט: סוג קובץ "סקיל". כשמעלים ZIP, אפשר לבחור לשמור אותו כסקיל — אחסון קבוע byte-for-byte, נפרד לגמרי מהגיבויים — במקום כגיבוי. נועד לארכיון קוד לטווח ארוך שמשותף הלאה: בלי מחיקת retention ובלי הזרקת
metadata.jsonלתוך הארכיון.📦 שינויים עיקריים
פירוט (סקילים):
SkillManagerחדש (file_manager.py): קולקציית GridFS"skills"נפרדת. שמירת bytes as-is (fs.putישיר — בלי לפתוח/לדחוס את ה-ZIP ובליmetadata.jsonמוזרק), כך שהורדה מחזירה את הקובץ byte-for-byte. תמיד מונגו, בלי תלות ב-BACKUPS_STORAGEובלי משתנה סביבה חדש.skill_idייחודי (timestamp+uuid) מונע התנגשות; שם קובץ עם סיומת ייחוד מונע דריסה בין סקילים עם אותו שם.skillsלעולם לא נסרקת/נמחקת ע"יcleanup_expired_backups(כבול ל-backups), ו-restore ... purge=Trueאינו חשוף עליה.handlers/documents.py):_maybe_store_zip_copyמציג שני כפתורים "📝 סקיל" / "📦 גיבוי" במקום שמירה אוטומטית. ה-bytes נשמרים זמנית בקובץ tmp ייעודי (עזרי stash ב-utils.py, מחיקות מוגבלות ל-allowlist) עד לבחירה מפורשת; אם לא בוחרים — כלום לא נשמר.SkillMenuHandlerחדש (skill_menu_handler.py, prefixskill_): רשימת סקילים + פעולות זהות ל-ZIP (הורדה/מחיקה/תיוג/הערה). כפתור "📝 סקילים" נוסף בתפריט "📚 הצג את כל הקבצים שלי". תיוג/הערה עוברים דרך ה-facade הגנרי הקיים (מפתחuser_id + id).🧪 בדיקות
tests/test_skill_manager.py(5 טסטים): שמירה+הורדה byte-for-byte (הקריטי), אימות בעלות, בידוד מ-cleanup של הגיבויים, ושני סקילים עם אותו שם שאינם דורסים. טסטי הגיבויים הקיימים (test_gridfs_backups,test_backup_cleanup_gridfs_budget) נשארו ירוקים — 52 טסטים רלוונטיים ירוקים בסך הכל.py_compileנקי על כל הקבצים ששונו.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
environment-variables.rst/config_inspector_service.py. עיינתי בהנחיות הריפו לפני המימוש.codebot_pending_zip/) עם בדיקת נתיב (_is_under_pending_dir)🧩 השפעות/סיכונים
שינוי התנהגות: העלאת ZIP רגילה כעת דורשת בחירה מפורשת (סקיל/גיבוי) במקום שמירה אוטומטית לגיבוי (בהתאם להחלטת המשתמש). אין מיגרציות DB. קולקציית
skillsחדשה ונפרדת — אינה נוגעת בנתוני הגיבויים הקיימים.🧯 סיכון / החזרה לאחור (Rollback)
git revertשל קומיט הסקילים (606a1c1). קולקצייתskillsעצמאית; אין schema/מיגרציה. ביטול בטוח — הגיבויים ממשיכים לעבוד כרגיל.