hermes update is interactive and foreground again on Windows

hermes_cli/main.py

@@ -8822,31 +8822,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.
-
-    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.
-
-    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.
-
-    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.
-
-    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.
+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 exit at once,
+    releasing the shim before the child reaches ``pip install -e .``. Returns
+    False to continue the sync in-process.
+
+    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.
+
+    ``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).
+
+    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
@@ -8867,18 +8883,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
 
 
@@ -9001,7 +9017,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.
@@ -10096,14 +10112,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

@@ -3606,21 +3606,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:
@@ -5108,10 +5123,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

@@ -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,32 +157,100 @@ 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 == []
 
 
 def test_reexec_falls_through_when_venv_python_is_missing(venv, monkeypatch, capsys):
     (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
 # ---------------------------------------------------------------------------