fix(browser): avoid eager agent-browser probe

[luntion]

Summary

• Avoid running agent-browser --version during browser tool availability checks.
• Keep validating the cached/resolved agent-browser executable on real browser use.
• Add regression tests for the lightweight probe path and the later validating lookup.

Root cause

Desktop startup builds the agent tool list, which evaluates tool check_fns. The browser check_fn called _find_agent_browser(), and that path validated candidates by executing agent-browser --version. On Windows, resolving the local npm .CMD shim runs through cmd.exe, causing a visible console flash during desktop startup even though the user did not invoke a browser tool.

Changes

• Add a validate parameter to _find_agent_browser().
• Use validate=False from check_browser_requirements() so tool-list assembly remains side-effect-free.
• Preserve validate=True as the default for real browser execution paths, including revalidating a candidate cached by a lightweight probe.
• Add regression coverage for both paths.

Closes #54364.

Testing

• python -m py_compile tools/browser_tool.py tests/tools/test_browser_agent_browser_probe.py
• git diff --check
• Manual regression script verified:
  • check_browser_requirements() does not call agent_browser_runnable().
  • A candidate cached by validate=False is still validated when _find_agent_browser(validate=True) is called.

Note: pytest is not installed in the available local Python environments on this Windows machine, so the new pytest file was syntax-checked and the equivalent assertions were run manually without installing packages.

Risk

Low. The behavior change only affects availability probing. Real browser command execution still validates the executable before use, so stale/broken npm shims are not silently accepted.

[luntion]

Ran the test suite for this PR with a real pytest (Python 3.11.13, pytest 9.1.1) on Windows — the two new tests in test_browser_agent_browser_probe.py both pass:

tests/tools/test_browser_agent_browser_probe.py::test_check_browser_requirements_does_not_execute_agent_browser PASSED
tests/tools/test_browser_agent_browser_probe.py::test_validating_lookup_rechecks_candidate_cached_by_probe PASSED
2 passed

However, running the existing browser tests against this branch surfaced a regression. Switching check_browser_requirements() to call _find_agent_browser(validate=False) breaks 3 existing tests that monkeypatch _find_agent_browser with a zero-arg lambda:. They now fail with:

TypeError: <lambda>() got an unexpected keyword argument 'validate'
tools/browser_tool.py:3846: TypeError

Affected tests:

• tests/tools/test_browser_chromium_check.py::TestCheckBrowserRequirementsChromium::test_local_mode_with_chromium_returns_true
• tests/tools/test_browser_chromium_check.py::TestCheckBrowserRequirementsChromium::test_cloud_mode_does_not_require_local_chromium
• tests/tools/test_browser_homebrew_paths.py::TestBrowserRequirements::test_termux_requires_real_agent_browser_install_not_npx_fallback

The fix is to make those stubs tolerate the new keyword arg (lambda *a, **k:). With the patch below, the browser suite goes from 7 failed → 4 failed, and the remaining 4 are pre-existing, environment-only failures unrelated to this PR (macOS-only Homebrew discovery + Path.home() not honoring HOME on Windows — they fail identically on main).

diff --git a/tests/tools/test_browser_chromium_check.py b/tests/tools/test_browser_chromium_check.py
@@ class TestCheckBrowserRequirementsChromium:
     def test_local_mode_with_chromium_returns_true(self, monkeypatch, tmp_path):
         monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False)
-        monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser")
+        monkeypatch.setattr(bt, "_find_agent_browser", lambda *a, **k: "/usr/local/bin/agent-browser")
@@ class TestCheckBrowserRequirementsChromium:
         monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False)
-        monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser")
+        monkeypatch.setattr(bt, "_find_agent_browser", lambda *a, **k: "/usr/local/bin/agent-browser")
diff --git a/tests/tools/test_browser_homebrew_paths.py b/tests/tools/test_browser_homebrew_paths.py
@@ class TestBrowserRequirements:
     def test_cdp_override_does_not_require_agent_browser_cli(self, monkeypatch):
         monkeypatch.setenv("BROWSER_CDP_URL", "ws://127.0.0.1:9222/devtools/browser/test")
         monkeypatch.setattr("tools.browser_tool._is_camofox_mode", lambda: False)
-        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda: (_ for _ in ()).throw(FileNotFoundError("not found")))
+        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda *a, **k: (_ for _ in ()).throw(FileNotFoundError("not found")))
@@ class TestBrowserRequirements:
         monkeypatch.setattr("tools.browser_tool._get_cloud_provider", lambda: None)
-        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda: "npx agent-browser")
+        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda *a, **k: "npx agent-browser")
@@ class TestRunBrowserCommandTermuxFallback:
     def test_termux_local_mode_rejects_bare_npx_fallback(self, monkeypatch):
         monkeypatch.setenv("PREFIX", "/data/data/com.termux/files/usr")
-        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda: "npx agent-browser")
+        monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda *a, **k: "npx agent-browser")
         monkeypatch.setattr("tools.browser_tool._get_cloud_provider", lambda: None)

(The CDP-override and _run_browser_command stubs don't strictly need it today — the first returns before the call, the second invokes _find_agent_browser() with no args — but updating all of them keeps the stubs robust against the signature change.)

[luntion]

Pushed this stub fix directly to the branch as 7890a89 ("test(browser): tolerate _find_agent_browser(validate=) kwarg in stubs") via the Git Data API. The branch head is now 7890a89 and the PR shows 2 commits. Verified the 3 previously-failing tests pass and the browser suite is at 4 failed (all pre-existing env-only failures, identical on main).

[teknium1]

Closing as superseded by #54417.

The browser eager-probe fix from this PR has landed on main as part of #54417: check_browser_requirements() now uses _find_agent_browser(validate=False), and the existing browser tests were updated to tolerate the new kwarg. The remaining Windows console-flash work is tracked in #54220.

Thanks for narrowing this down — this PR/report directly matched one of the follow-up legs.