diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 5ea7c0dcec95..a395f4d77b0e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -4567,6 +4567,7 @@ def _remove_custom_provider(config): "_scan_dashboard_processes", ), "hermes_cli.update_cmd": ( + "_abort_if_update_index_locked", "_abort_dependency_sync_if_self_locked", "_add_upstream_remote", "_atomic_replace_dir", @@ -4633,6 +4634,7 @@ def _remove_custom_provider(config): "_surviving_gateway_pids_after_failed_restart", "_sync_fork_with_upstream", "_sync_with_upstream_if_needed", + "_update_index_lock_path", "_update_node_dependencies", "_update_via_zip", "_upgrade_pip_before_lazy_refresh", diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 9ed589504114..5d17c0e8d111 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -4305,6 +4305,56 @@ def _rebuild_desktop_after_update( print(" ✓ Desktop app up to date") +def _update_index_lock_path(project_root: Path) -> Path | None: + """Return the checkout's index lock path, including linked worktrees.""" + git_marker = project_root / ".git" + if git_marker.is_dir(): + return git_marker / "index.lock" + if not git_marker.is_file(): + return None + + try: + marker = git_marker.read_text(encoding="utf-8", errors="replace").strip() + except OSError: + return None + prefix = "gitdir:" + if not marker.lower().startswith(prefix): + return None + + git_dir = Path(marker[len(prefix) :].strip()) + if not git_dir.is_absolute(): + git_dir = (project_root / git_dir).resolve() + return git_dir / "index.lock" + + +def _abort_if_update_index_locked(project_root: Path) -> None: + """Refuse to update while Git's index lock exists. + + ``index.lock`` does not record its owner, so neither its age nor contents + can prove that no live Git operation owns it. Preserve the lock and give + the user an explicit recovery command instead of risking repository + corruption by unlinking it. + """ + lock_path = _update_index_lock_path(project_root) + if lock_path is None or not lock_path.exists(): + return + + if _m()._is_windows(): + quoted_path = str(lock_path).replace("'", "''") + recovery = f"Remove-Item -LiteralPath '{quoted_path}'" + else: + import shlex + + recovery = f"rm -f -- {shlex.quote(str(lock_path))}" + + print(f"✗ Git index lock exists: {lock_path}") + print(" Another Git operation may still be using this repository.") + print(" Close or wait for it to finish, then retry `hermes update`.") + print(" If no Git operation is running, remove the orphaned lock:") + print(f" {recovery}") + sys.exit(2) + + def _cmd_update_impl(args, gateway_mode: bool): """Body of ``cmd_update`` — kept separate so the wrapper can always restore stdio even on ``sys.exit``.""" @@ -4361,6 +4411,11 @@ def _cmd_update_impl(args, gateway_mode: bool): print(_format_concurrent_instances_message(concurrent, scripts_dir)) sys.exit(2) + # Git's lock file has no reliable ownership metadata. Refuse before backup + # or checkout mutation rather than guessing from its age and racing a + # legitimate long-running Git operation. + _m()._abort_if_update_index_locked(_m().PROJECT_ROOT) + # Pre-update backup — runs before any git/file mutation so users can # always roll back to the exact state they had before this update. # Returns the quick-snapshot id (or None when disabled/failed); the diff --git a/tests/hermes_cli/test_update_orphan_backend_reap.py b/tests/hermes_cli/test_update_orphan_backend_reap.py index baf2399d5c0a..c8fca6a08a24 100644 --- a/tests/hermes_cli/test_update_orphan_backend_reap.py +++ b/tests/hermes_cli/test_update_orphan_backend_reap.py @@ -258,6 +258,8 @@ def __truediv__(self, _other): with patch.object(cli_main, "_is_windows", return_value=True), patch.object( cli_main, "_venv_scripts_dir", return_value=None + ), patch.object( + cli_main, "_abort_if_update_index_locked" ), patch.object(cli_main, "_run_pre_update_backup"), patch.object( cli_main, "_pause_windows_gateways_for_update", return_value=None ), patch.object( diff --git a/tests/hermes_cli/test_update_stale_index_lock.py b/tests/hermes_cli/test_update_stale_index_lock.py new file mode 100644 index 000000000000..ce40f2fde14c --- /dev/null +++ b/tests/hermes_cli/test_update_stale_index_lock.py @@ -0,0 +1,94 @@ +"""Regression tests for index locks wedging ``hermes update`` (#63038).""" + +from __future__ import annotations + +import os +import time +from types import SimpleNamespace +from unittest.mock import Mock + +import pytest + +from hermes_cli import main as hermes_main + + +@pytest.mark.parametrize("age_seconds", [0, 7200]) +def test_update_index_lock_aborts_without_deleting( + tmp_path, capsys, monkeypatch, age_seconds +): + git_dir = tmp_path / ".git" + git_dir.mkdir() + lock = git_dir / "index.lock" + lock.touch() + if age_seconds: + old_time = time.time() - age_seconds + os.utime(lock, (old_time, old_time)) + monkeypatch.setattr(hermes_main, "_is_windows", lambda: False) + + with pytest.raises(SystemExit) as exc_info: + hermes_main._abort_if_update_index_locked(tmp_path) + + assert exc_info.value.code == 2 + assert lock.exists() + output = capsys.readouterr().out + assert f"Git index lock exists: {lock}" in output + assert "rm -f -- " in output + assert str(lock) in output + + +def test_missing_update_index_lock_is_noop(tmp_path): + (tmp_path / ".git").mkdir() + assert hermes_main._abort_if_update_index_locked(tmp_path) is None + + +def test_linked_worktree_index_lock_aborts_without_deleting( + tmp_path, capsys, monkeypatch +): + git_dir = tmp_path / "actual-git-dir" + git_dir.mkdir() + lock = git_dir / "index.lock" + lock.touch() + worktree = tmp_path / "worktree" + worktree.mkdir() + (worktree / ".git").write_text("gitdir: ../actual-git-dir\n", encoding="utf-8") + monkeypatch.setattr(hermes_main, "_is_windows", lambda: False) + + with pytest.raises(SystemExit) as exc_info: + hermes_main._abort_if_update_index_locked(worktree) + + assert exc_info.value.code == 2 + assert lock.exists() + assert str(lock) in capsys.readouterr().out + + +def test_windows_index_lock_recovery_uses_powershell(tmp_path, capsys, monkeypatch): + git_dir = tmp_path / ".git" + git_dir.mkdir() + lock = git_dir / "index.lock" + lock.touch() + monkeypatch.setattr(hermes_main, "_is_windows", lambda: True) + + with pytest.raises(SystemExit): + hermes_main._abort_if_update_index_locked(tmp_path) + + assert lock.exists() + assert "Remove-Item -LiteralPath" in capsys.readouterr().out + + +def test_update_aborts_before_backup_or_git_mutation(tmp_path, monkeypatch): + git_dir = tmp_path / ".git" + git_dir.mkdir() + (git_dir / "index.lock").touch() + backup = Mock() + git_run = Mock() + monkeypatch.setattr(hermes_main, "PROJECT_ROOT", tmp_path) + monkeypatch.setattr(hermes_main, "_is_windows", lambda: False) + monkeypatch.setattr(hermes_main, "_run_pre_update_backup", backup) + monkeypatch.setattr(hermes_main.subprocess, "run", git_run) + + with pytest.raises(SystemExit) as exc_info: + hermes_main._cmd_update_impl(SimpleNamespace(), gateway_mode=False) + + assert exc_info.value.code == 2 + backup.assert_not_called() + git_run.assert_not_called() diff --git a/tests/hermes_cli/test_update_venv_health.py b/tests/hermes_cli/test_update_venv_health.py index aaa5f7b915ae..9f2cb0b037e5 100644 --- a/tests/hermes_cli/test_update_venv_health.py +++ b/tests/hermes_cli/test_update_venv_health.py @@ -125,6 +125,8 @@ def __truediv__(self, _other): with patch.object(cli_main, "_is_windows", return_value=True), patch.object( cli_main, "_venv_scripts_dir", return_value=None + ), patch.object( + cli_main, "_abort_if_update_index_locked" ), patch.object(cli_main, "_run_pre_update_backup"), patch.object( cli_main, "_pause_windows_gateways_for_update", return_value=None ), patch.object(