fix(agent): guard finalize_turn cleanup chain so it never drops the response - #50059
Closed
Megashubham wants to merge 1 commit into
Closed
fix(agent): guard finalize_turn cleanup chain so it never drops the response#50059Megashubham wants to merge 1 commit into
Megashubham wants to merge 1 commit into
Conversation
…esponse When a turn hit max_iterations, finalize_turn ran three unguarded cleanup steps after the model's summary — _save_trajectory (file I/O), _cleanup_task_resources (remote VM/browser teardown), and _persist_session (SQLite write). Any raise there propagated out of run_conversation, discarding the partial final_response the caller was waiting for; subprocess wrappers saw an empty stdout with no traceback (#8049). Each step is now guarded independently so one failure can't skip the others. Failures log at ERROR with a traceback and are surfaced on the result dict via cleanup_errors; the partial response is always returned. Clean turns never get a cleanup_errors key. Changes: - agent/turn_finalizer.py: wrap each of the three core cleanup steps in its own try/except, collect errors in _cleanup_errors list, surface on result - tests/agent/test_turn_finalizer_cleanup_guard.py: regression coverage — all-three-raise, each-one-raises-in-isolation (others still run), and clean-turn-no-key (5 tests, all pass) Closes #8049
Collaborator
|
Duplicate of #50009 — same fix for #8049, filed ~1.5h earlier. Both wrap the three post-loop cleanup steps ( |
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.
What does this PR do?
When a turn hits
max_iterations,finalize_turnran three unguarded cleanup steps after the model's summary:_save_trajectory(file I/O)_cleanup_task_resources(remote VM/browser teardown)_persist_session(SQLite write)Any exception from these propagated out of
run_conversation, silently discardingfinal_response. Subprocess wrappers saw empty stdout with no traceback.Each step is now wrapped in its own
try/exceptso one failure cannot skip the others. Failures log atERRORwith a traceback and are surfaced on the result dict viacleanup_errors; the partial response is always returned. Clean turns never get acleanup_errorskey.Related Issue
Fixes #8049
Type of Change
Changes Made
agent/turn_finalizer.py: wrap each of the three core cleanup steps in its owntry/except, collect errors in_cleanup_errorslist, surface viacleanup_errorson result dicttests/agent/test_turn_finalizer_cleanup_guard.py: regression coverage — all-three-raise, each-one-raises-in-isolation (others still run), and clean-turn-no-keyHow to Test
pytest tests/agent/test_turn_finalizer_cleanup_guard.py -v— 5 tests pass_save_trajectoryraises; before this fixrun_conversationreturns no response; after this fix the response is preserved andcleanup_errorsshows the error.Checklist
Code
pytest tests/agent/test_turn_finalizer_cleanup_guard.py -vand all 5 tests passDocumentation & Housekeeping