8b861b77c1
* refactor: remove browser_close tool — auto-cleanup handles it
The browser_close tool was called in only 9% of browser sessions (13/144
navigations across 66 sessions), always redundantly — cleanup_browser()
already runs via _cleanup_task_resources() at conversation end, and the
background inactivity reaper catches anything else.
Removing it saves one tool schema slot in every browser-enabled API call.
Also fixes a latent bug: cleanup_browser() now handles Camofox sessions
too (previously only Browserbase). Camofox sessions were never auto-cleaned
per-task because they live in a separate dict from _active_sessions.
Files changed (13):
- tools/browser_tool.py: remove function, schema, registry entry; add
camofox cleanup to cleanup_browser()
- toolsets.py, model_tools.py, prompt_builder.py, display.py,
acp_adapter/tools.py: remove browser_close from all tool lists
- tests/: remove browser_close test, update toolset assertion
- docs/skills: remove all browser_close references
* fix: repeat browser_scroll 5x per call for meaningful page movement
Most backends scroll ~100px per call — barely visible on a typical
viewport. Repeating 5x gives ~500px (~half a viewport), making each
scroll tool call actually useful.
Backend-agnostic approach: works across all 7+ browser backends without
needing to configure each one's scroll amount individually. Breaks
early on error for the agent-browser path.
* feat: auto-return compact snapshot from browser_navigate
Every browser session starts with navigate → snapshot. Now navigate
returns the compact accessibility tree snapshot inline, saving one
tool call per browser task.
The snapshot captures the full page DOM (not viewport-limited), so
scroll position doesn't affect it. browser_snapshot remains available
for refreshing after interactions or getting full=true content.
Both Browserbase and Camofox paths auto-snapshot. If the snapshot
fails for any reason, navigation still succeeds — the snapshot is
a bonus, not a requirement.
Schema descriptions updated to guide models: navigate mentions it
returns a snapshot, snapshot mentions it's for refresh/full content.
* refactor: slim cronjob tool schema — consolidate model/provider, drop unused params
Session data (151 calls across 67 sessions) showed several schema
properties were never used by models. Consolidated and cleaned up:
Removed from schema (still work via backend/CLI):
- skill (singular): use skills array instead
- reason: pause-only, unnecessary
- include_disabled: now defaults to true
- base_url: extreme edge case, zero usage
- provider (standalone): merged into model object
Consolidated:
- model + provider → single 'model' object with {model, provider} fields.
If provider is omitted, the current main provider is pinned at creation
time so the job stays stable even if the user changes their default.
Kept:
- script: useful data collection feature
- skills array: standard interface for skill loading
Schema shrinks from 14 to 10 properties. All backend functionality
preserved — the Python function signature and handler lambda still
accept every parameter.
* fix: remove mixture_of_agents from core toolsets — opt-in only via hermes tools
MoA was in _HERMES_CORE_TOOLS and composite toolsets (hermes-cli,
hermes-messaging, safe), which meant it appeared in every session
for anyone with OPENROUTER_API_KEY set. The _DEFAULT_OFF_TOOLSETS
gate only works after running 'hermes tools' explicitly.
Now MoA only appears when a user explicitly enables it via
'hermes tools'. The moa toolset definition and check_fn remain
unchanged — it just needs to be opted into.
85 lines
3.5 KiB
Python
85 lines
3.5 KiB
Python
"""Regression tests for browser session cleanup and screenshot recovery."""
|
|
|
|
from unittest.mock import patch
|
|
|
|
|
|
class TestScreenshotPathRecovery:
|
|
def test_extracts_standard_absolute_path(self):
|
|
from tools.browser_tool import _extract_screenshot_path_from_text
|
|
|
|
assert (
|
|
_extract_screenshot_path_from_text("Screenshot saved to /tmp/foo.png")
|
|
== "/tmp/foo.png"
|
|
)
|
|
|
|
def test_extracts_quoted_absolute_path(self):
|
|
from tools.browser_tool import _extract_screenshot_path_from_text
|
|
|
|
assert (
|
|
_extract_screenshot_path_from_text(
|
|
"Screenshot saved to '/Users/david/.hermes/browser_screenshots/shot.png'"
|
|
)
|
|
== "/Users/david/.hermes/browser_screenshots/shot.png"
|
|
)
|
|
|
|
|
|
class TestBrowserCleanup:
|
|
def setup_method(self):
|
|
from tools import browser_tool
|
|
|
|
self.browser_tool = browser_tool
|
|
self.orig_active_sessions = browser_tool._active_sessions.copy()
|
|
self.orig_session_last_activity = browser_tool._session_last_activity.copy()
|
|
self.orig_recording_sessions = browser_tool._recording_sessions.copy()
|
|
self.orig_cleanup_done = browser_tool._cleanup_done
|
|
|
|
def teardown_method(self):
|
|
self.browser_tool._active_sessions.clear()
|
|
self.browser_tool._active_sessions.update(self.orig_active_sessions)
|
|
self.browser_tool._session_last_activity.clear()
|
|
self.browser_tool._session_last_activity.update(self.orig_session_last_activity)
|
|
self.browser_tool._recording_sessions.clear()
|
|
self.browser_tool._recording_sessions.update(self.orig_recording_sessions)
|
|
self.browser_tool._cleanup_done = self.orig_cleanup_done
|
|
|
|
def test_cleanup_browser_clears_tracking_state(self):
|
|
browser_tool = self.browser_tool
|
|
browser_tool._active_sessions["task-1"] = {
|
|
"session_name": "sess-1",
|
|
"bb_session_id": None,
|
|
}
|
|
browser_tool._session_last_activity["task-1"] = 123.0
|
|
|
|
with (
|
|
patch("tools.browser_tool._maybe_stop_recording") as mock_stop,
|
|
patch(
|
|
"tools.browser_tool._run_browser_command",
|
|
return_value={"success": True},
|
|
) as mock_run,
|
|
patch("tools.browser_tool.os.path.exists", return_value=False),
|
|
):
|
|
browser_tool.cleanup_browser("task-1")
|
|
|
|
assert "task-1" not in browser_tool._active_sessions
|
|
assert "task-1" not in browser_tool._session_last_activity
|
|
mock_stop.assert_called_once_with("task-1")
|
|
mock_run.assert_called_once_with("task-1", "close", [], timeout=10)
|
|
|
|
def test_emergency_cleanup_clears_all_tracking_state(self):
|
|
browser_tool = self.browser_tool
|
|
browser_tool._cleanup_done = False
|
|
browser_tool._active_sessions["task-1"] = {"session_name": "sess-1"}
|
|
browser_tool._active_sessions["task-2"] = {"session_name": "sess-2"}
|
|
browser_tool._session_last_activity["task-1"] = 1.0
|
|
browser_tool._session_last_activity["task-2"] = 2.0
|
|
browser_tool._recording_sessions.update({"task-1", "task-2"})
|
|
|
|
with patch("tools.browser_tool.cleanup_all_browsers") as mock_cleanup_all:
|
|
browser_tool._emergency_cleanup_all_sessions()
|
|
|
|
mock_cleanup_all.assert_called_once_with()
|
|
assert browser_tool._active_sessions == {}
|
|
assert browser_tool._session_last_activity == {}
|
|
assert browser_tool._recording_sessions == set()
|
|
assert browser_tool._cleanup_done is True
|