diff --git a/agent/replay_cleanup.py b/agent/replay_cleanup.py index 84815d756fa..19ed38e059b 100644 --- a/agent/replay_cleanup.py +++ b/agent/replay_cleanup.py @@ -20,6 +20,9 @@ from __future__ import annotations 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__) @@ -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), ) @@ -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] diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index 91ad75cd121..1b6cae98ac6 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -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). @@ -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 diff --git a/agent/tool_executor.py b/agent/tool_executor.py index e39263fe758..ac505c6d829 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -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, @@ -827,9 +828,11 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe # 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, @@ -876,6 +879,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe 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( @@ -964,7 +969,12 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe # 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 ( @@ -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, @@ -1691,6 +1702,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe 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, diff --git a/agent/tool_result_classification.py b/agent/tool_result_classification.py index e136e2964da..c71fc6c31db 100644 --- a/agent/tool_result_classification.py +++ b/agent/tool_result_classification.py @@ -9,6 +9,20 @@ from typing import Any 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): diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index c768968a0d6..6b8dea04e30 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -171,6 +171,7 @@ class ChatCompletionsTransport(ProviderTransport): "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 @@ -212,12 +213,14 @@ class ChatCompletionsTransport(ProviderTransport): "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 diff --git a/hermes_state.py b/hermes_state.py index a0b6dfd09cc..f1ef14cb5fc 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -753,6 +753,7 @@ CREATE TABLE IF NOT EXISTS messages ( tool_call_id TEXT, tool_calls TEXT, tool_name TEXT, + effect_disposition TEXT, timestamp REAL NOT NULL, token_count INTEGER, finish_reason TEXT, @@ -3455,6 +3456,7 @@ class SessionDB: codex_message_items: Any = None, platform_message_id: str = None, observed: bool = False, + effect_disposition: Optional[str] = None, timestamp: Any = None, ) -> int: """ @@ -3505,10 +3507,10 @@ class SessionDB: 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, @@ -3516,6 +3518,7 @@ class SessionDB: tool_call_id, tool_calls_json, tool_name, + effect_disposition, message_timestamp, token_count, finish_reason, @@ -3597,10 +3600,10 @@ class SessionDB: 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, @@ -3608,6 +3611,7 @@ class SessionDB: 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"), @@ -4103,7 +4107,7 @@ class SessionDB: 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})" @@ -4131,6 +4135,8 @@ class SessionDB: 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"]) diff --git a/tests/agent/test_replay_cleanup.py b/tests/agent/test_replay_cleanup.py index 590bb269909..14b44e8f2b2 100644 --- a/tests/agent/test_replay_cleanup.py +++ b/tests/agent/test_replay_cleanup.py @@ -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"}] @@ -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(): diff --git a/tests/agent/test_tool_dispatch_helpers.py b/tests/agent/test_tool_dispatch_helpers.py index fb70cd6d845..ab93970d710 100644 --- a/tests/agent/test_tool_dispatch_helpers.py +++ b/tests/agent/test_tool_dispatch_helpers.py @@ -222,6 +222,12 @@ class TestMakeToolResultMessage: "tool_call_id": "call_1", } + def test_effect_disposition_is_internal_message_metadata(self): + msg = make_tool_result_message( + "terminal", "timed out", "call_effect", effect_disposition="unknown" + ) + assert msg["effect_disposition"] == "unknown" + def test_high_risk_message_content_wrapped(self): msg = make_tool_result_message("web_extract", SAMPLE_LONG_TEXT, "call_2") assert msg["role"] == "tool" diff --git a/tests/agent/test_tool_result_classification.py b/tests/agent/test_tool_result_classification.py index 2b4b5b150cf..257f679e815 100644 --- a/tests/agent/test_tool_result_classification.py +++ b/tests/agent/test_tool_result_classification.py @@ -2,7 +2,9 @@ import json -from agent.tool_result_classification import file_mutation_result_landed +from agent.tool_result_classification import ( + file_mutation_result_landed, +) def test_write_file_with_nested_lint_error_counts_as_landed(): @@ -28,3 +30,14 @@ def test_top_level_file_mutation_error_does_not_count_as_landed(): result = json.dumps({"success": True, "error": "post-write verification failed"}) assert file_mutation_result_landed("patch", result) is False + + +def test_side_effect_classification_keeps_session_mutations(): + from agent.tool_result_classification import tool_may_have_side_effect + + assert tool_may_have_side_effect("todo") is True + assert tool_may_have_side_effect("memory") is True + assert tool_may_have_side_effect("write_file") is True + assert tool_may_have_side_effect("mcp_unknown") is True + assert tool_may_have_side_effect("read_file") is False + assert tool_may_have_side_effect("web_search") is False diff --git a/tests/agent/transports/test_chat_completions.py b/tests/agent/transports/test_chat_completions.py index 688fa4d3600..f1eece83dad 100644 --- a/tests/agent/transports/test_chat_completions.py +++ b/tests/agent/transports/test_chat_completions.py @@ -30,6 +30,19 @@ class TestChatCompletionsBasic: result = transport.convert_messages(msgs) assert result is msgs # no copy needed + def test_convert_messages_strips_internal_effect_disposition(self, transport): + msgs = [{ + "role": "tool", + "content": "uncertain", + "tool_call_id": "c1", + "effect_disposition": "unknown", + }] + + result = transport.convert_messages(msgs) + + assert "effect_disposition" not in result[0] + assert msgs[0]["effect_disposition"] == "unknown" + def test_convert_messages_strips_codex_fields(self, transport): msgs = [ {"role": "assistant", "content": "ok", "codex_reasoning_items": [{"id": "rs_1"}], diff --git a/tests/gateway/test_auto_continue.py b/tests/gateway/test_auto_continue.py index c1917a971a9..74c4d984b3c 100644 --- a/tests/gateway/test_auto_continue.py +++ b/tests/gateway/test_auto_continue.py @@ -96,7 +96,7 @@ class TestAutoDetection: class TestInterruptedReplayFiltering: - def test_interrupted_tool_tail_is_removed_from_agent_history(self): + def test_interrupted_side_effect_is_replayed_as_unknown(self): from gateway.run import _build_gateway_agent_history history = [ @@ -118,9 +118,12 @@ class TestInterruptedReplayFiltering: agent_history, observed_context = _build_gateway_agent_history(history) assert observed_context is None - assert agent_history == [{"role": "user", "content": "transcribe this video"}] + assert agent_history[:2] == history[:2] + assert agent_history[-1]["role"] == "tool" + assert agent_history[-1]["tool_call_id"] == "call_1" + assert agent_history[-1]["effect_disposition"] == "unknown" - def test_mixed_tail_with_one_interrupted_result_is_removed(self): + def test_mixed_tail_preserves_results_and_marks_interrupted_effect_unknown(self): from gateway.run import _build_gateway_agent_history history = [ @@ -143,7 +146,10 @@ class TestInterruptedReplayFiltering: agent_history, _observed_context = _build_gateway_agent_history(history) - assert agent_history == [{"role": "user", "content": "search and transcribe"}] + assert agent_history[:3] == history[:3] + assert agent_history[-1]["role"] == "tool" + assert agent_history[-1]["tool_call_id"] == "call_2" + assert agent_history[-1]["effect_disposition"] == "unknown" def test_successful_tool_tail_is_preserved(self): from gateway.run import _build_gateway_agent_history @@ -165,14 +171,14 @@ class TestInterruptedReplayFiltering: assert agent_history[-1]["role"] == "tool" assert agent_history[-1]["content"] == "deployed successfully" - def test_dangling_unanswered_tool_call_tail_is_removed(self): - """A trailing assistant(tool_calls) with NO tool answers is stripped. + def test_dangling_unanswered_side_effect_is_replayed_as_unknown(self): + """A trailing side-effecting call gets an UNKNOWN result, not a retry. This is the SIGKILL signature from #49201: the tool itself ran a restart/shutdown command and killed the gateway before its result was persisted. The transcript tail is an assistant message with tool_calls - and zero matching tool rows. Without stripping it, the model re-issues - the unanswered call on resume and loops the restart forever. + and zero matching tool rows. A synthetic UNKNOWN result closes the tool + pair without claiming the restart did not happen or inviting a retry. """ from gateway.run import _build_gateway_agent_history @@ -195,13 +201,16 @@ class TestInterruptedReplayFiltering: agent_history, _observed_context = _build_gateway_agent_history(history) - assert agent_history == [{"role": "user", "content": "restart the container"}] + assert agent_history[:2] == history + assert agent_history[-1]["role"] == "tool" + assert agent_history[-1]["tool_call_id"] == "call_1" + assert agent_history[-1]["effect_disposition"] == "unknown" - def test_dangling_tail_after_completed_pair_is_removed_only_at_tail(self): - """Only the trailing unanswered tool-call block is stripped. + def test_dangling_tail_after_completed_pair_gets_unknown_result(self): + """The completed pair survives and the trailing call becomes UNKNOWN. - An earlier completed assistant→tool pair must survive — we only drop - the final assistant(tool_calls) that has no answers. + An earlier completed assistant→tool pair must survive, and the final + assistant(tool_calls) receives a matching UNKNOWN result. """ from gateway.run import _build_gateway_agent_history @@ -232,18 +241,19 @@ class TestInterruptedReplayFiltering: agent_history, _observed_context = _build_gateway_agent_history(history) - # The completed call_1 pair survives; the dangling call_2 tail is gone. + # The completed call_1 pair survives; call_2 is closed truthfully. assert agent_history[-1]["role"] == "tool" - assert agent_history[-1]["content"] == "found it" - # The surviving assistant(tool_calls) is the completed call_1 (which - # has a matching tool answer), not the stripped dangling call_2. + assert agent_history[-1]["tool_call_id"] == "call_2" + assert agent_history[-1]["effect_disposition"] == "unknown" + assert agent_history[2]["content"] == "found it" + # Both assistant calls survive with matching tool results. _surviving_calls = [ tc.get("id") for m in agent_history if m.get("role") == "assistant" and m.get("tool_calls") for tc in m["tool_calls"] ] - assert _surviving_calls == ["call_1"] + assert _surviving_calls == ["call_1", "call_2"] def test_persisted_auto_continue_note_is_not_replayed(self): from gateway.run import _build_gateway_agent_history diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index dfe77d007c1..396657ea4e8 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2775,6 +2775,7 @@ class TestConcurrentToolExecution: assert "fast-result" in messages[0]["content"] assert messages[1]["tool_call_id"] == "c2" assert "timed out after" in messages[1]["content"] + assert messages[1]["effect_disposition"] == "unknown" assert [batch[-1]["tool_call_id"] for batch in flushed] == ["c1", "c2"] assert "fast-result" in flushed[0][-1]["content"] assert "timed out after" in flushed[1][-1]["content"] diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 99f2ccb71e9..b2697c2ab6e 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -934,6 +934,19 @@ class TestMessageStorage: tool_msg = next(m for m in msgs if m["role"] == "tool") assert tool_msg["tool_name"] == "web_search" + def test_tool_effect_disposition_round_trips_through_session_db(self, db): + from agent.tool_dispatch_helpers import make_tool_result_message + + db.create_session(session_id="s1", source="cli") + db.replace_messages( + "s1", + [make_tool_result_message( + "write_file", "worker detached", "c1", effect_disposition="unknown" + )], + ) + + assert db.get_messages_as_conversation("s1")[0]["effect_disposition"] == "unknown" + def test_replace_messages_handles_multimodal_content(self, db): """`replace_messages` (used by /retry, /undo, /compress) must also handle list content without crashing."""