Repository navigation
Conversation
A model response can pair a call that needs approval with a call to a tool that is not registered. With `tool_not_found_behavior` set to `"return_error_to_model"`, the runner answers the unknown call locally and then interrupts for the approval, so that error output exists but has never been sent. On resume under `conversation_id`, `previous_response_id`, or `auto_previous_response_id`, the unsent-output lookup enumerated only the pending tool-run groups on the last `ProcessedResponse`. Those groups never include `function_tools_not_found`, so the tracker treated the error output as already delivered and the server was left holding a function call with no output. The same run without server-managed continuation delivered both outputs. Adding `function_tools_not_found` to that enumeration is not sufficient, because the group is not part of the serialized `RunState` and would therefore only fix an in-memory resume. Seed the interruption branch from the tool call IDs of the latest model response instead, which is the rule the `NextStepRunAgain` branch already applies and the response the resume path treats as authoritative. The set only grows, and `hydrate_from_state` records everything that response reported as server-owned before it consults the set, so no server-owned item is resent.
seratch
changed the title
|
@hyeonsang010716 This fix inspired a small engineering experiment on what to check when a resumed run appears to finish normally. We reran the ten added regressions at These were local ScriptedModel experiments, so our observation stops at request construction. The staged-approval case establishes one inclusion per output in that input; it does not tell us what happens if a server accepts a request but its acknowledgement is lost. For a follow-up fault-injection experiment, is there an existing SDK test boundary you would recommend for that ambiguous-acknowledgement case, or is it best exercised separately at the transport/server boundary? Reproduction scripts, passing/failing logs, and limitations. |
Summary
A single model response can pair a call that needs approval with a call to a tool that is not
registered. With
RunConfig(tool_not_found_behavior="return_error_to_model")the runner answers theunknown call locally and then interrupts for the approval, so that error output exists but has never
been sent. Resuming under
conversation_id,previous_response_id, orauto_previous_response_idnever delivers it, and the server keeps a function call that has no output. The same run without
server-managed continuation delivers both outputs, so the two paths disagreed.
get_unsent_tool_call_ids_for_interrupted_stateenumerated only the pending tool-run groups on thelast
ProcessedResponse. Those groups never includefunction_tools_not_found, sohydrate_from_staterecorded the locally built output as already delivered andprepare_inputdropped it from the delta.
Adding
function_tools_not_foundto that enumeration is not sufficient:_serialize_tool_action_groupsdoes not persist the group, so it would fix only an in-memory resume and still lose the output after a
RunStateround trip. The interruption branch is now seeded from the tool call IDs of the latest modelresponse, which is the rule the
NextStepRunAgainbranch already applies and the responseresolve_interrupted_turnalready treats as authoritative.Two properties worth confirming during review. The set only grows, so no output can be dropped, and
nothing server-owned can be resent:
hydrate_from_staterecords every item the response reported intoserver_item_idsandserver_tool_call_idsbefore it consults this set, andprepare_inputchecksthose first.
RunStateserialization is unchanged, so states written by earlier versions resume asbefore.
The existing tool-run enumeration is retained. It is redundant for every state the tests reach, since
ProcessedResponseis classified from the same response, but removing it would narrow releasedbehavior for pending runs without evidence, so it stays out of this fix. Happy to drop it in a
follow-up if you would rather have the single source of truth.
Test plan
tests/test_run_impl_resume_paths.py:test_resumed_server_managed_run_sends_tool_not_found_outputcovers streamed and non-streamed runs, live and JSON round-tripped
RunState, and bothauto_previous_response_idandconversation_id, for eight cases in total. It asserts the deliveredcall IDs as a list so a duplicate delivery fails it, and asserts the payload names the missing tool.
tests/test_run_impl_resume_paths.py:test_resumed_server_managed_run_sends_each_tool_output_oncestages two approvals, so the same model response stays current across both resumes, and asserts each
output reaches the model exactly once. It also asserts the middle resume issues no model request.
tests/test_server_conversation_tracker.py: a tracker-level case where the interruptedProcessedResponsecarries no pending tool run for the missing call, which is the shape a resumedstate actually has. The pre-existing interrupted-turn test keeps pinning the enumeration path; its
stand-in state gained
_model_responses, which every real interruptedRunStatehas.mainand pass with the change.unpatched tree produces on Windows without symlink privilege (Sandbox tar extraction and 12 sandbox tests fail on Windows without symlink privilege #4852). Serial suite: 77 passed.
ruff format --check,ruff check,check_optional_truthiness.py,mypy src(312 files) andpyrightare all clean.tests/test_run_state.pyis in the Windowscollect_ignorelist intests/conftest.py, so it doesnot run on this host. With its Unix sandbox import stubbed, its 445 tests pass locally, and CI covers
it on Linux.
makeis not installed on this host, andmake testswould not pass here anyway while those symlink cases fail. The
Makefiletargets were run directlyin the required order instead: format, lint, typecheck, then the parallel and serial suites.
Issue number
None.
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR