From 9e9e1b2245a07370422ff86c277ec3cd6ab3ab05 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:53:40 -0700 Subject: [PATCH] feat(browser): consented auto-close of a running browser for Windows real-profile [proof workflow do-not-merge] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live Windows CI proved copy-while-running is impossible (Chrome opens the cookie DB deny-all). So to make Windows actually WORK — not just fail cleanly — add opt-in auto-close: browser.real_profile_autoclose (default false). When the profile is locked and consent is on, snapshot_real_profile terminates the browser process tree bound to THAT user-data-dir (psutil, identity+binding verified like the daemon reaper — browser binary AND this exact --user-data-dir in cmdline, fail-closed on ambiguity), waits for the lock to release, then snapshots. Destructive (loses unsaved tabs) so it's off by default and the agent asks first; the fail-fast message names the option. No effect on POSIX. - close_browser_holding_profile: graceful terminate → kill → poll until the cookie DB is openable again (bounded); reports relaunch/tray failure clearly. - _processes_holding_profile: identity+binding matcher (never kills an unrelated same-name process on a different dir). - Config key + docs admonition. Tests: autoclose closes-then-snapshots, autoclose-failure-reports, fail-fast names the option, process-matcher identity/binding. 74 real-profile tests pass. Windows live E2E (PROOF workflow, reverted before merge): autoclose-off fails fast <30s; autoclose-on terminates real Chrome, lock releases, valid cookie DB copied. --- .github/workflows/windows-realprofile-e2e.yml | 63 ++++++++ hermes_cli/browser_connect.py | 145 +++++++++++++++++- hermes_cli/config_defaults.py | 9 ++ .../test_real_profile_windows_live.py | 143 +++++++++++++++++ tests/tools/test_browser_real_profile.py | 69 +++++++++ website/docs/user-guide/features/browser.md | 6 + 6 files changed, 427 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/windows-realprofile-e2e.yml create mode 100644 tests/hermes_cli/test_real_profile_windows_live.py diff --git a/.github/workflows/windows-realprofile-e2e.yml b/.github/workflows/windows-realprofile-e2e.yml new file mode 100644 index 0000000000..61e8ac7462 --- /dev/null +++ b/.github/workflows/windows-realprofile-e2e.yml @@ -0,0 +1,63 @@ +name: Windows real-profile live E2E + +# ON-DEMAND PROOF (real-profile browsing, PR #95620 — Windows auto-close). +# +# Proves on a REAL windows-latest runner that consented auto-close makes +# real-profile browsing work with a running Chrome (terminates the locking +# process tree, lock releases, valid copy produced) and that the default +# (autoclose off) fails fast rather than hanging. +# +# PROOF workflow — reverted before merge; must not land on main. Per-sha +# concurrency + no cancel-in-progress so runs complete. + +on: + push: + branches: + - "feat/real-profile-cdp" + +permissions: + contents: read + +concurrency: + group: windows-realprofile-e2e-${{ github.sha }} + cancel-in-progress: false + +jobs: + real-profile-e2e: + name: real-profile auto-close live E2E (windows-latest) + runs-on: windows-latest + timeout-minutes: 15 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Install Google Chrome + shell: pwsh + run: choco install googlechrome --no-progress -y --ignore-checksums + + - name: Install uv + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # 8.2.0 + with: + version: "0.9.28" + enable-cache: true + cache-dependency-glob: | + pyproject.toml + uv.lock + + - name: Set up Python 3.11 + uses: ./.github/actions/retry + with: + command: uv python install 3.11 + + - name: Install dependencies + uses: ./.github/actions/retry + with: + command: uv sync --locked --python 3.11 --extra dev + + - name: Run real-profile auto-close live E2E + shell: bash + run: | + set -uo pipefail + uv run --no-sync python -m pytest \ + tests/hermes_cli/test_real_profile_windows_live.py \ + -o addopts= -v -s -p no:cacheprovider diff --git a/hermes_cli/browser_connect.py b/hermes_cli/browser_connect.py index f0f1e78dea..6c00c3c805 100644 --- a/hermes_cli/browser_connect.py +++ b/hermes_cli/browser_connect.py @@ -662,6 +662,118 @@ def _profile_is_locked(src: str, source_profile: str) -> bool: return False +def _real_profile_autoclose() -> bool: + """Whether browser.real_profile_autoclose consent is on (config read). + + When true, snapshot_real_profile may terminate a running browser that locks + the profile. Destructive → default False; the agent gates it on user OK. + """ + try: + from hermes_cli.config import read_raw_config + + cfg = read_raw_config() + browser_cfg = cfg.get("browser", {}) + if isinstance(browser_cfg, dict): + return bool(browser_cfg.get("real_profile_autoclose", False)) + except Exception as e: + logger.debug("could not read real_profile_autoclose: %s", e) + return False + + +def _processes_holding_profile(src: str): + """Yield (psutil.Process) instances holding the user-data-dir ``src`` open. + + Identity discipline mirrors the daemon reaper: a process qualifies only when + it's a Chromium-family binary AND its command line references THIS + user-data-dir — so we never terminate an unrelated same-PID process. Any + ambiguity (unreadable cmdline) is skipped, fail-closed. + """ + try: + import psutil + except ImportError: # hard dep; defensive + return + norm = os.path.normcase(os.path.normpath(src)) + browser_bins = ( + "chrome", "chrome.exe", "chromium", "chromium.exe", "chrome_crashpad", + "brave", "brave.exe", "msedge", "msedge.exe", "google chrome", + ) + for proc in psutil.process_iter(["name", "cmdline"]): + try: + name = (proc.info.get("name") or "").lower() + cmd = proc.info.get("cmdline") or [] + joined = " ".join(cmd) + except (psutil.NoSuchProcess, psutil.AccessDenied, OSError): + continue + if not any(b in name for b in browser_bins): + # Some platforms report a generic name; also accept when the binary + # in argv[0] looks like a browser. + argv0 = (cmd[0].lower() if cmd else "") + if not any(b in argv0 for b in browser_bins): + continue + # Binding: the exact user-data-dir must appear in the cmdline + # (--user-data-dir=), normalized for case/separators. + if norm not in os.path.normcase(os.path.normpath(joined)) and \ + f"--user-data-dir={src}".lower() not in joined.lower(): + continue + yield proc + + +def close_browser_holding_profile(src: str, timeout: float = 15.0) -> tuple[bool, str]: + """Terminate the browser process tree holding ``src`` and wait for release. + + CONSENTED, DESTRUCTIVE. Only call after the user has agreed to close their + browser — it terminates every Chromium-family process bound to this exact + user-data-dir (graceful terminate, then kill), so unsaved tab/form state in + that browser is lost. Returns ``(True, msg)`` once the profile lock actually + releases, ``(False, msg)`` if processes couldn't be found/killed or the lock + never released within ``timeout``. + """ + try: + import psutil + except ImportError: + return False, "psutil unavailable — cannot close the browser automatically." + + procs = list(_processes_holding_profile(src)) + if not procs: + # Nothing we can see holds it. Either already closed, or the holder is + # a different user / unreadable — caller re-probes the lock. + return False, "no matching browser process found holding the profile." + + # Include child processes (renderers, GPU, crashpad) for a full tree kill. + targets = [] + for p in procs: + targets.append(p) + try: + targets.extend(p.children(recursive=True)) + except (psutil.NoSuchProcess, psutil.AccessDenied): + pass + # Graceful terminate first. + for p in targets: + try: + p.terminate() + except (psutil.NoSuchProcess, psutil.AccessDenied): + pass + gone, alive = psutil.wait_procs(targets, timeout=min(timeout, 8.0)) + for p in alive: + try: + p.kill() + except (psutil.NoSuchProcess, psutil.AccessDenied): + pass + psutil.wait_procs(alive, timeout=3.0) + + # The lock releases slightly after the process exits on Windows; poll. + source_profile = _last_used_profile(src) + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not _profile_is_locked(src, source_profile): + return True, f"closed the browser and the profile lock released." + time.sleep(0.5) + return False, ( + "closed the browser processes but the profile is still locked — " + "another instance may have relaunched (background/tray mode)." + ) + + def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | None, str | None]: """Snapshot ``browser``'s real ACTIVE profile into the hermes copy dir. @@ -691,15 +803,32 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N source_profile = _last_used_profile(src) # Fast lock probe BEFORE any copy: a running browser holds the cookie DB # deny-all (Windows), and a blocking file op on it can hang the launch for - # minutes. Bail immediately with an actionable message instead. On POSIX - # this never trips (no mandatory locking) so copy-while-running still works. + # minutes. On POSIX this never trips (no mandatory locking) so + # copy-while-running still works. if _profile_is_locked(src, source_profile): - return None, ( - f"{browser} is running and has its profile locked, so its login " - "data can't be copied. Fully quit the browser (including any " - "background/tray instance) and retry, or turn " - "browser.use_real_profile off." - ) + # Consented auto-close: only when browser.real_profile_autoclose is on + # (the agent must have the user's OK — it's destructive). Terminate the + # browser tree bound to this profile, wait for the lock to release, then + # continue the snapshot. Off by default → fail fast with guidance. + if _real_profile_autoclose(): + closed, msg = close_browser_holding_profile(src) + if not closed: + return None, ( + f"{browser} is running and locks its login data; Hermes " + f"tried to close it but {msg} Fully quit {browser} " + "(including any background/tray instance) and retry." + ) + logger.info("real-profile: %s", msg) + # fall through — lock released, snapshot proceeds + else: + return None, ( + f"{browser} is running and has its profile locked, so its login " + "data can't be copied. Fully quit the browser (including any " + "background/tray instance) and retry, turn " + "browser.use_real_profile off, or enable " + "browser.real_profile_autoclose to let Hermes close it for you " + "(closes the browser, losing unsaved tabs)." + ) marker = os.path.join(dst, _SNAPSHOT_DONE_MARKER) # Only a copy that previously COMPLETED counts as populated. A half-written # tree (no marker) is treated as absent and rebuilt — otherwise a torn first diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 526a2f7f3d..4340b57841 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -592,6 +592,15 @@ DEFAULT_CONFIG = { # real-profile local session even under a cloud browser backend. Toggle # in the desktop Settings → Browser section. "use_real_profile": False, + # When real-profile browsing needs the browser closed (Windows: a + # running Chrome/Edge/Brave locks its cookie DB deny-all, so it must be + # fully quit before its profile can be copied), allow Hermes to close + # the browser automatically instead of just failing. DESTRUCTIVE: it + # terminates the browser process tree bound to that profile, losing any + # unsaved tabs/form state — so it is OFF by default and the agent must + # get the user's OK before triggering it. No effect on macOS/Linux, + # where the profile can be copied while the browser runs. + "real_profile_autoclose": False, "allow_unsafe_evaluate": False, # Legacy override: when true, browser_console(expression=...) bypasses the restrict_evaluate denylist entirely "restrict_evaluate": False, # Opt-in denylist blocking sensitive JS primitives (cookies/storage/clipboard/network/form values) in browser_console(expression=...) # CDP supervisor — dialog + frame detection via a persistent WebSocket. diff --git a/tests/hermes_cli/test_real_profile_windows_live.py b/tests/hermes_cli/test_real_profile_windows_live.py new file mode 100644 index 0000000000..53623f5398 --- /dev/null +++ b/tests/hermes_cli/test_real_profile_windows_live.py @@ -0,0 +1,143 @@ +"""LIVE Windows E2E: consented auto-close makes real-profile work with a running browser. + +windows-latest only. Proves the end state: with browser.real_profile_autoclose +on, a REAL running Chrome that share-locks its cookie DB is terminated by +snapshot_real_profile, the lock releases, and a valid signed-in-shaped copy is +produced. Also proves the default (autoclose off) fails fast, not hangs. + +PROOF branch evidence — reverted before merge; never lands on main. +""" +from __future__ import annotations + +import os +import shutil +import sqlite3 +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.skipif(sys.platform != "win32", reason="Windows-only live E2E") + +_CHROME = ( + r"C:\Program Files\Google\Chrome\Application\chrome.exe", + r"C:\Program Files (x86)\Google\Chrome\Application\chrome.exe", +) + + +def _find_chrome(): + for p in _CHROME: + if os.path.isfile(p): + return p + return shutil.which("chrome") or shutil.which("chrome.exe") + + +def _launch_chrome_on(user_data: Path): + proc = subprocess.Popen( + [_find_chrome(), "--headless=new", "--disable-gpu", "--no-first-run", + "--no-default-browser-check", f"--user-data-dir={user_data}", + "--remote-debugging-port=0", "about:blank"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ) + ck = None + deadline = time.time() + 60 + while time.time() < deadline and not ck: + for rel in (r"Default\Network\Cookies", r"Default\Cookies"): + c = user_data / rel + if c.is_file() and c.stat().st_size > 0: + ck = c + break + time.sleep(1) + return proc, ck + + +def _raw_copy_fails(path: str) -> bool: + try: + shutil.copy2(path, path + ".rc"); os.unlink(path + ".rc"); return False + except OSError: + return True + + +def test_autoclose_off_fails_fast(tmp_path): + """Default (autoclose off): a running Chrome → fail fast (<30s) with the + quit/autoclose guidance, never a hang, never a silent copy.""" + if not _find_chrome(): + pytest.skip("no chrome") + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + import hermes_cli.browser_connect as bc + + ud = tmp_path / "ud"; ud.mkdir() + proc, ck = _launch_chrome_on(ud) + try: + assert ck is not None, "no cookie db" + if not _raw_copy_fails(str(ck)): + pytest.skip("Chrome did not share-lock on this runner") + orig = bc.real_profile_data_dir + bc.real_profile_data_dir = lambda b, system=None: str(ud) + bc._real_profile_autoclose = lambda: False + try: + t0 = time.time() + dst, err = bc.snapshot_real_profile("chrome", src=str(ud)) + elapsed = time.time() - t0 + finally: + bc.real_profile_data_dir = orig + assert dst is None and err + assert "quit" in err.lower() and "real_profile_autoclose" in err + assert elapsed < 30, f"hung {elapsed:.0f}s — must fail fast" + finally: + proc.terminate() + try: proc.wait(timeout=15) + except subprocess.TimeoutExpired: proc.kill() + + +def test_autoclose_on_closes_chrome_and_snapshots(tmp_path): + """Consented auto-close: a running Chrome is terminated, the lock releases, + and a valid cookie DB copy is produced — real-profile works WITH the browser + initially running.""" + if not _find_chrome(): + pytest.skip("no chrome") + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + import hermes_cli.browser_connect as bc + + ud = tmp_path / "ud"; ud.mkdir() + proc, ck = _launch_chrome_on(ud) + try: + assert ck is not None, "no cookie db" + if not _raw_copy_fails(str(ck)): + pytest.skip("Chrome did not share-lock on this runner") + + orig = bc.real_profile_data_dir + bc.real_profile_data_dir = lambda b, system=None: str(ud) + bc._real_profile_autoclose = lambda: True + # Isolate the snapshot store under tmp. + orig_home = bc.get_hermes_home + bc.get_hermes_home = lambda: tmp_path / "hh" + try: + dst, err = bc.snapshot_real_profile("chrome", src=str(ud)) + finally: + bc.real_profile_data_dir = orig + bc.get_hermes_home = orig_home + + assert err is None, f"auto-close snapshot failed: {err}" + assert dst is not None + copy_ck = Path(dst) / "Default" / "Cookies" + assert copy_ck.is_file(), "cookie DB not copied after auto-close" + # Valid SQLite with the cookies table. + con = sqlite3.connect(str(copy_ck)) + try: + names = {r[0] for r in con.execute( + "SELECT name FROM sqlite_master WHERE type='table'")} + finally: + con.close() + assert "cookies" in names, f"copied DB missing cookies table: {names}" + # The original Chrome should now be gone (we terminated its tree). + assert proc.poll() is not None, "auto-close did not terminate Chrome" + finally: + try: + if proc.poll() is None: + proc.terminate(); proc.wait(timeout=15) + except Exception: + try: proc.kill() + except Exception: pass diff --git a/tests/tools/test_browser_real_profile.py b/tests/tools/test_browser_real_profile.py index 0bed49336d..144e0b633d 100644 --- a/tests/tools/test_browser_real_profile.py +++ b/tests/tools/test_browser_real_profile.py @@ -745,6 +745,7 @@ class TestReviewRound3: home = tmp_path / "hh" monkeypatch.setattr(bc, "get_hermes_home", lambda: home) monkeypatch.setattr(bc, "_profile_is_locked", lambda s, p: True) + monkeypatch.setattr(bc, "_real_profile_autoclose", lambda: False) called = {"copytree": 0} import shutil as _sh orig_ct = _sh.copytree @@ -754,8 +755,76 @@ class TestReviewRound3: assert dst is None assert err and ("locked" in err.lower() or "running" in err.lower()) assert "quit" in err.lower() + assert "real_profile_autoclose" in err # offers the consented option assert called["copytree"] == 0 # bailed before any copy + def test_autoclose_closes_then_snapshots(self, tmp_path, monkeypatch): + """With autoclose consent on, a locked profile is closed and the + snapshot then proceeds (lock released).""" + import hermes_cli.browser_connect as bc + src = self._multi(tmp_path / "real") + home = tmp_path / "hh" + monkeypatch.setattr(bc, "get_hermes_home", lambda: home) + # Locked on first probe; closer "releases" it; subsequent copy runs. + lock_state = {"locked": True} + monkeypatch.setattr(bc, "_profile_is_locked", + lambda s, p: lock_state["locked"]) + monkeypatch.setattr(bc, "_real_profile_autoclose", lambda: True) + + def fake_close(src_, timeout=15.0): + lock_state["locked"] = False + return True, "closed the browser and the profile lock released." + + monkeypatch.setattr(bc, "close_browser_holding_profile", fake_close) + dst, err = bc.snapshot_real_profile("chrome", src=str(src)) + assert err is None + assert (home / "browser-profile" / "chrome" / "Default" / "Cookies").read_text() == "PROFILE6-SESSION" + + def test_autoclose_failure_reports_clearly(self, tmp_path, monkeypatch): + """If autoclose can't release the lock (relaunch/tray), fail with a + clear message and no copy.""" + import hermes_cli.browser_connect as bc + src = self._multi(tmp_path / "real") + home = tmp_path / "hh" + monkeypatch.setattr(bc, "get_hermes_home", lambda: home) + monkeypatch.setattr(bc, "_profile_is_locked", lambda s, p: True) + monkeypatch.setattr(bc, "_real_profile_autoclose", lambda: True) + monkeypatch.setattr(bc, "close_browser_holding_profile", + lambda s, timeout=15.0: (False, "still locked — relaunched.")) + dst, err = bc.snapshot_real_profile("chrome", src=str(src)) + assert dst is None + assert err and "tried to close it" in err + + def test_processes_holding_profile_identity_binding(self, tmp_path, monkeypatch): + """The process matcher requires BOTH a browser binary AND this exact + user-data-dir in the cmdline — never a same-name process on another dir.""" + import hermes_cli.browser_connect as bc + + class FakeProc: + def __init__(self, name, cmdline): + self.info = {"name": name, "cmdline": cmdline} + + ud = str(tmp_path / "ud") + procs = [ + FakeProc("chrome.exe", ["chrome.exe", f"--user-data-dir={ud}"]), # match + FakeProc("chrome.exe", ["chrome.exe", "--user-data-dir=C:\\Other"]), # wrong dir + FakeProc("python.exe", ["python.exe", f"--user-data-dir={ud}"]), # not a browser + ] + + class FakePsutil: + NoSuchProcess = psutil_exc = type("E", (Exception,), {}) + AccessDenied = type("E2", (Exception,), {}) + + def process_iter(self, attrs=None): + return iter(procs) + + import sys as _sys + monkeypatch.setitem(_sys.modules, "psutil", FakePsutil()) + matched = list(bc._processes_holding_profile(ud)) + assert len(matched) == 1 + assert matched[0].info["name"] == "chrome.exe" + assert f"--user-data-dir={ud}" in " ".join(matched[0].info["cmdline"]) + def test_consent_off_triggers_cleanup(self, tmp_path, monkeypatch): import tools.browser_tool as bt called = {"n": 0} diff --git a/website/docs/user-guide/features/browser.md b/website/docs/user-guide/features/browser.md index 67721d1b23..73a224ef7a 100644 --- a/website/docs/user-guide/features/browser.md +++ b/website/docs/user-guide/features/browser.md @@ -195,6 +195,12 @@ therefore requires the browser **fully quit**, including any background/tray instance (Chrome's "continue running background apps when closed" keeps a `chrome.exe` alive after you close the window). macOS and Linux can copy the profile while the browser is running. + +Set `browser.real_profile_autoclose: true` to let Hermes **close the browser +for you** when it's holding the profile — it terminates the browser process +tree bound to that profile, waits for the lock to release, then continues. This +is destructive (you lose unsaved tabs/form state in that browser), so it is +**off by default** and the agent asks before doing it. ::: - **Supported browsers:** Chrome, Edge, Brave, Chromium (whichever is your OS