diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/AuthStatusManagerTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/AuthStatusManagerTests.java index 64481d68d..c6efd4ee5 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/AuthStatusManagerTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/AuthStatusManagerTests.java @@ -8,6 +8,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; @@ -35,6 +37,31 @@ void setUp() { authStatusManager = new AuthStatusManager(mockConnection); } + @Test + void testCheckStatus_AccountSwitchWithSameStatus_NotifiesOncePerAccount() { + when(mockConnection.checkQuota()).thenReturn(CompletableFuture.completedFuture(CheckQuotaResult.empty())); + List accounts = new ArrayList<>(); + List notifications = new ArrayList<>(); + authStatusManager.addCopilotAuthStatusListener(status -> accounts.add(status.getUser())); + authStatusManager.addCopilotAuthStatusListener(notifications::add); + CopilotStatusResult alice = new CopilotStatusResult(); + alice.setUser("alice"); + alice.setStatus(CopilotStatusResult.OK); + CopilotStatusResult bob = new CopilotStatusResult(); + bob.setUser("bob"); + bob.setStatus(CopilotStatusResult.OK); + when(mockConnection.checkStatus(false)) + .thenReturn(CompletableFuture.completedFuture(alice), CompletableFuture.completedFuture(bob)); + + authStatusManager.checkStatus(); + authStatusManager.checkStatus(); + authStatusManager.checkStatus(); + + assertEquals(List.of("alice", "bob"), accounts); + assertEquals("alice", notifications.get(0).getUser(), "Queued notifications retain their original identity"); + assertEquals("bob", authStatusManager.getUserName()); + } + @Test void testSignInInitiate() throws InterruptedException, ExecutionException { SignInInitiateResult mockResult = mock(SignInInitiateResult.class); diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProviderTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProviderTests.java index a805907df..643559c05 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProviderTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProviderTests.java @@ -6,14 +6,32 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.io.IOException; import java.util.concurrent.TimeUnit; import org.eclipse.core.runtime.preferences.IEclipsePreferences; import org.eclipse.core.runtime.preferences.InstanceScope; +import org.eclipse.e4.core.contexts.EclipseContextFactory; +import org.eclipse.e4.core.contexts.IEclipseContext; +import org.eclipse.e4.core.services.events.IEventBroker; +import org.eclipse.lsp4j.jsonrpc.messages.NotificationMessage; +import org.eclipse.lsp4j.services.LanguageServer; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; +import org.osgi.framework.BundleContext; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.protocol.InitializationOptions; class LsStreamConnectionProviderTests { @@ -22,6 +40,36 @@ class LsStreamConnectionProviderTests { private static final String UI_PREFERENCE_NODE = "com.microsoft.copilot.eclipse.ui"; private static final long TERMINATION_GRACE_MS = 2000L; + @Test + void testHandleMessage_Initialized_EmitsOncePerProviderAndIgnoresStoppedProviders() { + IEventBroker broker = mock(IEventBroker.class); + IEclipseContext context = mock(IEclipseContext.class); + when(context.get(IEventBroker.class)).thenReturn(broker); + when(broker.post(eq(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED), anyLong())).thenReturn(true); + try (MockedStatic contexts = mockStatic(EclipseContextFactory.class)) { + contexts.when(() -> EclipseContextFactory.getServiceContext(any(BundleContext.class))).thenReturn(context); + LanguageServer server = mock(LanguageServer.class); + NotificationMessage message = new NotificationMessage(); + LsStreamConnectionProvider first = new LsStreamConnectionProvider(); + message.setMethod("didChangeStatus"); + first.handleMessage(message, server, null); + message.setMethod("initialized"); + first.handleMessage(message, server, null); + first.handleMessage(message, server, null); + first.stop(); + first.handleMessage(message, server, null); + LsStreamConnectionProvider stopped = new LsStreamConnectionProvider(); + stopped.stop(); + stopped.handleMessage(message, server, null); + new LsStreamConnectionProvider().handleMessage(message, server, null); + + ArgumentCaptor incarnations = ArgumentCaptor.forClass(Long.class); + verify(broker, times(2)).post(eq(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED), + incarnations.capture()); + assertTrue(incarnations.getAllValues().get(1) > incarnations.getAllValues().get(0)); + } + } + @Test void testInitializationOptions() { LsStreamConnectionProvider provider = new LsStreamConnectionProvider(); diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/AuthStatusManager.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/AuthStatusManager.java index 21284a152..b31f95d67 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/AuthStatusManager.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/AuthStatusManager.java @@ -31,6 +31,7 @@ public class AuthStatusManager { private CopilotLanguageServerConnection connection; private ConcurrentLinkedQueue copilotAuthStatusListeners; private CopilotStatusResult copilotStatusResult; + private String notifiedUser; private CheckQuotaResult checkQuotaResult; private IEventBroker eventBroker; @@ -99,9 +100,14 @@ public CopilotStatusResult signOut() throws InterruptedException, ExecutionExcep * Set the CopilotStatusResult string to the given status and notify the listeners. */ public CopilotStatusResult setCopilotStatus(String newCopilotStatusResult) { - if (!Objects.equals(this.copilotStatusResult.getStatus(), newCopilotStatusResult)) { + if (!Objects.equals(this.copilotStatusResult.getStatus(), newCopilotStatusResult) + || !Objects.equals(notifiedUser, this.copilotStatusResult.getUser())) { this.copilotStatusResult.setStatus(newCopilotStatusResult); - onDidCopilotStatusChange(this.copilotStatusResult); + notifiedUser = this.copilotStatusResult.getUser(); + CopilotStatusResult notification = new CopilotStatusResult(); + notification.setStatus(newCopilotStatusResult); + notification.setUser(notifiedUser); + onDidCopilotStatusChange(notification); } return this.copilotStatusResult; } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java index 59b7017b6..a6e9cb055 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java @@ -55,6 +55,11 @@ public class CopilotEventConstants { */ public static final String TOPIC_AUTH_STATUS_CHANGED = TOPIC_AUTH + "STATUS_CHANGED"; + /** + * Successful LSP initialization; data is a monotonically increasing connection incarnation. + */ + public static final String TOPIC_LANGUAGE_SERVER_INITIALIZED = TOPIC_BASE + "LANGUAGE_SERVER_INITIALIZED"; + /** * Event when MCP tools changed. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProvider.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProvider.java index 925dcc086..970dd79ea 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProvider.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/LsStreamConnectionProvider.java @@ -24,16 +24,25 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import org.apache.commons.lang3.StringUtils; import org.eclipse.core.runtime.FileLocator; import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.URIUtil; +import org.eclipse.e4.core.contexts.EclipseContextFactory; +import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.lsp4e.server.ProcessStreamConnectionProvider; +import org.eclipse.lsp4j.jsonrpc.messages.Message; +import org.eclipse.lsp4j.jsonrpc.messages.NotificationMessage; +import org.eclipse.lsp4j.services.LanguageServer; import org.osgi.framework.Bundle; +import org.osgi.framework.FrameworkUtil; import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotCapabilities; import com.microsoft.copilot.eclipse.core.lsp.protocol.InitializationOptions; import com.microsoft.copilot.eclipse.core.lsp.protocol.NameAndVersion; @@ -46,6 +55,34 @@ public class LsStreamConnectionProvider extends ProcessStreamConnectionProvider public static final String EDITOR_NAME = "Eclipse"; public static final String EDITOR_PLUGIN_NAME = "copilot-eclipse"; + private static final AtomicLong CONNECTION_INCARNATION = new AtomicLong(); + private final AtomicBoolean initialized = new AtomicBoolean(); + + @Override + public void handleMessage(Message message, LanguageServer languageServer, @Nullable URI rootUri) { + super.handleMessage(message, languageServer, rootUri); + // LSP4E invokes this hook after sending initialized, only after initialize has succeeded. + // It creates a fresh provider on restart; status notifications are not connection recovery. + if (message instanceof NotificationMessage notification && "initialized".equals(notification.getMethod()) + && initialized.compareAndSet(false, true)) { + try { + IEventBroker broker = EclipseContextFactory + .getServiceContext(FrameworkUtil.getBundle(getClass()).getBundleContext()).get(IEventBroker.class); + if (broker == null || !broker.post(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED, + CONNECTION_INCARNATION.incrementAndGet())) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot publish language server initialization")); + } + } catch (RuntimeException exception) { + CopilotCore.LOGGER.error("Cannot publish language server initialization", exception); + } + } + } + + @Override + public void stop() { + initialized.set(true); + super.stop(); + } @Override public Object getInitializationOptions(@Nullable URI rootUri) { diff --git a/com.microsoft.copilot.eclipse.ui.test/META-INF/MANIFEST.MF b/com.microsoft.copilot.eclipse.ui.test/META-INF/MANIFEST.MF index d1b546c8c..8c1c7ed5f 100644 --- a/com.microsoft.copilot.eclipse.ui.test/META-INF/MANIFEST.MF +++ b/com.microsoft.copilot.eclipse.ui.test/META-INF/MANIFEST.MF @@ -7,7 +7,8 @@ Bundle-Vendor: GitHub Copilot Bundle-RequiredExecutionEnvironment: JavaSE-17 Fragment-Host: com.microsoft.copilot.eclipse.ui Automatic-Module-Name: com.microsoft.copilot.eclipse.ui.test -Import-Package: org.objenesis;version="[3.4.0,4.0.0)", +Import-Package: org.eclipse.lsp4j.jsonrpc.messages, + org.objenesis;version="[3.4.0,4.0.0)", org.opentest4j Require-Bundle: junit-jupiter-api;bundle-version="5.10.1", junit-jupiter-params;bundle-version="5.10.1", diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorageTest.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorageTest.java index fe54a612f..d350b0192 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorageTest.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorageTest.java @@ -36,6 +36,8 @@ import java.util.concurrent.atomic.AtomicLong; import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.e4.core.services.events.IEventBroker; +import org.eclipse.ui.PlatformUI; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -48,6 +50,7 @@ import com.microsoft.copilot.eclipse.core.AuthStatusManager; import com.microsoft.copilot.eclipse.core.CopilotAuthStatusListener; import com.microsoft.copilot.eclipse.core.chat.UserPreference; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; @@ -89,6 +92,109 @@ void tearDown() { realm.drain(); } + @Test + void testRecovery_FailureAndDuplicateSignals_RetriesOncePerConnectionAndAllowsManualRetry() throws Exception { + storage.initialize(); + drainWork(); + response.completeExceptionally(new IOException("offline")); + realm.drain(); + response = new CompletableFuture<>(); + recovered(1); + recovered(1); + storage.retry(); + drainWork(); + assertEquals(State.LOADING, storage.getState()); + verify(connection, times(2)).persistence(); + response.completeExceptionally(new IOException("still offline")); + realm.drain(); + recovered(1); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + drainWork(); + assertEquals(State.FAILED, storage.getState()); + verify(connection, times(2)).persistence(); + + response = new CompletableFuture<>(); + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"restored\"}"); + storage.retry(); + recovered(2); + recovered(1); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + assertEquals("restored", storage.getReadyPreferences().getChatModel()); + verify(connection, times(3)).persistence(); + } + + @Test + void testRecovery_SameAccountDirtyChoicesAndSaveFailure_RemainUnchanged() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + Mockito.doThrow(new IOException("read-only")).when(files).write(any(), any()); + storage.update(preference -> preference.setChatModel("unsaved")); + drainWork(); + realm.drain(); + recovered(1); + recovered(2); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + drainWork(); + realm.drain(); + + assertEquals("unsaved", storage.getReadyPreferences().getChatModel()); + assertTrue(storage.isDirty()); + assertEquals(PreferenceStorage.SaveState.FAILED, storage.getSaveStatus().getValue()); + verify(connection).persistence(); + verify(files).write(any(), any()); + } + + @Test + void testRecovery_SignedOutAndDisposed_DoesNotLoadOrPublish() { + when(auth.isSignedIn()).thenReturn(false); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + recovered(1); + drainWork(); + realm.drain(); + assertEquals(State.UNAVAILABLE, storage.getState()); + assertNull(storage.getReadyPreferences()); + storage.dispose(); + when(auth.isSignedIn()).thenReturn(true); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + recovered(2); + drainWork(); + realm.drain(); + + assertEquals(State.DISPOSED, storage.getState()); + verify(connection, never()).persistence(); + verifyNoInteractions(files); + } + + @Test + void testRecovery_TimedOutOldRpc_CannotReplaceSuccessfulRecovery() throws Exception { + response = new UncancellableFuture(); + CompletableFuture old = response; + storage.initialize(); + drainWork(); + expire(); + realm.drain(); + response = new CompletableFuture<>(); + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"new\"}"); + recovered(1); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + old.complete(persistence()); + drainWork(); + realm.drain(); + assertEquals("new", storage.getReadyPreferences().getChatModel()); + verify(files).read(any()); + } + + private void recovered(long incarnation) { + PlatformUI.getWorkbench().getService(IEventBroker.class) + .send(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED, incarnation); + } + @Test void testUpdate_RapidChanges_AreImmediateAndCoalesceBeforeWriting() throws Exception { when(files.read(any(Path.class))).thenReturn("{}"); @@ -202,6 +308,8 @@ void testUpdate_AccountChangesDuringOldWrite_OldFailureCannotAffectNewSave() thr assertEquals(Path.of(persistence().getPath(), "alice", "pref.json"), invocation.getArgument(0)); when(auth.getUserName()).thenReturn("bob"); response = new CompletableFuture<>(); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + recovered(1); load(); storage.update(preference -> preference.setChatModel("bob-model")); throw new IOException("old account failure"); @@ -667,7 +775,7 @@ void testAccountChange_LateOldRpc_DoesNotAffectNewAccount() throws Exception { when(auth.getUserName()).thenReturn("bob"); authListener().onDidCopilotStatusChange(new CopilotStatusResult()); realm.drain(); - assertEquals(State.UNAVAILABLE, storage.getState()); + assertEquals(State.LOADING, storage.getState()); verify(connection).persistence(); response = new CompletableFuture<>(); @@ -687,7 +795,7 @@ void testAccountChange_LateOldRpc_DoesNotAffectNewAccount() throws Exception { } @Test - void testAuthNotification_SignOut_InvalidatesWithoutAutomaticRetry() throws Exception { + void testAuthNotification_SignOutThenLogin_LoadsOnceWithoutManualRetry() throws Exception { when(files.read(any(Path.class))).thenReturn("{}"); load(); when(auth.isSignedIn()).thenReturn(false); @@ -696,12 +804,18 @@ void testAuthNotification_SignOut_InvalidatesWithoutAutomaticRetry() throws Exce realm.drain(); assertEquals(State.UNAVAILABLE, storage.getReadiness().getValue()); + response = new CompletableFuture<>(); when(auth.isSignedIn()).thenReturn(true); authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); drainWork(); realm.drain(); - verify(connection).persistence(); - assertEquals(State.UNAVAILABLE, storage.getState()); + verify(connection, times(2)).persistence(); + assertEquals(State.LOADING, storage.getState()); + response.complete(persistence()); + drainWork(); + realm.drain(); + assertEquals(State.READY, storage.getState()); } @Test diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceServiceTest.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceServiceTest.java index fe3aedd3e..acddae413 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceServiceTest.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceServiceTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.times; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -38,6 +39,8 @@ import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.jface.databinding.swt.DisplayRealm; +import org.eclipse.lsp4j.jsonrpc.messages.NotificationMessage; +import org.eclipse.lsp4j.services.LanguageServer; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.Label; @@ -63,10 +66,12 @@ import com.microsoft.copilot.eclipse.core.chat.UserPreference; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.LsStreamConnectionProvider; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatMode; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; +import com.microsoft.copilot.eclipse.core.lsp.protocol.quota.CheckQuotaResult; import com.microsoft.copilot.eclipse.ui.swt.DropdownButton; import com.microsoft.copilot.eclipse.ui.chat.Messages; import com.microsoft.copilot.eclipse.ui.chat.PreferenceStatus; @@ -108,6 +113,89 @@ void tearDown() { }); } + @Test + void testAuthentication_QuickSignOutAndBackIn_RejectsOldModeDiscovery() throws Exception { + CompletableFuture old = new CompletableFuture<>(); + CompletableFuture current = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(old, current); + when(connection.persistence()).thenReturn(CompletableFuture.completedFuture(persistence())); + when(connection.checkQuota()).thenReturn(CompletableFuture.completedFuture(CheckQuotaResult.empty())); + auth = new AuthStatusManager(connection); + auth.setCopilotUser("user"); + auth.setCopilotStatus(CopilotStatusResult.OK); + writePreferences("{\"chatModeName\":\"Plan\"}"); + runOnUi(() -> { + storage = new PreferenceStorage(connection, auth); + createControls(); + }); + verify(connection, timeout(5000)).listConversationModes(any()); + runOnUi(() -> { + auth.setCopilotStatus(CopilotStatusResult.NOT_SIGNED_IN); + auth.setCopilotStatus(CopilotStatusResult.OK); + old.complete(new ConversationMode[] {builtInMode("before-relogin", "Plan")}); + assertNull(BuiltInChatModeManager.INSTANCE.getBuiltInModeById("before-relogin")); + }); + verify(connection, timeout(5000).times(2)).listConversationModes(any()); + current.complete(new ConversationMode[] {builtInMode("after-relogin", "Plan")}); + awaitUi(service::isActiveModeReady); + runOnUi(() -> assertEquals("after-relogin", + BuiltInChatModeManager.INSTANCE.getBuiltInModeByDisplayName("Plan").getId())); + } + + @Test + void testRecovery_InitializedMessage_RestoresFailedPreferencesAndEnablesControls() throws Exception { + writePreferences("{\"chatModeName\":\"Plan\",\"chatModel\":\"restored\"}"); + startAuthenticated(CompletableFuture.failedFuture(new IOException("offline"))); + awaitUi(() -> storage.getState() == PreferenceStorage.State.FAILED); + CompletableFuture recovered = new CompletableFuture<>(); + when(connection.persistence()).thenReturn(recovered); + LsStreamConnectionProvider provider = new LsStreamConnectionProvider(); + NotificationMessage initialized = new NotificationMessage(); + initialized.setMethod("initialized"); + LanguageServer server = mock(LanguageServer.class); + provider.handleMessage(initialized, server, null); + verify(connection, timeout(5000).times(2)).persistence(); + CompletableFuture responsive = new CompletableFuture<>(); + Display.getDefault().asyncExec(() -> { + try { + responsive.complete(!picker.getEnabled() && storage.getState() == PreferenceStorage.State.LOADING); + } catch (Throwable failure) { + responsive.completeExceptionally(failure); + } + }); + assertTrue(responsive.get(5, TimeUnit.SECONDS)); + provider.handleMessage(initialized, server, null); + runOnUi(storage::retry); + recovered.complete(persistence()); + awaitUi(() -> picker.getEnabled() && service.isActiveModeReady()); + runOnUi(() -> { + assertEquals("Plan", service.getActiveModeNameOrId()); + assertEquals("restored", storage.getReadyPreferences().getChatModel()); + assertFalse(status.getVisible()); + }); + verify(connection, times(2)).persistence(); + } + + @Test + void testRecovery_FailedModeDiscovery_RestoresPlanWithoutReloadingPreferences() throws Exception { + writePreferences("{\"chatModeName\":\"Plan\"}"); + when(connection.listConversationModes(any())) + .thenReturn(CompletableFuture.failedFuture(new IOException("offline"))); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> storage.getState() == PreferenceStorage.State.READY + && service.getModeDiscoveryState() == UserPreferenceService.ModeDiscoveryState.FAILED); + CompletableFuture modes = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(modes); + NotificationMessage initialized = new NotificationMessage(); + initialized.setMethod("initialized"); + new LsStreamConnectionProvider().handleMessage(initialized, mock(LanguageServer.class), null); + verify(connection, timeout(5000).times(2)).listConversationModes(any()); + modes.complete(new ConversationMode[] {builtInMode("reconnected-plan", "Plan")}); + awaitUi(service::isActiveModeReady); + runOnUi(() -> assertEquals("Plan", service.getActiveModeNameOrId())); + verify(connection).persistence(); + } + @Test void testFirstInitialization_PendingModeDiscovery_ProcessesSwtAndPublishesModesAfterCompletion() throws Exception { when(auth.isSignedIn()).thenReturn(true); @@ -254,7 +342,7 @@ void testModeDiscovery_FailedOrEmpty_ShowsRetryWithoutReloadingReadyPreferences( } @Test - void testModeDiscovery_SupersededAttempt_CannotReplaceRecoveredInventory() throws Exception { + void testModeDiscovery_DuplicateAuth_JoinsPendingDiscovery() throws Exception { CompletableFuture obsolete = new CompletableFuture<>(); CompletableFuture current = new CompletableFuture<>(); when(connection.listConversationModes(any())).thenReturn(obsolete, current); @@ -267,23 +355,37 @@ void testModeDiscovery_SupersededAttempt_CannotReplaceRecoveredInventory() throw CopilotStatusResult statusResult = new CopilotStatusResult(); statusResult.setStatus(CopilotStatusResult.OK); statusResult.setUser("user"); - IEventBroker broker = PlatformUI.getWorkbench().getService(IEventBroker.class); runOnUi(() -> { - broker.send(CopilotEventConstants.TOPIC_AUTH_STATUS_CHANGED, statusResult); + notifyAuthentication(statusResult); service.retryModeDiscovery(); }); - verify(connection, timeout(5000).times(2)).listConversationModes(any()); - current.complete(new ConversationMode[] {builtInMode("current-plan", "Plan")}); + obsolete.complete(new ConversationMode[] {builtInMode("current-plan", "Plan")}); awaitUi(() -> service.isActiveModeReady() && !status.getVisible()); - obsolete.complete(new ConversationMode[] {builtInMode("obsolete-plan", "Plan")}); runOnUi(() -> { assertEquals("current-plan", BuiltInChatModeManager.INSTANCE.getBuiltInModeByDisplayName("Plan").getId()); assertEquals(UserPreferenceService.ModeDiscoveryState.READY, service.getModeDiscoveryState()); assertEquals(restored, storage.getReadyPreferences()); }); + verify(connection).listConversationModes(any()); verify(connection).persistence(); } + @Test + void testModeDiscovery_RepeatedAuthAfterFailure_DoesNotAutomaticallyRetry() throws Exception { + when(connection.listConversationModes(any())) + .thenReturn(CompletableFuture.failedFuture(new IOException("offline"))); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> service.getModeDiscoveryState() == UserPreferenceService.ModeDiscoveryState.FAILED); + CopilotStatusResult signedIn = new CopilotStatusResult(); + signedIn.setStatus(CopilotStatusResult.OK); + signedIn.setUser("user"); + runOnUi(() -> { + notifyAuthentication(signedIn); + assertEquals(UserPreferenceService.ModeDiscoveryState.FAILED, service.getModeDiscoveryState()); + }); + verify(connection).listConversationModes(any()); + } + @Test void testModeDiscovery_DisposedDuringManualRetry_RejectsLateSuccess() throws Exception { when(connection.listConversationModes(any())) @@ -422,12 +524,10 @@ void testSignOut_InvalidatesLoadedHistoryAndDisablesPicker() throws Exception { writePreferences("{\"chatModeName\":\"Ask\",\"userInputs\":[\"private history\"]}"); startAuthenticated(CompletableFuture.completedFuture(persistence())); awaitUi(() -> "Ask".equals(service.getActiveModeNameOrId())); - ArgumentCaptor listener = ArgumentCaptor.forClass(CopilotAuthStatusListener.class); - verify(auth).addCopilotAuthStatusListener(listener.capture()); when(auth.isSignedIn()).thenReturn(false); CopilotStatusResult signedOut = new CopilotStatusResult(); signedOut.setStatus(CopilotStatusResult.NOT_SIGNED_IN); - listener.getValue().onDidCopilotStatusChange(signedOut); + notifyAuthentication(signedOut); awaitUi(() -> storage.getReadiness().getValue() == PreferenceStorage.State.UNAVAILABLE); runOnUi(() -> { assertFalse(picker.getEnabled()); @@ -623,6 +723,12 @@ private void startAuthenticated(CompletableFuture rpc) { }); } + private void notifyAuthentication(CopilotStatusResult statusResult) { + ArgumentCaptor listeners = ArgumentCaptor.forClass(CopilotAuthStatusListener.class); + verify(auth, times(2)).addCopilotAuthStatusListener(listeners.capture()); + listeners.getAllValues().forEach(listener -> listener.onDidCopilotStatusChange(statusResult)); + } + private void createControls() { service = new UserPreferenceService(connection, auth, storage); shell = new Shell(Display.getDefault()); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorage.java index ecd1e4432..d29a9cf7e 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorage.java @@ -29,13 +29,17 @@ import org.eclipse.core.databinding.observable.Realm; import org.eclipse.core.databinding.observable.value.IObservableValue; import org.eclipse.core.databinding.observable.value.WritableValue; +import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.jface.databinding.swt.DisplayRealm; import org.eclipse.swt.widgets.Display; +import org.eclipse.ui.PlatformUI; +import org.osgi.service.event.EventHandler; import com.microsoft.copilot.eclipse.core.AuthStatusManager; import com.microsoft.copilot.eclipse.core.CopilotAuthStatusListener; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.chat.UserPreference; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; @@ -43,6 +47,8 @@ * Owns account-scoped chat preferences, asynchronous restoration and one serialized background writer. * Reads return detached snapshots; edits capture revisions without holding a UI-needed lock during file I/O. * Accepted saves retain their account path even after account changes or disposal. + * Login transitions and successful LSP initialization retry unsuccessful loads once per lifecycle event. + * They join pending loads, preserve ready choices, and never automatically retry failed saves. */ public class PreferenceStorage { private static final Gson GSON = new Gson(); @@ -72,8 +78,12 @@ public enum SaveState { private final WritableValue readiness; private final WritableValue saveStatus; private final CopilotAuthStatusListener authListener; + private final IEventBroker eventBroker; + private final EventHandler connectionListener; + private long connectionIncarnation; private State state = State.UNAVAILABLE; private String account; + private String notifiedAccount; private long generation; private Attempt attempt; private UserPreference preferences; @@ -109,8 +119,26 @@ public PreferenceStorage(CopilotLanguageServerConnection connection, AuthStatusM this.readiness = new WritableValue<>(realm, State.UNAVAILABLE, State.class); this.saveStatus = new WritableValue<>(realm, SaveState.SAVED, SaveState.class); this.account = currentAccount(); - this.authListener = status -> refreshAccount(); + this.notifiedAccount = account; + this.authListener = status -> authenticationChanged(); auth.addCopilotAuthStatusListener(authListener); + this.eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + this.connectionListener = event -> { + if (event.getProperty(IEventBroker.DATA) instanceof Long incarnation) { + synchronized (lock) { + if (state == State.DISPOSED || incarnation <= connectionIncarnation) { + return; + } + connectionIncarnation = incarnation; + } + retry(); + } + }; + if (eventBroker != null) { + eventBroker.subscribe(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED, null, connectionListener, false); + } else { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot subscribe to language server initialization")); + } } /** @@ -323,6 +351,9 @@ public void dispose() { previous = clear(State.DISPOSED); } auth.removeCopilotAuthStatusListener(authListener); + if (eventBroker != null) { + eventBroker.unsubscribe(connectionListener); + } cancel(previous); // Accepted snapshots may finish on the shared writer; disposal never waits for OS I/O. dispatchWriter(); @@ -367,6 +398,19 @@ private void refreshAccount() { publishSaveStatus(version); } + private void authenticationChanged() { + boolean changed; + synchronized (lock) { + String current = currentAccount(); + changed = !Objects.equals(notifiedAccount, current); + notifiedAccount = current; + } + refreshAccount(); + if (changed) { + retry(); + } + } + private Attempt clear(State next) { final Attempt previous = attempt; attempt = null; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceService.java index 5dfc763c4..1c58197c5 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/UserPreferenceService.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicLong; import org.apache.commons.lang3.StringUtils; import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; @@ -22,6 +23,7 @@ import org.osgi.service.event.EventHandler; import com.microsoft.copilot.eclipse.core.AuthStatusManager; +import com.microsoft.copilot.eclipse.core.CopilotAuthStatusListener; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.FeatureFlags; import com.microsoft.copilot.eclipse.core.chat.BuiltInChatMode; @@ -34,7 +36,6 @@ import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatMode; -import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidChangeFeatureFlagsParams; import com.microsoft.copilot.eclipse.ui.chat.ChatView; import com.microsoft.copilot.eclipse.ui.chat.Messages; @@ -59,7 +60,12 @@ public enum ModeDiscoveryState { private final PreferenceStorage preferenceStorage; private ISideEffect readinessSideEffect; private volatile boolean disposed; + private final Object authenticationLock = new Object(); + private final AtomicLong authenticationGeneration = new AtomicLong(); + private String notifiedAccount; + private final CopilotAuthStatusListener authListener; private long modeDiscoveryGeneration; + private String modeDiscoveryAccount; private CompletableFuture> modeDiscovery; private IObservableValue modeDiscoveryState; private IObservableValue chatModeObservable; @@ -73,8 +79,9 @@ public enum ModeDiscoveryState { // Event handling private IEventBroker eventBroker; - private EventHandler authStatusChangedEventHandler; private EventHandler featureFlagNotifiedEventHandler; + private EventHandler connectionInitializedEventHandler; + private long connectionIncarnation; /** * Constructor for the UserPreferenceService. @@ -83,6 +90,8 @@ public UserPreferenceService(CopilotLanguageServerConnection lsConnection, AuthS PreferenceStorage preferenceStorage) { super(lsConnection, authStatusManager); this.preferenceStorage = preferenceStorage; + notifiedAccount = authStatusManager.isSignedIn() ? authStatusManager.getUserName() : null; + authListener = status -> authenticationChanged(); ensureRealm(() -> { chatModeObservable = new WritableValue<>(getAvailableChatModes(), String[].class); @@ -102,23 +111,22 @@ public UserPreferenceService(CopilotLanguageServerConnection lsConnection, AuthS initializeEventHandlers(); subscribeToEvents(); + authStatusManager.addCopilotAuthStatusListener(authListener); preferenceStorage.initialize(); - reloadBuiltInModes(); + reloadBuiltInModes(false); } private void initializeEventHandlers() { - authStatusChangedEventHandler = event -> { - Object property = event.getProperty(IEventBroker.DATA); - if (property instanceof CopilotStatusResult statusResult) { - if (statusResult.isSignedIn() && authStatusManager.isSignedIn() - && Objects.equals(statusResult.getUser(), authStatusManager.getUserName())) { - reloadBuiltInModes(); - } else if (!statusResult.isSignedIn()) { - ensureRealm(this::cancelModeDiscovery); - } + connectionInitializedEventHandler = event -> { + if (event.getProperty(IEventBroker.DATA) instanceof Long incarnation) { + ensureRealm(() -> { + if (incarnation > connectionIncarnation) { + connectionIncarnation = incarnation; + retryModeDiscovery(); + } + }); } }; - featureFlagNotifiedEventHandler = event -> { Object property = event.getProperty(IEventBroker.DATA); if (property instanceof DidChangeFeatureFlagsParams params) { @@ -135,21 +143,49 @@ private void initializeEventHandlers() { }; } - private void reloadBuiltInModes() { + private void authenticationChanged() { + long generation; + synchronized (authenticationLock) { + String current = authStatusManager.isSignedIn() ? authStatusManager.getUserName() : null; + if (disposed || Objects.equals(notifiedAccount, current)) { + return; + } + notifiedAccount = current; + // Invalidate callbacks immediately, before a rapid sign-out/sign-in can overtake the UI queue. + generation = authenticationGeneration.incrementAndGet(); + } + ensureRealm(() -> { + if (generation == authenticationGeneration.get()) { + cancelModeDiscovery(); + reloadBuiltInModes(false); + } + }); + } + + private void reloadBuiltInModes(boolean retry) { ensureRealm(() -> { - cancelModeDiscovery(); if (!authStatusManager.isSignedIn()) { + cancelModeDiscovery(); return; } - long generation = modeDiscoveryGeneration; String account = authStatusManager.getUserName(); + ModeDiscoveryState state = modeDiscoveryState.getValue(); + if (Objects.equals(account, modeDiscoveryAccount) && state != ModeDiscoveryState.UNAVAILABLE + && (state != ModeDiscoveryState.FAILED || !retry)) { + return; + } + cancelModeDiscovery(); + modeDiscoveryAccount = account; + long generation = modeDiscoveryGeneration; + long authentication = authenticationGeneration.get(); modeDiscoveryState.setValue(ModeDiscoveryState.LOADING); CompletableFuture> discovery = CompletableFuture .supplyAsync(() -> new BuiltInChatModeService().loadBuiltInModes(lsConnection)) .thenCompose(result -> result); modeDiscovery = discovery; discovery.thenAccept(modes -> ensureRealm(() -> { - if (generation != modeDiscoveryGeneration || !authStatusManager.isSignedIn() + if (generation != modeDiscoveryGeneration || authentication != authenticationGeneration.get() + || !authStatusManager.isSignedIn() || !Objects.equals(account, authStatusManager.getUserName())) { return; } @@ -165,7 +201,8 @@ private void reloadBuiltInModes() { } })).exceptionally(exception -> { ensureRealm(() -> { - if (generation == modeDiscoveryGeneration && authStatusManager.isSignedIn() + if (generation == modeDiscoveryGeneration && authentication == authenticationGeneration.get() + && authStatusManager.isSignedIn() && Objects.equals(account, authStatusManager.getUserName())) { modeDiscoveryState.setValue(ModeDiscoveryState.FAILED); CopilotCore.LOGGER.error("Failed to reload built-in modes", exception); @@ -178,6 +215,7 @@ private void reloadBuiltInModes() { private void cancelModeDiscovery() { modeDiscoveryGeneration++; + modeDiscoveryAccount = null; modeDiscoveryState.setValue(ModeDiscoveryState.UNAVAILABLE); if (modeDiscovery != null) { modeDiscovery.cancel(true); @@ -187,7 +225,7 @@ private void cancelModeDiscovery() { private void subscribeToEvents() { eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); if (eventBroker != null) { - eventBroker.subscribe(CopilotEventConstants.TOPIC_AUTH_STATUS_CHANGED, authStatusChangedEventHandler); + eventBroker.subscribe(CopilotEventConstants.TOPIC_LANGUAGE_SERVER_INITIALIZED, connectionInitializedEventHandler); eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_FEATURE_FLAGS, featureFlagNotifiedEventHandler); } else { CopilotCore.LOGGER.error(new IllegalStateException("Event broker is null")); @@ -425,7 +463,7 @@ public void retryModeDiscovery() { ensureRealm(() -> { ModeDiscoveryState state = modeDiscoveryState.getValue(); if (state == ModeDiscoveryState.FAILED || state == ModeDiscoveryState.UNAVAILABLE) { - reloadBuiltInModes(); + reloadBuiltInModes(true); } }); } @@ -721,6 +759,7 @@ public void dispose() { } persistUserPreference(); disposed = true; + authStatusManager.removeCopilotAuthStatusListener(authListener); super.ensureRealm(() -> { cancelModeDiscovery(); readinessSideEffect.dispose(); @@ -735,9 +774,8 @@ public void dispose() { }); if (eventBroker != null) { - eventBroker.unsubscribe(authStatusChangedEventHandler); + eventBroker.unsubscribe(connectionInitializedEventHandler); eventBroker.unsubscribe(featureFlagNotifiedEventHandler); - authStatusChangedEventHandler = null; featureFlagNotifiedEventHandler = null; eventBroker = null; } @@ -776,8 +814,10 @@ protected void ensureRealm(Runnable runnable) { return; } String account = authStatusManager.getUserName(); + long generation = authenticationGeneration.get(); super.ensureRealm(() -> { - if (!disposed && Objects.equals(account, authStatusManager.getUserName())) { + if (!disposed && generation == authenticationGeneration.get() + && Objects.equals(account, authStatusManager.getUserName())) { runnable.run(); } });