From fb28678b641726ec8a2e35e73672d62d6623a45e Mon Sep 17 00:00:00 2001 From: longle325 Date: Sat, 18 Apr 2026 23:16:04 +0700 Subject: [PATCH] fix: prevent GitStore from creating nested repos and overwriting .gitignore (#2980) GitStore.init() now checks if the workspace is already inside a git repository before calling porcelain.init(). If so, it refuses to create a nested repo. Additionally, existing .gitignore files are preserved by appending only missing Dream-specific entries rather than overwriting. Closes #2980 --- .gitignore | 1 + nanobot/utils/gitstore.py | 38 +++++++++++++++- tests/utils/test_gitstore.py | 85 +++++++++++++++++++++++++++++++++++- 3 files changed, 121 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 054e5ce70..18d4df7ea 100644 --- a/.gitignore +++ b/.gitignore @@ -92,3 +92,4 @@ logs/ tmp/ temp/ *.tmp +.oss/ diff --git a/nanobot/utils/gitstore.py b/nanobot/utils/gitstore.py index e51a63cc3..ffa241e77 100644 --- a/nanobot/utils/gitstore.py +++ b/nanobot/utils/gitstore.py @@ -64,14 +64,35 @@ class GitStore: if self.is_initialized(): return False + if self._is_inside_git_repo(): + logger.warning( + "Workspace {} is already inside a git repo; " + "skipping nested repo initialization", + self._workspace, + ) + return False + try: from dulwich import porcelain porcelain.init(str(self._workspace)) - # Write .gitignore + # Write .gitignore (merge with existing if present) gitignore = self._workspace / ".gitignore" - gitignore.write_text(self._build_gitignore(), encoding="utf-8") + dream_entries = self._build_gitignore() + if gitignore.exists(): + existing = gitignore.read_text(encoding="utf-8") + existing_lines = set(existing.splitlines()) + new_lines = [ + line + for line in dream_entries.splitlines() + if line not in existing_lines + ] + if new_lines: + merged = existing.rstrip("\n") + "\n" + "\n".join(new_lines) + "\n" + gitignore.write_text(merged, encoding="utf-8") + else: + gitignore.write_text(dream_entries, encoding="utf-8") # Ensure tracked files exist (touch them if missing) so the initial # commit has something to track. @@ -155,6 +176,19 @@ class GitStore: except Exception: return None + def _is_inside_git_repo(self) -> bool: + """Check if self._workspace is already inside a git repository. + + Walks up from self._workspace to the filesystem root, returning True + if any parent directory contains a .git directory. + """ + current = self._workspace.resolve() + while current != current.parent: + if (current / ".git").is_dir(): + return True + current = current.parent + return False + def _build_gitignore(self) -> str: """Generate .gitignore content from tracked files.""" dirs: set[str] = set() diff --git a/tests/utils/test_gitstore.py b/tests/utils/test_gitstore.py index 8c401e389..b7ee0ef29 100644 --- a/tests/utils/test_gitstore.py +++ b/tests/utils/test_gitstore.py @@ -1,7 +1,7 @@ """Tests for GitStore — line_ages() and core git operations.""" import time -from datetime import datetime, timezone, timedelta +from datetime import datetime, timedelta, timezone from unittest.mock import patch import pytest @@ -89,3 +89,86 @@ class TestLineAges: # "- new" line and "- keep" line both age=0 (same day), but # the key point is we get per-line results assert len(ages) == 7 + + +class TestNestedRepoProtection: + """Regression tests for GitHub issue #2980: nested repo protection.""" + + def test_init_refuses_inside_git_repo(self, tmp_path): + """init() should detect it's inside an existing git repo and refuse.""" + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + + workspace = project / "workspace" + workspace.mkdir() + + g = GitStore(workspace, tracked_files=["MEMORY.md"]) + result = g.init() + + assert result is False + assert not (workspace / ".git").is_dir() + + def test_init_preserves_existing_gitignore(self, tmp_path): + """init() should preserve existing .gitignore entries and append new ones.""" + workspace = tmp_path / "workspace" + workspace.mkdir() + + existing = "*.pyc\n__pycache__/\n" + (workspace / ".gitignore").write_text(existing, encoding="utf-8") + + g = GitStore(workspace, tracked_files=["MEMORY.md"]) + result = g.init() + + assert result is True + gitignore = (workspace / ".gitignore").read_text(encoding="utf-8") + assert "*.pyc" in gitignore + assert "__pycache__/" in gitignore + assert "!MEMORY.md" in gitignore + assert "!.gitignore" in gitignore + + def test_init_no_gitignore_creates_new(self, tmp_path): + """init() should create .gitignore with Dream content when none exists.""" + workspace = tmp_path / "workspace" + workspace.mkdir() + + g = GitStore(workspace, tracked_files=["MEMORY.md"]) + result = g.init() + + assert result is True + gitignore = (workspace / ".gitignore").read_text(encoding="utf-8") + expected = g._build_gitignore() + assert gitignore == expected + + def test_init_gitignore_merge_idempotent(self, tmp_path): + """init() should not duplicate Dream entries already in .gitignore.""" + workspace = tmp_path / "workspace" + workspace.mkdir() + + # Pre-existing .gitignore that already has some Dream entries + existing = "*.pyc\n/*\n!MEMORY.md\n" + (workspace / ".gitignore").write_text(existing, encoding="utf-8") + + g = GitStore(workspace, tracked_files=["MEMORY.md"]) + result = g.init() + + assert result is True + gitignore = (workspace / ".gitignore").read_text(encoding="utf-8") + # No duplicate lines + lines = gitignore.splitlines() + assert lines.count("/*") == 1 + assert lines.count("!MEMORY.md") == 1 + # Existing entry preserved, new Dream entries appended + assert "*.pyc" in gitignore + assert "!.gitignore" in gitignore + + def test_init_outside_git_repo_works_normally(self, tmp_path): + """init() should succeed and create .git when not inside a git repo.""" + workspace = tmp_path / "workspace" + workspace.mkdir() + + g = GitStore(workspace, tracked_files=["MEMORY.md"]) + result = g.init() + + assert result is True + assert (workspace / ".git").is_dir()