From 5be61b0e4d12b80ea2ff0aeeb34663b6fd99ed8e Mon Sep 17 00:00:00 2001
From: ljluestc <63439129+ljluestc@users.noreply.github.com>
Date: Fri, 2 Oct 2026 22:17:51 -0700
Subject: [PATCH] fix(terminal): name the stray args when refusing a foreground
 notify/heartbeat call

The refusal for background-only arguments on a foreground terminal call
was generic and led with `background=true`. Models re-sent the identical
call (8x retry loops reported on #119196) or moved short commands into
the background and flooded completion notices. 4317ed0e71 stopped
schema-materialized providers from sending heartbeat=60, but callers that
still send it explicitly hit the same loop (#125303 shows kanban workers
doing this).

The refusal is kept, as intended. It now names the exact offending
arguments as sent (`heartbeat=60`), says the command did not run, and
puts the foreground retry (same call without those args) before the
background option.

Closes #121634
---
 tests/tools/test_process_heartbeat.py | 27 +++++++++++++++++++++++++++
 tools/terminal_tool.py                | 19 +++++++++++++++----
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/tests/tools/test_process_heartbeat.py b/tests/tools/test_process_heartbeat.py
index b708a2bc457f2..1cb0c05fe6278 100644
--- a/tests/tools/test_process_heartbeat.py
+++ b/tests/tools/test_process_heartbeat.py
@@ -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
diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py
index 474b64e89260f..236eb9572eaf7 100644
--- a/tools/terminal_tool.py
+++ b/tools/terminal_tool.py
@@ -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(