Skip to content

feat(py): FirestoreSessionStore (google-cloud plugin)#5685

Open
huangjeff5 wants to merge 15 commits into
jh-py-agentsfrom
jh-py-agents-firestore-session
Open

feat(py): FirestoreSessionStore (google-cloud plugin)#5685
huangjeff5 wants to merge 15 commits into
jh-py-agentsfrom
jh-py-agents-firestore-session

Conversation

@huangjeff5

Copy link
Copy Markdown
Contributor

Adds a Firestore-backed agent session store. PR 1 of 3 in the FastAPI-agents stack; PR2 (serve_agent/serve_flow) and PR3 (sample) build on this.

@github-actions github-actions Bot added docs Improvements or additions to documentation python Python labels Jul 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for agent snapshots in the Google Cloud plugin, along with corresponding unit tests. Feedback on the implementation highlights several critical improvements: unsubscribing from the Firestore snapshot listener upon reaching a terminal status to prevent resource leaks, ensuring only a single listener is started per snapshot ID, querying Firestore directly by sessionId instead of streaming the entire collection, and correcting the _sanitize function to properly filter out None values.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5 huangjeff5 force-pushed the jh-py-agents-firestore-session branch from 9efc34b to 0feada2 Compare July 6, 2026 05:34
@github-actions github-actions Bot added the config label Jul 6, 2026
@huangjeff5 huangjeff5 force-pushed the jh-py-agents-firestore-session branch from 07ad162 to 16ffefc Compare July 8, 2026 19:59
@huangjeff5

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for agent snapshots, adding the google-cloud-firestore dependency and implementing the store across both the genkit-google-cloud package and the google-cloud plugin, along with corresponding unit tests. The review feedback correctly identifies a critical concurrency issue in the listener implementation of both files, where calling loop.create_task from a background thread is not thread-safe and a race condition exists when unsubscribing from the snapshot listener. Applying the suggested thread-safe scheduling and unsubscription fixes is highly recommended.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud, along with corresponding unit tests and updated sample imports. Feedback highlights critical runtime issues in _start_listener due to unsupported async on_snapshot calls and callback signature mismatches. Additionally, it is recommended to use async transactions and server-side timestamps in _maybe_update_pointer to prevent concurrency race conditions, and to update the mock tests to reflect the correct SDK behavior.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread py/packages/genkit-google-cloud/tests/firestore_session_store_test.py Outdated
@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for agent snapshots in both the genkit-google-cloud and google-cloud plugins, and updates various agent samples to use the correct package imports. The review feedback highlights a performance and resource leak issue in _start_listener, recommending the lazy initialization and reuse of a single synchronous firestore.Client instance to avoid leaking gRPC channels, as well as updating the deprecated asyncio.get_event_loop() call to asyncio.get_running_loop().

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud Firestore. It includes the implementation of the store, associated unit tests, and dependency updates in pyproject.toml and uv.lock. Additionally, it updates several agent sample files to use the correct imports for GoogleAI and middleware components. No review comments were provided, so there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

