fix(memory): compact the full stored history of a limited session

[hsusul]

Summary

OpenAIResponsesCompactionSession loads compaction input with underlying_session.get_items(), which applies SessionSettings.limit. After responses.compact, it clears the entire underlying store and writes the compact result.

For a SQLite (or similar) session that stores full history but retrieves only the last N items, successful compaction therefore deletes everything older than N. The restore path already uses _get_all_underlying_session_items(); candidate loading did not.

Reproduction

from agents.memory import SQLiteSession, SessionSettings, OpenAIResponsesCompactionSession

underlying = SQLiteSession("s", session_settings=SessionSettings(limit=2))
# add 3+ items, then force input-mode compaction
# responses.compact receives only the last 2 items
# the session is then replaced with that compact output

Solution

Load candidates via the existing _get_all_underlying_session_items() helper (limit=2_147_483_647), matching the replacement/restore path.

This is a smaller version of the candidate-loading fix from closed stale #3827, without that PR's broader previous_response_id rewrite.

Test plan

• Added a SQLiteSession regression that sets SessionSettings(limit=2), stores 3 items, and asserts responses.compact receives all 3
• uv run pytest tests/memory/test_openai_responses_compaction_session.py (56 passed)
• uv run ruff format / ruff check on the changed files
• uv run pyright on the changed files
• git diff --check clean

Issue number

N/A. Related closed stale PR: #3827.

Checks

• I've added new tests, if relevant
• I've run targeted format, lint, typecheck, and tests on the changed files
• I've run .agents/skills/code-change-verification/scripts/run.sh (full make format currently fails on an unrelated pre-existing E501 in .agents/skills/implementation-final-review/scripts/test_skill_contract.py on main)
• If using Codex, I've run /review before submitting this PR

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76381b5c04

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid triggering response-ID compaction from hidden rows

When the underlying session has SessionSettings(limit=N) with N below the threshold but more than ten candidates stored, this full read now triggers default "auto" compaction; however, _resolve_compaction_mode() selects previous_response_id for a normally stored response, so the compaction request does not include these full session_items. Because that response was created using only the limited session window, its compacted output cannot represent the older rows, yet the replacement clears the entire local store. This therefore newly deletes the hidden history in the default mode; either switch to input-mode compaction when the full store differs from the retrieval window or keep hidden rows from triggering response-ID compaction.

AGENTS.md reference: AGENTS.md:L201-L203

Useful? React with 👍 / 👎.

[seratch]

Thanks for the contribution. The current full-history read makes the default auto mode unsafe. Hidden rows can now trigger compaction while auto selects previous_response_id; that response represents only the limited session window, but replacement clears the full local store.

Please make history selection mode-aware. Input compaction must use the full store, and auto must switch to input whenever the full store differs from the ordinary session view. An explicit previous_response_id request must fail before provider or storage side effects when hidden local rows make that response chain non-authoritative, with input mode identified as the supported alternative. Please add regressions for a visible count below the threshold with a full count above it, and for the explicit previous_response_id case, asserting both request shape and preserved history.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4142c69945

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Defer the incompatible-mode error until compaction is due

When compaction_mode="previous_response_id" wraps a session with a small retrieval limit, this condition raises as soon as stored history exceeds that limit—even if the default threshold has not been reached or a custom should_trigger_compaction hook would return False. Because the runner invokes run_compaction after persisting a successful response, such runs now fail after observable work despite no compaction being due; reject this configuration before the run starts, or raise only after the decision hook selects compaction.

AGENTS.md reference: AGENTS.md:L150-L150

Useful? React with 👍 / 👎.

[FU-max-boop]

Thanks for the mode-aware update at exact head 4142c699. I independently reran the four event-controlled ownership probes against this head with a real SQLite session. The limited/full-history auto-authority case and the provider/storage-side explicit-previous fail-fast case no longer reproduce; I am keeping the already-reviewed hook-order issue separate.

Two deterministic concurrency windows remain (2 passed, 2 failed):

1. A wrapper add_items(late-write) that completes while the compaction provider is suspended is erased when the older provider result later performs unconditional replacement. The final store contains only the compacted snapshot.
2. Two gated run_compaction attempts called with resp-first and resp-second both send resp-second. Each attempt writes the shared _response_id before an await, then later rereads that shared field while constructing its request.

These appear to be one attempt-ownership boundary: response ID, resolved mode, full-history snapshot, provider request, and replacement authority need to belong to the same transaction. The narrow serialization option is to hold the existing mutation lock from snapshot through replacement. If the provider await must stay outside the lock, the equivalent safe shape needs a generation/history check after reacquiring it and must discard the provider output without mutating storage when the snapshot changed; taking a newer pre-clear snapshot does not make output computed from the older snapshot safe.

Current-head focused tests remain green (60 passed) and Ruff passes. I have compact cleanup-safe tests for the two remaining windows and have not opened a competing change.