Skip to content

Commit 0c1088f

Browse files
ethanyhouCopilot
ethanyhou
and
Copilot
committed
Address comments.
Co-authored-by: Copilot <copilot@github.com>
1 parent ae95231 commit 0c1088f

4 files changed

Lines changed: 97 additions & 27 deletions

File tree

‎com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionServiceTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.concurrent.CompletableFuture;
1616

1717
import org.eclipse.core.runtime.jobs.Job;
18+
import org.eclipse.jface.preference.IPreferenceStore;
1819
import org.eclipse.ui.IWorkbench;
1920
import org.eclipse.ui.PlatformUI;
2021
import org.junit.jupiter.api.AfterAll;
@@ -24,6 +25,7 @@
2425
import org.mockito.Mockito;
2526

2627
import com.microsoft.copilot.eclipse.core.AuthStatusManager;
28+
import com.microsoft.copilot.eclipse.core.Constants;
2729
import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection;
2830
import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatMode;
2931
import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationAgent;
@@ -51,8 +53,11 @@ static void setUp() {
5153

5254
// Mock CopilotUi.getPlugin() so the constructor can register its preference listener
5355
CopilotUi mockPlugin = mock(CopilotUi.class);
56+
IPreferenceStore mockPreferenceStore = mock(IPreferenceStore.class);
5457
LanguageServerSettingManager mockSettingManager = mock(LanguageServerSettingManager.class);
5558
when(mockPlugin.getLanguageServerSettingManager()).thenReturn(mockSettingManager);
59+
when(mockPlugin.getPreferenceStore()).thenReturn(mockPreferenceStore);
60+
when(mockPreferenceStore.getBoolean(Constants.ENABLE_SKILLS)).thenReturn(true);
5661
copilotUiMock = Mockito.mockStatic(CopilotUi.class);
5762
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
5863

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionService.java‎

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.util.Set;
1111
import java.util.concurrent.ExecutionException;
1212

13+
import org.eclipse.core.resources.IResource;
1314
import org.eclipse.core.resources.IResourceChangeEvent;
1415
import org.eclipse.core.resources.IResourceChangeListener;
1516
import org.eclipse.core.resources.IResourceDelta;
@@ -20,7 +21,6 @@
2021
import org.eclipse.core.runtime.Status;
2122
import org.eclipse.core.runtime.jobs.Job;
2223
import org.eclipse.e4.core.services.events.IEventBroker;
23-
import org.eclipse.jface.util.IPropertyChangeListener;
2424
import org.eclipse.lsp4j.WorkspaceFolder;
2525
import org.eclipse.ui.PlatformUI;
2626
import org.osgi.service.event.EventHandler;
@@ -37,6 +37,7 @@
3737
import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTemplate;
3838
import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope;
3939
import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult;
40+
import com.microsoft.copilot.eclipse.core.lsp.protocol.TemplateSource;
4041
import com.microsoft.copilot.eclipse.core.utils.WorkspaceUtils;
4142
import com.microsoft.copilot.eclipse.ui.CopilotUi;
4243

@@ -57,7 +58,6 @@ public class ChatCompletionService implements CopilotAuthStatusListener {
5758
private CopilotLanguageServerConnection lsConnection;
5859
private AuthStatusManager authStatusManager;
5960
private IResourceChangeListener skillFileListener;
60-
private IPropertyChangeListener preferenceListener;
6161
private IEventBroker eventBroker;
6262
private EventHandler customPromptsChangedHandler;
6363

@@ -71,14 +71,10 @@ public ChatCompletionService(CopilotLanguageServerConnection lsConnection, AuthS
7171
this.authStatusManager = authStatusManager;
7272
this.lsConnection = lsConnection;
7373
this.authStatusManager.addCopilotAuthStatusListener(this);
74+
// TODO: Remove this listener once workspace-root is removed from workspaceFolders in CopilotLanguageClient as CLS
75+
// can watch the project prompt file change directly.
7476
this.skillFileListener = new SkillFileChangeListener();
7577
ResourcesPlugin.getWorkspace().addResourceChangeListener(skillFileListener, IResourceChangeEvent.POST_CHANGE);
76-
this.preferenceListener = event -> {
77-
if (Constants.ENABLE_SKILLS.equals(event.getProperty())) {
78-
fetchAsync();
79-
}
80-
};
81-
CopilotUi.getPlugin().getLanguageServerSettingManager().registerPropertyChangeListener(preferenceListener);
8278
this.eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class);
8379
if (this.eventBroker != null) {
8480
this.customPromptsChangedHandler = event -> fetchAsync();
@@ -94,10 +90,10 @@ private void fetchAsync() {
9490
Job refreshJob = new Job("Refresh slash commands service") {
9591
@Override
9692
protected IStatus run(IProgressMonitor monitor) {
93+
initConversationTemplates(monitor);
9794
if (monitor.isCanceled()) {
9895
return Status.CANCEL_STATUS;
9996
}
100-
initConversationTemplates();
10197
return Status.OK_STATUS;
10298
}
10399

@@ -110,18 +106,25 @@ public boolean belongsTo(Object family) {
110106
refreshJob.schedule();
111107
}
112108

113-
private void initConversationTemplates() {
109+
private void initConversationTemplates(IProgressMonitor monitor) {
114110
List<ConversationTemplate> newTemplates = new ArrayList<>();
115111
List<ConversationAgent> newAgents = new ArrayList<>();
116112
Set<String> newCommands = new HashSet<>();
113+
boolean skillsEnabled = isSkillsEnabled();
117114

118115
// Command: /***
119116
// Pass workspace folders so the language server returns workspace-specific
120117
// prompt files (.prompt.md) and skills (SKILL.md) alongside built-in templates.
121118
try {
122119
List<WorkspaceFolder> workspaceFolders = WorkspaceUtils.listWorkspaceFolders();
123120
ConversationTemplate[] rawTemplates = this.lsConnection.listConversationTemplates(workspaceFolders).get();
121+
if (monitor.isCanceled()) {
122+
return;
123+
}
124124
for (ConversationTemplate template : rawTemplates) {
125+
if (!skillsEnabled && template.source() == TemplateSource.SKILL) {
126+
continue;
127+
}
125128
if (!EXCLUDED_COMMANDS.contains(template.id())) {
126129
newTemplates.add(template);
127130
newCommands.add(TEMPLATE_MARK + template.id());
@@ -131,9 +134,16 @@ private void initConversationTemplates() {
131134
CopilotCore.LOGGER.error(e);
132135
}
133136

137+
if (monitor.isCanceled()) {
138+
return;
139+
}
140+
134141
// Command: @***
135142
try {
136143
ConversationAgent[] rawAgents = this.lsConnection.listConversationAgents().get();
144+
if (monitor.isCanceled()) {
145+
return;
146+
}
137147
for (ConversationAgent agent : rawAgents) {
138148
String agentSlug = agent.getSlug();
139149
// @see ui.chat.ChatView#replaceWorkspaceCommand(String)
@@ -151,13 +161,24 @@ private void initConversationTemplates() {
151161
CopilotCore.LOGGER.error(e);
152162
}
153163

164+
if (monitor.isCanceled()) {
165+
return;
166+
}
167+
154168
// Atomically swap the cached data so readers always see a consistent snapshot.
155169
// Publish immutable snapshots so readers cannot accidentally mutate a live collection.
156170
this.templates = List.copyOf(newTemplates);
157171
this.agents = List.copyOf(newAgents);
158172
this.allCommands = Set.copyOf(newCommands);
159173
}
160174

175+
private boolean isSkillsEnabled() {
176+
CopilotCore plugin = CopilotCore.getPlugin();
177+
FeatureFlags flags = plugin != null ? plugin.getFeatureFlags() : null;
178+
return CopilotUi.getPlugin().getPreferenceStore().getBoolean(Constants.ENABLE_SKILLS)
179+
&& flags != null && flags.isClientPreviewFeatureEnabled();
180+
}
181+
161182
/**
162183
* Returns templates filtered by the scope appropriate for the given chat mode. In Agent mode only {@code agent-panel}
163184
* scoped templates (including skills) are shown; in Ask mode only {@code chat-panel} scoped templates are shown.
@@ -254,7 +275,6 @@ private void syncCommands(String status) {
254275
public void dispose() {
255276
this.authStatusManager.removeCopilotAuthStatusListener(this);
256277
ResourcesPlugin.getWorkspace().removeResourceChangeListener(skillFileListener);
257-
CopilotUi.getPlugin().getLanguageServerSettingManager().unregisterPropertyChangeListener(preferenceListener);
258278
if (this.eventBroker != null && this.customPromptsChangedHandler != null) {
259279
this.eventBroker.unsubscribe(this.customPromptsChangedHandler);
260280
}
@@ -280,8 +300,10 @@ public void resourceChanged(IResourceChangeEvent event) {
280300
if (needsRefresh[0]) {
281301
return false;
282302
}
283-
String name = childDelta.getResource().getName();
284-
if (SKILL_FILE_NAME.equals(name) || name.endsWith(PROMPT_FILE_SUFFIX)) {
303+
if (!shouldVisitDelta(childDelta)) {
304+
return false;
305+
}
306+
if (isPromptOrSkillFileDelta(childDelta)) {
285307
needsRefresh[0] = true;
286308
return false;
287309
}
@@ -294,5 +316,28 @@ public void resourceChanged(IResourceChangeEvent event) {
294316
fetchAsync();
295317
}
296318
}
319+
320+
private boolean shouldVisitDelta(IResourceDelta delta) {
321+
IResource resource = delta.getResource();
322+
return resource != null && !resource.isDerived() && !resource.isTeamPrivateMember();
323+
}
324+
325+
private boolean isPromptOrSkillFileDelta(IResourceDelta delta) {
326+
IResource resource = delta.getResource();
327+
if (resource.getType() != IResource.FILE || !isRelevantFileDelta(delta)) {
328+
return false;
329+
}
330+
331+
String name = resource.getName();
332+
return SKILL_FILE_NAME.equals(name) || name.endsWith(PROMPT_FILE_SUFFIX);
333+
}
334+
335+
private boolean isRelevantFileDelta(IResourceDelta delta) {
336+
int kind = delta.getKind();
337+
if (kind == IResourceDelta.ADDED || kind == IResourceDelta.REMOVED) {
338+
return true;
339+
}
340+
return kind == IResourceDelta.CHANGED && (delta.getFlags() & IResourceDelta.CONTENT) != 0;
341+
}
297342
}
298343
}

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/ChatPreferencesPage.java‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,18 @@ public void createFieldEditors() {
8383
addNote(parent, Messages.preferences_page_sub_agent_note_content);
8484
addSeparator(parent);
8585

86-
// Add Enable Skills toggle
87-
Composite skillsComposite = createSectionComposite(parent, gdf);
86+
if (isClientPreviewFeatureEnabled()) {
87+
// Add Enable Skills toggle
88+
Composite skillsComposite = createSectionComposite(parent, gdf);
8889

89-
BooleanFieldEditor skillsField = new BooleanFieldEditor(Constants.ENABLE_SKILLS,
90-
Messages.preferences_page_skills_enabled, SWT.WRAP, skillsComposite);
91-
applyFieldWidthHint(skillsField, skillsComposite);
92-
addField(skillsField);
90+
BooleanFieldEditor skillsField = new BooleanFieldEditor(Constants.ENABLE_SKILLS,
91+
Messages.preferences_page_skills_enabled, SWT.WRAP, skillsComposite);
92+
applyFieldWidthHint(skillsField, skillsComposite);
93+
addField(skillsField);
9394

94-
addNote(parent, Messages.preferences_page_skills_enabled_note_content);
95-
addSeparator(parent);
95+
addNote(parent, Messages.preferences_page_skills_enabled_note_content);
96+
addSeparator(parent);
97+
}
9698

9799
// Add Agent Max Requests field
98100
Composite agentMaxRequestsComposite = createSectionComposite(parent, gdf);
@@ -196,7 +198,12 @@ private void addSeparator(Composite parent) {
196198

197199
private boolean isPolicyAllowsSubAgent() {
198200
FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags();
199-
return flags != null && flags.isClientPreviewFeatureEnabled() && flags.isSubAgentPolicyEnabled();
201+
return isClientPreviewFeatureEnabled() && flags != null && flags.isSubAgentPolicyEnabled();
202+
}
203+
204+
private boolean isClientPreviewFeatureEnabled() {
205+
FeatureFlags flags = CopilotCore.getPlugin().getFeatureFlags();
206+
return flags != null && flags.isClientPreviewFeatureEnabled();
200207
}
201208

202209
/**

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/LanguageServerSettingManager.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import com.microsoft.copilot.eclipse.core.Constants;
2727
import com.microsoft.copilot.eclipse.core.CopilotCore;
28+
import com.microsoft.copilot.eclipse.core.FeatureFlags;
2829
import com.microsoft.copilot.eclipse.core.chat.CustomChatModeManager;
2930
import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants;
3031
import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection;
@@ -50,6 +51,7 @@ public class LanguageServerSettingManager implements IProxyChangeListener, IProp
5051
CopilotLanguageServerConnection copilotLanguageServerConnection = null;
5152
IPreferenceStore preferenceStore;
5253
IProxyData proxyData = null;
54+
private IEventBroker eventBroker;
5355

5456
/**
5557
* Gets the settings.
@@ -82,8 +84,7 @@ public LanguageServerSettingManager(CopilotLanguageServerConnection conn, IProxy
8284
// agent related settings
8385
getSettings().getGithubSettings().getCopilotSettings().getAgent()
8486
.setAgentMaxRequests(preferenceStore.getInt(Constants.AGENT_MAX_REQUESTS));
85-
getSettings().getGithubSettings().getCopilotSettings().getAgent()
86-
.setEnableSkills(preferenceStore.getBoolean(Constants.ENABLE_SKILLS));
87+
getSettings().getGithubSettings().getCopilotSettings().getAgent().setEnableSkills(isSkillsEnabled());
8788

8889
// Set workspace context instructions when it is enabled
8990
if (preferenceStore.getBoolean(Constants.CUSTOM_INSTRUCTIONS_WORKSPACE_ENABLED)) {
@@ -97,7 +98,7 @@ public LanguageServerSettingManager(CopilotLanguageServerConnection conn, IProxy
9798
getSettings().getGithubSettings()
9899
.setGitCommitCopilotInstructions(preferenceStore.getString(Constants.CUSTOM_INSTRUCTIONS_GIT_COMMIT));
99100

100-
IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class);
101+
eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class);
101102
eventBroker.subscribe(CopilotEventConstants.TOPIC_DID_CHANGE_MCP_CONTRIBUTION_POINT_POLICY, event -> {
102103
Boolean enabled = (Boolean) event.getProperty(IEventBroker.DATA);
103104
if (!enabled.booleanValue()) {
@@ -171,15 +172,20 @@ public void propertyChange(PropertyChangeEvent event) {
171172
singleSetting = new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings());
172173
break;
173174
case Constants.ENABLE_SKILLS:
174-
settings.getGithubSettings().getCopilotSettings().getAgent()
175-
.setEnableSkills(preferenceStore.getBoolean(Constants.ENABLE_SKILLS));
175+
settings.getGithubSettings().getCopilotSettings().getAgent().setEnableSkills(isSkillsEnabled());
176176
singleSetting = new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings());
177177
break;
178178
default:
179179
return;
180180
}
181181

182182
syncSingleConfiguration(singleSetting);
183+
184+
if (Constants.ENABLE_SKILLS.equals(event.getProperty())) {
185+
if (eventBroker != null) {
186+
eventBroker.post(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES, null);
187+
}
188+
}
183189
}
184190

185191
/**
@@ -516,6 +522,13 @@ public boolean isAutoShowCompletionEnabled() {
516522
return preferenceStore.getBoolean(Constants.AUTO_SHOW_COMPLETION);
517523
}
518524

525+
private boolean isSkillsEnabled() {
526+
CopilotCore plugin = CopilotCore.getPlugin();
527+
FeatureFlags flags = plugin != null ? plugin.getFeatureFlags() : null;
528+
return preferenceStore.getBoolean(Constants.ENABLE_SKILLS) && flags != null
529+
&& flags.isClientPreviewFeatureEnabled();
530+
}
531+
519532
/**
520533
* Enable or disable auto show completions.
521534
*/

0 commit comments

Comments
 (0)