ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(update): hand off only the dependency sync, not the whole update (#90240)

Browse Source 
`hermes update` on Windows detached on every run, including the
`Already up to date!` no-op that never touches the venv. emozilla hit
the visible half: the shim exits, PowerShell takes the console back,
and a child prints the result under a fresh prompt — it reads as a
frozen update. The invisible half is worse: the hand-off sat ahead of
the fetch, so it also carried off the stash and branch-switch
questions, which #90205 then had to answer by closing stdin. Nobody
who mods Hermes got asked about their local changes again.

The shim lock is real and the child is still required — a launcher
holds venv\Scripts\hermes.exe open without FILE_SHARE_DELETE for the
whole command, so the quarantine rename is refused and uv fails with
os error 32. A parent that waits deadlocks against the handle it is
itself holding, and Windows has no exec to escape with.

But that lock only binds one step. Move the hand-off to the dependency
sync boundary, beside the native-module deferral that solves the same
"this process holds a file the sync must replace" problem — and for
the reason that placement already exists (#86735: a preflight ahead of
the fetch re-bricked the flow it was meant to protect). Everything
before the sync now runs foreground in the user's console: the
preflight, the stash question, the branch switch, git pull. An
up-to-date run never hands off at all.

Deferring to the next launch cannot substitute here the way it does
for a mapped .pyd: every future `hermes` launch is also the shim, so
the marker would defer forever. The child re-runs the update to keep
the node/web/lazy-refresh tail, and takes the sync it was spawned for
rather than the up-to-date early return.
This commit is contained in:
brooklyn!
2026-08-20 05:43:00 -05:00
committed by GitHub
parent 5ab74737e0
commit 6d85d790e4
3 changed files with 163 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/main.py
+45 -37
View File
@@ -8809,31 +8809,47 @@ def _windows_running_hermes_launcher_locked() -> bool:
_UPDATE_REEXEC_ENV = "HERMES_UPDATE_REEXEC"
def _reexec_update_off_windows_shim() -> bool:
"""Hand this update to the venv interpreter, off the console shim.
def _reexec_dependency_sync_off_windows_shim() -> bool:
"""Hand the dependency sync to the venv interpreter, off the console shim.
Returns True when a child was spawned and the caller must return at once,
so this process exits and releases the shim before the child reaches
``pip install -e .``. Returns False to continue in-process.
Returns True when a child was spawned and the caller must exit at once,
releasing the shim before the child reaches ``pip install -e .``. Returns
False to continue the sync in-process.
The child is spawned, not waited on — this process exiting IS the fix, so
the shell sees the spawn's status rather than the update's. The update
prints its own result, and ``--gateway`` writes the true exit code to
``.update_exit_code`` for the gateway watcher before restarting.
Called at the dependency-sync boundary, NOT at the top of the command —
the same placement rule as the native-module deferral beside it, and for
the same reason (#86735): a hand-off that fires before the fetch detaches
every run, including the ``Already up to date!`` no-op that never touches
the venv at all, and it takes the interactive prompts with it. By the time
we reach here the code swap is done and every question — stash, branch
switch, config migration — has already been asked and answered in the
user's own console. Only the venv rewrite is left, and that is the single
step that genuinely cannot run from inside the shim.
It also runs unattended, with stdin closed. The child inherits the
console, so left alone ``sys.stdin.isatty()`` still reports a terminal and
the update asks its local-changes question — but this process has already
exited and the shell has taken the console back, so the prompt cannot be
answered and the update hangs forever. Closing stdin makes the update take
the same path it takes for the gateway and Desktop: honour
``updates.non_interactive_local_changes`` (stash by default, nothing lost)
and keep going.
``venv\\Scripts\\hermes.exe`` is a launcher that runs the interpreter with
the shim as its script and holds it open without ``FILE_SHARE_DELETE`` for
the whole command, so the quarantine rename is refused and uv fails to
replace it with os error 32 (#88838, #89599).
Anything that stops the hand-off (no venv python, spawn refused) falls
through to the old in-process behaviour with the manual command printed,
so a broken venv still gets whatever the update can do rather than a
dead end.
A child is required, and waiting on it cannot work: this process holds the
handle the child needs released, so a parent that waits deadlocks against
the work it is waiting for. Windows has no exec to escape with either.
The shell therefore returns while the install runs on; the child keeps the
console and prints its own result, and ``--gateway`` writes the true exit
code to ``.update_exit_code`` for the gateway watcher.
The child re-runs ``hermes update``, so the whole remaining flow — the
dependency sync and the node/web/lazy-refresh tail behind it — still
happens exactly once. ``_UPDATE_REEXEC_ENV`` marks it so it cannot spawn
another child, and so the "already up to date" early return does not
swallow the sync it was spawned to perform (the checkout is current by
now; that is the point).
The caller has already written ``.update-incomplete``, so a child that
dies mid-install is finished by the next launch's recovery instead of
leaving a half-synced venv. Anything that stops the hand-off (no venv
python, spawn refused) returns False and syncs in-process, where the
pre-existing os-error-32 path and its marker recovery still apply.
"""
if os.environ.get(_UPDATE_REEXEC_ENV) == "1":
return False
@@ -8854,18 +8870,18 @@ def _reexec_update_off_windows_shim() -> bool:
)
print(
f"→ Windows: {shim.name} cannot replace itself while it runs; "
"continuing the update under the venv Python."
"finishing the dependency install under the venv Python."
)
print(
" It runs unattended from here — progress prints below and "
"this shell returns right away."
" The code update is already applied. The install continues "
"below and this shell returns right away."
)
return True
except OSError as exc:
logger.debug("Update re-exec via %s failed: %s", python_exe, exc)
print(f" ⚠ Could not re-run the update off {shim.name}. If the install")
print(" fails to replace it, run this from a fresh shell instead:")
print(f" {subprocess.list2cmdline(cmd)}")
logger.debug("Dependency-sync hand-off via %s failed: %s", python_exe, exc)
print(f" ⚠ Could not hand the dependency install off {shim.name}.")
print(" Continuing in-process; if it cannot replace the shim, run:")
print(f" {subprocess.list2cmdline(cmd)}")
return False
@@ -8988,7 +9004,7 @@ def _quarantine_running_hermes_exe(
The updater's own launcher is no longer one of those culprits: an update
started from ``hermes.exe`` re-runs itself under the venv Python before
reaching here (``_reexec_update_off_windows_shim``).
reaching here (``_reexec_dependency_sync_off_windows_shim``).
Returns the list of (original, quarantined) pairs so the caller can roll
back if the install itself fails before uv writes a replacement.
@@ -10083,14 +10099,6 @@ def cmd_update(args):
)
return
# Windows: an update launched through venv\Scripts\hermes.exe holds that
# shim open for its whole run, and the dependency sync has to replace it.
# Hand off to the venv interpreter before anything else — in particular
# before the update lock, so the child claims the marker itself instead of
# adopting one this process is about to release.
if _reexec_update_off_windows_shim():
return
gateway_mode = getattr(args, "gateway", False)
# Protect against mid-update terminal disconnects (SIGHUP) and tolerate
hermes_cli/update_cmd.py
+36 -11
View File
@@ -3641,21 +3641,36 @@ def _detect_self_loaded_native_modules() -> list[str]:
def _abort_dependency_sync_if_self_locked(gateway_resume=None) -> None:
"""Defer (exit 2) when THIS process holds a native module the sync must replace.
"""Defer the venv rewrite when THIS process holds something it must replace.
Runs at the last moment before the venv rewrite — after the code swap —
so the on-disk pyproject reflects the update target and a deferral
leaves the user on NEW code with only the dependency install pending
(completed by the next launch's marker recovery). No-op when nothing
at-risk is loaded.
leaves the user on NEW code with only the dependency install pending.
No-op when nothing at-risk is held.
Two hazards, both "this process holds a file the sync must replace", and
they end differently because their recoveries differ:
- A mapped native extension (``.pyd``). Exit 2 and let the next launch's
marker recovery finish the install: that launch runs the install before
importing anything heavy, so it maps nothing and the swap succeeds.
- The ``hermes.exe`` console shim we were launched from (#88838, #89599).
The marker cannot help here — every future ``hermes`` launch is also the
shim, so deferring to the next launch defers forever. Hand the install
to a child under the venv interpreter and exit, releasing the shim.
"""
locked = _m()._detect_self_loaded_native_modules()
if not locked:
return
_m()._defer_update_for_self_lock(locked)
if gateway_resume is not None:
_m()._resume_windows_gateways_after_update(gateway_resume)
sys.exit(2)
if locked:
_m()._defer_update_for_self_lock(locked)
if gateway_resume is not None:
_m()._resume_windows_gateways_after_update(gateway_resume)
sys.exit(2)
if _m()._reexec_dependency_sync_off_windows_shim():
if gateway_resume is not None:
_m()._resume_windows_gateways_after_update(gateway_resume)
sys.exit(0)
def _defer_update_for_self_lock(loaded: list[str]) -> None:
@@ -5130,10 +5145,20 @@ def _cmd_update_impl(args, gateway_mode: bool):
# otherwise "Already up to date!" gaslights the user while their
# install stays bricked.
healthy, detail = _venv_core_imports_healthy()
if not healthy:
# The Windows shim hand-off spawns this child precisely to run a
# sync its parent could not. The parent already pulled, so the
# checkout is current BY DESIGN and venv health is not the
# question — the pending sync is. Without this the child prints
# "Already up to date!" and exits without doing the one job it
# was spawned for.
handed_off_sync = os.environ.get(_m()._UPDATE_REEXEC_ENV) == "1"
if handed_off_sync:
print("→ Finishing the dependency install handed off by hermes.exe...")
elif not healthy:
print("⚠ Checkout is current, but the venv is unhealthy:")
print(f" {detail}")
print("→ Repairing Python dependencies...")
if handed_off_sync or not healthy:
# Self-lock deferral (#86735): the repair rewrites the venv
# too — same mapped-extension hazard as the update sync.
_m()._abort_dependency_sync_if_self_locked(_windows_gateway_resume)
tests/hermes_cli/test_update_shim_self_lock.py
+82 -9
View File
@@ -3,8 +3,13 @@
``venv\\Scripts\\hermes.exe`` is a launcher that runs the interpreter with the
shim itself as its script, keeping the file open without FILE_SHARE_DELETE for
the whole command. An update started that way must therefore replace a file it
is holding, which Windows refuses — so ``hermes update`` re-runs itself under
``venv\\Scripts\\python.exe`` before touching anything.
is holding, which Windows refuses — so the DEPENDENCY SYNC re-runs itself under
``venv\\Scripts\\python.exe``.
The hand-off sits at the sync boundary, not at the top of ``hermes update``:
everything before it (the fetch, the stash question, the branch switch) runs
in the user's own console, and an up-to-date run that never syncs never hands
off at all.
``_is_windows`` is patched so these paths are exercised on any host.
"""
@@ -129,7 +134,7 @@ def test_reexec_runs_same_args_under_venv_python(venv, monkeypatch, capsys):
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update", "--yes"])
calls = _capture_popen(monkeypatch)
assert cli_main._reexec_update_off_windows_shim() is True
assert cli_main._reexec_dependency_sync_off_windows_shim() is True
cmd, env, kwargs = calls[0]
assert cmd == [
str(venv / "python.exe"), "-m", "hermes_cli.main", "update", "--yes",
@@ -143,7 +148,7 @@ def test_reexec_child_runs_unattended(venv, monkeypatch):
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"])
calls = _capture_popen(monkeypatch)
assert cli_main._reexec_update_off_windows_shim() is True
assert cli_main._reexec_dependency_sync_off_windows_shim() is True
assert calls[0][2]["stdin"] is cli_main.subprocess.DEVNULL
@@ -152,13 +157,13 @@ def test_reexec_does_not_recurse(venv, monkeypatch):
monkeypatch.setenv(cli_main._UPDATE_REEXEC_ENV, "1")
calls = _capture_popen(monkeypatch)
assert cli_main._reexec_update_off_windows_shim() is False
assert cli_main._reexec_dependency_sync_off_windows_shim() is False
assert calls == []
def test_reexec_skipped_when_not_launched_from_a_shim(venv, monkeypatch):
calls = _capture_popen(monkeypatch)
assert cli_main._reexec_update_off_windows_shim() is False
assert cli_main._reexec_dependency_sync_off_windows_shim() is False
assert calls == []
@@ -166,18 +171,86 @@ def test_reexec_falls_through_when_venv_python_is_missing(venv, monkeypatch, cap
(venv / "python.exe").unlink()
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"])
assert cli_main._reexec_update_off_windows_shim() is False
assert "-m hermes_cli.main update" in capsys.readouterr().out
assert cli_main._reexec_dependency_sync_off_windows_shim() is False
assert "-m hermes_cli.main update" not in capsys.readouterr().out
def test_reexec_falls_through_when_spawn_fails(venv, monkeypatch, capsys):
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"])
_capture_popen(monkeypatch, raises=OSError("no exec"))
assert cli_main._reexec_update_off_windows_shim() is False
assert cli_main._reexec_dependency_sync_off_windows_shim() is False
assert "-m hermes_cli.main update" in capsys.readouterr().out
# ---------------------------------------------------------------------------
# Hand-off placement: the sync boundary, not the top of the command
# ---------------------------------------------------------------------------
def test_up_to_date_run_never_hands_off(venv, monkeypatch, capsys):
"""The regression that started this: a no-op update must not detach.
The hand-off used to run before the fetch, so every ``hermes update`` —
including the ``Already up to date!`` case that never touches the venv —
spawned a child and returned to the shell, leaving the child printing
into a console it no longer owned. ``--check`` is the cheapest real run
that reaches ``cmd_update`` and exits without syncing; nothing may be
spawned along the way.
"""
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update", "--check"])
calls = _capture_popen(monkeypatch)
monkeypatch.setattr(cli_main, "_cmd_update_check", lambda **kwargs: None)
cli_main.cmd_update(types.SimpleNamespace(check=True, branch=None))
assert calls == [], "an up-to-date run must not spawn a detached child"
def test_sync_guard_hands_off_when_only_the_shim_is_held(venv, monkeypatch):
"""No native module mapped, but we ARE the shim: hand off and exit 0."""
from hermes_cli import update_cmd
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"])
monkeypatch.setattr(cli_main, "_detect_self_loaded_native_modules", lambda: [])
calls = _capture_popen(monkeypatch)
with pytest.raises(SystemExit) as excinfo:
update_cmd._abort_dependency_sync_if_self_locked()
assert excinfo.value.code == 0
assert calls, "expected the dependency sync to be handed to the venv python"
def test_sync_guard_defers_native_lock_before_considering_the_shim(venv, monkeypatch):
"""A mapped .pyd still exits 2 — the marker recovery owns that case."""
from hermes_cli import update_cmd
monkeypatch.setattr(sys, "argv", [str(venv / "hermes.exe"), "update"])
monkeypatch.setattr(
cli_main, "_detect_self_loaded_native_modules", lambda: ["PyYAML (_yaml.pyd)"]
)
monkeypatch.setattr(cli_main, "_defer_update_for_self_lock", lambda loaded: None)
calls = _capture_popen(monkeypatch)
with pytest.raises(SystemExit) as excinfo:
update_cmd._abort_dependency_sync_if_self_locked()
assert excinfo.value.code == 2
assert calls == [], "a native-module deferral must not also spawn a child"
def test_sync_guard_is_a_noop_when_nothing_is_held(venv, monkeypatch):
"""Off the shim with nothing mapped, the sync just proceeds in-process."""
from hermes_cli import update_cmd
monkeypatch.setattr(cli_main, "_detect_self_loaded_native_modules", lambda: [])
calls = _capture_popen(monkeypatch)
update_cmd._abort_dependency_sync_if_self_locked()
assert calls == []
# ---------------------------------------------------------------------------
# Reboot-deferred renames
# ---------------------------------------------------------------------------