fix(sdk): tolerate orphaned tool results in view indices - #4594
Closed
chrischen-coder wants to merge 1 commit into
Closed
fix(sdk): tolerate orphaned tool results in view indices#4594chrischen-coder wants to merge 1 commit into
chrischen-coder wants to merge 1 commit into
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
AGENT:
Why
After an agent-server restart, crash recovery can persist an
AgentErrorEventfor an interrupted tool call whose matchingActionEventis later absent from the active view.ToolCallMatchingProperty.enforce()can already repair this orphaned result, butmanipulation_indices()calledset.remove()first and raisedKeyError.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
ToolCallMatchingProperty.enforce()andObservationUniquenessPropertyView.manipulation_indices, and post-enforcement repairIssue Number
Fixes #4591
How to Test
Directly exercised the production event and view path without a model or mocks:
Observed:
Automated validation:
uv run pytest -q tests/sdk/context->435 passeduv 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 registrationuv 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 502NO_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 passedVideo/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
Notes