Skip to content
Merged
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
70 changes: 65 additions & 5 deletions agent/replay_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import logging
from typing import Any, Dict, List

from agent.tool_dispatch_helpers import make_tool_result_message
from agent.tool_result_classification import tool_may_have_side_effect

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -64,8 +67,40 @@ def strip_interrupted_tool_tails(
is_interrupted_tool_result(m.get("content", ""))
for m in tool_results
):
calls = msg.get("tool_calls") or []
if any(
tool_may_have_side_effect(
str((call.get("function") or {}).get("name") or "")
)
for call in calls
):
call_names = {
str(call.get("id") or call.get("call_id") or ""): str(
(call.get("function") or {}).get("name") or ""
)
for call in calls
}
cleaned.append(msg)
for tool_result in tool_results:
if not is_interrupted_tool_result(tool_result.get("content", "")):
cleaned.append(tool_result)
continue
recovered = dict(tool_result)
name = call_names.get(str(tool_result.get("tool_call_id") or ""), "")
recovered["effect_disposition"] = (
"unknown" if tool_may_have_side_effect(name) else "none"
)
recovered["content"] = (
"[Orphan recovery: interrupted side-effecting tool may have "
"executed; its effect is UNKNOWN. Inspect state before retrying.]"
if recovered["effect_disposition"] == "unknown"
else "[Orphan recovery: interrupted read-only tool did not complete.]"
)
cleaned.append(recovered)
i = j
continue
logger.debug(
"Stripping interrupted assistant→tool replay block "
"Stripping interrupted read-only assistant→tool replay block "
"(indices %d–%d, tool_results=%d)",
i, j - 1, len(tool_results),
)
Expand Down Expand Up @@ -116,11 +151,36 @@ def strip_dangling_tool_call_tail(
):
return agent_history

tool_calls = last.get("tool_calls") or []
if any(
tool_may_have_side_effect(
str((call.get("function") or {}).get("name") or "")
)
for call in tool_calls
):
recovered = list(agent_history)
for call in tool_calls:
function = call.get("function") or {}
name = str(function.get("name") or "unknown")
call_id = str(call.get("id") or call.get("call_id") or "")
disposition = "unknown" if tool_may_have_side_effect(name) else "none"
content = (
"[Orphan recovery: this tool may have executed before Hermes stopped; "
"its effect is UNKNOWN. Inspect current state before retrying.]"
if disposition == "unknown"
else "[Orphan recovery: this read-only tool did not complete and had no effect.]"
)
recovered.append(make_tool_result_message(
name, content, call_id, effect_disposition=disposition,
))
logger.warning(
"Recovered dangling side-effecting tool call(s) as UNKNOWN instead of erasing them"
)
return recovered

logger.debug(
"Stripping dangling unanswered assistant(tool_calls) tail "
"(%d call(s)) — process likely killed mid-tool-call by a "
"restart/shutdown command (#49201)",
len(last.get("tool_calls") or []),
"Stripping dangling unanswered read-only assistant(tool_calls) tail (%d call(s))",
len(tool_calls),
)
return agent_history[:-1]

Expand Down
10 changes: 9 additions & 1 deletion agent/tool_dispatch_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,13 @@ def _trajectory_normalize_msg(msg: Dict[str, Any]) -> Dict[str, Any]:
return msg


