fix: Port session id validation, thread id scoping and Vertex RAG cleanup fixes to v1 - #6809
Merged
Conversation
…ssion (v1) Port of upstream commit b2916c7 (PR #5580) to the v1 branch. VertexAiSessionService.delete_session accepted a user_id argument and never read it, so the caller's identity had no bearing on which session was deleted; get_session has checked ownership since it was written. delete_session now fetches the session first, returns silently if it is already gone, and raises ValueError when the session belongs to a different user. Session ids were also interpolated straight into the resource path. A new _validate_session_id rejects anything outside ^[A-Za-z0-9_-]+$ in create_session, get_session, delete_session and append_event, so an id can no longer contribute extra path segments. Behaviour changes for existing users: a caller-chosen session_id containing a dot, a colon or any non-ASCII character is now rejected with ValueError, where before it was passed through. Server-generated Vertex session ids are numeric and unaffected. delete_session now costs one extra get round trip, and a missing session returns without raising instead of surfacing the delete's 404.
Port of upstream commit 19e2a72 to the v1 branch. LangGraphAgent passed the bare session id to the LangGraph checkpointer as its thread id. Session ids are caller-chosen and only unique within an (app name, user id) pair, so two users who picked the same session id read and wrote each other's graph state, and so did two apps. The thread id is now the SHA-256 digest of the app name, user id and session id together. Each component is length-prefixed before hashing, so a component containing the separator cannot stand in for a different triple, and hashing keeps the user id out of checkpointer storage. Behaviour change for existing users: checkpoints written by earlier releases are keyed on the old thread id and are not reused. With a persistent checkpointer, the first turn after upgrading resumes from empty graph state, and because the adapter sends only the last human message when a checkpointer is configured, the history that lived in the checkpointer is gone rather than re-sent. There is no migration path. Reading the old thread id as a fallback would mean reading the ambiguous key this change exists to remove, so it would reintroduce the cross-user collision. Affected users should finish in-flight conversations before upgrading.
…p (v1) Adapted from upstream commit 80a05b7. That commit rewrote this service onto a client SDK the v1 branch does not have, so the code here is written against v1's module-level vertexai RAG surface rather than cherry-picked. VertexAiRagMemoryService.add_session_to_memory writes the full session transcript to a plaintext file in the system temp directory before uploading it. The removal was the last statement of the happy path, so any upload that raised, or a cancelled task, left the transcript on disk indefinitely. The write and the uploads now sit in a try whose finally removes the file, tolerating FileNotFoundError, and the path is recorded before the write so a failed write is cleaned up too. The file is opened with an explicit utf-8 encoding instead of the locale default. Corpus names are also validated before the file is created rather than after, so a misconfigured service writes nothing at all. Both methods are declared async but called the synchronous RAG SDK, so every upload and retrieval blocked the event loop for the whole HTTP round trip. Both calls now run through asyncio.to_thread. This is the same two functions the transcript cleanup rewrites, which is why it is one change. to_thread copies the current contextvars context, so tracing spans still propagate into the worker thread, and both SDK entry points read the module globals set by vertexai.init and build a fresh client per call, so neither is thread-affine. Behaviour change for existing users: VertexAiRagMemoryService() constructed with no rag_corpus now raises ValueError("rag_corpus must be set on every RAG resource.") before doing any work. Previously the guard it replaces could never fire, because __init__ always builds a one-element rag_resources list even when rag_corpus is None, so that configuration wrote the transcript to disk and then failed inside the SDK with corpus_name=None. The CLI factory always passes a corpus, so it is unaffected. Separately, the SDK call now runs on a worker thread, which anyone substituting a thread-hostile fake for rag.upload_file or rag.retrieval_query would notice.
… (v1) Port of upstream commit fd8f7eb to the v1 branch. InMemoryMemoryService kept its per-user event store under the string f'{app_name}/{user_id}'. That flattening is not injective: an app named 'acme/alice' with user 'bob' produced the same key as app 'acme' with user 'alice/bob', so the two shared a store and each could read the other's sessions. The key is now the (app_name, user_id) tuple, which cannot be aliased. The store is private and in-memory, so nothing persists across a restart and there is no stored format to migrate. The only observable change is for a caller or test reaching into service._session_events with a string key.
GWeale
changed the title
DeanChensj
approved these changes
Aug 19, 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.
This PR ports four fixes to the
v1branch:fix: validate session_id and enforce ownership in delete_session(upstreamb2916c71)VertexAiSessionService.delete_sessionfetches the session and raisesValueErrorwhen the storeduser_iddiffers from the caller's.session_idmust match^[A-Za-z0-9_-]+$on create, get, delete and append.fix: scope LangGraph checkpointer thread id to app and user(upstream19e2a728)fix(memory): make Vertex RAG uploads async-safe(re-implemented from upstream80a05b7f)finallyblock.VertexAiRagMemoryServiceraisesValueErrorat construction when a RAG resource has norag_corpus.asyncio.to_thread.fix: key in-memory memory store by app_name and user_id tuple(upstreamfd8f7eb2)InMemoryMemoryServicekeys its event store on the(app_name, user_id)tuple.Item 3 is a re-implementation rather than a port.