Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
55 changes: 55 additions & 0 deletions hermes_cli/update_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``."""
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions tests/hermes_cli/test_update_orphan_backend_reap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
94 changes: 94 additions & 0 deletions tests/hermes_cli/test_update_stale_index_lock.py
Original file line number Diff line number Diff line change
@@ -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()
2 changes: 2 additions & 0 deletions tests/hermes_cli/test_update_venv_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading