Fix device connectivity, bridge protocol hardening, and 60+ integration tests#159
Merged
Fix device connectivity, bridge protocol hardening, and 60+ integration tests#159
Conversation
ChangeModelAsync had no remote mode path — it tried to use the local CopilotClient which is null on mobile, silently failing. The bridge protocol had no message type for model changes. Added the full bridge plumbing: - BridgeMessageTypes.ChangeModel constant - ChangeModelPayload with SessionName + NewModel - IWsBridgeClient.ChangeModelAsync interface method - WsBridgeClient.ChangeModelAsync implementation - WsBridgeServer handler dispatching to CopilotService.ChangeModelAsync - CopilotService.ChangeModelAsync remote mode delegation with optimistic local state update Tests: - ChangeModel message type constant - Round-trip serialization - Payload defaults - CamelCase serialization - AllClientToServerTypes uniqueness updated Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stand up a real WsBridgeServer on localhost, connect a real WsBridgeClient, and verify end-to-end message flows — simulating the mobile → devtunnel → desktop path without needing a real tunnel. Tests cover: connect, create session, change model, abort, close, and session list synchronization. Also fix ChangeModelAsync for demo mode — it was hitting the SDK path which requires a real CopilotClient. Demo mode now just updates the model label locally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ations - 31 integration tests using real WsBridgeServer/Client on localhost - Covers: connection, session CRUD, messaging, content deltas, queuing, model switching, session switching, directory listing, security (path traversal), and all organization commands (create/rename/delete group, pin/unpin, move, toggle collapsed, sort mode, broadcast) - Fix demo mode CreateSessionAsync missing SessionMeta in Organization - Fix StubDemoService to fire content/turn events for realistic testing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bug fixes: - AbortSessionAsync: Add IsDemoMode guard to prevent NRE when Session is null in demo mode - RenameSession: Add full bridge plumbing (message type, payload, client method, server handler, remote mode delegation) so renames propagate over devtunnel instead of silently reverting on next sync - EnqueueMessage: Add IsRemoteMode delegation to forward queued messages to the bridge server - CloseSessionAsync: Change guard from IsConnected to IsRemoteMode for consistency with all other bridge-delegating methods - ListDirectoriesAsync: Replace single _dirListTcs field with ConcurrentDictionary keyed by RequestId to prevent race condition when concurrent directory listing calls corrupt each other's state Tests: 9 new tests covering all 5 fixes including concurrent directory listing, demo mode abort, rename via bridge, and protocol round-trip serialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Root cause: SyncRemoteSessions updated IsProcessing and MessageCount from server broadcasts but NOT the Model field. When ChangeModelAsync succeeded on the server and broadcast the updated sessions list, the mobile client ignored the model update for existing sessions. Also fixed: - WsBridgeServer ChangeModel handler: check return value, send error to client on failure, and always broadcast sessions list for sync - SessionErrorEvent: reset ActiveToolCallCount and HasUsedToolsThisTurn to match abort/watchdog cleanup (prevents stale tool count on next turn) - DeletePersistedSession: add IsDemoMode guard to prevent deleting real session data from disk while in demo mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SaveActiveSessionsToDisk now skips writes in demo mode and during restore, preventing integration tests from overwriting the real ~/.polypilot/active-sessions.json file with empty/partial data. Also fix SessionDisposalResilienceTests to account for demo service now generating assistant responses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
InitializeRemoteAsync now waits for SessionsList and SessionHistory messages to arrive from the server before returning. Previously, ConnectAsync returned as soon as the WebSocket handshake completed, but the receive loop hadn't processed the initial state yet. The Dashboard's RefreshState guard (_initializationComplete) then dropped all OnStateChanged events during initialization, so history synced by SyncRemoteSessions was never rendered until the next user action. Now we poll for sessions and history before returning, then explicitly call SyncRemoteSessions to ensure all history is in local session objects before the Dashboard renders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bugs fixed: - SessionHistories changed from Dictionary to ConcurrentDictionary (was read/written from 2 threads, risked InvalidOperationException) - close_session handler now guards against empty SessionName (was the only handler missing this check) - URL normalization now handles ws:// and wss:// schemes without double-prefixing (was producing wss://ws://...) New test coverage (23 tests): - ResumeSession: GUID validation, non-existent session error - Broadcast events: TurnStart/TurnEnd reach client - ChangeModel guards: processing, non-existent, same, empty - Multi-client: both receive sessions list and content deltas - Close session: empty name ignored - URL normalization: ws:// and http:// connect correctly - DiffParser: 7 tests covering all parse paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- WsBridgeClient: Add SemaphoreSlim around SendAsync to prevent concurrent WebSocket sends (ClientWebSocket prohibits this) - WsBridgeServer: Fix Broadcast semaphore Release/Dispose ordering to prevent ObjectDisposedException and leaked waiters - RepoManager: Fix RepoIdFromUrl stripping .git from middle of repo names by only removing trailing .git suffix - WsBridgeIntegrationTests: Replace fixed Task.Delay with polling loops (WaitForAsync helper) for reliability under parallel test load - Add RepoManagerTests with 10 tests covering URL parsing and normalization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WsBridgeClient: - Fix ReconnectAsync CTS race: capture CTS as local to prevent ConnectAsync from replacing it mid-loop, causing dual connections - Fix HttpMessageInvoker leak on successful connection path - Dispose _sendLock in Dispose() to prevent SemaphoreSlim leak WsBridgeServer: - Fix Broadcast ObjectDisposedException: move WaitAsync inside try block so Stop()-disposed semaphores don't cause unobserved task exceptions - Add BroadcastSessionsList after SwitchSession for multi-client sync - Guard BuildSessionsListPayload against null _copilot in HandleClientAsync CopilotService: - Fix remote-mode RenameSession: add duplicate name check before overwrite - Fix remote-mode ChangeModelAsync: add IsProcessing and same-model guards - Fix SessionStartEvent: wrap SaveActiveSessionsToDisk in Invoke() to marshal file write to UI thread, preventing concurrent file corruption - Set IsRemoteMode before SyncRemoteSessions in InitializeRemoteAsync to prevent ReconcileOrganization from running unnecessary file I/O RepoManager: ssh:// protocol URLs and credential-embedded HTTPS URLs correctly - Add tests for ssh:// and credential URL parsing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…adcast - TurnEnd broadcast: verify client receives turn_end event after demo prompt - Auth loopback bypass: verify connection succeeds with AccessToken set when connecting via localhost (loopback trusted) - SwitchSession broadcast: verify session list is broadcast after switch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Added HasReceivedSessionsList flag to WsBridgeClient to distinguish 'server replied with zero sessions' from 'server hasn't replied yet'. Previously Sessions.Any() was used which stays false for empty lists, causing a 5-second busy-wait timeout on every connect to an empty server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace Task.Delay(500) with WaitForAsync polling in more bridge integration tests for reliability under parallel test load: - CreateSession_AppearsOnServer - CreateSession_WithModel_SetsModelOnServer - SendMessage_AddsUserMessageToServerHistory - ResumeSession_InvalidGuid_ReturnsErrorToClient - ResumeSession_NonExistentGuid_ReturnsResumeFailedError - ChangeModel_WhileProcessing_SendsErrorToClient All 771 tests pass consistently (5/5 runs clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix UriFormatException crash on malformed polypilot:// QR codes (use Uri.TryCreate) - Fix event handler leak: guard InitializeRemoteAsync to only wire bridge events once - Fix RenameSession remote: check IsConnected, return false when session not found - Fix Broadcast ODE catch: clean up stale client on ObjectDisposedException - Fix WsBridgeClient SendAsync: protect _sendLock against ObjectDisposedException - Fix directory list fallback: only use legacy fallback when RequestId is null - Fix expandedSession sync: keep in sync with active session on mobile - Bigger/clearer direct share QR code (black on white, 280px) - Regenerate direct QR code after IP discovery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix ChangeModelAsync returning true for non-existent remote sessions (4/6 reviewers) - Fix RenameSession remote: add _pendingRemoteSessions protection (3/6 reviewers) - Fix RenameSession server: check return value, send ErrorEvent on failure (3/6 reviewers) - Fix SendToClientAsync: add ODE protection matching Broadcast pattern (2/6 reviewers) - Fix RenameSession fire-and-forget: add error logging continuation - Fix SwitchSession test: assert ActiveSessionName instead of list membership - Fix ListDirectoriesAsync: drain pending requests on receive loop exit - Fix HasHistoryImmediately test: replace hardcoded delay with WaitForAsync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix EnqueueMessage: warn about dropped images in remote mode, add error logging - Fix rename ghost duplicate: track old name in _pendingRemoteRenames to prevent SyncRemoteSessions from re-adding it during the rename window - Fix CancellationTokenSource leak: dispose old CTS in Stop() before creating new - Skip re-adding old names from pending renames in SyncRemoteSessions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Wrap remote ChangeModelAsync bridge call in try/catch to match local path behavior (callers don't expect exceptions from this method) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
HandleComplete is called from CompleteResponse which is already marshaled to the UI thread via Invoke(SynchronizationContext.Post). The InvokeAsync wrapper queued a second deferral, causing StateHasChanged() to run too late — after other RefreshState calls consumed the dirty flag, so the DOM never updated with IsProcessing=false. Keep InvokeAsync only on the 10-second delayed cleanup callback which runs on the thread-pool via Task.Delay.ContinueWith. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Source-scanning test that verifies HandleComplete in Dashboard.razor has at most one InvokeAsync call (the delayed cleanup only), and that call must be inside a Task.Delay continuation. If someone wraps the body in InvokeAsync again, the test fails with an explanatory message about why it causes stuck 'Thinking' indicators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
changed the title
PureWeen
added a commit
that referenced
this pull request
Feb 20, 2026
Resolve 3 conflicts: - BridgeMessages.cs: keep both multi-agent + change-model/rename constants - CopilotService.cs: take main's fresh TCS + On() ordering, keep our ProcessingGeneration/HasUsedToolsThisTurn carry-forward - WsBridgeClient.cs: keep multi-agent methods + main's ConcurrentDictionary 4 pre-existing DiffParser test failures from main PR #159 (not ours). 1014 tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Summary
Fixes device connectivity issues (mobile session switching, QR code scanning, model sync), hardens the WebSocket bridge protocol against race conditions and resource leaks, and adds 60+ integration tests with real WebSocket connections. All findings from 3 rounds of multi-model code review (Opus 4.6, Sonnet 4.6, Codex 5.3) have been addressed.
21 files changed, +1,879 -72 lines | 801 tests passing
Bridge Protocol Hardening
SendToClientAsync,WsBridgeClient.SendAsync, andBroadcast- preventsObjectDisposedExceptioncrashes when semaphores are disposed during concurrent shutdownErrorEventon failure (matchingChangeModelpattern)_pendingRemoteSessionsprotection to preventSyncRemoteSessionsfrom pruning optimistic renames, plus error logging on fire-and-forgetfalsefor non-existent remote sessions instead of silently succeedingRequestId(legacy server), not unknown IDs_dirListRequestson disconnect so callers don't hang for 10s_bridgeEventsWiredguard on reconnectDevice Connectivity Fixes
change_modelcommandexpandedSessionstays in sync with active sessionpolypilot://URIs and JSON formats withUri.TryCreate(no more crashes on malformed input)InvokeAsyncwrapper that deferredStateHasChanged()causing stuck "Thinking" indicatorsNew Tests (60+ integration tests)
WsBridgeIntegrationTests.cs- real localhost WebSocket server+client pairs covering: state sync, session lifecycle (create/close/rename/switch), message flow (send/queue/turn events), model changes, organization commands, directory listing, auth, history sync, attention events, error propagationRepoManagerTests.cs- URL parsing for HTTPS/SSH repo IDs and normalizationHandleComplete_MustNotUseInvokeAsync_RegressionGuard- source-scanning test that prevents re-wrapping HandleComplete in InvokeAsyncTask.DelaywithWaitForAsyncpolling in flaky testsSwitchSessiontest false positive (assertsActiveSessionName, not list membership)Multi-Model Review Summary