fix(browser): R3 — restore fs cat root, daemon lane capture, grep hyphen guard

Address @yuh-yang's three R3 blockers on PR #308. All three land in this PR;
the grep parser issue (blocker 3) takes Path B — Python-side rejection with
a clear error message — while the real parseArgs `--` work goes upstream to
a future DOMShell release.

  1. fs cat at root no longer raises Python ValueError before reaching
     DOMShell. The absolute-root guard added in round 5 was overreaching —
     it converted DOMShell's standard "Usage: cat <name>" error into a
     Python exception. Guard removed; cat '' now reaches the kernel and
     surfaces DOMShell's own error string, restoring pre-migration
     behavior. Stale "should raise" tests flipped to "should not raise +
     return parseable error result".

  2. Daemon-mode/no-session lane handling is now consistent across daemon-
     alive and daemon-dead paths. The previous "shared" branch (commit
     99d1182) claimed per-connection-default stickiness that doesn't hold
     when _daemon_session is None and each call spawns a fresh
     ClientSession. Replaced with a module-level _daemon_lane_id captured
     on the first daemon-no-session call and reused on every subsequent
     call. Works in both paths: alive daemon reuses the id on its
     persistent connection; dead daemon's fresh spawns swapToAgentLane()
     into the existing Chrome tab-group by id (groups persist across
     MCP session boundaries). Stale-lane failure mode (user closes the
     group manually) propagates DOMShell's own error — same shape as the
     session-bearing path. Replaces the previous-round "shared" test
     with first-call-captures + subsequent-calls-reuse tests.

  3. Hyphen-prefixed grep patterns now raise a clear Python-side
     ValueError instead of silently failing at DOMShell. The current
     DOMShell parseArgs treats any arg starting with "-" as a flag —
     no "--" separator, no -e <pattern> form — so `grep -r -- -foo`
     and `grep -r -e -foo` both fail at the kernel. Tracked upstream as
     a parser-limitation issue; the real fix ships with the next
     DOMShell release that already justifies a Chrome Web Store
     submission. Until then, the wrapper rejects the input with
     guidance instead of letting it silently fall through.

191 passing locally (was 189 + 4 new tests - 1 deleted shared-lane test -
1 old cat-root test replaced).

Cc @yuh-yang

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py

@@ -146,11 +146,32 @@ def test_cat_relative_no_wrap(mock_call):
     assert mock_call.call_args.args[0] == "cat main/btn"
 
 
-def test_cat_root_raises_value_error():
-    with pytest.raises(ValueError, match="element name is required"):
-        backend.cat("/")
-    with pytest.raises(ValueError, match="element name is required"):
-        backend.cat("")
+@patch.object(backend, "_call_execute", new_callable=AsyncMock)
+def test_cat_root_path_does_not_raise_and_returns_parseable_result(mock_call):
+    """backend.cat("/") and backend.cat("") at the absolute root should
+    NOT raise a Python ValueError. Pre-migration behavior was to send
+    ``cat ''`` to DOMShell and surface its ``Usage: cat <name>`` error
+    string; the round-5 Python-side guard converted the kernel error
+    into a ValueError, which broke ``fs.read_element(session, "")``
+    callers landed at ``/``. @yuh-yang R3 blocker 1 required restoring
+    pre-migration behavior — guard removed, both inputs now reach
+    DOMShell, which has the same ``if (!targetName)`` check and
+    returns its standard Usage error.
+    """
+    mock_call.return_value = _make_result(
+        "\x1b[31mUsage: cat <name> (see cat --help)\x1b[0m\n[lane: shared]"
+    )
+    sess = _make_session(working_dir="/")  # satisfies absolute branch's session check
+
+    # MUST NOT raise — that was the regression.
+    result_root = backend.cat("/", session=sess)
+    result_empty = backend.cat("")
+
+    # Both surface DOMShell's error as a parsed dict, not a Python exception.
+    for r in (result_root, result_empty):
+        assert isinstance(r, dict)
+        # _parse_execute_result detects ANSI red → routes through error path.
+        assert "error" in r
 
 
 @patch.object(backend, "_call_execute", new_callable=AsyncMock)
@@ -903,6 +924,20 @@ def test_grep_rejects_newline_in_pattern():
         backend.grep("Login\nclick /admin", path="/main", prev="/")
 
 
