fix(desktop): look for app-folder holders only in the folder being renamed

The swap renames release/win-unpacked alone, but discovery, the re-check
before closing and the close itself looked for holders anywhere under
release/. A program whose working folder is a sibling (a stale
win-arm64-unpacked) was reported as blocking and offered for closing,
though it never blocked the rename. All three steps now take the folder
being renamed. close_holders compares start times with
_START_TIME_TOLERANCE_S instead of a literal 0.5, and take_consent decodes
the consent file after removing it and treats a non-UTF-8 file as no
consent instead of raising out of a swap documented never to raise.

Review folds 1, 2 and 4 (kshitijk4poor) on #144890.

hermes_cli/desktop_release_holders.py

@@ -93,13 +93,14 @@ def _exe_or_empty(proc) -> str:
         return ""
 
 
-def find_release_cwd_holders(release_dir: Path) -> list[ReleaseHolder]:
-    """Every other process whose working directory is ``release_dir`` or below it.
+def find_release_cwd_holders(folder: Path) -> list[ReleaseHolder]:
+    """Every other process whose working directory is ``folder`` (the app folder being renamed,
+    e.g. ``release/win-unpacked``) or below it.
 
     A process whose cwd cannot be read (another user's, access denied) is skipped: it cannot
     be shown to hold the tree. Never raises.
     """
-    root = _release_root(release_dir)
+    root = _release_root(folder)
     try:
         import psutil
         procs = psutil.process_iter(["pid", "name"])
@@ -130,13 +131,13 @@ def take_consent(release_dir: Path, run_id: Optional[str]) -> HolderConsent:
     or not it applies, and it applies only to the hand-off run that the Desktop started for it."""
     path = release_dir / CONSENT_FILE_NAME
     try:
-        raw = path.read_text(encoding="utf-8-sig")
+        raw = path.read_bytes()
     except OSError:
         return NO_CONSENT
     with contextlib.suppress(OSError):
         path.unlink()
     try:
-        data = json.loads(raw)
+        data = json.loads(raw.decode("utf-8-sig"))
         if not run_id or data.get("run_id") != run_id:
             return NO_CONSENT
         holders = frozenset(
@@ -160,10 +161,10 @@ def _ask_at_terminal(holders: list[ReleaseHolder], input_fn: Callable[[str], str
     return answer.strip().lower() in ("y", "yes")
 
 
-def close_holders(holders: list[ReleaseHolder], release_dir: Path) -> list[int]:
+def close_holders(holders: list[ReleaseHolder], folder: Path) -> list[int]:
     """Terminate exactly these holders, each re-checked right before: same pid AND creation
-    time (no recycled pid), cwd still inside ``release_dir``, and not one of our ancestors."""
-    root = _release_root(release_dir)
+    time (no recycled pid), cwd still inside ``folder``, and not one of our ancestors."""
+    root = _release_root(folder)
     try:
         import psutil
         ancestors = {p.pid for p in psutil.Process(os.getpid()).parents()}
@@ -173,7 +174,7 @@ def close_holders(holders: list[ReleaseHolder], release_dir: Path) -> list[int]:
     for holder in holders:
         try:
             proc = psutil.Process(holder.pid)
-            if proc.pid in ancestors or abs(proc.create_time() - holder.create_time) > 0.5:
+            if proc.pid in ancestors or abs(proc.create_time() - holder.create_time) > _START_TIME_TOLERANCE_S:
                 continue
             if root is None or not _inside(_norm(proc.cwd()), root):
                 continue
@@ -193,12 +194,12 @@ def close_holders(holders: list[ReleaseHolder], release_dir: Path) -> list[int]:
 
 
 def release_after_refused_rename(
-    release_dir: Path, *, consent: HolderConsent = NO_CONSENT, input_fn: Callable[[str], str] = input,
+    folder: Path, *, consent: HolderConsent = NO_CONSENT, input_fn: Callable[[str], str] = input,
 ) -> tuple[list[ReleaseHolder], list[int]]:
-    """After Windows refused a rename under ``release_dir``: name every cwd holder, then close
+    """After Windows refused to rename ``folder``: name every cwd holder inside it, then close
     the ones covered by ``consent`` (taken at the start of this swap attempt) or agreed to at the
     terminal. Returns (holders found, pids closed)."""
-    holders = find_release_cwd_holders(release_dir)
+    holders = find_release_cwd_holders(folder)
     if not holders:
         return [], []
     # Human lines only: the machine lines go out once, for the holders still in the way when the
@@ -216,7 +217,7 @@ def release_after_refused_rename(
         chosen = closable
     if not chosen:
         return holders, []
-    closed = close_holders(chosen, release_dir)
+    closed = close_holders(chosen, folder)
     if closed:
         print(f"  ✓ Closed with your consent: {', '.join(h.describe() for h in chosen if h.pid in closed)}")
     return holders, closed

hermes_cli/main_desktop.py

@@ -254,9 +254,11 @@ def _recover_interrupted_swap(live: Path, aside: Path) -> None:
     shutil.rmtree(aside, ignore_errors=True)
 
 
-def _move_live_app_aside(live_root: Path, previous: Path, release_dir: Path, held_by: Optional[list], consent) -> None:
+def _move_live_app_aside(live_root: Path, previous: Path, held_by: Optional[list], consent) -> None:
     """First swap rename. On Windows a refusal names the programs whose working folder is inside
-    ``release/`` (#101789) and retries once only after the user consented to closing them."""
+    ``live_root``, the folder being renamed (#101789), and retries once only after the user
+    consented to closing them. A program in a sibling folder (``win-arm64-unpacked``) does not
+    block this rename, so it is neither named nor offered for closing."""
     try:
         _rename_riding_out_file_lock(live_root, previous)
         return
@@ -264,7 +266,7 @@ def _move_live_app_aside(live_root: Path, previous: Path, release_dir: Path, hel
         if sys.platform != "win32":
             raise
         from hermes_cli.desktop_release_holders import find_release_cwd_holders, release_after_refused_rename
-        holders, closed = release_after_refused_rename(release_dir, consent=consent)
+        holders, closed = release_after_refused_rename(live_root, consent=consent)
         if not closed:
             if held_by is not None:
                 held_by.extend(holders)
@@ -273,7 +275,7 @@ def _move_live_app_aside(live_root: Path, previous: Path, release_dir: Path, hel
         _rename_riding_out_file_lock(live_root, previous)
     except PermissionError:
         if held_by is not None:
-            held_by.extend(find_release_cwd_holders(release_dir))
+            held_by.extend(find_release_cwd_holders(live_root))
         raise
 
 
@@ -308,7 +310,7 @@ def _swap_staged_desktop_app(
             stopped = _stop_desktop_processes_locking_build(desktop_dir, also_posix=True)
             if stopped:
                 logger.info("stopped desktop processes before staged app promotion: %s", stopped)
-            _move_live_app_aside(live_root, previous, release_dir, held_by, consent)
+            _move_live_app_aside(live_root, previous, held_by, consent)
         try:
             _rename_riding_out_file_lock(staged_root, live_root)
         except OSError:

tests/hermes_cli/test_desktop_release_holder_windows_live.py

@@ -9,6 +9,7 @@ and finish the swap.
 
 from __future__ import annotations
 
+import contextlib
 import json
 import subprocess
 import sys
@@ -144,3 +145,38 @@ def test_a_partial_close_reports_only_the_holder_still_in_the_way(swap_fixture,
         for proc in [*psutil.Process(protected.pid).children(recursive=True), psutil.Process(protected.pid)]:
             proc.kill()
         protected.wait(10)
+
+
+def test_a_program_in_a_sibling_unpacked_folder_is_neither_named_nor_closed(swap_fixture, tmp_path, capsys, monkeypatch):
+    """Review fold 1: only release/win-unpacked is renamed. A program whose working folder is a
+    sibling (a stale win-arm64-unpacked) does not block it, so even a consent naming it must not
+    close it, and the failure lines must not offer it."""
+    desktop_dir, staging, live_root, helper = swap_fixture
+    sibling_dir = live_root.parent / "win-arm64-unpacked"
+    sibling_dir.mkdir()
+    sibling_tree: list = []
+    sibling = subprocess.Popen(
+        [sys.executable, str(tmp_path / "foreign_helper.py")], cwd=sibling_dir, stdout=subprocess.PIPE,
+        text=True, encoding="utf-8", creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0),
+    )
+    try:
+        assert sibling.stdout.readline().strip() == "ready"
+        sibling_tree.extend([psutil.Process(sibling.pid), *psutil.Process(sibling.pid).children(recursive=True)])
+        consent, tree = _consent_for_run(desktop_dir, helper, "desktop-retry-1")
+        seen = json.loads(consent.read_text(encoding="utf-8"))
+        seen["holders"] += [{"pid": p.pid, "start_time": p.create_time(), "exe": p.exe()} for p in sibling_tree]
+        consent.write_text(json.dumps(seen), encoding="utf-8")
+        monkeypatch.setenv("HERMES_UPDATE_CORRELATION_ID", "desktop-retry-1")
+
+        promoted, error = _promote(desktop_dir, staging)
+
+        assert error == "" and promoted == live_root / "Hermes.exe"
+        assert helper.wait(10) is not None
+        assert not [p.pid for p in tree if p.is_running()]
+        assert all(p.is_running() for p in sibling_tree), "closed a program that never held the renamed folder"
+        assert f"pid {sibling.pid}" not in capsys.readouterr().out
+    finally:
+        for proc in sibling_tree:
+            with contextlib.suppress(psutil.NoSuchProcess):
+                proc.kill()
+        sibling.wait(10)

tests/hermes_cli/test_desktop_release_holders.py

@@ -118,3 +118,14 @@ def test_a_holder_whose_program_path_is_unreadable_is_left_for_the_user(holder,
     line = next(ln for ln in desktop_release_holders.describe_block(holders, release).splitlines()
                 if f"(pid {proc.pid}, started " in ln)
     assert line.endswith("path unreadable)")
+
+
+def test_a_consent_file_that_is_not_utf8_is_consumed_without_raising(tmp_path):
+    """Review fold 2: the swap that reads it is documented never to raise, and a consent file
+    left behind would fail every later swap."""
+    release = tmp_path / "release"
+    release.mkdir()
+    (release / CONSENT_FILE_NAME).write_bytes(b'{"run_id": "\xff\xfe"}')
+
+    assert take_consent(release, "desktop-retry-1") is desktop_release_holders.NO_CONSENT
+    assert not (release / CONSENT_FILE_NAME).exists()