diff --git a/devlaunch/worktree/branch_manager.py b/devlaunch/worktree/branch_manager.py index 1242979f..dc61b7bb 100644 --- a/devlaunch/worktree/branch_manager.py +++ b/devlaunch/worktree/branch_manager.py @@ -21,13 +21,25 @@ def ensure_branch_exists( remote: str = "origin", create_remote: bool = True, ssh_key_path: Optional[str] = None, + start_point: str = "HEAD", + use_local_refs: bool = False, ) -> None: - """Ensure branch exists locally and optionally remotely.""" + """Ensure branch exists locally and optionally remotely. + + Args: + use_local_refs: When True, infer remote branch existence from local + refs instead of calling ``git ls-remote``. Safe in bare repos + whose refspec maps remote heads to local heads (the default + ``+refs/heads/*:refs/heads/*``). + """ # Check if branch exists locally local_exists = self.local_branch_exists(base_repo_path, branch) # Check if branch exists remotely - remote_exists = self.remote_branch_exists(base_repo_path, branch, remote) + if use_local_refs: + remote_exists = local_exists + else: + remote_exists = self.remote_branch_exists(base_repo_path, branch, remote) if local_exists and remote_exists: logger.info(f"Branch {branch} already exists locally and remotely") @@ -42,7 +54,7 @@ def ensure_branch_exists( if not local_exists: # Create new local branch - self.create_local_branch(base_repo_path, branch) + self.create_local_branch(base_repo_path, branch, start_point) logger.info(f"Created local branch {branch}") if not remote_exists and create_remote: diff --git a/devlaunch/worktree/repo_manager.py b/devlaunch/worktree/repo_manager.py index 51c56431..281edfc6 100644 --- a/devlaunch/worktree/repo_manager.py +++ b/devlaunch/worktree/repo_manager.py @@ -146,6 +146,20 @@ def _should_fetch(self, repo: BaseRepository) -> bool: elapsed = (datetime.now() - repo.last_fetched).total_seconds() return elapsed > self.fetch_interval + def lazy_fetch(self, owner: str, repo: str) -> bool: + """Fetch only if the fetch interval has elapsed since the last fetch. + + Returns True if a fetch was performed, False if skipped. + Raises ValueError if the repository is not in metadata. + """ + base_repo = self.storage.get_repository(owner, repo) + if not base_repo: + raise ValueError(f"Repository {owner}/{repo} not found in metadata") + if self._should_fetch(base_repo): + self.fetch_repo(owner, repo) + return True + return False + def ensure_repo( self, owner: str, repo: str, remote_url: str, auto_fetch: bool = True ) -> BaseRepository: diff --git a/devlaunch/worktree/workspace_clone.py b/devlaunch/worktree/workspace_clone.py index f8298eed..2d61e5d3 100644 --- a/devlaunch/worktree/workspace_clone.py +++ b/devlaunch/worktree/workspace_clone.py @@ -108,12 +108,20 @@ def ensure_branch(self, owner: str, repo: str, branch: str) -> None: locally if needed. Does not push to the remote. """ bare_path = self.repo_manager.get_bare_path(owner, repo) - # Fetch first to have latest refs + # Lazy-fetch: only hits the network when the fetch interval has elapsed try: - self.repo_manager.fetch_repo(owner, repo) + self.repo_manager.lazy_fetch(owner, repo) except Exception as e: logger.warning(f"Failed to fetch before branch ensure: {e}") - self.branch_manager.ensure_branch_exists(bare_path, branch, create_remote=False) + + default_branch = self.repo_manager.get_default_branch(owner, repo) + self.branch_manager.ensure_branch_exists( + bare_path, + branch, + create_remote=False, + start_point=default_branch, + use_local_refs=True, + ) def ensure_workspace( self, @@ -141,6 +149,7 @@ def ensure_workspace( ws_path = self.get_workspace_path(owner, repo, branch) is_new_workspace = False + is_new_workspace = False if not self.workspace_exists(owner, repo, branch): is_new_workspace = True # Step 2: Clone from bare repo @@ -173,17 +182,19 @@ def ensure_workspace( logger.error(f"Failed to set remote URL: {e.stderr}") raise RuntimeError(f"Failed to set remote URL: {e.stderr}") from e - # Step 4: Fetch from origin to get all remote branches - try: - subprocess.run( - ["git", "fetch", "origin"], - cwd=ws_path, - capture_output=True, - text=True, - check=True, - ) - except subprocess.CalledProcessError as e: - logger.warning(f"Failed to fetch in workspace: {e.stderr}") + # Step 4: Fetch from origin — skip for newly-created workspaces since + # they were just cloned from a freshly-fetched bare repo. + if not is_new_workspace: + try: + subprocess.run( + ["git", "fetch", "origin"], + cwd=ws_path, + capture_output=True, + text=True, + check=True, + ) + except subprocess.CalledProcessError as e: + logger.warning(f"Failed to fetch in workspace: {e.stderr}") # Step 5: Checkout branch try: diff --git a/test/test_workspace_clone.py b/test/test_workspace_clone.py index 80fe6a86..09f5a161 100644 --- a/test/test_workspace_clone.py +++ b/test/test_workspace_clone.py @@ -121,15 +121,21 @@ class TestEnsureBranch: def test_fetches_then_ensures( self, clone_manager, mock_repo_manager, mock_branch_manager, tmp_repos_dir ): - """Test that ensure_branch fetches then delegates to BranchManager.""" + """Test that ensure_branch lazy-fetches then delegates to BranchManager.""" bare_path = tmp_repos_dir / "owner" / "repo" / ".bare" mock_repo_manager.get_bare_path.return_value = bare_path + mock_repo_manager.get_default_branch.return_value = "main" clone_manager.ensure_branch("owner", "repo", "newbranch") - mock_repo_manager.fetch_repo.assert_called_once_with("owner", "repo") + mock_repo_manager.lazy_fetch.assert_called_once_with("owner", "repo") + mock_repo_manager.get_default_branch.assert_called_once_with("owner", "repo") mock_branch_manager.ensure_branch_exists.assert_called_once_with( - bare_path, "newbranch", create_remote=False + bare_path, + "newbranch", + create_remote=False, + start_point="main", + use_local_refs=True, ) def test_continues_if_fetch_fails( @@ -138,12 +144,18 @@ def test_continues_if_fetch_fails( """Test that ensure_branch continues even if fetch fails.""" bare_path = tmp_repos_dir / "owner" / "repo" / ".bare" mock_repo_manager.get_bare_path.return_value = bare_path - mock_repo_manager.fetch_repo.side_effect = RuntimeError("network error") + mock_repo_manager.lazy_fetch.side_effect = RuntimeError("network error") + mock_repo_manager.get_default_branch.return_value = "main" clone_manager.ensure_branch("owner", "repo", "newbranch") + mock_repo_manager.get_default_branch.assert_called_once_with("owner", "repo") mock_branch_manager.ensure_branch_exists.assert_called_once_with( - bare_path, "newbranch", create_remote=False + bare_path, + "newbranch", + create_remote=False, + start_point="main", + use_local_refs=True, ) @@ -154,7 +166,11 @@ class TestEnsureWorkspace: def test_clones_from_bare_repo( self, mock_run, clone_manager, mock_repo_manager, mock_storage, tmp_repos_dir ): - """Test that a new workspace is cloned from the bare repo.""" + """Test that a new workspace is cloned from the bare repo. + + Newly-created workspaces skip ``git fetch origin`` because they were + just cloned from a freshly-fetched bare repo. + """ mock_run.return_value = MagicMock(returncode=0) repo_root = tmp_repos_dir / "owner" / "repo" bare_path = repo_root / ".bare" @@ -167,9 +183,9 @@ def test_clones_from_bare_repo( "owner", "repo", "nb4", "git@github.com:owner/repo.git", "repo-nb4" ) - # Should have called: git clone, git remote set-url, git fetch, - # git show-ref (remote ref check), git checkout -B - assert mock_run.call_count == 5 + # Should have called: git clone, git remote set-url, + # git show-ref (remote ref check), git checkout -B (no fetch) + assert mock_run.call_count == 4 # First call: git clone from bare repo clone_call = mock_run.call_args_list[0] @@ -185,12 +201,8 @@ def test_clones_from_bare_repo( "git@github.com:owner/repo.git", ] - # Third call: fetch origin - fetch_call = mock_run.call_args_list[2] - assert fetch_call[0][0] == ["git", "fetch", "origin"] - - # Fourth call: show-ref to check remote branch (returns 0 = exists) - showref_call = mock_run.call_args_list[3] + # Third call: show-ref to check remote branch (returns 0 = exists) + showref_call = mock_run.call_args_list[2] assert showref_call[0][0] == [ "git", "show-ref", @@ -198,8 +210,8 @@ def test_clones_from_bare_repo( "refs/remotes/origin/nb4", ] - # Fifth call: checkout -B from remote ref - checkout_call = mock_run.call_args_list[4] + # Fourth call: checkout -B from remote ref (no fetch for new workspaces) + checkout_call = mock_run.call_args_list[3] assert checkout_call[0][0] == ["git", "checkout", "-B", "nb4", "origin/nb4"] # Should track in metadata @@ -362,6 +374,46 @@ def test_returns_workspace_path( assert result == repo_root / "main" + @patch("devlaunch.worktree.workspace_clone.subprocess.run") + def test_newly_created_workspace_skips_fetch( + self, mock_run, clone_manager, mock_repo_manager, tmp_repos_dir + ): + """Newly-created workspaces must not run ``git fetch origin``.""" + mock_run.return_value = MagicMock(returncode=0) + repo_root = tmp_repos_dir / "owner" / "repo" + mock_repo_manager.get_repo_path.return_value = repo_root + mock_repo_manager.get_bare_path.return_value = repo_root / ".bare" + + clone_manager.ensure_workspace( + "owner", "repo", "nb4", "git@github.com:owner/repo.git", "repo-nb4" + ) + + # No call should contain "git fetch origin" + fetch_calls = [c for c in mock_run.call_args_list if c[0][0] == ["git", "fetch", "origin"]] + assert fetch_calls == [] + + @patch("devlaunch.worktree.workspace_clone.subprocess.run") + def test_existing_workspace_still_fetches( + self, mock_run, clone_manager, mock_repo_manager, tmp_repos_dir + ): + """Existing (stale) workspaces should still run ``git fetch origin``.""" + repo_root = tmp_repos_dir / "owner" / "repo" + mock_repo_manager.get_repo_path.return_value = repo_root + + # Create existing workspace + ws_path = repo_root / "nb4" + ws_path.mkdir(parents=True) + (ws_path / ".git").mkdir() + + mock_run.return_value = MagicMock(returncode=0) + + clone_manager.ensure_workspace( + "owner", "repo", "nb4", "git@github.com:owner/repo.git", "repo-nb4" + ) + + fetch_calls = [c for c in mock_run.call_args_list if c[0][0] == ["git", "fetch", "origin"]] + assert len(fetch_calls) == 1 + class TestRemoveWorkspace: """Tests for remove_workspace.""" diff --git a/test/test_worktree_branch_manager.py b/test/test_worktree_branch_manager.py index af057cce..7e3b5bc5 100644 --- a/test/test_worktree_branch_manager.py +++ b/test/test_worktree_branch_manager.py @@ -1,5 +1,5 @@ """Tests for worktree branch manager.""" -# pylint: disable=redefined-outer-name +# pylint: disable=redefined-outer-name,unused-argument import subprocess import tempfile @@ -312,7 +312,7 @@ def test_branch_does_not_exist( branch_manager.ensure_branch_exists(temp_repo, "new-branch") - mock_create.assert_called_once_with(temp_repo, "new-branch") + mock_create.assert_called_once_with(temp_repo, "new-branch", "HEAD") mock_push.assert_called_once() mock_track.assert_called_once() @@ -328,7 +328,93 @@ def test_branch_no_create_remote( branch_manager.ensure_branch_exists(temp_repo, "new-branch", create_remote=False) - mock_create.assert_called_once() + mock_create.assert_called_once_with(temp_repo, "new-branch", "HEAD") + + @patch.object(BranchManager, "local_branch_exists") + @patch.object(BranchManager, "remote_branch_exists") + @patch.object(BranchManager, "create_local_branch") + @patch.object(BranchManager, "track_remote_branch") + def test_branch_custom_start_point( + self, + mock_track, + mock_create, + mock_remote_exists, + mock_local_exists, + branch_manager, + temp_repo, + ): + """Test ensure_branch_exists passes custom start_point to create_local_branch.""" + mock_local_exists.return_value = False + mock_remote_exists.return_value = False + + branch_manager.ensure_branch_exists( + temp_repo, "new-branch", create_remote=False, start_point="origin/main" + ) + + mock_create.assert_called_once_with(temp_repo, "new-branch", "origin/main") + + +class TestEnsureBranchExistsUseLocalRefs: + """Tests for ensure_branch_exists with use_local_refs=True.""" + + @patch.object(BranchManager, "local_branch_exists") + @patch.object(BranchManager, "remote_branch_exists") + def test_use_local_refs_skips_ls_remote_when_branch_exists( + self, mock_remote_exists, mock_local_exists, branch_manager, temp_repo + ): + """When use_local_refs=True and branch exists locally, remote_branch_exists is not called.""" + mock_local_exists.return_value = True + + branch_manager.ensure_branch_exists(temp_repo, "main", use_local_refs=True) + + mock_local_exists.assert_called_once() + mock_remote_exists.assert_not_called() + + @patch.object(BranchManager, "local_branch_exists") + @patch.object(BranchManager, "remote_branch_exists") + @patch.object(BranchManager, "create_local_branch") + @patch.object(BranchManager, "track_remote_branch") + def test_use_local_refs_creates_branch_when_missing( + self, + mock_track, + mock_create, + mock_remote_exists, + mock_local_exists, + branch_manager, + temp_repo, + ): + """When use_local_refs=True and branch missing locally, it is created without ls-remote.""" + mock_local_exists.return_value = False + + branch_manager.ensure_branch_exists( + temp_repo, "new-branch", create_remote=False, use_local_refs=True + ) + + mock_remote_exists.assert_not_called() + mock_create.assert_called_once_with(temp_repo, "new-branch", "HEAD") + + @patch.object(BranchManager, "local_branch_exists") + @patch.object(BranchManager, "remote_branch_exists") + @patch.object(BranchManager, "create_local_branch") + @patch.object(BranchManager, "push_branch_to_remote") + @patch.object(BranchManager, "track_remote_branch") + def test_use_local_refs_false_still_calls_ls_remote( + self, + mock_track, + mock_push, + mock_create, + mock_remote_exists, + mock_local_exists, + branch_manager, + temp_repo, + ): + """Default use_local_refs=False still calls remote_branch_exists.""" + mock_local_exists.return_value = False + mock_remote_exists.return_value = False + + branch_manager.ensure_branch_exists(temp_repo, "new-branch") + + mock_remote_exists.assert_called_once() class TestCreateRemoteBranchViaSSH: diff --git a/test/test_worktree_repo_manager.py b/test/test_worktree_repo_manager.py index 2248febc..5a6a80d0 100644 --- a/test/test_worktree_repo_manager.py +++ b/test/test_worktree_repo_manager.py @@ -340,6 +340,80 @@ def test_remove_repository_keep_directory(self, repo_manager): assert repo_path.exists() # Directory should still exist +class TestLazyFetch: + """Tests for lazy_fetch method.""" + + @patch("devlaunch.worktree.repo_manager.subprocess.run") + def test_lazy_fetch_performs_fetch_when_interval_elapsed(self, mock_run, repo_manager): + """Test lazy_fetch fetches when interval has elapsed.""" + bare_path = repo_manager.get_bare_path("owner", "repo") + bare_path.mkdir(parents=True) + (bare_path / "HEAD").write_text("ref: refs/heads/main\n") + + repo = BaseRepository( + owner="owner", + repo="repo", + remote_url="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/owner/repo.git", + local_path=bare_path, + last_fetched=None, # Never fetched → should fetch + ) + repo_manager.storage.add_repository(repo) + mock_run.return_value = MagicMock(stdout="", stderr="", returncode=0) + + result = repo_manager.lazy_fetch("owner", "repo") + + assert result is True + assert mock_run.called + + @patch("devlaunch.worktree.repo_manager.subprocess.run") + def test_lazy_fetch_skips_when_recent(self, mock_run, repo_manager): + """Test lazy_fetch skips fetch when recently fetched.""" + from datetime import datetime + + bare_path = repo_manager.get_bare_path("owner", "repo") + bare_path.mkdir(parents=True) + (bare_path / "HEAD").write_text("ref: refs/heads/main\n") + + repo = BaseRepository( + owner="owner", + repo="repo", + remote_url="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/owner/repo.git", + local_path=bare_path, + last_fetched=datetime.now(), # Just fetched + ) + repo_manager.storage.add_repository(repo) + + result = repo_manager.lazy_fetch("owner", "repo") + + assert result is False + assert not mock_run.called + + def test_lazy_fetch_raises_when_repo_missing(self, repo_manager): + """Test lazy_fetch raises ValueError when repo not in metadata.""" + with pytest.raises(ValueError, match="not found in metadata"): + repo_manager.lazy_fetch("nonexistent", "repo") + + @patch("devlaunch.worktree.repo_manager.subprocess.run") + def test_lazy_fetch_propagates_fetch_error(self, mock_run, repo_manager): + """Test lazy_fetch propagates errors from fetch_repo.""" + bare_path = repo_manager.get_bare_path("owner", "repo") + bare_path.mkdir(parents=True) + (bare_path / "HEAD").write_text("ref: refs/heads/main\n") + + repo = BaseRepository( + owner="owner", + repo="repo", + remote_url="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/owner/repo.git", + local_path=bare_path, + last_fetched=None, + ) + repo_manager.storage.add_repository(repo) + mock_run.side_effect = subprocess.CalledProcessError(1, "git fetch", stderr="fail") + + with pytest.raises(RuntimeError, match="Failed to fetch"): + repo_manager.lazy_fetch("owner", "repo") + + class TestGetDefaultBranch: """Tests for _get_default_branch method."""