diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 949e21116..f50ac8268 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -1593,9 +1593,7 @@ class AgentLoop: if role == "tool": tool_call_id = entry.get("tool_call_id") if not tool_call_id or str(tool_call_id) not in declared_tool_call_ids: - # A tool result without a declared call violates the - # OpenAI/Anthropic pairing contract and would poison - # every future request built from this session (#4006). + # Undeclared tool results corrupt future provider requests. logger.warning( "Dropping orphaned tool result {} from session {} during persistence", tool_call_id or "(missing id)", @@ -1607,8 +1605,7 @@ class AgentLoop: elif isinstance(content, list): filtered = self._sanitize_persisted_blocks(content, should_truncate_text=True) if not filtered: - # Dropping the message would leave its assistant - # tool_call without a result; keep a placeholder. + # Preserve the tool_call/result pair after block filtering. filtered = [ {"type": "text", "text": "[tool result omitted during persistence]"} ] diff --git a/nanobot/session/turn_continuation.py b/nanobot/session/turn_continuation.py index 6ebfaaa7e..6b2a5bacf 100644 --- a/nanobot/session/turn_continuation.py +++ b/nanobot/session/turn_continuation.py @@ -182,11 +182,8 @@ def _save_skip_for_turn( """Return the persisted-message append boundary for this turn.""" if internal_continuation_inbound(message_metadata): return initial_message_count - # ``build_messages`` merges the current message into the last history - # entry when both share a role, so the prompt prefix is not always - # ``2 + history_count``. Runner-appended messages always start at - # ``initial_message_count``; only step back when the current message - # exists as a standalone entry that was not persisted early. + # build_messages may merge the current message into a same-role history tail. + # Runner-appended messages start at initial_message_count in either shape. has_standalone_current = initial_message_count > 1 + history_count if has_standalone_current and not user_persisted_early: return initial_message_count - 1 diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index fc0f49f5d..2df5f0910 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -1293,11 +1293,6 @@ async def test_system_subagent_followup_uses_thread_session_and_slack_metadata(t @pytest.mark.asyncio async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path) -> None: - """Regression for #4006: when history ends with a user message, - ``build_messages`` merges the new user message into it, shrinking the - prompt prefix by one. The save boundary must follow, or the first - new-turn assistant message (carrying tool_calls) is cut from - persistence and its tool results are orphaned.""" loop = _make_full_loop(tmp_path) loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign] @@ -1306,8 +1301,6 @@ async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path loop.sessions.save(session) async def fake_run_agent_loop(initial_messages, **_kwargs): - # The merge branch collapsed the current user message into the - # history tail: the prompt prefix is [system, merged-user]. assert [m["role"] for m in initial_messages] == ["system", "user"] return ( "done", @@ -1359,8 +1352,6 @@ async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path def test_save_turn_keeps_placeholder_for_empty_tool_result_blocks() -> None: - # Dropping the whole tool message would leave the assistant tool_call - # without a result, which strict APIs reject as firmly as orphans. loop = _mk_loop() session = Session(key="test:empty-tool-blocks") @@ -1388,8 +1379,6 @@ def test_save_turn_keeps_placeholder_for_empty_tool_result_blocks() -> None: def test_save_turn_drops_orphaned_tool_results() -> None: - # Defense in depth for #4006: whatever upstream bug produces a tool - # result whose call was never declared, it must not reach history. loop = _mk_loop() session = Session(key="test:orphan-guard") session.add_message("user", "hi") @@ -1424,8 +1413,6 @@ def test_save_turn_drops_tool_results_without_tool_call_id() -> None: def test_save_turn_keeps_tool_results_declared_in_prior_history() -> None: - # Declarations may live in already-persisted history (e.g. a restored - # runtime checkpoint), not only in the new-turn slice. loop = _mk_loop() session = Session(key="test:prior-declared") session.add_message( diff --git a/tests/session/test_turn_continuation.py b/tests/session/test_turn_continuation.py index 963ebc412..b27ebd932 100644 --- a/tests/session/test_turn_continuation.py +++ b/tests/session/test_turn_continuation.py @@ -142,11 +142,6 @@ def test_internal_continuation_requires_budget_boundary_and_queue(): def test_save_skip_matches_prefix_when_current_message_merged(): - # ``build_messages`` merges the current user message into a same-role - # history tail, so the prompt prefix is ``1 + history_count`` instead of - # ``2 + history_count``. The old boundary then cut the first new-turn - # assistant message (carrying tool_calls) from persistence, leaving its - # tool results orphaned in session history (#4006). skip = _save_skip_for_turn( message_metadata=None, initial_message_count=2, # [system, merged user] @@ -157,14 +152,13 @@ def test_save_skip_matches_prefix_when_current_message_merged(): def test_save_skip_unchanged_for_standalone_current_message(): - # [system, history user, current user] — early-persisted current message. + # [system, history user, current user] with the current user already saved. assert _save_skip_for_turn( message_metadata=None, initial_message_count=3, history_count=1, user_persisted_early=True, ) == 3 - # Same prefix, current message not persisted early: re-save it. assert _save_skip_for_turn( message_metadata=None, initial_message_count=3,