mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 18:33:15 +00:00
feat: restore forced tool call to prevent premature run stops (#1622)
Restore the ensure_no_empty_msg middleware and the always-call-a-tool system-prompt instruction that #1535 removed. When the model emits a message with no tool call (and hasn't already messaged the user or confirmed completion), the middleware re-injects a no_op / confirming_completion tool call so the run continues instead of ending mid-task. Shipping to test whether it fixes runs that stop halfway through. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
209132d355
commit
8c944381b7
8 changed files with 377 additions and 10 deletions
11
AGENTS.md
11
AGENTS.md
|
|
@ -68,12 +68,13 @@ 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. `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.
|
||||
6. `ensure_no_empty_msg` — after-model hook; when the model emits a message with no tool call (and hasn't already messaged the user or confirmed completion) it re-injects a synthetic `no_op` / `confirming_completion` tool call so the run continues instead of ending prematurely.
|
||||
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.
|
||||
|
||||
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.
|
||||
The system prompt instructs the agent to call a tool every turn, and `ensure_no_empty_msg` re-injects a tool call when it doesn't — together these keep runs from stopping partway through a task.
|
||||
|
||||
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`.
|
||||
|
||||
|
|
|
|||
|
|
@ -65,11 +65,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. `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.
|
||||
6. `ensure_no_empty_msg` — after-model hook; when the model emits a message with no tool call (and hasn't already messaged the user or confirmed completion) it re-injects a synthetic `no_op` / `confirming_completion` tool call so the run continues instead of ending prematurely.
|
||||
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.
|
||||
|
||||
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.
|
||||
The system prompt instructs the agent to call a tool every turn, and `ensure_no_empty_msg` re-injects a tool call when it doesn't — together these keep runs from stopping partway through a task.
|
||||
|
||||
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`.
|
||||
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ return create_deep_agent(
|
|||
middleware=[
|
||||
ToolErrorMiddleware(),
|
||||
check_message_queue_before_model,
|
||||
ensure_no_empty_msg,
|
||||
notify_step_limit_reached,
|
||||
],
|
||||
)
|
||||
|
|
@ -465,6 +466,7 @@ 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` | After model | Re-injects a tool call when the model stops without one, so runs don't end prematurely |
|
||||
| `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.
|
||||
|
|
@ -490,6 +492,7 @@ 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
|
||||
],
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
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
|
||||
|
|
@ -27,6 +28,7 @@ __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",
|
||||
|
|
|
|||
124
agent/middleware/ensure_no_empty_msg.py
Normal file
124
agent/middleware/ensure_no_empty_msg.py
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
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
|
||||
|
|
@ -83,7 +83,10 @@ OPEN_SWE_SHARED_BASE = """You are **Open SWE**, an open-source agent built on La
|
|||
### Communication
|
||||
|
||||
- Focus on the substance and keep summaries brief. Use light markdown (`###`/`####` headings, bold, code) — avoid `#`/`##` titles.
|
||||
- When delegated work to a subagent: the calling agent only sees your final message, so make it the complete answer."""
|
||||
- When delegated work to a subagent: the calling agent only sees your final message, so make it the complete answer.
|
||||
|
||||
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."""
|
||||
|
||||
|
||||
WORKING_ENV_SECTION = """### Working Environment
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ from .middleware import (
|
|||
ToolErrorMiddleware,
|
||||
WorkflowPushGuardMiddleware,
|
||||
check_message_queue_before_model,
|
||||
ensure_no_empty_msg,
|
||||
notify_step_limit_reached,
|
||||
refresh_github_proxy_before_model,
|
||||
)
|
||||
|
|
@ -851,6 +852,7 @@ 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,
|
||||
|
|
|
|||
231
tests/test_ensure_no_empty_msg.py
Normal file
231
tests/test_ensure_no_empty_msg.py
Normal file
|
|
@ -0,0 +1,231 @@
|
|||
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
|
||||
Loading…
Add table
Reference in a new issue