Skip to content

Commit 01f6e15

Browse files
committed
fix(sessions): the held record owns the conversion policy of its items
A detached re-park cannot see the Session backend, so it folded new items under the resuming run's own reasoning-id policy. For a Conversations-origin batch that strips the server id at the one point where nothing can restore it, and the reattach then drops the reasoning item as unpersistable. The park now records the conversion policy it actually used (None for a Conversations backend, the run's policy otherwise) on the held record, and a fold converts under the record's policy instead of the caller's. The key is gated and validated with the other held keys under the unreleased 1.18 schema, refused on ordinary pending writes and on unknown values; an absent key falls back to the caller's policy. Pinned at the fold and at the park, plus the validator rejection, each guard proven red by mutation.
1 parent b6a6636 commit 01f6e15

4 files changed

Lines changed: 114 additions & 6 deletions

File tree

src/agents/run_internal/session_persistence.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,9 +1061,13 @@ def defer_interrupted_session_write(
10611061

10621062
# The normal persistence path forces the reasoning-id policy to ``None`` for a
10631063
# Conversations backend so a server-identified reasoning item stays persistable;
1064-
# the registration conversion must match or the sanitization later drops it.
1064+
# the registration conversion must match or the sanitization later drops it. A
1065+
# standing record owns the policy its items were converted under, so a re-park
1066+
# folds new items under the same conversion instead of the resuming run's own.
10651067
if isinstance(session, OpenAIConversationsSession):
10661068
reasoning_item_id_policy = None
1069+
if pending is not None and "reasoning_item_id_policy" in pending:
1070+
reasoning_item_id_policy = pending["reasoning_item_id_policy"]
10671071
converted_run_items: list[TResponseInputItem] = []
10681072
for run_item in run_items:
10691073
as_input = run_item_to_input_item(run_item, reasoning_item_id_policy)
@@ -1105,6 +1109,7 @@ def defer_interrupted_session_write(
11051109
if (pending is not None and "response_id" in pending)
11061110
else response_id,
11071111
"store": pending["store"] if (pending is not None and "store" in pending) else store,
1112+
"reasoning_item_id_policy": reasoning_item_id_policy,
11081113
}
11091114
run_state._pending_session_write = record
11101115

@@ -1120,7 +1125,9 @@ def extend_held_session_write(
11201125
With no Session attached the resolved turn's save is a no-op, so the executed
11211126
tool output exists only in this process; folding it into the held batch lets the
11221127
reattaching resume settle call and output together. Does nothing when no held
1123-
batch stands.
1128+
batch stands. The fold converts under the batch's registration policy, not the
1129+
caller's: a detached run cannot see the original backend, and a server reasoning
1130+
id stripped here could not be restored at the settle.
11241131
"""
11251132
if run_state is None or run_state._pending_session_write is None:
11261133
return

src/agents/run_state.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
from .run_context import RunContextWrapper
110110
from .run_internal.items import (
111111
NestedHistoryOwnedItemRef,
112+
ReasoningItemIdPolicy,
112113
digest_input_item,
113114
ensure_nested_history_run_item_occurrence_key,
114115
nested_history_run_item_occurrence_key,
@@ -180,6 +181,11 @@ class _PendingSessionWrite(TypedDict):
180181
``response_id`` records the model response the withheld batch belongs to, and
181182
``store`` the store setting that response was produced under, so the settle runs
182183
the same compaction bookkeeping the ordinary persistence path would have run for
184+
``reasoning_item_id_policy`` records how the batch's items were converted, so a
185+
detached re-park folds new items under the same conversion: a Conversations-origin
186+
batch preserves server reasoning ids even when the resuming run's own policy would
187+
omit them, and an id stripped at registration cannot be restored at the settle.
188+
183189
it instead of appending behind its back.
184190
"""
185191

@@ -190,6 +196,7 @@ class _PendingSessionWrite(TypedDict):
190196
held: NotRequired[bool]
191197
response_id: NotRequired[str | None]
192198
store: NotRequired[bool | None]
199+
reasoning_item_id_policy: NotRequired[ReasoningItemIdPolicy | None]
193200

194201

195202
def _default_run_state_validation_error(
@@ -248,7 +255,8 @@ def _default_run_state_validation_error(
248255
),
249256
"1.18": (
250257
"Persists the interrupted turn's withheld Session write, including the response it "
251-
"belongs to, so an approval resume can settle it under the output-guardrail gate."
258+
"belongs to and the conversion policy its items were registered under, so an "
259+
"approval resume can settle it under the output-guardrail gate."
252260
),
253261
}
254262
SUPPORTED_SCHEMA_VERSIONS = frozenset(SCHEMA_VERSION_SUMMARIES)
@@ -4393,7 +4401,11 @@ async def _build_run_state_from_json(
43934401
for part in _HELD_PENDING_SESSION_WRITE_MIN_SCHEMA_VERSION.split(".", maxsplit=1)
43944402
)
43954403
base_keys = {"session_id", "items", "before", "persisted_count"}
4396-
held_keys = {"held", "response_id", "store"} if held_keys_allowed else set()
4404+
held_keys = (
4405+
{"held", "response_id", "store", "reasoning_item_id_policy"}
4406+
if held_keys_allowed
4407+
else set()
4408+
)
43974409
if (
43984410
(schema_major, schema_minor) < (1, 17)
43994411
or not isinstance(state._current_step, NextStepRunAgain | NextStepInterruption)
@@ -4410,12 +4422,20 @@ async def _build_run_state_from_json(
44104422
and pending_write["store"] is not None
44114423
and type(pending_write["store"]) is not bool
44124424
)
4413-
# Both keys describe the withheld response, so they are meaningless on an
4425+
or (
4426+
"reasoning_item_id_policy" in pending_write
4427+
and pending_write["reasoning_item_id_policy"] not in (None, "preserve", "omit")
4428+
)
4429+
# These keys describe the withheld batch, so they are meaningless on an
44144430
# ordinary pending write and are refused there rather than restored as
44154431
# state nothing consumes.
44164432
or (
44174433
not pending_write.get("held")
4418-
and ("response_id" in pending_write or "store" in pending_write)
4434+
and (
4435+
"response_id" in pending_write
4436+
or "store" in pending_write
4437+
or "reasoning_item_id_policy" in pending_write
4438+
)
44194439
)
44204440
or not isinstance(pending_write.get("session_id"), str)
44214441
or not isinstance(pending_write.get("items"), list)

tests/test_deferred_interrupted_session_write.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,80 @@ class _Session:
14871487
assert state._pending_session_write["response_id"] == "resp_parked"
14881488

14891489

1490+
@pytest.mark.asyncio
1491+
async def test_a_detached_re_park_folds_under_the_batch_registration_policy() -> None:
1492+
# A Conversations-origin batch was converted preserving server reasoning ids. The
1493+
# detached re-park cannot see the backend, so it must fold under the policy the
1494+
# record carries rather than the resuming run's own: an id stripped here is
1495+
# unrecoverable and the reattach would drop the reasoning item as unpersistable.
1496+
from agents.items import ReasoningItem
1497+
from agents.run_internal.session_persistence import extend_held_session_write
1498+
1499+
agent = _make_deferring_agent()
1500+
state = object.__new__(RunState)
1501+
state._pending_session_write = {
1502+
"session_id": "conv_abc",
1503+
"items": [
1504+
{"type": "function_call", "call_id": "call_PARKED", "name": "t", "arguments": "{}"}
1505+
],
1506+
"before": None,
1507+
"persisted_count": 1,
1508+
"held": True,
1509+
"response_id": "resp_parked",
1510+
"store": None,
1511+
"reasoning_item_id_policy": None,
1512+
}
1513+
state._current_turn_persisted_item_count = 0
1514+
reasoning = ReasoningItem(
1515+
agent=agent,
1516+
raw_item={"id": "rs_SERVER_ID", "type": "reasoning", "summary": [], "content": []},
1517+
)
1518+
1519+
extend_held_session_write(state, run_items=[reasoning], reasoning_item_id_policy="omit")
1520+
1521+
items = state._pending_session_write["items"]
1522+
reasoning_ids = [i.get("id") for i in items if i.get("type") == "reasoning"]
1523+
assert reasoning_ids == ["rs_SERVER_ID"]
1524+
assert state._pending_session_write["reasoning_item_id_policy"] is None
1525+
1526+
1527+
@pytest.mark.asyncio
1528+
async def test_the_park_records_the_conversion_policy_it_used() -> None:
1529+
# The record owns how its items were converted. A Conversations park forces the
1530+
# preserving policy regardless of the run's own setting, and the recorded value is
1531+
# what a later detached fold must reuse.
1532+
from agents.items import ToolCallItem
1533+
from agents.memory.openai_conversations_session import OpenAIConversationsSession
1534+
from agents.run_internal.session_persistence import defer_interrupted_session_write
1535+
1536+
session = object.__new__(OpenAIConversationsSession)
1537+
session._session_id = "conv_abc"
1538+
state = object.__new__(RunState)
1539+
state._pending_session_write = None
1540+
state._current_turn_persisted_item_count = 0
1541+
call = ToolCallItem(
1542+
agent=_make_deferring_agent(),
1543+
raw_item={
1544+
"type": "function_call",
1545+
"call_id": "call_PARKED",
1546+
"name": "t",
1547+
"arguments": "{}",
1548+
},
1549+
)
1550+
1551+
defer_interrupted_session_write(
1552+
state,
1553+
session,
1554+
run_items=[call],
1555+
reasoning_item_id_policy="omit",
1556+
response_id="resp_parked",
1557+
store=None,
1558+
)
1559+
1560+
assert state._pending_session_write is not None
1561+
assert state._pending_session_write["reasoning_item_id_policy"] is None
1562+
1563+
14901564
class _CompactionRecordingSession(SimpleListSession):
14911565
"""Record the compaction bookkeeping a compaction-aware backend expects."""
14921566

tests/test_run_impl_resume_paths.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,7 @@ async def test_failed_streamed_result_checkpoint_retains_detached_pending_write(
501501
"held-with-before",
502502
"held-under-1-17",
503503
"held-keys-without-held",
504+
"policy-shape",
504505
],
505506
)
506507
async def test_pending_session_write_rejects_invalid_serialized_checkpoint(invalid: str) -> None:
@@ -519,6 +520,12 @@ async def test_pending_session_write_rejects_invalid_serialized_checkpoint(inval
519520
# response_id and store describe the withheld response, so they are refused on
520521
# an ordinary pending write where nothing consumes them.
521522
payload["pending_session_write"]["response_id"] = "resp_1"
523+
elif invalid == "policy-shape":
524+
# The conversion-policy key only speaks the two policy literals or None; any
525+
# other value would silently change how a fold converts the batch's items.
526+
payload["pending_session_write"]["held"] = True
527+
payload["pending_session_write"]["before"] = None
528+
payload["pending_session_write"]["reasoning_item_id_policy"] = "banana"
522529
elif invalid == "held-under-1-17":
523530
# 1.17 defined the pending write as exactly four keys, so the held variant is
524531
# only readable under the version that introduced it.

0 commit comments

Comments
 (0)