fix(sync): strip .git/ from cloned repos to fix gitlink bug#720
fix(sync): strip .git/ from cloned repos to fix gitlink bug#720shashank-poola wants to merge 4 commits intogetnao:mainfrom
Conversation
|
This PR was auto-closed. Only contributors approved with Maintainers review auto-closed issues daily. Issues that do not meet the quality bar in CONTRIBUTING.md will not be reopened or receive a reply. If a maintainer replies See CONTRIBUTING.md. |
There was a problem hiding this comment.
2 issues found across 2 files
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="cli/nao_core/commands/sync/providers/repositories/provider.py">
<violation number="1" location="cli/nao_core/commands/sync/providers/repositories/provider.py:27">
P1: Unvalidated `repo.name` is used to build the deletion target, so `shutil.rmtree()` can delete outside the sync root via `../` or absolute paths.</violation>
<violation number="2" location="cli/nao_core/commands/sync/providers/repositories/provider.py:27">
P1: Non-atomic re-clone flow deletes existing repo before verifying clone succeeds, causing data loss on transient failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🚀 Preview Deployment
Preview will be automatically removed when this PR is closed. |
There was a problem hiding this comment.
1 issue found across 1 file (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="cli/nao_core/commands/sync/providers/repositories/provider.py">
<violation number="1" location="cli/nao_core/commands/sync/providers/repositories/provider.py:25">
P2: Path resolution for the new traversal guard happens outside the function's existing error handling, so a resolve failure can abort the whole sync instead of failing just this repo.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Closes #716
Problem
When
nao syncclones a git repo intorepos/<name>/, it leaves a.git/folder inside. Git sees this nested.git/and treats the whole directory as a gitlink (mode 160000) instead of regular tracked files.So when you commit and push the context repo, Git only stores a SHA pointer, not the actual file content. Anyone who clones that repo after you, a teammate, CI pipeline, or the deployed agent, gets an empty folder, silently, with no errors.
The files only ever exist on the machine that ran
nao sync.# Before this fix, git was storing a pointer, not files $ git ls-tree -r HEAD repos/ 160000 commit bf48ed0a... repos/dbtWhat We Changed
cli/nao_core/commands/sync/providers/repositories/provider.pyshutil.rmtree(repo_path / ".git")to strip the nested Git reporepos/<name>/becomes a plain folder,git addnow commits actual file content, not a SHA pointer.git/no longer exists after first sync,git pullon re-sync is not possible, changed to always re-clone fresh (delete → clone → strip)cli/nao_core/commands/sync/cleanup.py(bonus fix)cleanup_stale_reposcalledbase_path.iterdir()beforerepos/existed on first sync[Errno 2] No such file or directory: 'repos'if not base_path.exists(): returnProof
Impact
repos/doesn't exist yetrepos/<name>/are now portable: committable, pushable, and readable by the deployed agent