From 9fe18a127c3051fe21b0dfd219ea89bbda2ccd89 Mon Sep 17 00:00:00 2001 From: chrischen-coder Date: Mon, 24 Aug 2026 01:04:12 +0800 Subject: [PATCH] fix(sdk): tolerate orphaned tool results in view indices Co-authored-by: openhands --- .../view/properties/tool_call_matching.py | 11 ++--- .../properties/test_tool_call_matching.py | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/openhands-sdk/openhands/sdk/context/view/properties/tool_call_matching.py b/openhands-sdk/openhands/sdk/context/view/properties/tool_call_matching.py index 5d1daf2da0..7251f0c276 100644 --- a/openhands-sdk/openhands/sdk/context/view/properties/tool_call_matching.py +++ b/openhands-sdk/openhands/sdk/context/view/properties/tool_call_matching.py @@ -84,13 +84,10 @@ def manipulation_indices( case ActionEvent(): pending_tool_call_ids.add(event.tool_call_id) case ObservationBaseEvent(): - # Intentionally use remove(), not discard(): a second - # observation-like event for the same tool_call_id means the - # view has already violated the 1 action -> 1 result - # invariant that downstream LLM APIs expect. That case must - # be fixed by de-duplicating the view before serialization, - # not by silently tolerating it here. - pending_tool_call_ids.remove(event.tool_call_id) + # This method can run before enforce() repairs a persisted view. + # Tolerate invalid observations here; enforce() and the + # uniqueness property own removing them from the view. + pending_tool_call_ids.discard(event.tool_call_id) if pending_tool_call_ids: # The enumeration index corresponds to the position of the event, but we diff --git a/tests/sdk/context/view/properties/test_tool_call_matching.py b/tests/sdk/context/view/properties/test_tool_call_matching.py index 831a55f2e4..a53ce84143 100644 --- a/tests/sdk/context/view/properties/test_tool_call_matching.py +++ b/tests/sdk/context/view/properties/test_tool_call_matching.py @@ -10,6 +10,7 @@ from openhands.sdk.context.view.properties.tool_call_matching import ( ToolCallMatchingProperty, ) +from openhands.sdk.context.view.view import View from openhands.sdk.event.base import LLMConvertibleEvent from openhands.sdk.event.llm_convertible import ( ActionEvent, @@ -429,3 +430,47 @@ def test_empty_events(self) -> None: result = self.property.manipulation_indices(events) assert result == ManipulationIndices.complete(events) + + def test_orphaned_agent_error_does_not_crash_before_enforcement(self) -> None: + """A restart-recovery result can outlive its matching action in a view.""" + orphaned_error = AgentErrorEvent( + error="Tool execution was interrupted by a restart.", + tool_name="task", + tool_call_id="call_interrupted", + ) + user_message = message_event("Continue") + events: list[LLMConvertibleEvent] = [orphaned_error, user_message] + + assert self.property.manipulation_indices( + events + ) == ManipulationIndices.complete(events) + assert View(events=list(events)).manipulation_indices == ( + ManipulationIndices.complete(events) + ) + + repaired_view = View(events=list(events)) + repaired_view.enforce_properties(events) + assert repaired_view.events == [user_message] + + def test_duplicate_observation_does_not_crash_before_enforcement(self) -> None: + """ObservationUniquenessProperty owns duplicate-result cleanup.""" + action = create_action_event_with_none_action( + "action_1", "response_1", "call_1" + ) + first_error = AgentErrorEvent( + error="Tool execution was interrupted by a restart.", + tool_name="task", + tool_call_id="call_1", + ) + late_error = AgentErrorEvent( + error="Late result for the same tool call.", + tool_name="task", + tool_call_id="call_1", + ) + events: list[LLMConvertibleEvent] = [action, first_error, late_error] + + result = self.property.manipulation_indices(events) + + assert 1 not in result + assert 2 in result + assert 3 in result