fix(session runner): retry the sub-thread tool_use commit-race 400 instead of stranding (#1744) - #1753
Open
nadavshohat wants to merge 1 commit into
Open
Conversation
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.
Mitigates #1744. Not a root-cause fix — the 400 is generated server-side (the sub-thread
agent.tool_useevent isn't matchable yet when the result is posted), so the real fix has to happen there. This is the client half.The bug
When a tool runs inside a non-primary
session_thread_id(a sub-agent thread), posting itsuser.tool_resultintermittently returns400 … does not match any agent.tool_use event, even though the tool ran and the same tool succeeds on other attempts. TodaySessionToolRunner._send_resultclassifies that 400 as fatal viais_fatal_status_errorand bails on the first attempt, so a transient commit race becomes a permanently stranded tool call — the session then waits forever on that final pending result and never delivers.The change
Recognize that specific 400 and retry it on a more patient budget (
SUBTHREAD_RACE_SEND_RETRIES = 8, backoff capped atSTREAM_BACKOFF_CAP) so the result lands once the event commits, instead of giving up. Everything else is unchanged:_send_resultonly — the sharedis_fatal_status_erroris untouched, so no other retry path (stream loop, poller) changes behavior.lib/_retry.pyif you'd prefer it next to the other classification.Residual
The extended budget covers the common case (the event commits within the retry window). A pathologically long race can still exhaust it, at which point the call is surfaced as before — that final gap needs the server-side fix.
Tests
Three deterministic tests in
tests/lib/tools/test_session_runner.py: the race 400 is retried and posts, it retries pastSEND_RETRIES, and a non-race 400 still bails.ruff+ fulltest_session_runner.pygreen.Open question for a maintainer (same as #1744)
Is a client meant to be able to confirm a sub-thread
agent.tool_use, and is there a readiness signal we should wait on rather than retry blindly? If the intended fix is server-side, this can stand as a stopgap or close in favor of it.