fix(sessions): skip corrupt docs in MongoDBSession.pop_item - #3247
Merged
Conversation
When pop_item retrieves a document whose message_data fails to deserialize (invalid JSON, missing/non-string field), it currently returns None — the same value used to signal an empty session. A non-empty session with a corrupt latest doc therefore looks empty to callers, causing them to stop draining items even though valid messages remain. Wrap the pop in a loop that discards corrupt rows and tries the next-most recent until a valid item is decoded or the session is genuinely empty. This brings pop_item in line with get_items, which already skips corrupt documents. Adds two regression tests covering the mixed and all-corrupt cases.
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
seratch
approved these changes
May 9, 2026
This was referenced May 9, 2026
RudrenduPaul
pushed a commit
to RudrenduPaul/openai-agents-python
that referenced
this pull request
Jun 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MongoDBSession.pop_itemreturnsNonefrom two distinct conditions:message_datafails to deserialize (invalid JSON, missing field, or non-string type — the same(JSONDecodeError, KeyError, TypeError)triple thatget_itemsalready swallows).In the second case the corrupt document is removed by
find_one_and_deleteand the caller seesNone, even though valid older messages still exist. A loop that pops items until empty therefore stops one item early and leaves history behind.This mirrors the behavior
get_itemsalready provides — it silently skips corrupt rows and keeps reading valid ones — sopop_itemshould do the same to stay consistent.Fix
Wrap the pop in a
while Trueloop that discards corrupt documents and re-pops the next-most-recent until a valid item is decoded or the session is genuinely empty. The fix is local topop_itemand adds five lines of behavior plus a docstring note.Test plan
tests/extensions/memory/test_mongodb_session.py::test_pop_item_skips_corrupt_most_recent— corrupt latest + valid older returns the older item and leaves the collection empty.tests/extensions/memory/test_mongodb_session.py::test_pop_item_returns_none_when_only_corrupt_docs_remain— all-corrupt session drains every bad row before returningNone.MongoDBSessiontests still pass (33 total now).ruff check/ruff format --checkclean.