feat: Added Ollama as a BYOK model provider

[ethanyhou]

fix #93

Test:

1. Download Ollama latest client app: https://ollama.com/download
2. Pull some models you like
3. Configure the Ollama URL via Manage Models preferences page (http://localhost:11434 by default):
   
4. The configured models should appear in the model selector:
   
5. Chat with Copilot using Ollama models:
   

[Copilot]

Pull request overview

This PR adds Ollama as a BYOK (Bring Your Own Key) model provider in the GitHub Copilot for Eclipse plugin, introducing provider-level endpoint configuration (instead of API keys) and wiring it through the preference UI, service layer, and LSP protocol.

Changes:

• Adds Ollama as a ByokModelProvider and introduces provider-level config DTOs + LSP RPCs for saving/listing/deleting provider configs.
• Updates the BYOK preferences UI to configure/manage an Ollama endpoint URL and to treat Ollama models as auto-registered on discovery.
• Adds unit tests and updates SWTBot BYOK test plan coverage for the new Ollama provider flow.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties	Adds new localized strings for Ollama dialogs and URL management actions.
com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java	Exposes new NLS keys for Ollama + endpoint management strings.
com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ByokPreferencePage.java	Extends BYOK UI behavior to configure/manage an Ollama endpoint and gate remote loads on URL presence.
com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddOllamaUrlDialog.java	New dialog for configuring an Ollama endpoint URL with basic URL validation.
com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokService.java	Adds provider URL observable state + Ollama configure/delete flows and reload logic updates.
com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ByokServiceTests.java	New tests for Ollama endpoint persistence and discovery/registration behavior.
com.microsoft.copilot.eclipse.swtbot.test/test-plans/byok/byok.md	Extends BYOK SWTBot test plan with Ollama provider scenario and updated provider list.
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfig.java	New record representing provider-level BYOK configuration (name + URL).
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokModelProvider.java	Adds OLLAMA provider and helper predicates (isOllama, requiresApiKey).
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokListProviderConfigResponse.java	New response record for listing provider-level configs.
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnection.java	Adds client methods for provider-config save/delete/list RPCs.
com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServer.java	Adds JSON-RPC endpoints for provider-config save/delete/list.
com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/byok/ByokProviderConfigTests.java	Tests JSON (de)serialization for provider-config DTOs.

[jdneo]

Provider-specific branching is starting to spread

Not a blocker for this PR — the feature works as designed — but I think this is worth capturing before the next provider lands.

ByokModelProvider.isAzure() carries this Javadoc:

"This avoids scattering direct enum displayName comparisons across UI code."

That was the right instinct, but the helper only replaced scattered string comparisons with scattered if branches. After this PR we're at 13 provider-specific branch sites:

ByokService — :334 (isAzure, skip remote fetch), :339 (isOllama, check URL instead of key), :420 (isOllama, auto-register discovered models), :459 (isAzure, filter out of bulk fetch)

ByokPreferencePage — :519, :520, :523, :525, :724, :729, :848, :881, and the onProviderExpanded ternary

All of them are re-deriving answers to the same four orthogonal questions:

1. Does the provider need a provider-level API key? (Azure no — per-model credentials; Ollama no; rest yes)
2. Does it need a provider-level endpoint URL? (Ollama only)
3. Can it discover models remotely? (Azure no)
4. Are discovered models registered by default? (Ollama only)

The part I'd actually change now

requiresApiKey() is the right direction, but it's defined by exclusion:

return !isAzure(providerDisplayName) && !isOllama(providerDisplayName);

The next provider that doesn't take a top-level key — and there's demand for one in #93's comments ("OpenAI protocol compatible local LLM server") — silently gets the wrong answer until someone remembers to edit this line. A positive definition would fail loudly instead of quietly.

Longer-term shape

Push the capabilities onto the enum so each provider declares itself once:

public enum ByokModelProvider {
  AZURE("Azure", Credential.PER_MODEL, /* remoteDiscovery */ false, /* autoRegister */ false),
  OPENAI("OpenAI", Credential.API_KEY, true, false),
  …
  OLLAMA("Ollama", Credential.ENDPOINT_URL, true, true);

  public boolean requiresApiKey() { … }
  public boolean requiresEndpointUrl() { … }
  public boolean supportsRemoteDiscovery() { … }
  public boolean autoRegistersDiscoveredModels() { … }
}

Adding a provider then means one enum line instead of auditing 13 call sites.

I recognise this isn't free: the helpers take String because the UI only has the tree's display name, so instance methods would need a fromDisplayName(String) lookup plus an unknown-name policy. Happy for that to be a follow-up issue rather than scope in here — but the requiresApiKey inversion feels cheap enough to do now.

[ethanyhou]

Provider-specific branching is starting to spread

Not a blocker for this PR — the feature works as designed — but I think this is worth capturing before the next provider lands.

ByokModelProvider.isAzure() carries this Javadoc:

"This avoids scattering direct enum displayName comparisons across UI code."

That was the right instinct, but the helper only replaced scattered string comparisons with scattered if branches. After this PR we're at 13 provider-specific branch sites:

ByokService — :334 (isAzure, skip remote fetch), :339 (isOllama, check URL instead of key), :420 (isOllama, auto-register discovered models), :459 (isAzure, filter out of bulk fetch)

ByokPreferencePage — :519, :520, :523, :525, :724, :729, :848, :881, and the onProviderExpanded ternary

All of them are re-deriving answers to the same four orthogonal questions:

1. Does the provider need a provider-level API key? (Azure no — per-model credentials; Ollama no; rest yes)
2. Does it need a provider-level endpoint URL? (Ollama only)
3. Can it discover models remotely? (Azure no)
4. Are discovered models registered by default? (Ollama only)

The part I'd actually change now

requiresApiKey() is the right direction, but it's defined by exclusion:

return !isAzure(providerDisplayName) && !isOllama(providerDisplayName);

The next provider that doesn't take a top-level key — and there's demand for one in #93's comments ("OpenAI protocol compatible local LLM server") — silently gets the wrong answer until someone remembers to edit this line. A positive definition would fail loudly instead of quietly.

Longer-term shape

Push the capabilities onto the enum so each provider declares itself once:

public enum ByokModelProvider {
  AZURE("Azure", Credential.PER_MODEL, /* remoteDiscovery */ false, /* autoRegister */ false),
  OPENAI("OpenAI", Credential.API_KEY, true, false),
  …
  OLLAMA("Ollama", Credential.ENDPOINT_URL, true, true);

  public boolean requiresApiKey() { … }
  public boolean requiresEndpointUrl() { … }
  public boolean supportsRemoteDiscovery() { … }
  public boolean autoRegistersDiscoveredModels() { … }
}

Adding a provider then means one enum line instead of auditing 13 call sites.

I recognise this isn't free: the helpers take String because the UI only has the tree's display name, so instance methods would need a fromDisplayName(String) lookup plus an unknown-name policy. Happy for that to be a follow-up issue rather than scope in here — but the requiresApiKey inversion feels cheap enough to do now.

Addressed in #388