From 8da1a86329b29a91a50d0b4fad6ff85e66e7130f Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sun, 1 Mar 2026 18:48:55 +0000 Subject: [PATCH 1/2] Base new branches on repo's default branch instead of implicit HEAD Add start_point parameter to BranchManager.ensure_branch_exists() so callers can explicitly specify what to base new branches on. Update WorkspaceCloneManager.ensure_branch() to resolve the repo's default branch and pass it as start_point, ensuring new branches are always based on the latest default branch rather than a potentially stale HEAD. --- devlaunch/worktree/branch_manager.py | 3 ++- devlaunch/worktree/workspace_clone.py | 6 +++++- test/test_workspace_clone.py | 8 ++++++-- test/test_worktree_branch_manager.py | 28 +++++++++++++++++++++++++-- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/devlaunch/worktree/branch_manager.py b/devlaunch/worktree/branch_manager.py index 1242979f..aa972c2a 100644 --- a/devlaunch/worktree/branch_manager.py +++ b/devlaunch/worktree/branch_manager.py @@ -21,6 +21,7 @@ def ensure_branch_exists( remote: str = "origin", create_remote: bool = True, ssh_key_path: Optional[str] = None, + start_point: str = "HEAD", ) -> None: """Ensure branch exists locally and optionally remotely.""" # Check if branch exists locally @@ -42,7 +43,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/workspace_clone.py b/devlaunch/worktree/workspace_clone.py index ef2bc9e5..dba366ca 100644 --- a/devlaunch/worktree/workspace_clone.py +++ b/devlaunch/worktree/workspace_clone.py @@ -88,7 +88,11 @@ def ensure_branch(self, owner: str, repo: str, branch: str) -> None: self.repo_manager.fetch_repo(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 + ) def ensure_workspace( self, diff --git a/test/test_workspace_clone.py b/test/test_workspace_clone.py index 52eafeb7..d4ce9bb0 100644 --- a/test/test_workspace_clone.py +++ b/test/test_workspace_clone.py @@ -124,12 +124,14 @@ def test_fetches_then_ensures( """Test that ensure_branch 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.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" ) def test_continues_if_fetch_fails( @@ -139,11 +141,13 @@ def test_continues_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.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" ) diff --git a/test/test_worktree_branch_manager.py b/test/test_worktree_branch_manager.py index af057cce..0c6e4388 100644 --- a/test/test_worktree_branch_manager.py +++ b/test/test_worktree_branch_manager.py @@ -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,31 @@ 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 TestCreateRemoteBranchViaSSH: From 312005c3f4a325ee6c27a1fab6b02bcca7ff3150 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Thu, 12 Mar 2026 19:04:47 +0000 Subject: [PATCH 2/2] Minimize network operations when opening a new branch - Add lazy_fetch() to RepositoryManager: only fetches when the fetch interval has elapsed, avoiding redundant network calls - Add use_local_refs parameter to BranchManager.ensure_branch_exists(): infers remote branch existence from local refs in bare repos instead of calling git ls-remote - Update WorkspaceCloneManager.ensure_branch() to use lazy_fetch and use_local_refs=True - Skip git fetch origin for newly-created workspaces since they were just cloned from a freshly-fetched bare repo --- devlaunch/worktree/branch_manager.py | 15 ++++- devlaunch/worktree/repo_manager.py | 14 +++++ devlaunch/worktree/workspace_clone.py | 36 +++++++----- test/test_workspace_clone.py | 81 ++++++++++++++++++++++----- test/test_worktree_branch_manager.py | 65 ++++++++++++++++++++- test/test_worktree_repo_manager.py | 74 ++++++++++++++++++++++++ 6 files changed, 254 insertions(+), 31 deletions(-) diff --git a/devlaunch/worktree/branch_manager.py b/devlaunch/worktree/branch_manager.py index aa972c2a..dc61b7bb 100644 --- a/devlaunch/worktree/branch_manager.py +++ b/devlaunch/worktree/branch_manager.py @@ -22,13 +22,24 @@ def ensure_branch_exists( 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") diff --git a/devlaunch/worktree/repo_manager.py b/devlaunch/worktree/repo_manager.py index 2fe53991..19f8aa90 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 dba366ca..62bf9270 100644 --- a/devlaunch/worktree/workspace_clone.py +++ b/devlaunch/worktree/workspace_clone.py @@ -83,15 +83,19 @@ 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}") 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 + bare_path, + branch, + create_remote=False, + start_point=default_branch, + use_local_refs=True, ) def ensure_workspace( @@ -119,7 +123,9 @@ def ensure_workspace( ws_path = self.get_workspace_path(owner, repo, branch) + newly_created = False if not self.workspace_exists(owner, repo, branch): + newly_created = True # Step 2: Clone from bare repo logger.info(f"Creating workspace clone at {ws_path}") ws_path.parent.mkdir(parents=True, exist_ok=True) @@ -150,17 +156,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 newly_created: + 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 d4ce9bb0..5819645a 100644 --- a/test/test_workspace_clone.py +++ b/test/test_workspace_clone.py @@ -121,17 +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, start_point="main" + bare_path, + "newbranch", + create_remote=False, + start_point="main", + use_local_refs=True, ) def test_continues_if_fetch_fails( @@ -140,14 +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, start_point="main" + bare_path, + "newbranch", + create_remote=False, + start_point="main", + use_local_refs=True, ) @@ -158,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" @@ -171,8 +183,8 @@ 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 checkout - assert mock_run.call_count == 4 + # Should have called: git clone, git remote set-url, git checkout (no fetch) + assert mock_run.call_count == 3 # First call: git clone from bare repo clone_call = mock_run.call_args_list[0] @@ -188,12 +200,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: checkout branch - checkout_call = mock_run.call_args_list[3] + # Third call: checkout branch (no fetch for newly-created workspaces) + checkout_call = mock_run.call_args_list[2] assert checkout_call[0][0] == ["git", "checkout", "nb4"] # Should track in metadata @@ -267,6 +275,51 @@ 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 0c6e4388..987f6c90 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 @@ -355,6 +355,69 @@ def test_branch_custom_start_point( 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: """Tests for create_remote_branch_via_ssh method.""" diff --git a/test/test_worktree_repo_manager.py b/test/test_worktree_repo_manager.py index d510953c..37e65018 100644 --- a/test/test_worktree_repo_manager.py +++ b/test/test_worktree_repo_manager.py @@ -339,6 +339,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."""