diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManagerTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManagerTests.java new file mode 100644 index 000000000..f62036ac4 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManagerTests.java @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; + +@ExtendWith(MockitoExtension.class) +class BuiltInChatModeManagerTests { + + @Mock + private CopilotLanguageServerConnection mockConnection; + + @Test + void testLoadBuiltInModes_inlineAgentSkippedFromAgentModes() { + ConversationMode agentMode = createBuiltInMode("Agent", "Agent", "Agent", + "Advanced agent mode with access to tools and capabilities"); + ConversationMode inlineAgentMode = createBuiltInMode("InlineAgent", "Agent", "InlineAgent", + "Agent mode with a restricted tool set for inline editing"); + + when(mockConnection.listConversationModes(any(ConversationModesParams.class))) + .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { agentMode, inlineAgentMode })); + + List builtInModes = BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join(); + + assertEquals(1, builtInModes.size()); + + BuiltInChatMode builtInMode = builtInModes.get(0); + assertNotNull(builtInMode); + assertEquals("Agent", builtInMode.getId()); + assertEquals("Agent", builtInMode.getDisplayName()); + assertEquals("Agent", builtInMode.getKind()); + } + + @Test + void testLoadBuiltInModes_onlyAllowedBuiltInNames() { + ConversationMode ask = createBuiltInMode("ask", "aSk", "Ask", "Ask mode"); + ConversationMode debugger = createBuiltInMode("debugger", "Debugger", "Debugger", "Debugger mode"); + ConversationMode custom = createBuiltInMode("custom", "Plan", "Plan", "Custom mode"); + custom.setBuiltIn(false); + ConversationMode unknown = createBuiltInMode("unknown", "Unknown", "Unknown", "Unknown mode"); + + when(mockConnection.listConversationModes(any(ConversationModesParams.class))) + .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { + null, custom, unknown, ask, debugger })); + + List modes = BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join(); + + assertEquals(List.of("ask", "debugger"), modes.stream().map(BuiltInChatMode::getId).toList()); + } + + @Test + void testLoadBuiltInModes_nullConnectionReturnsEmptyList() { + assertEquals(List.of(), BuiltInChatModeManager.INSTANCE.loadBuiltInModes(null).join()); + } + + @Test + void testLoadBuiltInModes_conversionFailureSkipsMode() { + ConversationMode invalid = mock(ConversationMode.class); + when(invalid.isBuiltIn()).thenReturn(true); + when(invalid.getName()).thenReturn("Ask"); + when(invalid.getId()).thenReturn("invalid"); + when(invalid.getCustomTools()).thenThrow(new IllegalArgumentException("invalid tools")); + ConversationMode agent = createBuiltInMode("agent", "Agent", "Agent", "Agent mode"); + when(mockConnection.listConversationModes(any(ConversationModesParams.class))) + .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { invalid, agent })); + + List modes = BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join(); + + assertEquals(List.of("agent"), modes.stream().map(BuiltInChatMode::getId).toList()); + } + + @Test + void testLoadBuiltInModes_rpcFailurePropagates() { + IllegalStateException failure = new IllegalStateException("RPC failed"); + when(mockConnection.listConversationModes(any(ConversationModesParams.class))) + .thenReturn(CompletableFuture.failedFuture(failure)); + + CompletionException exception = assertThrows(CompletionException.class, + () -> BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join()); + + assertSame(failure, exception.getCause()); + } + + private ConversationMode createBuiltInMode(String id, String name, String kind, String description) { + ConversationMode mode = new ConversationMode(); + mode.setId(id); + mode.setName(name); + mode.setKind(kind); + mode.setBuiltIn(true); + mode.setDescription(description); + return mode; + } +} diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeServiceTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeServiceTests.java deleted file mode 100644 index 22039b462..000000000 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeServiceTests.java +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -package com.microsoft.copilot.eclipse.core.chat.service; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.when; - -import java.lang.reflect.Field; -import java.util.List; -import java.util.concurrent.CompletableFuture; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -import com.microsoft.copilot.eclipse.core.CopilotCore; -import com.microsoft.copilot.eclipse.core.chat.BuiltInChatMode; -import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; - -@ExtendWith(MockitoExtension.class) -class BuiltInChatModeServiceTests { - - @Mock - private CopilotLanguageServerConnection mockConnection; - - private BuiltInChatModeService builtInChatModeService; - - @BeforeEach - void setUp() throws Exception { - builtInChatModeService = new BuiltInChatModeService(); - - CopilotCore plugin = new CopilotCore(); - Field languageServerField = CopilotCore.class.getDeclaredField("copilotLanguageServer"); - languageServerField.setAccessible(true); - languageServerField.set(plugin, mockConnection); - } - - @Test - void testLoadBuiltInModes_inlineAgentSkippedFromAgentModes() { - ConversationMode agentMode = createBuiltInMode("Agent", "Agent", "Agent", - "Advanced agent mode with access to tools and capabilities"); - ConversationMode inlineAgentMode = createBuiltInMode("InlineAgent", "Agent", "InlineAgent", - "Agent mode with a restricted tool set for inline editing"); - - when(mockConnection.listConversationModes(any(ConversationModesParams.class))) - .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { agentMode, inlineAgentMode })); - - List builtInModes = builtInChatModeService.loadBuiltInModes().join(); - - assertEquals(1, builtInModes.size()); - - BuiltInChatMode builtInMode = builtInModes.get(0); - assertNotNull(builtInMode); - assertEquals("Agent", builtInMode.getId()); - assertEquals("Agent", builtInMode.getDisplayName()); - assertEquals("Agent", builtInMode.getKind()); - } - - private ConversationMode createBuiltInMode(String id, String name, String kind, String description) { - ConversationMode mode = new ConversationMode(); - mode.setId(id); - mode.setName(name); - mode.setKind(kind); - mode.setBuiltIn(true); - mode.setDescription(description); - return mode; - } -} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManager.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManager.java index 079628bf1..1f61a98b9 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManager.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManager.java @@ -4,33 +4,70 @@ package com.microsoft.copilot.eclipse.core.chat; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CompletableFuture; -import com.microsoft.copilot.eclipse.core.chat.service.BuiltInChatModeService; +import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; /** - * Singleton manager for built-in chat modes. Built-in modes are loaded once from the LSP API at startup. + * Shared snapshot of built-in chat modes discovered asynchronously by the chat lifecycle. */ public enum BuiltInChatModeManager { INSTANCE; - private final BuiltInChatModeService service; - private List builtInModes; + private static final List ALLOWED_BUILTIN_NAMES = Arrays.asList(BuiltInChatMode.ASK_MODE_NAME, + BuiltInChatMode.AGENT_MODE_NAME, BuiltInChatMode.PLAN_MODE_NAME, BuiltInChatMode.DEBUGGER_MODE_NAME); - BuiltInChatModeManager() { - this.service = new BuiltInChatModeService(); - this.builtInModes = new CopyOnWriteArrayList<>(); - loadModesSync(); + private volatile List builtInModes = List.of(); + + /** + * Loads built-in modes using the owning chat lifecycle's language-server connection. The LSP requires workspace + * folders even though built-in modes do not depend on workspace context. + * + * @param lsConnection the connection used by the chat services + * @return the discovered built-in modes + */ + public CompletableFuture> loadBuiltInModes(CopilotLanguageServerConnection lsConnection) { + if (lsConnection == null) { + return CompletableFuture.completedFuture(new ArrayList<>()); + } + ConversationModesParams params = new ConversationModesParams(Collections.emptyList()); + + return lsConnection.listConversationModes(params).thenApply(conversationModes -> { + List modes = new ArrayList<>(); + + for (ConversationMode mode : conversationModes) { + if (mode == null || !mode.isBuiltIn()) { + continue; + } + // Exclude InlineAgent kind — it is not a user-facing chat mode + if (BuiltInChatMode.INLINE_AGENT_KIND.equalsIgnoreCase(mode.getKind())) { + continue; + } + // Filter to only allowed built-in modes by name (case-insensitive) + if (ALLOWED_BUILTIN_NAMES.stream().anyMatch(name -> name.equalsIgnoreCase(mode.getName()))) { + BuiltInChatMode builtIn = convertToBuiltInChatMode(mode); + if (builtIn != null) { + modes.add(builtIn); + } + } + } + + return modes; + }); } - private void loadModesSync() { + private BuiltInChatMode convertToBuiltInChatMode(ConversationMode mode) { try { - List modes = service.loadBuiltInModes().get(); - this.builtInModes = new CopyOnWriteArrayList<>(modes); + return new BuiltInChatMode(mode); } catch (Exception e) { - // Initialize with empty list on failure - this.builtInModes = new CopyOnWriteArrayList<>(); + CopilotCore.LOGGER.error("Failed to convert built-in mode: " + mode.getId(), e); + return null; } } @@ -60,10 +97,11 @@ public BuiltInChatMode getBuiltInModeById(String id) { } /** - * Reloads built-in chat modes from the LSP API. This should be called when the user switches - * to ensure the latest modes are available for the current user context. + * Publishes a completed discovery after its owner has validated the account and lifecycle. + * + * @param modes the modes applicable to the current account */ - public void reloadModes() { - loadModesSync(); + public void updateModes(List modes) { + builtInModes = List.copyOf(modes); } } \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/UserPreference.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/UserPreference.java index 0d5cded98..ccf643a0f 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/UserPreference.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/UserPreference.java @@ -12,9 +12,8 @@ /** - * Preferences per GitHub user. All the getters and setters are synchronized due to that the ChatBaseService holds a - * shared (single) reference to the user preference. synchronized modifies makes sure the update to the instance are - * thread safe. + * Preferences per GitHub user, shared by the chat preference storage. Scalar access is synchronized and model option + * maps use immutable snapshots so readers never observe partially updated maps. */ public class UserPreference { diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeService.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeService.java deleted file mode 100644 index cc3765064..000000000 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/BuiltInChatModeService.java +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -package com.microsoft.copilot.eclipse.core.chat.service; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.CompletableFuture; - -import com.microsoft.copilot.eclipse.core.CopilotCore; -import com.microsoft.copilot.eclipse.core.chat.BuiltInChatMode; -import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; - -/** - * Service for loading built-in chat modes from the LSP API. Built-in modes include Ask, Agent, and Plan. - */ -public class BuiltInChatModeService { - - private static final List ALLOWED_BUILTIN_NAMES = Arrays.asList(BuiltInChatMode.ASK_MODE_NAME, - BuiltInChatMode.AGENT_MODE_NAME, BuiltInChatMode.PLAN_MODE_NAME, BuiltInChatMode.DEBUGGER_MODE_NAME); - - /** - * Loads built-in modes from the LSP API. Only modes with names in ALLOWED_BUILTIN_NAMES are returned. - * - *

Note: The LSP requires workspace folders to be passed even for loading built-in modes. While built-in modes - * don't depend on workspace context, the LSP API enforces this parameter. - */ - public CompletableFuture> loadBuiltInModes() { - ConversationModesParams params = new ConversationModesParams(Collections.emptyList()); - - CopilotLanguageServerConnection lspConnection = CopilotCore.getPlugin().getCopilotLanguageServer(); - if (lspConnection == null) { - return CompletableFuture.completedFuture(new ArrayList<>()); - } - - return lspConnection.listConversationModes(params).thenApply(conversationModes -> { - List builtInModes = new ArrayList<>(); - - for (ConversationMode mode : conversationModes) { - if (mode == null || !mode.isBuiltIn()) { - continue; - } - // Exclude InlineAgent kind — it is not a user-facing chat mode - if (BuiltInChatMode.INLINE_AGENT_KIND.equalsIgnoreCase(mode.getKind())) { - continue; - } - // Filter to only allowed built-in modes by name (case-insensitive) - if (ALLOWED_BUILTIN_NAMES.stream().anyMatch(name -> name.equalsIgnoreCase(mode.getName()))) { - BuiltInChatMode builtIn = convertToBuiltInChatMode(mode); - if (builtIn != null) { - builtInModes.add(builtIn); - } - } - } - - return builtInModes; - }).exceptionally(ex -> { - CopilotCore.LOGGER.error("Failed to load built-in modes", ex); - return new ArrayList<>(); - }); - } - - private BuiltInChatMode convertToBuiltInChatMode(ConversationMode mode) { - try { - return new BuiltInChatMode(mode); - } catch (Exception e) { - CopilotCore.LOGGER.error("Failed to convert built-in mode: " + mode.getId(), e); - return null; - } - } -} \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFileTest.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFileTest.java index 75a925fb2..11dd285ac 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFileTest.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFileTest.java @@ -3,9 +3,13 @@ package com.microsoft.copilot.eclipse.ui.chat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +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.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.spy; @@ -15,8 +19,11 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import org.eclipse.core.resources.IFile; +import org.eclipse.jface.preference.PreferenceStore; +import org.eclipse.jface.resource.ImageRegistry; import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; @@ -30,18 +37,21 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; +import com.microsoft.copilot.eclipse.core.AuthStatusManager; +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.CopilotModel; import com.microsoft.copilot.eclipse.ui.CopilotUi; +import com.microsoft.copilot.eclipse.ui.chat.contextwindow.ContextWindowService; import com.microsoft.copilot.eclipse.ui.chat.services.AgentToolService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatFontService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; import com.microsoft.copilot.eclipse.ui.chat.services.McpConfigService; import com.microsoft.copilot.eclipse.ui.chat.services.ModelService; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage; import com.microsoft.copilot.eclipse.ui.chat.services.ReferencedFileService; import com.microsoft.copilot.eclipse.ui.chat.services.UserPreferenceService; import com.microsoft.copilot.eclipse.ui.chat.tools.JavaDebuggerToolAdapter; -import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; @ExtendWith(MockitoExtension.class) class ReferencedFileTest { @@ -68,14 +78,21 @@ class ReferencedFileTest { private AgentToolService mockAgentToolService; @Mock private ChatFontService mockChatFontService; + @Mock + private ContextWindowService mockContextWindowService; + @Mock + private AuthStatusManager mockAuthStatusManager; + @Mock + private CopilotLanguageServerConnection mockConnection; private Shell shell; private ActionBar actionBar; + private PreferenceStorage preferenceStorage; private MockedStatic mockedCopilotUi; @BeforeEach void setUp() { - SwtUtils.invokeOnDisplayThread(() -> { + runOnUi(() -> { setupSwtComponents(); setupMockFiles(); setupMockServices(); @@ -84,16 +101,22 @@ void setUp() { @AfterEach void tearDown() { - SwtUtils.invokeOnDisplayThread(() -> { - if (actionBar != null && !actionBar.isDisposed()) { - actionBar.dispose(); - } - if (shell != null && !shell.isDisposed()) { - shell.dispose(); + try { + runOnUi(() -> { + if (actionBar != null && !actionBar.isDisposed()) { + actionBar.dispose(); + } + if (shell != null && !shell.isDisposed()) { + shell.dispose(); + } + if (preferenceStorage != null) { + preferenceStorage.dispose(); + } + }); + } finally { + if (mockedCopilotUi != null) { + mockedCopilotUi.close(); } - }); - if (mockedCopilotUi != null) { - mockedCopilotUi.close(); } } @@ -110,8 +133,13 @@ private void setupMockFiles() { } private void setupMockServices() { + ImageRegistry imageRegistry = CopilotUi.getPlugin().getImageRegistry(); mockedCopilotUi = mockStatic(CopilotUi.class); mockedCopilotUi.when(CopilotUi::getPlugin).thenReturn(mockCopilotUi); + when(mockCopilotUi.getImageRegistry()).thenReturn(imageRegistry); + when(mockCopilotUi.getPreferenceStore()).thenReturn(new PreferenceStore()); + preferenceStorage = new PreferenceStorage(mockConnection, mockAuthStatusManager); + preferenceStorage.initialize(); lenient().when(mockModel.getModelName()).thenReturn("test-model"); lenient().when(mockModelService.getActiveModel()).thenReturn(mockModel); @@ -121,18 +149,38 @@ private void setupMockServices() { lenient().when(mockChatServiceManager.getMcpConfigService()).thenReturn(mockMcpConfigService); lenient().when(mockChatServiceManager.getAgentToolService()).thenReturn(mockAgentToolService); lenient().when(mockChatServiceManager.getChatFontService()).thenReturn(mockChatFontService); + when(mockChatServiceManager.getContextWindowService()).thenReturn(mockContextWindowService); + when(mockChatServiceManager.getPreferenceStorage()).thenReturn(preferenceStorage); lenient().when(mockAgentToolService.getTool(JavaDebuggerToolAdapter.TOOL_NAME)).thenReturn(null); lenient().when(mockUserPreferenceService.getActiveChatMode()).thenReturn(ChatMode.Ask); lenient().when(mockCopilotUi.getChatServiceManager()).thenReturn(mockChatServiceManager); actionBar = spy(new ActionBar(shell, SWT.NONE, mockChatServiceManager)); } + @Test + void testImageReference_ModelUnavailable_ShowsUnsupportedChipAndLocalizedTooltip() { + runOnUi(() -> { + when(mockModelService.getActiveModel()).thenReturn(null); + assertEquals(PreferenceStorage.State.UNAVAILABLE, preferenceStorage.getState()); + + ReferencedFile image = assertDoesNotThrow(() -> new ReferencedFile(shell, mockImageFile, true)); + + assertSame(mockImageFile, image.getFile()); + assertTrue(image.isFileUnSupported()); + assertTrue(image.getVisible()); + assertEquals(3, image.getChildren().length); + for (Control child : image.getChildren()) { + assertEquals("Images are unavailable until a model is ready.", child.getToolTipText()); + } + }); + } + /** * Test ActionBar creates ReferencedFiles with correct strikeThrough state based on model vision support. */ @Test void testStrikeThroughBehavior() { - SwtUtils.invokeOnDisplayThread(() -> { + runOnUi(() -> { List testFiles = Arrays.asList(mockImageFile, mockTextFile); callUpdateReferencedFilesInternal(actionBar, testFiles, true); @@ -161,6 +209,20 @@ void testStrikeThroughBehavior() { }); } + private static void runOnUi(Runnable runnable) { + AtomicReference failure = new AtomicReference<>(); + Display.getDefault().syncExec(() -> { + try { + runnable.run(); + } catch (Throwable error) { + failure.set(error); + } + }); + if (failure.get() != null) { + fail("UI operation failed", failure.get()); + } + } + /** * Helper method to call private updateReferencedFilesInternal method using reflection. */ diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseServiceTests.java index 751291b2c..3adc0f118 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseServiceTests.java @@ -3,19 +3,39 @@ package com.microsoft.copilot.eclipse.ui.chat.services; -import static org.mockito.Mockito.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import org.junit.jupiter.api.BeforeEach; +import java.util.List; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.mockito.junit.jupiter.MockitoExtension; import com.microsoft.copilot.eclipse.core.AuthStatusManager; -import com.microsoft.copilot.eclipse.core.CopilotCore; 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.CopilotModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.quota.CheckQuotaResult; +import com.microsoft.copilot.eclipse.ui.swt.DropdownButton; +@ExtendWith(MockitoExtension.class) class ChatBaseServiceTests { @Mock @@ -24,40 +44,110 @@ class ChatBaseServiceTests { @Mock private AuthStatusManager mockAuthStatusManager; - @Mock - private CopilotCore mockCopilotCore; - - private TestChatBaseService chatBaseService; - - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - chatBaseService = new TestChatBaseService(mockLsConnection, mockAuthStatusManager); - } + @TempDir + private Path directory; @Test void persistUserPreference_WhenNotSignedIn_ShouldSkipLspInteraction() { - // Arrange when(mockAuthStatusManager.isSignedIn()).thenReturn(false); - - // Act - chatBaseService.persistUserPreference(); - - // Assert - verify(mockAuthStatusManager).isSignedIn(); - verifyNoInteractions(mockLsConnection); // LSP connection should not be used + PreferenceStorage storage = new PreferenceStorage(mockLsConnection, mockAuthStatusManager); + try { + storage.initialize(); + storage.persist(); + assertEquals(PreferenceStorage.State.UNAVAILABLE, storage.getState()); + verifyNoInteractions(mockLsConnection); + } finally { + storage.dispose(); + } } - /** - * Test implementation of ChatBaseService for testing purposes - */ - private static class TestChatBaseService extends ChatBaseService { - public TestChatBaseService(CopilotLanguageServerConnection lsConnection, AuthStatusManager authStatusManager) { - super(lsConnection, authStatusManager); - } + @Test + void testFirstInitialization_PendingAuthenticatedPersistence_DoesNotBlockSwt() throws Exception { + when(mockAuthStatusManager.isSignedIn()).thenReturn(true); + when(mockAuthStatusManager.getUserName()).thenReturn("pending-user"); + when(mockAuthStatusManager.getQuotaStatus()).thenReturn(CheckQuotaResult.empty()); + CompletableFuture persistence = new CompletableFuture<>(); + when(mockLsConnection.persistence()).thenReturn(persistence); + CopilotModel model = new CopilotModel(); + model.setId("saved-model"); + model.setModelName("Saved model"); + model.setModelFamily("family"); + model.setScopes(List.of(CopilotScope.CHAT_PANEL, CopilotScope.AGENT_PANEL)); + when(mockLsConnection.listModels()).thenReturn(CompletableFuture.completedFuture(new CopilotModel[] {model})); + Path file = directory.resolve("pending-user").resolve("pref.json"); + Files.createDirectories(file.getParent()); + String saved = "{\"chatModeName\":\"Ask\",\"chatModel\":\"" + model.getModelKey() + "\"}"; + Files.writeString(file, saved); + ByokListModelResponse byok = new ByokListModelResponse(); + byok.setModels(List.of()); + when(mockLsConnection.listByokModels(org.mockito.ArgumentMatchers.any())) + .thenReturn(CompletableFuture.completedFuture(byok)); - public void persistUserPreference() { - super.persistUserPreference(); + AtomicReference storage = new AtomicReference<>(); + AtomicReference preferences = new AtomicReference<>(); + AtomicReference models = new AtomicReference<>(); + AtomicReference shell = new AtomicReference<>(); + AtomicReference modeControl = new AtomicReference<>(); + AtomicReference modelControl = new AtomicReference<>(); + CompletableFuture uiAction = new CompletableFuture<>(); + Display.getDefault().asyncExec(() -> { + try { + storage.set(new PreferenceStorage(mockLsConnection, mockAuthStatusManager)); + models.set(new ModelService(mockLsConnection, mockAuthStatusManager, storage.get())); + preferences.set(new UserPreferenceService(mockLsConnection, mockAuthStatusManager, storage.get())); + shell.set(new Shell(Display.getDefault())); + DropdownButton modePicker = new DropdownButton(shell.get(), SWT.NONE); + DropdownButton modelPicker = new DropdownButton(shell.get(), SWT.NONE); + modeControl.set(modePicker); + modelControl.set(modelPicker); + preferences.get().bindChatModePicker(modePicker); + models.get().bindModelPicker(modelPicker); + Display.getDefault().asyncExec(() -> { + uiAction.complete(!modePicker.getEnabled() && !modelPicker.getEnabled() + && storage.get().getState() == PreferenceStorage.State.LOADING); + }); + } catch (Throwable error) { + uiAction.completeExceptionally(error); + } + }); + try { + assertTrue(uiAction.get(5, TimeUnit.SECONDS), "Preference controls must stay disabled while SWT processes work"); + assertFalse(persistence.isDone(), "SWT must process another action before persistence completes"); + Display.getDefault().syncExec(() -> assertNull(models.get().getActiveModel())); + ChatPersistence response = new ChatPersistence(); + response.setPath(directory.toString()); + persistence.complete(response); + AtomicReference restored = new AtomicReference<>(false); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + do { + Display.getDefault().syncExec(() -> restored.set( + "Ask".equals(preferences.get().getActiveModeNameOrId()) + && models.get().getActiveModel() == model + && modeControl.get().getEnabled() && modelControl.get().getEnabled())); + if (restored.get()) { + break; + } + Thread.sleep(10); + } while (System.nanoTime() < deadline); + assertTrue(restored.get(), "Mode and model must restore after the shared load succeeds"); + assertEquals(saved, Files.readString(file), "Placeholder observables must not save over stored preferences"); + } finally { + // Release a regressed blocking constructor before cleaning up on SWT. + persistence.completeExceptionally(new IllegalStateException("test cleanup")); + Display.getDefault().syncExec(() -> { + if (shell.get() != null) { + shell.get().dispose(); + } + if (preferences.get() != null) { + preferences.get().dispose(); + } + if (models.get() != null) { + models.get().dispose(); + } + if (storage.get() != null) { + storage.get().dispose(); + } + }); } } } \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelServiceTests.java index 1626af9a8..e006a1eeb 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelServiceTests.java @@ -6,6 +6,7 @@ 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.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -16,12 +17,16 @@ import java.nio.file.Path; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BooleanSupplier; import com.google.gson.Gson; +import org.eclipse.core.databinding.observable.Realm; +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.swt.widgets.Display; import org.eclipse.ui.PlatformUI; import org.junit.jupiter.api.AfterEach; @@ -40,6 +45,8 @@ 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.CopilotModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilities; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesSupports; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; @@ -59,13 +66,12 @@ class ModelServiceTests { private Path persistenceDirectory; private ModelService modelService; + private PreferenceStorage preferenceStorage; private FeatureFlags featureFlags; private boolean previewFeaturesEnabled; @BeforeEach void setUp() { - new PreferenceCacheResetter(lsConnection, authStatusManager).reset(); - ChatPersistence persistence = new ChatPersistence(); persistence.setPath(persistenceDirectory.toString()); ByokListModelResponse byokModels = new ByokListModelResponse(); @@ -80,6 +86,7 @@ void setUp() { assertNotNull(featureFlags); previewFeaturesEnabled = featureFlags.isClientPreviewFeatureEnabled(); featureFlags.setClientPreviewFeatureEnabled(false); + preferenceStorage = new PreferenceStorage(lsConnection, authStatusManager); } @AfterEach @@ -88,7 +95,64 @@ void tearDown() { modelService.dispose(); } featureFlags.setClientPreviewFeatureEnabled(previewFeaturesEnabled); - new PreferenceCacheResetter(lsConnection, authStatusManager).reset(); + preferenceStorage.dispose(); + } + + @Test + void testInitialize_PendingPreferences_UpdatesVisionBindingWhenRestored() throws InterruptedException { + CompletableFuture pending = new CompletableFuture<>(); + when(lsConnection.persistence()).thenReturn(pending); + CopilotModel visionModel = createModel("vision", "Vision", true); + visionModel.setCapabilities(new CopilotModelCapabilities( + new CopilotModelCapabilitiesSupports(true, List.of(), false), null)); + when(lsConnection.listModels()) + .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] {visionModel})); + AtomicBoolean supportsVision = new AtomicBoolean(); + AtomicBoolean uiActionProcessed = new AtomicBoolean(); + AtomicReference binding = new AtomicReference<>(); + Display.getDefault().syncExec(() -> { + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); + Realm.runWithDefault(DisplayRealm.getRealm(Display.getDefault()), + () -> binding.set(ISideEffect.create(modelService::isVisionSupported, supportsVision::set))); + Display.getDefault().asyncExec(() -> uiActionProcessed.set(true)); + }); + try { + waitUntil(uiActionProcessed::get); + assertFalse(pending.isDone()); + assertFalse(supportsVision.get()); + assertEquals(PreferenceStorage.State.LOADING, preferenceStorage.getState()); + + ChatPersistence persistence = new ChatPersistence(); + persistence.setPath(persistenceDirectory.toString()); + pending.complete(persistence); + + waitUntil(supportsVision::get); + assertEquals(PreferenceStorage.State.READY, preferenceStorage.getState()); + assertEquals("vision", getActiveModelId()); + } finally { + Display.getDefault().syncExec(() -> binding.get().dispose()); + } + } + + @Test + void testDispose_ActiveModelIsUnavailableWithoutAccessingDisposedObservable() throws Exception { + CopilotModel defaultModel = createModel("gpt-4o", "GPT-4o", true); + when(lsConnection.listModels()) + .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] {defaultModel})); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); + waitUntil(() -> defaultModel.getId().equals(getActiveModelId())); + + CompletableFuture result = new CompletableFuture<>(); + Display.getDefault().syncExec(() -> { + try { + modelService.dispose(); + result.complete(modelService.getActiveModel()); + } catch (Throwable failure) { + result.completeExceptionally(failure); + } + }); + + assertNull(result.get(5, TimeUnit.SECONDS)); } @Test @@ -98,7 +162,7 @@ void testAutoModelAvailableWhenEditorPreviewDisabled() throws InterruptedExcepti when(lsConnection.listModels()) .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> isModelAvailable(defaultModel.getModelKey())); assertTrue(isModelAvailable(autoModel.getModelKey())); @@ -112,7 +176,7 @@ void testAutoModelPolicyChangeRefreshesModelInventory() throws InterruptedExcept CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel }), CompletableFuture.completedFuture(new CopilotModel[] { defaultModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> isModelAvailable(autoModel.getModelKey())); IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); @@ -133,7 +197,7 @@ void testAutoPolicyDisableSelectsServerDefaultAndKeepsAutoPreference() CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel, otherModel }), CompletableFuture.completedFuture(new CopilotModel[] { otherModel, defaultModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> autoModel.getId().equals(getActiveModelId())); IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); @@ -155,7 +219,7 @@ void testAutoPolicyDisableUsesDeterministicFallbackAndKeepsAutoPreference() CompletableFuture.completedFuture(new CopilotModel[] { autoModel, lastModel, firstModel }), CompletableFuture.completedFuture(new CopilotModel[] { lastModel, firstModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> autoModel.getId().equals(getActiveModelId())); IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); @@ -176,7 +240,7 @@ void testAutoPolicyReEnableRestoresPersistedAutoPreference() throws IOException, CompletableFuture.completedFuture(new CopilotModel[] { defaultModel }), CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> autoModel.getId().equals(getActiveModelId())); IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); @@ -197,7 +261,7 @@ void testDefaultKeyCollisionSelectsModelFromCurrentInventory() throws IOExceptio when(lsConnection.listModels()) .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, inventoryModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> isModelAvailable(defaultModel.getModelKey())); AtomicReference activeModel = new AtomicReference<>(); @@ -217,7 +281,7 @@ void testSetActiveModel_AlreadyActiveModelDoesNotPersistPreference() throws Inte when(lsConnection.listModels()) .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] { defaultModel })); - modelService = new ModelService(lsConnection, authStatusManager); + modelService = new ModelService(lsConnection, authStatusManager, preferenceStorage); waitUntil(() -> defaultModel.getId().equals(getActiveModelId())); AtomicReference activeModel = new AtomicReference<>(); @@ -290,18 +354,6 @@ private String readPersistedModel() throws IOException { } private Path getPreferenceFile() { - return persistenceDirectory.resolve(TEST_USER).resolve(ChatBaseService.PREF_FILE_NAME); - } - - private static final class PreferenceCacheResetter extends ChatBaseService { - - private PreferenceCacheResetter(CopilotLanguageServerConnection lsConnection, - AuthStatusManager authStatusManager) { - super(lsConnection, authStatusManager); - } - - private void reset() { - clearUserPreferenceCache(); - } + return persistenceDirectory.resolve(TEST_USER).resolve("pref.json"); } } 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 new file mode 100644 index 000000000..de41c3e86 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorageTest.java @@ -0,0 +1,767 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.services; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +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.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.util.List; +import java.util.Queue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import org.eclipse.core.databinding.observable.Realm; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +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.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage.State; + +class PreferenceStorageTest { + private final CopilotLanguageServerConnection connection = mock(CopilotLanguageServerConnection.class); + private final AuthStatusManager auth = mock(AuthStatusManager.class); + private final ExecutorService worker = mock(ExecutorService.class); + private final ScheduledExecutorService timer = mock(ScheduledExecutorService.class); + private final TestFileAccess files = mock(TestFileAccess.class); + private final Queue work = new ConcurrentLinkedQueue<>(); + private final Queue deadlines = new ConcurrentLinkedQueue<>(); + private final TestRealm realm = new TestRealm(); + private final AtomicLong clock = new AtomicLong(); + private CompletableFuture response; + private PreferenceStorage storage; + + @BeforeEach + void setUp() { + when(auth.isSignedIn()).thenReturn(true); + when(auth.getUserName()).thenReturn("alice"); + response = new CompletableFuture<>(); + when(connection.persistence()).thenAnswer(invocation -> response); + Mockito.doAnswer(invocation -> { + work.add(invocation.getArgument(0)); + return null; + }).when(worker).execute(any(Runnable.class)); + when(timer.schedule(any(Runnable.class), anyLong(), any(TimeUnit.class))).thenAnswer(invocation -> { + deadlines.add(invocation.getArgument(0)); + return mock(ScheduledFuture.class); + }); + storage = new PreferenceStorage(connection, auth, realm, worker, timer, clock::get, files); + } + + @AfterEach + void tearDown() { + storage.dispose(); + realm.drain(); + } + + @Test + void testInitialize_SavedPreferences_RestoresOnlyInRealm() throws Exception { + when(files.read(any(Path.class))).thenReturn(""" + {"chatModel":"model-a","chatModeName":"Ask","userInputs":["hello"], + "skipGitHubJobConfirmDialog":true,"reasoningEffortByModel":{"model-a":"high"}, + "contextWindowByModel":{"model-a":128000}} + """); + + storage.initialize(); + assertEquals(State.LOADING, storage.getState()); + assertNull(storage.getReadyPreferences()); + drainWork(); + response.complete(persistence()); + drainWork(); + assertEquals(State.LOADING, storage.getState()); + realm.drain(); + + assertEquals(State.READY, storage.getReadiness().getValue()); + UserPreference restored = storage.getReadyPreferences(); + assertEquals("model-a", restored.getChatModel()); + assertEquals("Ask", restored.getChatModeName()); + assertEquals("hello", restored.getUserInputs().get(0)); + assertEquals(true, restored.isSkipGitHubJobConfirmDialog()); + assertEquals("high", restored.getReasoningEffort("model-a")); + assertEquals(128000, restored.getContextWindow("model-a")); + assertSame(restored, storage.getReadyPreferences()); + } + + @Test + void testInitialize_MissingFile_UsesFirstRunDefaultsWithoutWriting() throws Exception { + when(files.read(any(Path.class))).thenThrow(new NoSuchFileException("pref.json")); + load(); + + assertEquals(State.READY, storage.getState()); + assertNotNull(storage.getReadyPreferences()); + assertNull(storage.getReadyPreferences().getChatModel()); + assertTrue(storage.getReadyPreferences().getReasoningEffortSnapshot().isEmpty()); + verify(files, never()).write(any(), any()); + } + + @Test + void testInitialize_UnreadableFile_FailsAndDoesNotOverwrite() throws Exception { + when(files.read(any(Path.class))).thenThrow(new AccessDeniedException("pref.json")); + load(); + + assertFailedWithoutWrites(); + storage.initialize(); + drainWork(); + verify(connection).persistence(); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {" ", "null", "[]", "{", "{\"chatModel\":\"x\"} garbage", "{chatModel:'x'}", + "{\"chatModel\":\"x\",}", "/*comment*/{}", "{\"chatModel\":{}}", "{\"contextWindowByModel\":false}"}) + void testInitialize_InvalidJson_FailsAndDoesNotOverwrite(String content) throws Exception { + when(files.read(any(Path.class))).thenReturn(content); + load(); + + assertFailedWithoutWrites(); + } + + @Test + void testInitialize_NullModelMaps_NormalizesSafeDefaults() throws Exception { + when(files.read(any(Path.class))).thenReturn(""" + {"reasoningEffortByModel":null,"contextWindowByModel":null} + """); + load(); + + assertEquals(State.READY, storage.getState()); + UserPreference restored = storage.getReadyPreferences(); + assertNull(restored.getReasoningEffort("model-a")); + assertNull(restored.getContextWindow("model-a")); + restored.setReasoningEffort("model-a", "high"); + assertTrue(restored.setContextWindow("model-a", 128000)); + } + + @ParameterizedTest + @ValueSource(strings = {"{\"chatModel\":\"line\nbreak\"}", "{\"chatModel\":\"tab\tcharacter\"}", + "{\"chatModel\":\"escaped\\\nnewline\"}", "{\"chatModel\":\"single\\'quote\"}", + "{\"skipGitHubJobConfirmDialog\":TRUE}", "{\"chatModel\":NULL}"}) + void testInitialize_LegacyGsonExtensions_FailWithoutOverwriting(String content) throws Exception { + when(files.read(any(Path.class))).thenReturn(content); + load(); + + assertFailedWithoutWrites(); + } + + @Test + void testInitialize_ValidJsonEscapes_RestoresOriginalText() throws Exception { + when(files.read(any(Path.class))).thenReturn(""" + {"userInputs":["line\\nbreak","tab\\tcharacter","quote\\" and slash\\\\","\\u0041", + "single'quote"],"skipGitHubJobConfirmDialog":false} + """); + load(); + + assertEquals(State.READY, storage.getState()); + assertEquals(List.of("line\nbreak", "tab\tcharacter", "quote\" and slash\\", "A", "single'quote"), + storage.getReadyPreferences().getUserInputs()); + } + + @Test + void testInitialize_PendingRpc_LeavesUiPublicationAndGettersAvailable() { + storage.initialize(); + drainWork(); + realm.drain(); + + assertEquals(State.LOADING, storage.getReadiness().getValue()); + assertNull(storage.getReadyPreferences()); + assertFalse(response.isDone()); + verifyNoInteractions(files); + verify(timer).schedule(any(Runnable.class), eq(15L), eq(TimeUnit.SECONDS)); + } + + @Test + void testInitialize_ConcurrentCallersAndRetry_CoalescesPendingAndReadyLoads() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + storage.initialize(); + storage.initialize(); + storage.retry(); + storage.retry(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + UserPreference restored = storage.getReadyPreferences(); + restored.setChatModel("session-choice"); + storage.initialize(); + storage.retry(); + drainWork(); + + verify(connection).persistence(); + assertSame(restored, storage.getReadyPreferences()); + assertEquals("session-choice", storage.getReadyPreferences().getChatModel()); + } + + @Test + void testInitialize_ParallelCallers_ShareOnePendingRequest() throws Exception { + CompletableFuture[] callers = new CompletableFuture[12]; + for (int i = 0; i < callers.length; i++) { + callers[i] = CompletableFuture.runAsync(storage::initialize); + } + CompletableFuture.allOf(callers).get(5, TimeUnit.SECONDS); + drainWork(); + realm.drain(); + + assertEquals(State.LOADING, storage.getReadiness().getValue()); + assertNull(storage.getReadyPreferences()); + verify(connection).persistence(); + } + + @Test + void testInitialize_RpcFailure_RequiresExplicitRetry() throws Exception { + storage.initialize(); + drainWork(); + response.completeExceptionally(new IOException("offline")); + realm.drain(); + assertFailedWithoutWrites(); + storage.initialize(); + drainWork(); + verify(connection).persistence(); + + response = new CompletableFuture<>(); + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"recovered\"}"); + storage.retry(); + storage.retry(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + + assertEquals("recovered", storage.getReadyPreferences().getChatModel()); + verify(connection, times(2)).persistence(); + } + + @Test + void testInitialize_NullRpcResponse_Fails() throws Exception { + storage.initialize(); + drainWork(); + response.complete(null); + drainWork(); + realm.drain(); + + assertFailedWithoutWrites(); + verify(files, never()).read(any()); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {" ", "relative-path", "invalid\u0000path"}) + void testInitialize_InvalidPersistencePath_Fails(String path) throws Exception { + storage.initialize(); + drainWork(); + ChatPersistence invalid = new ChatPersistence(); + invalid.setPath(path); + response.complete(invalid); + drainWork(); + realm.drain(); + + assertFailedWithoutWrites(); + verify(files, never()).read(any()); + } + + @Test + void testInitialize_RpcInvocationThrows_FailsWithoutReading() throws Exception { + when(connection.persistence()).thenThrow(new IllegalStateException("unavailable")); + storage.initialize(); + drainWork(); + realm.drain(); + + assertFailedWithoutWrites(); + verify(files, never()).read(any()); + } + + @Test + void testInitialize_NullRpcFuture_FailsWithoutReading() throws Exception { + when(connection.persistence()).thenReturn(null); + storage.initialize(); + drainWork(); + realm.drain(); + + assertFailedWithoutWrites(); + verify(files, never()).read(any()); + } + + @Test + void testDeadline_PendingRpc_FailsAndCancelsRequest() throws Exception { + storage.initialize(); + drainWork(); + expire(); + realm.drain(); + + assertFailedWithoutWrites(); + assertTrue(response.isCancelled()); + } + + @Test + void testDeadline_BeforeWorkerDispatch_DoesNotStartRpc() throws Exception { + storage.initialize(); + expire(); + drainWork(); + realm.drain(); + + assertFailedWithoutWrites(); + verify(connection, never()).persistence(); + } + + @Test + void testDeadline_DelayedTimer_StillRejectsPublicationAfterDeadline() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + storage.initialize(); + drainWork(); + response.complete(persistence()); + drainWork(); + clock.set(TimeUnit.SECONDS.toNanos(15)); + realm.drain(); + + assertFailedWithoutWrites(); + } + + @Test + void testDeadline_DuringRead_RejectsLateReadResult() throws Exception { + when(files.read(any(Path.class))).thenAnswer(invocation -> { + expire(); + return "{\"chatModel\":\"too-late\"}"; + }); + load(); + + assertFailedWithoutWrites(); + } + + @Test + void testRetry_LateReadFromTimedOutAttempt_DoesNotReplaceSuccessfulRetry() throws Exception { + when(files.read(any(Path.class))).thenAnswer(invocation -> { + Mockito.doReturn("{\"chatModel\":\"new\"}").when(files).read(any(Path.class)); + expire(); + response = new CompletableFuture<>(); + storage.retry(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + return "{\"chatModel\":\"old\"}"; + }); + load(); + + assertEquals(State.READY, storage.getState()); + assertEquals("new", storage.getReadyPreferences().getChatModel()); + verify(connection, times(2)).persistence(); + } + + @Test + void testInitialize_DuringFileRead_GettersRemainAvailableToOtherThreads() throws Exception { + when(files.read(any(Path.class))).thenAnswer(invocation -> { + assertEquals(State.LOADING, CompletableFuture.supplyAsync(storage::getState).get(5, TimeUnit.SECONDS)); + assertNull(CompletableFuture.supplyAsync(storage::getReadyPreferences).get(5, TimeUnit.SECONDS)); + return "{}"; + }); + load(); + + assertEquals(State.READY, storage.getState()); + } + + @Test + void testRetry_LateRpcFromTimedOutAttempt_CannotReplaceNewPreferences() 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\"}"); + storage.retry(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + old.complete(persistence()); + drainWork(); + realm.drain(); + + assertEquals(State.READY, storage.getState()); + assertEquals("new", storage.getReadyPreferences().getChatModel()); + verify(files).read(any()); + verify(files, never()).write(any(), any()); + } + + @Test + void testInitialize_SignedOut_DoesNotReadOrResolvePath() { + when(auth.isSignedIn()).thenReturn(false); + storage.initialize(); + storage.retry(); + drainWork(); + realm.drain(); + + assertEquals(State.UNAVAILABLE, storage.getState()); + assertEquals(State.UNAVAILABLE, storage.getReadiness().getValue()); + assertNull(storage.getReadyPreferences()); + verifyNoInteractions(connection, files); + } + + @Test + void testInitialize_BlankAccount_DoesNotResolvePath() { + when(auth.getUserName()).thenReturn(" "); + storage.initialize(); + drainWork(); + realm.drain(); + + assertEquals(State.UNAVAILABLE, storage.getState()); + verifyNoInteractions(connection, files); + } + + @Test + void testAccountChange_ReadyPreferences_AreInaccessibleBeforeRealmCallback() throws Exception { + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"alice-model\"}"); + load(); + when(auth.getUserName()).thenReturn("bob"); + + assertNull(storage.getReadyPreferences()); + assertEquals(State.UNAVAILABLE, storage.getState()); + storage.persist(); + realm.drain(); + assertEquals(State.UNAVAILABLE, storage.getReadiness().getValue()); + verify(files, never()).write(any(), any()); + verify(connection).persistence(); + } + + @Test + void testAccountChange_QueuedRestoration_CannotPublishOldPreferences() throws Exception { + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"alice-model\"}"); + storage.initialize(); + drainWork(); + response.complete(persistence()); + drainWork(); + when(auth.getUserName()).thenReturn("bob"); + realm.drain(); + + assertEquals(State.UNAVAILABLE, storage.getReadiness().getValue()); + assertNull(storage.getReadyPreferences()); + verify(connection).persistence(); + } + + @Test + void testAccountChange_LateOldRpc_DoesNotAffectNewAccount() throws Exception { + response = new UncancellableFuture(); + CompletableFuture old = response; + storage.initialize(); + drainWork(); + when(auth.getUserName()).thenReturn("bob"); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + realm.drain(); + assertEquals(State.UNAVAILABLE, storage.getState()); + verify(connection).persistence(); + + response = new CompletableFuture<>(); + when(files.read(any(Path.class))).thenReturn("{\"chatModel\":\"bob-model\"}"); + storage.retry(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + old.complete(persistence()); + drainWork(); + realm.drain(); + + assertEquals("bob-model", storage.getReadyPreferences().getChatModel()); + verify(files).read(Path.of(persistence().getPath(), "bob", "pref.json")); + verify(files, never()).read(Path.of(persistence().getPath(), "alice", "pref.json")); + } + + @Test + void testAuthNotification_SignOut_InvalidatesWithoutAutomaticRetry() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + when(auth.isSignedIn()).thenReturn(false); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + assertNull(storage.getReadyPreferences()); + realm.drain(); + assertEquals(State.UNAVAILABLE, storage.getReadiness().getValue()); + + when(auth.isSignedIn()).thenReturn(true); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + drainWork(); + realm.drain(); + verify(connection).persistence(); + assertEquals(State.UNAVAILABLE, storage.getState()); + } + + @Test + void testAuthNotification_SameAccount_DoesNotResetReadySessionChoices() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + UserPreference restored = storage.getReadyPreferences(); + restored.setChatModel("session-choice"); + authListener().onDidCopilotStatusChange(new CopilotStatusResult()); + drainWork(); + realm.drain(); + + assertSame(restored, storage.getReadyPreferences()); + assertEquals("session-choice", storage.getReadyPreferences().getChatModel()); + verify(connection).persistence(); + } + + @Test + void testDispose_PendingRpc_DetachesListenerAndRejectsLateResults() throws Exception { + response = new UncancellableFuture(); + storage.initialize(); + drainWork(); + CopilotAuthStatusListener listener = authListener(); + storage.dispose(); + storage.dispose(); + storage.initialize(); + storage.retry(); + response.complete(persistence()); + drainWork(); + realm.drain(); + + assertEquals(State.DISPOSED, storage.getState()); + assertNull(storage.getReadyPreferences()); + assertTrue(storage.getReadiness().isDisposed()); + verify(auth).removeCopilotAuthStatusListener(listener); + verify(worker).shutdownNow(); + verify(timer).shutdownNow(); + verify(connection).persistence(); + verifyNoInteractions(files); + } + + @Test + void testDispose_QueuedRestoration_DoesNotPublishReady() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + storage.initialize(); + drainWork(); + response.complete(persistence()); + drainWork(); + storage.dispose(); + realm.drain(); + + assertEquals(State.DISPOSED, storage.getState()); + assertNull(storage.getReadyPreferences()); + assertTrue(storage.getReadiness().isDisposed()); + } + + @Test + void testPersist_ReadyPreferences_UsesResolvedAccountPathWithoutRpc() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + storage.getReadyPreferences().setChatModel("saved-model"); + storage.persist(); + + ArgumentCaptor saved = ArgumentCaptor.forClass(String.class); + verify(files).write(eq(Path.of(persistence().getPath(), "alice", "pref.json")), saved.capture()); + assertTrue(saved.getValue().contains("\"chatModel\":\"saved-model\"")); + verify(connection).persistence(); + } + + @Test + void testPersist_ObsoleteAccountPreference_CannotSaveNewAccount() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + UserPreference original = storage.getReadyPreferences(); + when(auth.getUserName()).thenReturn("bob"); + response = new CompletableFuture<>(); + load(); + + storage.persist(original); + verify(files, never()).write(any(), any()); + storage.getReadyPreferences().setChatModel("bob-model"); + storage.persist(storage.getReadyPreferences()); + + ArgumentCaptor saved = ArgumentCaptor.forClass(String.class); + verify(files).write(eq(Path.of(persistence().getPath(), "bob", "pref.json")), saved.capture()); + assertTrue(saved.getValue().contains("\"chatModel\":\"bob-model\"")); + verify(connection, times(2)).persistence(); + } + + @Test + void testPersist_EqualButNotAuthoritativePreference_DoesNotWrite() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + UserPreference unrelated = new UserPreference(); + assertEquals(storage.getReadyPreferences(), unrelated); + + storage.persist(unrelated); + storage.persist(null); + + verify(files, never()).write(any(), any()); + } + + @Test + void testPersist_DisposedPreference_DoesNotWrite() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + UserPreference original = storage.getReadyPreferences(); + storage.dispose(); + + storage.persist(original); + storage.persist(); + + verify(files, never()).write(any(), any()); + } + + @Test + void testPersist_DuringFileWrite_GettersRemainAvailableToOtherThreads() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + UserPreference original = storage.getReadyPreferences(); + Mockito.doAnswer(invocation -> { + assertEquals(State.READY, CompletableFuture.supplyAsync(storage::getState).get(5, TimeUnit.SECONDS)); + assertSame(original, CompletableFuture.supplyAsync(storage::getReadyPreferences).get(5, TimeUnit.SECONDS)); + return null; + }).when(files).write(any(), any()); + + storage.persist(original); + + verify(files).write(any(), any()); + } + + @Test + void testPersist_AccountChangesDuringWrite_UsesOnlyCapturedAccountPath() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + Mockito.doAnswer(invocation -> { + when(auth.getUserName()).thenReturn("bob"); + assertNull(storage.getReadyPreferences()); + return null; + }).when(files).write(any(), any()); + storage.persist(); + + verify(files).write(eq(Path.of(persistence().getPath(), "alice", "pref.json")), any()); + assertEquals(State.UNAVAILABLE, storage.getState()); + verify(connection).persistence(); + } + + @Test + void testPersist_WriteFailure_PreservesReadySessionChoices() throws Exception { + when(files.read(any(Path.class))).thenReturn("{}"); + load(); + storage.getReadyPreferences().setChatModel("session-choice"); + Mockito.doThrow(new IOException("read-only")).when(files).write(any(), any()); + storage.persist(); + + assertEquals(State.READY, storage.getState()); + assertEquals("session-choice", storage.getReadyPreferences().getChatModel()); + } + + @Test + void testPersist_StillLoading_DoesNotWriteDefaultsOrResolveAgain() throws Exception { + storage.initialize(); + drainWork(); + storage.persist(); + + assertEquals(State.LOADING, storage.getState()); + verifyNoInteractions(files); + verify(connection).persistence(); + } + + private CopilotAuthStatusListener authListener() { + ArgumentCaptor listener = ArgumentCaptor.forClass(CopilotAuthStatusListener.class); + verify(auth).addCopilotAuthStatusListener(listener.capture()); + return listener.getValue(); + } + + private void expire() { + clock.addAndGet(TimeUnit.SECONDS.toNanos(15)); + deadlines.remove().run(); + } + + private void load() { + storage.initialize(); + drainWork(); + response.complete(persistence()); + drainWork(); + realm.drain(); + } + + private void assertFailedWithoutWrites() throws IOException { + assertEquals(State.FAILED, storage.getState()); + assertEquals(State.FAILED, storage.getReadiness().getValue()); + assertNull(storage.getReadyPreferences()); + storage.persist(); + verify(files, never()).write(any(), any()); + } + + private ChatPersistence persistence() { + ChatPersistence result = new ChatPersistence(); + result.setPath(Path.of("preferences").toAbsolutePath().toString()); + return result; + } + + private void drainWork() { + while (!work.isEmpty()) { + work.remove().run(); + } + } + + private static class TestRealm extends Realm { + private final Queue publications = new ConcurrentLinkedQueue<>(); + + @Override + public boolean isCurrent() { + return true; + } + + @Override + public void asyncExec(Runnable runnable) { + publications.add(runnable); + } + + void drain() { + while (!publications.isEmpty()) { + publications.remove().run(); + } + } + } + + private static class UncancellableFuture extends CompletableFuture { + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return false; + } + } + + // A public concrete boundary adapter supports Mockito's inline mock maker across OSGi class loaders. + public static class TestFileAccess implements PreferenceStorage.FileAccess { + @Override + public String read(Path path) throws IOException { + return Files.readString(path); + } + + @Override + public void write(Path path, String content) throws IOException { + Files.writeString(path, content); + } + } +} 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 7b36a797a..ffc8eb6eb 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 @@ -4,180 +4,563 @@ package com.microsoft.copilot.eclipse.ui.chat.services; 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.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; - -import java.lang.reflect.Field; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.timeout; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BooleanSupplier; + +import org.eclipse.swt.SWT; +import org.eclipse.core.databinding.observable.Realm; +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.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Link; +import org.eclipse.ui.PlatformUI; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.osgi.service.event.Event; import org.osgi.service.event.EventHandler; import com.microsoft.copilot.eclipse.core.AuthStatusManager; -import com.microsoft.copilot.eclipse.core.chat.InputNavigation; +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.BuiltInChatModeManager; +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.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.ui.swt.DropdownButton; +import com.microsoft.copilot.eclipse.ui.chat.Messages; +import com.microsoft.copilot.eclipse.ui.chat.PreferenceStatus; @ExtendWith(MockitoExtension.class) class UserPreferenceServiceTest { - @Mock - private CopilotLanguageServerConnection mockLsConnection; - + private CopilotLanguageServerConnection connection; @Mock - private AuthStatusManager mockAuthStatusManager; + private AuthStatusManager auth; + @TempDir + private Path directory; - private UserPreferenceService userPreferenceService; + private PreferenceStorage storage; + private UserPreferenceService service; + private Shell shell; + private DropdownButton picker; + private PreferenceStatus status; @BeforeEach void setUp() { - when(mockAuthStatusManager.isSignedIn()).thenReturn(false); + lenient().when(connection.listConversationModes(any())) + .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { + builtInMode("Ask", "Ask"), builtInMode("Agent", "Agent"), builtInMode("Plan", "Plan")})); } @AfterEach void tearDown() { - if (userPreferenceService != null) { - userPreferenceService.dispose(); - } + runOnUi(() -> { + if (shell != null) { + shell.dispose(); + } + if (service != null) { + service.dispose(); + } + if (storage != null) { + storage.dispose(); + } + }); } @Test - void testAuthStatusChangedEventHandler_UserSignsOut_ClearsUserPreferenceCache() { - // Arrange - userPreferenceService = new UserPreferenceService(mockLsConnection, mockAuthStatusManager); + void testFirstInitialization_PendingModeDiscovery_ProcessesSwtAndPublishesModesAfterCompletion() throws Exception { + when(auth.isSignedIn()).thenReturn(true); + when(auth.getUserName()).thenReturn("user"); + CompletableFuture preferences = new CompletableFuture<>(); + CompletableFuture modes = new CompletableFuture<>(); + when(connection.persistence()).thenReturn(preferences); + when(connection.listConversationModes(any())).thenReturn(modes); + writePreferences("{\"chatModeName\":\"Ask\"}"); + CompletableFuture uiAction = new CompletableFuture<>(); + Display.getDefault().asyncExec(() -> { + try { + storage = new PreferenceStorage(connection, auth); + createControls(); + Display.getDefault().asyncExec(() -> { + try { + uiAction.complete(!picker.getEnabled() && storage.getState() == PreferenceStorage.State.LOADING); + } catch (Throwable error) { + uiAction.completeExceptionally(error); + } + }); + } catch (Throwable error) { + uiAction.completeExceptionally(error); + } + }); + try { + assertTrue(uiAction.get(5, TimeUnit.SECONDS), "Cold mode discovery must not prevent SWT processing"); + verify(connection, timeout(5000)).listConversationModes(any()); + assertFalse(modes.isDone()); + assertFalse(preferences.isDone()); + ConversationMode ask = new ConversationMode(); + ask.setId("pending-discovery-ask"); + ask.setName("Ask"); + ask.setKind("Ask"); + ask.setBuiltIn(true); + modes.complete(new ConversationMode[] {ask}); + preferences.complete(persistence()); + awaitUi(() -> picker.getEnabled() && "Ask".equals(picker.getSelectedItemId()) + && BuiltInChatModeManager.INSTANCE.getBuiltInModeById("pending-discovery-ask") != null); + runOnUi(() -> assertEquals(ChatMode.Ask, service.getActiveChatMode())); + } finally { + modes.complete(new ConversationMode[0]); + preferences.completeExceptionally(new IllegalStateException("test cleanup")); + } + } - // Set up initial state with input navigation - setInputNavigationForService(new InputNavigation()); - assertNotNull(getInputNavigationFromService(), "Input navigation should be set initially"); + @Test + void testInitialization_PreferencesBeforeModeDiscovery_GatesPlanActionsAndRefreshesConsumers() throws Exception { + CompletableFuture modes = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(modes); + writePreferences("{\"chatModeName\":\"Plan\"}"); + AtomicBoolean actionsReady = new AtomicBoolean(); + AtomicBoolean resolvedModeNotification = new AtomicBoolean(); + AtomicInteger notifications = new AtomicInteger(); + AtomicReference readinessBinding = new AtomicReference<>(); + IEventBroker broker = PlatformUI.getWorkbench().getService(IEventBroker.class); + EventHandler listener = event -> { + notifications.incrementAndGet(); + if (BuiltInChatModeManager.INSTANCE.getBuiltInModeById("delayed-plan") != null) { + resolvedModeNotification.set(event.getProperty(IEventBroker.DATA) == ChatMode.Agent); + } + }; + broker.subscribe(CopilotEventConstants.TOPIC_CHAT_MODE_CHANGED, listener); + try { + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> storage.getState() == PreferenceStorage.State.READY && notifications.get() > 0); + runOnUi(() -> { + Realm.runWithDefault(DisplayRealm.getRealm(Display.getDefault()), () -> readinessBinding.set( + ISideEffect.create(service::isActiveModeReady, actionsReady::set))); + assertEquals("Plan", service.getActiveModeNameOrId()); + assertFalse(service.isActiveModeReady(), "Unresolved Plan must not be sent as Agent"); + }); + assertFalse(actionsReady.get()); + + modes.complete(new ConversationMode[] {builtInMode("delayed-plan", "Plan")}); + awaitUi(() -> actionsReady.get() && resolvedModeNotification.get()); + runOnUi(() -> { + assertEquals("Plan", service.getActiveModeNameOrId()); + assertEquals("delayed-plan", + BuiltInChatModeManager.INSTANCE.getBuiltInModeByDisplayName("Plan").getId()); + }); + } finally { + broker.unsubscribe(listener); + runOnUi(() -> { + if (readinessBinding.get() != null) { + readinessBinding.get().dispose(); + } + }); + modes.complete(new ConversationMode[0]); + } + } - // Get the auth status changed event handler - EventHandler authHandler = getAuthStatusChangedEventHandler(); - assertNotNull(authHandler, "Auth status changed event handler should be available"); + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testModeDiscovery_FailedOrEmpty_ShowsRetryWithoutReloadingReadyPreferences(boolean empty) throws Exception { + CompletableFuture initial = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(initial); + Path file = writePreferences("{\"chatModeName\":\"Plan\"}"); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> "Plan".equals(service.getActiveModeNameOrId())); + UserPreference restored = storage.getReadyPreferences(); + runOnUi(() -> { + assertTrue(status.getVisible()); + assertEquals(Messages.modeDiscoveryLoading, statusMessage().getText()); + assertFalse(service.isActiveModeReady()); + }); + if (empty) { + initial.complete(new ConversationMode[0]); + } else { + initial.completeExceptionally(new IllegalStateException("mode discovery failed")); + } + awaitUi(() -> Messages.modeDiscoveryFailed.equals(statusMessage().getText())); + + CompletableFuture retry = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(retry); + runOnUi(() -> { + assertEquals(PreferenceStorage.State.READY, storage.getState()); + assertTrue(status.getVisible()); + Link retryLink = Arrays.stream(status.getChildren()).filter(Link.class::isInstance) + .map(Link.class::cast).findFirst().orElseThrow(); + assertTrue(retryLink.getEnabled()); + retryLink.notifyListeners(SWT.Selection, new org.eclipse.swt.widgets.Event()); + service.retryModeDiscovery(); + service.retryModeDiscovery(); + assertFalse(service.isActiveModeReady()); + }); + awaitUi(() -> Messages.modeDiscoveryLoading.equals(statusMessage().getText())); + runOnUi(() -> { + Link retryLink = Arrays.stream(status.getChildren()).filter(Link.class::isInstance) + .map(Link.class::cast).findFirst().orElseThrow(); + assertFalse(retryLink.getEnabled()); + }); + verify(connection, timeout(5000).times(2)).listConversationModes(any()); + retry.complete(new ConversationMode[] {builtInMode("recovered-plan", "Plan")}); + awaitUi(() -> service.isActiveModeReady() && !status.getVisible()); + runOnUi(() -> { + assertSame(restored, storage.getReadyPreferences()); + assertEquals("Plan", service.getActiveModeNameOrId()); + assertEquals("recovered-plan", + BuiltInChatModeManager.INSTANCE.getBuiltInModeByDisplayName("Plan").getId()); + }); + verify(connection).persistence(); + assertEquals("{\"chatModeName\":\"Plan\"}", Files.readString(file)); + } - Event signOutEvent = createAuthStatusEvent(CopilotStatusResult.NOT_SIGNED_IN); + @Test + void testModeDiscovery_SupersededAttempt_CannotReplaceRecoveredInventory() throws Exception { + CompletableFuture obsolete = new CompletableFuture<>(); + CompletableFuture current = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(obsolete, current); + writePreferences("{\"chatModeName\":\"Plan\"}"); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> "Plan".equals(service.getActiveModeNameOrId())); + verify(connection, timeout(5000)).listConversationModes(any()); + UserPreference restored = storage.getReadyPreferences(); - // Act - authHandler.handleEvent(signOutEvent); + 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); + service.retryModeDiscovery(); + }); + verify(connection, timeout(5000).times(2)).listConversationModes(any()); + current.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()); + assertSame(restored, storage.getReadyPreferences()); + }); + verify(connection).persistence(); + } - // Assert - assertNull(getInputNavigationFromService(), "Input navigation should be cleared when user signs out"); + @Test + void testModeDiscovery_DisposedDuringManualRetry_RejectsLateSuccess() throws Exception { + when(connection.listConversationModes(any())) + .thenReturn(CompletableFuture.failedFuture(new IllegalStateException("offline"))); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> service.getModeDiscoveryState() == UserPreferenceService.ModeDiscoveryState.FAILED); + CompletableFuture retry = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(retry); + runOnUi(service::retryModeDiscovery); + verify(connection, timeout(5000).times(2)).listConversationModes(any()); + runOnUi(() -> { + shell.dispose(); + service.dispose(); + }); + retry.complete(new ConversationMode[] {builtInMode("disposed-retry-mode", "Plan")}); + runOnUi(() -> { + assertFalse(service.isActiveModeReady()); + assertEquals(UserPreferenceService.ModeDiscoveryState.UNAVAILABLE, service.getModeDiscoveryState()); + assertNull(BuiltInChatModeManager.INSTANCE.getBuiltInModeById("disposed-retry-mode")); + }); } @Test - void testAuthStatusChangedEventHandler_SignOutThenSignIn() { - // Arrange - userPreferenceService = new UserPreferenceService(mockLsConnection, mockAuthStatusManager); + void testModeDiscovery_AccountChanges_RejectsOldAccountModes() throws Exception { + CompletableFuture modes = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(modes); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + verify(connection, timeout(5000)).listConversationModes(any()); + awaitUi(() -> picker.getEnabled()); + + when(auth.getUserName()).thenReturn("other-user"); + modes.complete(new ConversationMode[] {builtInMode("obsolete-account-mode", "Ask")}); + runOnUi(() -> { + assertNull(BuiltInChatModeManager.INSTANCE.getBuiltInModeById("obsolete-account-mode")); + assertNull(service.getActiveModeNameOrId()); + }); + } - EventHandler authHandler = getAuthStatusChangedEventHandler(); - assertNotNull(authHandler, "Auth status changed event handler should be available"); + @Test + void testModeDiscovery_DisposedService_DoesNotPublishLateModes() throws Exception { + CompletableFuture modes = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(modes); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + verify(connection, timeout(5000)).listConversationModes(any()); + runOnUi(() -> { + shell.dispose(); + service.dispose(); + }); + + modes.complete(new ConversationMode[] {builtInMode("disposed-service-mode", "Ask")}); + runOnUi(() -> + assertNull(BuiltInChatModeManager.INSTANCE.getBuiltInModeById("disposed-service-mode"))); + } - Event signOutEvent = createAuthStatusEvent(CopilotStatusResult.NOT_SIGNED_IN); - Event signInEvent = createAuthStatusEvent(CopilotStatusResult.OK, "test-user"); + @Test + void testModeDiscovery_AgentPolicyDisabled_KeepsAskViewAfterDelayedDiscovery() throws Exception { + CompletableFuture modes = new CompletableFuture<>(); + when(connection.listConversationModes(any())).thenReturn(modes); + writePreferences("{\"chatModeName\":\"Agent\"}"); + FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags(); + boolean original = flags.isAgentModeEnabled(); + try { + flags.setAgentModeEnabled(false); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + verify(connection, timeout(5000)).listConversationModes(any()); + awaitUi(() -> picker.getEnabled()); + modes.complete(new ConversationMode[] { + builtInMode("delayed-policy-agent", "Agent"), builtInMode("delayed-policy-ask", "Ask")}); + awaitUi(() -> BuiltInChatModeManager.INSTANCE.getBuiltInModeById("delayed-policy-ask") != null); + runOnUi(() -> assertEquals(ChatMode.Ask, service.getActiveChatMode())); + } finally { + flags.setAgentModeEnabled(original); + } + } - // Act - Sign out then sign in - authHandler.handleEvent(signOutEvent); - assertNull(getInputNavigationFromService(), "Input navigation should be null after sign out"); + @Test + void testInitialization_RestoresModeHistoryAndConfirmationWithoutSaving() throws Exception { + String saved = """ + {"chatModeName":"Ask","userInputs":["first","second"],"skipGitHubJobConfirmDialog":true} + """; + Path file = writePreferences(saved); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> "Ask".equals(service.getActiveModeNameOrId()) && !status.getVisible()); + runOnUi(() -> { + assertEquals(ChatMode.Ask, service.getActiveChatMode()); + assertEquals("second", service.getPreviousInput("")); + assertEquals("first", service.getPreviousInput("")); + assertTrue(service.isSkipGitHubJobConfirmDialog()); + assertTrue(picker.getEnabled()); + assertFalse(status.getVisible()); + }); + assertEquals(saved, Files.readString(file), "Restoration is not a user edit"); + } - authHandler.handleEvent(signInEvent); - InputNavigation initialNavigation = new InputNavigation(List.of("input1", "input2")); - setInputNavigationForService(initialNavigation); - assertNotNull(getInputNavigationFromService(), "Input navigation should be set initially"); + @Test + void testInitialization_AgentPolicyDisabled_RestoresAskView() throws Exception { + writePreferences("{\"chatModeName\":\"Agent\"}"); + FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags(); + boolean original = flags.isAgentModeEnabled(); + try { + flags.setAgentModeEnabled(false); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> "Agent".equals(service.getActiveModeNameOrId())); + runOnUi(() -> assertEquals(ChatMode.Ask, service.getActiveChatMode())); + } finally { + flags.setAgentModeEnabled(original); + } + } - // Assert - After sign in, input navigation should be restored - assertNotNull(getInputNavigationFromService(), "Input navigation should be restored after sign in"); - assertEquals("input2", getInputNavigationFromService().getLatestInput(), "Input navigation should be restored"); + @Test + void testRetry_FailedLoadKeepsPickerDisabledThenRestoresSavedChoice() throws Exception { + writePreferences("{\"chatModeName\":\"Ask\"}"); + startAuthenticated(CompletableFuture.failedFuture(new IllegalStateException("offline"))); + awaitUi(() -> storage.getReadiness().getValue() == PreferenceStorage.State.FAILED); + runOnUi(() -> { + assertFalse(picker.getEnabled()); + assertTrue(status.getVisible()); + Label message = Arrays.stream(status.getChildren()).filter(Label.class::isInstance) + .map(Label.class::cast).findFirst().orElseThrow(); + assertEquals(Messages.preferenceLoadFailed, message.getText()); + service.setActiveChatMode("Agent"); + assertNull(service.getActiveModeNameOrId()); + }); + when(connection.persistence()).thenReturn(CompletableFuture.completedFuture(persistence())); + runOnUi(() -> { + Link retry = Arrays.stream(status.getChildren()).filter(Link.class::isInstance) + .map(Link.class::cast).findFirst().orElseThrow(); + assertTrue(retry.getEnabled()); + retry.notifyListeners(SWT.Selection, new org.eclipse.swt.widgets.Event()); + }); + awaitUi(() -> picker.getEnabled() && "Ask".equals(service.getActiveModeNameOrId())); } @Test - void testAuthStatusChangedEventHandler_UserSignsIn_ReloadsBuiltInModes() { - // Arrange - userPreferenceService = new UserPreferenceService(mockLsConnection, mockAuthStatusManager); + 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); + awaitUi(() -> storage.getReadiness().getValue() == PreferenceStorage.State.UNAVAILABLE); + runOnUi(() -> { + assertFalse(picker.getEnabled()); + assertNull(service.getActiveModeNameOrId()); + assertEquals("", service.getPreviousInput("")); + }); + } - EventHandler authHandler = getAuthStatusChangedEventHandler(); - assertNotNull(authHandler, "Auth status changed event handler should be available"); + @Test + void testModeChange_AfterRestoration_UpdatesSharedStateAndPersists() throws Exception { + writePreferences("{\"chatModeName\":\"Ask\"}"); + startAuthenticated(CompletableFuture.completedFuture(persistence())); + awaitUi(() -> "Ask".equals(service.getActiveModeNameOrId())); + runOnUi(() -> { + service.setActiveChatMode("Agent"); + assertEquals("Agent", service.getActiveModeNameOrId()); + assertNotNull(storage.getReadyPreferences()); + assertEquals("Agent", storage.getReadyPreferences().getChatModeName()); + }); + assertTrue(Files.readString(directory.resolve("user").resolve("pref.json")).contains("Agent")); + } - Event signInEvent = createAuthStatusEvent(CopilotStatusResult.OK, "test-user"); + @Test + void testInitialization_DeadlineExpires_ShowsFailureAndRetryWithoutEnablingPicker() throws Exception { + when(auth.isSignedIn()).thenReturn(true); + when(auth.getUserName()).thenReturn("user"); + CompletableFuture pending = new CompletableFuture<>(); + when(connection.persistence()).thenReturn(pending); + ScheduledExecutorService timer = mock(ScheduledExecutorService.class); + AtomicReference deadline = new AtomicReference<>(); + when(timer.schedule(any(Runnable.class), eq(15L), eq(TimeUnit.SECONDS))).thenAnswer(invocation -> { + deadline.set(invocation.getArgument(0)); + return mock(ScheduledFuture.class); + }); + AtomicLong clock = new AtomicLong(); + runOnUi(() -> { + storage = new PreferenceStorage(connection, auth, DisplayRealm.getRealm(Display.getDefault()), + Executors.newSingleThreadExecutor(), timer, clock::get, new PreferenceStorage.FileAccess() { + @Override + public String read(Path path) throws IOException { + return Files.readString(path); + } + + @Override + public void write(Path path, String content) throws IOException { + Files.writeString(path, content); + } + }); + createControls(); + }); + awaitUi(() -> storage.getReadiness().getValue() == PreferenceStorage.State.LOADING); + runOnUi(() -> assertFalse(picker.getEnabled())); + clock.set(TimeUnit.SECONDS.toNanos(15)); + deadline.get().run(); + awaitUi(() -> storage.getReadiness().getValue() == PreferenceStorage.State.FAILED); + runOnUi(() -> { + assertFalse(picker.getEnabled()); + assertTrue(status.getVisible()); + Link retry = Arrays.stream(status.getChildren()).filter(Link.class::isInstance) + .map(Link.class::cast).findFirst().orElseThrow(); + assertTrue(retry.getEnabled()); + }); + } - // Act - Simulate user sign in - authHandler.handleEvent(signInEvent); + private void startAuthenticated(CompletableFuture rpc) { + when(auth.isSignedIn()).thenReturn(true); + when(auth.getUserName()).thenReturn("user"); + when(connection.persistence()).thenReturn(rpc); + runOnUi(() -> { + storage = new PreferenceStorage(connection, auth); + createControls(); + }); + } - // Assert - No exception should be thrown and the handler should complete successfully - // Note: Since BuiltInChatModeManager is a singleton and we can't easily mock it in this test, - // we primarily verify that the event handler doesn't throw exceptions when reloading modes. - // The actual reloading functionality is tested through integration tests. - assertNotNull(authHandler, "Auth handler should still be functional after handling sign-in event"); + private void createControls() { + service = new UserPreferenceService(connection, auth, storage); + shell = new Shell(Display.getDefault()); + status = new PreferenceStatus(shell, storage, service); + picker = new DropdownButton(shell, SWT.NONE); + service.bindChatModePicker(picker); } - /** - * Helper method to create an auth status changed event - */ - private Event createAuthStatusEvent(String status) { - return createAuthStatusEvent(status, null); + private ChatPersistence persistence() { + ChatPersistence result = new ChatPersistence(); + result.setPath(directory.toString()); + return result; } - /** - * Helper method to create an auth status changed event with user - */ - private Event createAuthStatusEvent(String status, String user) { - CopilotStatusResult statusResult = new CopilotStatusResult(); - statusResult.setStatus(status); - if (user != null) { - statusResult.setUser(user); - } + private Label statusMessage() { + return Arrays.stream(status.getChildren()).filter(Label.class::isInstance) + .map(Label.class::cast).findFirst().orElseThrow(); + } - Map eventProperties = new HashMap<>(); - eventProperties.put(IEventBroker.DATA, statusResult); - return new Event(CopilotEventConstants.TOPIC_AUTH_STATUS_CHANGED, eventProperties); + private static ConversationMode builtInMode(String id, String name) { + ConversationMode mode = new ConversationMode(); + mode.setId(id); + mode.setName(name); + mode.setKind(name); + mode.setBuiltIn(true); + return mode; } - /** - * Helper method to access private authStatusChangedEventHandler field for - * testing - */ - private EventHandler getAuthStatusChangedEventHandler() { - try { - Field field = UserPreferenceService.class.getDeclaredField("authStatusChangedEventHandler"); - field.setAccessible(true); - return (EventHandler) field.get(userPreferenceService); - } catch (Exception e) { - throw new RuntimeException("Failed to access authStatusChangedEventHandler field", e); - } + private Path writePreferences(String content) throws Exception { + Path file = directory.resolve("user").resolve("pref.json"); + Files.createDirectories(file.getParent()); + Files.writeString(file, content); + return file; } - /** - * Helper method to access private inputNavigation field for testing - */ - private InputNavigation getInputNavigationFromService() { - try { - Field field = UserPreferenceService.class.getDeclaredField("inputNavigation"); - field.setAccessible(true); - return (InputNavigation) field.get(userPreferenceService); - } catch (Exception e) { - throw new RuntimeException("Failed to access inputNavigation field", e); + private static void runOnUi(Runnable action) { + AtomicReference failure = new AtomicReference<>(); + Display.getDefault().syncExec(() -> { + try { + action.run(); + } catch (Throwable error) { + failure.set(error); + } + }); + if (failure.get() != null) { + fail("UI action failed", failure.get()); } } - /** - * Helper method to set private inputNavigation field for testing - */ - private void setInputNavigationForService(InputNavigation inputNavigation) { - try { - Field field = UserPreferenceService.class.getDeclaredField("inputNavigation"); - field.setAccessible(true); - field.set(userPreferenceService, inputNavigation); - } catch (Exception e) { - throw new RuntimeException("Failed to set inputNavigation field", e); - } + private static void awaitUi(BooleanSupplier condition) throws InterruptedException { + AtomicBoolean complete = new AtomicBoolean(); + long deadline = System.nanoTime() + java.util.concurrent.TimeUnit.SECONDS.toNanos(5); + do { + runOnUi(() -> complete.set(condition.getAsBoolean())); + if (complete.get()) { + return; + } + Thread.sleep(10); + } while (System.nanoTime() < deadline); + assertTrue(complete.get(), "Timed out waiting for the preference UI"); } -} \ No newline at end of file +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ActionBar.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ActionBar.java index 43930884d..16d8528f6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ActionBar.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ActionBar.java @@ -14,6 +14,8 @@ import java.util.UUID; import org.apache.commons.lang3.StringUtils; +import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IFolder; @@ -76,6 +78,7 @@ import com.microsoft.copilot.eclipse.ui.chat.contextwindow.ContextSizeDonut; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; import com.microsoft.copilot.eclipse.ui.chat.services.ModelService; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage; import com.microsoft.copilot.eclipse.ui.chat.services.ReferencedFileService; import com.microsoft.copilot.eclipse.ui.chat.services.UserPreferenceService; import com.microsoft.copilot.eclipse.ui.chat.tools.JavaDebuggerToolAdapter; @@ -145,6 +148,8 @@ public ActionBar(Composite parent, int style, ChatServiceManager chatServiceMana this.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); this.setData(CssConstants.CSS_ID_KEY, "chat-action-bar-wrapper"); this.chatServiceManager = chatServiceManager; + new PreferenceStatus(this, chatServiceManager.getPreferenceStorage(), + chatServiceManager.getUserPreferenceService()); this.updateSendButtonToCancelButtonHandler = event -> { updateButtonState(SendOrCancelButtonStates.CANCEL_ENABLED); }; @@ -337,6 +342,24 @@ private void updateTableLayout(Table table) { // Update send to job button and send button together updateButtonsLayout(); + PreferenceStorage storage = chatServiceManager.getPreferenceStorage(); + Realm.runWithDefault(storage.getReadiness().getRealm(), () -> { + ISideEffect readinessEffect = ISideEffect.create(() -> { + return storage.getReadiness().getValue() == PreferenceStorage.State.READY + && chatServiceManager.getUserPreferenceService().isActiveModeReady(); + }, ready -> { + if (isDisposed()) { + return; + } + mcpToolButton.setEnabled(ready); + autoBreakpointButton.setEnabled(ready); + if (isSendButton) { + updateButtonState(StringUtils.isBlank(inputTextViewer.getContent()) + ? SendOrCancelButtonStates.SEND_DISABLED : SendOrCancelButtonStates.SEND_ENABLED); + } + }); + addDisposeListener(event -> readinessEffect.dispose()); + }); } /** @@ -433,7 +456,7 @@ public void updateButtonsLayout() { this.sendToJobButton = UiUtils.createIconButton(this.bottomRightButtonsComposite, SWT.PUSH | SWT.FLAT); boolean hasText = !StringUtils.isBlank(this.inputTextViewer.getContent()); - this.sendToJobButton.setEnabled(hasText); + this.sendToJobButton.setEnabled(hasText && isSendButton && preferencesReady()); this.sendToJobButton.setImage(hasText ? sendToJobImage : sendToJobDisabledImage); this.sendToJobButton.setToolTipText(Messages.chat_actionBar_sendToJobButton_Tooltip); AccessibilityUtils.addAccessibilityNameForUiComponent(this.sendToJobButton, @@ -457,7 +480,7 @@ public void widgetSelected(SelectionEvent e) { this.sendDisabledImage = CopilotImages.getImage(CopilotImages.IMG_CHAT_SEND_DISABLED); this.btnMsgToggle = UiUtils.createIconButton(bottomRightButtonsComposite, SWT.PUSH | SWT.FLAT); boolean isEnabled = !StringUtils.isBlank(this.inputTextViewer.getContent()); - this.btnMsgToggle.setEnabled(isEnabled); + this.btnMsgToggle.setEnabled(isEnabled && preferencesReady()); this.btnMsgToggle.setImage(isEnabled ? this.sendImage : this.sendDisabledImage); this.btnMsgToggle.setToolTipText(Messages.chat_actionBar_sendButton_Tooltip); GridData sendGd = new GridData(SWT.RIGHT, SWT.CENTER, false, false); @@ -477,6 +500,9 @@ public void widgetSelected(org.eclipse.swt.events.SelectionEvent e) { AccessibilityUtils.addAccessibilityNameForUiComponent(this.btnMsgToggle, Messages.chat_actionBar_sendButton_Tooltip); } + if (!isSendButton) { + updateButtonState(SendOrCancelButtonStates.CANCEL_ENABLED); + } // Refresh the layout this.bottomRightButtonsComposite.requestLayout(); } @@ -762,6 +788,10 @@ public void setInputTextViewerContent(String content) { * Handles the send message event. */ public void handleSendMessage() { + if (!preferencesReady()) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot send chat before preferences are ready")); + return; + } updateButtonState(SendOrCancelButtonStates.CANCEL_ENABLED); String message = this.inputTextViewer.getContent(); String workDoneToken = UUID.randomUUID().toString(); @@ -773,6 +803,10 @@ public void handleSendMessage() { * Handles the send to job button click event. Shows a dialog to inform user about git repository requirement. */ private void handleSendToJob() { + if (!preferencesReady()) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot send a job before preferences are ready")); + return; + } ChatServiceManager chatServiceManager = (ChatServiceManager) CopilotCore.getPlugin().getChatServiceManager(); if (chatServiceManager != null) { UserPreferenceService userPreferenceService = chatServiceManager.getUserPreferenceService(); @@ -853,6 +887,9 @@ public boolean isSendButton() { } private void updateButtonState(SendOrCancelButtonStates state) { + if (state == SendOrCancelButtonStates.SEND_ENABLED && !preferencesReady()) { + state = SendOrCancelButtonStates.SEND_DISABLED; + } switch (state) { case SEND_ENABLED: isSendButton = true; @@ -873,6 +910,12 @@ private void updateButtonState(SendOrCancelButtonStates state) { default: break; } + + } + + private boolean preferencesReady() { + return chatServiceManager.getPreferenceStorage().getState() == PreferenceStorage.State.READY + && chatServiceManager.getUserPreferenceService().isActiveModeReady(); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 29b8eb3d4..71ff46920 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -90,6 +90,7 @@ import com.microsoft.copilot.eclipse.ui.chat.services.AgentToolService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; import com.microsoft.copilot.eclipse.ui.chat.services.DebugEventAutoResponseHandler; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage; import com.microsoft.copilot.eclipse.ui.chat.services.ReferencedFileService; import com.microsoft.copilot.eclipse.ui.chat.services.TodoListService; import com.microsoft.copilot.eclipse.ui.chat.viewers.AfterLoginWelcomeViewer; @@ -1029,6 +1030,14 @@ public void setFocus() { private void onSendInternal(String workDoneToken, String message, String agentSlug, String agentJobWorkspaceFolder, boolean createNewTurn) { + if (chatServiceManager.getPreferenceStorage().getState() != PreferenceStorage.State.READY + || !chatServiceManager.getUserPreferenceService().isActiveModeReady()) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot send chat before preferences are ready")); + if (actionBar != null && !actionBar.isDisposed()) { + actionBar.resetSendButton(); + } + return; + } // Persist the user input to history chatServiceManager.getUserPreferenceService().addInputToHistory(message); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/HandoffContainer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/HandoffContainer.java index 6d986e0c7..b943ca428 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/HandoffContainer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/HandoffContainer.java @@ -7,6 +7,8 @@ import java.util.ArrayList; import java.util.List; +import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; import org.eclipse.swt.SWT; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; @@ -22,6 +24,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode.HandOff; import com.microsoft.copilot.eclipse.ui.chat.services.ChatFontService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage; import com.microsoft.copilot.eclipse.ui.swt.CssConstants; /** @@ -60,6 +63,15 @@ public HandoffContainer(Composite parent, ChatServiceManager chatServiceManager, // Initially hidden this.setVisible(false); ((GridData) this.getLayoutData()).exclude = true; + PreferenceStorage storage = chatServiceManager.getPreferenceStorage(); + Realm.runWithDefault(storage.getReadiness().getRealm(), () -> { + ISideEffect effect = ISideEffect.create(storage.getReadiness()::getValue, state -> { + if (!isDisposed() && state != PreferenceStorage.State.READY) { + hide(); + } + }); + addDisposeListener(event -> effect.dispose()); + }); } /** @@ -78,6 +90,10 @@ public void hide() { * Show handoff buttons based on the current mode and update their content. */ public void show() { + if (chatServiceManager.getPreferenceStorage().getState() != PreferenceStorage.State.READY) { + hide(); + return; + } // Clear existing buttons clearHandoffs(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java index bb946f7a3..9e911649a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java @@ -21,6 +21,12 @@ public final class Messages extends NLS { public static String chat_warnWidget_defaultErrorMsg; public static String chat_warnWidget_byokQuotaUsageMessage; public static String configureModes; + public static String preferenceLoading; + public static String preferenceLoadFailed; + public static String preferenceUnavailable; + public static String preferenceRetry; + public static String modeDiscoveryLoading; + public static String modeDiscoveryFailed; public static String agentMessageWidget_openInBrowserButton; public static String agentMessageWidget_openInBrowserTooltip; public static String agentMessageWidget_openJobListButton; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/PreferenceStatus.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/PreferenceStatus.java new file mode 100644 index 000000000..7e2766421 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/PreferenceStatus.java @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat; + +import org.eclipse.core.databinding.observable.Realm; +import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; +import org.eclipse.swt.SWT; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Link; + +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage; +import com.microsoft.copilot.eclipse.ui.chat.services.PreferenceStorage.State; +import com.microsoft.copilot.eclipse.ui.chat.services.UserPreferenceService; +import com.microsoft.copilot.eclipse.ui.chat.services.UserPreferenceService.ModeDiscoveryState; + +/** + * Inline preference-loading status, independent of model and conversation loading. + */ +public class PreferenceStatus extends Composite { + /** + * Creates a status message and an explicit retry action. + * + * @param parent parent control + * @param storage shared chat preference storage + */ + public PreferenceStatus(Composite parent, PreferenceStorage storage) { + this(parent, storage, null); + } + + /** + * Creates status and retry controls for preference loading and independent mode discovery. + * + * @param parent parent control + * @param storage shared chat preference storage + * @param preferences mode-discovery owner, or {@code null} for preference-only status + */ + public PreferenceStatus(Composite parent, PreferenceStorage storage, UserPreferenceService preferences) { + super(parent, SWT.NONE); + setLayout(new GridLayout(2, false)); + GridData data = new GridData(SWT.FILL, SWT.CENTER, true, false); + setLayoutData(data); + Label message = new Label(this, SWT.WRAP); + message.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + message.setData("org.eclipse.swtbot.widget.key", "preference-status"); + Link retry = new Link(this, SWT.NONE); + retry.setText("" + Messages.preferenceRetry + ""); + retry.setData("org.eclipse.swtbot.widget.key", "preference-retry"); + GridData retryData = new GridData(SWT.RIGHT, SWT.CENTER, false, false); + retry.setLayoutData(retryData); + retry.addListener(SWT.Selection, event -> { + if (storage.getState() == State.READY && preferences != null) { + preferences.retryModeDiscovery(); + } else { + storage.retry(); + } + }); + Realm.runWithDefault(storage.getReadiness().getRealm(), () -> { + ISideEffect effect = ISideEffect.create(() -> { + return new Readiness(storage.getReadiness().getValue(), + preferences == null ? ModeDiscoveryState.READY : preferences.getModeDiscoveryState()); + }, readiness -> { + if (isDisposed()) { + return; + } + State state = readiness.preferences(); + boolean modePending = state == State.READY && readiness.modes() != ModeDiscoveryState.READY; + boolean visible = state != State.DISPOSED && (state != State.READY || modePending); + data.exclude = !visible; + setVisible(visible); + String text = switch (state) { + case LOADING -> Messages.preferenceLoading; + case FAILED -> Messages.preferenceLoadFailed; + default -> Messages.preferenceUnavailable; + }; + if (modePending) { + text = readiness.modes() == ModeDiscoveryState.LOADING + ? Messages.modeDiscoveryLoading : Messages.modeDiscoveryFailed; + } + message.setText(text); + boolean canRetry = state == State.FAILED || state == State.UNAVAILABLE + || (modePending && readiness.modes() != ModeDiscoveryState.LOADING); + retryData.exclude = !canRetry; + retry.setVisible(canRetry); + retry.setEnabled(canRetry); + requestLayout(); + }); + addDisposeListener(event -> effect.dispose()); + }); + } + + private record Readiness(State preferences, ModeDiscoveryState modes) { + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java index 050d5d487..19668f85c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java @@ -27,6 +27,7 @@ import org.eclipse.ui.model.WorkbenchLabelProvider; import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.ui.CopilotImages; import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.chat.services.ReferencedFileService; @@ -197,10 +198,9 @@ public void getName(AccessibleEvent event) { private void setupUnsupportedFileDisplay() { // Set warning icon lblfileIcon.setImage(CopilotImages.getSharedImage(ISharedImages.IMG_OBJS_WARN_TSK)); - // Set tooltip with model name - String modelName = CopilotUi.getPlugin().getChatServiceManager().getModelService().getActiveModel() - .getModelName(); - String tooltipText = String.format(Messages.chat_referencedFile_noVision_tooltip, modelName); + CopilotModel model = CopilotUi.getPlugin().getChatServiceManager().getModelService().getActiveModel(); + String tooltipText = model == null ? Messages.chat_referencedFile_modelUnavailable_tooltip + : String.format(Messages.chat_referencedFile_noVision_tooltip, model.getModelName()); lblfileIcon.setToolTipText(tooltipText); lblFileName.setToolTipText(tooltipText); lblClose.setToolTipText(tooltipText); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties index 671b22f88..43aa47fa8 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties @@ -77,3 +77,10 @@ confirmation_message_fileOperation=The model wants to access sensitive file ({0} # Misc confirmation_autoApprovedDescription=Auto-approved from dialog +# Chat preference loading +preferenceLoading=Loading chat preferences... +preferenceLoadFailed=Could not load chat preferences. Retry to restore your saved settings. +preferenceUnavailable=Chat preferences are unavailable. +preferenceRetry=Retry +modeDiscoveryLoading=Loading chat modes... +modeDiscoveryFailed=Could not load chat modes. Retry to use your selected mode. diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseService.java index 0ad576304..b87636a2c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatBaseService.java @@ -3,40 +3,22 @@ package com.microsoft.copilot.eclipse.ui.chat.services; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.concurrent.ExecutionException; - -import com.google.gson.Gson; -import com.google.gson.JsonSyntaxException; -import org.apache.commons.lang3.StringUtils; import org.eclipse.core.databinding.observable.Realm; import org.eclipse.core.databinding.observable.value.IObservableValue; -import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jface.databinding.swt.DisplayRealm; import org.eclipse.swt.widgets.Display; import com.microsoft.copilot.eclipse.core.AuthStatusManager; -import com.microsoft.copilot.eclipse.core.CopilotCore; -import com.microsoft.copilot.eclipse.core.chat.UserPreference; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; -import com.microsoft.copilot.eclipse.core.utils.PlatformUtils; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; /** * Base class for chat services. */ public abstract class ChatBaseService { - protected static final Gson gson = new Gson(); - protected static final String PREF_FILE_NAME = "pref.json"; - protected CopilotLanguageServerConnection lsConnection; protected AuthStatusManager authStatusManager; - protected String persistentPath; - private static UserPreference userPreference; - /** * Constructor for the ChatBaseService. */ @@ -45,53 +27,6 @@ protected ChatBaseService(CopilotLanguageServerConnection lsConnection, AuthStat this.authStatusManager = authStatusManager; } - /** - * Get User Preference. - */ - protected synchronized UserPreference getUserPreference() { - if (userPreference != null) { - return userPreference; - } - - Path path = getPersistentFilePath(); - if (path != null) { - try { - String jsonContent = PlatformUtils.readFileContent(path); - if (!jsonContent.isEmpty()) { - userPreference = gson.fromJson(jsonContent, UserPreference.class); - if (userPreference != null) { - return userPreference; - } - } - } catch (JsonSyntaxException e) { - CopilotCore.LOGGER.error("Failed to get user preference, will generate a new one.", e); - } - } - - userPreference = new UserPreference(); - return userPreference; - } - - /** - * Clear User Preference. - */ - protected void clearUserPreferenceCache() { - userPreference = null; - } - - /** - * Persist User Preference. - */ - public synchronized void persistUserPreference() { - Path path = getPersistentFilePath(); - if (path == null) { - return; - } - - String jsonContent = gson.toJson(userPreference); - PlatformUtils.writeFileContent(path, jsonContent); - } - /** * Ensures operations run in the correct Realm. * @@ -113,32 +48,6 @@ protected void ensureRealm(Runnable runnable) { } } - /** - * Get the path for the persistent file. - */ - private @Nullable Path getPersistentFilePath() { - if (!this.authStatusManager.isSignedIn()) { - return null; - } - if (this.persistentPath == null) { - try { - ChatPersistence chatPersistence = this.lsConnection.persistence().get(); - this.persistentPath = chatPersistence.getPath(); - } catch (InterruptedException | ExecutionException e) { - CopilotCore.LOGGER.error("Failed to get persistent path", e); - return null; - } - } - - final String user = this.authStatusManager.getUserName(); - if (StringUtils.isBlank(user)) { - CopilotCore.LOGGER.error(new IllegalStateException("User name is empty")); - return null; - } - - return Paths.get(this.persistentPath, user, PREF_FILE_NAME); - } - /** * Update the value of an observable in its realm. * @@ -147,7 +56,11 @@ protected void ensureRealm(Runnable runnable) { */ protected void updateObservable(IObservableValue observable, final T value) { if (observable != null) { - observable.getRealm().asyncExec(() -> observable.setValue(value)); + observable.getRealm().asyncExec(() -> { + if (!observable.isDisposed()) { + observable.setValue(value); + } + }); } } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java index 62e6c4a6a..6f356be96 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java @@ -24,6 +24,7 @@ public class ChatServiceManager implements IChatServiceManager { private ModelService modelService; private ByokService byokService; private UserPreferenceService userPreferenceService; + private PreferenceStorage preferenceStorage; private AvatarService avatarService; private AgentToolService agentToolService; private FileToolService fileToolService; @@ -45,8 +46,9 @@ public ChatServiceManager() { this.lsConnection = CopilotCore.getPlugin().getCopilotLanguageServer(); this.authStatusManager = CopilotCore.getPlugin().getAuthStatusManager(); chatCompletionService = new ChatCompletionService(this.lsConnection, this.authStatusManager); - modelService = new ModelService(this.lsConnection, this.authStatusManager); - userPreferenceService = new UserPreferenceService(this.lsConnection, this.authStatusManager); + preferenceStorage = new PreferenceStorage(this.lsConnection, this.authStatusManager); + userPreferenceService = new UserPreferenceService(this.lsConnection, this.authStatusManager, preferenceStorage); + modelService = new ModelService(this.lsConnection, this.authStatusManager, preferenceStorage); avatarService = new AvatarService(this.authStatusManager); agentToolService = new AgentToolService(this.lsConnection); fileToolService = new FileToolService(this.lsConnection); @@ -112,6 +114,15 @@ public UserPreferenceService getUserPreferenceService() { return userPreferenceService; } + /** + * Returns the lifecycle-owned chat preference storage. + * + * @return shared preference storage + */ + public PreferenceStorage getPreferenceStorage() { + return preferenceStorage; + } + /** * Get the model service. * @@ -208,6 +219,7 @@ public void dispose() { this.chatCompletionService.dispose(); this.modelService.dispose(); this.userPreferenceService.dispose(); + this.preferenceStorage.dispose(); this.agentToolService.dispose(); this.referencedFileService.dispose(); this.mcpConfigService.dispose(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java index 88bcdf711..c612e8163 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java @@ -7,6 +7,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -50,6 +51,12 @@ * BYOK integration, UI binding, and communicates with other services through pure events. */ public class ModelService extends ChatBaseService { + private final PreferenceStorage preferenceStorage; + private ISideEffect readinessSideEffect; + private volatile boolean disposed; + private long modelGeneration; + private Job modelJob; + private String modelAccount; // models for the model picker private IObservableValue> modelObservable; @@ -79,27 +86,51 @@ public class ModelService extends ChatBaseService { /** * Constructor for the ModelService. */ - public ModelService(CopilotLanguageServerConnection lsConnection, AuthStatusManager authStatusManager) { + public ModelService(CopilotLanguageServerConnection lsConnection, AuthStatusManager authStatusManager, + PreferenceStorage preferenceStorage) { super(lsConnection, authStatusManager); + this.preferenceStorage = preferenceStorage; ensureRealm(() -> { modelObservable = new WritableValue<>(new HashMap<>(), HashMap.class); activeModelObservable = new WritableValue<>(null, CopilotModel.class); - Map initialEfforts = Map.of(); - UserPreference initialPreference = getUserPreference(); - if (initialPreference != null) { - initialEfforts = initialPreference.getReasoningEffortSnapshot(); - } - reasoningEffortObservable = new WritableValue<>(initialEfforts, Map.class); - Map initialContextWindows = Map.of(); - if (initialPreference != null) { - initialContextWindows = initialPreference.getContextWindowSnapshot(); - } - contextWindowObservable = new WritableValue<>(initialContextWindows, Map.class); + reasoningEffortObservable = new WritableValue<>(Map.of(), Map.class); + contextWindowObservable = new WritableValue<>(Map.of(), Map.class); + readinessSideEffect = ISideEffect.create(preferenceStorage.getReadiness()::getValue, state -> { + UserPreference preference = getUserPreference(); + if (state == PreferenceStorage.State.READY && preference != null) { + currentChatMode = "Ask".equalsIgnoreCase(preference.getChatModeName()) ? ChatMode.Ask : ChatMode.Agent; + FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags(); + if (flags != null && !flags.isAgentModeEnabled()) { + currentChatMode = ChatMode.Ask; + } + reasoningEffortObservable.setValue(preference.getReasoningEffortSnapshot()); + contextWindowObservable.setValue(preference.getContextWindowSnapshot()); + reconcileReasoningEfforts(); + reconcileContextWindows(); + updateModelsForChatMode(currentChatMode); + } else { + activeModelObservable.setValue(null); + reasoningEffortObservable.setValue(Map.of()); + contextWindowObservable.setValue(Map.of()); + if (state == PreferenceStorage.State.UNAVAILABLE) { + copilotModels = new HashMap<>(); + registeredByokModels = new HashMap<>(); + defaultModel = null; + fallbackModel = null; + modelObservable.setValue(Map.of()); + modelGeneration++; + if (modelJob != null) { + modelJob.cancel(); + } + } + } + }); }); initializeEventHandlers(); subscribeToEvents(); + preferenceStorage.initialize(); initializeModels(); } @@ -124,10 +155,12 @@ private void initializeEventHandlers() { if (property instanceof Map modelsMap) { @SuppressWarnings("unchecked") Map> byokModels = (Map>) modelsMap; - saveRegisteredByokModels(byokModels); - reconcileReasoningEfforts(); - reconcileContextWindows(); - ensureRealm(() -> updateModelsForChatMode(currentChatMode)); + ensureRealm(() -> { + saveRegisteredByokModels(byokModels); + reconcileReasoningEfforts(); + reconcileContextWindows(); + updateModelsForChatMode(currentChatMode); + }); } }; @@ -183,32 +216,49 @@ private void subscribeToEvents() { } private void initializeModels() { - if (authStatusManager.isSignedIn()) { - Job job = new Job("Fetching all models...") { + ensureRealm(() -> { + if (!authStatusManager.isSignedIn()) { + return; + } + long generation = ++modelGeneration; + String account = authStatusManager.getUserName(); + modelJob = new Job("Fetching all models...") { @Override protected IStatus run(IProgressMonitor monitor) { try { - fetchCopilotModels(); - fetchByokModels(); - reconcileReasoningEfforts(); - reconcileContextWindows(); + CopilotModel[] models = lsConnection.listModels().get(); + ByokListModelResponse byok = lsConnection.listByokModels(new ByokListModelParams(null, false)).get(); ensureRealm(() -> { + if (generation != modelGeneration || !authStatusManager.isSignedIn() + || !Objects.equals(account, authStatusManager.getUserName())) { + return; + } + saveCopilotModels(models); + modelAccount = account; + if (byok != null && byok.getModels() != null) { + saveRegisteredByokModels(byok.getModels().stream() + .collect(java.util.stream.Collectors.groupingBy(ByokModel::getProviderName))); + } + reconcileReasoningEfforts(); + reconcileContextWindows(); updateModelsForChatMode(currentChatMode); }); - } catch (InterruptedException | ExecutionException e) { + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return Status.CANCEL_STATUS; + } catch (ExecutionException e) { CopilotCore.LOGGER.error("Failed to initialize models", e); } return Status.OK_STATUS; } }; - job.setSystem(true); - job.schedule(); - } + modelJob.setSystem(true); + modelJob.schedule(); + }); } - private void fetchCopilotModels() throws InterruptedException, ExecutionException { - CopilotModel[] modelArray = lsConnection.listModels().get(); + private void saveCopilotModels(CopilotModel[] modelArray) { Map newModels = new HashMap<>(); CopilotModel newDefaultModel = null; CopilotModel newFallbackModel = null; @@ -232,15 +282,6 @@ private void fetchCopilotModels() throws InterruptedException, ExecutionExceptio fallbackModel = newFallbackModel; } - private void fetchByokModels() throws InterruptedException, ExecutionException { - ByokListModelResponse response = lsConnection.listByokModels(new ByokListModelParams(null, false)).get(); - if (response != null && response.getModels() != null) { - Map> modelsByProvider = response.getModels().stream() - .collect(java.util.stream.Collectors.groupingBy(ByokModel::getProviderName)); - saveRegisteredByokModels(modelsByProvider); - } - } - private void saveRegisteredByokModels(Map> byokModels) { Map newByokModels = new HashMap<>(); for (List providerModels : byokModels.values()) { @@ -268,15 +309,21 @@ private String restoreActiveModel() { } private void updateModelsForChatMode(ChatMode chatMode) { + if (getUserPreference() == null) { + return; + } String scope = modeToScope(chatMode); // Filter models for the current mode from combined models final Map modelsForCurrentMode = new HashMap<>(); Map allModels = new HashMap<>(); - allModels.putAll(copilotModels); + boolean currentAccount = Objects.equals(modelAccount, authStatusManager.getUserName()); + if (currentAccount) { + allModels.putAll(copilotModels); + } // TODO: need to remove this logic after group policy is available FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags(); - if (flags == null || flags.isByokEnabled()) { + if (currentAccount && (flags == null || flags.isByokEnabled())) { allModels.putAll(registeredByokModels); } @@ -299,6 +346,9 @@ private void updateModelsForChatMode(ChatMode chatMode) { * user preference or falling back to default. */ private void validateAndSetActiveModelForMode(Map modelsForCurrentMode) { + if (getUserPreference() == null) { + return; + } CopilotModel currentActive = getActiveModel(); String restoredModelKey = restoreActiveModel(); CopilotModel restoredModel = restoredModelKey == null ? null : modelsForCurrentMode.get(restoredModelKey); @@ -335,8 +385,11 @@ private CopilotModel selectReplacementModel(Map modelsForC private void persistModelSelection(CopilotModel model) { UserPreference preference = getUserPreference(); + if (preference == null) { + return; + } preference.setChatModel(model.getModelKey()); - CompletableFuture.runAsync(this::persistUserPreference); + persistUserPreferenceAsync(preference); } private String modeToScope(ChatMode mode) { @@ -355,13 +408,22 @@ private String modeToScope(ChatMode mode) { } private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) { + if (copilotStatusResult.isSignedIn() != authStatusManager.isSignedIn() + || (copilotStatusResult.isSignedIn() + && !Objects.equals(copilotStatusResult.getUser(), authStatusManager.getUserName()))) { + return; + } String status = copilotStatusResult.getStatus(); switch (status) { case CopilotStatusResult.OK, CopilotStatusResult.NOT_AUTHORIZED: initializeModels(); break; default: - disposeAllSideEffects(); + ensureRealm(() -> { + modelGeneration++; + activeModelObservable.setValue(null); + modelObservable.setValue(Map.of()); + }); break; } } @@ -372,6 +434,10 @@ private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) { * @param modelName the name of the model */ public void setActiveModel(String modelName) { + if (getUserPreference() == null) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot change model before preferences are ready")); + return; + } Map currentModels = modelObservable.getValue(); final CopilotModel model = currentModels.values().stream() @@ -383,10 +449,6 @@ public void setActiveModel(String modelName) { if (activeModel != null && activeModel.getModelKey().equals(model.getModelKey())) { return; } - // Persist asynchronously to avoid deadlock: persistUserPreference() calls - // persistence().get() which blocks waiting for the LSP listener thread. - // If called on the UI thread while the listener is in syncExec, both threads - // deadlock. persistModelSelection(model); // Update observable @@ -400,7 +462,11 @@ public void setActiveModel(String modelName) { * @return the active model */ public CopilotModel getActiveModel() { - return activeModelObservable.getValue(); + if (disposed) { + return null; + } + CopilotModel model = activeModelObservable.getValue(); + return getUserPreference() == null ? null : model; } /** @@ -489,10 +555,12 @@ public void setSelectedReasoningEffort(CopilotModel model, String reasoningEffor String key = model.getModelKey(); UserPreference preference = getUserPreference(); if (preference == null) { + CopilotCore.LOGGER.error( + new IllegalStateException("Cannot change reasoning effort before preferences are ready")); return; } preference.setReasoningEffort(key, reasoningEffort); - CompletableFuture.runAsync(this::persistUserPreference); + persistUserPreferenceAsync(preference); // Publish a fresh snapshot to drive bound picker re-renders. The actual rendering reads // resolveEffectiveReasoningEffort (which queries UserPreference), so this observable serves // purely as a change signal. @@ -509,6 +577,9 @@ public void setSelectedReasoningEffort(CopilotModel model, String reasoningEffor * failed) so a transient outage cannot wipe every stored selection. */ private void reconcileReasoningEfforts() { + if (!Objects.equals(modelAccount, authStatusManager.getUserName())) { + return; + } if (copilotModels.isEmpty() && registeredByokModels.isEmpty()) { return; } @@ -542,7 +613,7 @@ private void reconcileReasoningEfforts() { } } if (preference.setReasoningEfforts(reconciled)) { - CompletableFuture.runAsync(this::persistUserPreference); + persistUserPreferenceAsync(preference); ensureRealm(() -> reasoningEffortObservable.setValue(preference.getReasoningEffortSnapshot())); } } @@ -619,12 +690,13 @@ public void setSelectedContextWindow(CopilotModel model, Integer contextWindow) } UserPreference preference = getUserPreference(); if (preference == null) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot change context window before preferences are ready")); return; } if (!preference.setContextWindow(model.getModelKey(), contextWindow)) { return; } - CompletableFuture.runAsync(this::persistUserPreference); + persistUserPreferenceAsync(preference); // Publish a fresh snapshot to drive bound picker re-renders. The actual rendering reads // resolveEffectiveContextWindowText (which queries UserPreference), so this observable serves purely as a change // signal. @@ -641,6 +713,9 @@ public void setSelectedContextWindow(CopilotModel model, Integer contextWindow) * failed) so a transient outage cannot wipe every stored selection. */ private void reconcileContextWindows() { + if (!Objects.equals(modelAccount, authStatusManager.getUserName())) { + return; + } if (copilotModels.isEmpty() && registeredByokModels.isEmpty()) { return; } @@ -670,7 +745,7 @@ private void reconcileContextWindows() { } } if (preference.setContextWindows(reconciled)) { - CompletableFuture.runAsync(this::persistUserPreference); + persistUserPreferenceAsync(preference); ensureRealm(() -> contextWindowObservable.setValue(preference.getContextWindowSnapshot())); } } @@ -716,7 +791,16 @@ public void bindModelPicker(final DropdownButton picker) { }); // Store the side effects for later disposal - modelButtonSideEffects.put(picker, new ISideEffect[] { modelsSideEffect, activeModelSideEffect }); + ISideEffect readinessEffect = ISideEffect.create(() -> { + return preferenceStorage.getReadiness().getValue() == PreferenceStorage.State.READY + && !modelObservable.getValue().isEmpty(); + }, ready -> { + if (!picker.isDisposed()) { + picker.setEnabled(ready); + } + }); + modelButtonSideEffects.put(picker, + new ISideEffect[] { modelsSideEffect, activeModelSideEffect, readinessEffect }); // Add a dispose listener to auto-unbind when the combo is disposed picker.addDisposeListener(e -> unbindModelPicker(picker)); @@ -792,24 +876,31 @@ public void unbindActionBarForSupportVisionChange(ActionBar actionBar) { } } - private void disposeAllSideEffects() { - ensureRealm(() -> { - for (ISideEffect[] effects : modelButtonSideEffects.values()) { - for (ISideEffect effect : effects) { - if (effect != null) { - effect.dispose(); - } - } - } - }); - - modelButtonSideEffects.clear(); - } - /** * Dispose the service. */ public void dispose() { + if (disposed) { + return; + } + disposed = true; + if (modelJob != null) { + modelJob.cancel(); + } + super.ensureRealm(() -> { + readinessSideEffect.dispose(); + for (DropdownButton picker : List.copyOf(modelButtonSideEffects.keySet())) { + unbindModelPicker(picker); + } + if (actionBarSideEffect != null) { + actionBarSideEffect.dispose(); + actionBarSideEffect = null; + } + modelObservable.dispose(); + activeModelObservable.dispose(); + reasoningEffortObservable.dispose(); + contextWindowObservable.dispose(); + }); if (eventBroker != null) { eventBroker.unsubscribe(authStatusChangedEventHandler); eventBroker.unsubscribe(chatModeChangedEventHandler); @@ -819,8 +910,29 @@ public void dispose() { eventBroker.unsubscribe(customModeModelChangedEventHandler); eventBroker = null; } + } + + private UserPreference getUserPreference() { + return disposed ? null : preferenceStorage.getReadyPreferences(); + } - // Should not call disposeAllSideEffects here due to issue #1301 - modelButtonSideEffects.clear(); + private void persistUserPreferenceAsync(UserPreference preference) { + CompletableFuture.runAsync(() -> { + if (!disposed) { + preferenceStorage.persist(preference); + } + }); + } + + @Override + protected void ensureRealm(Runnable runnable) { + if (disposed) { + return; + } + super.ensureRealm(() -> { + if (!disposed) { + runnable.run(); + } + }); } } 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 new file mode 100644 index 000000000..476d5ba31 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/PreferenceStorage.java @@ -0,0 +1,512 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.services; + +import java.io.IOException; +import java.io.StringReader; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.FutureTask; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.function.LongSupplier; + +import com.google.gson.Gson; +import com.google.gson.JsonSyntaxException; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonToken; +import org.apache.commons.lang3.StringUtils; +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.jface.databinding.swt.DisplayRealm; +import org.eclipse.swt.widgets.Display; + +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.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; + +/** + * Owns account-scoped chat preferences and their asynchronous initial restoration. + */ +public class PreferenceStorage { + private static final Gson GSON = new Gson(); + private static final long LOAD_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(15); + + /** + * Availability of preferences for the current account. + */ + public enum State { + UNAVAILABLE, LOADING, READY, FAILED, DISPOSED + } + + private final Object lock = new Object(); + private final Object saveLock = new Object(); + private final CopilotLanguageServerConnection connection; + private final AuthStatusManager auth; + private final ExecutorService worker; + private final ScheduledExecutorService timer; + private final LongSupplier clock; + private final FileAccess files; + private final WritableValue readiness; + private final CopilotAuthStatusListener authListener; + private State state = State.UNAVAILABLE; + private String account; + private long generation; + private Attempt attempt; + private UserPreference preferences; + private Path preferencePath; + + /** + * Creates storage without starting RPC or file work. + * + * @param connection the existing language server connection + * @param auth the existing account manager + */ + public PreferenceStorage(CopilotLanguageServerConnection connection, AuthStatusManager auth) { + this(connection, auth, DisplayRealm.getRealm(Display.getDefault()), + Executors.newCachedThreadPool(runnable -> daemonThread(runnable, "Copilot preference loading")), + Executors.newSingleThreadScheduledExecutor(runnable -> daemonThread(runnable, "Copilot preference deadline")), + System::nanoTime, new FileAccess() { + @Override + public String read(Path path) throws IOException { + return Files.readString(path); + } + + @Override + public void write(Path path, String content) throws IOException { + if (Files.notExists(path)) { + Files.createDirectories(path.getParent()); + } + Files.writeString(path, content); + } + }); + } + + PreferenceStorage(CopilotLanguageServerConnection connection, AuthStatusManager auth, Realm realm, + ExecutorService worker, ScheduledExecutorService timer, LongSupplier clock, FileAccess files) { + this.connection = connection; + this.auth = auth; + this.worker = worker; + this.timer = timer; + this.clock = clock; + this.files = files; + this.readiness = new WritableValue<>(realm, State.UNAVAILABLE, State.class); + this.account = currentAccount(); + this.authListener = status -> refreshAccount(); + auth.addCopilotAuthStatusListener(authListener); + } + + /** + * Returns current readiness without waiting for RPC, file work, or the UI thread. + * + * @return readiness applicable to the current account + */ + public State getState() { + refreshAccount(); + synchronized (lock) { + return state == State.DISPOSED || Objects.equals(account, currentAccount()) ? state : State.UNAVAILABLE; + } + } + + /** + * Returns the UI-Realm observable, available before initialization starts. + * + * @return the readiness observable + */ + public IObservableValue getReadiness() { + return readiness; + } + + /** + * Starts the first load, coalescing callers without implicitly retrying failures. + */ + public void initialize() { + start(false); + } + + /** + * Explicitly retries unavailable or failed preferences without replacing ready choices. + */ + public void retry() { + start(true); + } + + /** + * Returns the authoritative preference object only when ready for the current account. + * + * @return loaded preferences, or {@code null} when unavailable + */ + public UserPreference getReadyPreferences() { + refreshAccount(); + synchronized (lock) { + return state == State.READY && Objects.equals(account, currentAccount()) ? preferences : null; + } + } + + /** + * Synchronously saves ready preferences to their already resolved account path, without RPC. + */ + public void persist() { + persist(getReadyPreferences()); + } + + /** + * Saves a captured preference only while it remains authoritative for the current account. + */ + void persist(UserPreference expected) { + if (expected == null || getReadyPreferences() != expected) { + return; + } + synchronized (saveLock) { + refreshAccount(); + try { + Path path; + String json; + synchronized (lock) { + if (state != State.READY || preferences != expected + || !Objects.equals(account, currentAccount())) { + return; + } + path = preferencePath; + synchronized (expected) { + json = GSON.toJson(expected); + } + } + files.write(path, json); + } catch (IOException | RuntimeException exception) { + CopilotCore.LOGGER.error("Failed to save chat preferences", exception); + } + } + } + + /** + * Invalidates pending work and detaches listeners without waiting for background operations. + */ + public void dispose() { + Attempt previous; + synchronized (lock) { + if (state == State.DISPOSED) { + return; + } + previous = clear(State.DISPOSED); + } + auth.removeCopilotAuthStatusListener(authListener); + cancel(previous); + worker.shutdownNow(); + timer.shutdownNow(); + readiness.getRealm().asyncExec(() -> { + if (!readiness.isDisposed()) { + readiness.setValue(State.DISPOSED); + readiness.dispose(); + } + }); + } + + private static Thread daemonThread(Runnable runnable, String name) { + Thread thread = new Thread(runnable, name); + thread.setDaemon(true); + return thread; + } + + private String currentAccount() { + String user = auth.getUserName(); + return auth.isSignedIn() && StringUtils.isNotBlank(user) ? user : null; + } + + private void refreshAccount() { + Attempt previous; + long version; + synchronized (lock) { + String current = currentAccount(); + if (state == State.DISPOSED || Objects.equals(account, current)) { + return; + } + account = current; + previous = clear(State.UNAVAILABLE); + version = generation; + } + cancel(previous); + publishState(version, State.UNAVAILABLE); + } + + private Attempt clear(State next) { + final Attempt previous = attempt; + attempt = null; + preferences = null; + preferencePath = null; + state = next; + generation++; + return previous; + } + + private void start(boolean retry) { + refreshAccount(); + Attempt next; + synchronized (lock) { + if (account == null || state == State.DISPOSED || state == State.LOADING || state == State.READY + || (!retry && state == State.FAILED)) { + return; + } + state = State.LOADING; + next = new Attempt(++generation, account, clock.getAsLong()); + attempt = next; + } + publishState(next.generation, State.LOADING); + try { + ScheduledFuture timeout = + timer.schedule(() -> fail(next, null), LOAD_TIMEOUT_NANOS, TimeUnit.NANOSECONDS); + synchronized (lock) { + if (attempt == next) { + next.timeout = timeout; + } else { + timeout.cancel(false); + } + } + dispatch(next, () -> resolve(next)); + } catch (RuntimeException exception) { + fail(next, exception); + } + } + + private void dispatch(Attempt pending, Runnable runnable) { + FutureTask task = new FutureTask<>(() -> { + if (isCurrent(pending)) { + runnable.run(); + } + }, null); + synchronized (lock) { + if (!matches(pending)) { + return; + } + pending.work = task; + } + try { + worker.execute(task); + } catch (RuntimeException exception) { + fail(pending, exception); + } + } + + private void resolve(Attempt pending) { + try { + CompletableFuture rpc = connection.persistence(); + if (rpc == null) { + throw new IllegalStateException("Persistence request returned no future"); + } + boolean applicable; + synchronized (lock) { + applicable = matches(pending); + if (applicable) { + pending.rpc = rpc; + } + } + if (!applicable) { + rpc.cancel(true); + return; + } + rpc.whenComplete((result, failure) -> { + if (failure != null) { + fail(pending, failure); + } else { + dispatch(pending, () -> read(pending, result)); + } + }); + } catch (RuntimeException exception) { + fail(pending, exception); + } + } + + private void read(Attempt pending, ChatPersistence result) { + try { + if (result == null || StringUtils.isBlank(result.getPath())) { + throw new IllegalArgumentException("Persistence response has no path"); + } + Path directory = Path.of(result.getPath()); + if (!directory.isAbsolute()) { + throw new IllegalArgumentException("Persistence path is not absolute"); + } + Path path = directory.resolve(pending.account).resolve("pref.json"); + UserPreference restored; + try { + restored = parse(files.read(path)); + } catch (NoSuchFileException exception) { + restored = new UserPreference(); + } + publishPreferences(pending, path, restored); + } catch (IOException | RuntimeException exception) { + fail(pending, exception); + } + } + + private UserPreference parse(String json) throws IOException { + rejectLegacyJsonExtensions(json); + try (JsonReader reader = new JsonReader(new StringReader(json))) { + reader.setLenient(false); + if (reader.peek() != JsonToken.BEGIN_OBJECT) { + throw new JsonSyntaxException("Preferences must contain an object"); + } + // The adapter preserves JsonReader strictness, unlike Gson.fromJson(JsonReader, Class). + UserPreference restored = GSON.getAdapter(UserPreference.class).read(reader); + if (reader.peek() != JsonToken.END_DOCUMENT) { + throw new JsonSyntaxException("Unexpected content after preferences"); + } + restored.setReasoningEfforts(restored.getReasoningEffortSnapshot()); + restored.setContextWindows(restored.getContextWindowSnapshot()); + return restored; + } + } + + private void rejectLegacyJsonExtensions(String json) { + // Gson 2.10's non-lenient mode still accepts control characters, non-JSON escapes and mixed-case literals. + // Reject those extensions before the reader validates the document's remaining syntax and types. + boolean quoted = false; + for (int index = 0; index < json.length(); index++) { + char character = json.charAt(index); + if (quoted) { + if (character < 0x20) { + throw new JsonSyntaxException("Unescaped control character in preferences"); + } + if (character == '\\') { + index++; + if (index == json.length() || "\"\\/bfnrtu".indexOf(json.charAt(index)) < 0) { + throw new JsonSyntaxException("Invalid escape in preferences"); + } + } else if (character == '"') { + quoted = false; + } + } else if (character == '"') { + quoted = true; + } else if ("tTfFnN".indexOf(character) >= 0) { + int start = index; + while (index + 1 < json.length() && Character.isLetter(json.charAt(index + 1))) { + index++; + } + String literal = json.substring(start, index + 1); + if (!"true".equals(literal) && !"false".equals(literal) && !"null".equals(literal)) { + throw new JsonSyntaxException("Invalid literal in preferences"); + } + } + } + } + + private void publishPreferences(Attempt pending, Path path, UserPreference restored) { + readiness.getRealm().asyncExec(() -> { + if (!isCurrent(pending)) { + return; + } + synchronized (lock) { + if (!matches(pending)) { + return; + } + preferences = restored; + preferencePath = path; + state = State.READY; + attempt = null; + if (!readiness.isDisposed()) { + readiness.setValue(State.READY); + } + } + if (pending.timeout != null) { + pending.timeout.cancel(false); + } + }); + } + + private boolean matches(Attempt pending) { + return state == State.LOADING && attempt == pending + && Objects.equals(account, pending.account) && Objects.equals(account, currentAccount()); + } + + private boolean isCurrent(Attempt pending) { + refreshAccount(); + synchronized (lock) { + if (!matches(pending)) { + return false; + } + if (clock.getAsLong() - pending.started < LOAD_TIMEOUT_NANOS) { + return true; + } + } + fail(pending, null); + return false; + } + + private void fail(Attempt pending, Throwable failure) { + refreshAccount(); + long version; + synchronized (lock) { + if (!matches(pending)) { + return; + } + clear(State.FAILED); + version = generation; + } + cancel(pending); + if (failure != null) { + CopilotCore.LOGGER.error("Failed to load chat preferences", failure); + } + publishState(version, State.FAILED); + } + + private void publishState(long version, State next) { + readiness.getRealm().asyncExec(() -> { + refreshAccount(); + synchronized (lock) { + if (generation != version || state != next || readiness.isDisposed()) { + return; + } + readiness.setValue(next); + } + }); + } + + private void cancel(Attempt pending) { + if (pending == null) { + return; + } + if (pending.timeout != null) { + pending.timeout.cancel(false); + } + if (pending.work != null) { + pending.work.cancel(false); + } + if (pending.rpc != null) { + pending.rpc.cancel(true); + } + } + + interface FileAccess { + String read(Path path) throws IOException; + + void write(Path path, String content) throws IOException; + } + + /** + * Account and lifecycle identity captured before dispatching any load work. + */ + private static class Attempt { + private final long generation; + private final String account; + private final long started; + private volatile CompletableFuture rpc; + private volatile FutureTask work; + private volatile ScheduledFuture timeout; + + Attempt(long generation, String account, long started) { + this.generation = generation; + this.account = account; + this.started = started; + } + } +} 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 0ca4b9699..16ec1618b 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 @@ -8,6 +8,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; import org.apache.commons.lang3.StringUtils; import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; @@ -20,7 +22,6 @@ 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; @@ -46,7 +47,20 @@ /** * Service for managing chat modes and input navigation. */ -public class UserPreferenceService extends ChatBaseService implements CopilotAuthStatusListener { +public class UserPreferenceService extends ChatBaseService { + /** + * Availability of the independently discovered built-in mode inventory. + */ + public enum ModeDiscoveryState { + UNAVAILABLE, LOADING, READY, FAILED + } + + private final PreferenceStorage preferenceStorage; + private ISideEffect readinessSideEffect; + private volatile boolean disposed; + private long modeDiscoveryGeneration; + private CompletableFuture> modeDiscovery; + private IObservableValue modeDiscoveryState; private IObservableValue chatModeObservable; private IObservableValue activeChatModeObservable; // Controls which view to show: Ask or Agent private IObservableValue activeModeNameOrIdObservable; // Tracks current mode name/ID for UI elements @@ -64,47 +78,42 @@ public class UserPreferenceService extends ChatBaseService implements CopilotAut /** * Constructor for the UserPreferenceService. */ - public UserPreferenceService(CopilotLanguageServerConnection lsConnection, AuthStatusManager authStatusManager) { + public UserPreferenceService(CopilotLanguageServerConnection lsConnection, AuthStatusManager authStatusManager, + PreferenceStorage preferenceStorage) { super(lsConnection, authStatusManager); + this.preferenceStorage = preferenceStorage; - this.authStatusManager.addCopilotAuthStatusListener(this); ensureRealm(() -> { chatModeObservable = new WritableValue<>(getAvailableChatModes(), String[].class); activeChatModeObservable = new WritableValue<>(null, ChatMode.class); activeModeNameOrIdObservable = new WritableValue<>(null, String.class); + modeDiscoveryState = new WritableValue<>(ModeDiscoveryState.UNAVAILABLE, ModeDiscoveryState.class); + readinessSideEffect = ISideEffect.create(preferenceStorage.getReadiness()::getValue, state -> { + if (state == PreferenceStorage.State.READY) { + init(); + } else { + activeChatModeObservable.setValue(null); + activeModeNameOrIdObservable.setValue(null); + inputNavigation = new InputNavigation(); + } + }); }); initializeEventHandlers(); subscribeToEvents(); - init(); + preferenceStorage.initialize(); + reloadBuiltInModes(); } private void initializeEventHandlers() { authStatusChangedEventHandler = event -> { Object property = event.getProperty(IEventBroker.DATA); if (property instanceof CopilotStatusResult statusResult) { - // If the user signs out, we need to clear the preference cache to avoid the current preference being used in - // the next sign in account. - if (statusResult.isNotSignedIn()) { - clearUserPreferenceCache(); - this.inputNavigation = null; - } else { - // User has signed in - reload built-in modes to ensure we have the latest modes for this user - try { - BuiltInChatModeManager.INSTANCE.reloadModes(); - - // Update available chat modes in the observable to reflect any changes - ensureRealm(() -> { - if (!Arrays.deepEquals(getAvailableChatModes(), chatModeObservable.getValue())) { - chatModeObservable.setValue(getAvailableChatModes()); - } - }); - - // Reinitialize user preferences for the new user - init(); - } catch (Exception e) { - CopilotCore.LOGGER.error("Failed to reload built-in modes on user switch", e); - } + if (statusResult.isSignedIn() && authStatusManager.isSignedIn() + && Objects.equals(statusResult.getUser(), authStatusManager.getUserName())) { + reloadBuiltInModes(); + } else if (!statusResult.isSignedIn()) { + ensureRealm(this::cancelModeDiscovery); } } }; @@ -125,6 +134,55 @@ private void initializeEventHandlers() { }; } + private void reloadBuiltInModes() { + ensureRealm(() -> { + cancelModeDiscovery(); + if (!authStatusManager.isSignedIn()) { + return; + } + long generation = modeDiscoveryGeneration; + String account = authStatusManager.getUserName(); + modeDiscoveryState.setValue(ModeDiscoveryState.LOADING); + CompletableFuture> discovery = CompletableFuture + .supplyAsync(() -> BuiltInChatModeManager.INSTANCE.loadBuiltInModes(lsConnection)) + .thenCompose(result -> result); + modeDiscovery = discovery; + discovery.thenAccept(modes -> ensureRealm(() -> { + if (generation != modeDiscoveryGeneration || !authStatusManager.isSignedIn() + || !Objects.equals(account, authStatusManager.getUserName())) { + return; + } + if (modes.isEmpty()) { + modeDiscoveryState.setValue(ModeDiscoveryState.FAILED); + return; + } + BuiltInChatModeManager.INSTANCE.updateModes(modes); + chatModeObservable.setValue(getAvailableChatModes()); + modeDiscoveryState.setValue(ModeDiscoveryState.READY); + if (eventBroker != null && getUserPreference() != null) { + eventBroker.post(CopilotEventConstants.TOPIC_CHAT_MODE_CHANGED, getActiveChatMode()); + } + })).exceptionally(exception -> { + ensureRealm(() -> { + if (generation == modeDiscoveryGeneration && authStatusManager.isSignedIn() + && Objects.equals(account, authStatusManager.getUserName())) { + modeDiscoveryState.setValue(ModeDiscoveryState.FAILED); + CopilotCore.LOGGER.error("Failed to reload built-in modes", exception); + } + }); + return null; + }); + }); + } + + private void cancelModeDiscovery() { + modeDiscoveryGeneration++; + modeDiscoveryState.setValue(ModeDiscoveryState.UNAVAILABLE); + if (modeDiscovery != null) { + modeDiscovery.cancel(true); + } + } + private void subscribeToEvents() { eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); if (eventBroker != null) { @@ -136,7 +194,7 @@ private void subscribeToEvents() { } private void init() { - if (authStatusManager.isSignedIn()) { + if (preferenceStorage.getState() == PreferenceStorage.State.READY) { // Initialize chat mode preferences String chatModeName = restoreChatModeName(); @@ -150,11 +208,14 @@ private void init() { } final ChatMode viewMode = rawViewMode; + inputNavigation = new InputNavigation(restoreUserInputs()); ensureRealm(() -> { - activeChatModeObservable.setValue(viewMode); activeModeNameOrIdObservable.setValue(chatModeName); + activeChatModeObservable.setValue(viewMode); }); - inputNavigation = new InputNavigation(restoreUserInputs()); + if (eventBroker != null) { + eventBroker.post(CopilotEventConstants.TOPIC_CHAT_MODE_CHANGED, viewMode); + } } } @@ -201,19 +262,6 @@ private List restoreUserInputs() { return new ArrayList<>(); } - @Override - public void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) { - String status = copilotStatusResult.getStatus(); - switch (status) { - case CopilotStatusResult.OK, CopilotStatusResult.NOT_AUTHORIZED: - init(); - break; - default: - disposeAllSideEffects(); - break; - } - } - /** * Get available chat modes based on feature flags. Includes built-in modes, custom modes, and "Add New Mode" option * with separators. @@ -249,6 +297,11 @@ private String[] getAvailableChatModes() { * @param chatModeNameOrId the name/ID of the chat mode to set */ public void setActiveChatMode(String chatModeNameOrId) { + UserPreference preference = getUserPreference(); + if (preference == null) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot change chat mode before preferences are ready")); + return; + } if (StringUtils.isBlank(chatModeNameOrId)) { return; } @@ -274,7 +327,6 @@ public void setActiveChatMode(String chatModeNameOrId) { ChatMode uiViewMode = getViewModeForModeName(chatModeNameOrId); // Step 4: Persist user preference - UserPreference preference = getUserPreference(); preference.setChatModeName(chatModeNameOrId); persistUserPreference(); @@ -302,6 +354,9 @@ public void setActiveChatMode(String chatModeNameOrId) { * @return the active chat mode for UI rendering */ public ChatMode getActiveChatMode() { + if (getUserPreference() == null) { + return ChatMode.Agent; + } ChatMode activeChatMode = activeChatModeObservable.getValue(); if (activeChatMode != null) { return activeChatMode; @@ -319,7 +374,48 @@ public ChatMode getActiveChatMode() { * @return the active mode name (for built-in modes) or custom mode ID (for custom modes) */ public String getActiveModeNameOrId() { - return activeModeNameOrIdObservable.getValue(); + return getUserPreference() == null ? null : activeModeNameOrIdObservable.getValue(); + } + + /** + * Returns whether the active preference can be resolved without falling back to another chat mode. + * + * @return whether actions depending on the selected mode can run + */ + public boolean isActiveModeReady() { + if (disposed) { + return false; + } + boolean inventoryReady = modeDiscoveryState.getValue() == ModeDiscoveryState.READY; + String modeName = activeModeNameOrIdObservable.getValue(); + if (getUserPreference() == null) { + return false; + } + if (CustomChatModeManager.INSTANCE.isCustomMode(modeName)) { + return CustomChatModeManager.INSTANCE.getCustomModeById(modeName) != null; + } + return inventoryReady && BuiltInChatModeManager.INSTANCE.getBuiltInModeByDisplayName(modeName) != null; + } + + /** + * Returns mode discovery readiness in the UI Realm, independently of preference-file loading. + * + * @return the current discovery state + */ + public ModeDiscoveryState getModeDiscoveryState() { + return disposed ? ModeDiscoveryState.UNAVAILABLE : modeDiscoveryState.getValue(); + } + + /** + * Retries failed or unavailable mode discovery without reloading ready preferences. + */ + public void retryModeDiscovery() { + ensureRealm(() -> { + ModeDiscoveryState state = modeDiscoveryState.getValue(); + if (state == ModeDiscoveryState.FAILED || state == ModeDiscoveryState.UNAVAILABLE) { + reloadBuiltInModes(); + } + }); } /** @@ -407,7 +503,13 @@ public void bindChatModePicker(final DropdownButton picker) { } }); - chatModeButtonSideEffects.put(picker, new ISideEffect[] { groupsSideEffect, selectionSideEffect }); + ISideEffect readinessEffect = ISideEffect.create(preferenceStorage.getReadiness()::getValue, state -> { + if (!picker.isDisposed()) { + picker.setEnabled(state == PreferenceStorage.State.READY); + } + }); + chatModeButtonSideEffects.put(picker, + new ISideEffect[] { groupsSideEffect, selectionSideEffect, readinessEffect }); // Add a dispose listener to auto-unbind when the dropdown button is disposed picker.addDisposeListener(e -> unbindChatModePicker(picker)); }); @@ -513,8 +615,12 @@ public void unbindChatView() { * Add input to the input history. */ public void addInputToHistory(String input) { - inputNavigation.add(input); UserPreference preference = getUserPreference(); + if (preference == null) { + CopilotCore.LOGGER.error(new IllegalStateException("Cannot record chat input before preferences are ready")); + return; + } + inputNavigation.add(input); preference.setUserInputs(inputNavigation.getInputHistoryList()); } @@ -525,6 +631,9 @@ public void addInputToHistory(String input) { * @return the previous input or an empty string if at the top of the history. */ public String getPreviousInput(String currentInput) { + if (getUserPreference() == null) { + return StringUtils.EMPTY; + } if (inputNavigation.atBottom() && StringUtils.isNotEmpty(currentInput)) { inputNavigation.add(currentInput); inputNavigation.updateCursorPosition(inputNavigation.size() - 1); @@ -532,7 +641,15 @@ public String getPreviousInput(String currentInput) { return inputNavigation.navigateUp(); } + /** + * Returns the next input, or an empty string when preference history is unavailable. + * + * @return next input in the history + */ public String getNextInput() { + if (getUserPreference() == null) { + return StringUtils.EMPTY; + } return inputNavigation.navigateDown(); } @@ -564,7 +681,12 @@ public boolean isSkipGitHubJobConfirmDialog() { */ public void setSkipGitHubJobConfirmDialog(boolean skip) { UserPreference preference = getUserPreference(); - if (preference != null && preference.isSkipGitHubJobConfirmDialog() != skip) { + if (preference == null) { + CopilotCore.LOGGER.error( + new IllegalStateException("Cannot change confirmation preferences before preferences are ready")); + return; + } + if (preference.isSkipGitHubJobConfirmDialog() != skip) { preference.setSkipGitHubJobConfirmDialog(skip); persistUserPreference(); } @@ -574,11 +696,23 @@ public void setSkipGitHubJobConfirmDialog(boolean skip) { * Dispose of the service. */ public void dispose() { + if (disposed) { + return; + } persistUserPreference(); - // Ideally we should dispose all side effects and observable here. But since the service is - // singleton and will only be disposed when the bundle is stopped. So right now they are not - // explicitly disposed here. - this.authStatusManager.removeCopilotAuthStatusListener(this); + disposed = true; + super.ensureRealm(() -> { + cancelModeDiscovery(); + readinessSideEffect.dispose(); + unbindChatView(); + for (DropdownButton picker : List.copyOf(chatModeButtonSideEffects.keySet())) { + unbindChatModePicker(picker); + } + chatModeObservable.dispose(); + activeChatModeObservable.dispose(); + activeModeNameOrIdObservable.dispose(); + modeDiscoveryState.dispose(); + }); if (eventBroker != null) { eventBroker.unsubscribe(authStatusChangedEventHandler); @@ -603,18 +737,28 @@ private boolean isModeAvailable(String modeNameOrId) { return false; } - private void disposeAllSideEffects() { - ensureRealm(() -> { - // Dispose chat mode combo side effects - for (ISideEffect[] effects : chatModeButtonSideEffects.values()) { - for (ISideEffect effect : effects) { - if (effect != null) { - effect.dispose(); - } - } + private UserPreference getUserPreference() { + return disposed ? null : preferenceStorage.getReadyPreferences(); + } + + /** + * Saves the currently loaded preferences without resolving another persistence path. + */ + public void persistUserPreference() { + if (!disposed) { + preferenceStorage.persist(); + } + } + + @Override + protected void ensureRealm(Runnable runnable) { + if (disposed) { + return; + } + super.ensureRealm(() -> { + if (!disposed) { + runnable.run(); } }); - - chatModeButtonSideEffects.clear(); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java index 3f16e3321..f75dc5524 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java @@ -169,6 +169,7 @@ public final class Messages extends NLS { public static String chat_customModels; public static String chat_addPremiumModels; public static String chat_referencedFile_noVision_tooltip; + public static String chat_referencedFile_modelUnavailable_tooltip; public static String agent_tool_compareEditor_titlePrefix; public static String agent_tool_compareEditor_proposedChangesTitle; public static String agentFileEditor_contentAssist_statusMessage; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties index f377e3108..453e84437 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties @@ -171,6 +171,7 @@ chat_premiumModels=Premium Models chat_customModels=Custom Models chat_addPremiumModels=Add Premium Models chat_referencedFile_noVision_tooltip=%s does not support images. +chat_referencedFile_modelUnavailable_tooltip=Images are unavailable until a model is ready. agent_tool_compareEditor_titlePrefix=Changes from GitHub Copilot: agent_tool_compareEditor_proposedChangesTitle=Proposed Changes