fix(update): remove stale .git/shallow.lock before fetch (#75133) - #75168
fix(update): remove stale .git/shallow.lock before fetch (#75133)#75168RelaxJonh wants to merge 1 commit into
Conversation
…h#75133) When an update is interrupted (network timeout, app force-close, system sleep), Git leaves behind a stale .git/shallow.lock in shallow-cloned repositories. The next fetch fails with "Unable to create '.git/shallow.lock': File exists", which the GUI misinterprets as a concurrent process and traps the user in an unrecoverable update loop. Fix: before running git fetch on shallow repos, check for and remove stale .shallow.lock files. This is safe because the lock is only meaningful during an active fetch — a stale lock from a dead process has no holder.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the stale-lock failure. The report identifies a current unrecovered fetch path, but this patch needs rework before it can address it safely.
Problems
- The added cleanup is only in
_cmd_update_check, reached by the--checkreturn athermes_cli/main.py:8991-8999. Normal apply updates fetch separately in_cmd_update_implathermes_cli/update_cmd.py:3295-3300, which this PR does not change. --checkreturns before sharedUpdateLockacquisition (hermes_cli/main.py:8991-9020). The added existence-only unlink can therefore remove ashallow.lockbelonging to a concurrent fetch.- The PR changes only
hermes_cli/update_cmd.pyand adds no regression coverage for either stale or live shallow locks.
Suggested changes
- Put recovery on the applying fetch path, and make check/apply use a safe ownership or liveness rule rather than deleting any existing lock.
- Add isolated shallow-repository tests for stale-lock recovery and preservation of a live lock.
Automated hermes-sweeper review.
| _shallow_lock = os.path.join(_m().PROJECT_ROOT, ".git", "shallow.lock") | ||
| if os.path.exists(_shallow_lock): | ||
| try: | ||
| os.remove(_shallow_lock) |
There was a problem hiding this comment.
This --check path returns before UpdateLock is acquired (hermes_cli/main.py:8991-9020), so existence alone cannot establish that this lock is stale. It can unlink a lock held by a concurrent fetch; use a shared ownership/liveness guard instead of deleting solely because the path exists.
…with compare-API status Follow-up on the cherry-picked gitlock work (#80501 by @RGerrish, covering the #75133 / #75168 wedge first reported and fixed by @RelaxJonh): - Drop the PR's ancestor-check halves in banner.py, update-count.ts and main.ts: superseded by the compare-API status recovery that landed in #86257/#86331 (ahead_by == 0 already reports local-ahead as up to date). The salvaged update_cmd.py check path keeps main's compare-API structure instead of the PR's tip-SHA-plus-ancestry print. - Keep and wire clear_stale_git_locks() at the remaining wedge sites the original PR targeted: hermes update apply, hermes update --check, and the passive banner check. - Add the desktop counterpart (electron/gitlock.ts) so checkUpdates() heals the same wedge instead of reporting fetch-failed forever; mirrored age + git-process guards; vitest coverage. E2E verified: real --depth 1 clone with an aged .git/shallow.lock reproduces "Unable to create '.git/shallow.lock': File exists"; clear_stale_git_locks removes it and the fetch succeeds; a fresh lock (in-flight fetch) is preserved.
…with compare-API status Follow-up on the cherry-picked gitlock work (#80501 by @RGerrish, covering the #75133 / #75168 wedge first reported and fixed by @RelaxJonh): - Drop the PR's ancestor-check halves in banner.py, update-count.ts and main.ts: superseded by the compare-API status recovery that landed in #86257/#86331 (ahead_by == 0 already reports local-ahead as up to date). The salvaged update_cmd.py check path keeps main's compare-API structure instead of the PR's tip-SHA-plus-ancestry print. - Keep and wire clear_stale_git_locks() at the remaining wedge sites the original PR targeted: hermes update apply, hermes update --check, and the passive banner check. - Add the desktop counterpart (electron/gitlock.ts) so checkUpdates() heals the same wedge instead of reporting fetch-failed forever; mirrored age + git-process guards; vitest coverage. E2E verified: real --depth 1 clone with an aged .git/shallow.lock reproduces "Unable to create '.git/shallow.lock': File exists"; clear_stale_git_locks removes it and the fetch succeeds; a fresh lock (in-flight fetch) is preserved.
|
Thanks @RelaxJonh — earliest fix for this wedge (#75133), and the diagnosis in your PR body (interrupted fetch → stale |
Summary
Fixes #75133.
When an update is interrupted (network timeout, app force-close, system sleep), Git leaves behind a stale
.git/shallow.lockin shallow-cloned repositories. On the next update:git fetchfails withfatal: Unable to create '.git/shallow.lock': File existshermes updateexits with an error"UPDATE DIDN'T FINISH. Hermes is still running".hermes-update-in-progresslock file is left orphaned, trapping users in an unrecoverable update loopFix
Before running
git fetchon shallow repos, check for and remove stale.shallow.lockfiles. This is safe because the lock is only meaningful during an active fetch — a stale lock from a dead process has no holder.Testing
touch ~/.hermes/hermes-agent/.git/shallow.lockhermes update