mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
refactor(terminal): remove check_interval parameter (#8001)
The check_interval parameter on terminal_tool sent periodic output updates to the gateway chat, but these were display-only — the agent couldn't see or act on them. This added schema bloat and introduced a bug where notify_on_complete=True was silently dropped when check_interval was also set (the not-check_interval guard skipped fast-watcher registration, and the check_interval watcher dict was missing the notify_on_complete key). Removing check_interval entirely: - Eliminates the notify_on_complete interaction bug - Reduces tool schema size (one fewer parameter for the model) - Simplifies the watcher registration path - notify_on_complete (agent wake-on-completion) still works - watch_patterns (output alerting) still works - process(action='poll') covers manual status checking Closes #7947 (root cause eliminated rather than patched).
This commit is contained in:
parent
06f862fa1b
commit
14ccd32cee
6 changed files with 8 additions and 48 deletions
|
|
@ -351,8 +351,9 @@ Cache-breaking forces dramatically higher costs. The ONLY time we alter context
|
||||||
|
|
||||||
### Background Process Notifications (Gateway)
|
### Background Process Notifications (Gateway)
|
||||||
|
|
||||||
When `terminal(background=true, check_interval=...)` is used, the gateway runs a watcher that
|
When `terminal(background=true, notify_on_complete=true)` is used, the gateway runs a watcher that
|
||||||
pushes status updates to the user's chat. Control verbosity with `display.background_process_notifications`
|
detects process completion and triggers a new agent turn. Control verbosity of background process
|
||||||
|
messages with `display.background_process_notifications`
|
||||||
in config.yaml (or `HERMES_BACKGROUND_NOTIFICATIONS` env var):
|
in config.yaml (or `HERMES_BACKGROUND_NOTIFICATIONS` env var):
|
||||||
|
|
||||||
- `all` — running-output updates + final message (default)
|
- `all` — running-output updates + final message (default)
|
||||||
|
|
|
||||||
|
|
@ -787,7 +787,7 @@ display:
|
||||||
|
|
||||||
# Background process notifications (gateway/messaging only).
|
# Background process notifications (gateway/messaging only).
|
||||||
# Controls how chatty the process watcher is when you use
|
# Controls how chatty the process watcher is when you use
|
||||||
# terminal(background=true, check_interval=...) from Telegram/Discord/etc.
|
# terminal(background=true, notify_on_complete=true) from Telegram/Discord/etc.
|
||||||
# off: No watcher messages at all
|
# off: No watcher messages at all
|
||||||
# result: Only the final completion message
|
# result: Only the final completion message
|
||||||
# error: Only the final message when exit code != 0
|
# error: Only the final message when exit code != 0
|
||||||
|
|
|
||||||
|
|
@ -380,7 +380,7 @@ class TestStubSchemaDrift(unittest.TestCase):
|
||||||
# Parameters that are internal (injected by the handler, not user-facing)
|
# Parameters that are internal (injected by the handler, not user-facing)
|
||||||
_INTERNAL_PARAMS = {"task_id", "user_task"}
|
_INTERNAL_PARAMS = {"task_id", "user_task"}
|
||||||
# Parameters intentionally blocked in the sandbox
|
# Parameters intentionally blocked in the sandbox
|
||||||
_BLOCKED_TERMINAL_PARAMS = {"background", "check_interval", "pty", "notify_on_complete"}
|
_BLOCKED_TERMINAL_PARAMS = {"background", "pty", "notify_on_complete"}
|
||||||
|
|
||||||
def test_stubs_cover_all_schema_params(self):
|
def test_stubs_cover_all_schema_params(self):
|
||||||
"""Every user-facing parameter in the real schema must appear in the
|
"""Every user-facing parameter in the real schema must appear in the
|
||||||
|
|
|
||||||
|
|
@ -301,7 +301,7 @@ def _call(tool_name, args):
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
# Terminal parameters that must not be used from ephemeral sandbox scripts
|
# Terminal parameters that must not be used from ephemeral sandbox scripts
|
||||||
_TERMINAL_BLOCKED_PARAMS = {"background", "check_interval", "pty", "notify_on_complete", "watch_patterns"}
|
_TERMINAL_BLOCKED_PARAMS = {"background", "pty", "notify_on_complete", "watch_patterns"}
|
||||||
|
|
||||||
|
|
||||||
def _rpc_server_loop(
|
def _rpc_server_loop(
|
||||||
|
|
|
||||||
|
|
@ -1137,7 +1137,6 @@ def terminal_tool(
|
||||||
task_id: Optional[str] = None,
|
task_id: Optional[str] = None,
|
||||||
force: bool = False,
|
force: bool = False,
|
||||||
workdir: Optional[str] = None,
|
workdir: Optional[str] = None,
|
||||||
check_interval: Optional[int] = None,
|
|
||||||
pty: bool = False,
|
pty: bool = False,
|
||||||
notify_on_complete: bool = False,
|
notify_on_complete: bool = False,
|
||||||
watch_patterns: Optional[List[str]] = None,
|
watch_patterns: Optional[List[str]] = None,
|
||||||
|
|
@ -1152,7 +1151,6 @@ def terminal_tool(
|
||||||
task_id: Unique identifier for environment isolation (optional)
|
task_id: Unique identifier for environment isolation (optional)
|
||||||
force: If True, skip dangerous command check (use after user confirms)
|
force: If True, skip dangerous command check (use after user confirms)
|
||||||
workdir: Working directory for this command (optional, uses session cwd if not set)
|
workdir: Working directory for this command (optional, uses session cwd if not set)
|
||||||
check_interval: Seconds between auto-checks for background processes (gateway only, min 30)
|
|
||||||
pty: If True, use pseudo-terminal for interactive CLI tools (local backend only)
|
pty: If True, use pseudo-terminal for interactive CLI tools (local backend only)
|
||||||
notify_on_complete: If True and background=True, auto-notify the agent when the process exits
|
notify_on_complete: If True and background=True, auto-notify the agent when the process exits
|
||||||
watch_patterns: List of strings to watch for in background output; triggers notification on match
|
watch_patterns: List of strings to watch for in background output; triggers notification on match
|
||||||
|
|
@ -1424,7 +1422,7 @@ def terminal_tool(
|
||||||
# turn. CLI mode uses the completion_queue directly.
|
# turn. CLI mode uses the completion_queue directly.
|
||||||
from gateway.session_context import get_session_env as _gse
|
from gateway.session_context import get_session_env as _gse
|
||||||
_gw_platform = _gse("HERMES_SESSION_PLATFORM", "")
|
_gw_platform = _gse("HERMES_SESSION_PLATFORM", "")
|
||||||
if _gw_platform and not check_interval:
|
if _gw_platform:
|
||||||
_gw_chat_id = _gse("HERMES_SESSION_CHAT_ID", "")
|
_gw_chat_id = _gse("HERMES_SESSION_CHAT_ID", "")
|
||||||
_gw_thread_id = _gse("HERMES_SESSION_THREAD_ID", "")
|
_gw_thread_id = _gse("HERMES_SESSION_THREAD_ID", "")
|
||||||
_gw_user_id = _gse("HERMES_SESSION_USER_ID", "")
|
_gw_user_id = _gse("HERMES_SESSION_USER_ID", "")
|
||||||
|
|
@ -1452,39 +1450,6 @@ def terminal_tool(
|
||||||
proc_session.watch_patterns = list(watch_patterns)
|
proc_session.watch_patterns = list(watch_patterns)
|
||||||
result_data["watch_patterns"] = proc_session.watch_patterns
|
result_data["watch_patterns"] = proc_session.watch_patterns
|
||||||
|
|
||||||
# Register check_interval watcher (gateway picks this up after agent run)
|
|
||||||
if check_interval and background:
|
|
||||||
effective_interval = max(30, check_interval)
|
|
||||||
if check_interval < 30:
|
|
||||||
result_data["check_interval_note"] = (
|
|
||||||
f"Requested {check_interval}s raised to minimum 30s"
|
|
||||||
)
|
|
||||||
from gateway.session_context import get_session_env as _gse2
|
|
||||||
watcher_platform = _gse2("HERMES_SESSION_PLATFORM", "")
|
|
||||||
watcher_chat_id = _gse2("HERMES_SESSION_CHAT_ID", "")
|
|
||||||
watcher_thread_id = _gse2("HERMES_SESSION_THREAD_ID", "")
|
|
||||||
watcher_user_id = _gse2("HERMES_SESSION_USER_ID", "")
|
|
||||||
watcher_user_name = _gse2("HERMES_SESSION_USER_NAME", "")
|
|
||||||
|
|
||||||
# Store on session for checkpoint persistence
|
|
||||||
proc_session.watcher_platform = watcher_platform
|
|
||||||
proc_session.watcher_chat_id = watcher_chat_id
|
|
||||||
proc_session.watcher_user_id = watcher_user_id
|
|
||||||
proc_session.watcher_user_name = watcher_user_name
|
|
||||||
proc_session.watcher_thread_id = watcher_thread_id
|
|
||||||
proc_session.watcher_interval = effective_interval
|
|
||||||
|
|
||||||
process_registry.pending_watchers.append({
|
|
||||||
"session_id": proc_session.id,
|
|
||||||
"check_interval": effective_interval,
|
|
||||||
"session_key": session_key,
|
|
||||||
"platform": watcher_platform,
|
|
||||||
"chat_id": watcher_chat_id,
|
|
||||||
"user_id": watcher_user_id,
|
|
||||||
"user_name": watcher_user_name,
|
|
||||||
"thread_id": watcher_thread_id,
|
|
||||||
})
|
|
||||||
|
|
||||||
return json.dumps(result_data, ensure_ascii=False)
|
return json.dumps(result_data, ensure_ascii=False)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return json.dumps({
|
return json.dumps({
|
||||||
|
|
@ -1767,11 +1732,6 @@ TERMINAL_SCHEMA = {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Working directory for this command (absolute path). Defaults to the session working directory."
|
"description": "Working directory for this command (absolute path). Defaults to the session working directory."
|
||||||
},
|
},
|
||||||
"check_interval": {
|
|
||||||
"type": "integer",
|
|
||||||
"description": "Seconds between automatic status checks for background processes (gateway/messaging only, minimum 30). When set, I'll proactively report progress.",
|
|
||||||
"minimum": 30
|
|
||||||
},
|
|
||||||
"pty": {
|
"pty": {
|
||||||
"type": "boolean",
|
"type": "boolean",
|
||||||
"description": "Run in pseudo-terminal (PTY) mode for interactive CLI tools like Codex, Claude Code, or Python REPL. Only works with local and SSH backends. Default: false.",
|
"description": "Run in pseudo-terminal (PTY) mode for interactive CLI tools like Codex, Claude Code, or Python REPL. Only works with local and SSH backends. Default: false.",
|
||||||
|
|
@ -1800,7 +1760,6 @@ def _handle_terminal(args, **kw):
|
||||||
timeout=args.get("timeout"),
|
timeout=args.get("timeout"),
|
||||||
task_id=kw.get("task_id"),
|
task_id=kw.get("task_id"),
|
||||||
workdir=args.get("workdir"),
|
workdir=args.get("workdir"),
|
||||||
check_interval=args.get("check_interval"),
|
|
||||||
pty=args.get("pty", False),
|
pty=args.get("pty", False),
|
||||||
notify_on_complete=args.get("notify_on_complete", False),
|
notify_on_complete=args.get("notify_on_complete", False),
|
||||||
watch_patterns=args.get("watch_patterns"),
|
watch_patterns=args.get("watch_patterns"),
|
||||||
|
|
|
||||||
|
|
@ -153,7 +153,7 @@ When your script calls a function like `web_search("query")`:
|
||||||
3. The result is sent back over the socket
|
3. The result is sent back over the socket
|
||||||
4. The function returns the parsed result
|
4. The function returns the parsed result
|
||||||
|
|
||||||
This means tool calls inside scripts behave identically to normal tool calls — same rate limits, same error handling, same capabilities. The only restriction is that `terminal()` is foreground-only (no `background`, `pty`, or `check_interval` parameters).
|
This means tool calls inside scripts behave identically to normal tool calls — same rate limits, same error handling, same capabilities. The only restriction is that `terminal()` is foreground-only (no `background` or `pty` parameters).
|
||||||
|
|
||||||
## Error Handling
|
## Error Handling
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue