fix: align git mount implementation with spec
- _checkout_branch now returns bool and falls back gracefully on failure - Glob warning message includes matched file count - Fix database model comment to reference remote_url - Update tests for new branch checkout behavior All 51 tests pass
This commit is contained in:
@@ -143,11 +143,14 @@ async def _resolve_single_git_mount(
|
||||
|
||||
# Handle branch checkout if specified
|
||||
if branch and repo_path:
|
||||
try:
|
||||
await asyncio.to_thread(_checkout_branch, repo_path, branch)
|
||||
success = await asyncio.to_thread(_checkout_branch, repo_path, branch)
|
||||
if success:
|
||||
logger.info("Checked out branch %s for %s", branch, remote_url)
|
||||
except Exception as exc:
|
||||
logger.warning("Branch checkout failed for %s@%s: %s", remote_url, branch, exc)
|
||||
else:
|
||||
logger.warning(
|
||||
"Branch %s not found in %s, using current branch",
|
||||
branch, remote_url
|
||||
)
|
||||
|
||||
# Build source path and expand globs
|
||||
if source_path and source_path != ".":
|
||||
@@ -186,8 +189,12 @@ async def _resolve_single_git_mount(
|
||||
return volume_mounts
|
||||
|
||||
|
||||
def _checkout_branch(repo_path: str, branch: str) -> None:
|
||||
"""Checkout a specific branch in a git repository."""
|
||||
def _checkout_branch(repo_path: str, branch: str) -> bool:
|
||||
"""Checkout a specific branch in a git repository.
|
||||
|
||||
Returns True if checkout succeeded, False if it failed.
|
||||
On failure, the repository remains on its current branch.
|
||||
"""
|
||||
import subprocess
|
||||
|
||||
# First try to checkout existing branch
|
||||
@@ -211,7 +218,10 @@ def _checkout_branch(repo_path: str, branch: str) -> None:
|
||||
)
|
||||
|
||||
if result.returncode != 0:
|
||||
raise RuntimeError(f"Failed to checkout branch {branch}: {result.stderr}")
|
||||
logger.warning("Failed to checkout branch %s in %s: %s", branch, repo_path, result.stderr.strip())
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
|
||||
def _pull_repository_updates(repo_path: str, remote_url: str) -> None:
|
||||
@@ -258,6 +268,7 @@ def _expand_glob_source(source_path: str, repo_path: str) -> list[str]:
|
||||
|
||||
# Expand glob pattern
|
||||
matched = glob_module.glob(source_path, recursive=True)
|
||||
total_matched = len(matched)
|
||||
|
||||
# Filter to only paths within the repo and limit count
|
||||
results = []
|
||||
@@ -266,7 +277,7 @@ def _expand_glob_source(source_path: str, repo_path: str) -> list[str]:
|
||||
if abs_path.startswith(os.path.abspath(repo_path)):
|
||||
results.append(abs_path)
|
||||
if len(results) >= MAX_GLOB_MATCHES:
|
||||
logger.warning("Glob pattern matched too many files, limiting to %d", MAX_GLOB_MATCHES)
|
||||
logger.warning("Glob pattern matched %d files, limited to %d", total_matched, MAX_GLOB_MATCHES)
|
||||
break
|
||||
|
||||
return results
|
||||
|
||||
@@ -41,7 +41,7 @@ class ConfigProfile(UUIDPrimaryKeyMixin, TimestampMixin, Base):
|
||||
) # {"rel/path": "content", ...}
|
||||
git_mounts: Mapped[list] = mapped_column(
|
||||
JSON, default=list, nullable=False
|
||||
) # [{"repo_id": "uuid", "source_path": ".", "target_path": "/path", "branch": "main"}, ...]
|
||||
) # [{"remote_url": "https://github.com/user/repo.git", "source_path": ".", "target_path": "/path", "branch": "main"}, ...]
|
||||
is_default: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False)
|
||||
|
||||
user: Mapped["User"] = relationship()
|
||||
|
||||
@@ -91,23 +91,22 @@ class TestCheckoutBranch:
|
||||
assert result == "feature"
|
||||
|
||||
def test_checkout_nonexistent_branch(self, tmp_path: Path) -> None:
|
||||
"""Test checking out a non-existent branch raises error."""
|
||||
"""Test checking out a non-existent branch returns False."""
|
||||
os.system(f"cd {tmp_path} && git init && git config user.email 'test@test.com' && git config user.name 'Test'")
|
||||
(tmp_path / "file.txt").write_text("content")
|
||||
os.system(f"cd {tmp_path} && git add . && git commit -m 'initial'")
|
||||
|
||||
with pytest.raises(RuntimeError, match="Failed to checkout branch"):
|
||||
_checkout_branch(str(tmp_path), "nonexistent")
|
||||
result = _checkout_branch(str(tmp_path), "nonexistent")
|
||||
assert result is False
|
||||
|
||||
|
||||
class TestResolveSingleGitMount:
|
||||
"""Unit tests for resolving a single git mount."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_missing_repo(self, db_session) -> None:
|
||||
"""Test that missing repo returns empty list."""
|
||||
async def test_resolve_missing_remote_url(self, db_session) -> None:
|
||||
"""Test that missing remote_url returns empty list."""
|
||||
git_mount = {
|
||||
"repo_id": "12345678-1234-1234-1234-123456789abc",
|
||||
"source_path": ".",
|
||||
"target_path": "/app",
|
||||
}
|
||||
@@ -119,21 +118,9 @@ class TestResolveSingleGitMount:
|
||||
async def test_resolve_missing_target_path(self, db_session) -> None:
|
||||
"""Test that missing target path returns empty list."""
|
||||
git_mount = {
|
||||
"repo_id": "12345678-1234-1234-1234-123456789abc",
|
||||
"remote_url": "https://github.com/user/repo.git",
|
||||
"source_path": ".",
|
||||
}
|
||||
|
||||
result = await _resolve_single_git_mount(db_session, git_mount)
|
||||
assert result == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_invalid_repo_id(self, db_session) -> None:
|
||||
"""Test that invalid repo_id returns empty list."""
|
||||
git_mount = {
|
||||
"repo_id": "not-a-uuid",
|
||||
"source_path": ".",
|
||||
"target_path": "/app",
|
||||
}
|
||||
|
||||
result = await _resolve_single_git_mount(db_session, git_mount)
|
||||
assert result == []
|
||||
|
||||
Reference in New Issue
Block a user