1 similar comment
@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for agent snapshots in the genkit-google-cloud package, including comprehensive unit tests and updated import paths in several sample files. The review feedback highlights a potential issue where instantiating asyncio.Lock() directly in init can raise a RuntimeError if called outside of an active event loop, and suggests lazily initializing the lock to resolve this.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@huangjeff5 huangjeff5 force-pushed the jh-py-agents-firestore-session branch from 01779a6 to e72c75a Compare July 9, 2026 19:10

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud, along with corresponding unit tests. It also updates several sample files to use the new standalone package imports for GoogleAI and middleware. The review feedback focuses on improving the robustness of the session store listener subscription by handling exceptions during startup, and enhancing test coverage to properly exercise the synchronous fallback path when the async document reference lacks the on_snapshot attribute.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread py/packages/genkit-google-cloud/tests/firestore_session_store_test.py Outdated
Comment thread py/packages/genkit-google-cloud/tests/firestore_session_store_test.py Outdated
@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots, along with corresponding unit tests and dependency updates. It also refactors imports in several sample agent files. The code review identified a critical bug in the pointer update logic where the session pointer is not updated on subsequent turns because it compares the current pointer to the new snapshot ID instead of the parent snapshot ID. Additionally, the reviewer suggested optimizing the real-time listener to skip starting for snapshots already in a terminal status, and recommended adding a test case to verify pointer updates for existing sessions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud, along with corresponding unit tests and import cleanups across several sample files. The review feedback highlights a critical issue in the pointer update logic during snapshot saves: using a simple boolean flag (is_new) prevents the session pointer from updating when appending to an existing session. To resolve this, the reviewer suggests implementing a proper compare-and-swap mechanism by passing the parent snapshot ID to maybe_update_pointer and adding a test case to verify appending to an existing session.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud Firestore, along with corresponding unit tests and updates to sample imports. The feedback recommends preserving client_options when falling back to the synchronous Firestore client and running the blocking unsubscribe() call in an executor to prevent blocking the asyncio event loop.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Firestore-backed session store (FirestoreSessionStore) for persisting agent snapshots in Google Cloud Firestore. It includes the implementation of the store, associated unit tests, and updates to the project's dependencies (pyproject.toml and uv.lock). Additionally, several sample agent files have been updated to use the correct import paths for genkit_google_genai and genkit_middleware. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@huangjeff5

Copy link
Copy Markdown
Contributor Author

FirestoreSessionStore End-to-End & Bug-Bash Audit Report

Date: 2026-07-09
Target Package: genkit-google-cloud (FirestoreSessionStore)
Target Repository & PR: genkit-ai/genkit PR #5685 (commit a6697d5a9)
Test Environments:

  1. In-memory deterministic simulation (MockFirestoreEngine supporting compare-and-swap transactions, queries, and watch streams).
  2. Live Google Cloud Firestore service (project: aim-testing, collections: genkit-e2e-sessions / genkit-e2e-sessions-pointers).

1. Executive Summary

We conducted an exhaustive bug-bash and end-to-end (E2E) integration audit of FirestoreSessionStore, evaluating its conformance with the Genkit Python Agents runtime (Agent, Session, load_session, AgentChat, tool calls, detached background tasks, interrupts, and real-time status subscriptions).

During edge-case stress testing, we discovered and remediated 2 critical bugs in FirestoreSessionStore. Both test suites (firestore_session_store_bug_bash_test.py and firestore_session_store_e2e_test.py) have been added to the permanent test suite and pushed to PR #5685. All 83 tests across genkit-google-cloud pass cleanly in 13.48s alongside 100% compliance with ruff, pyright, pyrefly, and ty.


2. Bugs Discovered & Remediated

Bug 1: AsyncDocumentReference.on_snapshot NotImplementedError Crash

  • Symptom: Calling on_snapshot_status_change(snapshot_id) when the store was initialized with an AsyncClient crashed with NotImplementedError from google.cloud.firestore_v1.base_document.py:359.
  • Root Cause: In the Google Cloud Firestore Python SDK, AsyncDocumentReference defines on_snapshot(self, callback) solely to raise NotImplementedError, as real-time watch listeners are currently only implemented on synchronous clients. Our fallback condition checked if not hasattr(ref, 'on_snapshot'):, which evaluated to True on AsyncDocumentReference and attempted to execute the unimplemented method directly.
  • Remediation: Updated start_listener in firestore.py to check if isinstance(self.client, firestore.AsyncClient) or not hasattr(ref, 'on_snapshot'):. When an async client is provided, the store automatically initializes and caches self.sync_client with matching credentials and client options to manage real-time watch streams safely.

Bug 2: Terminal Snapshot Listener Queue Hang

  • Symptom: When on_snapshot_status_change subscribed to a snapshot that was already in a terminal status (COMPLETED, ABORTED, FAILED, EXPIRED), consumer loops (such as AgentRuntime.watch_snapshot_abort) hung indefinitely awaiting queue items.
  • Root Cause: In on_snapshot_status_change, if a snapshot was already terminal, start_listener(snapshot_id) was intentionally bypassed to conserve gRPC resources. However, subscribe only seeded the queue with the initial status (e.g., ABORTED). Because the listener was never started, notify(self.subs, snapshot_id, None) was never invoked to send the terminating sentinel (None). Consumers expecting None to signal stream termination hung forever on await queue.get().
  • Remediation: Updated on_snapshot_status_change to inspect the initial status upon subscription. If current.status in TERMINAL_STATUSES, the store immediately puts None into the queue following the status seed, pops snapshot_id from self.subs, and returns without opening a gRPC watch stream.

