Skip to content

fix(sdk): tolerate orphaned tool results in view indices - #4594

Closed
chrischen-coder wants to merge 1 commit into
OpenHands:mainfrom
chrischen-coder:fix/orphaned-tool-result-manipulation
Closed

fix(sdk): tolerate orphaned tool results in view indices#4594
chrischen-coder wants to merge 1 commit into
OpenHands:mainfrom
chrischen-coder:fix/orphaned-tool-result-manipulation

Conversation

@chrischen-coder

Copy link
Copy Markdown

HUMAN:


AGENT:

Why

After an agent-server restart, crash recovery can persist an AgentErrorEvent for an interrupted tool call whose matching ActionEvent is later absent from the active view. ToolCallMatchingProperty.enforce() can already repair this orphaned result, but manipulation_indices() called set.remove() first and raised KeyError.

Because every later turn reloads the same persisted history, the exception was non-retryable and permanently bricked the conversation before the LLM could be called. The failure was observed in a real 408-event conversation and is reproducible without credentials or network access.

The invariant is that boundary analysis must remain total over states the enforcement layer can repair. This follows the same durable-recovery principle used by DeepSeek Harness session repair: persisted tool-call anomalies are made replay-safe at recovery boundaries instead of making recovery itself fail.

Summary

  • tolerate orphaned or duplicate observations while calculating manipulation indices
  • preserve cleanup ownership in ToolCallMatchingProperty.enforce() and ObservationUniquenessProperty
  • cover restart-recovery orphans, duplicate results, top-level View.manipulation_indices, and post-enforcement repair

Issue Number

Fixes #4591

How to Test

Directly exercised the production event and view path without a model or mocks:

uv run python - <<PY
from openhands.sdk.context.view.properties.tool_call_matching import ToolCallMatchingProperty
from openhands.sdk.context.view.view import View
from openhands.sdk.event import AgentErrorEvent, MessageEvent
from openhands.sdk.llm import Message, TextContent

orphan = AgentErrorEvent(
    source="agent",
    error="tool execution was interrupted by restart",
    tool_name="task",
    tool_call_id="restart-interrupted-call",
)
message = MessageEvent(
    source="user",
    llm_message=Message(
        role="user",
        content=[TextContent(text="continue")],
    ),
)
events = [orphan, message]

print(ToolCallMatchingProperty().manipulation_indices(events))
print(View(events=list(events)).manipulation_indices)
view = View(events=list(events))
view.enforce_properties(events)
print([type(event).__name__ for event in view.events])
PY

Observed:

ManipulationIndices({0, 1, 2})
ManipulationIndices({0, 1, 2})
[MessageEvent]

Automated validation:

  • uv run pytest -q tests/sdk/context -> 435 passed
  • uv run pre-commit run --files openhands-sdk/openhands/sdk/context/view/properties/tool_call_matching.py tests/sdk/context/view/properties/test_tool_call_matching.py -> all hooks passed, including Ruff, Pyright, import rules, and tool registration
  • uv run pytest -q tests/sdk -> 5961 passed, 9 skipped, 10 xfailed; the only failure was an unrelated localhost MCP negative test because the host proxy converted connection refusal into HTTP 502
  • NO_PROXY=127.0.0.1,localhost uv run pytest -q tests/sdk/mcp/test_create_mcp_tool.py::test_create_mcp_tools_connection_to_nonexistent_server -> 1 passed

Video/Screenshots

Not applicable. This is a non-visual SDK recovery-path bug; the direct terminal reproduction and outputs are included above.

Design Doc

Not applicable. The production change is one line and does not alter a public API or serialized event shape.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • Well-formed action/result histories keep identical manipulation boundaries.
  • No provider-specific branch, prompt, tool scheduling, or public schema changes were introduced.
  • No live-model eval was run because the defect occurs before any LLM request; the full SDK suite covers unchanged valid-history behavior.

Co-authored-by: openhands <openhands@all-hands.dev>
@chrischen-coder

Copy link
Copy Markdown
Author

Closing in favor of #4592, which was opened first and covers the same restart-recovery failure. I compared both implementations and am consolidating review effort there rather than maintaining a competing duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ToolCallMatchingProperty.manipulation_indices crashes on orphaned observation, permanently bricking conversations after a restart

1 participant