diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClientTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClientTests.java index 52e9f3c7..a4d7fe1a 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClientTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClientTests.java @@ -4,8 +4,10 @@ package com.microsoft.copilot.eclipse.core.lsp; 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.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -13,9 +15,16 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import com.google.gson.Gson; import org.eclipse.core.resources.IFile; +import org.eclipse.e4.core.contexts.EclipseContextFactory; +import org.eclipse.e4.core.services.events.IEventBroker; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -23,15 +32,19 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.osgi.framework.FrameworkUtil; +import org.osgi.service.event.EventHandler; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.FeatureFlags; import com.microsoft.copilot.eclipse.core.chat.service.IChatServiceManager; import com.microsoft.copilot.eclipse.core.chat.service.IReferencedFileService; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationCapabilities; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationContextParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.CurrentEditorContext; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidChangeFeatureFlagsParams; +import com.microsoft.copilot.eclipse.core.lsp.protocol.policy.DidChangePolicyParams; import com.microsoft.copilot.eclipse.core.utils.FileUtils; @ExtendWith(MockitoExtension.class) @@ -131,4 +144,53 @@ void testOnDidChangeFeatureFlagsWithEmptyFeatureFlags() { verify(mockFeatureFlags).setByokEnabled(true); } } + + @Test + void testOnDidChangePolicy_publishesAutoModelPolicyEventOnlyWhenValueChanges() throws InterruptedException { + IEventBroker eventBroker = EclipseContextFactory + .getServiceContext(FrameworkUtil.getBundle(getClass()).getBundleContext()) + .get(IEventBroker.class); + assertNotNull(eventBroker); + + CountDownLatch eventReceived = new CountDownLatch(1); + CountDownLatch duplicateEventReceived = new CountDownLatch(1); + AtomicInteger eventCount = new AtomicInteger(); + AtomicReference eventData = new AtomicReference<>(); + EventHandler eventHandler = event -> { + eventData.set(event.getProperty(IEventBroker.DATA)); + if (eventCount.incrementAndGet() == 1) { + eventReceived.countDown(); + } else { + duplicateEventReceived.countDown(); + } + }; + eventBroker.subscribe(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, eventHandler); + + DidChangePolicyParams params = new Gson().fromJson(""" + { + "mcp.contributionPoint.enabled": false, + "customAgent.enabled": true, + "agentMode.autoApproval.enabled": true, + "autoModel.enabled": false + } + """, DidChangePolicyParams.class); + FeatureFlags featureFlags = new FeatureFlags(); + + try (MockedStatic copilotCoreMock = Mockito.mockStatic(CopilotCore.class)) { + copilotCoreMock.when(CopilotCore::getPlugin).thenReturn(plugin); + when(plugin.getFeatureFlags()).thenReturn(featureFlags); + + client.onDidChangePolicy(params); + + assertTrue(eventReceived.await(5, TimeUnit.SECONDS)); + assertEquals(Boolean.FALSE, eventData.get()); + + client.onDidChangePolicy(params); + + assertFalse(duplicateEventReceived.await(500, TimeUnit.MILLISECONDS)); + assertEquals(1, eventCount.get()); + } finally { + eventBroker.unsubscribe(eventHandler); + } + } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/FeatureFlags.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/FeatureFlags.java index e3414736..6f4f3a7e 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/FeatureFlags.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/FeatureFlags.java @@ -26,6 +26,8 @@ public class FeatureFlags { private boolean autoApprovalPolicyEnabled = true; + private boolean autoModelPolicyEnabled = true; + public boolean isAgentModeEnabled() { return agentModeEnabled; } @@ -85,6 +87,14 @@ public void setAutoApprovalPolicyEnabled(boolean autoApprovalPolicyEnabled) { this.autoApprovalPolicyEnabled = autoApprovalPolicyEnabled; } + public boolean isAutoModelPolicyEnabled() { + return autoModelPolicyEnabled; + } + + public void setAutoModelPolicyEnabled(boolean autoModelPolicyEnabled) { + this.autoModelPolicyEnabled = autoModelPolicyEnabled; + } + public boolean isClientPreviewFeatureEnabled() { return clientPreviewFeatureEnabled; } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java index 8b5b6fa3..59b7017b 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java @@ -91,6 +91,11 @@ public class CopilotEventConstants { */ public static final String TOPIC_DID_CHANGE_CUSTOM_AGENT_POLICY = TOPIC_POLICY + "CUSTOM_AGENT_ENABLED"; + /** + * Event when the Auto model policy flag is updated. + */ + public static final String TOPIC_DID_CHANGE_AUTO_MODEL_POLICY = TOPIC_POLICY + "AUTO_MODEL_ENABLED"; + /** * Event when the chat mode is changed. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java index e7008691..079c2ec1 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java @@ -356,6 +356,12 @@ public void onDidChangePolicy(DidChangePolicyParams params) { eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_CUSTOM_AGENT_POLICY, params.isCustomAgentEnabled()); } flags.setAutoApprovalPolicyEnabled(params.isAutoApprovalPolicyEnabled()); + if (flags.isAutoModelPolicyEnabled() != params.isAutoModelEnabled()) { + flags.setAutoModelPolicyEnabled(params.isAutoModelEnabled()); + if (eventBroker != null) { + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, params.isAutoModelEnabled()); + } + } } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/policy/DidChangePolicyParams.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/policy/DidChangePolicyParams.java index 1f194a50..9959bccc 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/policy/DidChangePolicyParams.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/policy/DidChangePolicyParams.java @@ -22,6 +22,9 @@ public class DidChangePolicyParams { @SerializedName("agentMode.autoApproval.enabled") private boolean autoApprovalPolicyEnabled = true; + @SerializedName("autoModel.enabled") + private boolean autoModelEnabled = true; + public boolean isMcpContributionPointEnabled() { return mcpContributionPointEnabled; } @@ -46,10 +49,18 @@ public void setAutoApprovalPolicyEnabled(boolean autoApprovalPolicyEnabled) { this.autoApprovalPolicyEnabled = autoApprovalPolicyEnabled; } + public boolean isAutoModelEnabled() { + return autoModelEnabled; + } + + public void setAutoModelEnabled(boolean autoModelEnabled) { + this.autoModelEnabled = autoModelEnabled; + } + @Override public int hashCode() { return Objects.hash(mcpContributionPointEnabled, customAgentEnabled, - autoApprovalPolicyEnabled); + autoApprovalPolicyEnabled, autoModelEnabled); } @Override @@ -66,7 +77,8 @@ public boolean equals(Object obj) { DidChangePolicyParams other = (DidChangePolicyParams) obj; return mcpContributionPointEnabled == other.mcpContributionPointEnabled && customAgentEnabled == other.customAgentEnabled - && autoApprovalPolicyEnabled == other.autoApprovalPolicyEnabled; + && autoApprovalPolicyEnabled == other.autoApprovalPolicyEnabled + && autoModelEnabled == other.autoModelEnabled; } @Override @@ -75,6 +87,7 @@ public String toString() { builder.append("mcpContributionPointEnabled", mcpContributionPointEnabled); builder.append("customAgentEnabled", customAgentEnabled); builder.append("autoApprovalPolicyEnabled", autoApprovalPolicyEnabled); + builder.append("autoModelEnabled", autoModelEnabled); return builder.toString(); } } 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 new file mode 100644 index 00000000..dfc383d8 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelServiceTests.java @@ -0,0 +1,286 @@ +// 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.assertNotNull; +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.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.CompletableFuture; +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.e4.core.services.events.IEventBroker; +import org.eclipse.swt.widgets.Display; +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.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +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.FeatureFlags; +import com.microsoft.copilot.eclipse.core.chat.UserPreference; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; +import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatPersistence; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; + +@ExtendWith(MockitoExtension.class) +class ModelServiceTests { + + private static final String TEST_USER = "model-service-test-user"; + private static final Gson GSON = new Gson(); + + @Mock + private CopilotLanguageServerConnection lsConnection; + + @Mock + private AuthStatusManager authStatusManager; + + @TempDir + private Path persistenceDirectory; + + private ModelService modelService; + 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(); + byokModels.setModels(List.of()); + + when(authStatusManager.isSignedIn()).thenReturn(true); + when(authStatusManager.getUserName()).thenReturn(TEST_USER); + when(lsConnection.persistence()).thenReturn(CompletableFuture.completedFuture(persistence)); + when(lsConnection.listByokModels(any())).thenReturn(CompletableFuture.completedFuture(byokModels)); + + featureFlags = CopilotCore.getPlugin().getFeatureFlags(); + assertNotNull(featureFlags); + previewFeaturesEnabled = featureFlags.isClientPreviewFeatureEnabled(); + featureFlags.setClientPreviewFeatureEnabled(false); + } + + @AfterEach + void tearDown() { + if (modelService != null) { + modelService.dispose(); + } + featureFlags.setClientPreviewFeatureEnabled(previewFeaturesEnabled); + new PreferenceCacheResetter(lsConnection, authStatusManager).reset(); + } + + @Test + void testAutoModelAvailableWhenEditorPreviewDisabled() throws InterruptedException { + CopilotModel defaultModel = createModel("gpt-4o", "GPT-4o", true); + CopilotModel autoModel = createModel("auto", "Auto", false); + when(lsConnection.listModels()) + .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + + waitUntil(() -> isModelAvailable(defaultModel.getModelKey())); + assertTrue(isModelAvailable(autoModel.getModelKey())); + } + + @Test + void testAutoModelPolicyChangeRefreshesModelInventory() throws InterruptedException { + CopilotModel defaultModel = createModel("gpt-4o", "GPT-4o", true); + CopilotModel autoModel = createModel("auto", "Auto", false); + when(lsConnection.listModels()).thenReturn( + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel }), + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + waitUntil(() -> isModelAvailable(autoModel.getModelKey())); + + IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + assertNotNull(eventBroker); + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, Boolean.FALSE); + + waitUntil(() -> !isModelAvailable(autoModel.getModelKey())); + } + + @Test + void testAutoPolicyDisableSelectsServerDefaultAndKeepsAutoPreference() + throws IOException, InterruptedException { + CopilotModel defaultModel = createModel("gpt-4o", "GPT-4o", true); + CopilotModel otherModel = createModel("aaa-other", "Other", false); + CopilotModel autoModel = createModel("auto", "Auto", false); + writePersistedModel(autoModel.getModelKey()); + when(lsConnection.listModels()).thenReturn( + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel, otherModel }), + CompletableFuture.completedFuture(new CopilotModel[] { otherModel, defaultModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + waitUntil(() -> autoModel.getId().equals(getActiveModelId())); + + IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + assertNotNull(eventBroker); + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, Boolean.FALSE); + + waitUntil(() -> defaultModel.getId().equals(getActiveModelId())); + assertPersistedModelRemains(autoModel.getModelKey()); + } + + @Test + void testAutoPolicyDisableUsesDeterministicFallbackAndKeepsAutoPreference() + throws IOException, InterruptedException { + CopilotModel firstModel = createModel("aaa-model", "First", false); + CopilotModel lastModel = createModel("zzz-model", "Last", false); + CopilotModel autoModel = createModel("auto", "Auto", false); + writePersistedModel(autoModel.getModelKey()); + when(lsConnection.listModels()).thenReturn( + CompletableFuture.completedFuture(new CopilotModel[] { autoModel, lastModel, firstModel }), + CompletableFuture.completedFuture(new CopilotModel[] { lastModel, firstModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + waitUntil(() -> autoModel.getId().equals(getActiveModelId())); + + IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + assertNotNull(eventBroker); + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, Boolean.FALSE); + + waitUntil(() -> firstModel.getId().equals(getActiveModelId())); + assertPersistedModelRemains(autoModel.getModelKey()); + } + + @Test + void testAutoPolicyReEnableRestoresPersistedAutoPreference() throws IOException, InterruptedException { + CopilotModel defaultModel = createModel("gpt-4o", "GPT-4o", true); + CopilotModel autoModel = createModel("auto", "Auto", false); + writePersistedModel(autoModel.getModelKey()); + when(lsConnection.listModels()).thenReturn( + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel }), + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel }), + CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, autoModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + waitUntil(() -> autoModel.getId().equals(getActiveModelId())); + + IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + assertNotNull(eventBroker); + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, Boolean.FALSE); + waitUntil(() -> defaultModel.getId().equals(getActiveModelId())); + + eventBroker.post(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, Boolean.TRUE); + + waitUntil(() -> autoModel.getId().equals(getActiveModelId())); + } + + @Test + void testDefaultKeyCollisionSelectsModelFromCurrentInventory() throws IOException, InterruptedException { + CopilotModel defaultModel = createModel("gpt-4o", "Default", true); + CopilotModel inventoryModel = createModel("gpt-4o", "Inventory", false); + writePersistedModel("unavailable-model"); + when(lsConnection.listModels()) + .thenReturn(CompletableFuture.completedFuture(new CopilotModel[] { defaultModel, inventoryModel })); + + modelService = new ModelService(lsConnection, authStatusManager); + waitUntil(() -> isModelAvailable(defaultModel.getModelKey())); + + AtomicReference activeModel = new AtomicReference<>(); + AtomicReference pickerModel = new AtomicReference<>(); + Display.getDefault().syncExec(() -> { + activeModel.set(modelService.getActiveModel()); + pickerModel.set(modelService.getModels().get(defaultModel.getModelKey())); + }); + + assertSame(inventoryModel, pickerModel.get()); + assertSame(pickerModel.get(), activeModel.get()); + } + + private static CopilotModel createModel(String id, String name, boolean isChatDefault) { + CopilotModel model = new CopilotModel(); + model.setId(id); + model.setModelName(name); + model.setModelFamily(id); + model.setScopes(List.of(CopilotScope.CHAT_PANEL, CopilotScope.AGENT_PANEL)); + model.setChatDefault(isChatDefault); + return model; + } + + private static void waitUntil(BooleanSupplier condition) throws InterruptedException { + long deadline = System.currentTimeMillis() + 5000; + while (!condition.getAsBoolean() && System.currentTimeMillis() < deadline) { + Thread.sleep(25); + } + assertTrue(condition.getAsBoolean(), "Timed out waiting for model service state update"); + } + + private boolean isModelAvailable(String modelKey) { + AtomicBoolean available = new AtomicBoolean(); + Display.getDefault().syncExec(() -> available.set(modelService.getModels().containsKey(modelKey))); + return available.get(); + } + + private String getActiveModelId() { + AtomicReference activeModelId = new AtomicReference<>(); + Display.getDefault().syncExec(() -> { + CopilotModel activeModel = modelService.getActiveModel(); + activeModelId.set(activeModel == null ? null : activeModel.getId()); + }); + return activeModelId.get(); + } + + private void writePersistedModel(String modelKey) throws IOException { + UserPreference preference = new UserPreference(); + preference.setChatModel(modelKey); + Path preferenceFile = getPreferenceFile(); + Files.createDirectories(preferenceFile.getParent()); + Files.writeString(preferenceFile, GSON.toJson(preference)); + } + + private void assertPersistedModelRemains(String expectedModelKey) throws IOException, InterruptedException { + long deadline = System.currentTimeMillis() + 500; + while (System.currentTimeMillis() < deadline) { + assertEquals(expectedModelKey, readPersistedModel()); + Thread.sleep(25); + } + } + + private String readPersistedModel() throws IOException { + Path preferenceFile = getPreferenceFile(); + if (!Files.exists(preferenceFile)) { + return null; + } + UserPreference preference = GSON.fromJson(Files.readString(preferenceFile), UserPreference.class); + return preference == null ? null : preference.getChatModel(); + } + + 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(); + } + } +} diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java index e2185223..7bbe7434 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java @@ -132,7 +132,8 @@ void testSupportsReasoningEffortLevel_falseWhenCapabilityFlagUnset() { @Test void testSupportsReasoningEffortLevel_falseForAutoModel() { CopilotModel model = new CopilotModel(); - model.setModelName("Auto"); + model.setId("auto"); + model.setModelName("Automatic"); // Even if the server were to advertise the capability, the Auto model routes to other models and does not // own its own effort selection. model.setCapabilities(new CopilotModelCapabilities( @@ -173,18 +174,29 @@ void testGetModelSuffix_providerNameTakesPrecedenceOverCustomModel() { } @Test - void testIsAutoModel() { + void testGetModelSuffix_autoUsesStableModelId() { CopilotModel auto = new CopilotModel(); - auto.setModelName("Auto"); - assertTrue(ModelUtils.isAutoModel(auto)); + auto.setId("auto"); + auto.setModelName("Automatic"); + assertEquals("Variable", ModelUtils.getModelSuffix(auto, null)); + + CopilotModel matchingDisplayName = new CopilotModel(); + matchingDisplayName.setId("gpt-5"); + matchingDisplayName.setModelName("Auto"); + assertEquals("", ModelUtils.getModelSuffix(matchingDisplayName, null)); + } - CopilotModel autoLower = new CopilotModel(); - autoLower.setModelName("auto"); - assertFalse(ModelUtils.isAutoModel(autoLower)); + @Test + void testIsAutoModel_usesStableModelId() { + CopilotModel auto = new CopilotModel(); + auto.setId("auto"); + auto.setModelName("Automatic"); + assertTrue(ModelUtils.isAutoModel(auto)); - CopilotModel other = new CopilotModel(); - other.setModelName("gpt-5"); - assertFalse(ModelUtils.isAutoModel(other)); + CopilotModel matchingDisplayName = new CopilotModel(); + matchingDisplayName.setId("gpt-5"); + matchingDisplayName.setModelName("Auto"); + assertFalse(ModelUtils.isAutoModel(matchingDisplayName)); assertFalse(ModelUtils.isAutoModel(null)); } 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 c76cd6b1..9820c2e0 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 @@ -50,8 +50,6 @@ */ public class ModelService extends ChatBaseService { - private static final String AUTO_MODEL_ID = "auto"; - // models for the model picker private IObservableValue> modelObservable; private IObservableValue activeModelObservable; @@ -73,6 +71,7 @@ public class ModelService extends ChatBaseService { private EventHandler chatModeChangedEventHandler; private EventHandler byokModelsUpdatedEventHandler; private EventHandler featureFlagsChangedEventHandler; + private EventHandler autoModelPolicyChangedEventHandler; private EventHandler customModeModelChangedEventHandler; /** @@ -132,6 +131,13 @@ private void initializeEventHandlers() { } }; + autoModelPolicyChangedEventHandler = event -> { + Object property = event.getProperty(IEventBroker.DATA); + if (property instanceof Boolean) { + initializeModels(); + } + }; + customModeModelChangedEventHandler = event -> { Object property = event.getProperty(IEventBroker.DATA); if (property instanceof String modelNameWithFamily) { @@ -159,6 +165,8 @@ private void subscribeToEvents() { eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_MODE_CHANGED, chatModeChangedEventHandler); eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_BYOK_MODELS_UPDATED, byokModelsUpdatedEventHandler); eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_FEATURE_FLAGS, featureFlagsChangedEventHandler); + eventBroker.subscribe(CopilotEventConstants.TOPIC_DID_CHANGE_AUTO_MODEL_POLICY, + autoModelPolicyChangedEventHandler); eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_CUSTOM_MODE_MODEL_CHANGED, customModeModelChangedEventHandler); } else { @@ -193,27 +201,26 @@ protected IStatus run(IProgressMonitor monitor) { private void fetchCopilotModels() throws InterruptedException, ExecutionException { CopilotModel[] modelArray = lsConnection.listModels().get(); Map newModels = new HashMap<>(); + CopilotModel newDefaultModel = null; + CopilotModel newFallbackModel = null; for (CopilotModel model : modelArray) { - // TODO: remove it once CLS supports filtering models by preview flag - if (!CopilotCore.getPlugin().getFeatureFlags().isClientPreviewFeatureEnabled() - && AUTO_MODEL_ID.equals(model.getId())) { - continue; - } boolean supportsChat = model.getScopes().contains(CopilotScope.CHAT_PANEL); boolean supportsAgent = model.getScopes().contains(CopilotScope.AGENT_PANEL); if (supportsChat || supportsAgent) { newModels.put(model.getModelKey(), model); } if (model.isChatDefault()) { - defaultModel = model; + newDefaultModel = model; } if (model.isChatFallback()) { - fallbackModel = model; + newFallbackModel = model; } } copilotModels = newModels; + defaultModel = newDefaultModel; + fallbackModel = newFallbackModel; } private void fetchByokModels() throws InterruptedException, ExecutionException { @@ -245,7 +252,7 @@ private String restoreActiveModel() { } if (defaultModel != null) { - return defaultModel.getId(); + return defaultModel.getModelKey(); } return null; @@ -274,7 +281,7 @@ private void updateModelsForChatMode(ChatMode chatMode) { ensureRealm(() -> { modelObservable.setValue(modelsForCurrentMode); // Validate and set active model for the current mode - validateAndSetActiveModelForMode(modelsForCurrentMode, scope); + validateAndSetActiveModelForMode(modelsForCurrentMode); }); } @@ -282,23 +289,45 @@ private void updateModelsForChatMode(ChatMode chatMode) { * Validate and set the appropriate active model for the current chat mode. This method handles the logic of restoring * user preference or falling back to default. */ - private void validateAndSetActiveModelForMode(Map modelsForCurrentMode, String scope) { + private void validateAndSetActiveModelForMode(Map modelsForCurrentMode) { CopilotModel currentActive = getActiveModel(); + String restoredModelKey = restoreActiveModel(); + CopilotModel restoredModel = restoredModelKey == null ? null : modelsForCurrentMode.get(restoredModelKey); + if (restoredModel != null) { + if (currentActive != restoredModel) { + ensureRealm(() -> activeModelObservable.setValue(restoredModel)); + } + return; + } + boolean isCurrentModelAvailable = currentActive != null && modelsForCurrentMode.containsKey(currentActive.getModelKey()); if (currentActive == null || !isCurrentModelAvailable) { - // Try to restore user's preferred model if it's available in current mode - String restoredModelId = restoreActiveModel(); - if (restoredModelId != null && modelsForCurrentMode.containsKey(restoredModelId)) { - ensureRealm(() -> activeModelObservable.setValue(modelsForCurrentMode.get(restoredModelId))); - return; + CopilotModel replacementModel = selectReplacementModel(modelsForCurrentMode); + if (replacementModel != null) { + ensureRealm(() -> activeModelObservable.setValue(replacementModel)); } - // fall back the first available model in the current mode - if (!modelsForCurrentMode.isEmpty()) { - CopilotModel firstModel = modelsForCurrentMode.values().iterator().next(); - ensureRealm(() -> activeModelObservable.setValue(firstModel)); + } + } + + private CopilotModel selectReplacementModel(Map modelsForCurrentMode) { + if (defaultModel != null) { + CopilotModel availableDefaultModel = modelsForCurrentMode.get(defaultModel.getModelKey()); + if (availableDefaultModel != null) { + return availableDefaultModel; } } + return modelsForCurrentMode.keySet().stream() + .sorted() + .findFirst() + .map(modelsForCurrentMode::get) + .orElse(null); + } + + private void persistModelSelection(CopilotModel model) { + UserPreference preference = getUserPreference(); + preference.setChatModel(model.getModelKey()); + CompletableFuture.runAsync(this::persistUserPreference); } private String modeToScope(ChatMode mode) { @@ -336,28 +365,16 @@ private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) { public void setActiveModel(String modelName) { Map currentModels = modelObservable.getValue(); - // Find model by model name and get its composite key - String compositeKey = null; - final CopilotModel model; - CopilotModel foundModel = null; - - for (Map.Entry entry : currentModels.entrySet()) { - if (entry.getValue().getModelName().equals(modelName)) { - compositeKey = entry.getKey(); - foundModel = entry.getValue(); - break; - } - } - model = foundModel; - if (model != null && compositeKey != null) { - // Persist using the composite key for proper identification - UserPreference preference = getUserPreference(); - preference.setChatModel(compositeKey); + final CopilotModel model = currentModels.values().stream() + .filter(candidateModel -> candidateModel.getModelName().equals(modelName)) + .findFirst() + .orElse(null); + if (model != null) { // 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. - CompletableFuture.runAsync(this::persistUserPreference); + persistModelSelection(model); // Update observable ensureRealm(() -> activeModelObservable.setValue(model)); @@ -657,6 +674,7 @@ public void dispose() { eventBroker.unsubscribe(chatModeChangedEventHandler); eventBroker.unsubscribe(byokModelsUpdatedEventHandler); eventBroker.unsubscribe(featureFlagsChangedEventHandler); + eventBroker.unsubscribe(autoModelPolicyChangedEventHandler); eventBroker.unsubscribe(customModeModelChangedEventHandler); eventBroker = null; } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index 5919742d..e022ecff 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -352,7 +352,7 @@ public static List getSupportedReasoningEfforts(CopilotModel model) { * @return {@code true} when the model is the Auto model */ public static boolean isAutoModel(CopilotModel model) { - return model != null && "Auto".equals(model.getModelName()); + return model != null && "auto".equals(model.getId()); } /**