diff --git a/PolyPilot.Tests/CodespaceSessionCreationTests.cs b/PolyPilot.Tests/CodespaceSessionCreationTests.cs new file mode 100644 index 0000000000..59bb1b23e9 --- /dev/null +++ b/PolyPilot.Tests/CodespaceSessionCreationTests.cs @@ -0,0 +1,226 @@ +using PolyPilot.Models; +using PolyPilot.Services; +using Microsoft.Extensions.DependencyInjection; + +namespace PolyPilot.Tests; + +/// +/// Regression tests for codespace session creation. +/// Covers: missing-client guard, descriptive error messages, and health check +/// stale-client detection conditions. +/// Bug: clicking "New Session" on a connected codespace group silently did nothing +/// because (1) the async Task was fire-and-forget (void lambda), (2) errors were +/// not displayed in the codespace section, and (3) the health check didn't detect +/// dead CopilotClients behind a live SSH tunnel. +/// +public class CodespaceSessionCreationTests +{ + private readonly StubChatDatabase _chatDb = new(); + private readonly StubServerManager _serverManager = new(); + private readonly StubWsBridgeClient _bridgeClient = new(); + private readonly StubDemoService _demoService = new(); + private readonly RepoManager _repoManager = new(); + private readonly IServiceProvider _serviceProvider; + + public CodespaceSessionCreationTests() + { + var services = new ServiceCollection(); + _serviceProvider = services.BuildServiceProvider(); + } + + private CopilotService CreateService() => + new CopilotService(_chatDb, _serverManager, _bridgeClient, _repoManager, _serviceProvider, _demoService); + + [Fact] + public async Task CreateSessionAsync_CodespaceGroup_NoClient_ThrowsDescriptiveError() + { + // Regression: CreateSessionAsync must throw a clear error when the codespace + // client is missing from _codespaceClients, rather than falling through to the + // generic "Service not initialized" error which maps to a misleading message. + var svc = CreateService(); + var group = new SessionGroup + { + Name = "test-cs", + CodespaceName = "fuzzy-space-guide", + CodespaceRepository = "org/repo", + ConnectionState = CodespaceConnectionState.Connected + }; + svc.Organization.Groups.Add(group); + + var ex = await Assert.ThrowsAsync( + () => svc.CreateSessionAsync("TestSession", groupId: group.Id)); + + // Must mention the codespace name — not "Service not initialized" + Assert.Contains("not connected", ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains(group.Name, ex.Message); + } + + [Fact] + public async Task CreateSessionAsync_CodespaceGroup_NoClient_DoesNotLeakSessionState() + { + // Regression: When CreateSessionAsync throws for a codespace group, the optimistic + // session should NOT be left in Organization.Sessions or _sessions. + var svc = CreateService(); + var group = new SessionGroup + { + Name = "test-cs", + CodespaceName = "fuzzy-space-guide", + CodespaceRepository = "org/repo", + ConnectionState = CodespaceConnectionState.Connected + }; + svc.Organization.Groups.Add(group); + + await Assert.ThrowsAsync( + () => svc.CreateSessionAsync("LeakedSession", groupId: group.Id)); + + // The guard fires BEFORE the optimistic add, so no cleanup needed + Assert.DoesNotContain(svc.Organization.Sessions, m => m.SessionName == "LeakedSession"); + Assert.Null(svc.GetSession("LeakedSession")); + } + + [Fact] + public async Task CreateSessionAsync_NonCodespaceGroup_SkipsCodespaceGuard() + { + // A non-codespace group should NOT hit the codespace client check. + // It should fall through to the normal "Service not initialized" error + // (since we don't call InitializeAsync in this test). + var svc = CreateService(); + var group = new SessionGroup { Name = "regular-group" }; + svc.Organization.Groups.Add(group); + + var ex = await Assert.ThrowsAsync( + () => svc.CreateSessionAsync("TestSession", groupId: group.Id)); + + // Should NOT mention codespace — it's a regular initialization error + Assert.DoesNotContain("Codespace", ex.Message); + Assert.Contains("initialized", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void HealthCheck_StaleClientCondition_TunnelAliveButPortUnreachable() + { + // Regression: The health check previously only checked _codespaceClients.ContainsKey() + // which returns true even when the CopilotClient is broken. The new TCP probe + // detects when the remote port is unreachable and removes the stale client. + // + // This test validates the model-level conditions that trigger reconnection. + var group = new SessionGroup + { + Name = "test-cs", + CodespaceName = "test-codespace", + CodespaceRepository = "org/repo", + ConnectionState = CodespaceConnectionState.Connected + }; + + // Scenario: tunnel alive, client exists, but port unreachable + bool tunnelAlive = true; + bool clientExists = true; + bool portReachable = false; + + // Old behavior: would skip (continue) because all three were true + bool oldBehaviorSkips = tunnelAlive && clientExists + && group.ConnectionState == CodespaceConnectionState.Connected; + Assert.True(oldBehaviorSkips, "Old health check would have skipped this group"); + + // New behavior: TCP probe fails → stale client removed → reconnect triggered + if (tunnelAlive && clientExists && group.ConnectionState == CodespaceConnectionState.Connected) + { + if (!portReachable) + { + clientExists = false; // Stale client removed + } + } + + Assert.False(clientExists, "Stale client should have been removed after failed TCP probe"); + + // With clientExists=false, health check proceeds to reconnect logic + bool shouldReconnect = !(tunnelAlive && clientExists + && group.ConnectionState == CodespaceConnectionState.Connected); + Assert.True(shouldReconnect, "Health check should proceed to reconnect path"); + } + + [Fact] + public void HealthCheck_HealthyGroup_TunnelAliveAndPortReachable_Skips() + { + // When everything is truly healthy, the health check should skip the group. + var group = new SessionGroup + { + Name = "test-cs", + CodespaceName = "test-codespace", + ConnectionState = CodespaceConnectionState.Connected + }; + + bool tunnelAlive = true; + bool clientExists = true; + bool portReachable = true; + + bool shouldSkip = tunnelAlive && clientExists && portReachable + && group.ConnectionState == CodespaceConnectionState.Connected; + + Assert.True(shouldSkip, "Truly healthy group should be skipped"); + } + + [Fact] + public void QuickCreateGuard_DisconnectedState_BlocksCreation() + { + // Regression: QuickCreateSessionForCodespace should block session creation + // for any state other than Connected, and provide a clear error message. + var states = new[] + { + CodespaceConnectionState.Unknown, + CodespaceConnectionState.Reconnecting, + CodespaceConnectionState.CodespaceStopped, + CodespaceConnectionState.StartingCodespace, + CodespaceConnectionState.WaitingForCopilot, + CodespaceConnectionState.SetupRequired, + }; + + foreach (var state in states) + { + var group = new SessionGroup + { + CodespaceName = "test", + ConnectionState = state + }; + + bool shouldBlock = group.ConnectionState != CodespaceConnectionState.Connected; + Assert.True(shouldBlock, + $"State {state} should block session creation"); + } + } + + [Fact] + public void QuickCreateGuard_ConnectedState_AllowsCreation() + { + var group = new SessionGroup + { + CodespaceName = "test", + ConnectionState = CodespaceConnectionState.Connected + }; + + bool shouldBlock = group.ConnectionState != CodespaceConnectionState.Connected; + Assert.False(shouldBlock, "Connected state should allow session creation"); + } + + [Fact] + public async Task CreateSessionAsync_CodespaceGroup_ErrorMessage_MentionsHealthCheck() + { + // The error message should tell the user that the health check will reconnect + // automatically, guiding them to retry instead of panicking. + var svc = CreateService(); + var group = new SessionGroup + { + Name = "my-codespace", + CodespaceName = "fuzzy-guide", + CodespaceRepository = "org/repo", + ConnectionState = CodespaceConnectionState.Connected + }; + svc.Organization.Groups.Add(group); + + var ex = await Assert.ThrowsAsync( + () => svc.CreateSessionAsync("Test", groupId: group.Id)); + + Assert.Contains("health check", ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("retry", ex.Message, StringComparison.OrdinalIgnoreCase); + } +} diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor b/PolyPilot/Components/Layout/SessionSidebar.razor index 743eafa9a7..2791eb3cef 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor +++ b/PolyPilot/Components/Layout/SessionSidebar.razor @@ -830,7 +830,7 @@ else
@if (!csDisconnected) { - } @@ -932,6 +932,14 @@ else OnFixWithCopilot="() => OpenFixItForSession(sName)" OnAnalyze="() => AnalyzeSessionEfficiency(sName)" /> } + @if (!string.IsNullOrEmpty(_codespaceErrors.GetValueOrDefault(group.Id))) + { + var dismissId = group.Id; + var csError = _codespaceErrors[group.Id]; +
+ ⚠️ @csError +
+ } } } } @@ -1002,9 +1010,11 @@ else private string selectedModel = "claude-opus-4.6"; private string sessionFilter = ""; + private readonly HashSet _creatingGroups = new(); private bool isCreating = false; private string newSessionName = ""; private string? createError = null; + private readonly Dictionary _codespaceErrors = new(); private bool showPersistedSessions = false; private string? renamingSession = null; private string? renamingGroupId = null; @@ -1347,21 +1357,18 @@ else private async Task QuickCreateSessionForCodespace(string groupId) { - if (isCreating) return; + if (_creatingGroups.Contains(groupId)) return; var group = CopilotService.Organization.Groups.FirstOrDefault(g => g.Id == groupId); if (group?.ConnectionState != CodespaceConnectionState.Connected) { - createError = "Codespace is not connected yet. Wait for the green dot before creating a session."; + _codespaceErrors[groupId] = "Codespace is not connected yet. Wait for the green dot before creating a session."; return; } - isCreating = true; - createError = null; + _creatingGroups.Add(groupId); + _codespaceErrors.Remove(groupId); try { - var existingNames = CopilotService.Organization.Sessions - .Where(m => m.GroupId == groupId) - .Select(m => m.SessionName) - .ToHashSet(); + var existingNames = CopilotService.GetAllSessionNames().ToHashSet(); var sessionName = "Main"; if (existingNames.Contains(sessionName)) { @@ -1376,12 +1383,12 @@ else } catch (Exception ex) { - createError = ex.Message; + _codespaceErrors[groupId] = ex.Message; Console.WriteLine($"Error creating codespace session: {ex}"); } finally { - isCreating = false; + _creatingGroups.Remove(groupId); } } diff --git a/PolyPilot/Components/Layout/SessionSidebar.razor.css b/PolyPilot/Components/Layout/SessionSidebar.razor.css index 9a55015e48..3e61098fd2 100644 --- a/PolyPilot/Components/Layout/SessionSidebar.razor.css +++ b/PolyPilot/Components/Layout/SessionSidebar.razor.css @@ -499,6 +499,15 @@ text-overflow: ellipsis; } +.codespace-create-error { + font-size: 0.7rem; + color: var(--error, #e05252); + padding: 0.3rem 0.75rem; + cursor: pointer; + opacity: 0.9; + word-break: break-word; +} + @keyframes pulse-dot { 0%, 100% { opacity: 1; } 50% { opacity: 0.3; } diff --git a/PolyPilot/Services/CopilotService.Codespace.cs b/PolyPilot/Services/CopilotService.Codespace.cs index 0c9030eb88..3b5af6f2da 100644 --- a/PolyPilot/Services/CopilotService.Codespace.cs +++ b/PolyPilot/Services/CopilotService.Codespace.cs @@ -376,8 +376,40 @@ private async Task RunCodespaceHealthCheckAsync(CodespaceService svc, Cancellati bool tunnelAlive = _tunnelHandles.TryGetValue(group.Id, out var tunnel) && tunnel.IsAlive; bool clientExists = _codespaceClients.ContainsKey(group.Id); + // Even when tunnel + client exist, verify the remote end is still reachable + // by probing the tunnel port. The CopilotClient can become stale if the remote + // copilot process died while the SSH tunnel stayed open. + // Note: A simple TCP connect suffices here because SSH tunnels refuse connections + // outright when the remote end isn't listening (unlike `gh cs ports forward` which + // accepts locally). See CodespaceService.IsCopilotListeningAsync for the more + // sophisticated probe needed for non-SSH tunnels. if (tunnelAlive && clientExists && group.ConnectionState == CodespaceConnectionState.Connected) - continue; + { + bool portReachable = false; + try + { + using var probe = new System.Net.Sockets.TcpClient(); + using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + probeCts.CancelAfter(TimeSpan.FromSeconds(3)); + await probe.ConnectAsync("127.0.0.1", tunnel.LocalPort, probeCts.Token); + portReachable = true; + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { /* per-probe timeout — treat as unreachable */ } + catch (OperationCanceledException) { throw; } + catch { /* TCP connect failed — remote end not listening */ } + + if (portReachable) + continue; + + // Client is stale — remove it so the reconnect path kicks in + Debug($"[HEALTH] Codespace '{group.Name}' tunnel alive but remote port {tunnel.LocalPort} not reachable — marking for reconnect"); + if (_codespaceClients.TryRemove(group.Id, out var staleClient)) + { + try { await staleClient.DisposeAsync(); } catch { } + } + clientExists = false; + } // Skip groups where SSH is known-unavailable — user must configure dotfiles first if (group.ConnectionState == CodespaceConnectionState.SetupRequired && group.SshAvailable == false) diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index c9870a34d1..346dc584bf 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -3160,6 +3160,7 @@ public void ClearHistory(string name) } public IEnumerable GetAllSessions() => _sessions.Values.Select(s => s.Info).Where(s => !s.IsHidden); + public IEnumerable GetAllSessionNames() => _sessions.Keys; public int SessionCount => _sessions.Count;