Fix session lifecycle bugs and add tests

[dgellow]

DeleteUser now cascades to session cleanup in both memory and Firestore backends, and the admin handler terminates live stdio sessions before deletion. Firestore TrackSession and UpsertUser no longer do racy read-before-write — TrackSession uses a simple Set, UpsertUser uses a transaction. Memory TrackSession respects caller-provided Created timestamps. Admin dashboard filters stale sessions. BrowserCookie gets an IsExpired() method used by middleware.

[gemini-code-assist]

Summary of Changes

Hello @dgellow, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses several critical session lifecycle management issues across both memory and Firestore storage backends. It ensures robust cleanup of user-related sessions upon user deletion, prevents race conditions during user and session updates, and improves the accuracy of the admin dashboard by filtering out stale sessions. The changes enhance data consistency and system reliability.

Highlights

• Session Cleanup Cascade: Implemented cascading session cleanup in both memory and Firestore storage backends when a user is deleted, ensuring all associated sessions are removed.
• Admin Handler Session Termination: Added logic to the admin handler to actively terminate live stdio sessions for a user before their deletion, preventing orphaned sessions.
• Firestore Race Condition Fixes: Refactored Firestore's UpsertUser to utilize transactions and TrackSession to use a direct Set operation, effectively eliminating read-before-write race conditions and improving data consistency.
• Memory Storage Session Creation Timestamp: Ensured that the Memory storage's TrackSession correctly utilizes caller-provided Created timestamps, falling back to time.Now() if not specified.
• Admin Dashboard Stale Session Filtering: Introduced filtering of stale sessions from the admin dashboard based on a configurable timeout multiplier, improving the accuracy of displayed active sessions.
• BrowserCookie Expiration Method: Added an IsExpired() method to the BrowserCookie struct for a clearer and more encapsulated way to check session expiration.
• Comprehensive Session Tests: Included new, comprehensive tests for MemoryStorage to cover session tracking, revocation, and the user deletion cascade.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• internal/server/admin_handlers.go
  • Added staleSessionTimeoutMultiplier constant for session filtering.
  • Filtered stale sessions from the admin dashboard based on inactivity.
  • Implemented termination of active stdio sessions for a user during deletion.
• internal/server/middleware.go
  • Updated session expiration check to use the new IsExpired() method on BrowserCookie.
• internal/session/session.go
  • Introduced IsExpired() method to the BrowserCookie struct.
• internal/session/session_test.go
  • Renamed TestBrowserCookie_Expiry to TestBrowserCookie_IsExpired.
  • Modified tests to utilize the IsExpired() method for checking session expiry.
• internal/storage/firestore.go
  • Refactored UpsertUser to use a Firestore transaction, preventing read-before-write race conditions.
  • Added logic to DeleteUser to iterate and delete all associated sessions.
  • Simplified TrackSession to directly Set the session document, removing the read-before-write pattern.
• internal/storage/memory.go
  • Implemented session deletion cascade within DeleteUser.
  • Modified TrackSession to respect the Created timestamp if provided, otherwise setting it to time.Now().
• internal/storage/storage_test.go
  • Added a new test suite TestMemoryStorageSessions covering TrackSession, RevokeSession, and DeleteUser session cascade.

Activity

• No specific activity (comments, reviews, or progress updates) has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request introduces several important fixes and improvements related to session lifecycle management and user deletion. Key changes include implementing cascading session cleanup for both in-memory and Firestore backends when a user is deleted, ensuring that live stdio sessions are terminated by the admin handler. The Firestore UpsertUser and TrackSession methods have been refactored to use transactions and direct Set operations, respectively, addressing potential race conditions. Additionally, a new IsExpired() method has been added to BrowserCookie for better encapsulation of expiration logic, and the admin dashboard now filters stale sessions. Comprehensive tests have been added for the memory storage session functionality, which is a great addition.

[gemini-code-assist]

If h.storage.GetActiveSessions(r.Context()) returns an error, this error is not logged, and the session termination logic is silently skipped. This could lead to active sessions for a deleted user not being terminated if there's a temporary issue with session retrieval from storage. It's important to log such errors to ensure visibility into potential cleanup failures.

sessions, sessErr := h.storage.GetActiveSessions(r.Context())
		if sessErr != nil {
			log.LogErrorWithFields("admin", "Failed to get active sessions for user deletion", map[string]any{
				"error":     sessErr.Error(),
				"userEmail": targetEmail,
			})
		} else {
			for _, s := range sessions {