+def test_grep_rejects_hyphen_prefixed_pattern():
+    """DOMShell's parseArgs treats any arg starting with "-" as a flag,
+    so the grep wrapper can't pass hyphen-prefixed search strings as
+    patterns. The wrapper raises ValueError with a clear message so
+    users see the limitation immediately rather than getting DOMShell's
+    generic Usage reply. Real fix needs upstream parseArgs ``--``
+    support; tracked upstream. (@yuh-yang R3 blocker 3, Path B.)
+    """
+    with pytest.raises(ValueError, match="patterns starting with '-'"):
+        backend.grep("-disabled")
+    with pytest.raises(ValueError, match="patterns starting with '-'"):
+        backend.grep("--content")
+
+
 def test_grep_rejects_newline_in_prev():
     with pytest.raises(ValueError, match="newline"):
         backend.grep("Login", path="/main", prev="/\nclick /admin")
@@ -1150,34 +1185,67 @@ def test_call_execute_passes_group_id_new_when_lane_is_none():
     assert sess.domshell_lane_id == "brand-new"
 
 
-def test_call_execute_passes_group_id_shared_for_daemon_mode_without_session():
-    """Daemon mode + no session: route to the default per-connection lane
-    via ``group_id="shared"``. The daemon's persistent stdio connection
-    keeps that lane sticky across calls, so direct callers like
-    ``open_url(use_daemon=True)`` followed by ``ls(use_daemon=True)``
-    share browser state without needing a Session object to carry a
-    lane id.
+def test_call_execute_daemon_no_session_first_call_passes_new_and_captures_lane():
+    """Daemon mode + no session, first call: passes ``group_id="new"``
+    and captures the lane id from the response into the module-level
+    ``_daemon_lane_id`` for subsequent calls. Replaces the previous
+    ``"shared"`` behavior (commit 99d1182) per @yuh-yang R3 blocker 2.
 
