fix: optimistic session creation for instant UI feedback#252
Merged
StephaneDelcroix merged 3 commits intomainfrom Mar 3, 2026
Merged
Conversation
cc4a178 to
eed392b
Compare
In local (Persistent/Embedded) mode, CreateSessionAsync blocked the UI during the slow SDK call. The session only appeared in the sidebar after the SDK finished, causing a noticeable delay. Apply the same optimistic UI pattern already used for remote mode: - Add a placeholder AgentSessionInfo with IsCreating=true immediately - Fire OnStateChanged so the sidebar and chat view update instantly - Complete the SDK session creation in the background - On success: update the state with the real CopilotSession - On failure: clean up the placeholder and re-throw Also adds a guard in SendPromptAsync to reject prompts to sessions that are still being created. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose existing session before overwriting in _sessions[name] to prevent leaking the old CopilotSession when a name collision occurs - Save previousActiveSessionName before optimistic add and restore it in the catch block so _activeSessionName is not left dangling after a failed SDK session creation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
If a session is closed while awaiting SDK creation, the returned CopilotSession would be assigned to an orphaned state object and leak permanently (with a dangling event handler). Now checks _sessions.ContainsKey after the await and disposes the SDK session if it was removed during creation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eed392b to
02f4f2f
Compare
StephaneDelcroix
commented
Mar 3, 2026
Collaborator
Author
StephaneDelcroix
left a comment
There was a problem hiding this comment.
✅ Multi-model consensus review (5 models × 2 rounds)
Previous Findings -- All Fixed
| Finding | Status |
|---|---|
🔴 CRITICAL -- Dangling _activeSessionName after SDK creation failure |
✅ FIXED -- previousActiveSessionName saved/restored in catch block |
🔴 CRITICAL -- Silent overwrite of _sessions[name] leaks CopilotSession |
✅ FIXED -- Existing session disposed before overwrite |
| 🟡 MODERATE -- Close-during-create race leaks SDK session | ✅ FIXED -- !_sessions.ContainsKey(name) guard after await CreateSessionAsync, disposes orphaned session |
Summary
Three fix commits address all consensus findings from two review rounds:
fix: optimistic session creation for instant UI feedback-- original featureFix session overwrite leak and dangling _activeSessionName-- fixes both CRITICALsGuard against close-during-create race-- fixes the MODERATE race condition
The IsCreating flag correctly guards SendPromptAsync (line 1825) to prevent prompts during creation. CloseSessionCoreAsync handles null! Session via is not null check. Tests pass (1666/1667, 1 pre-existing).
Action: ✅ Approve
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
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.
Problem
When creating a session in local (Persistent/Embedded) mode, the UI blocked during the slow SDK call (
_client.CreateSessionAsync). The session only appeared in the sidebar after the SDK finished creating it, causing a noticeable delay where the user had no visual feedback.Fix
Apply the same optimistic UI pattern already used for remote mode to local mode:
AgentSessionInfowithIsCreating = trueand add it to_sessionsbefore the SDK callOnStateChangedand set_activeSessionNameso the sidebar and chat view update immediatelyCopilotSession, setIsCreating = false_sessionsand Organization, re-throwAlso adds:
IsCreatingproperty onAgentSessionInfoto signal creating state to the UISendPromptAsyncto reject prompts to sessions still being createdSessionState.Sessionchanged frominittosetto allow updating after optimistic addTests Added
IsCreating_DefaultsToFalse— model property testIsCreating_CanBeSetAndCleared— model property testCreateSessionAsync_DemoMode_IsCreatingIsFalse— demo path doesn't use IsCreatingCreateSessionAsync_DemoMode_SessionActiveImmediately— session is immediately activeCreateSession_BeforeInitialize_NoOptimisticAdd— no placeholder on init failureCreateSessionAsync_EmptyName_Throws— validation before optimistic addAll 1666 tests pass (1 pre-existing unrelated failure in PlatformHelperTests).