Consolidate asynchronous built-in chat mode discovery

com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManagerTests.java

@@ -0,0 +1,142 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.core.chat;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+
+import 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.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection;
+import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode;
+import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams;
+
+@ExtendWith(MockitoExtension.class)
+class BuiltInChatModeManagerTests {
+
+  @Mock
+  private CopilotLanguageServerConnection mockConnection;
+
+  @BeforeEach
+  void setUp() {
+    BuiltInChatModeManager.INSTANCE.updateModes(List.of());
+  }
+
+  @AfterEach
+  void clearSnapshot() {
+    BuiltInChatModeManager.INSTANCE.updateModes(List.of());
+  }
+
+  @Test
+  void testLoadBuiltInModes_inlineAgentSkippedFromAgentModes() {
+    ConversationMode agentMode = createConversationMode("Agent", "Agent", "Agent");
+    ConversationMode inlineAgentMode = createConversationMode("InlineAgent", "Agent", "InlineAgent");
+
+    when(mockConnection.listConversationModes(any(ConversationModesParams.class)))
+        .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { agentMode, inlineAgentMode }));
+
+    List<BuiltInChatMode> builtInModes = BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join();
+
+    assertEquals(1, builtInModes.size());
+    BuiltInChatMode builtInMode = builtInModes.get(0);
+    assertEquals("Agent", builtInMode.getId());
+    assertEquals("Agent", builtInMode.getDisplayName());
+    assertEquals("Agent", builtInMode.getKind());
+  }
+
+  @Test
+  void testLoadBuiltInModes_successDoesNotPublishSnapshot() {
+    BuiltInChatMode publishedMode = createBuiltInChatMode("Ask");
+    BuiltInChatModeManager.INSTANCE.updateModes(List.of(publishedMode));
+    ConversationMode loadedMode = createConversationMode("Agent", "Agent", "Agent");
+    when(mockConnection.listConversationModes(any(ConversationModesParams.class)))
+        .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { loadedMode }));
+
+    List<BuiltInChatMode> result = BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join();
+
+    assertEquals(List.of("Agent"), result.stream().map(BuiltInChatMode::getId).toList());
+    assertEquals(List.of(publishedMode), BuiltInChatModeManager.INSTANCE.getBuiltInModes());
+  }
+
+  @Test
+  void testLoadBuiltInModes_failureDoesNotReplaceSnapshot() {
+    BuiltInChatMode publishedMode = createBuiltInChatMode("Ask");
+    BuiltInChatModeManager.INSTANCE.updateModes(List.of(publishedMode));
+    IllegalStateException failure = new IllegalStateException("mode discovery failed");
+    when(mockConnection.listConversationModes(any(ConversationModesParams.class)))
+        .thenReturn(CompletableFuture.failedFuture(failure));
+
+    CompletionException exception = assertThrows(CompletionException.class,
+        () -> BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join());
+
+    assertSame(failure, exception.getCause());
+    assertEquals(List.of(publishedMode), BuiltInChatModeManager.INSTANCE.getBuiltInModes());
+  }
+
+  @Test
+  void testLoadBuiltInModes_nullConnectionReturnsEmptyWithoutPublishing() {
+    BuiltInChatMode publishedMode = createBuiltInChatMode("Ask");
+    BuiltInChatModeManager.INSTANCE.updateModes(List.of(publishedMode));
+
+    assertEquals(List.of(), BuiltInChatModeManager.INSTANCE.loadBuiltInModes(null).join());
+    assertEquals(List.of(publishedMode), BuiltInChatModeManager.INSTANCE.getBuiltInModes());
+    verifyNoInteractions(mockConnection);
+  }
+
+  @Test
+  void testLoadBuiltInModes_conversionFailureSkipsMode() {
+    ConversationMode invalidMode = mock(ConversationMode.class);
+    when(invalidMode.isBuiltIn()).thenReturn(true);
+    when(invalidMode.getKind()).thenReturn("Agent");
+    when(invalidMode.getName()).thenReturn("Agent");
+    when(invalidMode.getId()).thenReturn("invalid");
+    when(invalidMode.getDescription()).thenThrow(new IllegalArgumentException("invalid mode"));
+    when(mockConnection.listConversationModes(any(ConversationModesParams.class)))
+        .thenReturn(CompletableFuture.completedFuture(new ConversationMode[] { invalidMode }));
+
+    assertEquals(List.of(), BuiltInChatModeManager.INSTANCE.loadBuiltInModes(mockConnection).join());
+  }
+
+  @Test
+  void testUpdateModes_publishesDefensiveSnapshot() {
+    BuiltInChatMode mode = createBuiltInChatMode("Plan");
+    List<BuiltInChatMode> source = new ArrayList<>(List.of(mode));
+
+    BuiltInChatModeManager.INSTANCE.updateModes(source);
+    source.clear();
+    List<BuiltInChatMode> snapshot = BuiltInChatModeManager.INSTANCE.getBuiltInModes();
+    snapshot.clear();
+
+    assertEquals(List.of(mode), BuiltInChatModeManager.INSTANCE.getBuiltInModes());
+  }
+
+  private BuiltInChatMode createBuiltInChatMode(String name) {
+    return new BuiltInChatMode(createConversationMode(name, name, name));
+  }
+
+  private ConversationMode createConversationMode(String id, String name, String kind) {
+    ConversationMode mode = new ConversationMode();
+    mode.setId(id);
+    mode.setName(name);
+    mode.setKind(kind);
+    mode.setBuiltIn(true);
+    mode.setDescription(name + " description");
+    return mode;
+  }
+}

