fix(agent-tool): preserve nested run cache across tool-call GC#2491
fix(agent-tool): preserve nested run cache across tool-call GC#2491OiPunk wants to merge 2 commits intoopenai:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17ed2afba0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks for flagging this edge case. I pushed d0a3619 to clear any stale signature index for the object id before storing a new run result, which prevents old signature mappings from surviving id reuse. I also expanded with targeted cases for id-reuse overwrite, ambiguous fallback candidates, direct consume/peek/drop paths, and fallback-drop cleanup. |
|
Thanks for flagging this edge case. I pushed commit d0a3619 to clear any stale signature index for the current object id before recording a new run result. This prevents old signature mappings from surviving object-id reuse and accidentally influencing later fallback lookups. I also expanded tests/test_agent_tool_state.py with focused coverage for:
Local checks run on this update:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0a361979b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _agent_tool_run_results_by_obj[tool_call_obj_id] = run_result | ||
| _index_agent_tool_run_result(tool_call, tool_call_obj_id) |
There was a problem hiding this comment.
record_agent_tool_run_result now keeps interrupted nested RunResult objects in module-level dicts with no GC-triggered eviction. If a paused run is abandoned before consume_agent_tool_run_result/drop_agent_tool_run_result is called, the entry is never removed and retains the full nested run state. In long-lived workers this can cause unbounded memory growth.
Useful? React with 👍 / 👎.
|
Closing this in favor of #2500 |
2 participants
Summary
Fixes #2487.
agent.as_tool()resume logic currently depends on an in-memory cache keyed by tool-call identity. That cache was tied to the originaltool_callobject lifetime via weakref callbacks. After serializing state and dropping the originalRunResult, GC could clear the cache entry before resume, causing nested runs to restart and repeatedly pause on the same approval.This PR decouples cache retention from
tool_callGC and keeps entries until explicit consume/drop.Changes
src/agents/agent_tool_state.pytests/test_agent_tool_state.pyValidation
uv run --with ruff ruff check src/agents/agent_tool_state.py tests/test_agent_tool_state.pyuv run mypy src/agents/agent_tool_state.py tests/test_agent_tool_state.pyenv -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy -u NO_PROXY -u no_proxy uv run --with pytest pytest -q tests/test_agent_tool_state.py tests/test_agent_as_tool.py -k "rejected_nested_approval_resumes_run or agent_tool_result_survives_tool_call_gc_until_consumed"(2 passed)