fix(cli): repair shallow boundaries already dropped by the stale-graft prune

[JoaoMarcos44]

Summary

prune_stale_shallow_grafts() (landed via #108053) dropped .git/shallow grafts that reflog-only commits still needed, leaving shallow installer checkouts with a commit whose parent object was never downloaded and whose shallow boundary is gone. git gc, git fsck --connectivity-only and git fetch's auto maintenance then fail (#108286).

This PR deliberately does NOT touch prune_stale_shallow_grafts(). The prevention half of #108286 is owned by @KoNit-K in #108290 (fix(cli): preserve reflog-reachable shallow grafts), which adds git rev-list --all --reflog to the keep-set and fixes the fail-safe. This PR is the strictly complementary repair half, which #108290's own body declares out of scope:

"If a repository was already corrupted by an earlier prune, the reachability probe fails open and leaves repair to an explicit operator recovery path."

git diff origin/main...HEAD -- hermes_cli/gitlock.py is purely additive — zero lines changed inside prune_stale_shallow_grafts().

Why prevention alone is not enough

Every shallow install that ran hermes update after #108053 landed is corrupted right now, and it cannot self-heal. The corruption is only cleared when the offending reflog entries expire, and reflog expiry happens during git gc — which is exactly the command the corruption breaks. Without an explicit repair those installs stay broken indefinitely, with hermes update failing on error: failed to perform geometric repack.

Investigation

Reproduced on current main (git 2.53.0.windows.2), isolated fixture, no dependency on this repo's history:

check	before prune	after prune
rev-list --count HEAD / --count --all (the existing fail-safe)	0 / 0	0 / 0 — false negative
rev-list --count --all --reflog	rc 0	rc 128, fatal: Failed to traverse parents of commit 76b673f
fsck --connectivity-only	clean	rc 2, broken link ... missing commit a70b5de
git gc	rc 0	rc 128, fatal: failed to run repack

Root cause: git fetch --depth 1 records the fetched tip in refs/remotes/<remote>/<branch>'s reflog. When a later fetch supersedes that tip it becomes reflog-only — no ref points at it — so the old keep-set (HEAD + FETCH_HEAD + for-each-ref) dropped its graft. The commit is still reachable via the reflog, its parent was never downloaded, and nothing protects the boundary any more.

Alternatives evaluated and rejected

Delegation to git's own prune_shallow() (shallow.c, invoked by git prune after mark_reachable_objects(..., mark_reflog=1)) was measured as a candidate root-cause design. Rejected on evidence:

• git prune --expire=now deletes unreachable objects (verified: git cat-file -e <sha> → rc 1 afterwards). Unacceptable on an installer checkout inside hermes update --check.
• It does not shrink .git/shallow while reflogs still retain the commits (21 → 21 grafts on a 20-fetch fixture); only a destructive git reflog expire --expire-unreachable=now --all makes it shrink, which destroys the reflog data backing the documented git reflog && git reset --hard <sha> recovery path in update_cmd.py.
• Critically, it does not repair an already-corrupted repo (verified on a corrupted fixture: the removed graft was not restored).

Reflog expiry of refs/remotes/* was also evaluated: it does not restore a missing graft either, and it discards data. The chosen approach is the only validated one that repairs without destroying anything.

The fix

repair_broken_shallow_boundaries(repo_root) -> int in hermes_cli/gitlock.py, called before prune_stale_shallow_grafts() at both existing updater call sites (_cmd_update_check, _cmd_update_impl) so a corrupted repo is healed first and the prune then operates on a consistent one.

Algorithm — walk-free by necessity:

1. Resolve .git/shallow via git rev-parse --git-path shallow; no-op if absent/empty.
2. Enumerate candidates with git cat-file --batch-all-objects --batch-check (type commit) plus git reflog show --all --format=%H. This must not use git rev-list --reflog --all: on an already-corrupted repo that walk is precisely what fails, returning a truncated candidate set that omits the broken commit. This was an actual bug caught during implementation — the first version used rev-list and could not repair the very repos it targets.
3. One git cat-file --batch over all candidates reads parent edges; one git cat-file --batch-check resolves which parents are missing.
4. Any commit present locally with a missing parent is a broken boundary; append those SHAs to .git/shallow atomically (temp file + os.replace, matching the neighbouring prune).
5. Verify with git rev-list --count --all --reflog; on failure restore the original file byte-for-byte.
6. Never raises; logger.debug(..., exc_info=True) on failure, logger.info on success.

Non-destructive by construction: it only ever appends lines to .git/shallow. It never expires a reflog, never runs git prune, never deletes an object, never touches the working tree.

Bounded cost: a small constant number of git subprocesses regardless of repository size. The cat-file --batch output is parsed size-driven over bytes (<sha> <type> <size>\n<body>\n), not by scanning for blank lines — a commit message containing blank lines would desynchronise a line-based cursor.

Tests

New file tests/hermes_cli/test_shallow_boundary_repair.py (separate from test_shallow_graft_prune.py, which #108290 also edits, to avoid conflicts). Behaviour contracts, no change-detectors:

• test_repair_restores_boundary_for_reflog_only_commit_with_unfetched_parent — builds the real corruption fixture and asserts the precondition (fsck really reports broken link) so it can never silently pass on a healthy fixture; then asserts rev-list --all --reflog rc 0, fsck clean, and git gc -q rc 0 after repair.
• test_repair_does_not_touch_reflogs — git reflog show --all byte-identical before/after. This is the safety contract separating this approach from the destructive alternatives.
• test_repair_uses_a_bounded_number_of_git_subprocesses — counts subprocess.run calls on a large (40-commit) and a small fixture and asserts the count is bounded and does not scale with candidate count.
• test_repair_handles_commit_messages_containing_blank_lines — pins the byte-framed parser against messages with blank lines and header-lookalike text.
• Plus no-op on healthy shallow checkout (byte-identical file), no-op on full clone, never-raises on a non-repo path, and idempotency.

RED-before-GREEN verified by stubbing repair_broken_shallow_boundaries to return 0: the corruption and rev-list assertions fail; restoring the implementation turns them green.

Results

scripts/run_tests.sh has no venv in the isolated worktree, so the repo venv's pytest was used:

$ .venv/Scripts/python.exe -m pytest \
    tests/hermes_cli/test_shallow_boundary_repair.py \
    tests/hermes_cli/test_shallow_graft_prune.py \
    tests/hermes_cli/test_gitlock_tmp_packs.py \
    tests/hermes_cli/test_update_check.py \
    tests/hermes_cli/test_cmd_update.py -q
67 passed in 50.42s

Per file: repair 8, graft prune 3, tmp packs 5, update check 4, cmd update 47. The existing test_shallow_graft_prune.py passes unmodified, confirming no interference with #108290's territory.

Scope

hermes_cli/gitlock.py (additive only), hermes_cli/update_cmd.py (repair call + a message printed only when N > 0, matching the existing (pruned N stale shallow graft(s) ...) style), and the new test file. Nothing else.

Fixes #108286. Complements #108290 (prevention), which should land independently; the two are orthogonal and conflict-free.

[ehz0ah]

Reviewed exact head 8500773a6a814186c9bf4b47771cc7e66486dbbb for #108286.

The standalone repair succeeds on the reported broken-reflog fixture, but the production updater immediately calls the current prune and reintroduces the corruption. I also reproduced unsafe boundary inference from commit messages and unrelated object loss, plus a concurrent depth-one fetch overwrite. The new regression file fails the repository's blocking Windows-footgun scan.

Validation: 67 focused and adjacent tests passed through scripts/run_tests.sh. Ruff, ty check hermes_cli/gitlock.py, and git diff --check passed. Real Git probes reproduced each graph-safety finding. The Windows-footgun scan failed on three new test lines. No Windows execution was performed. No hosted checks were available.

The findings below are blocking.

[ehz0ah]

[Bug] (blocking) The production sequence undoes this repair. Both call sites invoke this function and then call the current reflog-blind prune. In a real corrupted shallow fixture, repair returned 1 and restored rev-list --all --reflog and fsck, but the immediately following prune returned 1, removed the same reflog-only boundary, and restored exit codes 128 and 2. The new tests call repair alone, so they miss the actual updater behavior. Repair and prune need to enforce one reachability rule, with an end-to-end regression for this sequence.

[ehz0ah]

[Bug] (blocking) This scans the commit message as if it were part of the commit header. In a healthy depth-two shallow clone, a local commit whose message contained parent ffff... made repair add HEAD to .git/shallow and reduced rev-list --count HEAD from 2 to 1. A normal message can therefore truncate visible history. Parent parsing must stop at the blank line that ends the commit headers.

[ehz0ah]

[Bug] (blocking) A missing parent alone does not prove that #108286 dropped a shallow boundary. This scan covers every local commit object, so unrelated object loss is converted into valid shallow history. After deleting a referenced commit's parent object, repair returned 1, marked the child shallow, and made fsck pass although the parent remained absent. The repair needs evidence that a candidate was a legitimate former shallow boundary instead of hiding arbitrary repository corruption.

[ehz0ah]

[Bug] (blocking) This replace publishes a stale .git/shallow snapshot without Git's shallow.lock. I injected a real depth-one fetch after the initial read and before this replace. The fetch succeeded and added its new boundary, then repair overwrote it. Validation failed and rollback wrote the same stale original, leaving rev-list at exit 128 and fsck at exit 2. Publication and rollback must use Git's shallow lock protocol and preserve any concurrent writer's state.

[ehz0ah]

[Bug] (blocking) The repository's blocking Windows-footgun scan fails this new helper because text=True has no explicit UTF-8 encoding. It also flags the new read_text() and write_text() calls on lines 28 and 30. Add explicit encodings to all three calls so these tests do not decode or write with cp936 or cp1252 on Windows.

[kshitijk4poor]

Merged via #108888 with your authorship preserved (commit on main). The repair approach shipped essentially as designed — see #108888 for the review-finding rework that landed on top (prune reflog fail-safe, header-only parent parsing, refs/remotes-only candidates, shallow.lock protocol, cheap healthy-path gate). Thank you for the excellent investigation on #108286! 🙏

[kshitijk4poor]

Correction — the preserved commit on main is 2fb87f047f (backtick-quoting dropped it above).

[kshitijk4poor]

All five blocking findings from this review were independently reproduced, then fixed in #108888 (merged; your review directly shaped the rework — thank you):

1. Repair undone by prune (update_cmd.py:539) — reproduced exactly as you described (repair=1 → prune=1 → rc 128). Fixed by adding the rev-list --all --reflog walk to the prune's fail-safe, so a restored reflog-only boundary is never dropped again; end-to-end repair→prune regression test added and mutation-checked.
2. Commit-message parsing (gitlock.py:183) — reproduced (healthy history truncated 3→1). Fixed with header-only parent parsing (stop at the blank line); guard test at the _batch_missing_parents seam, mutation-checked red→green.
3. Corruption masking (gitlock.py:203) — reproduced (fsck went clean over a still-missing object). Fixed by restricting candidates to refs/remotes/* reflog tips (fetch-recorded tips) instead of --batch-all-objects; guard test deletes an object and asserts fsck still reports it.
4. shallow.lock / concurrent-writer overwrite (gitlock.py:239) — reproduced both cases (boundary lost; rollback left repo worse than before). Fixed with git's own shallow.lock protocol held across read→write→verify→restore in both shallow writers.
5. Windows-footgun encodings (test_shallow_boundary_repair.py:8) — encoding="utf-8" added to the subprocess call and both read_text/write_text sites; explicit scan of the test file now passes (note: the --all lane does not scan tests/, but the hits were real per house style and are fixed regardless).