Skip to content

Popping compaction session history retains the previous response chain #4867

Description

@sylvesterkaczmarek

Describe the bug

OpenAIResponsesCompactionSession.pop_item() removes the latest item from the client-managed session history, but it leaves the wrapper's retained Responses chain state unchanged: _response_id, _deferred_response_id, and _last_unstored_response_id.

After a successful pop, the local history and the retained server-managed response chain can therefore describe different conversations. A later compaction in previous_response_id mode can reuse a response ID whose server-side history still contains the item the caller explicitly removed locally.

Reproduction

from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock

from agents.memory import OpenAIResponsesCompactionSession
from tests.utils.simple_session import SimpleListSession

item = {"type": "message", "role": "assistant", "content": "old"}
underlying = SimpleListSession(history=[item])
client = MagicMock()
client.responses.compact = AsyncMock(
    return_value=SimpleNamespace(output=[], usage=None)
)

session = OpenAIResponsesCompactionSession(
    session_id="test",
    underlying_session=underlying,
    client=client,
    compaction_mode="previous_response_id",
)

# Below the threshold, this records the response ID without making a compact call.
await session.run_compaction({"response_id": "resp-old"})

# The local history no longer matches the response chain behind resp-old.
await session.pop_item()

# Current main still reuses resp-old here instead of requiring fresh chain state.
await session.run_compaction({"force": True})

On current main, the final call proceeds with previous_response_id="resp-old".

Expected behavior

A successful pop_item() that actually removes an item should invalidate all retained response-chain state owned by the compaction wrapper. A later previous_response_id compaction should require a newly supplied response ID before making a client call.

If pop_item() is a no-op because the session is empty, the existing response-chain state should remain unchanged. If the underlying pop raises, state should likewise remain unchanged because the mutation did not complete.

This is the pop_item() counterpart to #4864: both operations mutate local history, but pop_item() has its own successful-mutation versus no-op boundary.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions