Skip to content

Commit 525cd20

Browse files
yongpangeyongpange
authored
fix(sessions): reset compaction response chain on clear_session (#5000)
1 parent 265f16f commit 525cd20

2 files changed

Lines changed: 157 additions & 0 deletions

File tree

‎src/agents/memory/openai_responses_compaction_session.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,14 +737,19 @@ async def clear_session(self) -> None:
737737
try:
738738
await self.underlying_session.clear_session()
739739
except (Exception, asyncio.CancelledError):
740+
# The deletion may have committed before acknowledgement failed.
740741
self._compaction_candidate_items = None
741742
self._session_items = None
743+
self._response_id = None
742744
self._deferred_response_id = None
745+
self._last_unstored_response_id = None
743746
self._mutation_generation += 1
744747
raise
745748
self._compaction_candidate_items = []
746749
self._session_items = []
750+
self._response_id = None
747751
self._deferred_response_id = None
752+
self._last_unstored_response_id = None
748753
self._mutation_generation += 1
749754

750755
async def _ensure_compaction_candidates(

‎tests/memory/test_openai_responses_compaction_session.py‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,158 @@ def commit(self) -> None:
456456
await asyncio.gather(mutation, return_exceptions=True)
457457
underlying.close()
458458

459+
@pytest.mark.asyncio
460+
async def test_clear_session_invalidates_response_chain(self) -> None:
461+
item: TResponseInputItem = {"role": "assistant", "content": "remove me"}
462+
underlying = SimpleListSession(history=[item])
463+
mock_client = MagicMock()
464+
mock_client.responses.compact = AsyncMock(
465+
return_value=SimpleNamespace(output=[], usage=None)
466+
)
467+
session = OpenAIResponsesCompactionSession(
468+
session_id="test",
469+
underlying_session=underlying,
470+
client=mock_client,
471+
compaction_mode="previous_response_id",
472+
should_trigger_compaction=lambda ctx: False,
473+
)
474+
await session.run_compaction({"response_id": "resp-old", "store": False})
475+
# Seed deferred work through the runner's compaction hook.
476+
session.should_trigger_compaction = lambda ctx: True
477+
await session._defer_compaction("resp-old")
478+
479+
await session.clear_session()
480+
assert await session.get_items() == []
481+
with pytest.raises(ValueError, match="requires a response_id"):
482+
await session.run_compaction({"force": True})
483+
mock_client.responses.compact.assert_not_awaited()
484+
assert session._get_deferred_compaction_response_id() is None
485+
assert session._last_unstored_response_id is None
486+
487+
await session.run_compaction({"response_id": "resp-new", "force": True})
488+
assert mock_client.responses.compact.await_args is not None
489+
assert mock_client.responses.compact.await_args.kwargs["previous_response_id"] == "resp-new"
490+
491+
@pytest.mark.asyncio
492+
@pytest.mark.parametrize("outcome", ["cancelled", "error"])
493+
async def test_settled_clear_failure_invalidates_response_chain(
494+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, outcome: str
495+
) -> None:
496+
# Gate the real SQLite commit acknowledgement so cancellation cannot undo the clear.
497+
committed = threading.Event()
498+
allow_return = threading.Event()
499+
500+
class PausingCommitConnection(sqlite3.Connection):
501+
pause_commit = True
502+
503+
def commit(self) -> None:
504+
super().commit()
505+
if self.pause_commit:
506+
self.pause_commit = False
507+
committed.set()
508+
assert allow_return.wait(timeout=10)
509+
if outcome == "error":
510+
raise RuntimeError("commit acknowledgement failed")
511+
512+
underlying = SQLiteSession("settled-clear", tmp_path / "settled-clear.db")
513+
history: list[TResponseInputItem] = [
514+
{"role": "user", "content": "keep me"},
515+
{"role": "assistant", "content": "remove me"},
516+
]
517+
await underlying.add_items(history)
518+
connection = sqlite3.connect(
519+
str(tmp_path / "settled-clear.db"),
520+
check_same_thread=False,
521+
factory=PausingCommitConnection,
522+
)
523+
with underlying._connections_lock:
524+
underlying._connections.add(connection)
525+
monkeypatch.setattr(underlying, "_get_connection", lambda: connection)
526+
mock_client = MagicMock()
527+
mock_client.responses.compact = AsyncMock(
528+
return_value=SimpleNamespace(output=[], usage=None)
529+
)
530+
session = OpenAIResponsesCompactionSession(
531+
session_id="settled-clear",
532+
underlying_session=underlying,
533+
client=mock_client,
534+
compaction_mode="previous_response_id",
535+
should_trigger_compaction=lambda ctx: False,
536+
)
537+
await session.run_compaction({"response_id": "resp-old", "store": False})
538+
session.should_trigger_compaction = lambda ctx: True
539+
await session._defer_compaction("resp-old")
540+
mutation = asyncio.create_task(session.clear_session())
541+
try:
542+
assert await asyncio.to_thread(committed.wait, 10)
543+
if outcome == "cancelled":
544+
mutation.cancel()
545+
allow_return.set()
546+
if outcome == "cancelled":
547+
with pytest.raises(asyncio.CancelledError):
548+
await mutation
549+
else:
550+
with pytest.raises(RuntimeError, match="commit acknowledgement failed"):
551+
await mutation
552+
553+
# The deletion committed before the failure reached the caller.
554+
assert await session.get_items() == []
555+
assert not session._mutation_lock.locked()
556+
with pytest.raises(ValueError, match="requires a response_id"):
557+
await session.run_compaction({"force": True})
558+
mock_client.responses.compact.assert_not_awaited()
559+
assert session._get_deferred_compaction_response_id() is None
560+
assert session._last_unstored_response_id is None
561+
562+
# The session stays usable: a subsequent write survives.
563+
follow_up: TResponseInputItem = {"role": "user", "content": "after clear"}
564+
await session.add_items([follow_up])
565+
assert await session.get_items() == [follow_up]
566+
await session.run_compaction({"response_id": "resp-new", "force": True})
567+
assert mock_client.responses.compact.await_args is not None
568+
compact_kwargs = mock_client.responses.compact.await_args.kwargs
569+
assert compact_kwargs["previous_response_id"] == "resp-new"
570+
finally:
571+
allow_return.set()
572+
if not mutation.done():
573+
mutation.cancel()
574+
await asyncio.gather(mutation, return_exceptions=True)
575+
underlying.close()
576+
577+
@pytest.mark.asyncio
578+
async def test_run_compaction_auto_uses_input_after_clear(self) -> None:
579+
item: TResponseInputItem = {"role": "assistant", "content": "old turn"}
580+
underlying = SimpleListSession(history=[item])
581+
mock_client = MagicMock()
582+
mock_client.responses.compact = AsyncMock(
583+
return_value=SimpleNamespace(output=[], usage=None)
584+
)
585+
session = OpenAIResponsesCompactionSession(
586+
session_id="test",
587+
underlying_session=underlying,
588+
client=mock_client,
589+
)
590+
# Default auto mode follows the stored response chain first.
591+
await session.run_compaction({"response_id": "resp-old", "force": True})
592+
first_kwargs = mock_client.responses.compact.call_args.kwargs
593+
assert first_kwargs.get("previous_response_id") == "resp-old"
594+
assert "input" not in first_kwargs
595+
596+
await session.clear_session()
597+
new_items: list[TResponseInputItem] = [
598+
cast(TResponseInputItem, {"role": "user", "content": "fresh start"}),
599+
cast(TResponseInputItem, {"role": "assistant", "content": "fresh reply"}),
600+
]
601+
await session.add_items(new_items)
602+
603+
# After clearing the stored response, auto mode compacts from the
604+
# current local input instead of reusing the cleared response chain.
605+
await session.run_compaction({"force": True})
606+
assert mock_client.responses.compact.call_count == 2
607+
second_kwargs = mock_client.responses.compact.call_args.kwargs
608+
assert "previous_response_id" not in second_kwargs
609+
assert second_kwargs.get("input") == new_items
610+
459611
@pytest.mark.asyncio
460612
async def test_get_items_delegates(self) -> None:
461613
mock_session = self.create_mock_session()

0 commit comments

Comments
 (0)