Skip to content

Commit 33651b1

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 58ac099 commit 33651b1

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
@@ -1096,9 +1096,13 @@ def defer_interrupted_session_write(
10961096

10971097
# The normal persistence path forces the reasoning-id policy to ``None`` for a
10981098
# Conversations backend so a server-identified reasoning item stays persistable;
1099-
# the registration conversion must match or the sanitization later drops it.
1099+
# the registration conversion must match or the sanitization later drops it. A
1100+
# standing record owns the policy its items were converted under, so a re-park
1101+
# folds new items under the same conversion instead of the resuming run's own.
11001102
if isinstance(session, OpenAIConversationsSession):
11011103
reasoning_item_id_policy = None
1104+
if pending is not None and "reasoning_item_id_policy" in pending:
1105+
reasoning_item_id_policy = pending["reasoning_item_id_policy"]
11021106
converted_run_items: list[TResponseInputItem] = []
11031107
for run_item in run_items:
11041108
as_input = run_item_to_input_item(run_item, reasoning_item_id_policy)
@@ -1140,6 +1144,7 @@ def defer_interrupted_session_write(
11401144
if (pending is not None and "response_id" in pending)
11411145
else response_id,
11421146
"store": pending["store"] if (pending is not None and "store" in pending) else store,
1147+
"reasoning_item_id_policy": reasoning_item_id_policy,
11431148
}
11441149
run_state._pending_session_write = record
11451150

@@ -1155,7 +1160,9 @@ def extend_held_session_write(
11551160
With no Session attached the resolved turn's save is a no-op, so the executed
11561161
tool output exists only in this process; folding it into the held batch lets the
11571162
reattaching resume settle call and output together. Does nothing when no held
1158-
batch stands.
1163+
batch stands. The fold converts under the batch's registration policy, not the
1164+
caller's: a detached run cannot see the original backend, and a server reasoning
1165+
id stripped here could not be restored at the settle.
11591166
"""
11601167
if run_state is None or run_state._pending_session_write is None:
11611168
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)
@@ -4403,7 +4411,11 @@ async def _build_run_state_from_json(
44034411
for part in _HELD_PENDING_SESSION_WRITE_MIN_SCHEMA_VERSION.split(".", maxsplit=1)
44044412
)
44054413
base_keys = {"session_id", "items", "before", "persisted_count"}
4406-
held_keys = {"held", "response_id", "store"} if held_keys_allowed else set()
4414+
held_keys = (
4415+
{"held", "response_id", "store", "reasoning_item_id_policy"}
4416+
if held_keys_allowed
4417+
else set()
4418+
)
44074419
if (
44084420
(schema_major, schema_minor) < (1, 17)
44094421
or not isinstance(state._current_step, NextStepRunAgain | NextStepInterruption)
@@ -4420,12 +4432,20 @@ async def _build_run_state_from_json(
44204432
and pending_write["store"] is not None
44214433
and type(pending_write["store"]) is not bool
44224434
)
4423-
# Both keys describe the withheld response, so they are meaningless on an
4435+
or (
4436+
"reasoning_item_id_policy" in pending_write
4437+
and pending_write["reasoning_item_id_policy"] not in (None, "preserve", "omit")
4438+
)
4439+
# These keys describe the withheld batch, so they are meaningless on an
44244440
# ordinary pending write and are refused there rather than restored as
44254441
# state nothing consumes.
44264442
or (
44274443
not pending_write.get("held")
4428-
and ("response_id" in pending_write or "store" in pending_write)
4444+
and (
4445+
"response_id" in pending_write
4446+
or "store" in pending_write
4447+
or "reasoning_item_id_policy" in pending_write
4448+
)
44294449
)
44304450
or not isinstance(pending_write.get("session_id"), str)
44314451
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
@@ -508,6 +508,7 @@ async def test_failed_streamed_result_checkpoint_retains_detached_pending_write(
508508
"held-with-before",
509509
"held-under-1-17",
510510
"held-keys-without-held",
511+
"policy-shape",
511512
],
512513
)
513514
async def test_pending_session_write_rejects_invalid_serialized_checkpoint(invalid: str) -> None:
@@ -526,6 +527,12 @@ async def test_pending_session_write_rejects_invalid_serialized_checkpoint(inval
526527
# response_id and store describe the withheld response, so they are refused on
527528
# an ordinary pending write where nothing consumes them.
528529
payload["pending_session_write"]["response_id"] = "resp_1"
530+
elif invalid == "policy-shape":
531+
# The conversion-policy key only speaks the two policy literals or None; any
532+
# other value would silently change how a fold converts the batch's items.
533+
payload["pending_session_write"]["held"] = True
534+
payload["pending_session_write"]["before"] = None
535+
payload["pending_session_write"]["reasoning_item_id_policy"] = "banana"
529536
elif invalid == "held-under-1-17":
530537
# 1.17 defined the pending write as exactly four keys, so the held variant is
531538
# only readable under the version that introduced it.

0 commit comments

Comments
 (0)