def make_tool_result_message(name: str, content: Any, tool_call_id: str) -> dict:
def make_tool_result_message(
name: str,
content: Any,
tool_call_id: str,
*,
effect_disposition: str | None = None,
) -> dict:
"""Build a tool-result message dict with both the OpenAI-format ``name``
field (required by the wire format and provider adapters) and the internal
``tool_name`` field (written to the session DB messages table).
Expand Down Expand Up @@ -394,6 +400,8 @@ def make_tool_result_message(name: str, content: Any, tool_call_id: str) -> dict
else:
if risk_metadata is not None:
message["_tool_output_risk"] = risk_metadata
if effect_disposition is not None:
message["effect_disposition"] = effect_disposition
return message


Expand Down
14 changes: 13 additions & 1 deletion agent/tool_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe
tc.function.name,
f"[Tool execution cancelled — {tc.function.name} was skipped due to user interrupt]",
tc.id,
effect_disposition="none",
))
_flush_session_db_after_tool_progress(
agent,
Expand Down Expand Up @@ -827,9 +828,11 @@ def _run_tool(index, tool_call, function_name, function_args, middleware_trace):
# deadline snapshot (timed_out_indices, taken from not_done) and this
# loop. Prefer that real result over a fabricated timeout message — the
# tool genuinely succeeded, just slightly late.
effect_disposition = None
if i in timed_out_indices and r is None:
suffix = f"{timeout_s:.1f}s" if timeout_s is not None else "the configured timeout"
function_result = f"Error executing tool '{name}': timed out after {suffix}"
effect_disposition = "unknown"
_emit_terminal_post_tool_call(
agent,
function_name=name,
Expand Down Expand Up @@ -876,6 +879,8 @@ def _run_tool(index, tool_call, function_name, function_args, middleware_trace):
tool_duration = 0.0
else:
function_name, function_args, function_result, tool_duration, is_error, blocked, middleware_trace = r
if blocked:
effect_disposition = "none"

if not blocked:
function_result = agent._append_guardrail_observation(
Expand Down Expand Up @@ -964,7 +969,12 @@ def _run_tool(index, tool_call, function_name, function_args, middleware_trace):
# image tool result never poisons canonical session history.
# String results pass through unchanged.
_tool_content = agent._tool_result_content_for_active_model(name, function_result)
tool_message = make_tool_result_message(name, _tool_content, tc.id)
tool_message = make_tool_result_message(
name,
_tool_content,
tc.id,
effect_disposition=effect_disposition,
)
messages.append(tool_message)
risk_metadata = tool_message.get("_tool_output_risk")
if (
Expand Down Expand Up @@ -1027,6 +1037,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe
skipped_name,
f"[Tool execution cancelled — {skipped_name} was skipped due to user interrupt]",
skipped_tc.id,
effect_disposition="none",
))
_flush_session_db_after_tool_progress(
agent,
Expand Down Expand Up @@ -1691,6 +1702,7 @@ def _execute(next_args: dict) -> Any:
skipped_name,
f"[Tool execution skipped — {skipped_name} was not started. User sent a new message]",
skipped_tc.id,
effect_disposition="none",
))
_flush_session_db_after_tool_progress(
agent,
Expand Down
14 changes: 14 additions & 0 deletions agent/tool_result_classification.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,20 @@
FILE_MUTATING_TOOL_NAMES = frozenset({"write_file", "patch"})


# Tools whose interrupted/dangling execution is safe to discard because they
# cannot mutate either external state or Hermes session state. Unknown/plugin/
# MCP tools stay effect-capable by default.
NO_EFFECT_TOOL_NAMES = frozenset({
"read_file", "search_files", "session_search", "skill_view", "skills_list",
"web_extract", "web_search", "vision_analyze", "browser_snapshot",
"browser_get_images", "browser_console", "read_terminal",
})


def tool_may_have_side_effect(tool_name: str) -> bool:
return tool_name not in NO_EFFECT_TOOL_NAMES


def file_mutation_result_landed(tool_name: str, result: Any) -> bool:
"""Return True when a file mutation result proves the write landed."""
if tool_name not in FILE_MUTATING_TOOL_NAMES or not isinstance(result, str):
Expand Down
3 changes: 3 additions & 0 deletions agent/transports/chat_completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ def convert_messages(
"codex_reasoning_items" in msg
or "codex_message_items" in msg
or "tool_name" in msg
or "effect_disposition" in msg
or "timestamp" in msg # #47868 — strict providers reject this
):
needs_sanitize = True
Expand Down Expand Up @@ -212,12 +213,14 @@ def mutable_msg() -> dict[str, Any]:
"codex_reasoning_items" in msg
or "codex_message_items" in msg
or "tool_name" in msg
or "effect_disposition" in msg
or "timestamp" in msg # #47868 — leak into strict providers
):
out_msg = mutable_msg()
out_msg.pop("codex_reasoning_items", None)
out_msg.pop("codex_message_items", None)
out_msg.pop("tool_name", None)
out_msg.pop("effect_disposition", None)
out_msg.pop("timestamp", None) # #47868 — leak into strict providers


Expand Down
16 changes: 11 additions & 5 deletions hermes_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A
tool_call_id TEXT,
tool_calls TEXT,
tool_name TEXT,
effect_disposition TEXT,
timestamp REAL NOT NULL,
token_count INTEGER,
finish_reason TEXT,
Expand Down Expand Up @@ -3455,6 +3456,7 @@ def append_message(
codex_message_items: Any = None,
platform_message_id: str = None,
observed: bool = False,
effect_disposition: Optional[str] = None,
timestamp: Any = None,
) -> int:
"""
Expand Down Expand Up @@ -3505,17 +3507,18 @@ def append_message(
def _do(conn):
cursor = conn.execute(
"""INSERT INTO messages (session_id, role, content, tool_call_id,
tool_calls, tool_name, timestamp, token_count, finish_reason,
tool_calls, tool_name, effect_disposition, timestamp, token_count, finish_reason,
reasoning, reasoning_content, reasoning_details, codex_reasoning_items,
codex_message_items, platform_message_id, observed, active)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
(
session_id,
role,
stored_content,
tool_call_id,
tool_calls_json,
tool_name,
effect_disposition,
message_timestamp,
token_count,
finish_reason,
Expand Down Expand Up @@ -3597,17 +3600,18 @@ def _insert_message_rows(self, conn, session_id: str, messages: List[Dict[str, A

conn.execute(
"""INSERT INTO messages (session_id, role, content, tool_call_id,
tool_calls, tool_name, timestamp, token_count, finish_reason,
tool_calls, tool_name, effect_disposition, timestamp, token_count, finish_reason,
reasoning, reasoning_content, reasoning_details, codex_reasoning_items,
codex_message_items, platform_message_id, observed, active)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
(
session_id,
role,
self._encode_content(msg.get("content")),
msg.get("tool_call_id"),
tool_calls_json,
msg.get("tool_name"),
msg.get("effect_disposition"),
message_timestamp,
msg.get("token_count"),
msg.get("finish_reason"),
Expand Down Expand Up @@ -4103,7 +4107,7 @@ def get_messages_as_conversation(
with self._lock:
placeholders = ",".join("?" for _ in session_ids)
rows = self._conn.execute(
"SELECT role, content, tool_call_id, tool_calls, tool_name, "
"SELECT role, content, tool_call_id, tool_calls, tool_name, effect_disposition, "
"finish_reason, reasoning, reasoning_content, reasoning_details, "
"codex_reasoning_items, codex_message_items, platform_message_id, observed, timestamp "
f"FROM messages WHERE session_id IN ({placeholders})"
Expand Down Expand Up @@ -4131,6 +4135,8 @@ def get_messages_as_conversation(
msg["tool_call_id"] = row["tool_call_id"]
if row["tool_name"]:
msg["tool_name"] = row["tool_name"]
if row["effect_disposition"]:
msg["effect_disposition"] = row["effect_disposition"]
if row["tool_calls"]:
try:
msg["tool_calls"] = json.loads(row["tool_calls"])
Expand Down
70 changes: 63 additions & 7 deletions tests/agent/test_replay_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,75 @@ def test_is_interrupted_tool_result_markers():
assert not is_interrupted_tool_result(None)


def test_strip_dangling_tool_call_tail_removes_unanswered_tail():
history = [_user("hi"), _assistant_tc("write_file")]
def test_strip_dangling_tool_call_tail_removes_unanswered_read_only_tail():
history = [_user("hi"), _assistant_tc("read_file")]
out = strip_dangling_tool_call_tail(history)
assert out == [_user("hi")]


def test_dangling_side_effect_is_recovered_as_unknown_not_erased():
history = [_user("hi"), _assistant_tc("write_file")]

out = strip_dangling_tool_call_tail(history)

assert out[:-1] == history
assert out[-1]["role"] == "tool"
assert out[-1]["tool_call_id"] == "c1"
assert out[-1]["effect_disposition"] == "unknown"
assert "may have executed" in out[-1]["content"].lower()


def test_dangling_session_mutation_is_recovered_as_unknown():
history = [_user("hi"), _assistant_tc("todo")]

out = strip_dangling_tool_call_tail(history)

assert out[:-1] == history
assert out[-1]["effect_disposition"] == "unknown"
assert "may have executed" in out[-1]["content"].lower()


def test_mixed_dangling_batch_uses_truthful_per_call_wording():
assistant = {
"role": "assistant",
"content": "",
"tool_calls": [
{"id": "read", "function": {"name": "read_file", "arguments": "{}"}},
{"id": "write", "function": {"name": "write_file", "arguments": "{}"}},
],
}
out = strip_dangling_tool_call_tail([_user("hi"), assistant])

read_result, write_result = out[-2:]
assert read_result["effect_disposition"] == "none"
assert "no effect" in read_result["content"].lower()
assert "unknown" not in read_result["content"].lower()
assert write_result["effect_disposition"] == "unknown"
assert "unknown" in write_result["content"].lower()


def test_strip_dangling_tool_call_tail_preserves_answered_pair():
history = [_user("hi"), _assistant_tc("read_file"), _tool("contents")]
out = strip_dangling_tool_call_tail(history)
assert out == history # answered -> untouched


def test_strip_interrupted_tool_tails_removes_interrupted_block():
history = [_user("hi"), _assistant_tc("terminal"), _tool("[Command interrupted]")]
def test_strip_interrupted_tool_tails_removes_interrupted_read_only_block():
history = [_user("hi"), _assistant_tc("read_file"), _tool("[Command interrupted]")]
out = strip_interrupted_tool_tails(history)
assert out == [_user("hi")]


def test_interrupted_side_effect_is_preserved_as_unknown():
history = [_user("hi"), _assistant_tc("terminal"), _tool("[Command interrupted]")]

out = strip_interrupted_tool_tails(history)

assert out[:-1] == history[:-1]
assert out[-1]["role"] == "tool"
assert out[-1]["effect_disposition"] == "unknown"


def test_strip_interrupted_tool_tails_preserves_successful_block():
history = [_user("hi"), _assistant_tc("read_file"), _tool("ok"),
{"role": "assistant", "content": "done"}]
Expand All @@ -72,15 +123,20 @@ def test_strip_interrupted_tool_tails_removes_orphan_interrupted_tool():


def test_sanitize_replay_history_combines_both():
# interrupted block in the middle + dangling tail at the end
# interrupted block is removed; a dangling read-only call is safe to erase
history = [
_user("first"),
_assistant_tc("terminal"), _tool("[Command interrupted]"),
_user("second"),
_assistant_tc("write_file"), # dangling
_assistant_tc("read_file"), # dangling
]
out = sanitize_replay_history(history)
assert out == [_user("first"), _user("second")]
assert out[:2] == [
_user("first"),
_assistant_tc("terminal"),
]
assert out[2]["effect_disposition"] == "unknown"
assert out[-1] == _user("second")


def test_sanitize_replay_history_noop_on_clean_history():
Expand Down
Loading
Loading