feat: Add create_session and get_session classmethods#2812
feat: Add create_session and get_session classmethods#2812ecanlar wants to merge 4 commits intoopenai:mainfrom
Conversation
Introduces user-session association across all SQL-based session backends (SQLiteSession, AsyncSQLiteSession, SQLAlchemySession), similar to how Google's ADK models the User → Session relationship. Changes: - Add agent_users table with user_id, metadata, and timestamps - Add user_id foreign key to agent_sessions table - Add optional user_id parameter to session constructors - Add get_sessions_for_user() method to query sessions by user - Add user_id attribute to Session protocol and SessionABC - Add tests for user association functionality Closes openai#2808 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds limit and offset parameters to get_sessions_for_user() across all three SQL backends, consistent with how get_items() supports limiting retrieved history. This enables paginated retrieval of user sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ends Introduces two new classmethods for all SQL-based session backends: - create_session(user_id, ...): Creates a new session with an auto-generated UUID session_id, persisting user and session rows immediately. - get_session(user_id, session_id, ...): Retrieves an existing session, verifying it belongs to the given user. Returns None if not found. This builds on the agent_users table from openai#2809 and provides a proper factory pattern where the session_id is generated internally rather than requiring the caller to invent one. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65d97e10e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| users_table: str = "agent_users", | ||
| user_id: str | None = None, | ||
| session_settings: SessionSettings | None = None, |
There was a problem hiding this comment.
Preserve SQLiteSession positional argument order
Inserting users_table and user_id before session_settings changes the meaning of existing positional calls. Code that previously did SQLiteSession(session_id, db_path, sessions_table, messages_table, SessionSettings(...)) will now pass the SessionSettings object as users_table, which later produces invalid SQL/table-name behavior at runtime. This is a backward-incompatible API break for existing callers that used positional arguments.
Useful? React with 👍 / 👎.
src/agents/memory/session.py
Outdated
| @classmethod | ||
| @abstractmethod | ||
| async def create_session(cls, user_id: str, **kwargs: object) -> SessionABC: |
There was a problem hiding this comment.
Avoid making existing session backends abstract
Adding create_session/get_session as abstract methods on SessionABC makes every subclass that doesn't implement them uninstantiable. In this commit, backends like OpenAIConversationsSession, RedisSession, DaprSession, and EncryptedSession still do not define these methods, so constructing them will raise TypeError for abstract methods. This is an immediate runtime regression for those session backends.
Useful? React with 👍 / 👎.
| INSERT OR IGNORE INTO {self.sessions_table} (session_id, user_id) | ||
| VALUES (?, ?) |
There was a problem hiding this comment.
Migrate preexisting SQLite schemas before writing user_id
This write path now always inserts into (session_id, user_id), but schema setup only uses CREATE TABLE IF NOT EXISTS and does not alter existing agent_sessions tables. For users upgrading with an existing SQLite DB created by earlier versions (without user_id), add_items() will fail with sqlite3.OperationalError: table ... has no column named user_id on first write.
Useful? React with 👍 / 👎.
|
For the reason I mentioned at #2808 (comment), we don't plan to have this change at least for now. |
…ods from SessionABC - Rename get_session → get_sessions: returns all sessions for a user as a list of session instances (not just IDs), with limit/offset support - Remove create_session/get_session abstract methods from SessionABC to avoid forcing all implementations to support user_id - Update tests to cover get_sessions, pagination, and multi-user scenarios Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
create_session(user_id, ...)classmethod to all SQL-based session backends (SQLiteSession, AsyncSQLiteSession, SQLAlchemySession) that creates a new session with an auto-generated UUIDsession_id, persisting user and session rows immediatelyget_sessions(user_id, ...)classmethod that retrieves all sessions for a user as a list of ready-to-use session instances, withlimit/offsetpagination supportSessionABCor theSessionprotocol — these methods are backend-specific conveniencesAPI
Motivation
Builds on #2809 (agent_users table). Currently the caller must invent a
session_idand the session row is only created lazily onadd_items(). This PR provides a proper factory pattern where:user_idis the primary entry point (not thesession_id)session_idis generated internally viauuid4()get_sessionsreturns fully functional session instances (not just IDs likeget_sessions_for_user)Test plan
test_sqlite_create_session— verifies UUID generation, DB persistence, and normal usagetest_sqlite_get_sessions— verifies retrieval of all user sessions, item preservation, empty result for unknown usertest_sqlite_get_sessions_pagination— verifies limit/offset paginationtest_sqlite_create_multiple_sessions_for_user— verifies multiple sessions per user with unique IDs🤖 Generated with Claude Code