Skip to content

fix(cli): repair shallow boundaries dropped by the stale-graft prune and keep them pruned safely (#108286, salvage #108361) - #108888

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/108361-repair-shallow-boundaries
Sep 12, 2026
Merged

kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/108361-repair-shallow-boundaries

Conversation

@kshitijk4poor

Copy link
Copy Markdown

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: git gc, git fsck --connectivity-only, and git fetch's auto-maintenance all fail (#108286), and the corruption cannot self-heal because reflog expiry happens during git gc — exactly the command that broke.

Salvage of #108361 by @JoaoMarcos44 (cherry-picked, authorship preserved) plus a rework commit fixing the blocking review findings on the original PR. Complements the prevention fix #108290.

What changed

  • repair_broken_shallow_boundaries() in hermes_cli/gitlock.py: re-appends missing shallow boundaries for fetch-recorded tips (commits in refs/remotes/* reflogs) whose parent objects are absent. Called before the prune at both updater call sites (_cmd_update_check, _cmd_update_impl).
  • prune_stale_shallow_grafts() fail-safe now also walks rev-list --all --reflog, so a boundary a reflog-only commit still needs is never dropped (without this, the production repair→prune sequence re-broke the repo on every run).

Rework of the original PR (blocking review findings, each reproduced with real-git probes)

  • Repair undone by prune: verified — repair restored the boundary, the immediately-following prune dropped it again, rev-list back to rc 128. Fixed with the prune fail-safe --reflog leg; regression test added.
  • Commit-message parsing: a parent <sha> line in a commit message body was parsed as an edge, shallow-marking a healthy HEAD and truncating visible history 3→1. Fixed with header-only parsing (stop at the blank line); guard test at the _batch_missing_parents seam.
  • Corruption masking: any missing parent object was re-labelled as shallow history (fsck went "clean" while the object stayed absent). Fixed by restricting candidates to refs/remotes/* reflog tips instead of --batch-all-objects; guard test deletes an object and asserts fsck still reports it.
  • Concurrent-writer race: the temp+os.replace write clobbered a concurrent depth-1 fetch's boundary, and the rollback then made the repo worse. Fixed with git's own shallow.lock protocol, held across read→write→verify→restore in both shallow writers.
  • Cost: repair now gates on a cheap rev-list --all --reflog probe (healthy updates pay one subprocess instead of 5–6); test-file footguns fixed (encoding=, as_uri()).

Validation

  • 68 focused tests green (repair 9, graft prune 3, tmp packs 5, update check 4, cmd update 47) via the repo venv, merge-tree clean vs main.
  • Mutation checks: neutralizing the repair → 3 core tests red; removing the prune fail-safe --reflog leg → sequence test red; removing the header stop → parser guard red. All restored green.
  • Two-cycle E2E of the production sequence on a real corrupted fixture: run 1 repairs (rev-list rc 0, fsck clean, git gc rc 0), run 2 stays fixed.
  • Ruff, windows-footgun (incl. explicit scan of the new test file), compat-pointer, subprocess-stdin guards, git diff --check all clean.

Scope notes

Fixes #108286. Based on #108361 by @JoaoMarcos44 (cherry-picked to preserve authorship).

JoaoMarcos44 and others added 3 commits September 12, 2026 12:12
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 NousResearch#108290, which owns the prevention half.

Refs NousResearch#108286
…h-safety review findings

Rework of the repair pass from NousResearch#108361 (salvage) addressing the blocking
review findings, verified with real-git probes:

- Sequencing: prune_stale_shallow_grafts' fail-safe now also walks
  rev-list --all --reflog, so a boundary the repair just restored (one a
  reflog-only commit still needs) is never dropped again; previously the
  production repair->prune sequence re-broke the repo on every update run.
- Header-only parent parsing: a "parent <sha>" line inside a commit
  message body is prose; _batch_missing_parents stops at the blank line
  ending the commit header, so healthy history is never truncated.
- Candidates restricted to fetch-recorded tips (refs/remotes/* reflogs),
  not --batch-all-objects: unrelated object loss (a deleted parent of a
  locally-created commit) is no longer re-labelled as shallow history;
  fsck keeps reporting it.
- Concurrent-writer safety: both .git/shallow writers now hold git's own
  shallow.lock, so a depth-1 fetch between read and write fails fast
  instead of being clobbered (or clobbering us).
- Cheap gate: repair runs its subprocess fan-out only when
  rev-list --all --reflog already fails; healthy updates pay one probe.
- --batch-check returncode is now checked; shared helpers
  (_shallow_file_path, _ShallowLock) replace the copy-pasted plumbing;
  test file footguns fixed (encoding=, as_uri()) and the missing
  repair->prune end-to-end regression added, mutation-checked.
…he atomic write

Simplify-pass follow-up on the repair rework: the self-check rev-list walks
and the rollback restore now run inside the same _ShallowLock hold (rev-list
never takes shallow.lock), so lock contention can no longer defeat a failed
self-check's rollback and leave a broken .git/shallow in place. The tmp-write
+ os.replace sequence shared by repair and prune moves into _write_shallow.
Scope note added: fetch-by-SHA install tips (HEAD-reflog-only) are not repair
candidates; corruption of that shape is prevented by the prune's reflog
fail-safe. 68 focused tests green; two-cycle repair->prune E2E re-verified.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 12, 2026
@kshitijk4poor
kshitijk4poor merged commit eec131b into NousResearch:main Sep 12, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

prune_stale_shallow_grafts() corrupts shallow checkouts: drops grafts still needed by reflog-only commits (regression of #105951's own fix, #108053)

3 participants