Skip to content

fix(run): persist the max-turns handler output to the session - #4160

Merged
seratch merged 1 commit into
openai:mainfrom
LeSingh1:fix/max-turns-handler-session-save
Aug 4, 2026
Merged

fix(run): persist the max-turns handler output to the session#4160
seratch merged 1 commit into
openai:mainfrom
LeSingh1:fix/max-turns-handler-session-save

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

When a max_turns run error handler returns a final output with include_in_history=True, the non-streamed runner never wrote the synthesized assistant message to the session.

save_result_to_session uses run_state._current_turn_persisted_item_count as a slice offset into the new_items list it is given. That is only valid when the caller passes the cumulative turn item list. The max-turns block passes a fresh one-item list [synthesized_item], and it runs before the per-turn counter reset, so any previous turn that persisted at least one item makes already_persisted >= len(new_items) and nothing is saved at all. The next run on that session has no record of the assistant's answer.

The streamed path resets the counter before the max-turns check and persists the item correctly, so the two paths silently disagreed on what ends up in history.

Fix: pass run_state=None with an explicit reasoning_item_id_policy at that call site, matching what save_resumed_turn_items already does for the same "this is not a cumulative turn list" situation. Per-turn counter semantics are untouched everywhere else.

Test plan

Added a shared helper in tests/test_max_turns.py that runs one tool turn, trips max_turns=1 with a handler returning include_in_history=True, and asserts the resulting SQLiteSession contents.

  • non-streamed: fails on main (session is missing the final message), passes with the fix
  • streamed twin: passes both before and after, locking in the alignment between the two paths

tests/test_max_turns.py → 22 passed. tests/test_agent_runner.py tests/test_run_state.py tests/memory tests/test_hitl_session_scenario.py → 579 passed. .agents/skills/code-change-verification/scripts/run.sh passed.

Issue number

N/A — found while auditing session persistence parity between the streamed and non-streamed runners.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

@seratch
seratch enabled auto-merge (squash) August 4, 2026 02:27
@seratch seratch added this to the 0.19.x milestone Aug 4, 2026
@seratch
seratch merged commit 945332d into openai:main Aug 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants