Skip to content

Commit 58ac099

Browse files
committed
fix(sessions): count and classify the settling batch on the compaction path
Two defects in how a settling held batch meets a compaction-aware session: - The compaction-deferral branch returned the run-item count alone, and it is the branch every held settle with outputs takes on such a backend, so exactly the sessions that defer were the ones whose settled turns undercounted. The count gates the resumed-safety refusal and slices later saves of the same turn, so it must equal what the append wrote. The branch now returns the combined count. - The local-continuation classification knew the mapped tool outputs but not the hosted MCP approval response, which is the locally produced half of its approval pair and must stay associated with the response chain that carried the request. Compacting that response before the model consumes the approval drops it in previous_response_id mode. The constant is now _LOCAL_CONTINUATION_OUTPUT_TYPES and covers both carriers: the settled dict and the MCPApprovalResponseItem the non-deferred resume commits, because classifying one and not the other would defer or compact the same response depending on which path persisted it. The four-stage scenario behind the count (partial approval, held settlement with a lapsed gate, gate re-enable, remaining approval) is pinned end to end: it must end in the documented fail-fast refusal with nothing duplicated. Each fix is also pinned at the unit boundary and proven red by mutation.
1 parent b498f4b commit 58ac099

2 files changed

Lines changed: 141 additions & 6 deletions

File tree

src/agents/run_internal/session_persistence.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
HandoffOutputItem,
2222
InputItem,
2323
ItemHelpers,
24+
MCPApprovalResponseItem,
2425
ModelResponse,
2526
RunItem,
2627
ToolCallOutputItem,
@@ -95,9 +96,14 @@
9596

9697
_SESSION_LIMIT_UNSET = object()
9798

98-
# Serialized item types that represent a locally produced tool output, i.e. the output
99-
# kinds of the canonical call-to-output map.
100-
_LOCAL_TOOL_OUTPUT_TYPES = frozenset(_TOOL_CALL_TO_OUTPUT_TYPE.values())
99+
# Serialized item types produced locally as the continuation of a model response: the
100+
# output kinds of the canonical call-to-output map, plus the hosted MCP approval
101+
# response, which is the locally produced half of its approval pair. Compaction for
102+
# the response that carried the request must be deferred while any of these still
103+
# needs to be associated with that response chain.
104+
_LOCAL_CONTINUATION_OUTPUT_TYPES = frozenset(_TOOL_CALL_TO_OUTPUT_TYPE.values()) | {
105+
"mcp_approval_response"
106+
}
101107

102108

103109
async def admit_pending_input(
@@ -776,11 +782,12 @@ async def save_result_to_session(
776782
# landed. Only a settle reads that slot: on an ordinary save it holds the
777783
# caller's input, whose earlier outputs say nothing about this response.
778784
has_local_tool_outputs = any(
779-
isinstance(item, ToolCallOutputItem | HandoffOutputItem) for item in new_items
785+
isinstance(item, ToolCallOutputItem | HandoffOutputItem | MCPApprovalResponseItem)
786+
for item in new_items
780787
) or (
781788
settling_held_batch
782789
and any(
783-
isinstance(item, dict) and item.get("type") in _LOCAL_TOOL_OUTPUT_TYPES
790+
isinstance(item, dict) and item.get("type") in _LOCAL_CONTINUATION_OUTPUT_TYPES
784791
for item in items_to_save
785792
)
786793
)
@@ -797,7 +804,7 @@ async def save_result_to_session(
797804
"skip: deferring compaction for response %s due to local tool outputs",
798805
response_id,
799806
)
800-
return saved_run_items_count
807+
return saved_run_items_count + settled_batch_items
801808

802809
deferred_response_id = None
803810
get_deferred = getattr(session, "_get_deferred_compaction_response_id", None)

tests/test_deferred_interrupted_session_write.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,6 +1525,134 @@ async def test_the_compaction_deferral_reads_the_settling_batch_not_the_callers_
15251525
assert [entry for entry in session.compactions if "deferred" in entry] == []
15261526

15271527

1528+
@pytest.mark.asyncio
1529+
async def test_the_settled_count_survives_the_compaction_deferral_branch() -> None:
1530+
# The deferral branch is the one every held settle with outputs takes on a
1531+
# compaction-aware backend, so returning the run-item count alone there reports a
1532+
# turn that persisted less than it wrote. That count gates the final sweep's
1533+
# re-append protection on a later gate-enabled resume.
1534+
from agents.run_internal.session_persistence import save_result_to_session
1535+
1536+
session = _CompactionRecordingSession()
1537+
held: list[TResponseInputItem] = [
1538+
{"type": "function_call", "call_id": "call_PARKED", "name": "t", "arguments": "{}"},
1539+
{"type": "function_call_output", "call_id": "call_PARKED", "output": "ok"},
1540+
]
1541+
1542+
count = await save_result_to_session(
1543+
session, held, [], None, response_id="resp_parked", settling_held_batch=True
1544+
)
1545+
1546+
assert [entry for entry in session.compactions if "deferred" in entry] == [
1547+
{"deferred": "resp_parked", "store": None}
1548+
]
1549+
assert count == len(await session.get_items())
1550+
1551+
1552+
@pytest.mark.asyncio
1553+
@pytest.mark.parametrize("streamed", [False, True])
1554+
async def test_a_partial_settle_on_a_compaction_session_still_fails_the_gated_resume_fast(
1555+
streamed: bool,
1556+
) -> None:
1557+
# Park two calls, approve one, and let the gate lapse for that resume so the batch
1558+
# settles into a compaction-aware session mid-run. Re-enable the gate and approve
1559+
# the rest: the settled turn's count must cover what the settle wrote, or the
1560+
# final sweep treats the turn as unpersisted and appends the stored items again.
1561+
from agents.exceptions import UserError
1562+
1563+
session = _CompactionRecordingSession()
1564+
agent = _make_multi_approval_agent()
1565+
1566+
first = await _run(agent, "go", session, streamed=streamed)
1567+
state = await _serialized_round_trip(first, agent)
1568+
state.approve(
1569+
next(
1570+
interruption
1571+
for interruption in state.get_interruptions()
1572+
if getattr(interruption.raw_item, "call_id", None) == "call_PARKED"
1573+
)
1574+
)
1575+
gate = agent.output_guardrails
1576+
agent.output_guardrails = []
1577+
second = await _run(agent, state, session, streamed=streamed)
1578+
assert len(second.interruptions) == 1
1579+
agent.output_guardrails = gate
1580+
1581+
state = await _serialized_round_trip(second, agent)
1582+
for interruption in state.get_interruptions():
1583+
state.approve(interruption)
1584+
# The settled turn persisted items, so the re-enabled gate must refuse the resume
1585+
# outright; an undercounted turn is what would let it proceed and re-append the
1586+
# stored items through the final sweep.
1587+
with pytest.raises(UserError, match="output guardrails after current-turn items"):
1588+
await _run(agent, state, session, streamed=streamed)
1589+
1590+
items = await session.get_items()
1591+
assert _call_ids(items).count("call_PARKED") == 1
1592+
assert _call_ids(items).count("call_PARKED_2") == 1
1593+
outputs = {item.get("call_id") for item in items if item.get("type") == "function_call_output"}
1594+
# Only the second call may still be awaiting its output; nothing is duplicated.
1595+
assert outputs == {"call_PARKED"}
1596+
1597+
1598+
@pytest.mark.asyncio
1599+
async def test_a_held_mcp_approval_pair_defers_compaction_when_it_settles() -> None:
1600+
# The approval response is the locally produced half of its pair and must stay
1601+
# associated with the response chain that carried the request; compacting that
1602+
# response before the model consumes the approval drops it in
1603+
# ``previous_response_id`` mode.
1604+
from agents.run_internal.session_persistence import save_result_to_session
1605+
1606+
session = _CompactionRecordingSession()
1607+
held: list[TResponseInputItem] = [
1608+
{
1609+
"type": "mcp_approval_request",
1610+
"id": "mcpr_1",
1611+
"server_label": "srv",
1612+
"name": "do_it",
1613+
"arguments": "{}",
1614+
},
1615+
{"type": "mcp_approval_response", "approval_request_id": "mcpr_1", "approve": True},
1616+
]
1617+
1618+
count = await save_result_to_session(
1619+
session, held, [], None, response_id="resp_parked", settling_held_batch=True
1620+
)
1621+
1622+
assert [entry for entry in session.compactions if "deferred" in entry] == [
1623+
{"deferred": "resp_parked", "store": None}
1624+
]
1625+
assert [entry for entry in session.compactions if "response_id" in entry] == []
1626+
assert count == len(await session.get_items())
1627+
1628+
1629+
@pytest.mark.asyncio
1630+
async def test_an_ordinary_mcp_approval_response_defers_compaction_too() -> None:
1631+
# The non-deferred resume commits the approval response as a run item, and the
1632+
# classification must treat both carriers alike: deferring for the settled dict
1633+
# but not for the run item would leave the same response compacted or not
1634+
# depending on which path persisted it.
1635+
from agents.items import MCPApprovalResponseItem
1636+
from agents.run_internal.session_persistence import save_result_to_session
1637+
1638+
session = _CompactionRecordingSession()
1639+
agent = _make_deferring_agent()
1640+
response_item = MCPApprovalResponseItem(
1641+
agent=agent,
1642+
raw_item={
1643+
"type": "mcp_approval_response",
1644+
"approval_request_id": "mcpr_1",
1645+
"approve": True,
1646+
},
1647+
)
1648+
1649+
await save_result_to_session(session, [], [response_item], None, response_id="resp_live")
1650+
1651+
assert [entry for entry in session.compactions if "deferred" in entry] == [
1652+
{"deferred": "resp_live", "store": None}
1653+
]
1654+
1655+
15281656
@pytest.mark.asyncio
15291657
async def test_the_entry_settle_runs_the_compaction_bookkeeping() -> None:
15301658
# The entry settle goes through the canonical persistence path, so a

0 commit comments

Comments
 (0)