fix: keep committed tool session records when a streamed output guardrail trips

[LHMQ878]

Problem

When run_streamed builds its final output directly from a tool call, the tool has already run by the time the agent output guardrails execute. If a guardrail trips there, the turn's function_call / function_call_output pair is dropped from the session entirely — the committed side effect leaves no record.

This applies to every "the tool is the final output" path: tool_use_behavior="stop_on_first_tool", stop_at_tool_names, and a custom tool_use_behavior callable.

Runner.run does not have this problem, so the two paths disagree:

tool_use_behavior="stop_on_first_tool" + a tripwiring output guardrail

run      -> ['user', 'function_call', 'function_call_output']
streamed -> ['user']                                              <- side effect lost

The consequence is a duplicated side effect: the next run over the same session replays only the user message, the model re-issues the same call, and the tool runs a second time.

Cause

_finalize_streamed_final_output in src/agents/run_internal/run_loop.py saved the turn's items after the output guardrails:

    if persist_before_output_guardrails:
        await save_items(items, response_id, store_setting)

    output_guardrail_results = await _run_output_guardrails_for_stream(...)   # <- raises here
    ...
    if not persist_before_output_guardrails:
        await save_items(items, response_id, store_setting)                   # <- never reached

_run_output_guardrails_for_stream re-raises the tripwire, so the trailing save_items never executes. persist_before_output_guardrails=True is only set on the resumed-approval path (#4059), which is why that case already keeps its tool items.

Fix

Defer the decision, not the items. The full ordered batch is saved after the output guardrails pass, exactly as before; only the tripwire path saves the subset of items that record a side effect which already happened (tool_call_item, tool_call_output_item).

    try:
        output_guardrail_results = await _run_output_guardrails_for_stream(...)
    except Exception:
        if not persist_before_output_guardrails:
            committed_items = _committed_side_effect_items(items)
            if committed_items:
                await save_items(committed_items, response_id, store_setting)
        raise
    ...
    if not persist_before_output_guardrails:
        await save_items(items, response_id, store_setting)

Committed types are allow-listed rather than deny-listed on purpose: a future item type that records a side effect then defaults to being persisted instead of silently inheriting "discard on tripwire".

Two things this deliberately avoids — I hit both in the first pass on this branch and they are worth calling out, since the same split is proposed on #3998:

• Splitting the save unconditionally loses items on the passing path. The first save_items advances the turn's persisted-item count, so save_result_to_session's already_persisted >= len(new_items) check treats the shorter second batch as fully persisted and saves nothing. A mixed final turn silently loses its accepted assistant message.
• Splitting also reorders the turn. If the model emitted a message before the tool call, a two-phase save persists the tool call first, so a later run replays reordered history.

Keeping the success path as a single unsplit save avoids both; only the tripwire path — which by definition discards the deliverable output — writes a subset.

The resumed-approval path (persist_before_output_guardrails=True, #4059) is unchanged.

Tests

Eight new cases in tests/test_agent_runner_streamed.py:

Test	Covers
test_stop_on_first_tool_final_persists_committed_tool_items_on_tripwire[streamed]	the bug; also asserts the follow-up run replays the call instead of re-issuing it
...[non_streamed]	pins the run behaviour this restores parity with
test_streamed_blocked_message_final_output_is_not_persisted	control — a rejected message is still withheld
test_streamed_blocked_final_persists_tool_items_but_not_the_message	mixed turn splits correctly on a tripwire: tool kept, message withheld
test_mixed_final_turn_session_order_and_committed_items (4 cases: run/streamed x pass/trip)	a mixed final turn keeps full ordered history whenever it passes — the two hazards above
test_blocked_tool_final_keeps_reasoning_context_with_the_committed_call (4 cases: run/streamed x pass/trip)	a retained tool call keeps its reasoning item, in order, in the session and in the next model input

The committed-tool test asserts calls == ["ran"] before checking the session, so it cannot pass vacuously by the tool never executing.

A/B with only src/agents/run_internal/run_loop.py reverted:

without the fix: 3 failed, 9 passed
    assert [('user', None)] == [('user', None), ('function_call', 'call-committed'), ...]
    assert ['user'] == ['user', 'function_call', 'function_call_output']
    assert ['user'] == ['user', 'reasoning', 'function_call', 'function_call_output']
with the fix:    12 passed

The nine that pass both ways are the guards: they fail only if the fix over-withholds or reorders, which is exactly what caught the two hazards above.

Verification

ruff check and ruff format --check clean on both files. mypy reports zero errors in either changed file.

Full suite (pytest tests/ --ignore=tests/test_run_state.py, Windows, test_run_state.py is not collectable here):

base (bdc294fc): 69 failed, 5474 passed, 76 skipped
this branch:     69 failed, 5486 passed, 76 skipped

Compared as JUnit XML test-ID sets, the failing sets are identical — no new failures and none newly passing. The 69 are pre-existing Windows failures unrelated to this change; the +12 passed are the new tests. (The raw count is mildly flaky run to run — I saw 68/69/70 across runs of the same tree — which is why the comparison above is on test IDs rather than counts.)

Background

Found while verifying the review claims on #3998, which proposes making run match run_streamed for the stop_on_first_tool case. That parity argument is worth resolving in the other direction for this particular item class: at the merge base, streamed reported ['user'] here, so it isn't a behaviour #3998 invents — it's a pre-existing streamed gap. Fixing it here means #3998's parity comparison is against a streamed path that no longer drops committed side effects. The two changes don't conflict; this one only touches _finalize_streamed_final_output.

Review follow-up (535b0064, 747a1c70)

Two gaps found after the first commit, both reproduced with a probe before being fixed:

1. 535b0064 — a split save dropped the deliverable message on the passing path. The first save_items advances RunState._current_turn_persisted_item_count, and session_persistence treats already_persisted >= len(new_items) as "nothing to do", so a second save with a shorter subset was a silent no-op. It would also have reordered the turn. Fixed by deferring the decision rather than the items: the success path is one unsplit save, and only the tripwire path writes a subset.
2. 747a1c70 — a retained tool call lost its reasoning item. A Responses reasoning model requires the reasoning item preceding a function call to accompany that call in the next request, so the retained pair was not replayable. Reasoning items are now classified as retained context in a set separate from the side-effect types.

The helper's set-membership check is enumerated rather than derived, which means an item type added later is discarded here by default and has to be classified deliberately. That's the intended default: an unclassified side-effect record surfaces as a missing item, which is louder than a rejected message quietly reaching the session.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d08208465

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve deliverable items after partial streamed saves

When a streamed final turn contains both a deliverable item and a committed tool item—for example a Responses output list with a text message plus a function call, where tool_use_behavior="stop_on_first_tool" makes the tool result final—the earlier save_items(committed_items, ...) advances RunState._current_turn_persisted_item_count. This second call then passes only deliverable_items, so the count-based session saver treats them as already persisted and writes nothing, causing the assistant message to disappear from streamed session history while Runner.run keeps the full turn. Save with the original item list/counting semantics or an offset-aware helper so passing guardrails does not drop the message.

AGENTS.md reference: AGENTS.md:L124-L124

Useful? React with 👍 / 👎.

[LHMQ878]

Confirmed and fixed in 535b0064. I reproduced it on my own branch before fixing — STREAM trip=False came back ['user', 'function_call', 'function_call_output'], dropping assistant-preamble, exactly as described: the first save_items advanced _current_turn_persisted_item_count, so already_persisted >= len(new_items) made the second call with the shorter deliverable subset a silent no-op.

The fix defers the decision rather than the items: the success path stays one unsplit save_items(items, ...) after the guardrails return, and only the tripwire path (which is discarding the output anyway) writes a subset. That avoids the count arithmetic and the reordering hazard in one move — a split save would also have persisted the tool call ahead of the message that preceded it in the model response.

Regression: test_mixed_final_turn_session_order_and_committed_items, 4 cases over mode × tripwire, with the model output deliberately ordering the message before the tool call so a reordering split fails. At 8d082084 the streamed/passes case fails; it passes now.

[chatgpt-codex-connector]

Keep reasoning context with committed tool records

When a reasoning model returns a reasoning_item together with the function call that becomes the final output, an output guardrail trip now persists only the call/output pair because this allow-list excludes reasoning items. The next run over the same session replays an orphaned function call/output without the reasoning context that the streamed and non-streamed paths normally keep for follow-up model input, so include non-deliverable replay context such as reasoning items with the pre-guardrail committed subset while still withholding blocked assistant messages.

AGENTS.md reference: AGENTS.md:L125-L125

Useful? React with 👍 / 👎.

[LHMQ878]

Confirmed and fixed in 747a1c70. Probe with a ResponseReasoningItem ahead of the function call, tool_use_behavior="stop_on_first_tool":

before                                     after
RUN    trip=False -> user, reasoning, function_call, function_call_output   (unchanged)
RUN    trip=True  -> user, reasoning, function_call, function_call_output   (unchanged)
STREAM trip=False -> user, reasoning, function_call, function_call_output   (unchanged)
STREAM trip=True  -> user,            function_call, function_call_output   -> user, reasoning, function_call, function_call_output

Reasoning items are now classified as retained context in a separate set from the side-effect types, so the distinction between "this already happened" and "this is needed to replay what already happened" stays legible:

_SIDE_EFFECT_ITEM_TYPES = frozenset({"tool_call_item", "tool_call_output_item"})
_SIDE_EFFECT_CONTEXT_ITEM_TYPES = frozenset({"reasoning_item"})

Retention still requires a side effect to be present, so a reasoning item on a turn with no tool call is dropped with the rejected message. Regression test_blocked_tool_final_keeps_reasoning_context_with_the_committed_call asserts both the session order and that the follow-up run's model input replays reasoning → function_call → function_call_output; at 535b0064 only the streamed/trips case fails.

[seratch]

When a Responses reasoning model emits a reasoning item with the function call, the tripwire path currently saves only the call/output pair. Please preserve the reasoning item group associated with each retained tool call, while continuing to withhold rejected message output, and add a regression that verifies the session and next model input retain the ordered reasoning/function-call/function-call-output sequence. Please also update the helper comment, since an allow-list currently causes unknown future item types to be discarded rather than persisted.

[LHMQ878]

Thanks — both points were right, and the first one was a real gap rather than a style note. 747a1c70.

Reasoning items. Confirmed with a probe: a ResponseReasoningItem emitted before the function call, tool_use_behavior="stop_on_first_tool", session contents after the run:

	before	after
run, guardrail passes	user, reasoning, function_call, function_call_output	same
run, guardrail trips	user, reasoning, function_call, function_call_output	same
run_streamed, passes	user, reasoning, function_call, function_call_output	same
run_streamed, trips	user, function_call, function_call_output	user, reasoning, function_call, function_call_output

So the retained turn was not replayable: the next request carried a function_call with no reasoning item ahead of it. Fixed by classifying reasoning items as retained context rather than as a side effect — a second frozenset, so the distinction stays visible:

_SIDE_EFFECT_ITEM_TYPES = frozenset({"tool_call_item", "tool_call_output_item"})
_SIDE_EFFECT_CONTEXT_ITEM_TYPES = frozenset({"reasoning_item"})

Retention is still gated on a side effect actually being present, so a reasoning item on a turn with no tool call is dropped along with the rejected message. The single-pass filter keeps the model's own ordering.

Regression. test_blocked_tool_final_keeps_reasoning_context_with_the_committed_call, parametrized over mode × tripwire (4 cases). It asserts both halves you asked for — the session order, and that a follow-up run's model input replays reasoning → function_call → function_call_output in that order. A/B: at 535b0064 exactly the streamed/trips case fails (['user', 'function_call', 'function_call_output']); at main three of the four fail.

Comment. You're right and I had it backwards — enumerated sets discard an unclassified future item type. Rewritten to say so, and to give the actual reason that's the safer default here: an unclassified side-effect record surfaces as a missing item rather than as a rejected message quietly reaching the session.

Full suite A/B vs main (bdc294fc): 69 failed / 5486 passed / 76 skipped, JUnit failure-ID sets identical — nothing added, nothing masked. (The raw count is flaky on Windows, 68–70 on one tree, hence the ID-set comparison.) ruff check, ruff format --check, mypy clean on both changed files.

[seratch]

The simple reasoning -> function_call case is now fixed, but the retention helper currently keeps every reasoning item whenever the turn contains any tool item.

That over-retains reasoning associated with a rejected message. For example, with reasoning_for_message -> message -> reasoning_for_call -> function_call, the tripwire path drops the message but keeps both reasoning items. This conflicts with the existing rule that a reasoning item is tied to the next non-reasoning model-emitted item and can leave a dangling reasoning item in the next request.

Please derive the retained reasoning groups using that existing association rule, keeping only groups whose following item is a retained tool call. Please also add a mixed regression asserting that reasoning associated with the dropped message is removed while the tool reasoning, call, and output remain in order.

[LHMQ878]

You're right, and thanks for the precise example — reasoning_for_message -> message -> reasoning_for_call -> function_call is exactly where my previous version over-retained. Fixed in ef6cf2a3.

I derived the groups from the rule already in the codebase rather than writing a new one: _drop_reasoning_items_preceding_dropped_calls in run_internal/items.py treats a reasoning item as tied to the next non-reasoning model-emitted item, so the helper now walks backwards and keeps a group only when the item it is tied to is a retained tool call.

def _reasoning_indexes_tied_to_retained_items(items, retained_indexes):
    tied: set[int] = set()
    for index in range(len(items) - 1, -1, -1):
        if items[index].type != "reasoning_item":
            continue
        for next_index in range(index + 1, len(items)):
            if items[next_index].type == "reasoning_item":
                continue
            if next_index in retained_indexes:
                tied.add(index)
            break
    return tied

Verified against the helper directly before touching the runner, so each shape is checked independently of the streaming machinery:

seratch mixed                      in : [reasoning(rs_msg), message, reasoning(rs_call), function_call, function_call_output]
                                   out: [reasoning(rs_call), function_call, function_call_output]
stacked reasoning before message   in : [reasoning(rs_1), reasoning(rs_2), message, reasoning(rs_call), function_call, function_call_output]
                                   out: [reasoning(rs_call), function_call, function_call_output]
two calls, one reasoning each      in : [reasoning(rs_a), function_call, reasoning(rs_b), function_call, fco, fco]
                                   out: [reasoning(rs_a), function_call, reasoning(rs_b), function_call, fco, fco]
stacked reasoning before call      in : [reasoning(rs_1), reasoning(rs_2), function_call, function_call_output]
                                   out: [reasoning(rs_1), reasoning(rs_2), function_call, function_call_output]
trailing reasoning                 in : [reasoning(rs_call), function_call, function_call_output, reasoning(rs_tail)]
                                   out: [reasoning(rs_call), function_call, function_call_output]
message only (no tool)             in : [reasoning(rs_msg), message]
                                   out: []

One deliberate difference from the reference worth your review: a trailing reasoning group — one with no following non-reasoning item at all — is dropped here, whereas _drop_reasoning_items_preceding_dropped_calls keeps it. The reference runs over a history where the item it belongs to may still arrive; this helper runs on a completed turn, so there is nothing left to tie it to and keeping it would itself dangle. Documented in the docstring; happy to switch if you'd rather it match the reference exactly.

The mixed regression is test_blocked_tool_final_drops_reasoning_tied_to_the_rejected_message. It asserts on reasoning ids, not just types, so the "dropped the wrong one" case can't pass:

assert saved == ["user", "reasoning", "function_call", "function_call_output"]
assert "rs_committed" in saved_reasoning_ids
assert "rs_rejected" not in saved_reasoning_ids

then replays through a second turn and asserts the next model input is ["reasoning", "function_call", "function_call_output"] with no message. A/B against 747a1c70:

747a1c70  FAILED  at index 2 diff: 'reasoning' != 'function_call'   (rs_rejected leaks into the session)
ef6cf2a3  PASSED

I also updated the helper comment in the earlier commit as you asked — the sets are enumerated, so an item type added later is discarded here by default and has to be classified deliberately. I kept that default rather than switching to a deny-list: an unclassified item surfacing as a missing record is a louder failure than a rejected message quietly reaching the session, and _SIDE_EFFECT_ITEM_TYPES is now the only enumerated set (the reasoning set is gone, since it's derived).

One thing I should flag, since it's adjacent but not mine

While writing this I ran the mixed turn through the non-streamed path and it persists the entire turn on a tripwire — rejected message included:

mixed | run      | tripwire=True -> user, reasoning(rs_rejected), message, reasoning(rs_committed), function_call, function_call_output
mixed | streamed | tripwire=True -> user, reasoning(rs_committed), function_call, function_call_output

That's identical at this PR's base (bdc294fc), so it predates this change and isn't a regression — Runner.run never routed through the retention helper at all. I scoped the new regression to streamed rather than papering over it with a parametrize that asserts different things per mode. Happy to fix it here if you'd like the helper wired into the non-streamed tripwire path too, or to file it separately — your call on scope.

Full suites, tests/test_agent_runner_streamed.py tests/test_agent_runner.py tests/test_guardrails.py: 316 passed, 2 failed — test_blocking_guardrail_cancels_remaining_on_trigger{,_streaming}, which fail identically at bdc294fc and are unrelated. ruff check, ruff format --check, and mypy on run_loop.py are clean.