fix(browser): real-profile snapshot names the locked auth databases and the reason

On macOS/Linux a running Chrome lets `Cookies` back up but holds `Login Data`, `Login Data For Account` and `Web Data` with a hot write lock, so their SQLite online backup misses the five-second deadline. The launch failed closed with a bare "3 database(s) unavailable" count, and the docs still promised that macOS and Linux "can copy the profile while the browser is running" (#111647). - `_copy_auth_file` returns the failure reason (None on success) instead of a bool; the backup deadline is the named constant `_AUTH_BACKUP_DEADLINE_S` and raises `_AUTH_DB_LOCKED`, so the caller can tell "browser holds a write lock" from "file is not a database". - `_mirror_profile_auth` returns `{db name: reason}`; the new `_unavailable_auth_dbs_error` renders "chrome is running and holds the profile's Login Data, Login Data For Account, Web Data with a write lock … Fully quit chrome and retry" when every failure is the deadline, and the per-database SQLite error otherwise. No raw-copy fallback is added: the no-fallback stance from 58d6d5223a (WAL consistency) is unchanged. - Docs: the Real-profile note no longer says macOS/Linux copy live; it states which databases need the browser quit and what the error looks like. The `real_profile_autoclose` comment stops claiming copy-while-running works on POSIX.
2026-09-16 09:58:37 -07:00
parent 9cb1a65978
commit 864e9dccfc
5 changed files with 146 additions and 33 deletions
--- a/hermes_cli/browser_connect.py
+++ b/hermes_cli/browser_connect.py
@@ -391,18 +391,25 @@ def _secure_snapshot(path: str, *, contents: bool = False) -> None:
 # exclusive lock, so a raw copy raises WinError 32 and a best-effort skip leaves the copy
 # signed-out. They are copied via SQLite's online-backup API instead. Matched by basename.
 _SQLITE_AUTH_DBS = frozenset({"Cookies", "Login Data", "Login Data For Account", "Web Data"})
+# Budget for one auth DB's online backup. A running browser holds Login Data / Login Data For
+# Account / Web Data with a hot write lock, so backup makes no progress and this deadline is
+# what fails — the reason users see, so it is a named constant rather than a bare string.
+_AUTH_BACKUP_DEADLINE_S = 5.0
+_AUTH_DB_LOCKED = ("SQLite backup made no progress within five seconds — "
+                   "a running browser holds a write lock on it")


-def _copy_auth_file(src_file: str, dst_file: str) -> bool:
-    """Copy auth state; refuse a DB that cannot be snapshotted consistently within five seconds."""
+def _copy_auth_file(src_file: str, dst_file: str) -> str | None:
+    """Copy auth state; returns None on success, else WHY the file could not be snapshotted.
+    A DB that cannot be backed up consistently within the deadline is refused, never raw-copied."""
    os.makedirs(os.path.dirname(dst_file), exist_ok=True)
    try:
        if os.path.basename(src_file) in _SQLITE_AUTH_DBS:
-            deadline = time.monotonic() + 5.0
+            deadline = time.monotonic() + _AUTH_BACKUP_DEADLINE_S

            def check_deadline(_status: int, _remaining: int, _total: int) -> None:
                if _status != sqlite3.SQLITE_DONE and time.monotonic() >= deadline:
-                    raise TimeoutError("auth database backup exceeded five seconds")
+                    raise TimeoutError(_AUTH_DB_LOCKED)

            # SQLite must coordinate both ends: immutable ignores committed source WAL,
            # while replacing only the destination file can replay its abandoned WAL.
@@ -413,22 +420,42 @@ def _copy_auth_file(src_file: str, dst_file: str) -> bool:
                    source.backup(out, pages=256, progress=check_deadline, sleep=0.1)
        else:
            shutil.copy2(src_file, dst_file)
-        return True
+        return None
    except (OSError, sqlite3.Error) as e:
        # A raw DB copy can lose committed WAL or overwrite a locked destination.
        logger.debug("real-profile: could not copy %s: %s", src_file, e)
-        return False
+        return str(e) or type(e).__name__


-def _mirror_profile_auth(src: str, dst: str, source_profile: str) -> int:
+def _mirror_profile_auth(src: str, dst: str, source_profile: str) -> dict[str, str]:
    """Mirror ``source_profile``'s auth files into the copy's ``Default`` (agent-browser opens it);
-    returns the number of DB auth files that could NOT be copied (0 = clean)."""
-    failed_dbs = 0
+    returns ``{relative name: reason}`` for the DB auth files that could NOT be copied ({} = clean)."""
+    failed: dict[str, str] = {}
    for rel in _AUTH_REFRESH_PROFILE_FILES:
        s = os.path.join(src, source_profile, rel)
-        if os.path.isfile(s) and not _copy_auth_file(s, os.path.join(dst, "Default", rel)):
-            failed_dbs += os.path.basename(rel) in _SQLITE_AUTH_DBS
-    return failed_dbs
+        if not os.path.isfile(s):
+            continue
+        reason = _copy_auth_file(s, os.path.join(dst, "Default", rel))
+        if reason and os.path.basename(rel) in _SQLITE_AUTH_DBS:
+            failed[rel] = reason
+    return failed
+
+
+def _unavailable_auth_dbs_error(browser: str, failed: dict[str, str]) -> str:
+    """Fail-closed message naming WHICH auth databases could not be snapshotted and WHY. On
+    macOS/Linux a running browser typically lets Cookies back up but holds Login Data / Login Data
+    For Account / Web Data with a write lock, so the message must not read as "close the browser"
+    when the real cause is an unreadable file (and vice versa)."""
+    names = ", ".join(failed)
+    if all(reason == _AUTH_DB_LOCKED for reason in failed.values()):
+        return (f"{browser} is running and holds the profile's {names} with a write lock, so their "
+                "SQLite backup made no progress within five seconds. Hermes does not fall back to a "
+                "raw file copy (it could lose committed logins). Fully quit "
+                f"{browser} (including any background instance) and retry, or turn "
+                "browser.use_real_profile off.")
+    details = "; ".join(f"{name}: {reason}" for name, reason in failed.items())
+    return (f"could not read the '{browser}' profile's login data ({details}). "
+            f"Close {browser} and retry, or turn browser.use_real_profile off.")


 _SNAPSHOT_DONE_MARKER = ".hermes-snapshot-complete"
@@ -642,7 +669,8 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N
        return None, resolve_err
    dst = real_profile_copy_dir(browser)
    # Fast lock probe BEFORE any copy: a blocking file op on a Windows-locked cookie DB can
-    # hang the launch for minutes. Never trips on POSIX, so copy-while-running still works.
+    # hang the launch for minutes. Never trips on POSIX; there a running browser surfaces later as
+    # auth DB backups that miss their deadline (``_unavailable_auth_dbs_error``).
    if _profile_is_locked(src, source_profile):
        return None, _locked_profile_error(browser)
    marker = os.path.join(dst, _SNAPSHOT_DONE_MARKER)
@@ -661,9 +689,7 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N
        # Both paths: lock-aware auth DB copy into Default — also the per-launch re-sync.
        failed_dbs = _mirror_profile_auth(src, dst, source_profile)
        if failed_dbs:  # even online-backup failed: never launch a silently signed-out session
-            return None, (f"could not read the '{browser}' profile's login data ({failed_dbs} "
-                          f"database(s) unavailable). Close {browser} and retry, or turn "
-                          "browser.use_real_profile off.")
+            return None, _unavailable_auth_dbs_error(browser, failed_dbs)
        # Never carry live-instance leftovers into the copy.
        for leftover in ("SingletonLock", "SingletonSocket", "SingletonCookie"):
            with contextlib.suppress(OSError):
--- a/hermes_cli/config_defaults.py
+++ b/hermes_cli/config_defaults.py
@@ -411,8 +411,9 @@ DEFAULT_CONFIG = {
        # Windows only: a running Chrome/Edge/Brave locks its cookie DB, so the profile can't be
        # copied. When on, a locked profile still blocks and the agent ASKS first; on approval it
        # runs `hermes browser close-profile` (kills that profile's browser tree, unsaved tabs lost)
-        # and retries once; still locked -> stays blocked, no auto-kill. No effect on macOS/Linux
-        # (copy-while-running works).
+        # and retries once; still locked -> stays blocked, no auto-kill. No effect on macOS/Linux,
+        # where a running browser instead makes the Login Data / Web Data SQLite backups miss
+        # their deadline; quit the browser by hand there.
        "real_profile_autoclose": False,
        # Pin WHICH source profile directory is snapshotted for real-profile browsing (e.g. "Profile
        # 2"). Empty = browser's last-used profile, which on multi-profile machines can hand the
--- a/tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py
+++ b/tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py
@@ -0,0 +1,76 @@
+"""Real-profile snapshot error names the auth databases it could not read and why.
+
+A running Chrome on macOS/Linux lets ``Cookies`` back up but holds ``Login Data`` /
+``Login Data For Account`` / ``Web Data`` with a write lock, so their SQLite online backup misses
+its deadline. The launch must fail closed with the database NAMES and the lock reason, never a
+bare "3 database(s) unavailable" count (#111647).
+"""
+import json
+import os
+import sqlite3
+
+import pytest
+
+import hermes_cli.browser_connect as bc
+
+_LOCKED = ("Login Data", "Login Data For Account", "Web Data")
+
+
+def _fake_profile(root):
+    (root / "Default" / "Network").mkdir(parents=True)
+    (root / "Local State").write_text(json.dumps({"profile": {"last_used": "Default"}}))
+    (root / "Default" / "Preferences").write_text("{}")
+    for name in ("Cookies", *_LOCKED):
+        con = sqlite3.connect(root / "Default" / name)
+        con.execute("create table t(x)")
+        con.execute("insert into t values(1)")
+        con.commit()
+        con.close()
+
+
+def test_mirror_profile_auth_reports_locked_dbs_by_name(tmp_path, monkeypatch):
+    """Exclusive writers on the three login/autofill DBs → each is reported with the lock reason;
+    the unlocked Cookies DB still lands in the snapshot (no all-or-nothing regression)."""
+    root, dst = tmp_path / "real", tmp_path / "copy"
+    _fake_profile(root)
+    monkeypatch.setattr(bc, "_AUTH_BACKUP_DEADLINE_S", 0.3)  # keep the test fast; 5 s in prod
+    holders = []
+    for name in _LOCKED:
+        h = sqlite3.connect(root / "Default" / name)
+        h.execute("begin exclusive")
+        holders.append(h)
+    try:
+        failed = bc._mirror_profile_auth(str(root), str(dst), "Default")
+    finally:
+        for h in holders:
+            h.rollback()
+            h.close()
+    assert failed == {name: bc._AUTH_DB_LOCKED for name in _LOCKED}
+    assert os.path.isfile(dst / "Default" / "Cookies")
+    # Locks released → clean re-sync.
+    assert bc._mirror_profile_auth(str(root), str(dst), "Default") == {}
+
+
+@pytest.mark.parametrize("reason, expect", [
+    (None, ("is running and holds the profile's Login Data, Login Data For Account, Web Data "
+            "with a write lock", "Fully quit chrome")),
+    ("file is not a database", ("Login Data: file is not a database", "Close chrome")),
+])
+def test_snapshot_error_names_databases_and_reason(tmp_path, monkeypatch, reason, expect):
+    """The user-facing snapshot error carries the failed DB names plus the reason: the lock
+    wording when every failure is the backup deadline, the SQLite error otherwise."""
+    root = tmp_path / "real"
+    _fake_profile(root)
+    monkeypatch.setattr(bc, "get_hermes_home", lambda: tmp_path / "hh")
+    locked_reason = reason or bc._AUTH_DB_LOCKED
+
+    def fake_copy(src, dst_file):
+        return locked_reason if os.path.basename(src) in _LOCKED else None
+
+    monkeypatch.setattr(bc, "_copy_auth_file", fake_copy)
+    dst, err = bc.snapshot_real_profile("chrome", src=str(root))
+    assert dst is None and err
+    for fragment in expect:
+        assert fragment in err, err
+    assert "database(s) unavailable" not in err
+    assert "Cookies" not in err  # only the databases that actually failed are named
--- a/tests/tools/test_browser_real_profile.py
+++ b/tests/tools/test_browser_real_profile.py
@@ -1066,7 +1066,7 @@ class TestWindowsLockedProfileCopy:
        src = str(tmp_path / "Cookies")
        con = sqlite3.connect(src); con.execute("create table cookies(x)"); con.execute("insert into cookies values(1)"); con.commit(); con.close()
        dst = str(tmp_path / "out" / "Cookies")
-        assert bc._copy_auth_file(src, dst) is True
+        assert bc._copy_auth_file(src, dst) is None
        assert sqlite3.connect(dst).execute("select count(*) from cookies").fetchone()[0] == 1

    @pytest.mark.parametrize("locked", ["source", "destination"])
@@ -1093,14 +1093,14 @@ class TestWindowsLockedProfileCopy:
                 str(src), str(dst)],
                capture_output=True, text=True, timeout=15, stdin=subprocess.DEVNULL)
            assert result.returncode == 0, result.stderr
-            assert result.stdout.strip() == "False"
+            assert result.stdout.strip() == bc._AUTH_DB_LOCKED
        finally:
            holder.rollback()
            holder.close()
        with sqlite3.connect(dst) as conn:
            assert conn.execute("select x from cookies").fetchall() == [(99,)]
        conn.close()
-        assert bc._copy_auth_file(str(src), str(dst)) is True
+        assert bc._copy_auth_file(str(src), str(dst)) is None
        with sqlite3.connect(dst) as conn:
            assert conn.execute("select x from cookies").fetchall() == [(7,)]
        conn.close()
@@ -1129,7 +1129,7 @@ class TestWindowsLockedProfileCopy:
             str(dst)], check=True, timeout=15, stdin=subprocess.DEVNULL)
        assert os.path.exists(str(dst) + "-wal")
        try:
-            assert bc._copy_auth_file(str(src), str(dst)) is True
+            assert bc._copy_auth_file(str(src), str(dst)) is None
            with sqlite3.connect(dst) as conn:
                assert conn.execute("select x from cookies").fetchall() == [(8,)]
            conn.close()
@@ -1140,7 +1140,7 @@ class TestWindowsLockedProfileCopy:
        import hermes_cli.browser_connect as bc
        src = str(tmp_path / "Preferences"); open(src, "w").write('{"k":1}')
        dst = str(tmp_path / "out" / "Preferences")
-        assert bc._copy_auth_file(src, dst) is True
+        assert bc._copy_auth_file(src, dst) is None
        assert open(dst).read() == '{"k":1}'

    def test_fail_closed_when_db_unreadable(self, tmp_path, monkeypatch):
@@ -1157,7 +1157,7 @@ class TestWindowsLockedProfileCopy:
        monkeypatch.setattr(bc, "get_hermes_home", lambda: home)
        # Force both sqlite-backup and raw copy to fail for the DB.
        monkeypatch.setattr(bc, "_copy_auth_file",
-                            lambda s, d: False if os.path.basename(s) in bc._SQLITE_AUTH_DBS else True)
+                            lambda s, d: "file is not a database" if os.path.basename(s) in bc._SQLITE_AUTH_DBS else None)
        dst, err = bc.snapshot_real_profile("chrome", src=str(root))
        assert dst is None
        assert err and "login data" in err.lower() and "close" in err.lower()
--- a/website/docs/user-guide/features/browser.md
+++ b/website/docs/user-guide/features/browser.md
@@ -217,23 +217,33 @@ When you turn the toggle back off, Hermes deletes the snapshot store
 (`~/.hermes/browser-profile/`) on the next browser use, so the copied
 credentials don't linger after you revoke consent.

-:::note Windows: the browser must be fully closed
+:::note A running browser can block the login/autofill databases
 On Windows a running Chrome/Edge/Brave holds its cookie and login databases with
 an exclusive (deny-all) lock, so Hermes cannot copy them while the browser is
 open — it fails fast with a "fully quit the browser and retry" message rather
 than hang or produce a signed-out session. Real-profile browsing on Windows
 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 usually copy
-the profile while the browser is running. On every platform, each authentication
-database backup has a five-second retry budget. If the source or snapshot database
-stays locked, Hermes stops the launch and asks you to close the browser and retry.
-It preserves committed WAL data through SQLite rather than falling back to a raw
-file copy, which could silently lose recent logins. Unreadable or corrupt databases
-also stop the launch.
+`chrome.exe` alive after you close the window).
+
+On macOS and Linux the profile is not file-locked, but each authentication
+database is snapshotted through SQLite's online backup with a five-second
+budget, and a running Chrome typically holds `Login Data`, `Login Data For
+Account` and `Web Data` with a hot write lock that never yields within that
+budget (`Cookies` usually snapshots fine). When that happens Hermes stops the
+launch and names the databases it could not read — for example "chrome is
+running and holds the profile's Login Data, Login Data For Account, Web Data
+with a write lock" — so in practice **quit the browser before launching a
+real-profile session on macOS/Linux too**. You can reopen it once the session is
+up (the snapshot is a separate directory), but the auth files are re-synced on
+every fresh session launch, so a browser left running then hits the same lock.
+Hermes preserves committed
+WAL data through SQLite rather than falling back to a raw file copy, which could
+silently lose recent logins. Unreadable or corrupt databases also stop the
+launch, with the SQLite error named in the message.

 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
+browser for you** when it's holding the profile lock (the Windows case). 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,