fix(update): widen stale-lock self-heal to banner and desktop; align 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.
This commit is contained in:
69
apps/desktop/electron/gitlock.test.ts
Normal file
69
apps/desktop/electron/gitlock.test.ts
Normal file
@@ -0,0 +1,69 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import fs from 'node:fs'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { clearStaleGitLocks, LOCK_NAMES, STALE_LOCK_MIN_AGE_MS } from './gitlock'
|
||||
|
||||
function makeRepo(): string {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gitlock-test-'))
|
||||
fs.mkdirSync(path.join(root, '.git'))
|
||||
return root
|
||||
}
|
||||
|
||||
function writeLock(root: string, name: string, ageMs: number): string {
|
||||
const p = path.join(root, '.git', name)
|
||||
fs.writeFileSync(p, '')
|
||||
const t = new Date(Date.now() - ageMs)
|
||||
fs.utimesSync(p, t, t)
|
||||
return p
|
||||
}
|
||||
|
||||
const noGit = async () => false
|
||||
const gitRunning = async () => true
|
||||
|
||||
test('stale shallow.lock older than min age is removed', async () => {
|
||||
const root = makeRepo()
|
||||
const lock = writeLock(root, 'shallow.lock', STALE_LOCK_MIN_AGE_MS + 60_000)
|
||||
const removed = await clearStaleGitLocks(root, { isGitRunning: noGit })
|
||||
assert.deepEqual(removed, [lock])
|
||||
assert.equal(fs.existsSync(lock), false)
|
||||
})
|
||||
|
||||
test('fresh lock is presumed live and never removed', async () => {
|
||||
const root = makeRepo()
|
||||
const lock = writeLock(root, 'shallow.lock', 1_000)
|
||||
const removed = await clearStaleGitLocks(root, { isGitRunning: noGit })
|
||||
assert.deepEqual(removed, [])
|
||||
assert.equal(fs.existsSync(lock), true)
|
||||
})
|
||||
|
||||
test('running git process protects even ancient locks', async () => {
|
||||
const root = makeRepo()
|
||||
const lock = writeLock(root, 'shallow.lock', STALE_LOCK_MIN_AGE_MS * 10)
|
||||
const removed = await clearStaleGitLocks(root, { isGitRunning: gitRunning })
|
||||
assert.deepEqual(removed, [])
|
||||
assert.equal(fs.existsSync(lock), true)
|
||||
})
|
||||
|
||||
test('all known lock names are cleared when stale', async () => {
|
||||
const root = makeRepo()
|
||||
const locks = LOCK_NAMES.map(name => writeLock(root, name, STALE_LOCK_MIN_AGE_MS + 60_000))
|
||||
const removed = await clearStaleGitLocks(root, { isGitRunning: noGit })
|
||||
assert.deepEqual(removed.sort(), locks.sort())
|
||||
})
|
||||
|
||||
test('unknown lock-like files are left alone', async () => {
|
||||
const root = makeRepo()
|
||||
const stray = writeLock(root, 'config.lock', STALE_LOCK_MIN_AGE_MS * 10)
|
||||
await clearStaleGitLocks(root, { isGitRunning: noGit })
|
||||
assert.equal(fs.existsSync(stray), true)
|
||||
})
|
||||
|
||||
test('missing .git dir is a silent no-op', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gitlock-nogit-'))
|
||||
const removed = await clearStaleGitLocks(root, { isGitRunning: noGit })
|
||||
assert.deepEqual(removed, [])
|
||||
})
|
||||
79
apps/desktop/electron/gitlock.ts
Normal file
79
apps/desktop/electron/gitlock.ts
Normal file
@@ -0,0 +1,79 @@
|
||||
// Stale git lock-file recovery for the desktop update-check path.
|
||||
//
|
||||
// A crashed or killed `git fetch` on a shallow clone can leave
|
||||
// `.git/shallow.lock` behind. Every later fetch then fails with
|
||||
// "fatal: Unable to create '.git/shallow.lock': File exists", so the desktop
|
||||
// update check reports 'fetch-failed' forever — git never self-heals these
|
||||
// lock files. Mirrors hermes_cli/gitlock.py: a lock is removed only when it
|
||||
// is older than the min age AND no git process is currently running.
|
||||
|
||||
import { execFile } from 'node:child_process'
|
||||
import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
|
||||
// Lock files younger than this are presumed live (a fetch is in flight) and
|
||||
// are never removed. git lock files live for seconds under normal operation;
|
||||
// anything older than 10 minutes is abandoned.
|
||||
export const STALE_LOCK_MIN_AGE_MS = 10 * 60 * 1000
|
||||
|
||||
// Same self-healable lock set as hermes_cli/gitlock.py.
|
||||
export const LOCK_NAMES = ['shallow.lock', 'index.lock', 'HEAD.lock', 'MERGE_HEAD.lock']
|
||||
|
||||
function gitProcessRunning(): Promise<boolean> {
|
||||
return new Promise(resolve => {
|
||||
const [cmd, args] =
|
||||
process.platform === 'win32'
|
||||
? ['tasklist', ['/FI', 'IMAGENAME eq git.exe', '/FO', 'CSV']]
|
||||
: ['pgrep', ['-x', 'git']]
|
||||
execFile(cmd, args, { timeout: 10_000 }, (error, stdout) => {
|
||||
if (process.platform === 'win32') {
|
||||
// tasklist exits 0 either way; presence is signaled in stdout.
|
||||
resolve(Boolean(stdout && stdout.toLowerCase().includes('git.exe')))
|
||||
return
|
||||
}
|
||||
// pgrep: exit 0 = at least one match; 1 = none; other = probe failure.
|
||||
// On probe failure stay conservative: report "running" so no lock is
|
||||
// touched when we cannot tell.
|
||||
if (error && (error as any).code === 1) {
|
||||
resolve(false)
|
||||
return
|
||||
}
|
||||
resolve(true)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// Remove abandoned .git lock files under repoRoot. Returns removed paths.
|
||||
// Never throws: a lock we cannot stat or unlink is skipped.
|
||||
export async function clearStaleGitLocks(
|
||||
repoRoot: string,
|
||||
{ minAgeMs = STALE_LOCK_MIN_AGE_MS, isGitRunning = gitProcessRunning }: {
|
||||
minAgeMs?: number
|
||||
isGitRunning?: () => Promise<boolean>
|
||||
} = {}
|
||||
): Promise<string[]> {
|
||||
const gitDir = path.join(repoRoot, '.git')
|
||||
const removed: string[] = []
|
||||
try {
|
||||
if (!fs.statSync(gitDir).isDirectory()) return removed
|
||||
} catch {
|
||||
return removed
|
||||
}
|
||||
|
||||
if (await isGitRunning()) return removed
|
||||
|
||||
const cutoff = Date.now() - minAgeMs
|
||||
for (const name of LOCK_NAMES) {
|
||||
const lockPath = path.join(gitDir, name)
|
||||
try {
|
||||
const st = fs.statSync(lockPath)
|
||||
if (st.isFile() && st.mtimeMs < cutoff) {
|
||||
fs.unlinkSync(lockPath)
|
||||
removed.push(lockPath)
|
||||
}
|
||||
} catch {
|
||||
// Missing or concurrently removed — skipping is always safe.
|
||||
}
|
||||
}
|
||||
return removed
|
||||
}
|
||||
@@ -253,6 +253,7 @@ import {
|
||||
resolveCommitLogSelection,
|
||||
shouldCountCommits
|
||||
} from './update-count'
|
||||
import { clearStaleGitLocks } from './gitlock'
|
||||
import { waitForUpdateClearance } from './update-gate'
|
||||
import { readLiveUpdateMarker, updateHandoffConflict, writeUpdateMarker } from './update-marker'
|
||||
import { isOfficialSshRemote, OFFICIAL_REPO_HTTPS_URL } from './update-remote'
|
||||
@@ -2661,6 +2662,12 @@ async function checkUpdates() {
|
||||
}
|
||||
}
|
||||
|
||||
// Self-heal abandoned git lock files before fetching. A stale
|
||||
// .git/shallow.lock from a crashed/interrupted fetch otherwise fails every
|
||||
// later fetch ("Unable to create '.git/shallow.lock': File exists") and this
|
||||
// check reports 'fetch-failed' forever — git never removes these itself.
|
||||
await clearStaleGitLocks(updateRoot)
|
||||
|
||||
const fetched = await runGit(['fetch', '--quiet', 'origin', branch], { cwd: updateRoot })
|
||||
|
||||
if (fetched.code !== 0) {
|
||||
|
||||
@@ -319,6 +319,15 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]:
|
||||
is_shallow = shallow == "true"
|
||||
|
||||
try:
|
||||
# Self-heal abandoned git lock files before fetching. A stale
|
||||
# .git/shallow.lock from a crashed fetch makes the fetch fail, the
|
||||
# exception below is swallowed, and stale refs get compared against
|
||||
# HEAD — silently degrading the passive check until a human removes
|
||||
# the lock (git never self-heals these).
|
||||
from hermes_cli.gitlock import clear_stale_git_locks
|
||||
|
||||
clear_stale_git_locks(repo_dir)
|
||||
|
||||
# Scope the fetch to the one branch the behind-count compares against.
|
||||
# An unscoped ``git fetch origin`` transfers every remote head (~1,400
|
||||
# on this repo — measured 3.0 s vs 0.55 s scoped) and can burn the full
|
||||
|
||||
Reference in New Issue
Block a user