-    Migration note (PR #308 follow-up): the initial 2.0.2 migration
-    commit (be62f843b5) passed ``group_id="new"`` in this case, which
-    broke the daemon workflow by creating a fresh isolated lane per
-    call. Caught by Codex P2. Fix re-routes daemon-no-session calls to
-    ``"shared"``.
+    The R2 "shared" branch claimed per-connection-default stickiness
+    that holds when the persistent daemon connection is alive but
+    breaks in the fall-back spawn path (each call gets its own
+    per-MCP-session default lane). The captured-id approach works
+    consistently across both paths because Chrome tab-group ids
+    persist at the browser level, regardless of MCP session.
     """
-    fake_tool = AsyncMock(return_value=_make_result("✓\n[lane: daemon-default]"))
+    # Reset module state (other tests may have populated it).
+    backend._daemon_lane_id = None
 
-    fake_mcp_session = AsyncMock()
-    fake_mcp_session.call_tool = fake_tool
+    try:
+        fake_tool = AsyncMock(
+            return_value=_make_result("✓\n[lane: daemon-captured-99]")
+        )
+        fake_mcp_session = AsyncMock()
+        fake_mcp_session.call_tool = fake_tool
 
-    # Simulate a live daemon session — the same path open_url / ls take
-    # when called with use_daemon=True but no Session.
-    with patch.object(backend, "_daemon_session", fake_mcp_session):
-        import asyncio as _aio
-        _aio.run(backend._call_execute("ls /", use_daemon=True, session=None))
+        with patch.object(backend, "_daemon_session", fake_mcp_session):
+            import asyncio as _aio
+            _aio.run(backend._call_execute("ls /", use_daemon=True, session=None))
 
-    name, arguments = fake_tool.call_args.args
-    assert name == "domshell_execute"
-    assert arguments.get("group_id") == "shared"
+        name, arguments = fake_tool.call_args.args
+        assert name == "domshell_execute"
+        assert arguments.get("group_id") == "new"
+        assert backend._daemon_lane_id == "daemon-captured-99"
+    finally:
+        backend._daemon_lane_id = None
+
+
+def test_call_execute_daemon_no_session_subsequent_calls_reuse_captured_lane():
+    """Once ``_daemon_lane_id`` is set, daemon-no-session calls pass
+    that id as ``group_id`` instead of ``"new"`` — preserves browser
+    state across calls regardless of whether the daemon's stdio is
+    alive or whether we've fallen back to fresh spawns per call.
+    Capture stays stable; no clobber on subsequent calls.
+    """
+    backend._daemon_lane_id = "preexisting-77"
+
+    try:
+        fake_tool = AsyncMock(
+            return_value=_make_result("✓\n[lane: preexisting-77]")
+        )
+        fake_mcp_session = AsyncMock()
+        fake_mcp_session.call_tool = fake_tool
+
+        with patch.object(backend, "_daemon_session", fake_mcp_session):
+            import asyncio as _aio
+            _aio.run(backend._call_execute("ls /", use_daemon=True, session=None))
+
+        name, arguments = fake_tool.call_args.args
+        assert arguments.get("group_id") == "preexisting-77"
+        # Capture stays the same — no clobber on subsequent calls.
+        assert backend._daemon_lane_id == "preexisting-77"
+    finally:
+        backend._daemon_lane_id = None
 
 
 def test_distinct_sessions_have_isolated_lanes():

browser/agent-harness/cli_anything/browser/utils/domshell_backend.py

@@ -62,6 +62,25 @@ _daemon_read: Optional[Any] = None
 _daemon_write: Optional[Any] = None
 _daemon_client_context: Optional[Any] = None  # Store stdio_client context manager
 
+# Module-level lane id captured from the first daemon-mode-no-session
+# call. Reused on every subsequent daemon-no-session call so direct
+# wrappers like `open_url(use_daemon=True)` followed by
+# `ls(use_daemon=True)` preserve browser state across calls, regardless
+# of whether the daemon's persistent stdio connection is currently
+# alive or whether we've fallen back to spawning a fresh ClientSession
+# per call. The captured id is a numeric Chrome tab-group id, which
+# persists at the browser level even across MCP session boundaries —
+# so a fresh spawn can swapToAgentLane(id) into the lane the previous
+# spawn (or a still-living daemon) created.
+#
+# Stale-lane failure mode: if the user manually closes the Chrome tab
+# group this id points to, subsequent calls will surface DOMShell's
+# "Error: no lane 'A'" with its own recovery guidance. We don't
+# auto-recover here — same shape as `session.domshell_lane_id`
+# pointing to a closed group. Restart the daemon (or reset this attr)
+# to clear.
+_daemon_lane_id: Optional[str] = None
+
 
 def _check_npx() -> bool:
     """Check if npx is available."""
@@ -496,40 +515,31 @@ async def _call_execute(
     Raises:
         RuntimeError: If MCP server is not available or tool call fails
     """
-    global _daemon_session, _daemon_read, _daemon_write
+    global _daemon_session, _daemon_read, _daemon_write, _daemon_lane_id
 
     arguments: dict[str, Any] = {"command": command}
-    # Lane handling. DOMShell 2.0.2 deprecated omitting `group_id`
-    # (emits a [DEPRECATION] warning in the reply; hard error in 3.0.0),
-    # so we name the lane explicitly on every call. Three cases:
-    #   • subsequent calls (session has captured lane) → reuse that lane.
-    #   • non-daemon first call → group_id="new" — DOMShell creates a
-    #     fresh isolated lane and returns its id in the [lane: ...]
-    #     marker, which `_capture_lane` stores on the session for next
-    #     time.
-    #   • daemon-mode first call without a Session → group_id="shared".
-    #     When the persistent daemon connection is live, the default
-    #     per-connection lane stays sticky across calls — so direct
-    #     (sessionless) daemon workflows like `open_url(use_daemon=True)`
-    #     followed by `ls(use_daemon=True)` share browser state without
-    #     needing a Session to carry a lane id.
+    # Lane handling — three sources, in priority order:
+    #   • Session with a captured lane id  → reuse that lane (REPL pattern,
+    #     state preserved across non-daemon spawn-per-call sequences).
+    #   • Daemon mode + no session + previously captured _daemon_lane_id
+    #     → reuse the daemon-level captured id. Works whether the daemon's
+    #     stdio is alive (id reused on persistent connection) or dead
+    #     (fresh spawn joins the existing Chrome tab-group by id).
+    #   • Otherwise → group_id="new". DOMShell creates a fresh isolated
+    #     lane and returns the id in the [lane: ...] marker, captured into
+    #     either session.domshell_lane_id or _daemon_lane_id for next time.
     #
-    #     In the fall-back path (daemon dead / not started — i.e.
-    #     `_daemon_session is None` below), each call spawns its own
-    #     ClientSession, which triggers its own SESSION_START on
-    #     DOMShell and gets its own per-connection default lane.
-    #     "shared" is per-MCP-session, so it still routes correctly
-    #     there — to *that spawn's* default lane — giving per-call
-    #     isolation. Same outcome as omitting `group_id` would have
-    #     pre-2.0.2, minus the [DEPRECATION] warning, and crucially one
-    #     orphan tab-group per spawn instead of two (`group_id="new"`
-    #     would create an explicit second lane on top of the
-    #     SESSION_START default — the orphan flood reverted in commit
-    #     99d1182504).
+    # The "shared" branch from the previous version of this code (commit
+    # 99d1182) is gone. It claimed the per-connection default lane stayed
+    # sticky across calls — true when the persistent daemon connection is
+    # alive, but a lie in the fall-back spawn path where each call gets a
+    # different per-MCP-session default lane. @yuh-yang's review of
+    # b684732 flagged the inconsistency. The module-level capture path
+    # below is consistent across both daemon-alive and daemon-dead.
     if session is not None and getattr(session, "domshell_lane_id", None):
         arguments["group_id"] = session.domshell_lane_id
-    elif use_daemon:
-        arguments["group_id"] = "shared"
+    elif use_daemon and _daemon_lane_id:
+        arguments["group_id"] = _daemon_lane_id
     else:
         arguments["group_id"] = "new"
 
@@ -540,6 +550,13 @@ async def _call_execute(
                 "domshell_execute", arguments
             )
             _capture_lane(session, result)
+            # Daemon-level capture on first daemon-no-session call so
+            # subsequent calls reuse the same lane regardless of whether
+            # the daemon stays alive or we fall back to fresh spawns.
+            if use_daemon and session is None and _daemon_lane_id is None:
+                captured = _extract_lane_id(result)
+                if captured:
+                    _daemon_lane_id = captured
             return result
         except Exception as e:
             # Daemon died — log diagnosability and fall back to spawning
@@ -564,6 +581,12 @@ async def _call_execute(
                     "domshell_execute", arguments
                 )
                 _capture_lane(session, result)
+                # See daemon-path capture above for rationale — same
+                # logic applies on the fresh-spawn fall-back path.
+                if use_daemon and session is None and _daemon_lane_id is None:
+                    captured = _extract_lane_id(result)
+                    if captured:
+                        _daemon_lane_id = captured
                 return result
     except Exception as e:
         raise RuntimeError(
@@ -755,11 +778,14 @@ def cat(path: str, use_daemon: bool = False, *, session: Any = None) -> dict:
         {"output": "button: Submit\\n..."}
     """
     translated, is_absolute = _translate_path(path)
-    if not translated:
-        raise ValueError(
-            "cat: an element name is required — cannot cat the tab root. "
-            "Use `ls` to list the root's children, or pass a specific name."
-        )
+    # Note: a falsy `translated` (root path `/` or empty string) is no
+    # longer rejected here. Pre-migration `fs cat` at the root surfaced
+    # DOMShell's own `Usage: cat <name>` error string; the round-5 guard
+    # converted that into a Python ValueError, which broke the
+    # `fs.read_element(session, "")` fall-through to `session.working_dir`
+    # for callers landed at `/`. Removed per @yuh-yang R3 review of
+    # `b684732` — `cat ''` reaches DOMShell, which has the same
+    # `if (!targetName)` check and returns its standard Usage error.
     if is_absolute:
         _require_session_for_split_check("cat", session, use_daemon)
         # Split-and-check: anchor at tab root, halt if anchor fails,
@@ -827,6 +853,26 @@ def grep(
         {"matches": ["/main/button[0]"], "raw": "..."}
     """
     _assert_single_line("pattern", pattern)
+
+    # DOMShell's parseArgs treats any arg starting with "-" as a flag,
+    # with no "--" end-of-options separator and no "-e <pattern>" form
+    # (verified against src/background/index.ts:1692 in DOMShell 2.0.2).
+    # So `grep -r -- -foo` and `grep -r -e -foo` both fail at the
+    # kernel — neither survives parseArgs as a positional. Until
+    # DOMShell adds "--" support in a future release, raise a clear
+    # Python-side error so the user sees the limitation immediately
+    # instead of getting DOMShell's generic "Usage:" reply.
+    # (yuh-yang R3 blocker 3, Path B — Python-side soft validation.)
+    if pattern.startswith("-"):
+        raise ValueError(
+            f"grep: patterns starting with '-' are not supported by the "
+            f"current DOMShell parser — it would parse {pattern!r} as a "
+            f"flag rather than as the search pattern. Known limitation "
+            f"tracked upstream; will be resolved when DOMShell's "
+            f"parseArgs adds `--` end-of-options support. Workaround: "
+            f"drop the leading '-' from the pattern if possible, or "
+            f"wait for the upstream fix."
+        )
     translated_path, path_abs = _translate_path(path)
     if not translated_path:
         # Unrooted grep — operate on lane cwd, no cd, no restore.