Skip to content

fix: החזרת חישוב תיקיית השורש בשחזור ZIP לריפו (רגרסיה מ-#3205) - #3314

Merged
amirbiron merged 3 commits into
mainfrom
claude/fix-repo-zip-restore-structure
Sep 1, 2026
Merged

fix: החזרת חישוב תיקיית השורש בשחזור ZIP לריפו (רגרסיה מ-#3205)#3314
amirbiron merged 3 commits into
mainfrom
claude/fix-repo-zip-restore-structure

Conversation

@amirbiron

@amirbironamirbiron commented Sep 1, 2026

Copy link
Copy Markdown
Owner

✨ תיאור קצר

שחזור ZIP לריפו דרך /github ← גיבוי ושחזור פרס את כל תוכן הריפו בתוך תיקייה בשם owner-repo-sha, במקום בשורש. זו רגרסיה מ-#3205 (fd13dd5e, 6 באוגוסט 2026), והחישוב שהיה קודם מוחזר כלשונו.

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

  • קוד (Backend)
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • DevOps/CI/CD

פירוט נקודות:

  • handlers/documents.py_handle_github_restore_zip_to_repo חוזר לחישוב תיקיית השורש שהיה לו עד feat(bot): פריסת ZIP לתיקייה ספציפית בריפו #3205, מילה במילה: ספירת תיקיות בלבד.
  • detect_zip_common_root לא שונתה כלל. היא ממשיכה לשרת את ייבוא ה-ZIP ואת הפריסה לתיקייה, שאצלם הכלל שלה נכון. נוספה לה אזהרה בדוקסטרינג נגד איחוד חוזר עם מסלול השחזור — האיחוד הזה הוא שגרם לרגרסיה.
  • שתי בדיקות מקצה לקצה למסלול השחזור, שלא הייתה לו אף אחת.
  • docs/workflows/backup-flow.rst — תועד מבנה ה-ZIP של github_repo_zip ולמה השחזור סופר תיקיות בלבד. המבנה לא היה מתועד, וזה מה שאיפשר לשבור אותו.

🧪 בדיקות

מה השתנה ב-#3205, מאומת בהרצה של שתי הגרסאות על אותו ZIP:

הישן (עד fd13dd5e): שורש='owner-repo-6dfaac9' ← ['metadata.json', 'README.md', 'src/main.py']
החדש (מ-fd13dd5e): שורש=None ← ['metadata.json', 'owner-repo-6dfaac9/README.md', ...]

הקוד הישן ספר רק שמות שמכילים /, ולכן metadata.json שבשורש לא השתתף בספירה — נשארה תיקייה אחת, והיא נחתכה. #3205 החליף את זה ב-detect_zip_common_root, שבה קובץ בשורש מבטל את זיהוי השורש. הכלל ההוא נוסף במכוון ונכון לייבוא ZIP (הוא מונע חיתוך שגוי של ארכיון כמו README.md לצד src/, ויש עליו טסט) — אבל שגוי כאן.

  • Integration — הזרימה המלאה של handle_document במצב github_restore_zip_to_repo, מול stub של GitHub, ואימות הנתיבים שנשלחים ל-tree. נופלת על main עם ['metadata.json', 'owner-repo-6dfaac9/README.md', 'owner-repo-6dfaac9/src/main.py'] — בדיוק התסמין שדווח.
  • Integration — ארכיון עם שתי תיקיות עליונות אינו נחתך, כדי שחיתוך גורף לא ייראה תקין מול הבדיקה הראשונה.
  • Manual — לא הרצתי מול GitHub אמיתי; אין לי גישה לטוקן.

138 טסטים בקבצים שנוגעים ב-handlers.documents עוברים. flake8 זהה לבסיס (6 מול 6), mypy 8 מול 8, ה-RST אומת ב-docutils.

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

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

📝 סוג שינוי

  • fix: תיקון באג

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון (flake8/mypy — מושווה לבסיס)
  • בדיקות רצות ועוברות
  • תיעוד עודכן
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות/פעולות על root
  • הודעת הקומיט תואמת Conventional Commits
  • עיינתי במסמכי אתר התיעוד — נתיב: docs/workflows/backup-flow.rst | המשפט: "github_repo_zip אינו ארכיון שהבוט בונה בעצמו"

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

סיכון נמוך במיוחד: זו החזרה להתנהגות שרצה בפרודקשן חודשים, ולא לוגיקה חדשה. detect_zip_common_root לא נגעתי בה, ולכן ייבוא ZIP והפריסה לתיקייה אינם מושפעים כלל.

metadata.json ממשיך לעלות לריפו יחד עם התוכן — כך זה תמיד היה, כולל לפני הרגרסיה, וזו התנהגות מכוונת שנשמרת.

מה שלא בסקופ: משך השחזור. בלוגים שהתקבלו הוא נמדד כ-21 שניות; האיטיות שדווחה הייתה על ארכיון אחר בגודל 3.5MB, ולא נגעתי בה.

Summary by Sourcery

Restore the repository-root calculation for GitHub backup ZIPs while preserving the existing ZIP import behavior.

Bug Fixes:

  • Restore GitHub repository backup ZIPs directly to the repository root instead of leaving their contents nested under the zipball directory.

Enhancements:

  • Add user-facing guidance for ZIP restore behavior and document the backup archive structure and root-directory handling.

Documentation:

  • Document the structure of github_repo_zip backups and explain why restoration identifies roots by directories only.

Tests:

  • Add end-to-end coverage for repository backup restoration and archives with multiple or ambiguous top-level directories.

ה-ZIP של גיבוי ריפו הוא ה-zipball הרשמי של GitHub, שכל תוכנו יושב תחת
תיקייה אחת בשם owner-repo-sha, ואליו נוסף metadata.json בשורש הארכיון.
הקובץ בשורש הוא בדיוק מה שגורם ל-detect_zip_common_root להחזיר None —
וזה הכלל הנכון שלה, שנועד למנוע חיתוך שגוי של ZIP כמו README.md לצד
src/ — ולכן תיקיית ה-zipball לא נחתכה. השחזור העלה לריפו את המניפסט
ואת כל התוכן רובד אחד עמוק מדי.
המניפסט מוסר עכשיו לפני חישוב תיקיית השורש, ולא רק מרשימת הקבצים
לפריסה. ההכרה בו נעשית לפי תוכן ולא לפי שם: metadata.json הוא שם קובץ
לגיטימי בריפו אמיתי, ודילוג לפי שם היה מוחק קובץ תוכן. קובץ נחשב
מניפסט רק אם הוא JSON תקין עם backup_id (השדה שגם save_backup_file
וגם save_backup_bytes מבטיחים) ועוד שדה מזהה.
שני המסלולים שפורסים ZIP שכפלו את אותו חישוב, ולכן הוא רוכז
ב-zip_members_and_root — אחרת התיקון היה חל על אחד מהם בלבד.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbK6wRnQuBb9H9VC11n9NT
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@sourcery-aisourcery-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2 days and 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@github-actions

Copy link
Copy Markdown
Contributor

🧯 Dangerous deletes guard report

Policy: see .cursorrules — dangerous deletions are blocked unless wrapped safely.

Summary:

  • Flagged findings (blocking): 0
    0
  • Excluded matches (not blocking): 15
  • Total matches (all files): 129

Flagged findings (file:line:snippet):
(none)

Excluded matches (by path pattern)
./webapp/static/js/md_preview.bundle.js.map:4: "sourcesContent": ["// Markdown-it plugin to render GitHub-style task lists; see\n//\n// https://github.com/blog/1375-task-lists-in-gfm-issues-pulls-comments\n// https://github.com/blog/1825-t … [truncated]
./README.md:842:find . -name "__pycache__" -exec rm -rf {} +
./docs/DOCUMENTATION_GUIDE.md:453:rm -rf _build
./docs/Makefile:24:	rm -rf $(BUILDDIR)
./Dockerfile:42: rm -rf /var/lib/apt/lists/*
./Dockerfile:121: rm -rf /var/lib/apt/lists/*
./node_modules/katex/src/fonts/Makefile:139:	rm -rf pfa ff otf ttf woff woff2
./node_modules/katex/package.json:153: "build": "rimraf dist/ && mkdirp dist && cp README.md dist && rollup -c --failAfterWarnings && webpack && node update-sri.js package dist/README.md",
./node_modules/mermaid/dist/mermaid.js.map:4: "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values for tr … [truncated]
./node_modules/mermaid/dist/mermaid.min.js:1524:`,"getStyles"),c1e=RQe});var h1e={};dr(h1e,{diagram:()=>NQe});var NQe,f1e=N(()=>{"use strict";$ge();a1e();l1e();u1e();NQe={parser:Fge,db:n1e,renderer:o1e,styles:c1e}});var m1e,g1e=N(()=>{"use … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm/chunk-2M32CCKP.mjs.map:4: "sourcesContent": ["{\n \"name\": \"mermaid\",\n \"version\": \"11.12.0\",\n \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence d … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.core/chunk-KS23V3DP.mjs.map:4: "sourcesContent": ["{\n \"name\": \"mermaid\",\n \"version\": \"11.12.0\",\n \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs.map:4: "sourcesContent": ["{\n \"name\": \"mermaid\",\n \"version\": \"11.12.0\",\n \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequen … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs:1:var r={name:"mermaid",version:"11.12.0",description:"Markdown-ish syntax for generating flowcharts, mindmaps, sequence diagrams, class diagrams, gantt charts, git graph … [truncated]
./node_modules/mermaid/dist/mermaid.min.js.map:4: "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values fo … [truncated]

@sourcery-ai

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Fixes repository ZIP restoration by recognizing and removing the backup manifest before common-root detection, allowing GitHub zipball contents to be restored at the repository root while preserving legitimate user metadata.json files; both ZIP-consuming paths now share the behavior and are covered by unit and end-to-end tests.

Sequence diagram for corrected GitHub ZIP restoration

sequenceDiagram
participant User
participant Handler as handle_document
participant ZipHelper as zip_members_and_root
participant GitHub
User->>Handler: handle_document
Handler->>ZipHelper: zip_members_and_root
ZipHelper->>ZipHelper: is_ck_backup_manifest
ZipHelper->>ZipHelper: detect_zip_common_root
ZipHelper-->>Handler: members, common_root
Handler->>GitHub: create tree with stripped repository paths
GitHub-->>Handler: restoration result
Loading

Flow diagram for backup manifest and common-root handling

flowchart TD
A[ZIP archive] --> B[Collect file members]
B --> C{metadata.json is a CK backup manifest?}
C -- Yes --> D[Remove manifest from root detection]
C -- No --> E[Preserve metadata.json as repository content]
D --> F[detect_zip_common_root]
E --> F
F --> G[Strip zipball root from paths]
G --> H[Restore files at repository root]
Loading

File-Level Changes

ChangeDetailsFiles
Centralize ZIP member filtering and backup-root detection so valid backup manifests are removed before common-root calculation.
  • Added content-based manifest validation requiring valid JSON, backup_id, and backup_type or created_by.
  • Added shared ZIP parsing that excludes metadata only when it is a recognized backup manifest, then detects the zipball root.
  • Updated both repository restoration and generic ZIP import to use the shared parser.
handlers/documents.py
Add regression and integration coverage for repository backup ZIP restoration without an extra directory level.
  • Tested recognition and rejection cases for backup manifests.
  • Tested preservation of user-owned metadata.json and its effect on root detection.
  • Tested the full handle_document restoration flow and asserted root-level GitHub tree paths.
tests/test_zip_to_folder.py
Document the backup ZIP structure and manifest contract.
  • Documented that github_repo_zip wraps repository content in the GitHub zipball directory and stores metadata.json at the archive root.
  • Clarified that the manifest is auxiliary metadata rather than repository content.
docs/workflows/backup-flow.rst
docs/whats-new.rst

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitaiBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 69f0d3a7-06ec-47b5-9021-3ceb6cc0d639

📥 Commits

Reviewing files that changed from the base of the PR and between a652012 and 7ec8a9d.

📒 Files selected for processing (5)
  • docs/whats-new.rst
  • docs/workflows/backup-flow.rst
  • github_menu_handler.py
  • handlers/documents.py
  • tests/test_zip_to_folder.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/whats-new.rst

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

השחזור לריפו מחשב את שורש ה-ZIP לפי תיקיות בלבד ושומר את metadata.json. הייבוא הכללי ממשיך להשתמש ב-detect_zip_common_root על כל שמות הארכיון. נוספו בדיקות אינטגרציה, הנחיית משתמש ותיעוד מעודכן.

Changes

שחזור וייבוא ZIP

Layer / File(s)Summary
חישוב שורש בשחזור
handlers/documents.py, tests/test_zip_to_folder.py
מסלול השחזור מסנן פריטי מערכת, סופר תיקיות בלבד ואינו מסיר את metadata.json. הבדיקות מאמתות שחזור לשורש ושמירת מבנה מקונן.
חישוב שורש בייבוא
handlers/documents.py, tests/test_zip_to_folder.py
מסלול הייבוא מחשב את רשימת החברים מקומית ומשתמש ב-detect_zip_common_root על כל שמות ה-ZIP.
אזהרות ותיעוד השחזור
github_menu_handler.py, docs/workflows/backup-flow.rst, docs/whats-new.rst
נוספה RESTORE_ZIP_PROMPT עם אזהרת השטחה ל-ZIP שאינו גיבוי. התיעוד מסביר את ההפרדה בין חישובי השורש. Claude Code ביצע עבודת קידוד מדויקת.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to 7ec8a

This change restores GitHub backup ZIP contents to the repository root while preserving existing ZIP import behavior. The change is localized and supported by targeted tests, so no actionable merge-blocking risk remains beyond normal checks and review.

Poem

ZIP נכנס, והשורש מתיישר,
metadata.json נשמר ולא נעלם.
השחזור סופר תיקיות בלבד,
הייבוא שומר על מסלול נפרד.
Claude Code קידד בדיוק.
CodeKeeper forever 💫

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (2 skipped: …Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Title check✅ Passedכותרת ה-PR קצרה, ברורה ומתארת במדויק את השינוי המרכזי: החזרת חישוב תיקיית השורש בשחזור ZIP לריפו ותיקון הרגרסיה מ-#3205.
Description check✅ Passedתיאור ה-PR מפורט ורלוונטי. הוא כולל את הבעיה, הסיבה, השינויים, הבדיקות, הסיכונים והתיעוד. סעיף Rollback נפרד אינו מופיע, וחלק מהבדיקות הנדרשות אינן מסומנות כבוצעות, אך התיאור ברובו שלם.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-repo-zip-restore-structure

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

⏱️ Performance report

(No performance test durations collected. Mark tests with @pytest.mark.performance.)

@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation has been built successfully!

To view locally:

  1. Download the artifacts
  2. Extract the zip file
  3. Open index.html in your browser

@codecov

codecovBot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment threadhandlers/documents.py Outdated
עד fd13dd5 (2026-08-06) המסלול החזיק חישוב משלו שסופר תיקיות בלבד:
קובץ בשורש אינו מכיל "/", ולכן metadata.json שהגיבוי מוסיף לא השתתף
בספירה, נשארה תיקיית ה-zipball היחידה, והיא נחתכה. השחזור עבד.
אותו PR ריכז את החישוב ל-detect_zip_common_root, שבה קובץ בשורש מבטל
את זיהוי השורש — כלל נכון לייבוא ZIP, שמונע חיתוך שגוי של ארכיון כמו
README.md לצד src/, אבל שגוי כאן. מאז השחזור העלה לריפו את התוכן בתוך
תיקייה בשם owner-repo-sha.
החישוב המקורי הוחזר מילה במילה, ו-detect_zip_common_root לא שונתה
כלל: היא ממשיכה לשרת את הייבוא ואת הפריסה לתיקייה, שאצלם הכלל נכון.
נוספה לה אזהרה בדוקסטרינג נגד איחוד חוזר, כי האיחוד הזה הוא שגרם
לרגרסיה.
metadata.json ממשיך לעלות לריפו יחד עם התוכן, כפי שהיה תמיד.
למסלול השחזור לא הייתה בדיקה מקצה לקצה, ולכן ההחלפה עברה בשקט.
נוספו שתיים: אחת על מבנה הגיבוי האמיתי, ואחת על ארכיון בלי שורש יחיד
כדי שחיתוך גורף לא ייראה תקין.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbK6wRnQuBb9H9VC11n9NT
@amirbironamirbiron changed the title fix: שחזור ZIP לריפו מעלה לשורש, ולא לתוך תיקיית ה-zipballfix: החזרת חישוב תיקיית השורש בשחזור ZIP לריפו (רגרסיה מ-#3205)Sep 1, 2026

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="handlers/documents.py">
<violation number="1" location="handlers/documents.py:434">
P2: Custom agent: **Enforce Strict Maintainability Standards**
The two ZIP flows now duplicate the same member-filtering policy after removing `zip_members_and_root`. Extract the shared filtering into a helper while keeping the restore and import root-detection policies separate.</violation>
<violation number="2" location="handlers/documents.py:446">
P2: החישוב החדש של `common_root` במסלול שחזור-לריפו חותך תיקייה עליונה יחידה גם כשיש קבצים חופשיים בשורש הארכיון (למשל `README.md` לצד `src/`), בניגוד ל-`detect_zip_common_root` ששימש קודם בגרסת ה-main ומנע בדיוק את זה. עבור ZIP שרירותי שהועלה לשחזור התוצאה היא פילוס מבנה הריפו. בדוק שהכרה בשורש נלקחת בחשבון גם קובץ חופשי בשורש שאינו `metadata.json` לפני החיתוך.</violation>
<violation number="3" location="handlers/documents.py:628">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
When a backup ZIP contains the documented root-level `metadata.json`, both readers still treat it as content: the restore path uploads it, and `_parse_zip_for_import` passes it to `detect_zip_common_root`, which leaves the GitHub zipball directory nested. This contradicts the new manifest contract and the docs' claim that both paths remove the manifest before root detection; restore the shared content-based filtering before calculating the root and use it in both paths.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment threadhandlers/documents.py
@@ -45,6 +45,15 @@ def detect_zip_common_root(names: Iterable[str]) -> Optional[str]:
תיקיית שורש קיימת רק אם *כל* הרשומות נמצאות תחתיה. אם יש ולו קובץ אחד

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Flag AI Slop and Fabricated Changes

When a backup ZIP contains the documented root-level metadata.json, both readers still treat it as content: the restore path uploads it, and _parse_zip_for_import passes it to detect_zip_common_root, which leaves the GitHub zipball directory nested. This contradicts the new manifest contract and the docs' claim that both paths remove the manifest before root detection; restore the shared content-based filtering before calculating the root and use it in both paths.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At handlers/documents.py, line 628:
<comment>When a backup ZIP contains the documented root-level `metadata.json`, both readers still treat it as content: the restore path uploads it, and `_parse_zip_for_import` passes it to `detect_zip_common_root`, which leaves the GitHub zipball directory nested. This contradicts the new manifest contract and the docs' claim that both paths remove the manifest before root detection; restore the shared content-based filtering before calculating the root and use it in both paths.</comment>
<file context>
@@ -652,6 +625,7 @@ def _parse_zip_for_import(self, buf: BytesIO):
total_uncompressed += entry_size
if total_uncompressed > MAX_IMPORT_TOTAL_UNCOMPRESSED_BYTES:
raise ValueError("סך התוכן הלא-דחוס בארכיון חורג מהמגבלה המותרת.")
+ common_root = detect_zip_common_root(names_all)
def strip_root(path: str) -> str:
</file context>

Comment threaddocs/workflows/backup-flow.rst Outdated
Comment threadhandlers/documents.py
@@ -45,6 +45,15 @@ def detect_zip_common_root(names: Iterable[str]) -> Optional[str]:
תיקיית שורש קיימת רק אם *כל* הרשומות נמצאות תחתיה. אם יש ולו קובץ אחד

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Strict Maintainability Standards

The two ZIP flows now duplicate the same member-filtering policy after removing zip_members_and_root. Extract the shared filtering into a helper while keeping the restore and import root-detection policies separate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At handlers/documents.py, line 434:
<comment>The two ZIP flows now duplicate the same member-filtering policy after removing `zip_members_and_root`. Extract the shared filtering into a helper while keeping the restore and import root-detection policies separate.</comment>
<file context>
@@ -477,7 +430,20 @@ async def _handle_github_restore_zip_to_repo(self, update: Update, context: Cont
with zipfile.ZipFile(buf, "r") as zf:
- members, common_root = zip_members_and_root(zf)
+ all_names = [n for n in zf.namelist() if not n.endswith("/")]
+ members = [
+ n
+ for n in all_names
</file context>

Comment threaddocs/workflows/backup-flow.rst Outdated
Comment threadhandlers/documents.py
for name in zf.namelist():
if "/" in name and not name.startswith("__MACOSX/"):
top_levels.add(name.split("/", 1)[0])
common_root = list(top_levels)[0] if len(top_levels) == 1 else None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: החישוב החדש של common_root במסלול שחזור-לריפו חותך תיקייה עליונה יחידה גם כשיש קבצים חופשיים בשורש הארכיון (למשל README.md לצד src/), בניגוד ל-detect_zip_common_root ששימש קודם בגרסת ה-main ומנע בדיוק את זה. עבור ZIP שרירותי שהועלה לשחזור התוצאה היא פילוס מבנה הריפו. בדוק שהכרה בשורש נלקחת בחשבון גם קובץ חופשי בשורש שאינו metadata.json לפני החיתוך.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At handlers/documents.py, line 446:
<comment>החישוב החדש של `common_root` במסלול שחזור-לריפו חותך תיקייה עליונה יחידה גם כשיש קבצים חופשיים בשורש הארכיון (למשל `README.md` לצד `src/`), בניגוד ל-`detect_zip_common_root` ששימש קודם בגרסת ה-main ומנע בדיוק את זה. עבור ZIP שרירותי שהועלה לשחזור התוצאה היא פילוס מבנה הריפו. בדוק שהכרה בשורש נלקחת בחשבון גם קובץ חופשי בשורש שאינו `metadata.json` לפני החיתוך.</comment>
<file context>
@@ -477,7 +430,20 @@ async def _handle_github_restore_zip_to_repo(self, update: Update, context: Cont
+ for name in zf.namelist():
+ if "/" in name and not name.startswith("__MACOSX/"):
+ top_levels.add(name.split("/", 1)[0])
+ common_root = list(top_levels)[0] if len(top_levels) == 1 else None
def strip_root(path: str) -> str:
</file context>

Comment threaddocs/workflows/backup-flow.rst Outdated
Comment threaddocs/workflows/backup-flow.rst Outdated
הכפילות ב-backup-flow.rst היא שלי: חתכתי את הקובץ לפי index על
"יצירת גיבוי מלא", מחרוזת שמופיעה קודם בשורת ה-summary שבראש הקובץ.
נקודת החיתוך יצאה לפני נקודת ההתחלה, והפרוסה שכפלה את סקירה כללית,
סוגי גיבויים ומבנה ה-ZIP. הבלוק המשוכפל היה הגרסה הישנה של הסעיף,
שתיארה את zip_members_and_root — פונקציה שכבר לא קיימת בקוד — ולכן
התיעוד סתר את הקוד. נמחק, ונשארה גרסה אחת שתואמת את ההתנהגות.
והריוויו חשף מגבלה אמיתית בהתנהגות שהוחזרה: זיהוי השורש סופר תיקיות
בלבד, ולכן ZIP שאינו גיבוי — README.md בשורש לצד src/ — מאבד רובד.
עבור קובץ גיבוי זו בדיוק ההתנהגות הרצויה, ולכן היא נשמרת ומוסברת
למשתמש במסך שמבקש את הקובץ, במקום להשתנות.
הטקסט הוגדר כקבוע אחד: אותו callback מטופל בשני מקומות בקובץ, וטקסט
משוכפל היה מציג את ההערה רק באחד מהמסלולים.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbK6wRnQuBb9H9VC11n9NT

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_zip_to_folder.py">
<violation number="1" location="tests/test_zip_to_folder.py:1215">
P3: חומרת ממצא: 3/10 (נמוכה-בינונית). הבדיקה טוענת שהיא מקבעת את קיום ההתראה למשתמש ('and_the_user_is_warned', 'קיום ההתראה'), אבל היא רק בודקת שהקבוע RIPESTORE_ZIP_PROMPT מכיל שתי תת-מחרוזות. היא אינה מריצה את מסלול התצוגה בפועל (github_menu_handler שורות 2915/8940), ולכן אם יוסר הצגת ההתראה מהמסלול — הבדיקה תמשיך לעבור. כדי לאמת את הטענה יש לבדוק שההתראה נשלחת למשתמש במסלול, או לצמצם את טענת הבדיקה להצמדות לטקסט הקבוע בלבד.</violation>
</file>
<file name="github_menu_handler.py">
<violation number="1" location="github_menu_handler.py:2915">
P3: The RESTORE_ZIP_PROMPT edit at line 8940 sits in an unreachable copy of the `github_restore_zip_setpurge:` handler: `show_github_backup_menu` returns at line 8929, and the enclosing `if False and ...` guard can never fire. Only the live handler at line 2895 runs. The module comment (lines 158-165) justifies maintaining the constant 'בשני מקומות' (in two places) due to text duplication, but one of those places is dead code, so the claim is misleading. Remove the unreachable `if False` block and drop the dead editing; keep RESTORE_ZIP_PROMPT only at the live call site.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

# ההתראה חייבת להתקיים, אחרת המגבלה הופכת להפתעה שקטה
from github_menu_handler import RESTORE_ZIP_PROMPT

assert "עלול להיפרס שטוח" in RESTORE_ZIP_PROMPT, RESTORE_ZIP_PROMPT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: חומרת ממצא: 3/10 (נמוכה-בינונית). הבדיקה טוענת שהיא מקבעת את קיום ההתראה למשתמש ('and_the_user_is_warned', 'קיום ההתראה'), אבל היא רק בודקת שהקבוע RIPESTORE_ZIP_PROMPT מכיל שתי תת-מחרוזות. היא אינה מריצה את מסלול התצוגה בפועל (github_menu_handler שורות 2915/8940), ולכן אם יוסר הצגת ההתראה מהמסלול — הבדיקה תמשיך לעבור. כדי לאמת את הטענה יש לבדוק שההתראה נשלחת למשתמש במסלול, או לצמצם את טענת הבדיקה להצמדות לטקסט הקבוע בלבד.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_zip_to_folder.py, line 1215:
<comment>חומרת ממצא: 3/10 (נמוכה-בינונית). הבדיקה טוענת שהיא מקבעת את קיום ההתראה למשתמש ('and_the_user_is_warned', 'קיום ההתראה'), אבל היא רק בודקת שהקבוע RIPESTORE_ZIP_PROMPT מכיל שתי תת-מחרוזות. היא אינה מריצה את מסלול התצוגה בפועל (github_menu_handler שורות 2915/8940), ולכן אם יוסר הצגת ההתראה מהמסלול — הבדיקה תמשיך לעבור. כדי לאמת את הטענה יש לבדוק שההתראה נשלחת למשתמש במסלול, או לצמצם את טענת הבדיקה להצמדות לטקסט הקבוע בלבד.</comment>
<file context>
@@ -1183,3 +1183,34 @@ async def test_restore_keeps_nested_structure_when_there_is_no_single_root(
+ # ההתראה חייבת להתקיים, אחרת המגבלה הופכת להפתעה שקטה
+ from github_menu_handler import RESTORE_ZIP_PROMPT
+
+ assert "עלול להיפרס שטוח" in RESTORE_ZIP_PROMPT, RESTORE_ZIP_PROMPT
+ assert "קובץ גיבוי" in RESTORE_ZIP_PROMPT, RESTORE_ZIP_PROMPT
</file context>

("🧹 יבוצע ניקוי לפני העלאה. " if purge_flag else "🔁 ללא מחיקה. ") +
"שלח עכשיו קובץ ZIP לשחזור לריפו."
("🧹 יבוצע ניקוי לפני העלאה. " if purge_flag else "🔁 ללא מחיקה. ")
+ RESTORE_ZIP_PROMPT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The RESTORE_ZIP_PROMPT edit at line 8940 sits in an unreachable copy of the github_restore_zip_setpurge: handler: show_github_backup_menu returns at line 8929, and the enclosing if False and ... guard can never fire. Only the live handler at line 2895 runs. The module comment (lines 158-165) justifies maintaining the constant 'בשני מקומות' (in two places) due to text duplication, but one of those places is dead code, so the claim is misleading. Remove the unreachable if False block and drop the dead editing; keep RESTORE_ZIP_PROMPT only at the live call site.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At github_menu_handler.py, line 2915:
<comment>The RESTORE_ZIP_PROMPT edit at line 8940 sits in an unreachable copy of the `github_restore_zip_setpurge:` handler: `show_github_backup_menu` returns at line 8929, and the enclosing `if False and ...` guard can never fire. Only the live handler at line 2895 runs. The module comment (lines 158-165) justifies maintaining the constant 'בשני מקומות' (in two places) due to text duplication, but one of those places is dead code, so the claim is misleading. Remove the unreachable `if False` block and drop the dead editing; keep RESTORE_ZIP_PROMPT only at the live call site.</comment>
<file context>
@@ -2897,8 +2911,8 @@ def _upload_all_files():
- ("🧹 יבוצע ניקוי לפני העלאה. " if purge_flag else "🔁 ללא מחיקה. ") +
- "שלח עכשיו קובץ ZIP לשחזור לריפו."
+ ("🧹 יבוצע ניקוי לפני העלאה. " if purge_flag else "🔁 ללא מחיקה. ")
+ + RESTORE_ZIP_PROMPT
)
return
</file context>

@amirbiron
amirbiron merged commit 4b4da82 into mainSep 1, 2026
32 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@amirbiron@claude