From ddd719086489e9fd803deb96df7e9523f43cb3e4 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Mon, 15 Jun 2026 14:54:01 -0700 Subject: [PATCH] feat: let the agent stop naturally without forced tool calls (#1535) Remove the hardcoded "call a tool every turn" instruction from the system prompt and delete the ensure_no_empty_msg middleware that re-injected no_op / confirming_completion tool calls. The agent now ends its turn naturally when the model emits a final message with no tool call, which avoids needlessly extending trajectories (and token spend) on tasks that are already complete. Co-authored-by: open-swe[bot] --- AGENTS.md | 11 +- CLAUDE.md | 9 +- CUSTOMIZATION.md | 3 - agent/middleware/__init__.py | 2 - agent/middleware/ensure_no_empty_msg.py | 124 ------------- agent/prompt.py | 3 - agent/server.py | 2 - tests/test_ensure_no_empty_msg.py | 231 ------------------------ 8 files changed, 11 insertions(+), 374 deletions(-) delete mode 100644 agent/middleware/ensure_no_empty_msg.py delete mode 100644 tests/test_ensure_no_empty_msg.py diff --git a/AGENTS.md b/AGENTS.md index a3ba9c55..6e52f5c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,11 +68,12 @@ Configured in `agent/server.py:get_agent`, runs around every model call (in this 3. `ToolErrorMiddleware` — catches tool exceptions and surfaces them as tool messages. 4. `check_message_queue_before_model` — pulls Linear comments / Slack messages that arrived mid-run from the thread queue and injects them as user messages before the next LLM call. This is what makes "message the agent while it's working" work. 5. `SlackAssistantStatusMiddleware` — keeps the Slack "assistant is typing"-style status up to date around model calls. -6. `ensure_no_empty_msg` — guards against empty assistant messages that some providers reject. -7. `notify_step_limit_reached` — after-agent hook that posts a Slack reply when the agent hits the step limit, so the user gets a clear signal instead of silence. -8. `SandboxCircuitBreakerMiddleware` — trips the agent out of repeated sandbox failures instead of looping. -9. `ModelFallbackMiddleware` (optional) — added only when `LLM_FALLBACK_MODEL_ID` or the per-model default fallback differs from the primary model. -10. `SanitizeThinkingBlocksMiddleware` — strips malformed empty Anthropic thinking blocks immediately before provider calls. +6. `notify_step_limit_reached` — after-agent hook that posts a Slack reply when the agent hits the step limit, so the user gets a clear signal instead of silence. +7. `SandboxCircuitBreakerMiddleware` — trips the agent out of repeated sandbox failures instead of looping. +8. `ModelFallbackMiddleware` (optional) — added only when `LLM_FALLBACK_MODEL_ID` or the per-model default fallback differs from the primary model. +9. `SanitizeThinkingBlocksMiddleware` — strips malformed empty Anthropic thinking blocks immediately before provider calls. + +The agent ends its turn naturally when the model emits a final message with no tool call; there is intentionally no middleware that forces a tool call on every turn. Other middleware exists in `agent/middleware/` (`ExcludeToolsMiddleware`) but isn't wired into the default agent. The reviewer uses a leaner stack: `SanitizeToolInputsMiddleware`, `ModelCallLimitMiddleware`, `ToolErrorMiddleware`, `SlackAssistantStatusMiddleware`, `SanitizeThinkingBlocksMiddleware`. diff --git a/CLAUDE.md b/CLAUDE.md index 11942dc6..7707bcf1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,10 +65,11 @@ Configured in `agent/server.py:get_agent`, runs around every model call (in this 3. `ToolErrorMiddleware` — catches tool exceptions and surfaces them as tool messages. 4. `check_message_queue_before_model` — pulls Linear comments / Slack messages that arrived mid-run from the thread queue and injects them as user messages before the next LLM call. This is what makes "message the agent while it's working" work. 5. `SlackAssistantStatusMiddleware` — keeps the Slack "assistant is typing"-style status up to date around model calls. -6. `ensure_no_empty_msg` — guards against empty assistant messages that some providers reject. -7. `notify_step_limit_reached` — after-agent hook that posts a Slack reply when the agent hits the step limit, so the user gets a clear signal instead of silence. -8. `SandboxCircuitBreakerMiddleware` — trips the agent out of repeated sandbox failures instead of looping. -9. `ModelFallbackMiddleware` (optional, last) — added only when `LLM_FALLBACK_MODEL_ID` or the per-model default fallback differs from the primary model. +6. `notify_step_limit_reached` — after-agent hook that posts a Slack reply when the agent hits the step limit, so the user gets a clear signal instead of silence. +7. `SandboxCircuitBreakerMiddleware` — trips the agent out of repeated sandbox failures instead of looping. +8. `ModelFallbackMiddleware` (optional, last) — added only when `LLM_FALLBACK_MODEL_ID` or the per-model default fallback differs from the primary model. + +The agent ends its turn naturally when the model emits a final message with no tool call; there is intentionally no middleware that forces a tool call on every turn. Other middleware exists in `agent/middleware/` (`ExcludeToolsMiddleware`) but isn't wired into the default agent. The reviewer uses a leaner stack: `SanitizeToolInputsMiddleware`, `ModelCallLimitMiddleware`, `ToolErrorMiddleware`, `SlackAssistantStatusMiddleware`. diff --git a/CUSTOMIZATION.md b/CUSTOMIZATION.md index 628a3e20..859e6e57 100644 --- a/CUSTOMIZATION.md +++ b/CUSTOMIZATION.md @@ -17,7 +17,6 @@ return create_deep_agent( middleware=[ ToolErrorMiddleware(), check_message_queue_before_model, - ensure_no_empty_msg, notify_step_limit_reached, ], ) @@ -463,7 +462,6 @@ Middleware hooks run around the agent loop. Open SWE includes: |---|---|---| | `ToolErrorMiddleware` | Tool error handler | Catches and formats tool errors | | `check_message_queue_before_model` | Before model | Injects follow-up messages that arrived mid-run | -| `ensure_no_empty_msg` | Before model | Prevents empty messages from reaching the model | | `notify_step_limit_reached` | After agent | Posts a Slack reply when the agent hits the model-call limit | There is intentionally no after-agent middleware that opens a PR for the agent. The agent is responsible for committing, pushing, opening/updating the draft PR, and replying in the source channel. If you want a deterministic backstop for your fork, add an `@after_agent` hook here. @@ -489,7 +487,6 @@ Then add it to the middleware list: middleware=[ ToolErrorMiddleware(), check_message_queue_before_model, - ensure_no_empty_msg, notify_step_limit_reached, run_ci_check, # new middleware ], diff --git a/agent/middleware/__init__.py b/agent/middleware/__init__.py index 0318d530..78c8a8b5 100644 --- a/agent/middleware/__init__.py +++ b/agent/middleware/__init__.py @@ -1,5 +1,4 @@ from .check_message_queue import check_message_queue_before_model -from .ensure_no_empty_msg import ensure_no_empty_msg from .exclude_tools import ExcludeToolsMiddleware from .model_fallback import ModelFallbackMiddleware from .notify_step_limit import notify_step_limit_reached @@ -22,7 +21,6 @@ __all__ = [ "SandboxCircuitBreakerMiddleware", "SlackAssistantStatusMiddleware", "check_message_queue_before_model", - "ensure_no_empty_msg", "notify_step_limit_reached", "refresh_github_proxy_before_model", "settle_review_check_on_exit", diff --git a/agent/middleware/ensure_no_empty_msg.py b/agent/middleware/ensure_no_empty_msg.py deleted file mode 100644 index 6e5da553..00000000 --- a/agent/middleware/ensure_no_empty_msg.py +++ /dev/null @@ -1,124 +0,0 @@ -from typing import Any -from uuid import uuid4 - -from langchain.agents.middleware import AgentState, after_model -from langchain_core.messages import AnyMessage, ToolMessage -from langgraph.config import get_config -from langgraph.runtime import Runtime - -from .check_message_queue import DASHBOARD_HANDOFF_MARKER - -_DASHBOARD_SOURCE = "dashboard" - - -def get_every_message_since_last_human(state: AgentState) -> list[AnyMessage]: - messages = state["messages"] - last_human_idx = -1 - for i in range(len(messages) - 1, -1, -1): - if messages[i].type == "human": - last_human_idx = i - break - return messages[last_human_idx + 1 :] - - -def check_if_model_messaged_user(messages: list[AnyMessage]) -> bool: - for msg in messages: - if msg.type == "tool" and msg.name in [ - "slack_thread_reply", - "linear_comment", - ]: - return True - return False - - -def check_if_confirming_completion(messages: list[AnyMessage]) -> bool: - for msg in messages: - if msg.type == "tool" and msg.name == "confirming_completion": - return True - return False - - -def check_if_no_op(messages: list[AnyMessage]) -> bool: - for msg in messages: - if msg.type == "tool" and msg.name == "no_op": - return True - return False - - -def _content_contains_text(content: object, text: str) -> bool: - if isinstance(content, str): - return text in content - if not isinstance(content, list): - return False - for block in content: - if isinstance(block, dict) and text in str(block.get("text", "")): - return True - return False - - -def _last_human_is_dashboard_handoff(state: AgentState) -> bool: - for msg in reversed(state["messages"]): - if msg.type == "human": - return _content_contains_text(msg.content, DASHBOARD_HANDOFF_MARKER) - return False - - -def _is_dashboard_source() -> bool: - try: - config = get_config() - except RuntimeError: - return False - configurable = config.get("configurable", {}) - if not isinstance(configurable, dict): - return False - return configurable.get("source") == _DASHBOARD_SOURCE - - -@after_model -def ensure_no_empty_msg(state: AgentState, runtime: Runtime) -> dict[str, Any] | None: - last_msg = state["messages"][-1] - has_contents = bool(last_msg.text()) - has_tool_calls = bool(last_msg.tool_calls) - if not has_tool_calls and not has_contents: - messages_since_last_human = get_every_message_since_last_human(state) - if check_if_no_op(messages_since_last_human): - return None - - if check_if_model_messaged_user(messages_since_last_human): - return None - - tc_id = str(uuid4()) - last_msg.tool_calls = [{"name": "no_op", "args": {}, "id": tc_id}] - no_op_tool_msg = ToolMessage( - content="No operation performed." - + "Please continue with the task, ensuring you ALWAYS call at least one tool in" - + " every message unless you are absolutely sure the task has been fully completed.", - tool_call_id=tc_id, - ) - - return {"messages": [last_msg, no_op_tool_msg]} - - if has_contents and not has_tool_calls: - messages_since_last_human = get_every_message_since_last_human(state) - - if ( - check_if_model_messaged_user(messages_since_last_human) - or check_if_confirming_completion(messages_since_last_human) - or _is_dashboard_source() - or _last_human_is_dashboard_handoff(state) - ): - return None - - tc_id = str(uuid4()) - last_msg.tool_calls = [{"name": "confirming_completion", "args": {}, "id": tc_id}] - no_op_tool_msg = ToolMessage( - content="Confirming task completion. I see you did not call a tool, which would end the task, however you haven't called a tool to message the user or open a pull request." - + "This may indicate premature termination - please ensure you fully complete the task before ending it. " - + "If you do not call any tools it will end the task.", - name="confirming_completion", - tool_call_id=tc_id, - ) - - return {"messages": [last_msg, no_op_tool_msg]} - - return None diff --git a/agent/prompt.py b/agent/prompt.py index fdba01f4..ec7a45bb 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -55,9 +55,6 @@ All code execution and file operations happen in this sandbox environment. - Direct GitHub API calls from the sandbox are also authenticated by the proxy; do not ask the user for a GitHub token. - The `execute` tool enforces a 5-minute timeout by default (300 seconds) - If a command times out and needs longer, rerun it by explicitly passing `timeout=` to the `execute` tool (e.g. `timeout=600` for 10 minutes) - -IMPORTANT: You must ALWAYS call a tool in EVERY SINGLE TURN. If you don't call a tool, the session will end and you won't be able to resume without the user manually restarting you. -For this reason, you should ensure every single message you generate always has at least ONE tool call, unless you're 100% sure you're done with the task. """ diff --git a/agent/server.py b/agent/server.py index afc44071..d8cb4ce7 100644 --- a/agent/server.py +++ b/agent/server.py @@ -55,7 +55,6 @@ from .middleware import ( ToolArtifactMiddleware, ToolErrorMiddleware, check_message_queue_before_model, - ensure_no_empty_msg, notify_step_limit_reached, refresh_github_proxy_before_model, ) @@ -720,7 +719,6 @@ async def get_agent(config: RunnableConfig) -> Pregel: refresh_github_proxy_before_model, check_message_queue_before_model, SlackAssistantStatusMiddleware(), - ensure_no_empty_msg, notify_step_limit_reached, SandboxCircuitBreakerMiddleware(), *fallback_middleware, diff --git a/tests/test_ensure_no_empty_msg.py b/tests/test_ensure_no_empty_msg.py deleted file mode 100644 index 8c99850a..00000000 --- a/tests/test_ensure_no_empty_msg.py +++ /dev/null @@ -1,231 +0,0 @@ -from unittest.mock import MagicMock, patch - -from langchain_core.messages import AIMessage, HumanMessage, ToolMessage - -from agent.middleware.check_message_queue import DASHBOARD_HANDOFF_INSTRUCTION -from agent.middleware.ensure_no_empty_msg import ( - check_if_confirming_completion, - check_if_model_messaged_user, - ensure_no_empty_msg, - get_every_message_since_last_human, -) - - -class TestGetEveryMessageSinceLastHuman: - def test_returns_messages_after_last_human(self) -> None: - state = { - "messages": [ - HumanMessage(content="first human"), - AIMessage(content="ai response"), - HumanMessage(content="second human"), - AIMessage(content="final ai"), - ] - } - - result = get_every_message_since_last_human(state) - - assert len(result) == 1 - assert result[0].content == "final ai" - - def test_returns_all_messages_when_no_human(self) -> None: - state = { - "messages": [ - AIMessage(content="ai 1"), - AIMessage(content="ai 2"), - ] - } - - result = get_every_message_since_last_human(state) - - assert len(result) == 2 - assert result[0].content == "ai 1" - assert result[1].content == "ai 2" - - def test_returns_empty_when_human_is_last(self) -> None: - state = { - "messages": [ - AIMessage(content="ai response"), - HumanMessage(content="human last"), - ] - } - - result = get_every_message_since_last_human(state) - - assert len(result) == 0 - - def test_returns_multiple_messages_after_human(self) -> None: - state = { - "messages": [ - HumanMessage(content="human"), - AIMessage(content="ai 1"), - ToolMessage(content="tool result", tool_call_id="123"), - AIMessage(content="ai 2"), - ] - } - - result = get_every_message_since_last_human(state) - - assert len(result) == 3 - assert result[0].content == "ai 1" - assert result[1].content == "tool result" - assert result[2].content == "ai 2" - - -class TestCheckIfModelMessagedUser: - def test_returns_true_for_slack_thread_reply(self) -> None: - messages = [ - ToolMessage(content="sent", tool_call_id="123", name="slack_thread_reply"), - ] - - assert check_if_model_messaged_user(messages) is True - - def test_returns_true_for_linear_comment(self) -> None: - messages = [ - ToolMessage(content="commented", tool_call_id="123", name="linear_comment"), - ] - - assert check_if_model_messaged_user(messages) is True - - def test_returns_false_for_other_tools(self) -> None: - messages = [ - ToolMessage(content="result", tool_call_id="123", name="bash"), - ToolMessage(content="result", tool_call_id="456", name="read_file"), - ] - - assert check_if_model_messaged_user(messages) is False - - def test_returns_false_for_empty_list(self) -> None: - assert check_if_model_messaged_user([]) is False - - -class TestCheckIfConfirmingCompletion: - def test_returns_true_when_confirming_completion_called(self) -> None: - messages = [ - ToolMessage(content="confirmed", tool_call_id="123", name="confirming_completion"), - ] - - assert check_if_confirming_completion(messages) is True - - def test_returns_false_for_other_tools(self) -> None: - messages = [ - ToolMessage(content="result", tool_call_id="123", name="bash"), - ] - - assert check_if_confirming_completion(messages) is False - - def test_returns_false_for_empty_list(self) -> None: - assert check_if_confirming_completion([]) is False - - def test_finds_confirming_completion_among_other_messages(self) -> None: - messages = [ - AIMessage(content="working"), - ToolMessage(content="done", tool_call_id="1", name="bash"), - ToolMessage(content="confirmed", tool_call_id="2", name="confirming_completion"), - AIMessage(content="finished"), - ] - - assert check_if_confirming_completion(messages) is True - - -class TestEnsureNoEmptyMsgNotify: - def _make_runtime(self) -> MagicMock: - return MagicMock() - - def test_returns_none_when_user_messaged(self) -> None: - empty_ai = AIMessage(content="") - state = { - "messages": [ - HumanMessage(content="fix the bug"), - ToolMessage(content="message sent", tool_call_id="1", name="slack_thread_reply"), - empty_ai, - ] - } - - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is None - - def test_returns_none_with_linear_comment(self) -> None: - empty_ai = AIMessage(content="") - state = { - "messages": [ - HumanMessage(content="fix the bug"), - ToolMessage(content="commented", tool_call_id="1", name="linear_comment"), - empty_ai, - ] - } - - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is None - - def test_injects_no_op_when_user_not_messaged(self) -> None: - empty_ai = AIMessage(content="") - state = { - "messages": [ - HumanMessage(content="fix the bug"), - ToolMessage(content="result", tool_call_id="1", name="bash"), - empty_ai, - ] - } - - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is not None - assert len(result["messages"]) == 2 - assert result["messages"][0].tool_calls[0]["name"] == "no_op" - - def test_returns_none_when_only_user_messaged(self) -> None: - empty_ai = AIMessage(content="") - state = { - "messages": [ - HumanMessage(content="fix the bug"), - ToolMessage(content="message sent", tool_call_id="1", name="slack_thread_reply"), - empty_ai, - ] - } - - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is None - - def test_skips_confirming_completion_for_dashboard_source(self) -> None: - ai = AIMessage(content="Hi! How can I help?") - state = { - "messages": [ - HumanMessage(content="hello"), - ai, - ] - } - - with patch( - "agent.middleware.ensure_no_empty_msg.get_config", - return_value={"configurable": {"source": "dashboard"}}, - ): - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is None - assert not ai.tool_calls - - def test_skips_confirming_completion_for_dashboard_handoff(self) -> None: - ai = AIMessage(content="Done in web.") - state = { - "messages": [ - HumanMessage( - content=[ - {"type": "text", "text": DASHBOARD_HANDOFF_INSTRUCTION}, - {"type": "text", "text": "continue in web"}, - ] - ), - ai, - ] - } - - with patch( - "agent.middleware.ensure_no_empty_msg.get_config", - return_value={"configurable": {"source": "slack"}}, - ): - result = ensure_no_empty_msg.after_model(state, self._make_runtime()) - - assert result is None - assert not ai.tool_calls