mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(desktop): stop running Hermes.exe locking win-unpacked before Windows pack (#42100)
* fix(desktop): stop running app locking win-unpacked before pack On Windows a running Hermes.exe keeps an exclusive lock on release/win-unpacked/Hermes.exe, so electron-builder's pack cannot replace it and dies with "remove ...\Hermes.exe: Access is denied" / ERR_ELECTRON_BUILDER_CANNOT_EXECUTE (before-pack hits the same EPERM cleaning the dir, and the cache-purge retry repeats the failure since the lock is still held). Before building the packaged app, terminate any process whose executable lives inside this build's release/ tree so the rebuild -- including the installer's headless --update rebuild -- can replace the binary. Scope is narrow (only exes under release/), POSIX is a no-op (it can unlink a running binary), and the final error now points Windows users at the running-app cause. * test(desktop): cover the win-unpacked lock-breaker helper Verify _stop_desktop_processes_locking_build is a no-op off-Windows, terminates only processes whose exe lives under release/ (sparing our own PID and unrelated installs), and short-circuits when no release dir exists.
This commit is contained in:
xxxigm
GitHub
parent
abcf996b1f
commit
96fd9d4979
2 changed files with 163 additions and 0 deletions
|
|
@ -4959,6 +4959,79 @@ def _purge_electron_build_cache(desktop_dir: Path) -> list[Path]:
|
|||
return removed
|
||||
|
||||
|
||||
def _stop_desktop_processes_locking_build(desktop_dir: Path) -> list[int]:
|
||||
"""Terminate any running desktop app executing from this build's ``release``
|
||||
dir so a rebuild can replace its (otherwise locked) executable.
|
||||
|
||||
On Windows a running ``Hermes.exe`` keeps an exclusive lock on
|
||||
``release/win-unpacked/Hermes.exe``. electron-builder's pack then can't
|
||||
delete the stale binary and dies with ``remove …\\Hermes.exe: Access is
|
||||
denied`` / ``ERR_ELECTRON_BUILDER_CANNOT_EXECUTE`` (before-pack hits the same
|
||||
EPERM cleaning the dir). The retry path repeats the failure because the lock
|
||||
is still held. POSIX lets you unlink a running binary, so this is a no-op
|
||||
off-Windows.
|
||||
|
||||
Scope is deliberately narrow: only processes whose executable lives *inside*
|
||||
this desktop's ``release`` tree are stopped — a packaged install elsewhere or
|
||||
an unrelated "Hermes" process is never touched. Best-effort: never raises.
|
||||
Returns the PIDs we asked to stop.
|
||||
"""
|
||||
if sys.platform != "win32":
|
||||
return []
|
||||
try:
|
||||
import psutil
|
||||
except Exception:
|
||||
return []
|
||||
try:
|
||||
release_dir = (desktop_dir / "release").resolve()
|
||||
except OSError:
|
||||
return []
|
||||
if not release_dir.is_dir():
|
||||
return []
|
||||
|
||||
me = os.getpid()
|
||||
victims = []
|
||||
try:
|
||||
proc_iter = psutil.process_iter(["pid", "exe"])
|
||||
except Exception:
|
||||
return []
|
||||
for proc in proc_iter:
|
||||
try:
|
||||
info = proc.info
|
||||
except Exception:
|
||||
continue
|
||||
pid = info.get("pid")
|
||||
exe = info.get("exe")
|
||||
if not exe or pid is None or pid == me:
|
||||
continue
|
||||
try:
|
||||
exe_path = Path(exe).resolve()
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if release_dir in exe_path.parents:
|
||||
victims.append(proc)
|
||||
|
||||
stopped: list[int] = []
|
||||
for proc in victims:
|
||||
try:
|
||||
proc.terminate()
|
||||
stopped.append(int(proc.pid))
|
||||
except Exception:
|
||||
continue
|
||||
if stopped:
|
||||
# Wait for the handles (and thus the file locks) to actually release.
|
||||
try:
|
||||
_, alive = psutil.wait_procs(victims, timeout=5)
|
||||
for proc in alive:
|
||||
try:
|
||||
proc.kill()
|
||||
except Exception:
|
||||
continue
|
||||
except Exception:
|
||||
pass
|
||||
return stopped
|
||||
|
||||
|
||||
def _desktop_macos_relaunchable_fixup(desktop_dir: Path) -> None:
|
||||
"""Make a locally-built (unsigned) macOS desktop app survive in-place self-update.
|
||||
|
||||
|
|
@ -5115,6 +5188,15 @@ def cmd_gui(args: argparse.Namespace):
|
|||
build_label = "source build" if source_mode else "packaged app"
|
||||
print(f"→ Building desktop {build_label}...")
|
||||
build_script = "build" if source_mode else "pack"
|
||||
if not source_mode:
|
||||
# A running desktop instance launched from release/win-unpacked
|
||||
# holds Hermes.exe locked on Windows, so the pack can't replace
|
||||
# it ("Access is denied" / ERR_ELECTRON_BUILDER_CANNOT_EXECUTE).
|
||||
# Stop it first so the rebuild — including the installer's
|
||||
# headless --update rebuild — succeeds instead of failing cryptically.
|
||||
stopped = _stop_desktop_processes_locking_build(desktop_dir)
|
||||
if stopped:
|
||||
print(f" ⚠ Stopped running desktop app to free the build output (pid {', '.join(map(str, stopped))})")
|
||||
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
|
||||
if build_result.returncode != 0 and not source_mode:
|
||||
# A corrupt cached Electron zip makes `pack` fail with an ENOENT
|
||||
|
|
@ -5135,10 +5217,16 @@ def cmd_gui(args: argparse.Namespace):
|
|||
print(" ⚠ Desktop build failed; cleared cached Electron download and retrying once...")
|
||||
for p in purged:
|
||||
print(f" - {p}")
|
||||
# The purge can't remove a win-unpacked tree whose Hermes.exe
|
||||
# is still locked by a running instance; stop it before retry.
|
||||
_stop_desktop_processes_locking_build(desktop_dir)
|
||||
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
|
||||
if build_result.returncode != 0:
|
||||
print("✗ Desktop GUI build failed")
|
||||
print(f" Run manually: cd apps/desktop && npm run {build_script}")
|
||||
if sys.platform == "win32":
|
||||
print(" If this says \"Access is denied\" on Hermes.exe, close any")
|
||||
print(" running Hermes desktop window and retry.")
|
||||
sys.exit(build_result.returncode or 1)
|
||||
packaged_executable = _desktop_packaged_executable(desktop_dir)
|
||||
if not source_mode:
|
||||
|
|
|
|||
|
|
@ -519,3 +519,78 @@ def test_gui_does_not_retry_when_purge_finds_nothing(tmp_path, monkeypatch, caps
|
|||
mock_purge.assert_called_once()
|
||||
assert mock_run.call_count == 1
|
||||
assert "Desktop GUI build failed" in capsys.readouterr().out
|
||||
|
||||
|
||||
class _FakeProc:
|
||||
"""Minimal psutil.Process stand-in for the lock-breaker tests."""
|
||||
|
||||
def __init__(self, pid: int, exe: str | None):
|
||||
self.pid = pid
|
||||
self.info = {"pid": pid, "exe": exe}
|
||||
self.terminated = False
|
||||
self.killed = False
|
||||
|
||||
def terminate(self):
|
||||
self.terminated = True
|
||||
|
||||
def kill(self):
|
||||
self.killed = True
|
||||
|
||||
|
||||
def test_stop_desktop_build_lock_noop_off_windows(tmp_path, monkeypatch):
|
||||
"""POSIX can unlink a running binary, so the helper is a no-op there."""
|
||||
desktop_dir = tmp_path / "apps" / "desktop"
|
||||
exe = desktop_dir / "release" / "linux-unpacked" / "hermes"
|
||||
exe.parent.mkdir(parents=True)
|
||||
exe.write_text("", encoding="utf-8")
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
|
||||
proc = _FakeProc(4321, str(exe))
|
||||
with patch("psutil.process_iter", return_value=[proc]) as it:
|
||||
assert cli_main._stop_desktop_processes_locking_build(desktop_dir) == []
|
||||
it.assert_not_called()
|
||||
assert proc.terminated is False
|
||||
|
||||
|
||||
def test_stop_desktop_build_lock_terminates_only_release_procs(tmp_path, monkeypatch):
|
||||
desktop_dir = tmp_path / "apps" / "desktop"
|
||||
release = desktop_dir / "release" / "win-unpacked"
|
||||
release.mkdir(parents=True)
|
||||
locker_exe = release / "Hermes.exe"
|
||||
locker_exe.write_text("", encoding="utf-8")
|
||||
other_exe = tmp_path / "elsewhere" / "Hermes.exe"
|
||||
other_exe.parent.mkdir(parents=True)
|
||||
other_exe.write_text("", encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "win32")
|
||||
monkeypatch.setattr(cli_main.os, "getpid", lambda: 999)
|
||||
|
||||
locker = _FakeProc(101, str(locker_exe))
|
||||
unrelated = _FakeProc(102, str(other_exe))
|
||||
selfish = _FakeProc(999, str(locker_exe)) # our own PID — never killed
|
||||
no_exe = _FakeProc(103, None)
|
||||
|
||||
captured = {}
|
||||
|
||||
def _wait(procs, timeout=None):
|
||||
captured["waited"] = list(procs)
|
||||
return procs, []
|
||||
|
||||
with patch("psutil.process_iter", return_value=[locker, unrelated, selfish, no_exe]), \
|
||||
patch("psutil.wait_procs", side_effect=_wait):
|
||||
stopped = cli_main._stop_desktop_processes_locking_build(desktop_dir)
|
||||
|
||||
assert stopped == [101]
|
||||
assert locker.terminated is True
|
||||
assert unrelated.terminated is False
|
||||
assert selfish.terminated is False
|
||||
assert captured["waited"] == [locker]
|
||||
|
||||
|
||||
def test_stop_desktop_build_lock_no_release_dir(tmp_path, monkeypatch):
|
||||
desktop_dir = tmp_path / "apps" / "desktop"
|
||||
desktop_dir.mkdir(parents=True)
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "win32")
|
||||
with patch("psutil.process_iter") as it:
|
||||
assert cli_main._stop_desktop_processes_locking_build(desktop_dir) == []
|
||||
it.assert_not_called()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue