diff --git a/browser_cli/cli.py b/browser_cli/cli.py index 3faa419..a4b9a33 100755 --- a/browser_cli/cli.py +++ b/browser_cli/cli.py @@ -34,6 +34,7 @@ from browser_cli.client import ( save_remote_token, ) from browser_cli.platform import install_base_dir, is_windows +from browser_cli.registry import load_registry console = Console() @@ -231,10 +232,7 @@ def clients_group(ctx): else: profiles: dict[str, str] = {} if REGISTRY_PATH.exists(): - try: - profiles = json.loads(REGISTRY_PATH.read_text()) - except Exception: - pass + profiles = load_registry(REGISTRY_PATH) for profile_name, sock_path in profiles.items(): display_profile = display_browser_name(profile_name, sock_path) diff --git a/browser_cli/client.py b/browser_cli/client.py index 45fe403..61faeda 100644 --- a/browser_cli/client.py +++ b/browser_cli/client.py @@ -19,6 +19,7 @@ from pathlib import Path from typing import Any from browser_cli.platform import endpoint_for_alias, is_windows, registry_path +from browser_cli.registry import load_registry REGISTRY_PATH = registry_path() REMOTE_REGISTRY_PATH = Path(os.environ.get("XDG_CONFIG_HOME", Path.home() / ".config")) / "browser-cli" / "remotes.json" @@ -118,10 +119,7 @@ def _remote_browser_targets() -> list[BrowserTarget]: def active_browser_targets(*, include_remotes: bool = True) -> list[BrowserTarget]: targets: list[BrowserTarget] = [] if REGISTRY_PATH.exists(): - try: - reg = json.loads(REGISTRY_PATH.read_text()) - except Exception: - reg = {} + reg = load_registry(REGISTRY_PATH) targets.extend( BrowserTarget(profile=profile, display_name=display_browser_name(profile, sock_path), socket_path=sock_path) for profile, sock_path in _active_endpoints(reg).items() @@ -137,12 +135,9 @@ def _resolve_socket(profile: str | None = None) -> str: if target: if REGISTRY_PATH.exists(): - try: - reg = json.loads(REGISTRY_PATH.read_text()) - if target in reg: - return reg[target] - except Exception: - pass + reg = load_registry(REGISTRY_PATH) + if target in reg: + return reg[target] return endpoint_for_alias(target) # Auto-detect: error when multiple browser instances are active diff --git a/browser_cli/native_host.py b/browser_cli/native_host.py index 1fae9c3..a175442 100644 --- a/browser_cli/native_host.py +++ b/browser_cli/native_host.py @@ -18,6 +18,7 @@ from multiprocessing.connection import Listener from pathlib import Path from browser_cli.platform import DEFAULT_ALIAS, endpoint_for_alias, is_windows, registry_path, runtime_dir +from browser_cli.registry import update_registry SOCKET_PATH: str = "" # set after hello handshake PENDING: dict[str, queue.Queue] = {} @@ -67,20 +68,14 @@ def write_native_message(stream, msg: dict) -> None: def _registry_add(alias: str, sock_path: str) -> None: try: - reg = json.loads(REGISTRY_PATH.read_text()) if REGISTRY_PATH.exists() else {} - reg[alias] = sock_path - REGISTRY_PATH.write_text(json.dumps(reg)) + update_registry(alias, sock_path, REGISTRY_PATH) except Exception: pass def _registry_remove(alias: str) -> None: try: - if not REGISTRY_PATH.exists(): - return - reg = json.loads(REGISTRY_PATH.read_text()) - reg.pop(alias, None) - REGISTRY_PATH.write_text(json.dumps(reg)) + update_registry(alias, None, REGISTRY_PATH) except Exception: pass diff --git a/browser_cli/registry.py b/browser_cli/registry.py new file mode 100644 index 0000000..d8beb03 --- /dev/null +++ b/browser_cli/registry.py @@ -0,0 +1,99 @@ +"""Runtime registry helpers for active browser-cli native host endpoints.""" +import contextlib +import json +import os +import tempfile +from pathlib import Path +from typing import Iterator + +from browser_cli.platform import registry_path + +REGISTRY_PATH = registry_path() + +@contextlib.contextmanager +def _file_lock(path: Path) -> Iterator[None]: + """Best-effort cross-process lock for registry read/modify/write updates.""" + path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + lock_path = path.with_suffix(path.suffix + ".lock") + with lock_path.open("a+") as lock_file: + if os.name == "nt": + try: + import msvcrt + + msvcrt.locking(lock_file.fileno(), msvcrt.LK_LOCK, 1) + yield + finally: + try: + msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1) + except OSError: + pass + else: + try: + import fcntl + + fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX) + yield + finally: + try: + fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) + except OSError: + pass + +def _coerce_registry(data) -> dict[str, str]: + if not isinstance(data, dict): + return {} + return {str(alias): str(endpoint) for alias, endpoint in data.items() if alias and endpoint} + +def load_registry(path: Path | None = None) -> dict[str, str]: + """Load the active browser registry. + + Older native hosts wrote this file non-atomically, so tolerate trailing + garbage from interrupted/concurrent writes and keep the first valid JSON + object when possible. + """ + registry = path or REGISTRY_PATH + if not registry.exists(): + return {} + try: + text = registry.read_text(encoding="utf-8") + except OSError: + return {} + if not text.strip(): + return {} + try: + return _coerce_registry(json.loads(text)) + except json.JSONDecodeError: + try: + data, _ = json.JSONDecoder().raw_decode(text) + return _coerce_registry(data) + except json.JSONDecodeError: + return {} + +def save_registry(data: dict[str, str], path: Path | None = None) -> None: + """Atomically write the active browser registry.""" + registry = path or REGISTRY_PATH + registry.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + payload = json.dumps(_coerce_registry(data), sort_keys=True) + fd, tmp_name = tempfile.mkstemp(prefix=registry.name + ".", suffix=".tmp", dir=registry.parent) + try: + with os.fdopen(fd, "w", encoding="utf-8") as tmp: + tmp.write(payload) + tmp.flush() + os.fsync(tmp.fileno()) + os.replace(tmp_name, registry) + finally: + try: + os.unlink(tmp_name) + except FileNotFoundError: + pass + +def update_registry(alias: str, endpoint: str | None, path: Path | None = None) -> None: + """Add/update an alias, or remove it when endpoint is None.""" + registry = path or REGISTRY_PATH + with _file_lock(registry): + data = load_registry(registry) + if endpoint is None: + data.pop(alias, None) + else: + data[alias] = endpoint + save_registry(data, registry) diff --git a/tests/test_cli.py b/tests/test_cli.py index b9af573..d3b433b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -141,6 +141,25 @@ def test_clients_exits_cleanly_when_registry_is_missing(): assert result.exit_code == 1 assert "No browser clients found" in result.output + +def test_clients_reads_registry_with_trailing_garbage(tmp_path): + registry_path = tmp_path / "registry.json" + registry_path.write_text('{"main": "/tmp/.browser_cli/main.sock"}"}', encoding="utf-8") + + def fake_send_command(command, args=None, profile=None): + assert command == "clients.list" + assert profile == "main" + return [{"profile": "main", "name": "Chrome", "version": "1", "extensionVersion": "0.8.2"}] + + with patch("browser_cli.cli.REGISTRY_PATH", registry_path), patch( + "browser_cli.cli.send_command", side_effect=fake_send_command + ): + result = CliRunner().invoke(main, ["clients"]) + + assert result.exit_code == 0 + assert "main" in result.output + assert "0.8.2" in result.output + def test_clients_remote_uses_remote_endpoint_without_local_registry(): def fake_send_command(command, args=None, profile=None): assert command == "clients.list" diff --git a/tests/test_registry.py b/tests/test_registry.py new file mode 100644 index 0000000..eac8d31 --- /dev/null +++ b/tests/test_registry.py @@ -0,0 +1,30 @@ +import json + +from browser_cli.registry import load_registry, save_registry, update_registry + + +def test_load_registry_tolerates_trailing_garbage_from_old_non_atomic_writes(tmp_path): + registry = tmp_path / "registry.json" + registry.write_text('{"main": "/tmp/.browser_cli/main.sock"}"}', encoding="utf-8") + + assert load_registry(registry) == {"main": "/tmp/.browser_cli/main.sock"} + + +def test_update_registry_repairs_corrupted_registry_and_preserves_entries(tmp_path): + registry = tmp_path / "registry.json" + registry.write_text('{"main": "/tmp/.browser_cli/main.sock"}"}', encoding="utf-8") + + update_registry("work", "/tmp/.browser_cli/work.sock", registry) + + assert json.loads(registry.read_text(encoding="utf-8")) == { + "main": "/tmp/.browser_cli/main.sock", + "work": "/tmp/.browser_cli/work.sock", + } + + +def test_save_registry_writes_valid_json_atomically(tmp_path): + registry = tmp_path / "registry.json" + + save_registry({"main": "/tmp/main.sock"}, registry) + + assert json.loads(registry.read_text(encoding="utf-8")) == {"main": "/tmp/main.sock"}