From 3c5b00f928c3f89966f800f2b19e7ecfdfcdd293 Mon Sep 17 00:00:00 2001 From: b3nw Date: Mon, 6 Jul 2026 01:22:45 +0000 Subject: [PATCH] fix: detect and recover from stale git lock files during self-update - Detect lock file errors (lock_conflict flag) on fetch/status/pull failures in _apply_update_inner (#1) - Expose Force Update button on frontend when lock_conflict is received (#3) - Proactively clean stale lock files (>30s old) in apply_force_update using rglob discovery instead of a hardcoded path list (#2) - Add 10 tests: _is_git_lock_error parametrized unit tests, _apply_update_inner lock path coverage, and apply_force_update stale/recent lock behavior --- api/updates.py | 55 +++++++++++++++ static/ui.js | 2 +- tests/test_updates.py | 152 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 1 deletion(-) diff --git a/api/updates.py b/api/updates.py index d9c91d69..6d2ee582 100644 --- a/api/updates.py +++ b/api/updates.py @@ -214,6 +214,21 @@ def _run_git(args, cwd, timeout=10): return f'git failed to start: {exc}', False +_GIT_LOCK_SIGNATURES = ( + "index.lock': file exists", + "another git process seems to be running", + ".lock': file exists", + "lock file", +) + + +def _is_git_lock_error(output: str) -> bool: + if not output: + return False + lower_out = output.lower() + return any(sig in lower_out for sig in _GIT_LOCK_SIGNATURES) + + def _windows_git_from_registry(): """Best-effort resolve git.exe from the Git-for-Windows registry key. @@ -1363,6 +1378,28 @@ def apply_force_update(target: str) -> dict: if path is None or not (path / '.git').exists(): return {'ok': False, 'message': 'Not a git repository'} + # Clean up stale git lock files that are likely orphaned (older than + # 30 seconds). Live locks held by a concurrent git process are recent, + # so we skip those to avoid corrupting an active operation. + _LOCK_STALE_SECS = 30 + git_dir = path / '.git' + if git_dir.exists(): + for lock_path in git_dir.rglob('*.lock'): + if not lock_path.is_file(): + continue + try: + age = time.time() - lock_path.stat().st_mtime + except OSError: + continue + if age < _LOCK_STALE_SECS: + logger.warning(f"Skipping recent lock file (age={age:.1f}s): {lock_path}") + continue + try: + os.remove(lock_path) + logger.info(f"Removed stale lock file (age={age:.1f}s): {lock_path}") + except OSError as e: + logger.error(f"Failed to remove stale lock file {lock_path}: {e}") + # --force so a remote re-tag (e.g. squash-merge that re-points an # existing release tag) doesn't jam the apply path with "would clobber # existing tag". See #2756. @@ -1460,6 +1497,12 @@ def _apply_update_inner(target): # --force so a remote re-tag doesn't block the update path (see #2756). fetch_out, fetch_ok = _run_git(['fetch', 'origin', '--quiet', '--tags', '--force'], path, timeout=15) if not fetch_ok: + if _is_git_lock_error(fetch_out): + return { + 'ok': False, + 'message': f'Fetch failed due to a repository lock: {fetch_out.strip()}', + 'lock_conflict': True, + } return { 'ok': False, 'message': _apply_fetch_failure_message( @@ -1476,6 +1519,12 @@ def _apply_update_inner(target): ['status', '--porcelain', '--untracked-files=no'], path ) if not status_ok: + if _is_git_lock_error(status_out): + return { + 'ok': False, + 'message': f'Failed to inspect repo status due to a repository lock: {status_out.strip()}', + 'lock_conflict': True, + } return {'ok': False, 'message': f'Failed to inspect repo status: {status_out[:200]}'} # Fail early on unresolved merge conflicts if any(line[:2] in {'DD', 'AU', 'UD', 'UA', 'DU', 'AA', 'UU'} @@ -1508,6 +1557,12 @@ def _apply_update_inner(target): pull_args.extend(['origin', compare_ref]) pull_out, pull_ok = _run_git(pull_args, path, timeout=30) if not pull_ok: + if _is_git_lock_error(pull_out): + return { + 'ok': False, + 'message': f'Pull failed due to a repository lock: {pull_out.strip()}', + 'lock_conflict': True, + } pull_lower = pull_out.lower() detail = pull_out.strip()[:300] if pull_out.strip() else '(no output from git)' untracked_collision = ( diff --git a/static/ui.js b/static/ui.js index 352caf47..4deba79b 100644 --- a/static/ui.js +++ b/static/ui.js @@ -9100,7 +9100,7 @@ function _showUpdateError(target,res){ showToast(msg); } // Show "Force update" button when the error is recoverable by a hard reset - if(forceBtn&&(res.conflict||res.diverged)){ + if(forceBtn&&(res.conflict||res.diverged||res.lock_conflict)){ forceBtn.dataset.target=target; forceBtn.style.display='inline-block'; } diff --git a/tests/test_updates.py b/tests/test_updates.py index 083ef23e..c6898f48 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -1,6 +1,10 @@ """Tests for self-update diagnostics (api/updates.py).""" +import os +import time from unittest.mock import MagicMock, patch +import pytest + import api.updates as updates @@ -893,3 +897,151 @@ def test_select_apply_compare_ref_falls_through_when_latest_tag_is_not_ff_reacha assert ref == 'origin/main' + +# ── _is_git_lock_error unit tests ─────────────────────────────────────────── + + +@pytest.mark.parametrize('output', [ + "fatal: Unable to create '/app/.git/index.lock': File exists.", + "fatal: Unable to create '.git/index.lock': File exists.", + "another git process seems to be running in this repository", + "fatal: Unable to create '.git/FETCH_HEAD.lock': File exists.", + "fatal: Unable to create '.git/refs/heads/main.lock': File exists.", + "could not open lock file", + "error: something about a lock file here", +]) +def test_is_git_lock_error_detects_lock_error(output): + assert updates._is_git_lock_error(output) is True + + +@pytest.mark.parametrize('output', [ + '', + None, + "fatal: cannot lock ref 'refs/tags/v0.51.106': is at 123 but expected 456", + "fatal: unable to access 'https://github.com/nesquena/hermes-webui.git/': Could not resolve host", + "fatal: Not a git repository", + "error: failed to push some refs", +]) +def test_is_git_lock_error_returns_false_for_non_lock(output): + """Non-lock git errors must NOT be classified as lock_conflict.""" + assert updates._is_git_lock_error(output) is False + + +# ── _apply_update_inner lock detection tests ──────────────────────────────── + + +_MODULE = 'api.updates' + + +def _assert_lock_conflict_result(result): + assert result['ok'] is False + assert result.get('lock_conflict') is True + assert 'repository lock' in result['message'] + + +def test_apply_update_fetch_lock_error_returns_lock_conflict(tmp_path): + """Fetch failure caused by .git/index.lock returns lock_conflict: True.""" + (tmp_path / '.git').mkdir() + from api import updates as mod + with patch(f'{_MODULE}.REPO_ROOT', tmp_path), \ + patch(f'{_MODULE}._run_git') as mock_run_git: + mock_run_git.side_effect = [ + ("fatal: Unable to create '/app/.git/index.lock': File exists.", False), + ] + result = mod._apply_update_inner('webui') + _assert_lock_conflict_result(result) + + +def test_apply_update_fetch_lock_error_does_not_attempt_pull(tmp_path): + """If fetch fails with a lock error, no further git calls are made.""" + (tmp_path / '.git').mkdir() + from api import updates as mod + with patch(f'{_MODULE}.REPO_ROOT', tmp_path), \ + patch(f'{_MODULE}._run_git') as mock_run_git: + mock_run_git.side_effect = [ + ("fatal: Unable to create '.git/index.lock': File exists.", False), + ] + mod._apply_update_inner('webui') + assert mock_run_git.call_count == 1 + + +def test_apply_update_status_lock_error_returns_lock_conflict(tmp_path): + """Status failure caused by .git/index.lock returns lock_conflict: True.""" + (tmp_path / '.git').mkdir() + from api import updates as mod + with patch(f'{_MODULE}.REPO_ROOT', tmp_path), \ + patch(f'{_MODULE}._select_apply_compare_ref', return_value='origin/main'), \ + patch(f'{_MODULE}._run_git') as mock_run_git: + mock_run_git.side_effect = [ + ('', True), # fetch succeeds + ("fatal: Unable to create '.git/index.lock': File exists.", False), # status fails + ] + result = mod._apply_update_inner('webui') + _assert_lock_conflict_result(result) + + +def test_apply_update_pull_lock_error_returns_lock_conflict(tmp_path): + """Pull failure caused by .git/index.lock returns lock_conflict: True.""" + (tmp_path / '.git').mkdir() + from api import updates as mod + with patch(f'{_MODULE}.REPO_ROOT', tmp_path), \ + patch(f'{_MODULE}._select_apply_compare_ref', return_value='origin/main'), \ + patch(f'{_MODULE}.STREAMS', {}), \ + patch(f'{_MODULE}._run_git') as mock_run_git: + mock_run_git.side_effect = [ + ('', True), # fetch succeeds + ('', True), # status --porcelain (clean) + ("fatal: Unable to create '.git/index.lock': File exists.", False), # pull fails + ] + result = mod._apply_update_inner('webui') + _assert_lock_conflict_result(result) + + +def test_apply_update_non_lock_fetch_failure_does_not_include_lock_conflict(tmp_path): + """A non-lock fetch failure does NOT return lock_conflict.""" + (tmp_path / '.git').mkdir() + from api import updates as mod + with patch(f'{_MODULE}.REPO_ROOT', tmp_path), \ + patch(f'{_MODULE}._run_git') as mock_run_git: + mock_run_git.side_effect = [ + ("fatal: unable to access 'https://github.com/repo.git/': Could not resolve host", False), + ] + result = mod._apply_update_inner('webui') + assert result['ok'] is False + assert result.get('lock_conflict') is None + + +# ── apply_force_update lock cleanup tests ──────────────────────────────────── + + +def test_apply_force_update_removes_stale_lock_files(tmp_path): + """apply_force_update removes lock files older than 30 seconds.""" + (tmp_path / '.git').mkdir() + stale_lock = tmp_path / '.git' / 'index.lock' + stale_lock.write_text('') + old_mtime = time.time() - 60 + os.utime(stale_lock, (old_mtime, old_mtime)) + + with patch.object(updates, '_run_git', return_value=('', True)), \ + patch.object(updates, 'REPO_ROOT', tmp_path), \ + patch.object(updates, '_restart_blocker_snapshot', return_value={'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}): + updates.apply_force_update('webui') + + assert not stale_lock.exists(), "Stale lock file should have been removed" + + +def test_apply_force_update_skips_recent_lock_file(tmp_path): + """apply_force_update does NOT remove lock files younger than 30 seconds.""" + (tmp_path / '.git').mkdir() + recent_lock = tmp_path / '.git' / 'index.lock' + recent_lock.write_text('') + recent_mtime = time.time() - 5 + os.utime(recent_lock, (recent_mtime, recent_mtime)) + + with patch.object(updates, '_run_git', return_value=('', True)), \ + patch.object(updates, 'REPO_ROOT', tmp_path), \ + patch.object(updates, '_restart_blocker_snapshot', return_value={'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}): + updates.apply_force_update('webui') + + assert recent_lock.exists(), "Recent lock file should NOT have been removed" +