From 2d6a51cd04889f73e3eaddc234f2a5ea2b9c6197 Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 10:45:05 +0800 Subject: [PATCH 1/6] Added Language server API to support Ollama. --- .../core/lsp/CopilotLanguageServer.java | 20 +++++++++++ .../lsp/CopilotLanguageServerConnection.java | 33 +++++++++++++++++++ .../byok/ByokListProviderConfigResponse.java | 14 ++++++++ .../lsp/protocol/byok/ByokProviderConfig.java | 13 ++++++++ 4 files changed, 80 insertions(+) create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigResponse.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfig.java diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java index 18a5be6a6..a61dda7dd 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java @@ -55,7 +55,9 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokStatusResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.git.GenerateCommitMessageParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.git.GenerateCommitMessageResult; @@ -276,6 +278,24 @@ public interface CopilotLanguageServer extends LanguageServer { @JsonRequest("copilot/byok/listApiKeys") CompletableFuture listByokApiKeys(ByokApiKey apiKey); + /** + * Save a built-in BYOK provider configuration. + */ + @JsonRequest("copilot/byok/saveProviderConfig") + CompletableFuture saveByokProviderConfig(ByokProviderConfig providerConfig); + + /** + * Delete a built-in BYOK provider configuration. + */ + @JsonRequest("copilot/byok/deleteProviderConfig") + CompletableFuture deleteByokProviderConfig(ByokProviderConfig providerConfig); + + /** + * List built-in BYOK provider configurations. + */ + @JsonRequest("copilot/byok/listProviderConfigs") + CompletableFuture listByokProviderConfigs(ByokProviderConfig providerConfig); + /** * Update the status of the mcp server and tools. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java index e0f533be0..b74b92a63 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java @@ -73,7 +73,9 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokStatusResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.git.GenerateCommitMessageParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.git.GenerateCommitMessageResult; @@ -591,6 +593,37 @@ public CompletableFuture listByokApiKeys(ByokApiKey apiK return this.languageServerWrapper.execute(fn); } + /** + * Save a built-in BYOK provider configuration. + */ + public CompletableFuture saveByokProviderConfig(ByokProviderConfig providerConfig) { + Function> fn = server -> { + return ((CopilotLanguageServer) server).saveByokProviderConfig(providerConfig); + }; + return this.languageServerWrapper.execute(fn); + } + + /** + * Delete a built-in BYOK provider configuration. + */ + public CompletableFuture deleteByokProviderConfig(ByokProviderConfig providerConfig) { + Function> fn = server -> { + return ((CopilotLanguageServer) server).deleteByokProviderConfig(providerConfig); + }; + return this.languageServerWrapper.execute(fn); + } + + /** + * List built-in BYOK provider configurations. + */ + public CompletableFuture listByokProviderConfigs( + ByokProviderConfig providerConfig) { + Function> fn = server -> { + return ((CopilotLanguageServer) server).listByokProviderConfigs(providerConfig); + }; + return this.languageServerWrapper.execute(fn); + } + /** * Save a BYOK API key. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigResponse.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigResponse.java new file mode 100644 index 000000000..ccc21a2c9 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigResponse.java @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol.byok; + +import java.util.List; + +/** + * Response model for listing provider-level BYOK configurations. + * + * @param providers provider configurations + */ +public record ByokListProviderConfigResponse(List providers) { +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfig.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfig.java new file mode 100644 index 000000000..8eb22a84e --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfig.java @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol.byok; + +/** + * Provider-level BYOK configuration. + * + * @param providerName provider display name + * @param url provider endpoint URL + */ +public record ByokProviderConfig(String providerName, String url) { +} From 9a870aaefeaa8a7877e55932346d83b914dc6284 Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 11:21:38 +0800 Subject: [PATCH 2/6] Added Ollama URL configuration dialog. --- .../ui/preferences/AddOllamaUrlDialog.java | 112 ++++++++++++++++++ .../eclipse/ui/preferences/Messages.java | 7 ++ .../ui/preferences/messages.properties | 7 ++ 3 files changed, 126 insertions(+) create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java new file mode 100644 index 000000000..ffc122ffb --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java @@ -0,0 +1,112 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.preferences; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.function.Consumer; + +import org.apache.commons.lang3.StringUtils; +import org.eclipse.jface.dialogs.IDialogConstants; +import org.eclipse.jface.dialogs.TrayDialog; +import org.eclipse.swt.SWT; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Text; + +/** + * Dialog for configuring the URL shared by Ollama models. + */ +public class AddOllamaUrlDialog extends TrayDialog { + + public static final String DEFAULT_ENDPOINT = "http://localhost:11434"; + + private static final int CONTAINER_WIDTH = 400; + + private final String endpoint; + private final boolean editMode; + private final Consumer onSave; + private Text endpointText; + private Button okButton; + + /** + * Creates an Ollama URL dialog. + * + * @param parentShell parent shell + * @param endpoint existing endpoint, or {@code null} for the default + * @param onSave endpoint consumer + */ + public AddOllamaUrlDialog(Shell parentShell, String endpoint, Consumer onSave) { + super(parentShell); + this.editMode = StringUtils.isNotBlank(endpoint); + this.endpoint = StringUtils.defaultIfBlank(endpoint, DEFAULT_ENDPOINT); + this.onSave = onSave; + setShellStyle(getShellStyle() | SWT.RESIZE); + } + + @Override + protected void configureShell(Shell newShell) { + super.configureShell(newShell); + newShell.setText(editMode ? Messages.preferences_page_byok_ollama_dialog_title + : Messages.preferences_page_byok_ollama_create_dialog_title); + } + + @Override + protected Control createDialogArea(Composite parent) { + Composite container = (Composite) super.createDialogArea(parent); + GridLayout layout = new GridLayout(2, false); + layout.marginWidth = 10; + layout.marginHeight = 10; + container.setLayout(layout); + GridData containerData = new GridData(SWT.FILL, SWT.FILL, true, true); + containerData.widthHint = CONTAINER_WIDTH; + container.setLayoutData(containerData); + + new Label(container, SWT.NONE).setText(Messages.preferences_page_byok_ollama_endpoint); + endpointText = new Text(container, SWT.BORDER); + endpointText.setText(endpoint); + endpointText.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + endpointText.addModifyListener(event -> updateOkButton()); + return container; + } + + @Override + protected void createButtonsForButtonBar(Composite parent) { + okButton = createButton(parent, IDialogConstants.OK_ID, IDialogConstants.OK_LABEL, true); + createButton(parent, IDialogConstants.CANCEL_ID, IDialogConstants.CANCEL_LABEL, false); + updateOkButton(); + } + + @Override + protected void okPressed() { + String newEndpoint = endpointText.getText().trim(); + if (!isValidEndpoint(newEndpoint)) { + endpointText.setFocus(); + return; + } + onSave.accept(newEndpoint); + super.okPressed(); + } + + private void updateOkButton() { + if (okButton != null && !okButton.isDisposed()) { + okButton.setEnabled(isValidEndpoint(endpointText.getText().trim())); + } + } + + private boolean isValidEndpoint(String value) { + try { + URI uri = new URI(value); + return ("http".equalsIgnoreCase(uri.getScheme()) || "https".equalsIgnoreCase(uri.getScheme())) + && StringUtils.isNotBlank(uri.getHost()); + } catch (URISyntaxException e) { + return false; + } + } +} \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java index 7c17d6d63..ae4051b53 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java @@ -23,6 +23,8 @@ public class Messages extends NLS { public static String preferences_page_byok_removeModel; public static String preferences_page_byok_changeApi_button; public static String preferences_page_byok_deleteApi_button; + public static String preferences_page_byok_changeEndpoint_button; + public static String preferences_page_byok_deleteEndpoint_button; public static String preferences_page_byok_enableModel_button; public static String preferences_page_byok_disableModel_button; public static String preferences_page_byok_reload_button; @@ -31,6 +33,9 @@ public class Messages extends NLS { public static String preferences_page_byok_addModel_modelId; public static String preferences_page_byok_addModel_deploymentUrl; public static String preferences_page_byok_addModel_apiKey; + public static String preferences_page_byok_ollama_dialog_title; + public static String preferences_page_byok_ollama_create_dialog_title; + public static String preferences_page_byok_ollama_endpoint; public static String preferences_page_byok_addModel_displayName; public static String preferences_page_byok_addModel_supportVision; public static String preferences_page_byok_addModel_supportToolCalling; @@ -38,6 +43,8 @@ public class Messages extends NLS { public static String preferences_page_byok_changeApi_dialog_description; public static String preferences_page_byok_deleteApi_dialog_title; public static String preferences_page_byok_deleteApi_dialog_description; + public static String preferences_page_byok_deleteEndpoint_dialog_title; + public static String preferences_page_byok_deleteEndpoint_dialog_description; public static String preferences_page_byok_dialog_add; public static String preferences_page_byok_dialog_delete; public static String preferences_page_byok_dialog_yes; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties index 34b638c7d..7a9bbb848 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties @@ -11,6 +11,8 @@ preferences_page_byok_addModel_button=Add Model... preferences_page_byok_removeModel=Remove Model preferences_page_byok_changeApi_button=Change API... preferences_page_byok_deleteApi_button=Delete API... +preferences_page_byok_changeEndpoint_button=Change URL... +preferences_page_byok_deleteEndpoint_button=Delete URL... preferences_page_byok_enableModel_button=Enable preferences_page_byok_disableModel_button=Disable preferences_page_byok_reload_button=Reload @@ -19,6 +21,9 @@ preferences_page_byok_addModel_dialog_title=Add %s Models preferences_page_byok_addModel_modelId=Model ID: * preferences_page_byok_addModel_deploymentUrl=Deployment URL: * preferences_page_byok_addModel_apiKey=API Key: * +preferences_page_byok_ollama_dialog_title=Configure Ollama +preferences_page_byok_ollama_create_dialog_title=Add Ollama Provider +preferences_page_byok_ollama_endpoint=Endpoint URL: * preferences_page_byok_addModel_displayName=Display Name: preferences_page_byok_addModel_supportVision=Support Vision preferences_page_byok_addModel_supportToolCalling=Support Tool Calling @@ -26,6 +31,8 @@ preferences_page_byok_changeApi_dialog_title=Change %s API Key? preferences_page_byok_changeApi_dialog_description=Change API Keys may cause model damage and cannot use. preferences_page_byok_deleteApi_dialog_title=Delete %s API Key? preferences_page_byok_deleteApi_dialog_description=Removing this API key will permanently delete all associated models and their configurations. +preferences_page_byok_deleteEndpoint_dialog_title=Delete %s URL? +preferences_page_byok_deleteEndpoint_dialog_description=Removing this URL will permanently delete all associated models and their configurations. preferences_page_byok_dialog_add=Add preferences_page_byok_dialog_delete=Delete preferences_page_byok_dialog_yes=Yes From d155fb8581852c075835847a58752dc8c5e3a3a5 Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 11:30:42 +0800 Subject: [PATCH 3/6] Add support for Ollama provider configuration and management --- .../lsp/protocol/byok/ByokModelProvider.java | 17 ++- .../eclipse/ui/chat/services/ByokService.java | 107 ++++++++++++++++-- .../ui/preferences/ByokPreferencePage.java | 75 ++++++++++-- 3 files changed, 178 insertions(+), 21 deletions(-) diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokModelProvider.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokModelProvider.java index 18eccd74d..9c8f198c4 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokModelProvider.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokModelProvider.java @@ -12,7 +12,8 @@ public enum ByokModelProvider { GEMINI("Gemini"), GROQ("Groq"), OPENROUTER("OpenRouter"), - ANTHROPIC("Anthropic"); + ANTHROPIC("Anthropic"), + OLLAMA("Ollama"); private final String displayName; @@ -33,6 +34,20 @@ public static boolean isAzure(String providerDisplayName) { return AZURE.getDisplayName().equals(providerDisplayName); } + /** + * Utility to check if a provider display name corresponds to Ollama. + */ + public static boolean isOllama(String providerDisplayName) { + return OLLAMA.getDisplayName().equals(providerDisplayName); + } + + /** + * Returns whether the provider requires a provider-level API key. + */ + public static boolean requiresApiKey(String providerDisplayName) { + return !isAzure(providerDisplayName) && !isOllama(providerDisplayName); + } + @Override public String toString() { return displayName; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java index 417cb6ed5..1197d5b8b 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,6 +30,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelProvider; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; import com.microsoft.copilot.eclipse.ui.preferences.ByokPreferencePage; /** @@ -40,6 +42,7 @@ public class ByokService extends ChatBaseService { // Observable data for UI binding private IObservableValue>> byokModelsByProviderObservable; private IObservableValue> apiKeysObservable; + private IObservableValue> providerUrlsObservable; // Feature flag observable (byokEnabled) private IObservableValue byokEnabledObservable; @@ -50,6 +53,7 @@ public class ByokService extends ChatBaseService { // UI binding private ISideEffect modelsSideEffect; private ISideEffect apiKeysSideEffect; + private ISideEffect providerUrlsSideEffect; private ISideEffect byokFlagSideEffect; /** @@ -63,6 +67,7 @@ public ByokService(CopilotLanguageServerConnection lsConnection) { ensureRealm(() -> { byokModelsByProviderObservable = new WritableValue<>(new HashMap<>(), HashMap.class); apiKeysObservable = new WritableValue<>(new HashMap<>(), HashMap.class); + providerUrlsObservable = new WritableValue<>(new HashMap<>(), HashMap.class); byokEnabledObservable = new WritableValue<>( CopilotCore.getPlugin().getFeatureFlags().isByokEnabled(), Boolean.class); }); @@ -96,6 +101,9 @@ public void bindByokPreferencePage(ByokPreferencePage page) { return apiKeysObservable.getValue(); }, page::updateApiKeysDisplay); + providerUrlsSideEffect = ISideEffect.create(() -> providerUrlsObservable.getValue(), + page::updateProviderUrlsDisplay); + // Create side effect for byok flag updates byokFlagSideEffect = ISideEffect.create(() -> byokEnabledObservable.getValue(), flagValue -> page.updatePageState()); @@ -116,6 +124,11 @@ public void unbindByokPreferencePage() { apiKeysSideEffect = null; } + if (providerUrlsSideEffect != null) { + providerUrlsSideEffect.dispose(); + providerUrlsSideEffect = null; + } + if (byokFlagSideEffect != null) { byokFlagSideEffect.dispose(); byokFlagSideEffect = null; @@ -137,6 +150,18 @@ public CompletableFuture loadApiKeys() { }); } + /** + * Load provider-level endpoint URLs from persistent storage. + */ + public CompletableFuture loadProviderUrls() { + return lsConnection.listByokProviderConfigs(new ByokProviderConfig(null, null)).thenAccept(response -> { + Map providerUrls = response == null || response.providers() == null ? Map.of() + : response.providers().stream().filter(config -> config.url() != null) + .collect(Collectors.toMap(ByokProviderConfig::providerName, ByokProviderConfig::url)); + ensureRealm(() -> providerUrlsObservable.setValue(providerUrls)); + }); + } + /** * Load BYOK models from persistent storage. */ @@ -157,7 +182,57 @@ public CompletableFuture loadLocalModels() { * Refresh BYOK data (including API keys and models). */ public CompletableFuture refreshData() { - return loadApiKeys().thenCompose(unused -> loadLocalModels()); + return loadApiKeys().thenCompose(unused -> loadProviderUrls()).thenCompose(unused -> loadLocalModels()); + } + + /** + * Save an Ollama endpoint, discover its models, and register them for model selection. + */ + public CompletableFuture configureOllama(String endpointUrl) { + String providerName = ByokModelProvider.OLLAMA.getDisplayName(); + ByokProviderConfig config = new ByokProviderConfig(providerName, endpointUrl); + return lsConnection.saveByokProviderConfig(config).thenCompose(response -> { + if (!response.isSuccess()) { + String message = response.getMessage() != null ? response.getMessage() : "Failed to save Ollama endpoint"; + return CompletableFuture.failedFuture(new IllegalStateException(message)); + } + updateProviderUrl(providerName, endpointUrl); + return lsConnection.listByokModels(new ByokListModelParams(providerName, true)); + }).thenCompose(response -> { + List models = response == null ? null : response.getModels(); + if (models == null || models.isEmpty()) { + return refreshData(); + } + models.forEach(model -> model.setRegistered(true)); + return batchSaveByokModels(models).thenCompose(unused -> refreshData()); + }); + } + + /** + * Delete the Ollama URL configuration and all of its stored models. + */ + public CompletableFuture deleteOllamaConfig() { + String providerName = ByokModelProvider.OLLAMA.getDisplayName(); + return lsConnection.deleteByokProviderConfig(new ByokProviderConfig(providerName, null)).thenCompose(response -> { + if (!response.isSuccess()) { + String message = response.getMessage() != null ? response.getMessage() : "Failed to delete Ollama endpoint"; + return CompletableFuture.failedFuture(new IllegalStateException(message)); + } + updateProviderUrl(providerName, null); + return refreshData(); + }); + } + + private void updateProviderUrl(String providerName, String endpointUrl) { + ensureRealm(() -> { + Map providerUrls = new HashMap<>(providerUrlsObservable.getValue()); + if (endpointUrl == null) { + providerUrls.remove(providerName); + } else { + providerUrls.put(providerName, endpointUrl); + } + providerUrlsObservable.setValue(providerUrls); + }); } /** @@ -261,6 +336,18 @@ public CompletableFuture reloadProvider(String providerName) { return loadLocalModels(); } + if (ByokModelProvider.isOllama(providerName)) { + AtomicBoolean hasEndpoint = new AtomicBoolean(false); + ensureRealm(() -> { + Map currentProviderUrls = providerUrlsObservable.getValue(); + hasEndpoint.set(currentProviderUrls != null && currentProviderUrls.containsKey(providerName)); + }); + if (!hasEndpoint.get()) { + return CompletableFuture.completedFuture(null); + } + return fetchProviderModels(providerName).thenCompose(changed -> loadLocalModels()); + } + final AtomicBoolean hasApiKey = new AtomicBoolean(false); ensureRealm(() -> { Map currentKeys = apiKeysObservable != null ? apiKeysObservable.getValue() : null; @@ -277,12 +364,11 @@ public CompletableFuture reloadProvider(String providerName) { } /** - * Reload all providers sequentially to avoid file write conflicts. Only providers with API keys (excluding Azure) - * will fetch remote models. + * Reload all providers sequentially to avoid file write conflicts. Providers with API keys and Ollama with a + * configured URL will fetch remote models; Azure only uses its locally stored model configurations. */ public CompletableFuture reloadAllProviders() { - return fetchAllProvidersSequentially() - .thenCompose(changed -> changed ? refreshData() : CompletableFuture.completedFuture(null)); + return fetchAllProvidersSequentially().thenCompose(unused -> refreshData()); } /** @@ -331,7 +417,7 @@ private CompletableFuture mergeRemoteModelsWithLocal(String providerNam List toAdd = new ArrayList<>(); for (ByokModel remoteModel : remoteModels) { if (!localIds.contains(remoteModel.getModelId())) { - remoteModel.setRegistered(false); // newly discovered, keep as unregistered + remoteModel.setRegistered(ByokModelProvider.isOllama(providerName)); toAdd.add(remoteModel); } } @@ -364,11 +450,12 @@ private CompletableFuture fetchAllProvidersSequentially() { AtomicReference> providersRef = new AtomicReference<>(List.of()); ensureRealm(() -> { Map currentApiKeys = apiKeysObservable.getValue(); - if (currentApiKeys == null || currentApiKeys.isEmpty()) { - providersRef.set(List.of()); - return; + Set providers = currentApiKeys == null ? new HashSet<>() : new HashSet<>(currentApiKeys.keySet()); + Map currentProviderUrls = providerUrlsObservable.getValue(); + if (currentProviderUrls != null) { + providers.addAll(currentProviderUrls.keySet()); } - List providersToFetch = currentApiKeys.keySet().stream() + List providersToFetch = providers.stream() .filter(providerName -> !ByokModelProvider.isAzure(providerName)).toList(); providersRef.set(providersToFetch); }); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java index 1392a5294..860e23f8a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java @@ -69,6 +69,8 @@ public class ByokPreferencePage extends PreferencePage implements IWorkbenchPref private Map byProviderApiKeys = new HashMap<>(); + private Map byProviderUrls = new HashMap<>(); + // used to determine whether remote models are fetched private Set remotelyLoadedProviders = new HashSet<>(); @@ -509,18 +511,22 @@ private void refreshButtonsEnabled() { removeModelButton.setEnabled(selectedModel != null && selectedModel.isCustomModel()); toggleStatusButton.setEnabled(selectedModel != null); reloadButton.setEnabled(true); - // Check if provider is not Azure and has API key boolean canManageApiKey = false; + boolean canManageEndpoint = false; String providerName = getSelectedProviderName(); if (providerName != null) { - boolean isAzureProvider = ByokModelProvider.isAzure(providerName); boolean hasApiKeyForProvider = byProviderApiKeys.containsKey(providerName); - canManageApiKey = !isAzureProvider && hasApiKeyForProvider; + canManageApiKey = ByokModelProvider.requiresApiKey(providerName) && hasApiKeyForProvider; + canManageEndpoint = ByokModelProvider.isOllama(providerName) && byProviderUrls.containsKey(providerName); } - // Change API: enabled when provider is not Azure and has API key - changeApiButton.setEnabled(canManageApiKey); - // Delete API: enabled when provider is not Azure and has API key - deleteApiButton.setEnabled(canManageApiKey); + + changeApiButton.setText(ByokModelProvider.isOllama(providerName) + ? Messages.preferences_page_byok_changeEndpoint_button : Messages.preferences_page_byok_changeApi_button); + deleteApiButton.setText(ByokModelProvider.isOllama(providerName) + ? Messages.preferences_page_byok_deleteEndpoint_button : Messages.preferences_page_byok_deleteApi_button); + + changeApiButton.setEnabled(canManageApiKey || canManageEndpoint); + deleteApiButton.setEnabled(canManageApiKey || canManageEndpoint); } private void initializeTreeViewer() { @@ -578,6 +584,19 @@ public void updateApiKeysDisplay(Map apiKeys) { } } + /** + * Called by service to update provider-level endpoint URLs. + */ + public void updateProviderUrlsDisplay(Map providerUrls) { + if (viewer != null && !viewer.getControl().isDisposed()) { + byProviderUrls.clear(); + if (providerUrls != null) { + byProviderUrls.putAll(providerUrls); + } + refreshButtonsEnabled(); + } + } + /** * Restore the expansion state of the tree viewer. */ @@ -702,8 +721,12 @@ private void onAddModel() { if (providerName != null) { final String finalProviderName = providerName; + if (ByokModelProvider.isOllama(providerName)) { + openAddOllamaUrlDialog(); + return; + } boolean hasApiKey = byProviderApiKeys.containsKey(providerName); - if (!hasApiKey && !ByokModelProvider.isAzure(providerName)) { + if (!hasApiKey && ByokModelProvider.requiresApiKey(providerName)) { AddApiKeyDialog apiKeyDialog = new AddApiKeyDialog(getShell(), providerName, apiKey -> { if (apiKey != null && StringUtils.isNotBlank(apiKey) && byokService != null) { executeAsyncProviderOperation(finalProviderName, byokService.addApiKey(finalProviderName, apiKey), @@ -822,6 +845,10 @@ private void reloadAllProviders() { private void onChangeProviderApi() { String providerName = getSelectedProviderName(); + if (ByokModelProvider.isOllama(providerName)) { + openAddOllamaUrlDialog(); + return; + } String apiKey = byProviderApiKeys.get(providerName); if (!ByokModelProvider.isAzure(providerName)) { final String finalProviderName = providerName; @@ -851,6 +878,14 @@ private void onDeleteProviderApi() { final String finalProviderName = providerName; + if (ByokModelProvider.isOllama(providerName)) { + if (showDeleteEndpointConfirmationDialog(providerName)) { + executeAsyncProviderOperation(finalProviderName, byokService.deleteOllamaConfig(), + "Failed to delete Ollama endpoint"); + } + return; + } + if (!ByokModelProvider.isAzure(providerName)) { if (showDeleteApiKeyConfirmationDialog(providerName)) { executeAsyncProviderOperation(finalProviderName, byokService.deleteApiKey(providerName), @@ -870,11 +905,31 @@ private boolean showDeleteApiKeyConfirmationDialog(String providerName) { return dialog.open() == 0; } + private boolean showDeleteEndpointConfirmationDialog(String providerName) { + MessageDialog dialog = new MessageDialog(getShell(), + String.format(Messages.preferences_page_byok_deleteEndpoint_dialog_title, providerName), null, + Messages.preferences_page_byok_deleteEndpoint_dialog_description, MessageDialog.QUESTION, + new String[] { Messages.preferences_page_byok_dialog_delete, Messages.preferences_page_byok_dialog_cancel }, 0); + return dialog.open() == 0; + } + + private void openAddOllamaUrlDialog() { + String providerName = ByokModelProvider.OLLAMA.getDisplayName(); + AddOllamaUrlDialog dialog = new AddOllamaUrlDialog(getShell(), byProviderUrls.get(providerName), endpoint -> { + if (byokService != null) { + executeAsyncProviderOperation(providerName, byokService.configureOllama(endpoint), + "Failed to configure Ollama endpoint"); + } + }); + dialog.open(); + } + private void onProviderExpanded(String providerName) { // If provider is first expanded, need to fetch models for this provider from remote site if (!remotelyLoadedProviders.contains(providerName)) { - if (byProviderApiKeys == null || !byProviderApiKeys.containsKey(providerName)) { - // No API key for provider, skip loading + boolean canFetch = ByokModelProvider.isOllama(providerName) ? byProviderUrls.containsKey(providerName) + : byProviderApiKeys.containsKey(providerName); + if (!canFetch) { remotelyLoadedProviders.add(providerName); return; } From b2279d05de1ce7f56dea0227a95d5c822aa1528f Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 11:31:11 +0800 Subject: [PATCH 4/6] Add tests for Ollama provider configuration and model management --- .../byok/ByokProviderConfigTests.java | 36 +++++ .../test-plans/byok/byok.md | 43 +++++- .../ui/chat/services/ByokServiceTests.java | 135 ++++++++++++++++++ 3 files changed, 207 insertions(+), 7 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java create mode 100644 com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java new file mode 100644 index 000000000..fae84a278 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol.byok; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.google.gson.Gson; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import org.junit.jupiter.api.Test; + +class ByokProviderConfigTests { + + private static final Gson GSON = new Gson(); + + @Test + void testProviderConfig_serializesClsFieldNames() { + ByokProviderConfig config = new ByokProviderConfig("Ollama", "http://localhost:11434"); + + JsonObject json = JsonParser.parseString(GSON.toJson(config)).getAsJsonObject(); + + assertEquals("Ollama", json.get("providerName").getAsString()); + assertEquals("http://localhost:11434", json.get("url").getAsString()); + } + + @Test + void testListProviderConfigResponse_deserializesClsResponse() { + ByokListProviderConfigResponse response = GSON.fromJson( + "{\"providers\":[{\"providerName\":\"Ollama\",\"url\":\"http://localhost:11434\"}]}", + ByokListProviderConfigResponse.class); + + assertEquals(1, response.providers().size()); + assertEquals(new ByokProviderConfig("Ollama", "http://localhost:11434"), response.providers().get(0)); + } +} \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/byok/byok.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/byok/byok.md index f16571602..cd05b935a 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/byok/byok.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/byok/byok.md @@ -23,9 +23,9 @@ Entry points exercised: com.microsoft.copilot.eclipse.ui.preferences.ByokPreferencePage`. Providers covered (`ByokModelProvider`): `Azure`, `OpenAI`, `Gemini`, `Groq`, -`OpenRouter`, `Anthropic`. Azure is special-cased: it has no top-level API -key, so the **Change API…** / **Delete API…** buttons stay disabled for it -even when models are configured. +`OpenRouter`, `Anthropic`, `Ollama`. Azure has per-model deployment credentials. +Ollama has no API key and instead uses a provider-level endpoint URL, defaulting +to `http://localhost:11434`. Not exercised in this plan (separate scenarios): - Actually issuing chat completions through a registered BYOK model — that's @@ -52,6 +52,8 @@ Not exercised in this plan (separate scenarios): language server's secure store as part of the TC. - For Azure-specific TCs: a deployment URL and API key for an Azure OpenAI deployment (or skip the Azure cases). +- For Ollama-specific TCs: Ollama 0.6.4 or newer is running and has at least + one installed model. - No previously opened Preferences dialog. The probe runner pre-suppresses Quick Start, What's New, Welcome, and "Terminal Support Unavailable" pop-ups — keep that contract when authoring follow-up plans. @@ -86,8 +88,8 @@ Not exercised in this plan (separate scenarios): 5. Verify the **Provider** group is visible with the description **Select a provider before adding models.** 6. Verify the tree has two columns — **Custom Models** and **Status** — - and contains exactly the six providers `Azure`, `OpenAI`, `Gemini`, - `Groq`, `OpenRouter`, `Anthropic`. + and contains exactly the seven providers `Azure`, `OpenAI`, `Gemini`, + `Groq`, `OpenRouter`, `Anthropic`, `Ollama`. 7. Verify the action buttons are present on the right side: **Add Model...**, **Remove Model**, **Enable** / **Disable**, **Reload**, **Change API...**, **Delete API...**. With no selection, **Add Model...**, @@ -96,7 +98,7 @@ Not exercised in this plan (separate scenarios): #### Expected Result - The page opens without an error dialog. -- All six providers render as collapsible tree nodes. +- All seven providers render as collapsible tree nodes. - Button enablement matches the no-selection state described above. - `workspace.log` contains no `ERROR` entries from `com.microsoft.copilot.eclipse.ui.preferences.ByokPreferencePage` or @@ -105,7 +107,7 @@ Not exercised in this plan (separate scenarios): #### 📸 Key Screenshots - [ ] **Loading state** — overlay shown immediately after the page opens. -- [ ] **Loaded state** — provider tree visible with the six providers and +- [ ] **Loaded state** — provider tree visible with the seven providers and the action buttons on the right. #### Notes on failure modes @@ -275,6 +277,33 @@ Not exercised in this plan (separate scenarios): --- +### TC-005A: Configure Ollama and expose discovered models in the selector + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Preconditions +- TC-001 preconditions hold. +- Ollama 0.6.4 or newer is running with at least one installed model. + +#### Steps +1. Open the BYOK page, select **Ollama**, and click **Add Model...**. +2. Verify the **Configure Ollama** dialog contains an **Endpoint URL** field + defaulted to `http://localhost:11434` and no API key field. +3. Click **OK** and wait for the Ollama loading indicator to clear. +4. Expand **Ollama** and verify the installed models appear as enabled. +5. Open the chat view model selector and verify the enabled Ollama models are + listed under the Ollama provider. +6. Return to Model Management, select **Ollama**, and verify the actions read + **Change URL...** and **Delete URL...**. + +#### Expected Result +- The endpoint is persisted as provider-level configuration. +- Discovered Ollama models are registered automatically and appear in the + model selector without requiring an API key or a separate enable action. + +--- + ## 3. Custom model management ### TC-006: Add a custom model under a provider with an API key diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java new file mode 100644 index 000000000..e49eda724 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java @@ -0,0 +1,135 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.services; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +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.mockito.ArgumentCaptor; +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.byok.ByokApiKey; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelProvider; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokStatusResponse; +import com.microsoft.copilot.eclipse.ui.preferences.ByokPreferencePage; + +@ExtendWith(MockitoExtension.class) +class ByokServiceTests { + + private static final String OLLAMA_ENDPOINT = "http://localhost:11434"; + private static final String OLLAMA_PROVIDER = ByokModelProvider.OLLAMA.getDisplayName(); + + @Mock + private CopilotLanguageServerConnection lsConnection; + + @Mock + private ByokPreferencePage preferencePage; + + private ByokService byokService; + + @BeforeEach + void setUp() { + byokService = new ByokService(lsConnection); + byokService.bindByokPreferencePage(preferencePage); + clearInvocations(preferencePage); + } + + @AfterEach + void tearDown() { + byokService.ensureRealm(byokService::dispose); + } + + @Test + void testConfigureOllama_discoveryFailureKeepsSavedEndpointVisible() { + when(lsConnection.saveByokProviderConfig(any())).thenReturn(completedStatus()); + when(lsConnection.listByokModels(any())) + .thenReturn(CompletableFuture.failedFuture(new IllegalStateException("Ollama is unavailable"))); + + assertThrows(CompletionException.class, () -> byokService.configureOllama(OLLAMA_ENDPOINT).join()); + + verify(preferencePage).updateProviderUrlsDisplay(argThat( + providerUrls -> OLLAMA_ENDPOINT.equals(providerUrls.get(OLLAMA_PROVIDER)))); + } + + @Test + void testConfigureOllama_emptyDiscoveryRefreshesLocalModels() { + when(lsConnection.saveByokProviderConfig(any())).thenReturn(completedStatus()); + configureRefreshResponses(List.of()); + + byokService.configureOllama(OLLAMA_ENDPOINT).join(); + + verify(lsConnection, times(2)).listByokModels(any()); + verify(lsConnection).listByokModels(argThat(params -> Boolean.FALSE.equals(params.getEnableFetchUrl()))); + } + + @Test + void testConfigureOllama_discoveredModelsAreRegistered() { + ByokModel discoveredModel = new ByokModel(); + discoveredModel.setProviderName(OLLAMA_PROVIDER); + discoveredModel.setModelId("qwen3.5:0.8b"); + discoveredModel.setRegistered(false); + when(lsConnection.saveByokProviderConfig(any())).thenReturn(completedStatus()); + configureRefreshResponses(List.of(discoveredModel)); + when(lsConnection.saveByokModel(any())).thenReturn(completedStatus()); + + byokService.configureOllama(OLLAMA_ENDPOINT).join(); + + ArgumentCaptor modelCaptor = ArgumentCaptor.forClass(ByokModel.class); + verify(lsConnection).saveByokModel(modelCaptor.capture()); + assertTrue(modelCaptor.getValue().isRegistered()); + } + + @Test + void testDeleteOllamaConfig_removesEndpointBeforeRefresh() { + when(lsConnection.deleteByokProviderConfig(any())).thenReturn(completedStatus()); + configureRefreshResponses(List.of()); + byokService.loadProviderUrls().join(); + clearInvocations(preferencePage); + + byokService.deleteOllamaConfig().join(); + + verify(preferencePage).updateProviderUrlsDisplay(argThat(Map::isEmpty)); + } + + private void configureRefreshResponses(List discoveredModels) { + when(lsConnection.listByokModels(any())).thenAnswer(invocation -> { + ByokListModelResponse response = new ByokListModelResponse(); + response.setModels(discoveredModels); + return CompletableFuture.completedFuture(response); + }); + when(lsConnection.listByokApiKeys(any(ByokApiKey.class))) + .thenReturn(CompletableFuture.completedFuture(new ByokListApiKeyResponse(List.of()))); + when(lsConnection.listByokProviderConfigs(any(ByokProviderConfig.class))) + .thenReturn(CompletableFuture.completedFuture(new ByokListProviderConfigResponse( + List.of(new ByokProviderConfig(OLLAMA_PROVIDER, OLLAMA_ENDPOINT))))); + } + + private CompletableFuture completedStatus() { + ByokStatusResponse response = new ByokStatusResponse(); + response.setSuccess(true); + return CompletableFuture.completedFuture(response); + } +} \ No newline at end of file From 9069409e8cc4502d8066679e8edd55a8a4746bbe Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 15:19:12 +0800 Subject: [PATCH 5/6] Address comments. --- .../eclipse/ui/chat/services/ByokServiceTests.java | 14 ++++++++++++++ .../eclipse/ui/chat/services/ByokService.java | 8 ++++++-- .../eclipse/ui/preferences/ByokPreferencePage.java | 12 +++++++----- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java index e49eda724..86e347302 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java @@ -74,6 +74,20 @@ void testConfigureOllama_discoveryFailureKeepsSavedEndpointVisible() { providerUrls -> OLLAMA_ENDPOINT.equals(providerUrls.get(OLLAMA_PROVIDER)))); } + @Test + void testLoadProviderUrls_ignoresBlankUrlsAndKeepsFirstDuplicate() { + String duplicateEndpoint = "http://localhost:11435"; + when(lsConnection.listByokProviderConfigs(any(ByokProviderConfig.class))) + .thenReturn(CompletableFuture.completedFuture(new ByokListProviderConfigResponse(List.of( + new ByokProviderConfig(OLLAMA_PROVIDER, " "), + new ByokProviderConfig(OLLAMA_PROVIDER, OLLAMA_ENDPOINT), + new ByokProviderConfig(OLLAMA_PROVIDER, duplicateEndpoint))))); + + byokService.loadProviderUrls().join(); + + verify(preferencePage).updateProviderUrlsDisplay(Map.of(OLLAMA_PROVIDER, OLLAMA_ENDPOINT)); + } + @Test void testConfigureOllama_emptyDiscoveryRefreshesLocalModels() { when(lsConnection.saveByokProviderConfig(any())).thenReturn(completedStatus()); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java index 1197d5b8b..ab2cc4909 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java @@ -15,6 +15,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; import org.eclipse.core.databinding.observable.sideeffect.ISideEffect; import org.eclipse.core.databinding.observable.value.IObservableValue; import org.eclipse.core.databinding.observable.value.WritableValue; @@ -156,8 +157,11 @@ public CompletableFuture loadApiKeys() { public CompletableFuture loadProviderUrls() { return lsConnection.listByokProviderConfigs(new ByokProviderConfig(null, null)).thenAccept(response -> { Map providerUrls = response == null || response.providers() == null ? Map.of() - : response.providers().stream().filter(config -> config.url() != null) - .collect(Collectors.toMap(ByokProviderConfig::providerName, ByokProviderConfig::url)); + : response.providers().stream() + .filter(config -> config != null && StringUtils.isNotBlank(config.providerName()) + && StringUtils.isNotBlank(config.url())) + .collect(Collectors.toMap(ByokProviderConfig::providerName, ByokProviderConfig::url, + (firstUrl, duplicateUrl) -> firstUrl)); ensureRealm(() -> providerUrlsObservable.setValue(providerUrls)); }); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java index 860e23f8a..e9bc8d160 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java @@ -930,15 +930,17 @@ private void onProviderExpanded(String providerName) { boolean canFetch = ByokModelProvider.isOllama(providerName) ? byProviderUrls.containsKey(providerName) : byProviderApiKeys.containsKey(providerName); if (!canFetch) { - remotelyLoadedProviders.add(providerName); return; } byokService.reloadProvider(providerName).whenComplete((result, throwable) -> { - if (throwable != null) { - handleError(throwable.getMessage()); - } + SwtUtils.invokeOnDisplayThreadAsync(() -> { + if (throwable == null) { + remotelyLoadedProviders.add(providerName); + } else { + handleError(throwable.getMessage()); + } + }); }); - remotelyLoadedProviders.add(providerName); } } From 0e3ae62d4359712553912ff64a714c7534dfc4bf Mon Sep 17 00:00:00 2001 From: Ethan Hou Date: Mon, 3 Aug 2026 16:34:22 +0800 Subject: [PATCH 6/6] Address comments. --- .../byok/ByokProviderConfigTests.java | 21 ++++++++++++- .../core/lsp/CopilotLanguageServer.java | 6 ++-- .../lsp/CopilotLanguageServerConnection.java | 11 ++++--- .../byok/ByokDeleteProviderConfigParams.java | 12 +++++++ .../byok/ByokListProviderConfigParams.java | 14 +++++++++ .../ui/chat/services/ByokServiceTests.java | 7 +++-- .../eclipse/ui/chat/services/ByokService.java | 31 ++++++++++--------- .../ui/preferences/AddOllamaUrlDialog.java | 5 ++- 8 files changed, 79 insertions(+), 28 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokDeleteProviderConfigParams.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigParams.java diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java index fae84a278..1190ccb60 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java @@ -24,6 +24,25 @@ void testProviderConfig_serializesClsFieldNames() { assertEquals("http://localhost:11434", json.get("url").getAsString()); } + @Test + void testListProviderConfigParams_nullProviderSerializesEmptyObject() { + ByokListProviderConfigParams params = new ByokListProviderConfigParams(null); + + JsonObject json = JsonParser.parseString(GSON.toJson(params)).getAsJsonObject(); + + assertEquals(0, json.size()); + } + + @Test + void testDeleteProviderConfigParams_serializesOnlyProviderName() { + ByokDeleteProviderConfigParams params = new ByokDeleteProviderConfigParams("Ollama"); + + JsonObject json = JsonParser.parseString(GSON.toJson(params)).getAsJsonObject(); + + assertEquals(1, json.size()); + assertEquals("Ollama", json.get("providerName").getAsString()); + } + @Test void testListProviderConfigResponse_deserializesClsResponse() { ByokListProviderConfigResponse response = GSON.fromJson( @@ -33,4 +52,4 @@ void testListProviderConfigResponse_deserializesClsResponse() { assertEquals(1, response.providers().size()); assertEquals(new ByokProviderConfig("Ollama", "http://localhost:11434"), response.providers().get(0)); } -} \ No newline at end of file +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java index a61dda7dd..5fe118e1a 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java @@ -52,9 +52,11 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.UpdateMcpToolsStatusParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.WorkspaceFoldersParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokApiKey; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokDeleteProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; @@ -288,13 +290,13 @@ public interface CopilotLanguageServer extends LanguageServer { * Delete a built-in BYOK provider configuration. */ @JsonRequest("copilot/byok/deleteProviderConfig") - CompletableFuture deleteByokProviderConfig(ByokProviderConfig providerConfig); + CompletableFuture deleteByokProviderConfig(ByokDeleteProviderConfigParams params); /** * List built-in BYOK provider configurations. */ @JsonRequest("copilot/byok/listProviderConfigs") - CompletableFuture listByokProviderConfigs(ByokProviderConfig providerConfig); + CompletableFuture listByokProviderConfigs(ByokListProviderConfigParams params); /** * Update the status of the mcp server and tools. diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java index b74b92a63..f6b6a1020 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java @@ -70,9 +70,11 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.UpdateMcpToolsStatusParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.WorkspaceFoldersParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokApiKey; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokDeleteProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; @@ -606,9 +608,9 @@ public CompletableFuture saveByokProviderConfig(ByokProvider /** * Delete a built-in BYOK provider configuration. */ - public CompletableFuture deleteByokProviderConfig(ByokProviderConfig providerConfig) { + public CompletableFuture deleteByokProviderConfig(ByokDeleteProviderConfigParams params) { Function> fn = server -> { - return ((CopilotLanguageServer) server).deleteByokProviderConfig(providerConfig); + return ((CopilotLanguageServer) server).deleteByokProviderConfig(params); }; return this.languageServerWrapper.execute(fn); } @@ -617,9 +619,9 @@ public CompletableFuture deleteByokProviderConfig(ByokProvid * List built-in BYOK provider configurations. */ public CompletableFuture listByokProviderConfigs( - ByokProviderConfig providerConfig) { + ByokListProviderConfigParams params) { Function> fn = server -> { - return ((CopilotLanguageServer) server).listByokProviderConfigs(providerConfig); + return ((CopilotLanguageServer) server).listByokProviderConfigs(params); }; return this.languageServerWrapper.execute(fn); } @@ -697,7 +699,6 @@ public CompletableFuture searchPr(SearchPrParams params) { return this.languageServerWrapper.execute(fn); } - /** * Notify that an inline edit was shown. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokDeleteProviderConfigParams.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokDeleteProviderConfigParams.java new file mode 100644 index 000000000..6f0cecef6 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokDeleteProviderConfigParams.java @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol.byok; + +/** + * Parameters for deleting a built-in BYOK provider configuration. + * + * @param providerName provider name + */ +public record ByokDeleteProviderConfigParams(String providerName) { +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigParams.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigParams.java new file mode 100644 index 000000000..6eb3f1a23 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigParams.java @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol.byok; + +import org.eclipse.jdt.annotation.Nullable; + +/** + * Parameters for listing built-in BYOK provider configurations. + * + * @param providerName provider name, or {@code null} to list all configured providers + */ +public record ByokListProviderConfigParams(@Nullable String providerName) { +} diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java index 86e347302..a21e71160 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java @@ -29,6 +29,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokApiKey; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelResponse; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelProvider; @@ -77,7 +78,7 @@ void testConfigureOllama_discoveryFailureKeepsSavedEndpointVisible() { @Test void testLoadProviderUrls_ignoresBlankUrlsAndKeepsFirstDuplicate() { String duplicateEndpoint = "http://localhost:11435"; - when(lsConnection.listByokProviderConfigs(any(ByokProviderConfig.class))) + when(lsConnection.listByokProviderConfigs(any(ByokListProviderConfigParams.class))) .thenReturn(CompletableFuture.completedFuture(new ByokListProviderConfigResponse(List.of( new ByokProviderConfig(OLLAMA_PROVIDER, " "), new ByokProviderConfig(OLLAMA_PROVIDER, OLLAMA_ENDPOINT), @@ -136,7 +137,7 @@ private void configureRefreshResponses(List discoveredModels) { }); when(lsConnection.listByokApiKeys(any(ByokApiKey.class))) .thenReturn(CompletableFuture.completedFuture(new ByokListApiKeyResponse(List.of()))); - when(lsConnection.listByokProviderConfigs(any(ByokProviderConfig.class))) + when(lsConnection.listByokProviderConfigs(any(ByokListProviderConfigParams.class))) .thenReturn(CompletableFuture.completedFuture(new ByokListProviderConfigResponse( List.of(new ByokProviderConfig(OLLAMA_PROVIDER, OLLAMA_ENDPOINT))))); } @@ -146,4 +147,4 @@ private CompletableFuture completedStatus() { response.setSuccess(true); return CompletableFuture.completedFuture(response); } -} \ No newline at end of file +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java index ab2cc4909..6b492cda6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java @@ -28,7 +28,9 @@ import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidChangeFeatureFlagsParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokApiKey; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokDeleteProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; +import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListProviderConfigParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelProvider; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokProviderConfig; @@ -69,8 +71,8 @@ public ByokService(CopilotLanguageServerConnection lsConnection) { byokModelsByProviderObservable = new WritableValue<>(new HashMap<>(), HashMap.class); apiKeysObservable = new WritableValue<>(new HashMap<>(), HashMap.class); providerUrlsObservable = new WritableValue<>(new HashMap<>(), HashMap.class); - byokEnabledObservable = new WritableValue<>( - CopilotCore.getPlugin().getFeatureFlags().isByokEnabled(), Boolean.class); + byokEnabledObservable = new WritableValue<>(CopilotCore.getPlugin().getFeatureFlags().isByokEnabled(), + Boolean.class); }); // Subscribe to feature flag changes for BYOK @@ -155,7 +157,7 @@ public CompletableFuture loadApiKeys() { * Load provider-level endpoint URLs from persistent storage. */ public CompletableFuture loadProviderUrls() { - return lsConnection.listByokProviderConfigs(new ByokProviderConfig(null, null)).thenAccept(response -> { + return lsConnection.listByokProviderConfigs(new ByokListProviderConfigParams(null)).thenAccept(response -> { Map providerUrls = response == null || response.providers() == null ? Map.of() : response.providers().stream() .filter(config -> config != null && StringUtils.isNotBlank(config.providerName()) @@ -203,7 +205,7 @@ public CompletableFuture configureOllama(String endpointUrl) { updateProviderUrl(providerName, endpointUrl); return lsConnection.listByokModels(new ByokListModelParams(providerName, true)); }).thenCompose(response -> { - List models = response == null ? null : response.getModels(); + List models = response.getModels(); if (models == null || models.isEmpty()) { return refreshData(); } @@ -217,14 +219,15 @@ public CompletableFuture configureOllama(String endpointUrl) { */ public CompletableFuture deleteOllamaConfig() { String providerName = ByokModelProvider.OLLAMA.getDisplayName(); - return lsConnection.deleteByokProviderConfig(new ByokProviderConfig(providerName, null)).thenCompose(response -> { - if (!response.isSuccess()) { - String message = response.getMessage() != null ? response.getMessage() : "Failed to delete Ollama endpoint"; - return CompletableFuture.failedFuture(new IllegalStateException(message)); - } - updateProviderUrl(providerName, null); - return refreshData(); - }); + return lsConnection.deleteByokProviderConfig(new ByokDeleteProviderConfigParams(providerName)) + .thenCompose(response -> { + if (!response.isSuccess()) { + String message = response.getMessage() != null ? response.getMessage() : "Failed to delete Ollama endpoint"; + return CompletableFuture.failedFuture(new IllegalStateException(message)); + } + updateProviderUrl(providerName, null); + return refreshData(); + }); } private void updateProviderUrl(String providerName, String endpointUrl) { @@ -368,8 +371,8 @@ public CompletableFuture reloadProvider(String providerName) { } /** - * Reload all providers sequentially to avoid file write conflicts. Providers with API keys and Ollama with a - * configured URL will fetch remote models; Azure only uses its locally stored model configurations. + * Reload all providers sequentially to avoid file write conflicts. Providers with API keys and Ollama with a + * configured URL will fetch remote models; Azure only uses its locally stored model configurations. */ public CompletableFuture reloadAllProviders() { return fetchAllProvidersSequentially().thenCompose(unused -> refreshData()); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java index ffc122ffb..5590ac3a2 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java @@ -25,8 +25,7 @@ */ public class AddOllamaUrlDialog extends TrayDialog { - public static final String DEFAULT_ENDPOINT = "http://localhost:11434"; - + private static final String DEFAULT_ENDPOINT = "http://localhost:11434"; private static final int CONTAINER_WIDTH = 400; private final String endpoint; @@ -109,4 +108,4 @@ private boolean isValidEndpoint(String value) { return false; } } -} \ No newline at end of file +}