3. In-Memory Bug-Bash Suite (firestore_session_store_bug_bash_test.py)

We built MockFirestoreEngine, an in-memory simulation of Firestore supporting transactional Compare-And-Swap (CAS) operations, field queries (where().stream()), and real-time watcher callbacks. We verified 7 distinct edge-case scenarios:

Test Case Scenario Exercised Verification & Assertions
test_bug_bash_multiturn_linear_chat_pointer_advancement Multiturn linear chat (Turns 1, 2, 3). Verified save_snapshot appends link parent_id sequentially. Verified pointer document currentSnapshotId advances atomicaly from Turn 1 -> Turn 2 -> Turn 3. Verified get_snapshot(session_id=...) returns Turn 3 tip.
test_bug_bash_branching_and_forked_history History forking (Branch A tip vs Branch B fork from Root). Verified appending Branch B to Root leaves pointer intact at Branch A tip (CAS isolation). Verified un-pointed fallback with reject_ambiguous_session=False returns latest leaf chronologically, while reject_ambiguous_session=True raises GenkitError(FAILED_PRECONDITION, 'branching snapshots').
test_bug_bash_cas_concurrent_append_race_condition Concurrent append race condition (Turn 2X and Turn 2Y both targeting Turn 1). Simulated simultaneous transaction execution. Verified Turn 2X commits and updates pointer to turn-2x. Verified Turn 2Y CAS check (pointer == 'turn-1') fails and leaves pointer at turn-2x without overwriting.
test_bug_bash_inplace_update_vs_append In-place snapshot update vs appending new turns. Verified calling save_snapshot(turn1_id, fn) updates status from PENDING to COMPLETED idempotently without altering snapshot ID or creating duplicate records. Verified pointer remains stable.
test_bug_bash_realtime_status_listener_and_cleanup Real-time status subscription and thread-safe listener cleanup. Verified queue receives live transitions (PENDING -> COMPLETED -> None). Verified watch.unsubscribe() is executed in an executor and subscription is popped from store.subs.
test_bug_bash_terminal_status_optimization Subscribing to an already-terminal snapshot (ABORTED). Verified start_listener is bypassed. Verified queue receives ABORTED followed immediately by None sentinel.
test_bug_bash_error_handling_and_invalid_selectors Invalid selector combinations and missing metadata. Verified saving without sessionId raises GenkitError(INVALID_ARGUMENT). Verified calling get_snapshot with 0 or 2 selectors raises GenkitError(INVALID_ARGUMENT, 'exactly one of').

4. Live Google Cloud Firestore E2E Suite (firestore_session_store_e2e_test.py)

We executed end-to-end integration tests against the live Google Cloud Firestore service on project aim-testing using tenant-isolated prefixes (test-e2e-{uuid4().hex[:8]}). All test data was verified directly via Firestore DocumentReference queries and wiped clean during teardown.

Workflow 1: E2E Agent Workflow & Tool Execution (test_firestore_session_store_e2e_agent_workflow_and_tools)

  • Execution: Defined e2eAgent with a custom calculator tool (input_val * 2). Executed Turn 1 ("Calculate 21 * 2") and Turn 2 ("What was the answer again?") via AgentChat.
  • Database Verification:
    • Queried genkit-e2e-sessions/global/snapshots/{turn1_id} directly in Firestore. Verified status: 'completed', sessionId, and exact message history structure: 4 parts durably stored (role: 'user', role: 'model' with ToolRequestPart, role: 'tool' with ToolResponsePart output 42, and role: 'model' final reply).
    • Queried genkit-e2e-sessions-pointers/global/pointers/{session_id} in Firestore. Verified currentSnapshotId pointed to Turn 1 after Turn 1, and atomically advanced to Turn 2 after Turn 2.
    • Verified Turn 2 document in Firestore correctly referenced parentId: {turn1_id} and contained all 6 accumulated message parts.

