diff --git a/CHANGELOG.md b/CHANGELOG.md index fcd488ac..5381963c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,37 @@ table and should be judged on its own merits. ### Changed +- **A workspace with no LFS content no longer forks git-lfs on every launch.** + Preparing a workspace always asked `git lfs ls-files` whether there was + anything to materialize, and git-lfs is a large binary whose startup dominates + that answer. The question is now settled first from the clone itself. + `git lfs ls-files` reports the union of HEAD's tree and the index, and + `git ls-files --with-tree=HEAD` enumerates exactly that union — so if none of + those paths holds a pointer, the probe has nothing to report and the fork is + skipped. + + That check is cheaper, not free: it is a `git ls-files` fork plus reading the + first few bytes of each listed path, so it is the same O(tracked files) shape + as the probe it replaces, at a much smaller constant. Measured on the + reference machine, median of 7–9 runs: ~34ms → ~4ms for this repo's own + checkout (124 tracked files), ~119ms → ~18ms at 3000 files, ~1180ms → ~202ms + at 50 000. A workspace that really is holding pointers still pays the probe + and materializes exactly as before, and a clone whose paths cannot be + enumerated at all falls open to probing rather than being written off. + + The union is load-bearing, not belt-and-braces. The index alone is a strictly + smaller set than what git-lfs can name: a clone left with no `.git/index` — + an interrupted clone or checkout, which is precisely what the retry path + exists to recover from — makes `git ls-files` succeed with *empty* output, and + reading that as "nothing tracked, therefore no pointers" would strand the + workspace on stub files on every later launch. + + Deliberately a question about pointer content rather than about whether the + repo declares `filter=lfs`: a repo can hold committed pointers while declaring + nothing, and can be LFS-tracked through attributes git reads from outside the + clone. Either would have been read as "no LFS here" — leaving that workspace on + stub files on every launch, not just once. + - **A warm launch no longer builds the clone manager it never uses.** Every `dl owner/repo@branch -- cmd` read `config.toml`, loaded `metadata.json` twice under its flock, created `repos_dir` and ran the id-scheme migration — and diff --git a/devlaunch/worktree/workspace_clone.py b/devlaunch/worktree/workspace_clone.py index d7f11594..587256c2 100644 --- a/devlaunch/worktree/workspace_clone.py +++ b/devlaunch/worktree/workspace_clone.py @@ -111,11 +111,80 @@ def _remote_ref_exists(self, ws_path: Path, branch: str, remote: str = "origin") ) return result.returncode == 0 + @staticmethod + def _is_lfs_pointer(path: Path) -> bool: + """True if *path* holds an unmaterialized git-lfs pointer. + + A path that will not open is not a pointer. Every ordinary workspace has + several — a deleted file, a dangling symlink, a submodule's directory, a + path a sparse checkout leaves off disk — and none of them says anything + about LFS. Answering True instead is not a harmless over-estimate: it + reinstates the git-lfs fork at the gate, and at the materialization call + site it drives ``git lfs pull origin`` — unbounded and uncaptured — on + every launch of such a workspace, forever, since the pull cannot put a + path the checkout excludes back on disk. + """ + try: + with open(path, "rb") as f: + return f.read(len(_LFS_POINTER_PREFIX)) == _LFS_POINTER_PREFIX + except OSError: + return False + + @classmethod + def _may_hold_lfs_pointers(cls, ws_path: Path) -> bool: + """True unless nothing git-lfs could name holds a pointer. + + A necessary condition for _has_lfs_pointers, standing in front of the + git-lfs fork. ``git lfs ls-files`` reports the union of HEAD's tree and + the index, and ``--with-tree=HEAD`` is what makes ``git ls-files`` + enumerate that same union — so if none of those paths holds a pointer + the probe would answer False anyway, and forking git-lfs to hear it is + pure cost, which the overwhelmingly common non-LFS repo pays on every + single launch. + + Cheaper than the probe, not free: one fork plus the first few bytes of + each listed path. It is the same O(tracked files) shape as the probe it + stands in front of, at a much smaller constant. + + The union is load-bearing. The index alone is a strictly smaller set + than what git-lfs can name, and the gap is reachable with no user + action: a clone left with no ``.git/index`` — an interrupted clone or + checkout, exactly what the materialization retry exists to recover from + — makes ``git ls-files`` exit *zero with empty output*, and reading that + as "nothing tracked, so no pointers" would strand the workspace on stub + files on every later launch. + + Deliberately a question about pointer *content*, not about declarations: + a repo can hold committed pointers with no ``filter=lfs`` attribute of + its own, and can be LFS-tracked through attributes git reads from + outside the clone. Reading either as "no LFS here" would leave such a + workspace on stub files permanently. Content is the thing the caller + actually needs to know, and it is also the thing that stops being true + once materialization succeeds. + + Fails open: paths that cannot be enumerated mean "can't tell", not "no + LFS", so the probe runs — the same degradation _lfs_tracked_files + refuses. An unborn HEAD lands there too, and pays one probe to be told + that a repo with no commits holds nothing. + """ + result = subprocess.run( + ["git", "ls-files", "-z", "--with-tree=HEAD"], + cwd=ws_path, + capture_output=True, + check=False, + ) + if result.returncode != 0: + logger.warning(f"Could not list tracked files: {result.stderr.decode().strip()}") + return True + return any( + cls._is_lfs_pointer(ws_path / os.fsdecode(name)) + for name in result.stdout.split(b"\0") + if name + ) + @staticmethod def _lfs_tracked_files(ws_path: Path) -> list[str]: - """Paths in the tree that git-lfs tracks, empty if lfs is absent.""" - if shutil.which("git-lfs") is None: - return [] + """Paths in the tree that git-lfs tracks.""" result = subprocess.run( ["git", "lfs", "ls-files", "--name-only"], cwd=ws_path, @@ -137,15 +206,17 @@ def _has_lfs_pointers(cls, ws_path: Path) -> bool: Checked by content rather than by "did we just clone this", so an interrupted or failed materialization is retried on the next run instead of leaving the workspace on pointer files for good. + + The working-tree scan runs first and can only rule the answer out, never + in, so the git-lfs probe still decides which pointer-shaped files are + really LFS — the answer is what it always was, minus a fork nobody + needed. """ - for name in cls._lfs_tracked_files(ws_path): - try: - with open(ws_path / name, "rb") as f: - if f.read(len(_LFS_POINTER_PREFIX)) == _LFS_POINTER_PREFIX: - return True - except OSError: - continue - return False + if shutil.which("git-lfs") is None: + return False + if not cls._may_hold_lfs_pointers(ws_path): + return False + return any(cls._is_lfs_pointer(ws_path / name) for name in cls._lfs_tracked_files(ws_path)) def _materialize_lfs(self, ws_path: Path) -> None: """Replace LFS pointer files with real content from the origin remote. diff --git a/test/integration/test_lfs_probe_real.py b/test/integration/test_lfs_probe_real.py new file mode 100644 index 00000000..0828b3f5 --- /dev/null +++ b/test/integration/test_lfs_probe_real.py @@ -0,0 +1,342 @@ +"""Integration tests for the git-lfs pointer check against real git repositories. + +Whether a workspace still holds unmaterialized LFS pointers is a question about +what git and git-lfs actually see, and the ways a repository can end up holding +pointers are not reproducible with a fake git. These tests therefore build real +repositories with real git and let the real thing answer. + +Only the git-lfs executable is stubbed, and only where it is forked: what +`git lfs ls-files` reports for each of these repositories was verified against +git-lfs 3.7.1, and is recorded here so the suite does not require git-lfs to be +installed. Notably it reports a committed pointer file even when the repository +declares no `filter=lfs` attribute anywhere — `git check-attr` says +`filter: unspecified` for that same file — so "this repo declares LFS" is not a +usable stand-in for "this repo holds pointers". +""" + +import subprocess +from unittest.mock import patch + +import pytest + +from devlaunch.worktree.workspace_clone import WorkspaceCloneManager + +# A syntactically real pointer file, as `git lfs track` + commit would leave it +# in a clone made with GIT_LFS_SKIP_SMUDGE=1. +POINTER = b"version https://git-lfs.github.com/spec/v1\noid sha256:" + b"0" * 64 + b"\nsize 12\n" + +# What materialization would leave behind: the twelve bytes POINTER declares. +REAL_CONTENT = b"real content" +assert len(REAL_CONTENT) == 12, "must match the size the pointer declares" + +LFS_ATTRIBUTE_LINE = "*.bin filter=lfs diff=lfs merge=lfs -text\n" + + +def git(repo, *args): + """Run a real git command in *repo*, failing loudly.""" + subprocess.run(["git", "-C", str(repo), *args], check=True, capture_output=True) + + +def make_repo(path): + """Create a real git repository these tests can commit to. + + Signing is switched off explicitly rather than left to whatever the machine + running the suite has in its global config: with `commit.gpgsign = true` set + there, every commit below fails for want of a key, and the suite reports a + dozen failures about git-lfs that have nothing to do with git-lfs. Other + integration tests in this repo still inherit that config; this one does not + need to. + """ + path.mkdir(parents=True, exist_ok=True) + git(path, "init", "-q") + git(path, "config", "user.email", "test@example.com") + git(path, "config", "user.name", "Test") + git(path, "config", "commit.gpgsign", "false") + return path + + +def commit_pointer(path, name="big.bin"): + """Commit *name* as an unmaterialized LFS pointer.""" + (path / name).write_bytes(POINTER) + git(path, "add", "-A") + git(path, "commit", "-qm", "add pointer") + + +def check_pointers(ws_path, lfs_reports=()): + """Answer _has_lfs_pointers for a real repo, stubbing only the git-lfs fork. + + Returns (answer, commands_issued) so a test can pin both the answer and + whether the git-lfs fork this gate exists to avoid was paid. + """ + issued = [] + real_run = subprocess.run + + def spy(cmd, *args, **kwargs): + issued.append(list(cmd)) + if list(cmd[:2]) == ["git", "lfs"]: + return subprocess.CompletedProcess(cmd, 0, "".join(f"{n}\n" for n in lfs_reports), "") + # Forwarded verbatim, `check` included: this stands in for subprocess.run + # itself, so it must not impose a policy of its own. + return real_run(cmd, *args, **kwargs) # pylint: disable=subprocess-run-check + + with ( + patch("devlaunch.worktree.workspace_clone.shutil.which", return_value="/usr/bin/git-lfs"), + patch("devlaunch.worktree.workspace_clone.subprocess.run", side_effect=spy), + ): + answer = WorkspaceCloneManager._has_lfs_pointers(ws_path) # pylint: disable=protected-access + return answer, issued + + +def forked_git_lfs(issued): + """True if any issued command forked git-lfs.""" + return any(cmd[:2] == ["git", "lfs"] for cmd in issued) + + +@pytest.mark.integration +class TestPointerDetectionAgainstRealRepos: + """A workspace holding pointers must be recognised however it got them.""" + + def test_committed_pointer_without_gitattributes_is_detected(self, tmp_path): + """A pointer committed with no gitattributes anywhere still needs pulling. + + Nothing stops a pointer file being committed into a repository that + declares no LFS filter — a deleted .gitattributes, or a file added by a + tool. git-lfs lists such a file; git's own `check-attr` does not call it + LFS-filtered. A workspace holding one must still be materialized, or it + is shipped to the user as a stub. + """ + ws = make_repo(tmp_path / "ws") + commit_pointer(ws) + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_pointer_declared_by_out_of_clone_attributes_is_detected(self, tmp_path): + """LFS declared through core.attributesFile still materializes. + + git honours attributes from outside the working tree — core.attributesFile + here, /etc/gitattributes by the same mechanism — so a repository can be + LFS-tracked in git's view with no gitattributes file of its own. + """ + attributes = tmp_path / "global_gitattributes" + attributes.write_text(LFS_ATTRIBUTE_LINE) + ws = make_repo(tmp_path / "ws") + commit_pointer(ws) + git(ws, "config", "core.attributesFile", str(attributes)) + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_pointer_declared_by_local_info_attributes_is_detected(self, tmp_path): + """LFS declared only in .git/info/attributes still materializes. + + This declaration is local and untracked, so it never appears in the + index or the working tree. + """ + ws = make_repo(tmp_path / "ws") + commit_pointer(ws) + info = ws / ".git" / "info" + info.mkdir(parents=True, exist_ok=True) + (info / "attributes").write_text(LFS_ATTRIBUTE_LINE) + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_pointer_in_a_subdirectory_is_detected(self, tmp_path): + """Pointers are found at any depth, not only at the top level.""" + ws = make_repo(tmp_path / "ws") + (ws / "assets").mkdir() + (ws / ".gitattributes").write_text(LFS_ATTRIBUTE_LINE) + commit_pointer(ws, name="assets/big.bin") + + answer, issued = check_pointers(ws, lfs_reports=["assets/big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_tracked_paths_that_will_not_open_do_not_stop_the_scan(self, tmp_path): + """Unreadable tracked paths are skipped, not fatal, and not the end. + + Deciding from the working tree means looking at every tracked path + rather than the handful git-lfs names, and ordinary workspaces are full + of tracked paths that will not open: a file the user deleted, a symlink + whose target is gone, a submodule's directory. None of them is a + pointer. Treating any of them as an error would break the launch of a + perfectly normal workspace, and giving up at the first one would strand + a real pointer sitting behind it — so the pointer here is named to sort + last, behind all three. + """ + ws = make_repo(tmp_path / "ws") + (ws / "gone.txt").write_text("deleted from the working tree later\n") + (ws / "dangling").symlink_to("no-such-target") + submodule = make_repo(ws / "nested") + (submodule / "f").write_text("x\n") + git(submodule, "add", "-A") + git(submodule, "commit", "-qm", "nested") + commit_pointer(ws, name="zz_big.bin") + (ws / "gone.txt").unlink() + + answer, issued = check_pointers(ws, lfs_reports=["zz_big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_pointer_is_detected_when_the_clone_has_no_index(self, tmp_path): + """A clone left with no `.git/index` still gets its pointers materialized. + + An interrupted clone or checkout can leave the index missing entirely, + and git answers `ls-files` for such a clone with *success and no output* + — not with an error. A gate that asked only the index would read that as + "nothing is tracked, so nothing can be a pointer" and skip, while + git-lfs, which reads HEAD as well, still names the pointer. Nothing + about that heals on its own: the materialization retry exists for + exactly the interrupted operations that produce this state, so the skip + would repeat on every later launch. + """ + ws = make_repo(tmp_path / "ws") + (ws / ".gitattributes").write_text(LFS_ATTRIBUTE_LINE) + commit_pointer(ws) + (ws / ".git" / "index").unlink() + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_pointer_only_in_head_is_detected(self, tmp_path): + """A pointer git-lfs names from HEAD counts even when the index drops it. + + `git lfs ls-files` reports the union of HEAD's tree and the index, so + un-staging a tracked path leaves it named by git-lfs and absent from the + index. The gate has to ask the same union, or it answers "no pointers" + about a path the probe would have named. + """ + ws = make_repo(tmp_path / "ws") + (ws / ".gitattributes").write_text(LFS_ATTRIBUTE_LINE) + commit_pointer(ws) + git(ws, "rm", "-q", "--cached", "big.bin") + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) + + def test_unmaterialized_pointer_is_detected_on_every_launch(self, tmp_path): + """A workspace left on pointers is retried, not written off. + + A failed `git lfs pull` leaves an existing workspace holding pointers. + Deciding once that a repository needs no materialization would make that + state permanent — every later launch would build against stubs. + """ + ws = make_repo(tmp_path / "ws") + commit_pointer(ws) + + first, _ = check_pointers(ws, lfs_reports=["big.bin"]) + second, _ = check_pointers(ws, lfs_reports=["big.bin"]) + + assert first is True + assert second is True + + def test_materialized_workspace_needs_no_further_pull(self, tmp_path): + """Once the real content is on disk the workspace is done. + + The counterpart to the retry above: retrying must stop when it has + worked, or every launch would re-pull. + """ + ws = make_repo(tmp_path / "ws") + (ws / ".gitattributes").write_text(LFS_ATTRIBUTE_LINE) + commit_pointer(ws) + (ws / "big.bin").write_bytes(REAL_CONTENT) + + answer, _ = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is False + + +@pytest.mark.integration +class TestProbeCostAgainstRealRepos: + """The git-lfs fork is what this gate exists to avoid.""" + + def test_ordinary_repo_never_forks_git_lfs(self, tmp_path): + """A repository holding no pointer files must not pay a git-lfs fork. + + This is the common case by an enormous margin, and it pays the fork on + every launch for an answer already visible in the working tree. + """ + ws = make_repo(tmp_path / "ws") + (ws / "main.py").write_text("print('hi')\n") + (ws / "docs").mkdir() + (ws / "docs" / "readme.md").write_text("# hi\n") + git(ws, "add", "-A") + git(ws, "commit", "-qm", "init") + + answer, issued = check_pointers(ws) + + assert answer is False + assert not forked_git_lfs(issued) + + def test_materialized_lfs_repo_never_forks_git_lfs(self, tmp_path): + """A fully materialized LFS repository has nothing left to ask git-lfs. + + Declaring LFS is not the question; holding an unmaterialized pointer is. + A warm workspace whose LFS content is already on disk gets the same free + answer as a repository that never used LFS. + """ + ws = make_repo(tmp_path / "ws") + (ws / ".gitattributes").write_text(LFS_ATTRIBUTE_LINE) + commit_pointer(ws) + (ws / "big.bin").write_bytes(REAL_CONTENT) + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is False + assert not forked_git_lfs(issued) + + def test_tracked_paths_that_will_not_open_are_not_read_as_pointers(self, tmp_path): + """A path that cannot be opened is not evidence of a pointer. + + The counterpart to the scan-does-not-stop test above: every ordinary + workspace has tracked paths that will not open — a file the user + deleted, a dangling symlink, a submodule's directory — and none of them + says anything about LFS. Reading "cannot open it" as "assume pointer" + would reinstate the git-lfs fork on every launch of every such + workspace, which is the entire cost this gate removes, and would do it + with the answer unchanged. + """ + ws = make_repo(tmp_path / "ws") + (ws / "gone.txt").write_text("deleted from the working tree later\n") + (ws / "dangling").symlink_to("no-such-target") + submodule = make_repo(ws / "nested") + (submodule / "f").write_text("x\n") + git(submodule, "add", "-A") + git(submodule, "commit", "-qm", "nested") + git(ws, "add", "-A") + git(ws, "commit", "-qm", "init") + (ws / "gone.txt").unlink() + + answer, issued = check_pointers(ws) + + assert answer is False + assert not forked_git_lfs(issued) + + def test_unreadable_index_still_probes(self, tmp_path): + """When the tracked files cannot be listed, the probe runs anyway. + + The cheap check exists to save a fork, not to decide LFS is absent. + Reading "cannot tell" as "no LFS here" would strand a workspace on + pointer files — exactly the silent degradation the probe refuses. + """ + ws = tmp_path / "not-a-repo" + ws.mkdir() + (ws / "big.bin").write_bytes(POINTER) + + answer, issued = check_pointers(ws, lfs_reports=["big.bin"]) + + assert answer is True + assert forked_git_lfs(issued) diff --git a/test/test_workspace_clone.py b/test/test_workspace_clone.py index 1167b506..808f15c1 100644 --- a/test/test_workspace_clone.py +++ b/test/test_workspace_clone.py @@ -22,6 +22,30 @@ def leaf(branch="nb4", owner="owner", repo="repo"): return WorkspaceId(owner, repo, branch).value +def stub_git(tracked=(), lfs_files=(), index_readable=True): + """Stand in for git in the tests that mock the subprocess boundary wholesale. + + Only the two listings the LFS path reads are modelled, each in the shape git + really returns it: `git ls-files -z --with-tree=HEAD` answers with the union + of HEAD and the index as NUL-separated bytes, `git lfs ls-files` with the + same union as newline-separated text. Every other command succeeds silently. + `tracked` is that union, which is why one list feeds both. + """ + + def run(cmd, *_args, **_kwargs): + if cmd[:3] == ["git", "lfs", "ls-files"]: + return MagicMock(returncode=0, stdout="".join(f"{n}\n" for n in lfs_files), stderr="") + if cmd[:2] == ["git", "ls-files"]: + if not index_readable: + return MagicMock(returncode=128, stdout=b"", stderr=b"fatal: broken index") + return MagicMock( + returncode=0, stdout=b"".join(f"{n}\0".encode() for n in tracked), stderr=b"" + ) + return MagicMock(returncode=0, stdout="", stderr="") + + return run + + @pytest.fixture def tmp_repos_dir(tmp_path): """Create temporary repos directory.""" @@ -315,12 +339,7 @@ def test_existing_workspace_retries_unmaterialized_lfs( big = ws_path / "big.bin" big.write_bytes(b"version https://git-lfs.github.com/spec/v1\noid sha256:x\n") - def run_side_effect(cmd, *args, **kwargs): - if cmd[:3] == ["git", "lfs", "ls-files"]: - return MagicMock(returncode=0, stdout="big.bin\n", stderr="") - return MagicMock(returncode=0, stdout="", stderr="") - - mock_run.side_effect = run_side_effect + mock_run.side_effect = stub_git(tracked=["big.bin"], lfs_files=["big.bin"]) clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") @@ -341,12 +360,7 @@ def test_materialized_workspace_does_not_refetch_lfs( (ws_path / ".git").mkdir(parents=True) (ws_path / "big.bin").write_bytes(b"\x00\x01real binary content") - def run_side_effect(cmd, *args, **kwargs): - if cmd[:3] == ["git", "lfs", "ls-files"]: - return MagicMock(returncode=0, stdout="big.bin\n", stderr="") - return MagicMock(returncode=0, stdout="", stderr="") - - mock_run.side_effect = run_side_effect + mock_run.side_effect = stub_git(tracked=["big.bin"], lfs_files=["big.bin"]) clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") @@ -370,16 +384,12 @@ def test_new_workspace_materializes_lfs( # git clone is mocked, so stand in for what it would have left behind: # a tree whose LFS file is still a pointer (cloned with skip-smudge). - pointer = repo_root / leaf() / "assets" / "big.bin" + ws_root = repo_root / leaf() + pointer = ws_root / "assets" / "big.bin" pointer.parent.mkdir(parents=True) pointer.write_bytes(b"version https://git-lfs.github.com/spec/v1\noid sha256:x\n") - def run_side_effect(cmd, *args, **kwargs): - if cmd[:3] == ["git", "lfs", "ls-files"]: - return MagicMock(returncode=0, stdout="assets/big.bin\n", stderr="") - return MagicMock(returncode=0, stdout="", stderr="") - - mock_run.side_effect = run_side_effect + mock_run.side_effect = stub_git(tracked=["assets/big.bin"], lfs_files=["assets/big.bin"]) clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") @@ -387,6 +397,90 @@ def run_side_effect(cmd, *args, **kwargs): assert ["git", "lfs", "ls-files", "--name-only"] in lfs_calls assert ["git", "lfs", "pull", "origin"] in lfs_calls + @patch("devlaunch.worktree.workspace_clone.shutil.which", return_value="/usr/bin/git-lfs") + @patch("devlaunch.worktree.workspace_clone.subprocess.run") + def test_workspace_without_pointer_files_never_forks_git_lfs( + self, mock_run, _mock_which, clone_manager, mock_repo_manager, tmp_repos_dir + ): + """A workspace holding no pointer files must not pay a git-lfs fork. + + The overwhelmingly common repo has no LFS content at all, and probing it + with `git lfs ls-files` costs a fork on every single launch for an answer + already sitting in the working tree. + """ + 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" + + # An existing workspace with ordinary content. + ws_path = repo_root / leaf() + (ws_path / ".git").mkdir(parents=True) + (ws_path / "main.py").write_text("print('hi')\n") + + mock_run.side_effect = stub_git(tracked=["main.py"]) + + clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") + + issued = [c[0][0] for c in mock_run.call_args_list] + assert not any(cmd[:2] == ["git", "lfs"] for cmd in issued) + + @patch("devlaunch.worktree.workspace_clone.shutil.which", return_value="/usr/bin/git-lfs") + @patch("devlaunch.worktree.workspace_clone.subprocess.run") + def test_lfs_path_missing_from_the_working_tree_is_not_pulled_forever( + self, mock_run, _mock_which, clone_manager, mock_repo_manager, tmp_repos_dir + ): + """An LFS-tracked path that is not on disk is not an unmaterialized pointer. + + A sparse checkout leaves LFS-tracked paths out of the working tree + altogether, so opening them fails. Reading that failure as "still a + pointer" would run `git lfs pull origin` — an unbounded, uncaptured + fetch that can be gigabytes — on every launch of such a workspace, + forever, because the pull does not put the excluded path on disk and so + never changes the answer. + """ + 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" + + # An existing workspace whose one LFS-tracked path is absent from disk. + ws_path = repo_root / leaf() + (ws_path / ".git").mkdir(parents=True) + + mock_run.side_effect = stub_git(tracked=["big.bin"], lfs_files=["big.bin"]) + + clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") + + issued = [c[0][0] for c in mock_run.call_args_list] + assert ["git", "lfs", "pull", "origin"] not in issued + + @patch("devlaunch.worktree.workspace_clone.shutil.which", return_value="/usr/bin/git-lfs") + @patch("devlaunch.worktree.workspace_clone.subprocess.run") + def test_unlistable_index_fails_open_to_probing( + self, mock_run, _mock_which, clone_manager, mock_repo_manager, tmp_repos_dir + ): + """If the tracked files can't be listed, the probe runs anyway. + + The cheap check exists to save a fork, not to decide LFS is absent: when + the listing fails, skipping would silently strand a workspace on pointer + files — the same degradation the probe itself refuses. + """ + 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" + + ws_path = repo_root / leaf() + (ws_path / ".git").mkdir(parents=True) + (ws_path / "big.bin").write_bytes( + b"version https://git-lfs.github.com/spec/v1\noid sha256:x\n" + ) + + mock_run.side_effect = stub_git(lfs_files=["big.bin"], index_readable=False) + + clone_manager.ensure_workspace("owner", "repo", "nb4", "git@github.com:owner/repo.git") + + issued = [c[0][0] for c in mock_run.call_args_list] + assert ["git", "lfs", "pull", "origin"] in issued + @patch("devlaunch.worktree.workspace_clone.shutil.which", return_value=None) @patch("devlaunch.worktree.workspace_clone.subprocess.run") def test_new_workspace_new_branch_bases_on_default(