com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/BuiltInChatModeManager.java

@@ -4,34 +4,62 @@
 package com.microsoft.copilot.eclipse.core.chat;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CompletableFuture;
 
-import com.microsoft.copilot.eclipse.core.chat.service.BuiltInChatModeService;
+import com.microsoft.copilot.eclipse.core.CopilotCore;
+import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection;
+import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode;
+import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams;
 
 /**
- * Singleton manager for built-in chat modes. Built-in modes are loaded once from the LSP API at startup.
+ * Shared snapshot of built-in chat modes discovered asynchronously by the chat lifecycle.
  */
 public enum BuiltInChatModeManager {
   INSTANCE;
 
-  private final BuiltInChatModeService service;
-  private List<BuiltInChatMode> builtInModes;
+  private static final List<String> ALLOWED_BUILTIN_NAMES = Arrays.asList(BuiltInChatMode.ASK_MODE_NAME,
+      BuiltInChatMode.AGENT_MODE_NAME, BuiltInChatMode.PLAN_MODE_NAME, BuiltInChatMode.DEBUGGER_MODE_NAME);
 
-  BuiltInChatModeManager() {
-    this.service = new BuiltInChatModeService();
-    this.builtInModes = new CopyOnWriteArrayList<>();
-    loadModesSync();
-  }
+  private volatile List<BuiltInChatMode> builtInModes = List.of();
 
-  private void loadModesSync() {
-    try {
-      List<BuiltInChatMode> modes = service.loadBuiltInModes().get();
-      this.builtInModes = new CopyOnWriteArrayList<>(modes);
-    } catch (Exception e) {
-      // Initialize with empty list on failure
-      this.builtInModes = new CopyOnWriteArrayList<>();
+  /**
+   * Loads built-in modes using the owning chat lifecycle's language-server connection.
+   *
+   * @param lsConnection the connection used by the chat services
+   * @return the discovered built-in modes
+   */
+  public CompletableFuture<List<BuiltInChatMode>> loadBuiltInModes(
+      CopilotLanguageServerConnection lsConnection) {
+    if (lsConnection == null) {
+      return CompletableFuture.completedFuture(new ArrayList<>());
     }
+    ConversationModesParams params = new ConversationModesParams(Collections.emptyList());
+
+    return lsConnection.listConversationModes(params).thenApply(conversationModes -> {
+      List<BuiltInChatMode> loadedModes = new ArrayList<>();
+
+      for (ConversationMode mode : conversationModes) {
+        if (mode == null || !mode.isBuiltIn()) {
+          continue;
+        }
+        // Exclude InlineAgent kind — it is not a user-facing chat mode
+        if (BuiltInChatMode.INLINE_AGENT_KIND.equalsIgnoreCase(mode.getKind())) {
+          continue;
+        }
+        // Filter to only allowed built-in modes by name (case-insensitive)
+        if (ALLOWED_BUILTIN_NAMES.stream().anyMatch(name -> name.equalsIgnoreCase(mode.getName()))) {
+          BuiltInChatMode builtInMode = convertToBuiltInChatMode(mode);
+          if (builtInMode != null) {
+            loadedModes.add(builtInMode);
+          }
+        }
+      }
+
+      return loadedModes;
+    });
   }
 
   public List<BuiltInChatMode> getBuiltInModes() {
@@ -60,10 +88,20 @@ public BuiltInChatMode getBuiltInModeById(String id) {
   }
 
   /**
-   * Reloads built-in chat modes from the LSP API. This should be called when the user switches
-   * to ensure the latest modes are available for the current user context.
+   * Publishes a completed discovery after its owner has validated the account and lifecycle.
+   *
+   * @param modes the modes applicable to the current account
    */
-  public void reloadModes() {
-    loadModesSync();
+  public void updateModes(List<BuiltInChatMode> modes) {
+    builtInModes = List.copyOf(modes);
+  }
+
+  private BuiltInChatMode convertToBuiltInChatMode(ConversationMode mode) {
+    try {
+      return new BuiltInChatMode(mode);
+    } catch (Exception e) {
+      CopilotCore.LOGGER.error("Failed to convert built-in mode: " + mode.getId(), e);
+      return null;
+    }
   }
 }

com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/UserPreference.java

@@ -12,9 +12,8 @@
 
 
 /**
- * Preferences per GitHub user. All the getters and setters are synchronized due to that the ChatBaseService holds a
- * shared (single) reference to the user preference. synchronized modifies makes sure the update to the instance are
- * thread safe.
+ * Preferences per GitHub user, shared by the chat preference storage. Scalar access is synchronized and model option
+ * maps use immutable snapshots so readers never observe partially updated maps.
  */
 public class UserPreference {