Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions tests/tools/test_process_heartbeat.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,30 @@ def fake_terminal_tool(**kwargs):
bg = json.loads(dispatch({"command": "sleep 1", "background": True, "heartbeat": 120}))
assert "error" not in bg or not bg["error"]
assert captured["heartbeat"] == 120 and captured["notify_on_complete"] is True


@pytest.mark.parametrize("stray", [{"heartbeat": 60}, {"notify": True}, {"notify": ["READY"], "heartbeat": 90}])
def test_foreground_refusal_names_args_whose_removal_runs_the_call(monkeypatch, stray):
"""Regression for #121634: the refusal must name the exact background-only
args the model sent, and dropping just those must run the same call in the
foreground — so the model corrects once instead of re-sending it verbatim
or moving a short command into the background."""
from tools import terminal_tool as tt

captured = {}

def fake_terminal_tool(**kwargs):
captured.update(kwargs)
return json.dumps({"output": "/tmp", "exit_code": 0})

monkeypatch.setattr(tt, "terminal_tool", fake_terminal_tool)
call = {"command": "pwd", "background": False, "timeout": 20, **stray}

refused = json.loads(tt._handle_terminal(call))["error"]
named = [name for name in stray if f"{name}={json.dumps(stray[name])}" in refused]
assert named == list(stray)
assert not captured

retried = {k: v for k, v in call.items() if k not in named}
assert not json.loads(tt._handle_terminal(retried)).get("error")
assert captured["background"] is False and captured["notify_on_complete"] is False
19 changes: 15 additions & 4 deletions tools/terminal_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -1580,11 +1580,22 @@ def _handle_terminal(args, **kw):
if not isinstance(heartbeat, int) or isinstance(heartbeat, bool) or heartbeat < 0:
return tool_error("heartbeat must be a whole number of seconds (0 disables; positive values are clamped to min 60).")
if not args.get("background", False):
if notify or watch_patterns or notify_on_complete or heartbeat:
# Name the exact offending arguments and lead with the foreground
# retry: a generic message got the same call re-sent verbatim, and
# leading with background=true pushed short commands into the
# background and flooded completion notices (#119196, #121634).
stray = [
name for name in ("notify", "heartbeat", "notify_on_complete", "watch_patterns")
if args.get(name)
]
if stray:
given = ", ".join(f"{name}={json.dumps(args[name])}" for name in stray)
return tool_error(
"notify/heartbeat only apply to background commands (foreground "
"results return directly). Either drop them, or run as "
"terminal(command=..., background=true, notify=...)."
f"Not run: {given} only apply to background commands "
"(foreground results return directly). To run this in the "
f"foreground, retry the same call without {', '.join(stray)}. Use "
"background=true only for long-running jobs you need "
"progress or completion notices for."
)
if args.get("pty", False):
return tool_error(
Expand Down