From 2fb87f047fd30cc96edc34f45b102c5ac239a0cf Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Fri, 11 Sep 2026 12:28:14 -0300 Subject: [PATCH] fix(cli): repair shallow boundaries already dropped by stale-graft prune A reflog-only commit can remain present after stale-graft pruning drops the shallow boundary it needs, while its parent was never fetched. That leaves git gc, fsck, and rev-list unable to traverse the repository. Prevention alone is insufficient because a broken gc walk prevents reflogs from expiring. Repair scans local commit objects without graph traversal, identifies commits with missing parents, and atomically restores their shallow boundaries. It only updates .git/shallow and never expires reflogs, prunes, or deletes objects, so the operation is non-destructive and idempotent. This complements PR #108290, which owns the prevention half. Refs #108286 --- hermes_cli/gitlock.py | 104 ++++++++++++++++ hermes_cli/update_cmd.py | 10 +- .../test_shallow_boundary_repair.py | 112 ++++++++++++++++++ 3 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_shallow_boundary_repair.py diff --git a/hermes_cli/gitlock.py b/hermes_cli/gitlock.py index cb1f9cdf25..7374e53e92 100644 --- a/hermes_cli/gitlock.py +++ b/hermes_cli/gitlock.py @@ -143,6 +143,110 @@ def _git_stdout_lines(repo_root: Path, args: List[str]) -> List[str]: return [] +def _batch_missing_parents(repo_root: Path, candidates: List[str]) -> set[str]: + """Return local commit objects whose parent objects are missing.""" + if not candidates: + return set() + try: + parents_by_commit = {} + parents = set() + request = "\n".join(candidates) + "\n" + result = subprocess.run( + ["git", "cat-file", "--batch"], + cwd=str(repo_root), + input=request.encode(), + capture_output=True, + timeout=30, + ) + if result.returncode != 0: + return set() + data = result.stdout + cursor = 0 + for candidate in candidates: + header_end = data.find(b"\n", cursor) + if header_end < 0: + return set() + header = data[cursor:header_end].split() + cursor = header_end + 1 + if len(header) >= 3 and header[1] == b"commit": + size = int(header[2]) + body = data[cursor:cursor + size] + cursor += size + if data[cursor:cursor + 1] != b"\n": + return set() + cursor += 1 + decoded = body.decode(errors="replace") + commit_parents = { + fields[1] + for line in decoded.splitlines() + for fields in [line.split()] + if len(fields) >= 2 and fields[0] == "parent" + } + parents_by_commit[candidate] = commit_parents + parents.update(commit_parents) + elif len(header) < 2 or header[1] != b"missing": + return set() + if not parents: + return set() + check = subprocess.run( + ["git", "cat-file", "--batch-check"], + cwd=str(repo_root), + input=("\n".join(sorted(parents)) + "\n").encode(), + capture_output=True, + timeout=10, + ) + missing = { + line.split()[0] + for line in check.stdout.decode(errors="replace").splitlines() + if line.endswith(" missing") + } + return {commit for commit, commit_parents in parents_by_commit.items() if commit_parents & missing} + except Exception: + logger.debug("parent-object probe failed for %s", repo_root, exc_info=True) + return set() + + +def repair_broken_shallow_boundaries(repo_root: Path) -> int: + """Repair reflog-only shallow boundaries dropped by #108286's prune bug. + + This complements the prevention fix in PR #108290: existing corrupted installs need + boundaries reconstructed because their broken history prevents the reflogs from expiring. + """ + try: + shallow_rel = _git_stdout_lines(repo_root, ["rev-parse", "--git-path", "shallow"]) + if not shallow_rel: + return 0 + shallow_path = Path(shallow_rel[0]) + if not shallow_path.is_absolute(): + shallow_path = Path(repo_root) / shallow_path + if not shallow_path.is_file(): + return 0 + original = shallow_path.read_text(encoding="utf-8") + existing = {line for line in original.splitlines() if line} + if not existing: + return 0 + candidates = _git_stdout_lines(repo_root, ["cat-file", "--batch-all-objects", "--batch-check"]) + candidates = sorted({line.split()[0] for line in candidates + if len(line.split()) >= 2 and line.split()[1] == "commit"}) + candidates = sorted(set(candidates + _git_stdout_lines( + repo_root, ["reflog", "show", "--all", "--format=%H"]))) + broken = _batch_missing_parents(repo_root, candidates) + repaired = broken - existing + if not repaired: + return 0 + tmp_path = shallow_path.with_name(shallow_path.name + ".hermes-repair") + tmp_path.write_text("\n".join(sorted(existing | repaired)) + "\n", encoding="utf-8") + os.replace(tmp_path, shallow_path) + if not _git_stdout_lines(repo_root, ["rev-list", "--count", "--all", "--reflog"]): + shallow_path.write_text(original, encoding="utf-8") + return 0 + logger.info("Restored %d broken shallow boundary(ies) in %s", len(repaired), repo_root) + return len(repaired) + except Exception: + logger.debug("shallow boundary repair failed for %s", repo_root, exc_info=True) + return 0 + + def prune_stale_shallow_grafts(repo_root: Path) -> int: """Drop ``.git/shallow`` graft lines no live ref still points at (#105951). diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 615800d9c3..d55cf9481a 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -535,7 +535,10 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): # The depth-1 fetch above leaves the previous tip behind as a ``.git/shallow`` graft # (git never removes old grafts); prune the stale ones so the file stops growing and # merge-base / the orphan-divergence heuristic keep working (#105951). - from hermes_cli.gitlock import prune_stale_shallow_grafts + from hermes_cli.gitlock import repair_broken_shallow_boundaries, prune_stale_shallow_grafts + repaired = repair_broken_shallow_boundaries(_m().PROJECT_ROOT) + if repaired: + print(f" (restored {repaired} broken shallow boundary(ies))") pruned = prune_stale_shallow_grafts(_m().PROJECT_ROOT) if pruned: print(f" (pruned {pruned} stale shallow graft(s) left by past depth-1 checks)") @@ -1340,7 +1343,10 @@ def _cmd_update_impl(args, gateway_mode: bool): print(" (removed %d aborted-fetch pack temp file(s))" % len(swept)) # Shallow installer checkouts collect one `.git/shallow` graft per past depth-1 fetch # (#105951); stale grafts break merge-base and push this run into the divergence path. - from hermes_cli.gitlock import prune_stale_shallow_grafts + from hermes_cli.gitlock import repair_broken_shallow_boundaries, prune_stale_shallow_grafts + repaired = repair_broken_shallow_boundaries(_m().PROJECT_ROOT) + if repaired: + print(f" (restored {repaired} broken shallow boundary(ies))") pruned = prune_stale_shallow_grafts(_m().PROJECT_ROOT) if pruned: print(f" (pruned {pruned} stale shallow graft(s) left by past depth-1 checks)") diff --git a/tests/hermes_cli/test_shallow_boundary_repair.py b/tests/hermes_cli/test_shallow_boundary_repair.py new file mode 100644 index 0000000000..7bd1307c03 --- /dev/null +++ b/tests/hermes_cli/test_shallow_boundary_repair.py @@ -0,0 +1,112 @@ +import subprocess +from pathlib import Path + +import hermes_cli.gitlock as gitlock + + +def git(repo, *args, check=True): + return subprocess.run(["git", *args], cwd=repo, capture_output=True, text=True, check=check) + + +def fixture(tmp_path): + origin = tmp_path / "origin"; origin.mkdir() + git(origin, "init", "-q", "-b", "main") + git(origin, "config", "user.email", "t@example.com"); git(origin, "config", "user.name", "t") + git(origin, "commit", "--allow-empty", "-qm", "c0") + clone = tmp_path / "clone" + subprocess.run(["git", "clone", "-q", "--depth", "1", f"file://{origin}", str(clone)], check=True) + git(origin, "commit", "--allow-empty", "-qm", "c1") + git(origin, "commit", "--allow-empty", "-qm", "c2") + git(clone, "fetch", "-q", "--depth", "1", "origin", "main") + git(origin, "commit", "--allow-empty", "-qm", "c3") + git(clone, "fetch", "-q", "--depth", "1", "origin", "main") + return clone + + +def corrupt_fixture(clone): + path = clone / ".git" / "shallow" + lines = path.read_text().splitlines() + for removed in lines: + path.write_text("\n".join(x for x in lines if x != removed) + "\n") + fsck = git(clone, "fsck", "--connectivity-only", check=False) + if "broken link" in (fsck.stdout + fsck.stderr) or "missing commit" in (fsck.stdout + fsck.stderr): + return removed + raise AssertionError("fixture did not create a broken shallow boundary") + + +def test_repair_restores_boundary_for_reflog_only_commit_with_unfetched_parent(tmp_path): + clone = fixture(tmp_path) + corrupt_fixture(clone) + assert git(clone, "rev-list", "--count", "--all", "--reflog", check=False).returncode != 0 + assert "broken link" in git(clone, "fsck", "--connectivity-only", check=False).stdout + assert gitlock.repair_broken_shallow_boundaries(clone) >= 1 + assert git(clone, "rev-list", "--count", "--all", "--reflog").returncode == 0 + fsck = git(clone, "fsck", "--connectivity-only") + assert "broken link" not in (fsck.stdout + fsck.stderr) + assert git(clone, "gc", "-q").returncode == 0 + + +def test_repair_is_noop_on_healthy_shallow_checkout(tmp_path): + clone = fixture(tmp_path); path = clone / ".git" / "shallow"; before = path.read_bytes() + assert gitlock.repair_broken_shallow_boundaries(clone) == 0 + assert path.read_bytes() == before + + +def test_repair_is_noop_on_full_clone_without_shallow_file(tmp_path): + repo = tmp_path / "repo"; repo.mkdir(); git(repo, "init", "-q") + assert gitlock.repair_broken_shallow_boundaries(repo) == 0 + + +def test_repair_never_raises_on_broken_repo(tmp_path): + assert gitlock.repair_broken_shallow_boundaries(tmp_path / "missing") == 0 + + +def test_repair_does_not_touch_reflogs(tmp_path): + clone = fixture(tmp_path); corrupt_fixture(clone) + before = git(clone, "reflog", "show", "--all").stdout + gitlock.repair_broken_shallow_boundaries(clone) + assert git(clone, "reflog", "show", "--all").stdout == before + + +def test_repair_is_idempotent(tmp_path): + clone = fixture(tmp_path); corrupt_fixture(clone) + assert gitlock.repair_broken_shallow_boundaries(clone) >= 1 + assert gitlock.repair_broken_shallow_boundaries(clone) == 0 + + +def test_repair_uses_a_bounded_number_of_git_subprocesses(tmp_path, monkeypatch): + counts = [] + real_run = gitlock.subprocess.run + + for count in (4, 40): + root = tmp_path / str(count) + root.mkdir() + clone = fixture(root) + for index in range(count): + git(clone, "commit", "--allow-empty", "-qm", f"extra-{index}") + corrupt_fixture(clone) + calls = 0 + + def counting_run(*args, **kwargs): + nonlocal calls + calls += 1 + return real_run(*args, **kwargs) + + monkeypatch.setattr(gitlock.subprocess, "run", counting_run) + assert gitlock.repair_broken_shallow_boundaries(clone) >= 1 + assert git(clone, "rev-list", "--count", "--all", "--reflog").returncode == 0 + counts.append(calls) + monkeypatch.setattr(gitlock.subprocess, "run", real_run) + + assert all(count < 15 for count in counts) + assert abs(counts[0] - counts[1]) <= 2 + + +def test_repair_handles_commit_messages_containing_blank_lines(tmp_path): + clone = fixture(tmp_path) + git(clone, "commit", "--allow-empty", "-m", "subject\n\nbody line\n\ndeadbeef commit 123") + corrupt_fixture(clone) + assert gitlock.repair_broken_shallow_boundaries(clone) >= 1 + assert git(clone, "rev-list", "--count", "--all", "--reflog").returncode == 0 + fsck = git(clone, "fsck", "--connectivity-only") + assert "broken link" not in (fsck.stdout + fsck.stderr)