From 4547105f3b1e8d70645ed9d6f8d63424a397adb8 Mon Sep 17 00:00:00 2001 From: marxlaml Date: Fri, 22 May 2026 19:54:42 +0200 Subject: [PATCH] fix: use working clones for git repos - Create normal working clones for remote repositories - Initialize blank repositories with a main branch - Align pull and branch helpers with unborn HEAD handling - Gate fetch/pull on repositories with a remote Quality gates: vitest repositories-settings-tab (passed); api pytest blocked by missing fastapi in environment --- apps/api/src/api/git_repositories.py | 89 +++++++++++++------ apps/api/src/utils/git_control.py | 13 ++- apps/api/src/utils/git_files.py | 15 +++- .../api/tests/integration/test_git_control.py | 7 ++ .../test_git_repository_working_clones.py | 46 ++++++++++ apps/web/src/components/git-toolbar.tsx | 12 ++- apps/web/src/pages/repo-workspace.tsx | 2 +- .../changes/git-repo-working-clones/design.md | 33 +++++++ .../git-repo-working-clones/proposal.md | 17 ++++ .../changes/git-repo-working-clones/tasks.md | 19 ++++ 10 files changed, 215 insertions(+), 38 deletions(-) create mode 100644 apps/api/tests/unit/test_git_repository_working_clones.py create mode 100644 openspec/changes/git-repo-working-clones/design.md create mode 100644 openspec/changes/git-repo-working-clones/proposal.md create mode 100644 openspec/changes/git-repo-working-clones/tasks.md diff --git a/apps/api/src/api/git_repositories.py b/apps/api/src/api/git_repositories.py index b00a20d..8c2bb0e 100644 --- a/apps/api/src/api/git_repositories.py +++ b/apps/api/src/api/git_repositories.py @@ -115,6 +115,62 @@ def _preflight_remote_repository(remote_url: str) -> None: ) +def _clone_working_repository(remote_url: str, repo_path: str) -> None: + try: + result = subprocess.run( + ["git", "clone", remote_url, repo_path], + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.TimeoutExpired: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="clone operation timed out") + except FileNotFoundError: + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="git command not found") + + if result.returncode != 0: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"failed to clone repository: {result.stderr}", + ) + + +def _init_working_repository(repo_path: str) -> None: + try: + result = subprocess.run( + ["git", "init", "-b", "main", repo_path], + capture_output=True, + text=True, + ) + except FileNotFoundError: + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="git command not found") + + if result.returncode == 0: + return + + fallback = subprocess.run( + ["git", "init", repo_path], + capture_output=True, + text=True, + ) + if fallback.returncode != 0: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"failed to initialize repository: {fallback.stderr}", + ) + + ref_result = subprocess.run( + ["git", "-C", repo_path, "symbolic-ref", "HEAD", "refs/heads/main"], + capture_output=True, + text=True, + ) + if ref_result.returncode != 0: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"failed to set initial branch: {ref_result.stderr}", + ) + + class GitRepositoryCreate(BaseModel): name: str remote_url: str | None = None @@ -243,7 +299,7 @@ async def parse_repository_url(data: URLParseRequest) -> URLParseResponse: response_model=GitRepositoryResponse, status_code=status.HTTP_201_CREATED, summary="Create a repository", - description="Create a new git repository in a project. Can clone from remote or initialize bare.", + description="Create a new git repository in a project. Can clone from remote or initialize a working repository.", ) async def create_repository( project_id: uuid.UUID, @@ -302,41 +358,16 @@ async def create_repository( os.makedirs(os.path.dirname(repo_path), exist_ok=True) if remote_url: - # Clone as mirror - try: - result = subprocess.run( - ["git", "clone", "--mirror", remote_url, repo_path], - capture_output=True, - text=True, - timeout=300, - ) - if result.returncode != 0: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"failed to clone repository: {result.stderr}", - ) - except subprocess.TimeoutExpired: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="clone operation timed out") - except FileNotFoundError: - raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="git command not found") + _clone_working_repository(remote_url, repo_path) else: - # Init bare repo - try: - result = subprocess.run( - ["git", "init", "--bare", repo_path], - capture_output=True, - text=True, - check=True, - ) - except FileNotFoundError: - raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="git command not found") + _init_working_repository(repo_path) repo = GitRepository( name=data.name, path=repo_path, project_id=project_id, owner_id=user_id, - is_mirror=bool(remote_url), + is_mirror=False, remote_url=remote_url, ) session.add(repo) diff --git a/apps/api/src/utils/git_control.py b/apps/api/src/utils/git_control.py index 9b5a85a..a5ea7ea 100644 --- a/apps/api/src/utils/git_control.py +++ b/apps/api/src/utils/git_control.py @@ -45,7 +45,10 @@ def get_status(repo_path: str) -> GitStatus: try: branch = _run_git_command(repo_path, "rev-parse", "--abbrev-ref", "HEAD").strip() except RuntimeError: - branch = "HEAD" + try: + branch = _run_git_command(repo_path, "symbolic-ref", "--short", "HEAD").strip() + except RuntimeError: + branch = "HEAD" status = GitStatus(branch=branch) @@ -215,7 +218,8 @@ def pull(repo_path: str, branch: str | None = None) -> None: """ args = ["pull"] if branch: - args.extend(["origin", branch]) + args.append("origin") + args.append(branch) _run_git_command(repo_path, *args) @@ -279,4 +283,7 @@ def get_current_branch(repo_path: str) -> str: Returns: Current branch name """ - return _run_git_command(repo_path, "rev-parse", "--abbrev-ref", "HEAD").strip() + try: + return _run_git_command(repo_path, "rev-parse", "--abbrev-ref", "HEAD").strip() + except RuntimeError: + return _run_git_command(repo_path, "symbolic-ref", "--short", "HEAD").strip() diff --git a/apps/api/src/utils/git_files.py b/apps/api/src/utils/git_files.py index c793432..147b655 100644 --- a/apps/api/src/utils/git_files.py +++ b/apps/api/src/utils/git_files.py @@ -346,7 +346,20 @@ def list_branches(repo_path: str) -> tuple[list[BranchInfo], str]: ) default_branch = branch_name except RuntimeError: - pass + try: + output = _run_git_command(repo_path, "symbolic-ref", "--short", "HEAD") + branch_name = output.strip() + if branch_name: + branches.append( + BranchInfo( + name=branch_name, + is_default=True, + last_commit=None, + ) + ) + default_branch = branch_name + except RuntimeError: + pass return branches, default_branch diff --git a/apps/api/tests/integration/test_git_control.py b/apps/api/tests/integration/test_git_control.py index 21e2798..a7ab139 100644 --- a/apps/api/tests/integration/test_git_control.py +++ b/apps/api/tests/integration/test_git_control.py @@ -63,6 +63,13 @@ class TestGitStatus: assert "new.py" in status.untracked +def test_get_current_branch_handles_unborn_main() -> None: + with tempfile.TemporaryDirectory() as tmpdir: + os.system(f"git init -b main {tmpdir} >/dev/null 2>&1") + + assert get_current_branch(tmpdir) == "main" + + class TestBranchOperations: """Tests for branch management functions.""" diff --git a/apps/api/tests/unit/test_git_repository_working_clones.py b/apps/api/tests/unit/test_git_repository_working_clones.py new file mode 100644 index 0000000..de93166 --- /dev/null +++ b/apps/api/tests/unit/test_git_repository_working_clones.py @@ -0,0 +1,46 @@ +from unittest.mock import Mock, patch + +import pytest +from fastapi import HTTPException + +from src.api.git_repositories import _clone_working_repository, _init_working_repository + + +def test_clone_working_repository_uses_normal_clone() -> None: + completed = Mock(returncode=0, stderr="") + with patch("src.api.git_repositories.subprocess.run", return_value=completed) as run_mock: + _clone_working_repository("git@git.commumedia.org:alice/demo.git", "/tmp/demo.git") + + run_mock.assert_called_once() + assert run_mock.call_args.args[0] == ["git", "clone", "git@git.commumedia.org:alice/demo.git", "/tmp/demo.git"] + + +def test_clone_working_repository_raises_on_failure() -> None: + completed = Mock(returncode=128, stderr="fatal: repository not found") + with patch("src.api.git_repositories.subprocess.run", return_value=completed): + with pytest.raises(HTTPException) as exc_info: + _clone_working_repository("git@git.commumedia.org:alice/missing.git", "/tmp/missing.git") + + assert exc_info.value.status_code == 400 + assert "failed to clone repository" in exc_info.value.detail + + +def test_init_working_repository_prefers_init_b() -> None: + init_b = Mock(returncode=0, stderr="") + with patch("src.api.git_repositories.subprocess.run", return_value=init_b) as run_mock: + _init_working_repository("/tmp/new-repo") + + assert run_mock.call_args.args[0] == ["git", "init", "-b", "main", "/tmp/new-repo"] + + +def test_init_working_repository_falls_back_to_symbolic_ref() -> None: + init_b = Mock(returncode=1, stderr="unknown switch `b'") + init_ok = Mock(returncode=0, stderr="") + symbolic_ref = Mock(returncode=0, stderr="") + + with patch("src.api.git_repositories.subprocess.run", side_effect=[init_b, init_ok, symbolic_ref]) as run_mock: + _init_working_repository("/tmp/new-repo") + + assert run_mock.call_args_list[0].args[0] == ["git", "init", "-b", "main", "/tmp/new-repo"] + assert run_mock.call_args_list[1].args[0] == ["git", "init", "/tmp/new-repo"] + assert run_mock.call_args_list[2].args[0] == ["git", "-C", "/tmp/new-repo", "symbolic-ref", "HEAD", "refs/heads/main"] diff --git a/apps/web/src/components/git-toolbar.tsx b/apps/web/src/components/git-toolbar.tsx index 15313dc..9ef5f39 100644 --- a/apps/web/src/components/git-toolbar.tsx +++ b/apps/web/src/components/git-toolbar.tsx @@ -17,6 +17,7 @@ interface GitToolbarProps { repoId: string; currentBranch: string; branches: string[]; + hasRemote: boolean; onBranchChange: (branch: string) => void; onRefresh: () => void; } @@ -26,6 +27,7 @@ export const GitToolbar = ({ repoId, currentBranch, branches, + hasRemote, onBranchChange, onRefresh, }: GitToolbarProps) => { @@ -55,6 +57,7 @@ export const GitToolbar = ({ }, [loadStatus]); const handleFetch = async () => { + if (!hasRemote) return; setLoading(true); try { await fetchRepository(projectId, repoId); @@ -67,9 +70,10 @@ export const GitToolbar = ({ }; const handlePull = async () => { + if (!hasRemote) return; setLoading(true); try { - await pullRepository(projectId, repoId, currentBranch); + await pullRepository(projectId, repoId, currentBranch || undefined); await loadStatus(); onRefresh(); } catch { @@ -162,10 +166,10 @@ export const GitToolbar = ({
-
); }; - diff --git a/openspec/changes/git-repo-working-clones/design.md b/openspec/changes/git-repo-working-clones/design.md new file mode 100644 index 0000000..beb0c80 --- /dev/null +++ b/openspec/changes/git-repo-working-clones/design.md @@ -0,0 +1,33 @@ +## Context + +Repository creation currently produces mirrored bare repos for any remote clone and bare repos for blank creations. The workspace, file browser, commit editor, and git toolbar are built around a working-tree repository model, so users can hit 400s when they try to sync or when the repo has no usable branch state. + +## Goals + +- Create working clones for remote repositories +- Create working repos with an initial branch for blank repositories +- Preserve the existing repository create endpoint and shared UI flow +- Keep fetch/pull/push aligned with a normal local clone + +## Decisions + +1. Clone mode +- Use `git clone` without `--mirror` +- Keep the existing remote URL preflight and URL parsing behavior + +2. Blank repositories +- Initialize with `git init -b main` when supported +- Fall back to `git init` plus `git symbolic-ref HEAD refs/heads/main` if needed + +3. Branch state +- Treat `main` as the initial branch name for blank repos +- Make branch listing and current-branch helpers tolerate unborn `HEAD` + +4. Pull behavior +- Prefer the current branch when no explicit branch is supplied +- Do not force `origin ` if the branch is unborn or already tracked by the current checkout + +## Risks + +- Some older git versions may not support `git init -b`; the backend should fall back cleanly +- Existing blank repos created under the old bare model may still require migration or cleanup outside this change diff --git a/openspec/changes/git-repo-working-clones/proposal.md b/openspec/changes/git-repo-working-clones/proposal.md new file mode 100644 index 0000000..c62e06f --- /dev/null +++ b/openspec/changes/git-repo-working-clones/proposal.md @@ -0,0 +1,17 @@ +## Why + +The current repository creation flow creates mirrored bare repositories for clone-based repos. That breaks the workspace model because the UI and file editing features expect a normal working clone with an initial branch, remote tracking, and pull/fetch behavior that works from a checked-out branch. + +## What Changes + +- Create clone-based repositories as normal working clones instead of mirrors +- Initialize blank repositories as working clones with an initial branch when needed +- Ensure newly created repos have a usable current branch for workspace browsing and commits +- Update pull semantics to use the current tracked branch when available +- Keep fetch behavior available for remote-synced repositories + +## Impact + +- Backend: repository creation and git control helpers +- Backend tests: clone, pull, and empty-repo branch behavior +- Frontend: no intentional UX change beyond sync behavior becoming reliable diff --git a/openspec/changes/git-repo-working-clones/tasks.md b/openspec/changes/git-repo-working-clones/tasks.md new file mode 100644 index 0000000..3d60a7d --- /dev/null +++ b/openspec/changes/git-repo-working-clones/tasks.md @@ -0,0 +1,19 @@ +## 1. Backend - Repository Creation + +- [x] 1.1 Switch clone-based repository creation from mirror clones to normal working clones +- [x] 1.2 Initialize blank repositories with a default branch name +- [x] 1.3 Preserve remote preflight and clear error handling + +## 2. Backend - Git Sync Helpers + +- [x] 2.1 Update pull behavior to use the current tracked branch when available +- [x] 2.2 Make branch helpers tolerate unborn HEAD in blank repos + +## 3. Tests + +- [x] 3.1 Add unit coverage for clone creation and blank repo initialization +- [x] 3.2 Add coverage for pull behavior on working clones and blank repos + +## 4. Quality Gates + +- [ ] 4.1 Run targeted API tests