From e4451ec6e5923e35dde75aabcb14944b1f15c16a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:10:21 -0700 Subject: [PATCH] feat(browser): close-with-approval flow for Windows real-profile (toggle arms, agent asks, blocked if still locked) [proof do-not-merge] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refines the Windows path per three requirements: 1. Only when the toggle is set — closing is offered only if browser.real_profile_autoclose is on. 2. Blocked when locked — snapshot_real_profile NEVER kills; a locked profile always returns the [profile-locked] signal and the copy is refused. A later attempt that is still locked blocks again (no loop, no auto-kill). 3. Ask approval to close — closing is an explicit, user-approved step: (new CLI subcommand) runs close_browser_holding_profile only when the agent has the user's OK. The locked error tells the agent to ask first, then run it, then retry. - browser_connect: snapshot blocks with _PROFILE_LOCKED_PREFIX (autoclose-armed message offers the close; off message says fully-quit); no in-snapshot kill. - main.py: subcommand (identity+binding-verified tree kill via close_browser_holding_profile); added to _BUILTIN_SUBCOMMANDS. - browser_tool: surfaces the locked signal + the exact approved-close command. - Docs/config: toggle arms + agent asks + blocked-if-still-locked. Tests: snapshot blocks-not-kills with autoclose on AND off; process matcher identity/binding. 73 real-profile tests pass. Windows live E2E (proof): locked blocks fast without killing → approved close terminates Chrome → snapshot then copies a valid DB; autoclose-off blocks with quit guidance. --- .github/workflows/windows-realprofile-e2e.yml | 62 +++++++ hermes_cli/browser_connect.py | 43 +++-- hermes_cli/config_defaults.py | 13 +- hermes_cli/main.py | 56 +++++++ .../test_real_profile_windows_live.py | 156 ++++++++++++++++++ tests/tools/test_browser_real_profile.py | 43 ++--- tools/browser_tool.py | 13 ++ website/docs/user-guide/features/browser.md | 13 +- 8 files changed, 339 insertions(+), 60 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..2e8c7307df --- /dev/null +++ b/.github/workflows/windows-realprofile-e2e.yml @@ -0,0 +1,62 @@ +name: Windows real-profile live E2E + +# ON-DEMAND PROOF (real-profile browsing, PR #95620 — Windows close-with-approval). +# +# Proves on a REAL windows-latest runner that a running Chrome holding the +# profile makes snapshot BLOCK (never kill/hang), the explicit approved close +# then terminates Chrome and releases the lock, and snapshot then succeeds. +# +# 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 close-with-approval 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 close-with-approval 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 6c00c3c805..5db489206a 100644 --- a/hermes_cli/browser_connect.py +++ b/hermes_cli/browser_connect.py @@ -629,6 +629,11 @@ def _mirror_profile_auth(src: str, dst: str, source_profile: str) -> int: _SNAPSHOT_DONE_MARKER = ".hermes-snapshot-complete" +# Prefix stamped on the "profile is locked" error so the calling layer can +# recognize it as the specific needs-the-browser-closed condition (vs a generic +# snapshot failure) and surface the close-with-approval flow. +_PROFILE_LOCKED_PREFIX = "[profile-locked] " + def _profile_cookie_db(src: str, source_profile: str) -> str | None: """Path to the active profile's cookie DB (modern Network/ first).""" @@ -806,29 +811,31 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N # minutes. On POSIX this never trips (no mandatory locking) so # copy-while-running still works. if _profile_is_locked(src, source_profile): - # 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. + # NEVER kill from here. Closing the user's browser is destructive and + # must be an explicit, per-attempt, user-approved step — not a silent + # side effect of a snapshot. So we always BLOCK when locked and let the + # agent decide whether to ask the user to close it (only offered when + # browser.real_profile_autoclose arms the capability). A subsequent + # attempt that is still locked blocks again — no auto-retry, no loop. 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 + msg = ( + f"{browser} is running and has its profile locked, so its login " + "data can't be copied yet. Hermes can close it for you " + "(this quits the browser — you'll lose unsaved tabs). Ask the " + "user to confirm, then close it and retry; if it's still locked " + "after that, they must fully quit it (including any " + "background/tray instance)." + ) else: - return None, ( + msg = ( 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)." + "background/tray instance) and retry, or turn " + "browser.use_real_profile off. (Enable " + "browser.real_profile_autoclose to let Hermes offer to close it " + "for you.)" ) + return None, _PROFILE_LOCKED_PREFIX + msg 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 4340b57841..cc24a10ec5 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -594,12 +594,13 @@ DEFAULT_CONFIG = { "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. + # fully quit before its profile can be copied), arm the "offer to close + # it" flow. This does NOT auto-kill: when the profile is locked the + # snapshot always blocks and the agent asks the user first; only on + # approval does it run `hermes browser close-profile` (terminates the + # browser process tree bound to that profile, losing unsaved tabs) and + # retry. Still locked afterward → stays blocked, no loop, no auto-kill. + # OFF by default. No effect on macOS/Linux (copy-while-running works). "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=...) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 92c37c0657..b5774fa13f 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -12351,6 +12351,7 @@ _BUILTIN_SUBCOMMANDS = frozenset( "send", "sessions", "setup", "skin", "skills", "slack", "status", "sync", "tools", "uninstall", "update", "webhook", "whatsapp", "whatsapp-cloud", "worktree", "chat", "secrets", "security", + "browser", "verify", # Help-ish invocations — plugin commands not being listed in # top-level --help is an acceptable trade-off for skipping an @@ -13248,6 +13249,61 @@ def main(): worktree_parser.set_defaults(func=_dispatch_worktree) + # ========================================================================= + # browser command — real-profile helpers (agent-invoked, user-approved) + # ========================================================================= + browser_parser = subparsers.add_parser( + "browser", + help="Real-profile browsing helpers (close a browser locking its profile)", + description=( + "Helpers for real-profile browsing (browser.use_real_profile). " + "close-profile terminates the browser process tree holding your " + "default profile so Hermes can copy it — DESTRUCTIVE (unsaved tabs " + "in that browser are lost). The agent runs this only after you " + "approve closing the browser." + ), + ) + browser_subparsers = browser_parser.add_subparsers(dest="browser_action") + browser_close = browser_subparsers.add_parser( + "close-profile", + help="Close the browser locking your real profile (asks nothing — " + "run only with the user's explicit OK; loses unsaved tabs)", + ) + browser_close.add_argument( + "--browser", + help="Override detected default browser (chrome/edge/brave/chromium)", + ) + + def _dispatch_browser(_args): + from hermes_cli.browser_connect import ( + UNSUPPORTED_CHANNEL, + close_browser_holding_profile, + detect_default_chromium, + real_profile_data_dir, + ) + + action = getattr(_args, "browser_action", None) + if action != "close-profile": + browser_parser.print_help() + return 2 + browser = getattr(_args, "browser", None) or detect_default_chromium() + if not browser or browser == UNSUPPORTED_CHANNEL: + print("✗ No supported Chromium default browser detected.", file=sys.stderr) + return 1 + src = real_profile_data_dir(browser) + if not src: + print(f"✗ Could not resolve the {browser} profile directory.", file=sys.stderr) + return 1 + closed, msg = close_browser_holding_profile(src) + if closed: + print(f"✓ {msg}") + return 0 + print(f"✗ {msg}", file=sys.stderr) + return 1 + + browser_parser.set_defaults(func=_dispatch_browser) + + # ========================================================================= # secrets command — external secret managers (Bitwarden, 1Password) # ========================================================================= 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..f396f1a7e2 --- /dev/null +++ b/tests/hermes_cli/test_real_profile_windows_live.py @@ -0,0 +1,156 @@ +"""LIVE Windows E2E: locked-profile blocks; explicit approved close then works. + +windows-latest only. Proves the three-part contract: + 1. A running Chrome that share-locks its profile makes snapshot_real_profile + BLOCK (never kill, never hang) with the [profile-locked] signal. + 2. The explicit, user-approved close step (close_browser_holding_profile, + what `hermes browser close-profile` runs) terminates the browser and the + lock releases. + 3. After the close, snapshot_real_profile succeeds and copies a valid DB. + +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_locked_blocks_then_approved_close_then_snapshots(tmp_path): + 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_dd = bc.real_profile_data_dir + orig_home = bc.get_hermes_home + bc.real_profile_data_dir = lambda b, system=None: str(ud) + bc.get_hermes_home = lambda: tmp_path / "hh" + # autoclose armed → message OFFERS the close, but snapshot must NOT kill. + bc._real_profile_autoclose = lambda: True + try: + # 1. Locked → blocks fast with the [profile-locked] signal, no kill. + t0 = time.time() + dst, err = bc.snapshot_real_profile("chrome", src=str(ud)) + elapsed = time.time() - t0 + assert dst is None and err + assert err.startswith(bc._PROFILE_LOCKED_PREFIX), f"not the locked signal: {err}" + assert elapsed < 30, f"blocked slowly ({elapsed:.0f}s) — must be fast" + assert proc.poll() is None, "snapshot must NOT have killed Chrome on its own" + + # 2. Explicit approved close (the engine `hermes browser + # close-profile` runs) terminates Chrome; lock releases. + closed, msg = bc.close_browser_holding_profile(str(ud)) + assert closed, f"approved close failed: {msg}" + assert proc.poll() is not None, "close did not terminate Chrome" + + # 3. Snapshot now succeeds with a valid cookie DB. + dst2, err2 = bc.snapshot_real_profile("chrome", src=str(ud)) + assert err2 is None, f"post-close snapshot failed: {err2}" + assert dst2 is not None + cands = [Path(dst2) / "Default" / "Network" / "Cookies", + Path(dst2) / "Default" / "Cookies"] + copy_ck = next((c for c in cands if c.is_file()), None) + assert copy_ck is not None, ( + "cookie DB not copied after approved close; Default: " + + repr(sorted(os.listdir(Path(dst2) / "Default")) + if (Path(dst2) / "Default").is_dir() else "NONE")) + 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"missing cookies table: {names}" + finally: + bc.real_profile_data_dir = orig_dd + bc.get_hermes_home = orig_home + finally: + try: + if proc.poll() is None: + proc.terminate(); proc.wait(timeout=15) + except Exception: + try: proc.kill() + except Exception: pass + + +def test_autoclose_off_blocks_with_quit_guidance(tmp_path): + 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: + dst, err = bc.snapshot_real_profile("chrome", src=str(ud)) + assert dst is None and err and err.startswith(bc._PROFILE_LOCKED_PREFIX) + assert "quit" in err.lower() + assert proc.poll() is None, "must not kill Chrome when autoclose off" + finally: + bc.real_profile_data_dir = orig + finally: + proc.terminate() + try: proc.wait(timeout=15) + except subprocess.TimeoutExpired: proc.kill() diff --git a/tests/tools/test_browser_real_profile.py b/tests/tools/test_browser_real_profile.py index 144e0b633d..c66e409081 100644 --- a/tests/tools/test_browser_real_profile.py +++ b/tests/tools/test_browser_real_profile.py @@ -738,8 +738,8 @@ class TestReviewRound3: assert bc._profile_is_locked(str(tmp_path), "Default") is True def test_snapshot_fails_fast_when_locked(self, tmp_path, monkeypatch): - """snapshot_real_profile bails with the actionable message when the - active profile's cookie DB is locked — never proceeds to a heavy copy.""" + """snapshot_real_profile always BLOCKS when locked — never kills, never + proceeds to a heavy copy. autoclose off → plain quit guidance.""" import hermes_cli.browser_connect as bc src = self._multi(tmp_path / "real") home = tmp_path / "hh" @@ -753,47 +753,28 @@ class TestReviewRound3: lambda *a, **k: (called.__setitem__("copytree", called["copytree"] + 1), orig_ct(*a, **k))[1]) dst, err = bc.snapshot_real_profile("chrome", src=str(src)) assert dst is None - assert err and ("locked" in err.lower() or "running" in err.lower()) + assert err and err.startswith(bc._PROFILE_LOCKED_PREFIX) 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.""" + def test_snapshot_blocks_when_locked_even_with_autoclose(self, tmp_path, monkeypatch): + """Even with autoclose armed, snapshot_real_profile does NOT kill — it + blocks and defers the close to the explicit, user-approved step. The + message offers the close (mentions Hermes can close it).""" 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) + killed = {"n": 0} monkeypatch.setattr(bc, "close_browser_holding_profile", - lambda s, timeout=15.0: (False, "still locked — relaunched.")) + lambda *a, **k: (killed.__setitem__("n", killed["n"] + 1), (True, "x"))[1]) dst, err = bc.snapshot_real_profile("chrome", src=str(src)) assert dst is None - assert err and "tried to close it" in err + assert err and err.startswith(bc._PROFILE_LOCKED_PREFIX) + assert "close it for you" in err.lower() or "can close it" in err.lower() + assert killed["n"] == 0 # snapshot must NOT invoke the killer itself def test_processes_holding_profile_identity_binding(self, tmp_path, monkeypatch): """The process matcher requires BOTH a browser binary AND this exact diff --git a/tools/browser_tool.py b/tools/browser_tool.py index d1b7ab6cf0..4f01801fe6 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -1635,6 +1635,19 @@ def _real_profile_cdp() -> tuple: # No live browser owns the dir now — safe to (re)snapshot + overlay. snap_dir, err = snapshot_real_profile(browser) if err or not snap_dir: + from hermes_cli.browser_connect import _PROFILE_LOCKED_PREFIX + + if err and err.startswith(_PROFILE_LOCKED_PREFIX): + # The user's browser is holding the profile. Surface the guidance + # verbatim (it already tells the agent whether closing is armed) + # plus the exact approved-close command. The agent must ASK the + # user before running it — it quits their browser. + body = err[len(_PROFILE_LOCKED_PREFIX):] + return None, ( + body + " To close it (only after the user approves — it " + "quits their browser and loses unsaved tabs), run: " + "`hermes browser close-profile`, then retry." + ) return None, f"browser.use_real_profile is on, but {err}" copy_dir = snap_dir diff --git a/website/docs/user-guide/features/browser.md b/website/docs/user-guide/features/browser.md index 73a224ef7a..16a7e3cfbe 100644 --- a/website/docs/user-guide/features/browser.md +++ b/website/docs/user-guide/features/browser.md @@ -196,11 +196,14 @@ 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. +Set `browser.real_profile_autoclose: true` to let Hermes **offer to close the +browser for you** when it's holding the profile. Even with this on, Hermes never +closes it automatically — when the profile is locked it always stops and the +agent asks you first; only on your approval does it run `hermes browser +close-profile` (terminates the browser process tree bound to that profile, +losing unsaved tabs), then retries. If the profile is still locked after that +(e.g. a background/tray instance relaunched), Hermes stays blocked and tells you +to fully quit the browser — it won't loop or kill again on its own. ::: - **Supported browsers:** Chrome, Edge, Brave, Chromium (whichever is your OS