refactor: remove browser_close tool — auto-cleanup handles it (#5792)
* 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.
This commit is contained in:
@@ -240,6 +240,25 @@ def camofox_navigate(url: str, task_id: Optional[str] = None) -> str:
|
||||
"Browser is visible via VNC. "
|
||||
"Share this link with the user so they can watch the browser live."
|
||||
)
|
||||
|
||||
# Auto-take a compact snapshot so the model can act immediately
|
||||
try:
|
||||
snap_data = _get(
|
||||
f"/tabs/{session['tab_id']}/snapshot",
|
||||
params={"userId": session["user_id"]},
|
||||
)
|
||||
snapshot_text = snap_data.get("snapshot", "")
|
||||
from tools.browser_tool import (
|
||||
SNAPSHOT_SUMMARIZE_THRESHOLD,
|
||||
_truncate_snapshot,
|
||||
)
|
||||
if len(snapshot_text) > SNAPSHOT_SUMMARIZE_THRESHOLD:
|
||||
snapshot_text = _truncate_snapshot(snapshot_text)
|
||||
result["snapshot"] = snapshot_text
|
||||
result["element_count"] = snap_data.get("refsCount", 0)
|
||||
except Exception:
|
||||
pass # Navigation succeeded; snapshot is a bonus
|
||||
|
||||
return json.dumps(result)
|
||||
except requests.HTTPError as e:
|
||||
return json.dumps({"success": False, "error": f"Navigation failed: {e}"})
|
||||
|
||||
@@ -518,7 +518,7 @@ atexit.register(_stop_browser_cleanup_thread)
|
||||
BROWSER_TOOL_SCHEMAS = [
|
||||
{
|
||||
"name": "browser_navigate",
|
||||
"description": "Navigate to a URL in the browser. Initializes the session and loads the page. Must be called before other browser tools. For simple information retrieval, prefer web_search or web_extract (faster, cheaper). Use browser tools when you need to interact with a page (click, fill forms, dynamic content).",
|
||||
"description": "Navigate to a URL in the browser. Initializes the session and loads the page. Must be called before other browser tools. For simple information retrieval, prefer web_search or web_extract (faster, cheaper). Use browser tools when you need to interact with a page (click, fill forms, dynamic content). Returns a compact page snapshot with interactive elements and ref IDs — no need to call browser_snapshot separately after navigating.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -532,7 +532,7 @@ BROWSER_TOOL_SCHEMAS = [
|
||||
},
|
||||
{
|
||||
"name": "browser_snapshot",
|
||||
"description": "Get a text-based snapshot of the current page's accessibility tree. Returns interactive elements with ref IDs (like @e1, @e2) for browser_click and browser_type. full=false (default): compact view with interactive elements. full=true: complete page content. Snapshots over 8000 chars are truncated or LLM-summarized. Requires browser_navigate first.",
|
||||
"description": "Get a text-based snapshot of the current page's accessibility tree. Returns interactive elements with ref IDs (like @e1, @e2) for browser_click and browser_type. full=false (default): compact view with interactive elements. full=true: complete page content. Snapshots over 8000 chars are truncated or LLM-summarized. Requires browser_navigate first. Note: browser_navigate already returns a compact snapshot — use this to refresh after interactions that change the page, or with full=true for complete content.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -615,15 +615,7 @@ BROWSER_TOOL_SCHEMAS = [
|
||||
"required": ["key"]
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "browser_close",
|
||||
"description": "Close the browser session and release resources. Call this when done with browser tasks to free up Browserbase session quota.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"required": []
|
||||
}
|
||||
},
|
||||
|
||||
{
|
||||
"name": "browser_get_images",
|
||||
"description": "Get a list of all images on the current page with their URLs and alt text. Useful for finding images to analyze with the vision tool. Requires browser_navigate to be called first.",
|
||||
@@ -1229,7 +1221,22 @@ def browser_navigate(url: str, task_id: Optional[str] = None) -> str:
|
||||
"Consider upgrading Browserbase plan for proxy support."
|
||||
)
|
||||
response["stealth_features"] = active_features
|
||||
|
||||
|
||||
# Auto-take a compact snapshot so the model can act immediately
|
||||
# without a separate browser_snapshot call.
|
||||
try:
|
||||
snap_result = _run_browser_command(effective_task_id, "snapshot", ["-c"])
|
||||
if snap_result.get("success"):
|
||||
snap_data = snap_result.get("data", {})
|
||||
snapshot_text = snap_data.get("snapshot", "")
|
||||
refs = snap_data.get("refs", {})
|
||||
if len(snapshot_text) > SNAPSHOT_SUMMARIZE_THRESHOLD:
|
||||
snapshot_text = _truncate_snapshot(snapshot_text)
|
||||
response["snapshot"] = snapshot_text
|
||||
response["element_count"] = len(refs) if refs else 0
|
||||
except Exception as e:
|
||||
logger.debug("Auto-snapshot after navigate failed: %s", e)
|
||||
|
||||
return json.dumps(response, ensure_ascii=False)
|
||||
else:
|
||||
return json.dumps({
|
||||
@@ -1376,31 +1383,40 @@ def browser_scroll(direction: str, task_id: Optional[str] = None) -> str:
|
||||
Returns:
|
||||
JSON string with scroll result
|
||||
"""
|
||||
if _is_camofox_mode():
|
||||
from tools.browser_camofox import camofox_scroll
|
||||
return camofox_scroll(direction, task_id)
|
||||
|
||||
effective_task_id = task_id or "default"
|
||||
|
||||
# Validate direction
|
||||
if direction not in ["up", "down"]:
|
||||
return json.dumps({
|
||||
"success": False,
|
||||
"error": f"Invalid direction '{direction}'. Use 'up' or 'down'."
|
||||
}, ensure_ascii=False)
|
||||
|
||||
result = _run_browser_command(effective_task_id, "scroll", [direction])
|
||||
|
||||
if result.get("success"):
|
||||
return json.dumps({
|
||||
"success": True,
|
||||
"scrolled": direction
|
||||
}, ensure_ascii=False)
|
||||
else:
|
||||
return json.dumps({
|
||||
"success": False,
|
||||
"error": result.get("error", f"Failed to scroll {direction}")
|
||||
}, ensure_ascii=False)
|
||||
|
||||
# Repeat the scroll 5 times to get meaningful page movement.
|
||||
# Most backends scroll ~100px per call, which is barely visible.
|
||||
# 5x gives roughly half a viewport of travel, backend-agnostic.
|
||||
_SCROLL_REPEATS = 5
|
||||
|
||||
if _is_camofox_mode():
|
||||
from tools.browser_camofox import camofox_scroll
|
||||
result = None
|
||||
for _ in range(_SCROLL_REPEATS):
|
||||
result = camofox_scroll(direction, task_id)
|
||||
return result
|
||||
|
||||
effective_task_id = task_id or "default"
|
||||
|
||||
result = None
|
||||
for _ in range(_SCROLL_REPEATS):
|
||||
result = _run_browser_command(effective_task_id, "scroll", [direction])
|
||||
if not result.get("success"):
|
||||
return json.dumps({
|
||||
"success": False,
|
||||
"error": result.get("error", f"Failed to scroll {direction}")
|
||||
}, ensure_ascii=False)
|
||||
|
||||
return json.dumps({
|
||||
"success": True,
|
||||
"scrolled": direction
|
||||
}, ensure_ascii=False)
|
||||
|
||||
|
||||
def browser_back(task_id: Optional[str] = None) -> str:
|
||||
@@ -1463,33 +1479,7 @@ def browser_press(key: str, task_id: Optional[str] = None) -> str:
|
||||
}, ensure_ascii=False)
|
||||
|
||||
|
||||
def browser_close(task_id: Optional[str] = None) -> str:
|
||||
"""
|
||||
Close the browser session.
|
||||
|
||||
Args:
|
||||
task_id: Task identifier for session isolation
|
||||
|
||||
Returns:
|
||||
JSON string with close result
|
||||
"""
|
||||
if _is_camofox_mode():
|
||||
from tools.browser_camofox import camofox_close
|
||||
return camofox_close(task_id)
|
||||
|
||||
effective_task_id = task_id or "default"
|
||||
with _cleanup_lock:
|
||||
had_session = effective_task_id in _active_sessions
|
||||
|
||||
cleanup_browser(effective_task_id)
|
||||
|
||||
response = {
|
||||
"success": True,
|
||||
"closed": True,
|
||||
}
|
||||
if not had_session:
|
||||
response["warning"] = "Session may not have been active"
|
||||
return json.dumps(response, ensure_ascii=False)
|
||||
|
||||
|
||||
def browser_console(clear: bool = False, expression: Optional[str] = None, task_id: Optional[str] = None) -> str:
|
||||
@@ -1942,7 +1932,7 @@ def cleanup_browser(task_id: Optional[str] = None) -> None:
|
||||
Clean up browser session for a task.
|
||||
|
||||
Called automatically when a task completes or when inactivity timeout is reached.
|
||||
Closes both the agent-browser session and the Browserbase session.
|
||||
Closes both the agent-browser/Browserbase session and Camofox sessions.
|
||||
|
||||
Args:
|
||||
task_id: Task identifier to clean up
|
||||
@@ -1950,6 +1940,14 @@ def cleanup_browser(task_id: Optional[str] = None) -> None:
|
||||
if task_id is None:
|
||||
task_id = "default"
|
||||
|
||||
# Also clean up Camofox session if running in Camofox mode
|
||||
if _is_camofox_mode():
|
||||
try:
|
||||
from tools.browser_camofox import camofox_close
|
||||
camofox_close(task_id)
|
||||
except Exception as e:
|
||||
logger.debug("Camofox cleanup for task %s: %s", task_id, e)
|
||||
|
||||
logger.debug("cleanup_browser called for task_id: %s", task_id)
|
||||
logger.debug("Active sessions: %s", list(_active_sessions.keys()))
|
||||
|
||||
@@ -2168,14 +2166,7 @@ registry.register(
|
||||
check_fn=check_browser_requirements,
|
||||
emoji="⌨️",
|
||||
)
|
||||
registry.register(
|
||||
name="browser_close",
|
||||
toolset="browser",
|
||||
schema=_BROWSER_SCHEMA_MAP["browser_close"],
|
||||
handler=lambda args, **kw: browser_close(task_id=kw.get("task_id")),
|
||||
check_fn=check_browser_requirements,
|
||||
emoji="🚪",
|
||||
)
|
||||
|
||||
registry.register(
|
||||
name="browser_get_images",
|
||||
toolset="browser",
|
||||
|
||||
@@ -103,6 +103,32 @@ def _canonical_skills(skill: Optional[str] = None, skills: Optional[Any] = None)
|
||||
|
||||
|
||||
|
||||
|
||||
def _resolve_model_override(model_obj: Optional[Dict[str, Any]]) -> tuple:
|
||||
"""Resolve a model override object into (provider, model) for job storage.
|
||||
|
||||
If provider is omitted, pins the current main provider from config so the
|
||||
job doesn't drift when the user later changes their default via hermes model.
|
||||
|
||||
Returns (provider_str_or_none, model_str_or_none).
|
||||
"""
|
||||
if not model_obj or not isinstance(model_obj, dict):
|
||||
return (None, None)
|
||||
model_name = (model_obj.get("model") or "").strip() or None
|
||||
provider_name = (model_obj.get("provider") or "").strip() or None
|
||||
if model_name and not provider_name:
|
||||
# Pin to the current main provider so the job is stable
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
cfg = load_config()
|
||||
model_cfg = cfg.get("model", {})
|
||||
if isinstance(model_cfg, dict):
|
||||
provider_name = model_cfg.get("provider") or None
|
||||
except Exception:
|
||||
pass # Best-effort; provider stays None
|
||||
return (provider_name, model_name)
|
||||
|
||||
|
||||
def _normalize_optional_job_value(value: Optional[Any], *, strip_trailing_slash: bool = False) -> Optional[str]:
|
||||
if value is None:
|
||||
return None
|
||||
@@ -392,14 +418,9 @@ Use action='list' to inspect jobs.
|
||||
Use action='update', 'pause', 'resume', 'remove', or 'run' to manage an existing job.
|
||||
|
||||
Jobs run in a fresh session with no current-chat context, so prompts must be self-contained.
|
||||
If skill or skills are provided on create, the future cron run loads those skills in order, then follows the prompt as the task instruction.
|
||||
If skills are provided on create, the future cron run loads those skills in order, then follows the prompt as the task instruction.
|
||||
On update, passing skills=[] clears attached skills.
|
||||
|
||||
If script is provided on create, the referenced Python script runs before each agent turn.
|
||||
Its stdout is injected into the prompt as context. Use this for data collection and change
|
||||
detection — the script handles gathering data, the agent analyzes and reports.
|
||||
On update, pass script="" to clear an attached script.
|
||||
|
||||
NOTE: The agent's final response is auto-delivered to the target. Put the primary
|
||||
user-facing content in the final response. Cron jobs run autonomously with no user
|
||||
present — they cannot ask questions or request clarification.
|
||||
@@ -418,7 +439,7 @@ Important safety rule: cron-run sessions should not recursively schedule more cr
|
||||
},
|
||||
"prompt": {
|
||||
"type": "string",
|
||||
"description": "For create: the full self-contained prompt. If skill or skills are also provided, this becomes the task instruction paired with those skills."
|
||||
"description": "For create: the full self-contained prompt. If skills are also provided, this becomes the task instruction paired with those skills."
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
@@ -436,39 +457,30 @@ Important safety rule: cron-run sessions should not recursively schedule more cr
|
||||
"type": "string",
|
||||
"description": "Delivery target: origin, local, telegram, discord, slack, whatsapp, signal, matrix, mattermost, homeassistant, dingtalk, feishu, wecom, email, sms, or platform:chat_id or platform:chat_id:thread_id for Telegram topics. Examples: 'origin', 'local', 'telegram', 'telegram:-1001234567890:17585', 'discord:#engineering'"
|
||||
},
|
||||
"model": {
|
||||
"type": "string",
|
||||
"description": "Optional per-job model override used when the cron job runs"
|
||||
},
|
||||
"provider": {
|
||||
"type": "string",
|
||||
"description": "Optional per-job provider override used when resolving runtime credentials"
|
||||
},
|
||||
"base_url": {
|
||||
"type": "string",
|
||||
"description": "Optional per-job base URL override paired with provider/model routing"
|
||||
},
|
||||
"include_disabled": {
|
||||
"type": "boolean",
|
||||
"description": "For list: include paused/completed jobs"
|
||||
},
|
||||
"skill": {
|
||||
"type": "string",
|
||||
"description": "Optional single skill name to load before executing the cron prompt"
|
||||
},
|
||||
"skills": {
|
||||
"type": "array",
|
||||
"items": {"type": "string"},
|
||||
"description": "Optional ordered list of skills to load before executing the cron prompt. On update, pass an empty array to clear attached skills."
|
||||
"description": "Optional ordered list of skill names to load before executing the cron prompt. On update, pass an empty array to clear attached skills."
|
||||
},
|
||||
"reason": {
|
||||
"type": "string",
|
||||
"description": "Optional pause reason"
|
||||
"model": {
|
||||
"type": "object",
|
||||
"description": "Optional per-job model override. If provider is omitted, the current main provider is pinned at creation time so the job stays stable.",
|
||||
"properties": {
|
||||
"provider": {
|
||||
"type": "string",
|
||||
"description": "Provider name (e.g. 'openrouter', 'anthropic'). Omit to use and pin the current provider."
|
||||
},
|
||||
"model": {
|
||||
"type": "string",
|
||||
"description": "Model name (e.g. 'anthropic/claude-sonnet-4', 'claude-sonnet-4')"
|
||||
}
|
||||
},
|
||||
"required": ["model"]
|
||||
},
|
||||
"script": {
|
||||
"type": "string",
|
||||
"description": "Optional path to a Python script that runs before each cron job execution. Its stdout is injected into the prompt as context. Use for data collection and change detection. Relative paths resolve under ~/.hermes/scripts/. On update, pass empty string to clear."
|
||||
}
|
||||
},
|
||||
},
|
||||
"required": ["action"]
|
||||
}
|
||||
@@ -502,7 +514,7 @@ registry.register(
|
||||
name="cronjob",
|
||||
toolset="cronjob",
|
||||
schema=CRONJOB_SCHEMA,
|
||||
handler=lambda args, **kw: cronjob(
|
||||
handler=lambda args, **kw: (lambda _mo=_resolve_model_override(args.get("model")): cronjob(
|
||||
action=args.get("action", ""),
|
||||
job_id=args.get("job_id"),
|
||||
prompt=args.get("prompt"),
|
||||
@@ -510,16 +522,16 @@ registry.register(
|
||||
name=args.get("name"),
|
||||
repeat=args.get("repeat"),
|
||||
deliver=args.get("deliver"),
|
||||
include_disabled=args.get("include_disabled", False),
|
||||
include_disabled=args.get("include_disabled", True),
|
||||
skill=args.get("skill"),
|
||||
skills=args.get("skills"),
|
||||
model=args.get("model"),
|
||||
provider=args.get("provider"),
|
||||
model=_mo[1],
|
||||
provider=_mo[0] or args.get("provider"),
|
||||
base_url=args.get("base_url"),
|
||||
reason=args.get("reason"),
|
||||
script=args.get("script"),
|
||||
task_id=kw.get("task_id"),
|
||||
),
|
||||
))(),
|
||||
check_fn=check_cronjob_requirements,
|
||||
emoji="⏰",
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user