Workflow 2: E2E Detach, Abort, and Real-Time gRPC Watch Stream (test_firestore_session_store_e2e_detach_abort_and_watch)

  • Execution: Configured slowAgent with a 4-second delay. Initiated a detached background turn via task = await chat.detach('Run slow background analysis'). Subscribed to real-time status updates via e2e_store.on_snapshot_status_change(task.snapshot_id) over a live gRPC watch stream against Google Cloud Firestore.
  • Database & Stream Verification:
    • Verified initial status received from watch stream and direct DB query was PENDING.
    • Executed client-side abort via await task.abort().
    • Verified live gRPC watch stream pushed SnapshotStatus.ABORTED followed by sentinel None.
    • Queried Firestore document directly; verified durable status transitioned to aborted.
    • Verified awaiting the aborted turn raised asyncio.CancelledError.
    • Sent follow-up prompt on the same chat; verified conversation resumed cleanly from the last completed turn without stranding history.

Workflow 3: E2E Branching, CAS Isolation, & Strict Ambiguous Rejection (test_firestore_session_store_e2e_branching_and_cas_isolation)

  • Execution: Executed Root turn -> Branch A turn. Verified pointer in Firestore pointed to Branch A tip. Called agent.load_chat(snapshot_id=root_id) and executed Branch B turn ("Branch B prompt").
  • Database Verification:
    • Queried Branch B document in Firestore; verified parentId matched Root ID.
    • Verified pointer in Firestore remained safely at Branch A tip due to CAS isolation (currentSnapshotId == root_id evaluated to False when pointer was at Branch A).
    • Deleted pointer document in Firestore to simulate a legacy/un-pointed session with multiple branch tips.
    • Initialized strict_store with reject_ambiguous_session=True. Attempted to load session via strict_agent.load_chat(session_id=session_id).
    • Verified Firestore query detected 2 leaf documents and raised GenkitError(FAILED_PRECONDITION) matching 'branching snapshots'.

5. Verification & Test Run Logs

uv run --no-sync pytest packages/genkit-google-cloud/tests -v --no-cov
============================= test session starts ==============================
platform darwin -- Python 3.12.9, pytest-9.0.2, pluggy-1.6.0
rootdir: /Users/huangjeff/Desktop/genkit-firestore-session/py
configfile: pyproject.toml
plugins: anyio-4.12.1, mock-3.15.1, Faker-40.4.0, asyncio-1.3.0, typeguard-4.5.0, cov-7.0.0
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=session, asyncio_default_test_loop_scope=function
collecting ... collected 83 items

packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_multiturn_linear_chat_pointer_advancement PASSED [  1%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_branching_and_forked_history PASSED [  2%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_cas_concurrent_append_race_condition PASSED [  3%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_inplace_update_vs_append PASSED [  4%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_realtime_status_listener_and_cleanup PASSED [  6%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_terminal_status_optimization PASSED [  7%]
packages/genkit-google-cloud/tests/firestore_session_store_bug_bash_test.py::test_bug_bash_error_handling_and_invalid_selectors PASSED [  8%]
packages/genkit-google-cloud/tests/firestore_session_store_e2e_test.py::test_firestore_session_store_e2e_agent_workflow_and_tools PASSED [  9%]
packages/genkit-google-cloud/tests/firestore_session_store_e2e_test.py::test_firestore_session_store_e2e_detach_abort_and_watch PASSED [ 10%]
packages/genkit-google-cloud/tests/firestore_session_store_e2e_test.py::test_firestore_session_store_e2e_branching_and_cas_isolation PASSED [ 12%]
... [73 existing unit tests passed] ...
======================= 83 passed, 6 warnings in 13.48s ========================

All formatting (ruff format --check), linter rules (ruff check), and type checkers (pyright, pyrefly, ty) pass with 0 errors across genkit-google-cloud.

@huangjeff5 huangjeff5 force-pushed the jh-py-agents-firestore-session branch 2 times, most recently from e1a7574 to cdc333a Compare July 9, 2026 20:10
@huangjeff5 huangjeff5 force-pushed the jh-py-agents-firestore-session branch from cdc333a to d464bd7 Compare July 9, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config docs Improvements or additions to documentation python Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant